Add some CBB-based functions for crypto/x509 and crypto/asn1 types
For now, they forward to the i2d/i2c functions instead of being
CBB-based themselves. I've added a tag parameter to the crypto/asn1 ones
in anticipation of them being used for implicit tagging, once the rest
of the legacy parser becomes CBB-based.
But we already have enough things that cross the line that this seems
worthwhile.
Bug: 42290417
Change-Id: Ide588fe04d3d5862ca003ab94b068c009eadf2b1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/78448
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
diff --git a/crypto/asn1/a_bitstr.cc b/crypto/asn1/a_bitstr.cc
index 82df5c8..6bdd726 100644
--- a/crypto/asn1/a_bitstr.cc
+++ b/crypto/asn1/a_bitstr.cc
@@ -17,6 +17,7 @@
#include <limits.h>
#include <string.h>
+#include <openssl/bytestring.h>
#include <openssl/err.h>
#include <openssl/mem.h>
@@ -94,6 +95,21 @@
return ret;
}
+int asn1_marshal_bit_string(CBB *out, const ASN1_BIT_STRING *in,
+ CBS_ASN1_TAG tag) {
+ int len = i2c_ASN1_BIT_STRING(in, nullptr);
+ if (len <= 0) {
+ return 0;
+ }
+ tag = tag == 0 ? CBS_ASN1_BITSTRING : tag;
+ CBB child;
+ uint8_t *ptr;
+ return CBB_add_asn1(out, &child, tag) && //
+ CBB_add_space(&child, &ptr, static_cast<size_t>(len)) && //
+ i2c_ASN1_BIT_STRING(in, &ptr) == len && //
+ CBB_flush(out);
+}
+
ASN1_BIT_STRING *c2i_ASN1_BIT_STRING(ASN1_BIT_STRING **a,
const unsigned char **pp, long len) {
ASN1_BIT_STRING *ret = NULL;
diff --git a/crypto/asn1/a_int.cc b/crypto/asn1/a_int.cc
index 7b0e28b5..e641705 100644
--- a/crypto/asn1/a_int.cc
+++ b/crypto/asn1/a_int.cc
@@ -23,6 +23,7 @@
#include <openssl/mem.h>
#include "../internal.h"
+#include "internal.h"
ASN1_INTEGER *ASN1_INTEGER_dup(const ASN1_INTEGER *x) {
@@ -72,6 +73,20 @@
return 1;
}
+int asn1_marshal_integer(CBB *out, const ASN1_INTEGER *in, CBS_ASN1_TAG tag) {
+ int len = i2c_ASN1_INTEGER(in, nullptr);
+ if (len <= 0) {
+ return 0;
+ }
+ tag = tag == 0 ? CBS_ASN1_INTEGER : tag;
+ CBB child;
+ uint8_t *ptr;
+ return CBB_add_asn1(out, &child, tag) && //
+ CBB_add_space(&child, &ptr, static_cast<size_t>(len)) && //
+ i2c_ASN1_INTEGER(in, &ptr) == len && //
+ CBB_flush(out);
+}
+
int i2c_ASN1_INTEGER(const ASN1_INTEGER *in, unsigned char **outp) {
if (in == NULL) {
return 0;
diff --git a/crypto/asn1/internal.h b/crypto/asn1/internal.h
index 8d863fc..2baeb12 100644
--- a/crypto/asn1/internal.h
+++ b/crypto/asn1/internal.h
@@ -180,6 +180,17 @@
int asn1_bit_string_length(const ASN1_BIT_STRING *str,
uint8_t *out_padding_bits);
+// asn1_marshal_bit_string marshals |in| as a DER-encoded, ASN.1 BIT STRING and
+// writes the result to |out|. It returns one on success and zero on error. If
+// |tag| is non-zero, the tag is replaced with |tag|.
+int asn1_marshal_bit_string(CBB *out, const ASN1_BIT_STRING *in,
+ CBS_ASN1_TAG tag);
+
+// asn1_marshal_integer marshals |in| as a DER-encoded, ASN.1 INTEGER and writes
+// the result to |out|. It returns one on success and zero on error. If |tag| is
+// non-zero, the tag is replaced with |tag|.
+int asn1_marshal_integer(CBB *out, const ASN1_INTEGER *in, CBS_ASN1_TAG tag);
+
typedef struct {
int nid;
long minsize;
diff --git a/crypto/pkcs7/pkcs7_x509.cc b/crypto/pkcs7/pkcs7_x509.cc
index 8484c38..aabc9a2 100644
--- a/crypto/pkcs7/pkcs7_x509.cc
+++ b/crypto/pkcs7/pkcs7_x509.cc
@@ -26,6 +26,8 @@
#include <openssl/stack.h>
#include <openssl/x509.h>
+#include "../asn1/internal.h"
+#include "../x509/internal.h"
#include "../internal.h"
#include "internal.h"
@@ -431,24 +433,16 @@
const struct signer_info_data *const si_data =
reinterpret_cast<const struct signer_info_data *>(arg);
- int ret = 0;
- uint8_t *subject_bytes = NULL;
- uint8_t *serial_bytes = NULL;
-
- const int subject_len =
- i2d_X509_NAME(X509_get_subject_name(si_data->sign_cert), &subject_bytes);
- const int serial_len = i2d_ASN1_INTEGER(
- (ASN1_INTEGER *)X509_get0_serialNumber(si_data->sign_cert),
- &serial_bytes);
-
CBB seq, issuer_and_serial, signing_algo, null, signature;
- if (subject_len < 0 || serial_len < 0 ||
- !CBB_add_asn1(out, &seq, CBS_ASN1_SEQUENCE) ||
+ if (!CBB_add_asn1(out, &seq, CBS_ASN1_SEQUENCE) ||
// version
!CBB_add_asn1_uint64(&seq, 1) ||
!CBB_add_asn1(&seq, &issuer_and_serial, CBS_ASN1_SEQUENCE) ||
- !CBB_add_bytes(&issuer_and_serial, subject_bytes, subject_len) ||
- !CBB_add_bytes(&issuer_and_serial, serial_bytes, serial_len) ||
+ !x509_marshal_name(&issuer_and_serial,
+ X509_get_subject_name(si_data->sign_cert)) ||
+ !asn1_marshal_integer(&issuer_and_serial,
+ X509_get0_serialNumber(si_data->sign_cert),
+ /*tag=*/0) ||
!write_sha256_ai(&seq, NULL) ||
!CBB_add_asn1(&seq, &signing_algo, CBS_ASN1_SEQUENCE) ||
!OBJ_nid2cbb(&signing_algo, NID_rsaEncryption) ||
@@ -456,15 +450,10 @@
!CBB_add_asn1(&seq, &signature, CBS_ASN1_OCTETSTRING) ||
!CBB_add_bytes(&signature, si_data->signature, si_data->signature_len) ||
!CBB_flush(out)) {
- goto out;
+ return 0;
}
- ret = 1;
-
-out:
- OPENSSL_free(subject_bytes);
- OPENSSL_free(serial_bytes);
- return ret;
+ return 1;
}
PKCS7 *PKCS7_sign(X509 *sign_cert, EVP_PKEY *pkey, STACK_OF(X509) *certs,
diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index 1794d16..1fab446 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h
@@ -547,6 +547,18 @@
// TODO(https://crbug.com/boringssl/695): Remove this.
int DIST_POINT_set_dpname(DIST_POINT_NAME *dpn, X509_NAME *iname);
+// x509_marshal_name marshals |in| as a DER-encoded, X.509 Name and writes the
+// result to |out|. It returns one on success and zero on error.
+//
+// TODO(https://crbug.com/boringssl/407): This function should be const and
+// thread-safe but is currently neither in some cases, notably if |in| was
+// mutated.
+int x509_marshal_name(CBB *out, X509_NAME *in);
+
+// x509_marshal_algorithm marshals |in| as a DER-encoded, AlgorithmIdentifier
+// and writes the result to |out|. It returns one on success and zero on error.
+int x509_marshal_algorithm(CBB *out, const X509_ALGOR *in);
+
#if defined(__cplusplus)
} // extern C
diff --git a/crypto/x509/x_algor.cc b/crypto/x509/x_algor.cc
index 0a1162e..60eb006 100644
--- a/crypto/x509/x_algor.cc
+++ b/crypto/x509/x_algor.cc
@@ -20,6 +20,7 @@
#include <openssl/obj.h>
#include "../asn1/internal.h"
+#include "internal.h"
ASN1_SEQUENCE(X509_ALGOR) = {
@@ -105,3 +106,11 @@
}
return ASN1_TYPE_cmp(a->parameter, b->parameter);
}
+
+int x509_marshal_algorithm(CBB *out, const X509_ALGOR *in) {
+ uint8_t *ptr;
+ int len = i2d_X509_ALGOR(in, NULL);
+ return len > 0 && //
+ CBB_add_space(out, &ptr, static_cast<size_t>(len)) &&
+ i2d_X509_ALGOR(in, &ptr) == len;
+}
diff --git a/crypto/x509/x_name.cc b/crypto/x509/x_name.cc
index 16e60e4..035a03a 100644
--- a/crypto/x509/x_name.cc
+++ b/crypto/x509/x_name.cc
@@ -490,3 +490,12 @@
}
return 1;
}
+
+int x509_marshal_name(CBB *out, X509_NAME *in) {
+ int len = i2d_X509_NAME(in, nullptr);
+ if (len <= 0) {
+ return 0;
+ }
+ uint8_t *ptr;
+ return CBB_add_space(out, &ptr, len) && i2d_X509_NAME(in, &ptr) == len;
+}
diff --git a/crypto/x509/x_x509.cc b/crypto/x509/x_x509.cc
index ebb20d5..098c757 100644
--- a/crypto/x509/x_x509.cc
+++ b/crypto/x509/x_x509.cc
@@ -223,42 +223,26 @@
return -1;
}
- CBB cbb, cert;
- int len;
- if (!CBB_init(&cbb, 64) || //
- !CBB_add_asn1(&cbb, &cert, CBS_ASN1_SEQUENCE)) {
- goto err;
+ bssl::ScopedCBB cbb;
+ CBB cert;
+ if (!CBB_init(cbb.get(), 64) || //
+ !CBB_add_asn1(cbb.get(), &cert, CBS_ASN1_SEQUENCE)) {
+ return -1;
}
// TODO(crbug.com/boringssl/443): When the rest of the library is decoupled
// from the tasn_*.c implementation, replace this with |CBS|-based functions.
uint8_t *out;
- len = i2d_X509_CINF(x509->cert_info, NULL);
+ int len = i2d_X509_CINF(x509->cert_info, NULL);
if (len < 0 || //
- !CBB_add_space(&cert, &out, (size_t)len) ||
- i2d_X509_CINF(x509->cert_info, &out) != len) {
- goto err;
+ !CBB_add_space(&cert, &out, static_cast<size_t>(len)) ||
+ i2d_X509_CINF(x509->cert_info, &out) != len ||
+ !x509_marshal_algorithm(&cert, x509->sig_alg) ||
+ !asn1_marshal_bit_string(&cert, x509->signature, /*tag=*/0)) {
+ return -1;
}
- len = i2d_X509_ALGOR(x509->sig_alg, NULL);
- if (len < 0 || //
- !CBB_add_space(&cert, &out, (size_t)len) ||
- i2d_X509_ALGOR(x509->sig_alg, &out) != len) {
- goto err;
- }
-
- len = i2d_ASN1_BIT_STRING(x509->signature, NULL);
- if (len < 0 || //
- !CBB_add_space(&cert, &out, (size_t)len) ||
- i2d_ASN1_BIT_STRING(x509->signature, &out) != len) {
- goto err;
- }
-
- return CBB_finish_i2d(&cbb, outp);
-
-err:
- CBB_cleanup(&cbb);
- return -1;
+ return CBB_finish_i2d(cbb.get(), outp);
}
static int x509_new_cb(ASN1_VALUE **pval, const ASN1_ITEM *it) {
