diff --git a/crypto/x509/x509_ext.c b/crypto/x509/x509_ext.c index a8f0ab684..f6da54a4f 100644 --- a/crypto/x509/x509_ext.c +++ b/crypto/x509/x509_ext.c @@ -93,9 +93,10 @@ X509_EXTENSION *X509_CRL_delete_ext(X509_CRL *x, int loc) return (X509v3_delete_ext(x->crl->extensions, loc)); } -void *X509_CRL_get_ext_d2i(const X509_CRL *x, int nid, int *crit, int *idx) +void *X509_CRL_get_ext_d2i(const X509_CRL *crl, int nid, int *out_critical, + int *out_idx) { - return X509V3_get_d2i(x->crl->extensions, nid, crit, idx); + return X509V3_get_d2i(crl->crl->extensions, nid, out_critical, out_idx); } int X509_CRL_add1_ext_i2d(X509_CRL *x, int nid, void *value, int crit, @@ -145,9 +146,11 @@ int X509_add_ext(X509 *x, X509_EXTENSION *ex, int loc) return (X509v3_add_ext(&(x->cert_info->extensions), ex, loc) != NULL); } -void *X509_get_ext_d2i(const X509 *x, int nid, int *crit, int *idx) +void *X509_get_ext_d2i(const X509 *x509, int nid, int *out_critical, + int *out_idx) { - return X509V3_get_d2i(x->cert_info->extensions, nid, crit, idx); + return X509V3_get_d2i(x509->cert_info->extensions, nid, out_critical, + out_idx); } int X509_add1_ext_i2d(X509 *x, int nid, void *value, int crit, @@ -194,10 +197,10 @@ int X509_REVOKED_add_ext(X509_REVOKED *x, X509_EXTENSION *ex, int loc) return (X509v3_add_ext(&(x->extensions), ex, loc) != NULL); } -void *X509_REVOKED_get_ext_d2i(const X509_REVOKED *x, int nid, int *crit, - int *idx) +void *X509_REVOKED_get_ext_d2i(const X509_REVOKED *revoked, int nid, + int *out_critical, int *out_idx) { - return X509V3_get_d2i(x->extensions, nid, crit, idx); + return X509V3_get_d2i(revoked->extensions, nid, out_critical, out_idx); } int X509_REVOKED_add1_ext_i2d(X509_REVOKED *x, int nid, void *value, int crit, diff --git a/crypto/x509v3/v3_lib.c b/crypto/x509v3/v3_lib.c index d5eda3dd3..d89733fda 100644 --- a/crypto/x509v3/v3_lib.c +++ b/crypto/x509v3/v3_lib.c @@ -122,7 +122,7 @@ const X509V3_EXT_METHOD *X509V3_EXT_get_nid(int nid) return sk_X509V3_EXT_METHOD_value(ext_list, idx); } -const X509V3_EXT_METHOD *X509V3_EXT_get(X509_EXTENSION *ext) +const X509V3_EXT_METHOD *X509V3_EXT_get(const X509_EXTENSION *ext) { int nid; if ((nid = OBJ_obj2nid(ext->object)) == NID_undef) @@ -203,7 +203,7 @@ int X509V3_add_standard_extensions(void) /* Return an extension internal structure */ -void *X509V3_EXT_d2i(X509_EXTENSION *ext) +void *X509V3_EXT_d2i(const X509_EXTENSION *ext) { const X509V3_EXT_METHOD *method; const unsigned char *p; @@ -217,49 +217,38 @@ void *X509V3_EXT_d2i(X509_EXTENSION *ext) return method->d2i(NULL, &p, ext->value->length); } -/* - * Get critical flag and decoded version of extension from a NID. The "idx" - * variable returns the last found extension and can be used to retrieve - * multiple extensions of the same NID. However multiple extensions with the - * same NID is usually due to a badly encoded certificate so if idx is NULL - * we choke if multiple extensions exist. The "crit" variable is set to the - * critical value. The return value is the decoded extension or NULL on - * error. The actual error can have several different causes, the value of - * *crit reflects the cause: >= 0, extension found but not decoded (reflects - * critical value). -1 extension not found. -2 extension occurs more than - * once. - */ - -void *X509V3_get_d2i(STACK_OF(X509_EXTENSION) *x, int nid, int *crit, - int *idx) +void *X509V3_get_d2i(const STACK_OF(X509_EXTENSION) *extensions, int nid, + int *out_critical, int *out_idx) { int lastpos; size_t i; X509_EXTENSION *ex, *found_ex = NULL; - if (!x) { - if (idx) - *idx = -1; - if (crit) - *crit = -1; + if (!extensions) { + if (out_idx) + *out_idx = -1; + if (out_critical) + *out_critical = -1; return NULL; } - if (idx) - lastpos = *idx + 1; + if (out_idx) + lastpos = *out_idx + 1; else lastpos = 0; if (lastpos < 0) lastpos = 0; - for (i = lastpos; i < sk_X509_EXTENSION_num(x); i++) { - ex = sk_X509_EXTENSION_value(x, i); + for (i = lastpos; i < sk_X509_EXTENSION_num(extensions); i++) { + ex = sk_X509_EXTENSION_value(extensions, i); if (OBJ_obj2nid(ex->object) == nid) { - if (idx) { - *idx = i; + if (out_idx) { + /* TODO(https://crbug.com/boringssl/379): Consistently reject + * duplicate extensions. */ + *out_idx = i; found_ex = ex; break; } else if (found_ex) { /* Found more than one */ - if (crit) - *crit = -2; + if (out_critical) + *out_critical = -2; return NULL; } found_ex = ex; @@ -267,16 +256,16 @@ void *X509V3_get_d2i(STACK_OF(X509_EXTENSION) *x, int nid, int *crit, } if (found_ex) { /* Found it */ - if (crit) - *crit = X509_EXTENSION_get_critical(found_ex); + if (out_critical) + *out_critical = X509_EXTENSION_get_critical(found_ex); return X509V3_EXT_d2i(found_ex); } /* Extension not found */ - if (idx) - *idx = -1; - if (crit) - *crit = -1; + if (out_idx) + *out_idx = -1; + if (out_critical) + *out_critical = -1; return NULL; } diff --git a/include/openssl/x509.h b/include/openssl/x509.h index 580c591a5..aa32f0fd0 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -1238,7 +1238,15 @@ OPENSSL_EXPORT int X509_get_ext_by_critical(const X509 *x, int crit, OPENSSL_EXPORT X509_EXTENSION *X509_get_ext(const X509 *x, int loc); OPENSSL_EXPORT X509_EXTENSION *X509_delete_ext(X509 *x, int loc); OPENSSL_EXPORT int X509_add_ext(X509 *x, X509_EXTENSION *ex, int loc); -OPENSSL_EXPORT void *X509_get_ext_d2i(const X509 *x, int nid, int *crit, int *idx); + +// X509_get_ext_d2i behaves like |X509V3_get_d2i| but looks for the extension in +// |x509|'s extension list. +// +// WARNING: This function is difficult to use correctly. See the documentation +// for |X509V3_get_d2i| for details. +OPENSSL_EXPORT void *X509_get_ext_d2i(const X509 *x509, int nid, + int *out_critical, int *out_idx); + OPENSSL_EXPORT int X509_add1_ext_i2d(X509 *x, int nid, void *value, int crit, unsigned long flags); @@ -1251,8 +1259,15 @@ OPENSSL_EXPORT int X509_CRL_get_ext_by_critical(const X509_CRL *x, int crit, OPENSSL_EXPORT X509_EXTENSION *X509_CRL_get_ext(const X509_CRL *x, int loc); OPENSSL_EXPORT X509_EXTENSION *X509_CRL_delete_ext(X509_CRL *x, int loc); OPENSSL_EXPORT int X509_CRL_add_ext(X509_CRL *x, X509_EXTENSION *ex, int loc); -OPENSSL_EXPORT void *X509_CRL_get_ext_d2i(const X509_CRL *x, int nid, int *crit, - int *idx); + +// X509_CRL_get_ext_d2i behaves like |X509V3_get_d2i| but looks for the +// extension in |crl|'s extension list. +// +// WARNING: This function is difficult to use correctly. See the documentation +// for |X509V3_get_d2i| for details. +OPENSSL_EXPORT void *X509_CRL_get_ext_d2i(const X509_CRL *crl, int nid, + int *out_critical, int *out_idx); + OPENSSL_EXPORT int X509_CRL_add1_ext_i2d(X509_CRL *x, int nid, void *value, int crit, unsigned long flags); @@ -1270,8 +1285,16 @@ OPENSSL_EXPORT X509_EXTENSION *X509_REVOKED_delete_ext(X509_REVOKED *x, int loc); OPENSSL_EXPORT int X509_REVOKED_add_ext(X509_REVOKED *x, X509_EXTENSION *ex, int loc); -OPENSSL_EXPORT void *X509_REVOKED_get_ext_d2i(const X509_REVOKED *x, int nid, - int *crit, int *idx); + +// X509_REVOKED_get_ext_d2i behaves like |X509V3_get_d2i| but looks for the +// extension in |revoked|'s extension list. +// +// WARNING: This function is difficult to use correctly. See the documentation +// for |X509V3_get_d2i| for details. +OPENSSL_EXPORT void *X509_REVOKED_get_ext_d2i(const X509_REVOKED *revoked, + int nid, int *out_critical, + int *out_idx); + OPENSSL_EXPORT int X509_REVOKED_add1_ext_i2d(X509_REVOKED *x, int nid, void *value, int crit, unsigned long flags); diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h index 0a4e776cf..14d5e9539 100644 --- a/include/openssl/x509v3.h +++ b/include/openssl/x509v3.h @@ -633,13 +633,53 @@ OPENSSL_EXPORT int X509V3_EXT_add_list(X509V3_EXT_METHOD *extlist); OPENSSL_EXPORT int X509V3_EXT_add_alias(int nid_to, int nid_from); OPENSSL_EXPORT void X509V3_EXT_cleanup(void); -OPENSSL_EXPORT const X509V3_EXT_METHOD *X509V3_EXT_get(X509_EXTENSION *ext); +OPENSSL_EXPORT const X509V3_EXT_METHOD *X509V3_EXT_get( + const X509_EXTENSION *ext); OPENSSL_EXPORT const X509V3_EXT_METHOD *X509V3_EXT_get_nid(int nid); OPENSSL_EXPORT int X509V3_add_standard_extensions(void); OPENSSL_EXPORT STACK_OF(CONF_VALUE) *X509V3_parse_list(const char *line); -OPENSSL_EXPORT void *X509V3_EXT_d2i(X509_EXTENSION *ext); -OPENSSL_EXPORT void *X509V3_get_d2i(STACK_OF(X509_EXTENSION) *x, int nid, - int *crit, int *idx); + +// X509V3_EXT_d2i decodes |ext| and returns a pointer to a newly-allocated +// structure, with type dependent on the type of the extension. It returns NULL +// if |ext| is an unsupported extension or if there was a syntax error in the +// extension. The caller should cast the return value to the expected type and +// free the structure when done. +// +// WARNING: Casting the return value to the wrong type is a potentially +// exploitable memory error, so callers must not use this function before +// checking |ext| is of a known type. +OPENSSL_EXPORT void *X509V3_EXT_d2i(const X509_EXTENSION *ext); + +// X509V3_get_d2i finds and decodes the extension in |extensions| of type |nid|. +// If found, it decodes it and returns a newly-allocated structure, with type +// dependent on |nid|. If the extension is not found or on error, it returns +// NULL. The caller may distinguish these cases using the |out_critical| value. +// +// If |out_critical| is not NULL, this function sets |*out_critical| to one if +// the extension is found and critical, zero if it is found and not critical, -1 +// if it is not found, and -2 if there is an invalid duplicate extension. Note +// this function may set |*out_critical| to one or zero and still return NULL if +// the extension is found but has a syntax error. +// +// If |out_idx| is not NULL, this function looks for the first occurrence of the +// extension after |*out_idx|. It then sets |*out_idx| to the index of the +// extension, or -1 if not found. If |out_idx| is non-NULL, duplicate extensions +// are not treated as an error. Callers, however, should not rely on this +// behavior as it may be removed in the future. Duplicate extensions are +// forbidden in RFC5280. +// +// WARNING: This function is difficult to use correctly. Callers should pass a +// non-NULL |out_critical| and check both the return value and |*out_critical| +// to handle errors. If the return value is NULL and |*out_critical| is not -1, +// there was an error. Otherwise, the function succeeded and but may return NULL +// for a missing extension. Callers should pass NULL to |out_idx| so that +// duplicate extensions are handled correctly. +// +// Additionally, casting the return value to the wrong type is a potentially +// exploitable memory error, so callers must ensure the cast and |nid| match. +OPENSSL_EXPORT void *X509V3_get_d2i(const STACK_OF(X509_EXTENSION) *extensions, + int nid, int *out_critical, int *out_idx); + OPENSSL_EXPORT int X509V3_EXT_free(int nid, void *ext_data);