Embed X509_NAME into X509
Save a couple small allocations, and also avoid x509_new_null.
Bug: 42290417
Change-Id: I0b4f8b9024e0c32f56008be90b792d8224a58735
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81894
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index f5d2dc4..4310089 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h
@@ -112,14 +112,10 @@
uint8_t version; // One of the |X509_VERSION_*| constants.
ASN1_INTEGER serialNumber;
X509_ALGOR tbs_sig_alg;
- // TODO(crbug.com/42290417): When |X509_NAME| no longer uses the macro system,
- // try to embed this struct.
- X509_NAME *issuer;
+ X509_NAME issuer;
ASN1_TIME notBefore;
ASN1_TIME notAfter;
- // TODO(crbug.com/42290417): When |X509_NAME| no longer uses the macro system,
- // try to embed this struct.
- X509_NAME *subject;
+ X509_NAME subject;
X509_PUBKEY key;
ASN1_BIT_STRING *issuerUID; // [ 1 ] optional in v2
ASN1_BIT_STRING *subjectUID; // [ 2 ] optional in v2
@@ -582,6 +578,8 @@
const X509_NAME_CACHE *x509_name_get_cache(const X509_NAME *name);
void x509_name_invalidate_cache(X509_NAME *name);
+int x509_name_copy(X509_NAME *dst, const X509_NAME *src);
+
void x509_algor_init(X509_ALGOR *alg);
void x509_algor_cleanup(X509_ALGOR *alg);
diff --git a/crypto/x509/x509_cmp.cc b/crypto/x509/x509_cmp.cc
index e0a4b2a..5bd407a 100644
--- a/crypto/x509/x509_cmp.cc
+++ b/crypto/x509/x509_cmp.cc
@@ -29,11 +29,11 @@
int X509_issuer_name_cmp(const X509 *a, const X509 *b) {
- return X509_NAME_cmp(a->issuer, b->issuer);
+ return X509_NAME_cmp(&a->issuer, &b->issuer);
}
int X509_subject_name_cmp(const X509 *a, const X509 *b) {
- return X509_NAME_cmp(a->subject, b->subject);
+ return X509_NAME_cmp(&a->subject, &b->subject);
}
int X509_CRL_cmp(const X509_CRL *a, const X509_CRL *b) {
@@ -46,20 +46,20 @@
X509_NAME *X509_get_issuer_name(const X509 *a) {
// This function is not const-correct for OpenSSL compatibility.
- return a->issuer;
+ return const_cast<X509_NAME*>(&a->issuer);
}
uint32_t X509_issuer_name_hash(const X509 *x) {
- return X509_NAME_hash(x->issuer);
+ return X509_NAME_hash(&x->issuer);
}
uint32_t X509_issuer_name_hash_old(const X509 *x) {
- return X509_NAME_hash_old(x->issuer);
+ return X509_NAME_hash_old(&x->issuer);
}
X509_NAME *X509_get_subject_name(const X509 *a) {
// This function is not const-correct for OpenSSL compatibility.
- return a->subject;
+ return const_cast<X509_NAME*>(&a->subject);
}
ASN1_INTEGER *X509_get_serialNumber(X509 *a) { return &a->serialNumber; }
@@ -69,11 +69,11 @@
}
uint32_t X509_subject_name_hash(const X509 *x) {
- return X509_NAME_hash(x->subject);
+ return X509_NAME_hash(&x->subject);
}
uint32_t X509_subject_name_hash_old(const X509 *x) {
- return X509_NAME_hash_old(x->subject);
+ return X509_NAME_hash_old(&x->subject);
}
// Compare two certificates: they must be identical for this to work. NB:
diff --git a/crypto/x509/x509_set.cc b/crypto/x509/x509_set.cc
index 0da8cc5..12953a4 100644
--- a/crypto/x509/x509_set.cc
+++ b/crypto/x509/x509_set.cc
@@ -47,17 +47,17 @@
}
int X509_set_issuer_name(X509 *x, const X509_NAME *name) {
- if (x == NULL) {
+ if (x == nullptr) {
return 0;
}
- return (X509_NAME_set(&x->issuer, name));
+ return x509_name_copy(&x->issuer, name);
}
int X509_set_subject_name(X509 *x, const X509_NAME *name) {
- if (x == NULL) {
+ if (x == nullptr) {
return 0;
}
- return (X509_NAME_set(&x->subject, name));
+ return x509_name_copy(&x->subject, name);
}
int X509_set1_notBefore(X509 *x, const ASN1_TIME *tm) {
diff --git a/crypto/x509/x_name.cc b/crypto/x509/x_name.cc
index 973741a..54493cb 100644
--- a/crypto/x509/x_name.cc
+++ b/crypto/x509/x_name.cc
@@ -263,18 +263,25 @@
return CBB_add_bytes(out, cache->der, cache->der_len);
}
-X509_NAME *X509_NAME_dup(const X509_NAME *name) {
- const X509_NAME_CACHE *cache = x509_name_get_cache(name);
+int x509_name_copy(X509_NAME *dst, const X509_NAME *src) {
+ const X509_NAME_CACHE *cache = x509_name_get_cache(src);
if (cache == nullptr) {
- return nullptr;
+ return 0;
}
CBS cbs;
CBS_init(&cbs, cache->der, cache->der_len);
- bssl::UniquePtr<X509_NAME> copy(X509_NAME_new());
- if (copy == nullptr || !x509_parse_name(&cbs, copy.get())) {
- return nullptr;
+ if (!x509_parse_name(&cbs, dst)) {
+ return 0;
}
assert(CBS_len(&cbs) == 0);
+ return 1;
+}
+
+X509_NAME *X509_NAME_dup(const X509_NAME *name) {
+ bssl::UniquePtr<X509_NAME> copy(X509_NAME_new());
+ if (copy == nullptr || !x509_name_copy(copy.get(), name)) {
+ return nullptr;
+ }
return copy.release();
}
diff --git a/crypto/x509/x_x509.cc b/crypto/x509/x_x509.cc
index c9cc309..26c39fd 100644
--- a/crypto/x509/x_x509.cc
+++ b/crypto/x509/x_x509.cc
@@ -40,9 +40,7 @@
static constexpr CBS_ASN1_TAG kExtensionsTag =
CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 3;
-// x509_new_null returns a new |X509| object where the |issuer| and |subject|
-// fields are not yet filled in.
-static bssl::UniquePtr<X509> x509_new_null(void) {
+X509 *X509_new(void) {
bssl::UniquePtr<X509> ret(
reinterpret_cast<X509 *>(OPENSSL_zalloc(sizeof(X509))));
if (ret == nullptr) {
@@ -54,29 +52,15 @@
ret->version = X509_VERSION_1;
asn1_string_init(&ret->serialNumber, V_ASN1_INTEGER);
x509_algor_init(&ret->tbs_sig_alg);
+ x509_name_init(&ret->issuer);
asn1_string_init(&ret->notBefore, -1);
asn1_string_init(&ret->notAfter, -1);
+ x509_name_init(&ret->subject);
x509_pubkey_init(&ret->key);
x509_algor_init(&ret->sig_alg);
asn1_string_init(&ret->signature, V_ASN1_BIT_STRING);
CRYPTO_new_ex_data(&ret->ex_data);
CRYPTO_MUTEX_init(&ret->lock);
- return ret;
-}
-
-X509 *X509_new(void) {
- bssl::UniquePtr<X509> ret = x509_new_null();
- if (ret == nullptr) {
- return nullptr;
- }
- // TODO(crbug.com/42290417): When the |X509_NAME| parser is CBS-based and
- // writes into a pre-existing |X509_NAME|, we will no longer need the
- // |X509_new| and |x509_new_null| split.
- ret->issuer = X509_NAME_new();
- ret->subject = X509_NAME_new();
- if (ret->issuer == nullptr || ret->subject == nullptr) {
- return nullptr;
- }
return ret.release();
}
@@ -89,10 +73,10 @@
asn1_string_cleanup(&x509->serialNumber);
x509_algor_cleanup(&x509->tbs_sig_alg);
- X509_NAME_free(x509->issuer);
+ x509_name_cleanup(&x509->issuer);
asn1_string_cleanup(&x509->notBefore);
asn1_string_cleanup(&x509->notAfter);
- X509_NAME_free(x509->subject);
+ x509_name_cleanup(&x509->subject);
x509_pubkey_cleanup(&x509->key);
ASN1_BIT_STRING_free(x509->issuerUID);
ASN1_BIT_STRING_free(x509->subjectUID);
@@ -111,23 +95,10 @@
OPENSSL_free(x509);
}
-static int parse_name(CBS *cbs, X509_NAME **out) {
- // TODO(crbug.com/42290417): Make the |X509_NAME| parser CBS-based and avoid
- // this awkward conversion.
- const uint8_t *p = CBS_data(cbs);
- X509_NAME_free(*out);
- *out = d2i_X509_NAME(nullptr, &p, CBS_len(cbs));
- if (*out == nullptr) {
- return 0;
- }
- BSSL_CHECK(CBS_skip(cbs, p - CBS_data(cbs)));
- return 1;
-}
-
X509 *X509_parse_with_algorithms(CRYPTO_BUFFER *buf,
const EVP_PKEY_ALG *const *algs,
size_t num_algs) {
- bssl::UniquePtr<X509> ret(x509_new_null());
+ bssl::UniquePtr<X509> ret(X509_new());
if (ret == nullptr) {
return nullptr;
}
@@ -181,14 +152,14 @@
CBS validity;
if (!asn1_parse_integer(&tbs, &ret->serialNumber, /*tag=*/0) ||
!x509_parse_algorithm(&tbs, &ret->tbs_sig_alg) ||
- !parse_name(&tbs, &ret->issuer) ||
+ !x509_parse_name(&tbs, &ret->issuer) ||
!CBS_get_asn1(&tbs, &validity, CBS_ASN1_SEQUENCE) ||
!asn1_parse_time(&validity, &ret->notBefore,
/*allow_utc_timezone_offset=*/1) ||
!asn1_parse_time(&validity, &ret->notAfter,
/*allow_utc_timezone_offset=*/1) ||
CBS_len(&validity) != 0 || //
- !parse_name(&tbs, &ret->subject) ||
+ !x509_parse_name(&tbs, &ret->subject) ||
!x509_parse_public_key(&tbs, &ret->key, bssl::Span(algs, num_algs))) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_DECODE_ERROR);
return nullptr;
@@ -285,11 +256,11 @@
}
if (!asn1_marshal_integer(&tbs, &x509->serialNumber, /*tag=*/0) ||
!x509_marshal_algorithm(&tbs, &x509->tbs_sig_alg) ||
- !x509_marshal_name(&tbs, x509->issuer) ||
+ !x509_marshal_name(&tbs, &x509->issuer) ||
!CBB_add_asn1(&tbs, &validity, CBS_ASN1_SEQUENCE) ||
!asn1_marshal_time(&validity, &x509->notBefore) ||
!asn1_marshal_time(&validity, &x509->notAfter) ||
- !x509_marshal_name(&tbs, x509->subject) ||
+ !x509_marshal_name(&tbs, &x509->subject) ||
!x509_marshal_public_key(&tbs, &x509->key) ||
(x509->issuerUID != nullptr &&
!asn1_marshal_bit_string(&tbs, x509->issuerUID, kIssuerUIDTag)) ||
