From 94a608a1f5d4339d95252902a9e9381c2cc1c225 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 14 Jul 2021 13:46:27 -0400 Subject: [PATCH] Make X509_EXTENSION opaque. I've switched a few things to the accessors where it was easy, but X509_EXTENSION is, in us and upstream, not const-correct right now, so it's a little goofy. Update-Note: Use X509_EXTENSION_get_* instead. Change-Id: Ife9636051a924a950b1c739b7720baf12e35f9c7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48505 Reviewed-by: Adam Langley --- crypto/x509/internal.h | 6 ++++++ crypto/x509/x509_v3.c | 28 +++++++++++++++++----------- crypto/x509/x_crl.c | 15 ++++++++------- crypto/x509/x_exten.c | 2 ++ crypto/x509v3/v3_lib.c | 2 ++ crypto/x509v3/v3_prn.c | 25 ++++++++++++++----------- include/openssl/x509.h | 8 +------- 7 files changed, 50 insertions(+), 36 deletions(-) diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h index 8c37985f9..2782f2386 100644 --- a/crypto/x509/internal.h +++ b/crypto/x509/internal.h @@ -94,6 +94,12 @@ struct x509_cert_aux_st { STACK_OF(X509_ALGOR) *other; // other unspecified info } /* X509_CERT_AUX */; +struct X509_extension_st { + ASN1_OBJECT *object; + ASN1_BOOLEAN critical; + ASN1_OCTET_STRING *value; +} /* X509_EXTENSION */; + typedef struct { ASN1_ENCODING enc; ASN1_INTEGER *version; diff --git a/crypto/x509/x509_v3.c b/crypto/x509/x509_v3.c index 00d63992f..985161d54 100644 --- a/crypto/x509/x509_v3.c +++ b/crypto/x509/x509_v3.c @@ -62,6 +62,9 @@ #include #include +#include "internal.h" + + int X509v3_get_ext_count(const STACK_OF(X509_EXTENSION) *x) { if (x == NULL) @@ -102,21 +105,24 @@ int X509v3_get_ext_by_OBJ(const STACK_OF(X509_EXTENSION) *sk, int X509v3_get_ext_by_critical(const STACK_OF(X509_EXTENSION) *sk, int crit, int lastpos) { - int n; - X509_EXTENSION *ex; + if (sk == NULL) { + return -1; + } - if (sk == NULL) - return (-1); lastpos++; - if (lastpos < 0) + if (lastpos < 0) { lastpos = 0; - n = sk_X509_EXTENSION_num(sk); + } + + crit = !!crit; + int n = sk_X509_EXTENSION_num(sk); for (; lastpos < n; lastpos++) { - ex = sk_X509_EXTENSION_value(sk, lastpos); - if (((ex->critical > 0) && crit) || ((ex->critical <= 0) && !crit)) - return (lastpos); + const X509_EXTENSION *ex = sk_X509_EXTENSION_value(sk, lastpos); + if (X509_EXTENSION_get_critical(ex) == crit) { + return lastpos; + } } - return (-1); + return -1; } X509_EXTENSION *X509v3_get_ext(const STACK_OF(X509_EXTENSION) *x, int loc) @@ -265,7 +271,7 @@ ASN1_OCTET_STRING *X509_EXTENSION_get_data(X509_EXTENSION *ex) return (ex->value); } -int X509_EXTENSION_get_critical(X509_EXTENSION *ex) +int X509_EXTENSION_get_critical(const X509_EXTENSION *ex) { if (ex == NULL) return (0); diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c index f6fbd0a9f..0d419ac1d 100644 --- a/crypto/x509/x_crl.c +++ b/crypto/x509/x_crl.c @@ -205,11 +205,12 @@ static int crl_set_issuers(X509_CRL *crl) for (k = 0; k < sk_X509_EXTENSION_num(exts); k++) { ext = sk_X509_EXTENSION_value(exts, k); - if (ext->critical > 0) { - if (OBJ_obj2nid(ext->object) == NID_certificate_issuer) - continue; - crl->flags |= EXFLAG_CRITICAL; - break; + if (X509_EXTENSION_get_critical(ext)) { + if (OBJ_obj2nid(X509_EXTENSION_get_object(ext)) == + NID_certificate_issuer) + continue; + crl->flags |= EXFLAG_CRITICAL; + break; } } @@ -298,10 +299,10 @@ static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it, for (idx = 0; idx < sk_X509_EXTENSION_num(exts); idx++) { int nid; ext = sk_X509_EXTENSION_value(exts, idx); - nid = OBJ_obj2nid(ext->object); + nid = OBJ_obj2nid(X509_EXTENSION_get_object(ext)); if (nid == NID_freshest_crl) crl->flags |= EXFLAG_FRESHEST; - if (ext->critical > 0) { + if (X509_EXTENSION_get_critical(ext)) { /* We handle IDP and deltas */ if ((nid == NID_issuing_distribution_point) || (nid == NID_authority_key_identifier) diff --git a/crypto/x509/x_exten.c b/crypto/x509/x_exten.c index 36403e488..89998ca14 100644 --- a/crypto/x509/x_exten.c +++ b/crypto/x509/x_exten.c @@ -59,6 +59,8 @@ #include #include +#include "internal.h" + ASN1_SEQUENCE(X509_EXTENSION) = { ASN1_SIMPLE(X509_EXTENSION, object, ASN1_OBJECT), diff --git a/crypto/x509v3/v3_lib.c b/crypto/x509v3/v3_lib.c index d89733fda..3fb0285b8 100644 --- a/crypto/x509v3/v3_lib.c +++ b/crypto/x509v3/v3_lib.c @@ -66,6 +66,8 @@ #include #include +#include "../x509/internal.h" + #include "ext_dat.h" static STACK_OF(X509V3_EXT_METHOD) *ext_list = NULL; diff --git a/crypto/x509v3/v3_prn.c b/crypto/x509v3/v3_prn.c index f6f341a22..79e96e680 100644 --- a/crypto/x509v3/v3_prn.c +++ b/crypto/x509v3/v3_prn.c @@ -107,20 +107,20 @@ int X509V3_EXT_print(BIO *out, X509_EXTENSION *ext, unsigned long flag, { void *ext_str = NULL; char *value = NULL; - const unsigned char *p; const X509V3_EXT_METHOD *method; STACK_OF(CONF_VALUE) *nval = NULL; int ok = 1; if (!(method = X509V3_EXT_get(ext))) return unknown_ext_print(out, ext, flag, indent, 0); - p = ext->value->data; - if (method->it) - ext_str = - ASN1_item_d2i(NULL, &p, ext->value->length, - ASN1_ITEM_ptr(method->it)); - else - ext_str = method->d2i(NULL, &p, ext->value->length); + const ASN1_STRING *ext_data = X509_EXTENSION_get_data(ext); + const unsigned char *p = ASN1_STRING_get0_data(ext_data); + if (method->it) { + ext_str = ASN1_item_d2i(NULL, &p, ASN1_STRING_length(ext_data), + ASN1_ITEM_ptr(method->it)); + } else { + ext_str = method->d2i(NULL, &p, ASN1_STRING_length(ext_data)); + } if (!ext_str) return unknown_ext_print(out, ext, flag, indent, 1); @@ -183,7 +183,7 @@ int X509V3_extensions_print(BIO *bp, const char *title, return 0; if (!X509V3_EXT_print(bp, ex, flag, indent + 4)) { BIO_printf(bp, "%*s", indent + 4, ""); - ASN1_STRING_print(bp, ex->value); + ASN1_STRING_print(bp, X509_EXTENSION_get_data(ex)); } if (BIO_write(bp, "\n", 1) <= 0) return 0; @@ -207,8 +207,11 @@ static int unknown_ext_print(BIO *out, X509_EXTENSION *ext, return 1; case X509V3_EXT_PARSE_UNKNOWN: - case X509V3_EXT_DUMP_UNKNOWN: - return BIO_hexdump(out, ext->value->data, ext->value->length, indent); + case X509V3_EXT_DUMP_UNKNOWN: { + const ASN1_STRING *data = X509_EXTENSION_get_data(ext); + return BIO_hexdump(out, ASN1_STRING_get0_data(data), + ASN1_STRING_length(data), indent); + } default: return 1; diff --git a/include/openssl/x509.h b/include/openssl/x509.h index 4960b35ea..dafa67737 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -134,12 +134,6 @@ struct X509_name_st { DEFINE_STACK_OF(X509_NAME) -struct X509_extension_st { - ASN1_OBJECT *object; - ASN1_BOOLEAN critical; - ASN1_OCTET_STRING *value; -} /* X509_EXTENSION */; - typedef STACK_OF(X509_EXTENSION) X509_EXTENSIONS; DEFINE_STACK_OF(X509_EXTENSION) @@ -1702,7 +1696,7 @@ OPENSSL_EXPORT ASN1_OCTET_STRING *X509_EXTENSION_get_data(X509_EXTENSION *ne); // X509_EXTENSION_get_critical returns one if |ex| is critical and zero // otherwise. -OPENSSL_EXPORT int X509_EXTENSION_get_critical(X509_EXTENSION *ex); +OPENSSL_EXPORT int X509_EXTENSION_get_critical(const X509_EXTENSION *ex); // X509at_get_attr_count returns the number of attributes in |x|. OPENSSL_EXPORT int X509at_get_attr_count(const STACK_OF(X509_ATTRIBUTE) *x);