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) {