From ad211a916cc47bb568f527536f0ae65be91ffe08 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 1 Dec 2023 16:21:40 -0500 Subject: [PATCH 01/12] Fix build with -Wmissing-field-initializers Since it's otherwise pretty tedious, let's try this with C99 designated initializers. From testing, I remember they worked pretty reliably in C. (In C++, it's a little trickier because MSVC won't accept them outside C++20. Although I think all our supported MSVCs have a C++20 mode now...) AWS-LC: Changes to CMakeLists were not taken as that flag exists there already. Fixed: 671 Change-Id: Ia29ade8721ecfe2140a2d183ad60c8a730c631f0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64447 Auto-Submit: David Benjamin Reviewed-by: Bob Beck Commit-Queue: David Benjamin (cherry picked from commit a9a4c6dc89aff96f64c6ed93c1c3fc4d0c8e6e74) --- crypto/x509/x509_vpm.c | 47 ++++++++++++++---------------------------- 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/crypto/x509/x509_vpm.c b/crypto/x509/x509_vpm.c index 21f64afbf2..8d92a03c3e 100644 --- a/crypto/x509/x509_vpm.c +++ b/crypto/x509/x509_vpm.c @@ -492,44 +492,27 @@ int X509_VERIFY_PARAM_get_depth(const X509_VERIFY_PARAM *param) { #define vpm_empty_id NULL, 0U, NULL, 0, NULL, 0, 0 static const X509_VERIFY_PARAM kDefaultParam = { - /*check_time=*/0, - /*inh_flags=*/0, - /*flags=*/X509_V_FLAG_TRUSTED_FIRST, - /*purpose=*/0, - /*trust=*/0, - /*depth=*/100, - /*policies=*/NULL, - vpm_empty_id}; + .flags = X509_V_FLAG_TRUSTED_FIRST, + .depth = 100, +}; static const X509_VERIFY_PARAM kSMIMESignParam = { - /*check_time=*/0, - /*inh_flags=*/0, - /*flags=*/0, - /*purpose=*/X509_PURPOSE_SMIME_SIGN, - /*trust=*/X509_TRUST_EMAIL, - /*depth=*/-1, - /*policies=*/NULL, - vpm_empty_id}; + .purpose = X509_PURPOSE_SMIME_SIGN, + .trust = X509_TRUST_EMAIL, + .depth = -1, +}; static const X509_VERIFY_PARAM kSSLClientParam = { - /*check_time=*/0, - /*inh_flags=*/0, - /*flags=*/0, - /*purpose=*/X509_PURPOSE_SSL_CLIENT, - /*trust=*/X509_TRUST_SSL_CLIENT, - /*depth=*/-1, - /*policies=*/NULL, - vpm_empty_id}; + .purpose = X509_PURPOSE_SSL_CLIENT, + .trust = X509_TRUST_SSL_CLIENT, + .depth = -1, +}; static const X509_VERIFY_PARAM kSSLServerParam = { - /*check_time=*/0, - /*inh_flags=*/0, - /*flags=*/0, - /*purpose=*/X509_PURPOSE_SSL_SERVER, - /*trust=*/X509_TRUST_SSL_SERVER, - /*depth=*/-1, - /*policies=*/NULL, - vpm_empty_id}; + .purpose = X509_PURPOSE_SSL_SERVER, + .trust = X509_TRUST_SSL_SERVER, + .depth = -1, +}; const X509_VERIFY_PARAM *X509_VERIFY_PARAM_lookup(const char *name) { if (strcmp(name, "default") == 0) { From 13022bb3c180def93415d6be663dc87cf8ef48ed Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sat, 25 Nov 2023 12:06:49 -0500 Subject: [PATCH 02/12] Simplify and document X509_VERIFY_PARAM inheritance X509_VERIFY_PARAM inheritance is unbelievably thorny, and I'm not sure it was ever thought through very well. We can make it slightly less complicated by removing the internal inh_flags value. We don't support X509_VERIFY_PARAM_set_inh_flags (and really should keep it that way!), so there are actually only two possible values, zero and X509_VP_FLAG_DEFAULT, used to implement X509_VERIFY_PARAM_inherit, and X509_VERIFY_PARAM_set1, respectively. This still leaves some weird behaviors that are expected through the public API, which I've documented in the headers. They'll probably need another pass when they're grouped into sections and whatnot. Bug: 441, 426 Change-Id: Ib0a855afd35e597c65c249627addfef76ed7099d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64253 Commit-Queue: David Benjamin Reviewed-by: Bob Beck (cherry picked from commit 51ae958ff7c99103932e07905c40cbc71a22b389) --- crypto/x509/internal.h | 1 - crypto/x509/x509_lu.c | 2 +- crypto/x509/x509_test.cc | 91 ++++++++++++++++++++++++ crypto/x509/x509_vpm.c | 146 +++++++++++++-------------------------- include/openssl/x509.h | 81 +++++++++++++++++----- 5 files changed, 203 insertions(+), 118 deletions(-) diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h index 92bcf65166..87a77e56a2 100644 --- a/crypto/x509/internal.h +++ b/crypto/x509/internal.h @@ -248,7 +248,6 @@ struct X509_crl_st { struct X509_VERIFY_PARAM_st { int64_t check_time; // POSIX time to use - unsigned long inh_flags; // Inheritance flags unsigned long flags; // Various verify flags int purpose; // purpose to check untrusted certificates int trust; // trust setting to check diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c index 71487c3b64..1629b50a98 100644 --- a/crypto/x509/x509_lu.c +++ b/crypto/x509/x509_lu.c @@ -630,7 +630,7 @@ int X509_STORE_set_trust(X509_STORE *ctx, int trust) { return X509_VERIFY_PARAM_set_trust(ctx->param, trust); } -int X509_STORE_set1_param(X509_STORE *ctx, X509_VERIFY_PARAM *param) { +int X509_STORE_set1_param(X509_STORE *ctx, const X509_VERIFY_PARAM *param) { return X509_VERIFY_PARAM_set1(ctx->param, param); } diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc index ae79e1b50c..cc2a683720 100644 --- a/crypto/x509/x509_test.cc +++ b/crypto/x509/x509_test.cc @@ -7128,3 +7128,94 @@ TEST(X509Test, ExternalData) { EXPECT_EQ(retrieved_data->custom_data, 123); } +TEST(X509Test, ParamInheritance) { + // |X509_VERIFY_PARAM_inherit| with both unset. + { + bssl::UniquePtr dest(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(dest); + bssl::UniquePtr src(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(src); + ASSERT_TRUE(X509_VERIFY_PARAM_inherit(dest.get(), src.get())); + EXPECT_EQ(X509_VERIFY_PARAM_get_depth(dest.get()), -1); + } + + // |X509_VERIFY_PARAM_inherit| with source set. + { + bssl::UniquePtr dest(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(dest); + bssl::UniquePtr src(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(src); + X509_VERIFY_PARAM_set_depth(src.get(), 5); + ASSERT_TRUE(X509_VERIFY_PARAM_inherit(dest.get(), src.get())); + EXPECT_EQ(X509_VERIFY_PARAM_get_depth(dest.get()), 5); + } + + // |X509_VERIFY_PARAM_inherit| with destination set. + { + bssl::UniquePtr dest(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(dest); + bssl::UniquePtr src(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(src); + X509_VERIFY_PARAM_set_depth(dest.get(), 5); + ASSERT_TRUE(X509_VERIFY_PARAM_inherit(dest.get(), src.get())); + EXPECT_EQ(X509_VERIFY_PARAM_get_depth(dest.get()), 5); + } + + // |X509_VERIFY_PARAM_inherit| with both set. + { + bssl::UniquePtr dest(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(dest); + bssl::UniquePtr src(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(src); + X509_VERIFY_PARAM_set_depth(dest.get(), 5); + X509_VERIFY_PARAM_set_depth(src.get(), 10); + ASSERT_TRUE(X509_VERIFY_PARAM_inherit(dest.get(), src.get())); + // The existing value is used. + EXPECT_EQ(X509_VERIFY_PARAM_get_depth(dest.get()), 5); + } + + // |X509_VERIFY_PARAM_set1| with both unset. + { + bssl::UniquePtr dest(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(dest); + bssl::UniquePtr src(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(src); + ASSERT_TRUE(X509_VERIFY_PARAM_set1(dest.get(), src.get())); + EXPECT_EQ(X509_VERIFY_PARAM_get_depth(dest.get()), -1); + } + + // |X509_VERIFY_PARAM_set1| with source set. + { + bssl::UniquePtr dest(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(dest); + bssl::UniquePtr src(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(src); + X509_VERIFY_PARAM_set_depth(src.get(), 5); + ASSERT_TRUE(X509_VERIFY_PARAM_set1(dest.get(), src.get())); + EXPECT_EQ(X509_VERIFY_PARAM_get_depth(dest.get()), 5); + } + + // |X509_VERIFY_PARAM_set1| with destination set. + { + bssl::UniquePtr dest(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(dest); + bssl::UniquePtr src(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(src); + X509_VERIFY_PARAM_set_depth(dest.get(), 5); + ASSERT_TRUE(X509_VERIFY_PARAM_set1(dest.get(), src.get())); + EXPECT_EQ(X509_VERIFY_PARAM_get_depth(dest.get()), 5); + } + + // |X509_VERIFY_PARAM_set1| with both set. + { + bssl::UniquePtr dest(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(dest); + bssl::UniquePtr src(X509_VERIFY_PARAM_new()); + ASSERT_TRUE(src); + X509_VERIFY_PARAM_set_depth(dest.get(), 5); + X509_VERIFY_PARAM_set_depth(src.get(), 10); + ASSERT_TRUE(X509_VERIFY_PARAM_set1(dest.get(), src.get())); + // The new value is used. + EXPECT_EQ(X509_VERIFY_PARAM_get_depth(dest.get()), 10); + } +} diff --git a/crypto/x509/x509_vpm.c b/crypto/x509/x509_vpm.c index 8d92a03c3e..3031ff9a8d 100644 --- a/crypto/x509/x509_vpm.c +++ b/crypto/x509/x509_vpm.c @@ -129,9 +129,6 @@ X509_VERIFY_PARAM *X509_VERIFY_PARAM_new(void) { return NULL; } param->depth = -1; - // TODO(crbug.com/boringssl/441): This line was commented out. Figure out what - // this was for: - // param->inh_flags = X509_VP_FLAG_DEFAULT; return param; } @@ -146,143 +143,98 @@ void X509_VERIFY_PARAM_free(X509_VERIFY_PARAM *param) { OPENSSL_free(param); } -//- -// This function determines how parameters are "inherited" from one structure -// to another. There are several different ways this can happen. -// -// 1. If a child structure needs to have its values initialized from a parent -// they are simply copied across. For example SSL_CTX copied to SSL. -// 2. If the structure should take on values only if they are currently unset. -// For example the values in an SSL structure will take appropriate value -// for SSL servers or clients but only if the application has not set new -// ones. -// -// The "inh_flags" field determines how this function behaves. -// -// Normally any values which are set in the default are not copied from the -// destination and verify flags are ORed together. -// -// If X509_VP_FLAG_DEFAULT is set then anything set in the source is copied -// to the destination. Effectively the values in "to" become default values -// which will be used only if nothing new is set in "from". -// -// If X509_VP_FLAG_OVERWRITE is set then all value are copied across whether -// they are set or not. Flags is still Ored though. -// -// If X509_VP_FLAG_RESET_FLAGS is set then the flags value is copied instead -// of ORed. -// -// If X509_VP_FLAG_LOCKED is set then no values are copied. -// -// If X509_VP_FLAG_ONCE is set then the current inh_flags setting is zeroed -// after the next call. - -// Macro to test if a field should be copied from src to dest - -#define test_x509_verify_param_copy(field, def) \ - (to_overwrite || \ - ((src->field != (def)) && (to_default || (dest->field == (def))))) - -// Macro to test and copy a field if necessary - -#define x509_verify_param_copy(field, def) \ - if (test_x509_verify_param_copy(field, def)) \ - dest->field = src->field - -int X509_VERIFY_PARAM_inherit(X509_VERIFY_PARAM *dest, - const X509_VERIFY_PARAM *src) { - unsigned long inh_flags; - int to_default, to_overwrite; - if (!src) { - return 1; - } - inh_flags = dest->inh_flags | src->inh_flags; - - if (inh_flags & X509_VP_FLAG_ONCE) { - dest->inh_flags = 0; +static int should_copy(int dest_is_set, int src_is_set, int prefer_src) { + if (prefer_src) { + // We prefer the source, so as long as there is a value to copy, copy it. + return src_is_set; } - if (inh_flags & X509_VP_FLAG_LOCKED) { - return 1; - } + // We prefer the destination, so only copy if the destination is unset. + return src_is_set && !dest_is_set; +} - if (inh_flags & X509_VP_FLAG_DEFAULT) { - to_default = 1; - } else { - to_default = 0; +static void copy_int_param(int *dest, const int *src, int default_val, + int prefer_src) { + if (should_copy(*dest != default_val, *src != default_val, prefer_src)) { + *dest = *src; } +} - if (inh_flags & X509_VP_FLAG_OVERWRITE) { - to_overwrite = 1; - } else { - to_overwrite = 0; +// x509_verify_param_copy copies fields from |src| to |dest|. If both |src| and +// |dest| have some field set, |prefer_src| determines whether |src| or |dest|'s +// version is used. +static int x509_verify_param_copy(X509_VERIFY_PARAM *dest, + const X509_VERIFY_PARAM *src, + int prefer_src) { + if (src == NULL) { + return 1; } - x509_verify_param_copy(purpose, 0); - x509_verify_param_copy(trust, 0); - x509_verify_param_copy(depth, -1); - - // If overwrite or check time not set, copy across + copy_int_param(&dest->purpose, &src->purpose, /*default_val=*/0, prefer_src); + copy_int_param(&dest->trust, &src->trust, /*default_val=*/0, prefer_src); + copy_int_param(&dest->depth, &src->depth, /*default_val=*/-1, prefer_src); - if (to_overwrite || !(dest->flags & X509_V_FLAG_USE_CHECK_TIME)) { + // |check_time|, unlike all other parameters, does not honor |prefer_src|. + // This means |X509_VERIFY_PARAM_set1| will not overwrite it. This behavior + // comes from OpenSSL but may have been a bug. + if (!(dest->flags & X509_V_FLAG_USE_CHECK_TIME)) { dest->check_time = src->check_time; - dest->flags &= ~X509_V_FLAG_USE_CHECK_TIME; - // Don't need to copy flag: that is done below - } - - if (inh_flags & X509_VP_FLAG_RESET_FLAGS) { - dest->flags = 0; + // The source |X509_V_FLAG_USE_CHECK_TIME| flag, if set, is copied below. } dest->flags |= src->flags; - if (test_x509_verify_param_copy(policies, NULL)) { + if (should_copy(dest->policies != NULL, src->policies != NULL, prefer_src)) { if (!X509_VERIFY_PARAM_set1_policies(dest, src->policies)) { return 0; } } - // Copy the host flags if and only if we're copying the host list - if (test_x509_verify_param_copy(hosts, NULL)) { - if (dest->hosts) { - sk_OPENSSL_STRING_pop_free(dest->hosts, str_free); - dest->hosts = NULL; - } + if (should_copy(dest->hosts != NULL, src->hosts != NULL, prefer_src)) { + sk_OPENSSL_STRING_pop_free(dest->hosts, str_free); + dest->hosts = NULL; if (src->hosts) { dest->hosts = sk_OPENSSL_STRING_deep_copy(src->hosts, OPENSSL_strdup, str_free); if (dest->hosts == NULL) { return 0; } + // Copy the host flags if and only if we're copying the host list. Note + // this means mechanisms like |X509_STORE_CTX_set_default| cannot be used + // to set host flags. E.g. we cannot change the defaults using + // |kDefaultParam| below. dest->hostflags = src->hostflags; } } - if (test_x509_verify_param_copy(email, NULL)) { + if (should_copy(dest->email != NULL, src->email != NULL, prefer_src)) { if (!X509_VERIFY_PARAM_set1_email(dest, src->email, src->emaillen)) { return 0; } } - if (test_x509_verify_param_copy(ip, NULL)) { + if (should_copy(dest->ip != NULL, src->ip != NULL, prefer_src)) { if (!X509_VERIFY_PARAM_set1_ip(dest, src->ip, src->iplen)) { return 0; } } dest->poison = src->poison; - return 1; } +int X509_VERIFY_PARAM_inherit(X509_VERIFY_PARAM *dest, + const X509_VERIFY_PARAM *src) { + // Prefer the destination. That is, this function only changes unset + // parameters in |dest|. + return x509_verify_param_copy(dest, src, /*prefer_src=*/0); +} + int X509_VERIFY_PARAM_set1(X509_VERIFY_PARAM *to, const X509_VERIFY_PARAM *from) { - unsigned long save_flags = to->inh_flags; - int ret; - to->inh_flags |= X509_VP_FLAG_DEFAULT; - ret = X509_VERIFY_PARAM_inherit(to, from); - to->inh_flags = save_flags; - return ret; + // Prefer the source. That is, values in |to| are only preserved if they were + // unset in |from|. + return x509_verify_param_copy(to, from, /*prefer_src=*/1); } static int int_x509_param_set1_email(char **pdest, size_t *pdestlen, @@ -365,7 +317,7 @@ int X509_VERIFY_PARAM_clear_flags(X509_VERIFY_PARAM *param, return 1; } -unsigned long X509_VERIFY_PARAM_get_flags(X509_VERIFY_PARAM *param) { +unsigned long X509_VERIFY_PARAM_get_flags(const X509_VERIFY_PARAM *param) { return param->flags; } diff --git a/include/openssl/x509.h b/include/openssl/x509.h index 7468a871c0..e8eb4ef4de 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -3291,12 +3291,6 @@ OPENSSL_EXPORT int X509_LOOKUP_add_dir(X509_LOOKUP *lookup, const char *path, // verification. #define X509_V_FLAG_NO_CHECK_TIME 0x200000 -#define X509_VP_FLAG_DEFAULT 0x1 -#define X509_VP_FLAG_OVERWRITE 0x2 -#define X509_VP_FLAG_RESET_FLAGS 0x4 -#define X509_VP_FLAG_LOCKED 0x8 -#define X509_VP_FLAG_ONCE 0x10 - // Internal use: mask of policy related options (hidden) #define X509_V_FLAG_POLICY_MASK \ @@ -3364,16 +3358,34 @@ OPENSSL_EXPORT STACK_OF(X509_CRL) *X509_STORE_get1_crls(X509_STORE_CTX *st, // the |X509_STORE|. See discussion in |X509_STORE_get0_param|. OPENSSL_EXPORT int X509_STORE_set_flags(X509_STORE *store, unsigned long flags); -OPENSSL_EXPORT int X509_STORE_set_purpose(X509_STORE *ctx, int purpose); -OPENSSL_EXPORT int X509_STORE_set_trust(X509_STORE *ctx, int trust); -OPENSSL_EXPORT int X509_STORE_set1_param(X509_STORE *ctx, - X509_VERIFY_PARAM *pm); - -// X509_STORE_get0_param returns |store|'s default verification parameters. This -// object is mutable and may be modified by the caller. -// -// TODO(crbug.com/boringssl/441): Discuss the semantics of this notion of -// "default". +OPENSSL_EXPORT int X509_STORE_set_purpose(X509_STORE *store, int purpose); +OPENSSL_EXPORT int X509_STORE_set_trust(X509_STORE *store, int trust); + +// X509_STORE_set1_param copies verification parameters from |param| as in +// |X509_VERIFY_PARAM_set1|. It returns one on success and zero on error. +OPENSSL_EXPORT int X509_STORE_set1_param(X509_STORE *store, + const X509_VERIFY_PARAM *param); + +// X509_STORE_get0_param returns |store|'s verification parameters. This object +// is mutable and may be modified by the caller. For an individual certificate +// verification operation, |X509_STORE_CTX_init| initializes the +// |X509_STORE_CTX|'s parameters with these parameters. +// +// WARNING: |X509_STORE_CTX_init| applies some default parameters (as in +// |X509_VERIFY_PARAM_inherit|) after copying |store|'s parameters. This means +// it is impossible to leave some parameters unset at |store|. They must be +// explicitly unset after creating the |X509_STORE_CTX|. +// +// As of writing these late defaults are a depth limit (see +// |X509_VERIFY_PARAM_set_depth|) and the |X509_V_FLAG_TRUSTED_FIRST| flag. This +// warning does not apply if the parameters were set in |store|. That is, +// callers may safely set a concrete depth limit in |store|, but unlimited depth +// must be configured at |X509_STORE_CTX|. +// +// TODO(crbug.com/boringssl/441): This behavior is very surprising. Can we +// remove this notion of late defaults? A depth limit of 100 can probably be +// applied unconditionally. |X509_V_FLAG_TRUSTED_FIRST| is mostly a workaround +// for poor path-building. OPENSSL_EXPORT X509_VERIFY_PARAM *X509_STORE_get0_param(X509_STORE *store); // X509_STORE_set_verify_cb acts like |X509_STORE_CTX_set_verify_cb| but sets @@ -3409,6 +3421,14 @@ OPENSSL_EXPORT int X509_STORE_CTX_get1_issuer(X509 **issuer, // X509_STORE_CTX_free releases memory associated with |ctx|. OPENSSL_EXPORT void X509_STORE_CTX_free(X509_STORE_CTX *ctx); +// X509_STORE_CTX_init initializes |ctx| to verify |x509|, using trusted +// certificates and parameters in |store|. It returns one on success and zero on +// error. |chain| is a list of untrusted intermediate certificates to use in +// verification. +// +// |ctx| stores pointers to |store|, |x509|, and |chain|. Each of these objects +// must outlive |ctx| and may not be mutated for the duration of the certificate +// verification. OPENSSL_EXPORT int X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store, X509 *x509, STACK_OF(X509) *chain); @@ -3535,11 +3555,21 @@ OPENSSL_EXPORT X509_VERIFY_PARAM *X509_STORE_CTX_get0_param( // and takes ownership of |param|. After this function returns, the caller // should not free |param|. // -// TODO(crbug.com/boringssl/441): The bug notes some odd interactions with -// the different notions of default. Discuss this. +// WARNING: This function discards any values which were previously applied in +// |ctx|, including the "default" parameters applied late in +// |X509_STORE_CTX_init|. These late defaults are not applied to parameters +// created standalone by |X509_VERIFY_PARAM_new|. +// +// TODO(crbug.com/boringssl/441): This behavior is very surprising. Should we +// re-apply the late defaults in |param|, or somehow avoid this notion of late +// defaults altogether? OPENSSL_EXPORT void X509_STORE_CTX_set0_param(X509_STORE_CTX *ctx, X509_VERIFY_PARAM *param); +// X509_STORE_CTX_set_default looks up the set of parameters named |name| and +// applies those default verification parameters for |ctx|. As in +// |X509_VERIFY_PARAM_inherit|, only unset parameters are changed. This function +// returns one on success and zero on error. OPENSSL_EXPORT int X509_STORE_CTX_set_default(X509_STORE_CTX *ctx, const char *name); @@ -3564,8 +3594,15 @@ OPENSSL_EXPORT X509_VERIFY_PARAM *X509_VERIFY_PARAM_new(void); // X509_VERIFY_PARAM_free releases memory associated with |param|. OPENSSL_EXPORT void X509_VERIFY_PARAM_free(X509_VERIFY_PARAM *param); +// X509_VERIFY_PARAM_inherit applies |from| as the default values for |to|. That +// is, for each parameter that is unset in |to|, it copies the value in |from|. +// This function returns one on success and zero on error. OPENSSL_EXPORT int X509_VERIFY_PARAM_inherit(X509_VERIFY_PARAM *to, const X509_VERIFY_PARAM *from); + +// X509_VERIFY_PARAM_set1 copies parameters from |from| to |to|. If a parameter +// is unset in |from|, the existing value in |to| is preserved. This function +// returns one on success and zero on error. OPENSSL_EXPORT int X509_VERIFY_PARAM_set1(X509_VERIFY_PARAM *to, const X509_VERIFY_PARAM *from); @@ -3575,10 +3612,16 @@ OPENSSL_EXPORT int X509_VERIFY_PARAM_set1(X509_VERIFY_PARAM *to, OPENSSL_EXPORT int X509_VERIFY_PARAM_set_flags(X509_VERIFY_PARAM *param, unsigned long flags); +// X509_VERIFY_PARAM_clear_flags disables all values in |flags| in |param|'s +// verification flags and returns one. |flags| should be a combination of +// |X509_V_FLAG_*| constants. OPENSSL_EXPORT int X509_VERIFY_PARAM_clear_flags(X509_VERIFY_PARAM *param, unsigned long flags); + +// X509_VERIFY_PARAM_get_flags returns |param|'s verification flags. OPENSSL_EXPORT unsigned long X509_VERIFY_PARAM_get_flags( - X509_VERIFY_PARAM *param); + const X509_VERIFY_PARAM *param); + OPENSSL_EXPORT int X509_VERIFY_PARAM_set_purpose(X509_VERIFY_PARAM *param, int purpose); OPENSSL_EXPORT int X509_VERIFY_PARAM_set_trust(X509_VERIFY_PARAM *param, From 60aabb4c368dabdc14748be5185ce5f67d3bb3a5 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 22 Nov 2023 02:55:41 -0500 Subject: [PATCH 03/12] Fix the names of some X509_STORE_CTX functions This matches an upstream change. Add macros for the old names. We may be able to unexport these, but for now just pick up the less confusing names. (I found only one user of one of them, goma, which will be replaced next year. Though wanting *some* API to query the dirhash machinery is not completely implausible.) Change-Id: Idd2352c07c294c4f63c4ef12e5d97804f42225b9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64254 Reviewed-by: Bob Beck Commit-Queue: David Benjamin (cherry picked from commit 33a5e94645787d74593efa0e63200a31372325a2) --- crypto/x509/x509_lu.c | 7 ++++--- crypto/x509/x509_vfy.c | 4 ++-- include/openssl/x509.h | 13 +++++++++---- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c index 1629b50a98..58fcc696da 100644 --- a/crypto/x509/x509_lu.c +++ b/crypto/x509/x509_lu.c @@ -231,7 +231,7 @@ X509_LOOKUP *X509_STORE_add_lookup(X509_STORE *v, const X509_LOOKUP_METHOD *m) { } int X509_STORE_CTX_get_by_subject(X509_STORE_CTX *vs, int type, X509_NAME *name, - X509_OBJECT *ret) { + X509_OBJECT *ret) { X509_STORE *ctx = vs->ctx; X509_OBJECT stmp; CRYPTO_MUTEX_lock_write(&ctx->objs_lock); @@ -445,7 +445,7 @@ STACK_OF(X509_OBJECT) *X509_STORE_get0_objects(X509_STORE *st) { return st->objs; } -STACK_OF(X509) *X509_STORE_get1_certs(X509_STORE_CTX *ctx, X509_NAME *nm) { +STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *ctx, X509_NAME *nm) { int cnt; STACK_OF(X509) *sk = sk_X509_new_null(); if (sk == NULL) { @@ -485,7 +485,8 @@ STACK_OF(X509) *X509_STORE_get1_certs(X509_STORE_CTX *ctx, X509_NAME *nm) { return sk; } -STACK_OF(X509_CRL) *X509_STORE_get1_crls(X509_STORE_CTX *ctx, X509_NAME *nm) { +STACK_OF(X509_CRL) *X509_STORE_CTX_get1_crls(X509_STORE_CTX *ctx, + X509_NAME *nm) { int cnt; X509_OBJECT xobj; STACK_OF(X509_CRL) *sk = sk_X509_CRL_new_null(); diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 360db6981d..73b2089f14 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -139,7 +139,7 @@ static X509 *lookup_cert_match(X509_STORE_CTX *ctx, X509 *x) { X509 *xtmp = NULL; size_t i; // Lookup all certs with matching subject name - certs = X509_STORE_get1_certs(ctx, X509_get_subject_name(x)); + certs = X509_STORE_CTX_get1_certs(ctx, X509_get_subject_name(x)); if (certs == NULL) { return NULL; } @@ -1154,7 +1154,7 @@ static int get_crl(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509 *x) { } // Lookup CRLs from store - skcrl = X509_STORE_get1_crls(ctx, nm); + skcrl = X509_STORE_CTX_get1_crls(ctx, nm); // If no CRLs found and a near match from get_crl_sk use that if (!skcrl && crl) { diff --git a/include/openssl/x509.h b/include/openssl/x509.h index e8eb4ef4de..6ab021dfd9 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -2954,6 +2954,11 @@ OPENSSL_EXPORT int X509V3_add_standard_extensions(void); OPENSSL_EXPORT STACK_OF(CONF_VALUE) *X509V3_parse_list(const char *line); +// The following symbols are legacy aliases for |X509_STORE_CTX| functions. +#define X509_STORE_get1_certs X509_STORE_CTX_get1_certs +#define X509_STORE_get1_crls X509_STORE_CTX_get1_crls + + // Private structures. struct X509_algor_st { @@ -3345,10 +3350,10 @@ OPENSSL_EXPORT int X509_STORE_up_ref(X509_STORE *store); OPENSSL_EXPORT void X509_STORE_free(X509_STORE *store); OPENSSL_EXPORT STACK_OF(X509_OBJECT) *X509_STORE_get0_objects(X509_STORE *st); -OPENSSL_EXPORT STACK_OF(X509) *X509_STORE_get1_certs(X509_STORE_CTX *st, - X509_NAME *nm); -OPENSSL_EXPORT STACK_OF(X509_CRL) *X509_STORE_get1_crls(X509_STORE_CTX *st, - X509_NAME *nm); +OPENSSL_EXPORT STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *st, + X509_NAME *nm); +OPENSSL_EXPORT STACK_OF(X509_CRL) *X509_STORE_CTX_get1_crls(X509_STORE_CTX *st, + X509_NAME *nm); // X509_STORE_set_flags enables all values in |flags| in |store|'s verification // flags. |flags| should be a combination of |X509_V_FLAG_*| constants. From a37695582431cf35f099ce5aef66d3ee2b4d5125 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sat, 25 Nov 2023 13:27:08 -0500 Subject: [PATCH 04/12] Consistently call CRYPTO_free_ex_data first (Or second, if there's a method table.) CRYPTO_EX_free is passed the parent object, so the caller could, in principle, inspect the object. We should pass in an object in a self-consistent state. Also fix up the documentation. I think some bits of a since removed CRYPTO_EX_new got jumbled up in there. Change-Id: I316d00aee61bf544f59d4dac2efcd825e4bfa9b1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64255 Reviewed-by: Bob Beck Commit-Queue: David Benjamin (cherry picked from commit 59906b3aa8d9f48ad7303edc540912bd588a8e46) --- crypto/fipsmodule/ec/ec_key.c | 4 ++-- crypto/x509/x509_vfy.c | 4 +--- include/openssl/ex_data.h | 9 +++++---- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/crypto/fipsmodule/ec/ec_key.c b/crypto/fipsmodule/ec/ec_key.c index 2373aab38e..46e0d67b4c 100644 --- a/crypto/fipsmodule/ec/ec_key.c +++ b/crypto/fipsmodule/ec/ec_key.c @@ -163,12 +163,12 @@ void EC_KEY_free(EC_KEY *r) { METHOD_unref(r->ecdsa_meth); } + CRYPTO_free_ex_data(g_ec_ex_data_class_bss_get(), r, &r->ex_data); + EC_GROUP_free(r->group); EC_POINT_free(r->pub_key); ec_wrapped_scalar_free(r->priv_key); - CRYPTO_free_ex_data(g_ec_ex_data_class_bss_get(), r, &r->ex_data); - OPENSSL_free(r); } diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 73b2089f14..f99d464226 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -1740,11 +1740,9 @@ void X509_STORE_CTX_trusted_stack(X509_STORE_CTX *ctx, STACK_OF(X509) *sk) { } void X509_STORE_CTX_cleanup(X509_STORE_CTX *ctx) { + CRYPTO_free_ex_data(&g_ex_data_class, ctx, &(ctx->ex_data)); X509_VERIFY_PARAM_free(ctx->param); - ctx->param = NULL; sk_X509_pop_free(ctx->chain, X509_free); - ctx->chain = NULL; - CRYPTO_free_ex_data(&g_ex_data_class, ctx, &(ctx->ex_data)); OPENSSL_memset(ctx, 0, sizeof(X509_STORE_CTX)); } diff --git a/include/openssl/ex_data.h b/include/openssl/ex_data.h index 7d03f1766a..5fac7c3d9c 100644 --- a/include/openssl/ex_data.h +++ b/include/openssl/ex_data.h @@ -163,10 +163,11 @@ OPENSSL_EXPORT void *TYPE_get_ex_data(const TYPE *t, int index); // callback has been passed to |SSL_get_ex_new_index| then it may be called each // time an |SSL*| is destroyed. // -// The callback is passed the new object (i.e. the |SSL*|) in |parent|. The -// arguments |argl| and |argp| contain opaque values that were given to -// |CRYPTO_get_ex_new_index|. The callback should return one on success, but -// the value is ignored. +// The callback is passed the to-be-destroyed object (i.e. the |SSL*|) in +// |parent|. As |parent| will shortly be destroyed, callers must not perform +// operations that would increment its reference count, pass ownership, or +// assume the object outlives the function call. The arguments |argl| and |argp| +// contain opaque values that were given to |CRYPTO_get_ex_new_index|. // // This callback may be called with a NULL value for |ptr| if |parent| has no // value set for this index. However, the callbacks may also be skipped entirely From d3470a2e66a9c40aecd7c1b5c35b0882dd874802 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 6 Dec 2023 09:50:54 -0500 Subject: [PATCH 05/12] Add missing include Change-Id: Ifaef253aa82b07d0930dddbd773724132a7724c4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64587 Reviewed-by: Adam Langley Commit-Queue: Adam Langley Auto-Submit: David Benjamin (cherry picked from commit c41de812877a5ba256ba78e64c06dcbe3c8343d3) --- crypto/fipsmodule/sha/sha_test.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crypto/fipsmodule/sha/sha_test.cc b/crypto/fipsmodule/sha/sha_test.cc index 651181aace..28ddf92c57 100644 --- a/crypto/fipsmodule/sha/sha_test.cc +++ b/crypto/fipsmodule/sha/sha_test.cc @@ -14,6 +14,8 @@ #include +#include + #include #include "../../test/abi_test.h" From 68b9d6bbb1131d00c5b8c268800a714b716a9535 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 27 Nov 2023 23:26:09 -0500 Subject: [PATCH 06/12] Document or unexport some more of x509.h Get the remaining config APIs, extensions accessors, and the get1_email family. I'm not sure yet whether the various remaining extension-specific functions should get their own sections (probably), in which case, maybe we should move the accessors these into their sections? Put them with the rest of the certificate getters for now. As part of this, deduplicate the X509v3_KU_* and KU_* constants. See https://github.com/openssl/openssl/issues/22955 Bug: 426 Change-Id: I31a9b887eb1e6cfa272f04d2ee80dbb5a9ed98f7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64256 Reviewed-by: Bob Beck Commit-Queue: Bob Beck (cherry picked from commit 314c2520eab450615d8e78df21169c090d6f51e5) --- crypto/x509/internal.h | 10 + crypto/x509/v3_alt.c | 8 +- crypto/x509/v3_info.c | 8 +- crypto/x509/v3_purp.c | 33 +-- crypto/x509/v3_utl.c | 14 +- crypto/x509/x509_req.c | 8 +- crypto/x509/x509_vfy.c | 2 +- include/openssl/base.h | 1 + include/openssl/x509.h | 461 +++++++++++++++++++++++------------------ 9 files changed, 315 insertions(+), 230 deletions(-) diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h index 87a77e56a2..e178f0a289 100644 --- a/crypto/x509/internal.h +++ b/crypto/x509/internal.h @@ -543,6 +543,16 @@ OPENSSL_EXPORT int GENERAL_NAME_cmp(const GENERAL_NAME *a, // |name|, or NULL if no such name is defined. const X509_VERIFY_PARAM *X509_VERIFY_PARAM_lookup(const char *name); +GENERAL_NAME *v2i_GENERAL_NAME(const X509V3_EXT_METHOD *method, + const X509V3_CTX *ctx, const CONF_VALUE *cnf); +GENERAL_NAME *v2i_GENERAL_NAME_ex(GENERAL_NAME *out, + const X509V3_EXT_METHOD *method, + const X509V3_CTX *ctx, const CONF_VALUE *cnf, + int is_nc); +GENERAL_NAMES *v2i_GENERAL_NAMES(const X509V3_EXT_METHOD *method, + const X509V3_CTX *ctx, + const STACK_OF(CONF_VALUE) *nval); + #if defined(__cplusplus) } // extern C diff --git a/crypto/x509/v3_alt.c b/crypto/x509/v3_alt.c index f61645a77a..61ea4a9371 100644 --- a/crypto/x509/v3_alt.c +++ b/crypto/x509/v3_alt.c @@ -446,10 +446,10 @@ GENERAL_NAME *v2i_GENERAL_NAME(const X509V3_EXT_METHOD *method, return v2i_GENERAL_NAME_ex(NULL, method, ctx, cnf, 0); } -GENERAL_NAME *a2i_GENERAL_NAME(GENERAL_NAME *out, - const X509V3_EXT_METHOD *method, - const X509V3_CTX *ctx, int gen_type, - const char *value, int is_nc) { +static GENERAL_NAME *a2i_GENERAL_NAME(GENERAL_NAME *out, + const X509V3_EXT_METHOD *method, + const X509V3_CTX *ctx, int gen_type, + const char *value, int is_nc) { if (!value) { OPENSSL_PUT_ERROR(X509V3, X509V3_R_MISSING_VALUE); return NULL; diff --git a/crypto/x509/v3_info.c b/crypto/x509/v3_info.c index eb190de280..ce7e523e11 100644 --- a/crypto/x509/v3_info.c +++ b/crypto/x509/v3_info.c @@ -67,6 +67,9 @@ #include #include +#include "internal.h" + + static STACK_OF(CONF_VALUE) *i2v_AUTHORITY_INFO_ACCESS( const X509V3_EXT_METHOD *method, void *ext, STACK_OF(CONF_VALUE) *ret); static void *v2i_AUTHORITY_INFO_ACCESS(const X509V3_EXT_METHOD *method, @@ -206,8 +209,3 @@ static void *v2i_AUTHORITY_INFO_ACCESS(const X509V3_EXT_METHOD *method, sk_ACCESS_DESCRIPTION_pop_free(ainfo, ACCESS_DESCRIPTION_free); return NULL; } - -int i2a_ACCESS_DESCRIPTION(BIO *bp, const ACCESS_DESCRIPTION *a) { - i2a_ASN1_OBJECT(bp, a->method); - return 2; -} diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c index 24b59c65ff..6ef3cb1c94 100644 --- a/crypto/x509/v3_purp.c +++ b/crypto/x509/v3_purp.c @@ -500,7 +500,7 @@ int x509v3_cache_extensions(X509 *x) { x->ex_flags |= EXFLAG_SI; // If SKID matches AKID also indicate self signed if (X509_check_akid(x, x->akid) == X509_V_OK && - !ku_reject(x, KU_KEY_CERT_SIGN)) { + !ku_reject(x, X509v3_KU_KEY_CERT_SIGN)) { x->ex_flags |= EXFLAG_SS; } } @@ -539,7 +539,7 @@ int x509v3_cache_extensions(X509 *x) { // otherwise. static int check_ca(const X509 *x) { // keyUsage if present should allow cert signing - if (ku_reject(x, KU_KEY_CERT_SIGN)) { + if (ku_reject(x, X509v3_KU_KEY_CERT_SIGN)) { return 0; } // Version 1 certificates are considered CAs and don't have extensions. @@ -566,7 +566,7 @@ static int check_purpose_ssl_client(const X509_PURPOSE *xp, const X509 *x, return check_ca(x); } // We need to do digital signatures or key agreement - if (ku_reject(x, KU_DIGITAL_SIGNATURE | KU_KEY_AGREEMENT)) { + if (ku_reject(x, X509v3_KU_DIGITAL_SIGNATURE | X509v3_KU_KEY_AGREEMENT)) { return 0; } // nsCertType if present should allow SSL client use @@ -579,7 +579,9 @@ static int check_purpose_ssl_client(const X509_PURPOSE *xp, const X509 *x, // Key usage needed for TLS/SSL server: digital signature, encipherment or // key agreement. The ssl code can check this more thoroughly for individual // key types. -#define KU_TLS (KU_DIGITAL_SIGNATURE | KU_KEY_ENCIPHERMENT | KU_KEY_AGREEMENT) +#define X509v3_KU_TLS \ + (X509v3_KU_DIGITAL_SIGNATURE | X509v3_KU_KEY_ENCIPHERMENT | \ + X509v3_KU_KEY_AGREEMENT) static int check_purpose_ssl_server(const X509_PURPOSE *xp, const X509 *x, int ca) { @@ -593,7 +595,7 @@ static int check_purpose_ssl_server(const X509_PURPOSE *xp, const X509 *x, if (ns_reject(x, NS_SSL_SERVER)) { return 0; } - if (ku_reject(x, KU_TLS)) { + if (ku_reject(x, X509v3_KU_TLS)) { return 0; } @@ -608,7 +610,7 @@ static int check_purpose_ns_ssl_server(const X509_PURPOSE *xp, const X509 *x, return ret; } // We need to encipher or Netscape complains - if (ku_reject(x, KU_KEY_ENCIPHERMENT)) { + if (ku_reject(x, X509v3_KU_KEY_ENCIPHERMENT)) { return 0; } return ret; @@ -641,7 +643,7 @@ static int check_purpose_smime_sign(const X509_PURPOSE *xp, const X509 *x, if (!ret || ca) { return ret; } - if (ku_reject(x, KU_DIGITAL_SIGNATURE | KU_NON_REPUDIATION)) { + if (ku_reject(x, X509v3_KU_DIGITAL_SIGNATURE | X509v3_KU_NON_REPUDIATION)) { return 0; } return ret; @@ -654,7 +656,7 @@ static int check_purpose_smime_encrypt(const X509_PURPOSE *xp, const X509 *x, if (!ret || ca) { return ret; } - if (ku_reject(x, KU_KEY_ENCIPHERMENT)) { + if (ku_reject(x, X509v3_KU_KEY_ENCIPHERMENT)) { return 0; } return ret; @@ -665,7 +667,7 @@ static int check_purpose_crl_sign(const X509_PURPOSE *xp, const X509 *x, if (ca) { return check_ca(x); } - if (ku_reject(x, KU_CRL_SIGN)) { + if (ku_reject(x, X509v3_KU_CRL_SIGN)) { return 0; } return 1; @@ -696,8 +698,10 @@ static int check_purpose_timestamp_sign(const X509_PURPOSE *xp, const X509 *x, // and/or nonRepudiation (other values are not consistent and shall // be rejected). if ((x->ex_flags & EXFLAG_KUSAGE) && - ((x->ex_kusage & ~(KU_NON_REPUDIATION | KU_DIGITAL_SIGNATURE)) || - !(x->ex_kusage & (KU_NON_REPUDIATION | KU_DIGITAL_SIGNATURE)))) { + ((x->ex_kusage & + ~(X509v3_KU_NON_REPUDIATION | X509v3_KU_DIGITAL_SIGNATURE)) || + !(x->ex_kusage & + (X509v3_KU_NON_REPUDIATION | X509v3_KU_DIGITAL_SIGNATURE)))) { return 0; } @@ -744,7 +748,7 @@ int X509_check_issued(X509 *issuer, X509 *subject) { } } - if (ku_reject(issuer, KU_KEY_CERT_SIGN)) { + if (ku_reject(issuer, X509v3_KU_KEY_CERT_SIGN)) { return X509_V_ERR_KEYUSAGE_NO_CERTSIGN; } return X509_V_OK; @@ -803,6 +807,9 @@ uint32_t X509_get_key_usage(X509 *x) { if (x->ex_flags & EXFLAG_KUSAGE) { return x->ex_kusage; } + // If there is no extension, key usage is unconstrained, so set all bits to + // one. Note that, although we use |UINT32_MAX|, |ex_kusage| only contains the + // first 16 bits when the extension is present. return UINT32_MAX; } @@ -813,6 +820,8 @@ uint32_t X509_get_extended_key_usage(X509 *x) { if (x->ex_flags & EXFLAG_XKUSAGE) { return x->ex_xkusage; } + // If there is no extension, extended key usage is unconstrained, so set all + // bits to one. return UINT32_MAX; } diff --git a/crypto/x509/v3_utl.c b/crypto/x509/v3_utl.c index dbeabd0f9b..7428014dff 100644 --- a/crypto/x509/v3_utl.c +++ b/crypto/x509/v3_utl.c @@ -555,7 +555,7 @@ static int sk_strcmp(const char *const *a, const char *const *b) { return strcmp(*a, *b); } -STACK_OF(OPENSSL_STRING) *X509_get1_email(X509 *x) { +STACK_OF(OPENSSL_STRING) *X509_get1_email(const X509 *x) { GENERAL_NAMES *gens; STACK_OF(OPENSSL_STRING) *ret; @@ -565,7 +565,7 @@ STACK_OF(OPENSSL_STRING) *X509_get1_email(X509 *x) { return ret; } -STACK_OF(OPENSSL_STRING) *X509_get1_ocsp(X509 *x) { +STACK_OF(OPENSSL_STRING) *X509_get1_ocsp(const X509 *x) { AUTHORITY_INFO_ACCESS *info; STACK_OF(OPENSSL_STRING) *ret = NULL; size_t i; @@ -588,7 +588,7 @@ STACK_OF(OPENSSL_STRING) *X509_get1_ocsp(X509 *x) { return ret; } -STACK_OF(OPENSSL_STRING) *X509_REQ_get1_email(X509_REQ *x) { +STACK_OF(OPENSSL_STRING) *X509_REQ_get1_email(const X509_REQ *x) { GENERAL_NAMES *gens; STACK_OF(X509_EXTENSION) *exts; STACK_OF(OPENSSL_STRING) *ret; @@ -1155,12 +1155,8 @@ ASN1_OCTET_STRING *a2i_IPADDRESS_NC(const char *ipasc) { return ret; err: - if (iptmp) { - OPENSSL_free(iptmp); - } - if (ret) { - ASN1_OCTET_STRING_free(ret); - } + OPENSSL_free(iptmp); + ASN1_OCTET_STRING_free(ret); return NULL; } diff --git a/crypto/x509/x509_req.c b/crypto/x509/x509_req.c index de11723506..98d561e5f9 100644 --- a/crypto/x509/x509_req.c +++ b/crypto/x509/x509_req.c @@ -123,7 +123,7 @@ int X509_REQ_extension_nid(int req_nid) { return req_nid == NID_ext_req || req_nid == NID_ms_ext_req; } -STACK_OF(X509_EXTENSION) *X509_REQ_get_extensions(X509_REQ *req) { +STACK_OF(X509_EXTENSION) *X509_REQ_get_extensions(const X509_REQ *req) { if (req == NULL || req->req_info == NULL) { return NULL; } @@ -136,8 +136,10 @@ STACK_OF(X509_EXTENSION) *X509_REQ_get_extensions(X509_REQ *req) { return NULL; } - X509_ATTRIBUTE *attr = X509_REQ_get_attr(req, idx); - ASN1_TYPE *ext = X509_ATTRIBUTE_get0_type(attr, 0); + const X509_ATTRIBUTE *attr = X509_REQ_get_attr(req, idx); + // TODO(davidben): |X509_ATTRIBUTE_get0_type| is not const-correct. It should + // take and return a const pointer. + const ASN1_TYPE *ext = X509_ATTRIBUTE_get0_type((X509_ATTRIBUTE *)attr, 0); if (!ext || ext->type != V_ASN1_SEQUENCE) { return NULL; } diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index f99d464226..1cb706c0d2 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -1209,7 +1209,7 @@ static int check_crl(X509_STORE_CTX *ctx, X509_CRL *crl) { if (issuer) { // Check for cRLSign bit if keyUsage present if ((issuer->ex_flags & EXFLAG_KUSAGE) && - !(issuer->ex_kusage & KU_CRL_SIGN)) { + !(issuer->ex_kusage & X509v3_KU_CRL_SIGN)) { ctx->error = X509_V_ERR_KEYUSAGE_NO_CRL_SIGN; ok = ctx->verify_cb(0, ctx); if (!ok) { diff --git a/include/openssl/base.h b/include/openssl/base.h index 20be65f086..ba8e5a082b 100644 --- a/include/openssl/base.h +++ b/include/openssl/base.h @@ -378,6 +378,7 @@ typedef struct trust_token_client_st TRUST_TOKEN_CLIENT; typedef struct trust_token_issuer_st TRUST_TOKEN_ISSUER; typedef struct trust_token_method_st TRUST_TOKEN_METHOD; typedef struct v3_ext_ctx X509V3_CTX; +typedef struct v3_ext_method X509V3_EXT_METHOD; typedef struct x509_attributes_st X509_ATTRIBUTE; typedef struct x509_lookup_st X509_LOOKUP; typedef struct x509_lookup_method_st X509_LOOKUP_METHOD; diff --git a/include/openssl/x509.h b/include/openssl/x509.h index 6ab021dfd9..6ad3a3190f 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -233,6 +233,45 @@ OPENSSL_EXPORT void X509_get0_uids(const X509 *x509, const ASN1_BIT_STRING **out_issuer_uid, const ASN1_BIT_STRING **out_subject_uid); +// The following bits are returned from |X509_get_extension_flags|. + +// EXFLAG_BCONS indicates the certificate has a basic constraints extension. +#define EXFLAG_BCONS 0x1 +// EXFLAG_KUSAGE indicates the certifcate has a key usage extension. +#define EXFLAG_KUSAGE 0x2 +// EXFLAG_XKUSAGE indicates the certifcate has an extended key usage extension. +#define EXFLAG_XKUSAGE 0x4 +// EXFLAG_NSCERT indicates the certificate has a legacy Netscape certificate +// type extension. +#define EXFLAG_NSCERT 0x8 +// EXFLAG_CA indicates the certificate has a basic constraints extension with +// the CA bit set. +#define EXFLAG_CA 0x10 +// EXFLAG_SI indicates the certificate is self-issued, i.e. its subject and +// issuer names match. +#define EXFLAG_SI 0x20 +// EXFLAG_V1 indicates an X.509v1 certificate. +#define EXFLAG_V1 0x40 +// EXFLAG_INVALID indicates an error processing some extension. The certificate +// should not be accepted. Note the lack of this bit does not imply all +// extensions are valid, only those used to compute extension flags. +#define EXFLAG_INVALID 0x80 +// EXFLAG_SET is an internal bit that indicates extension flags were computed. +#define EXFLAG_SET 0x100 +// EXFLAG_CRITICAL indicates an unsupported critical extension. The certificate +// should not be accepted. +#define EXFLAG_CRITICAL 0x200 +// EXFLAG_SS indicates the certificate is likely self-signed. That is, if it is +// self-issued, its authority key identifer (if any) matches itself, and its key +// usage extension (if any) allows certificate signatures. The signature itself +// is not checked in computing this bit. +#define EXFLAG_SS 0x2000 + +// X509_get_extension_flags decodes a set of extensions from |x509| and returns +// a collection of |EXFLAG_*| bits which reflect |x509|. If there was an error +// in computing this bitmask, the result will include the |EXFLAG_INVALID| bit. +OPENSSL_EXPORT uint32_t X509_get_extension_flags(X509 *x509); + // X509_get_pathlen returns path length constraint from the basic constraints // extension in |x509|. (See RFC 5280, section 4.2.1.9.) It returns -1 if the // constraint is not present, or if some extension in |x509| was invalid. @@ -242,6 +281,101 @@ OPENSSL_EXPORT void X509_get0_uids(const X509 *x509, // |X509_get_extensions_flags| and check the |EXFLAG_INVALID| bit. OPENSSL_EXPORT long X509_get_pathlen(X509 *x509); +// X509v3_KU_* are key usage bits returned from |X509_get_key_usage|. +#define X509v3_KU_DIGITAL_SIGNATURE 0x0080 +#define X509v3_KU_NON_REPUDIATION 0x0040 +#define X509v3_KU_KEY_ENCIPHERMENT 0x0020 +#define X509v3_KU_DATA_ENCIPHERMENT 0x0010 +#define X509v3_KU_KEY_AGREEMENT 0x0008 +#define X509v3_KU_KEY_CERT_SIGN 0x0004 +#define X509v3_KU_CRL_SIGN 0x0002 +#define X509v3_KU_ENCIPHER_ONLY 0x0001 +#define X509v3_KU_DECIPHER_ONLY 0x8000 + +// X509_get_key_usage returns a bitmask of key usages (see Section 4.2.1.3 of +// RFC 5280) which |x509| is valid for. This function only reports the first 16 +// bits, in a little-endian byte order, but big-endian bit order. That is, bits +// 0 though 7 are reported at 1<<7 through 1<<0, and bits 8 through 15 are +// reported at 1<<15 through 1<<8. +// +// Instead of depending on this bit order, callers should compare against the +// |X509v3_KU_*| constants. +// +// If |x509| has no key usage extension, all key usages are valid and this +// function returns |UINT32_MAX|. If there was an error processing |x509|'s +// extensions, or if the first 16 bits in the key usage extension were all zero, +// this function returns zero. +OPENSSL_EXPORT uint32_t X509_get_key_usage(X509 *x509); + +// XKU_* are extended key usage bits returned from +// |X509_get_extended_key_usage|. +#define XKU_SSL_SERVER 0x1 +#define XKU_SSL_CLIENT 0x2 +#define XKU_SMIME 0x4 +#define XKU_CODE_SIGN 0x8 +#define XKU_SGC 0x10 +#define XKU_OCSP_SIGN 0x20 +#define XKU_TIMESTAMP 0x40 +#define XKU_DVCS 0x80 +#define XKU_ANYEKU 0x100 + +// X509_get_extended_key_usage returns a bitmask of extended key usages (see +// Section 4.2.1.12 of RFC 5280) which |x509| is valid for. The result will be +// a combination of |XKU_*| constants. If checking an extended key usage not +// defined above, callers should extract the extended key usage extension +// separately, e.g. via |X509_get_ext_d2i|. +// +// If |x509| has no extended key usage extension, all extended key usages are +// valid and this function returns |UINT32_MAX|. If there was an error +// processing |x509|'s extensions, or if |x509|'s extended key usage extension +// contained no recognized usages, this function returns zero. +OPENSSL_EXPORT uint32_t X509_get_extended_key_usage(X509 *x509); + +// X509_get0_subject_key_id returns |x509|'s subject key identifier, if present. +// (See RFC 5280, section 4.2.1.2.) It returns NULL if the extension is not +// present or if some extension in |x509| was invalid. +// +// TODO(crbug.com/boringssl/381): Decoding an |X509| object will not check for +// invalid extensions. To detect the error case, call +// |X509_get_extensions_flags| and check the |EXFLAG_INVALID| bit. +OPENSSL_EXPORT const ASN1_OCTET_STRING *X509_get0_subject_key_id(X509 *x509); + +// X509_get0_authority_key_id returns keyIdentifier of |x509|'s authority key +// identifier, if the extension and field are present. (See RFC 5280, +// section 4.2.1.1.) It returns NULL if the extension is not present, if it is +// present but lacks a keyIdentifier field, or if some extension in |x509| was +// invalid. +// +// TODO(crbug.com/boringssl/381): Decoding an |X509| object will not check for +// invalid extensions. To detect the error case, call +// |X509_get_extensions_flags| and check the |EXFLAG_INVALID| bit. +OPENSSL_EXPORT const ASN1_OCTET_STRING *X509_get0_authority_key_id(X509 *x509); + +DEFINE_STACK_OF(GENERAL_NAME) +typedef STACK_OF(GENERAL_NAME) GENERAL_NAMES; + +// X509_get0_authority_issuer returns the authorityCertIssuer of |x509|'s +// authority key identifier, if the extension and field are present. (See +// RFC 5280, section 4.2.1.1.) It returns NULL if the extension is not present, +// if it is present but lacks a authorityCertIssuer field, or if some extension +// in |x509| was invalid. +// +// TODO(crbug.com/boringssl/381): Decoding an |X509| object will not check for +// invalid extensions. To detect the error case, call +// |X509_get_extensions_flags| and check the |EXFLAG_INVALID| bit. +OPENSSL_EXPORT const GENERAL_NAMES *X509_get0_authority_issuer(X509 *x509); + +// X509_get0_authority_serial returns the authorityCertSerialNumber of |x509|'s +// authority key identifier, if the extension and field are present. (See +// RFC 5280, section 4.2.1.1.) It returns NULL if the extension is not present, +// if it is present but lacks a authorityCertSerialNumber field, or if some +// extension in |x509| was invalid. +// +// TODO(crbug.com/boringssl/381): Decoding an |X509| object will not check for +// invalid extensions. To detect the error case, call +// |X509_get_extensions_flags| and check the |EXFLAG_INVALID| bit. +OPENSSL_EXPORT const ASN1_INTEGER *X509_get0_authority_serial(X509 *x509); + // X509_get0_extensions returns |x509|'s extension list, or NULL if |x509| omits // it. OPENSSL_EXPORT const STACK_OF(X509_EXTENSION) *X509_get0_extensions( @@ -332,6 +466,30 @@ OPENSSL_EXPORT int i2d_X509_tbs(X509 *x509, unsigned char **outp); // validation. OPENSSL_EXPORT int X509_verify(X509 *x509, EVP_PKEY *pkey); +// X509_get1_email returns a newly-allocated list of NUL-terminated strings +// containing all email addresses in |x509|'s subject and all rfc822name names +// in |x509|'s subject alternative names. Email addresses which contain embedded +// NUL bytes are skipped. +// +// On error, or if there are no such email addresses, it returns NULL. When +// done, the caller must release the result with |X509_email_free|. +OPENSSL_EXPORT STACK_OF(OPENSSL_STRING) *X509_get1_email(const X509 *x509); + +// X509_get1_ocsp returns a newly-allocated list of NUL-terminated strings +// containing all OCSP URIs in |x509|. That is, it collects all URI +// AccessDescriptions with an accessMethod of id-ad-ocsp in |x509|'s authority +// information access extension. URIs which contain embedded NUL bytes are +// skipped. +// +// On error, or if there are no such URIs, it returns NULL. When done, the +// caller must release the result with |X509_email_free|. +OPENSSL_EXPORT STACK_OF(OPENSSL_STRING) *X509_get1_ocsp(const X509 *x509); + +// X509_email_free releases memory associated with |sk|, including |sk| itself. +// Each |OPENSSL_STRING| in |sk| must be a NUL-terminated string allocated with +// |OPENSSL_malloc|. If |sk| is NULL, no action is taken. +OPENSSL_EXPORT void X509_email_free(STACK_OF(OPENSSL_STRING) *sk); + // Issuing certificates. // @@ -1033,16 +1191,18 @@ OPENSSL_EXPORT int X509_REQ_get_attr_by_OBJ(const X509_REQ *req, // (a Microsoft szOID_CERT_EXTENSIONS variant). OPENSSL_EXPORT int X509_REQ_extension_nid(int nid); -// X509_REQ_get_extensions decodes the list of requested extensions in |req| and -// returns a newly-allocated |STACK_OF(X509_EXTENSION)| containing the result. -// It returns NULL on error, or if |req| did not request extensions. +// X509_REQ_get_extensions decodes the most preferred list of requested +// extensions in |req| and returns a newly-allocated |STACK_OF(X509_EXTENSION)| +// containing the result. It returns NULL on error, or if |req| did not request +// extensions. // // CSRs do not store extensions directly. Instead there are attribute types // which are defined to hold extensions. See |X509_REQ_extension_nid|. This // function supports both pkcs-9-at-extensionRequest from RFC 2985 and the // Microsoft szOID_CERT_EXTENSIONS variant. If both are present, // pkcs-9-at-extensionRequest is preferred. -OPENSSL_EXPORT STACK_OF(X509_EXTENSION) *X509_REQ_get_extensions(X509_REQ *req); +OPENSSL_EXPORT STACK_OF(X509_EXTENSION) *X509_REQ_get_extensions( + const X509_REQ *req); // X509_REQ_get0_signature sets |*out_sig| and |*out_alg| to the signature and // signature algorithm of |req|, respectively. Either output pointer may be NULL @@ -1060,6 +1220,17 @@ OPENSSL_EXPORT int X509_REQ_get_signature_nid(const X509_REQ *req); // one if the signature is valid and zero otherwise. OPENSSL_EXPORT int X509_REQ_verify(X509_REQ *req, EVP_PKEY *pkey); +// X509_REQ_get1_email returns a newly-allocated list of NUL-terminated strings +// containing all email addresses in |req|'s subject and all rfc822name names +// in |req|'s subject alternative names. The subject alternative names extension +// is extracted from the result of |X509_REQ_get_extensions|. Email addresses +// which contain embedded NUL bytes are skipped. +// +// On error, or if there are no such email addresses, it returns NULL. When +// done, the caller must release the result with |X509_email_free|. +OPENSSL_EXPORT STACK_OF(OPENSSL_STRING) *X509_REQ_get1_email( + const X509_REQ *req); + // Issuing certificate requests. // @@ -2852,6 +3023,85 @@ OPENSSL_EXPORT X509_EXTENSION *X509V3_EXT_conf(LHASH_OF(CONF_VALUE) *conf, const char *name, const char *value); +// i2s_ASN1_OCTET_STRING returns a human-readable representation of |oct| as a +// newly-allocated, NUL-terminated string, or NULL on error. |method| is +// ignored. The caller must release the result with |OPENSSL_free| when done. +OPENSSL_EXPORT char *i2s_ASN1_OCTET_STRING(const X509V3_EXT_METHOD *method, + const ASN1_OCTET_STRING *oct); + +// s2i_ASN1_OCTET_STRING decodes |str| as a hexdecimal byte string, with +// optional colon separators between bytes. It returns a newly-allocated +// |ASN1_OCTET_STRING| with the result on success, or NULL on error. |method| +// and |ctx| are ignored. +OPENSSL_EXPORT ASN1_OCTET_STRING *s2i_ASN1_OCTET_STRING( + const X509V3_EXT_METHOD *method, const X509V3_CTX *ctx, const char *str); + +// i2s_ASN1_INTEGER returns a human-readable representation of |aint| as a +// newly-allocated, NUL-terminated string, or NULL on error. |method| is +// ignored. The caller must release the result with |OPENSSL_free| when done. +OPENSSL_EXPORT char *i2s_ASN1_INTEGER(const X509V3_EXT_METHOD *method, + const ASN1_INTEGER *aint); + +// s2i_ASN1_INTEGER decodes |value| as the ASCII representation of an integer, +// and returns a newly-allocated |ASN1_INTEGER| containing the result, or NULL +// on error. |method| is ignored. If |value| begins with "0x" or "0X", the input +// is decoded in hexadecimal, otherwise decimal. +OPENSSL_EXPORT ASN1_INTEGER *s2i_ASN1_INTEGER(const X509V3_EXT_METHOD *method, + const char *value); + +// i2s_ASN1_ENUMERATED returns a human-readable representation of |aint| as a +// newly-allocated, NUL-terminated string, or NULL on error. |method| is +// ignored. The caller must release the result with |OPENSSL_free| when done. +OPENSSL_EXPORT char *i2s_ASN1_ENUMERATED(const X509V3_EXT_METHOD *method, + const ASN1_ENUMERATED *aint); + +// X509V3_conf_free releases memory associated with |CONF_VALUE|. +OPENSSL_EXPORT void X509V3_conf_free(CONF_VALUE *val); + +// i2v_GENERAL_NAME serializes |gen| as a |CONF_VALUE|. If |ret| is non-NULL, it +// appends the value to |ret| and returns |ret| on success or NULL on error. If +// it returns NULL, the caller is still responsible for freeing |ret|. If |ret| +// is NULL, it returns a newly-allocated |STACK_OF(CONF_VALUE)| containing the +// result. |method| is ignored. When done, the caller should release the result +// with |sk_CONF_VALUE_pop_free| and |X509V3_conf_free|. +// +// Do not use this function. This is an internal implementation detail of the +// human-readable print functions. If extracting a SAN list from a certificate, +// look at |gen| directly. +OPENSSL_EXPORT STACK_OF(CONF_VALUE) *i2v_GENERAL_NAME( + const X509V3_EXT_METHOD *method, const GENERAL_NAME *gen, + STACK_OF(CONF_VALUE) *ret); + +// i2v_GENERAL_NAMES serializes |gen| as a list of |CONF_VALUE|s. If |ret| is +// non-NULL, it appends the values to |ret| and returns |ret| on success or NULL +// on error. If it returns NULL, the caller is still responsible for freeing +// |ret|. If |ret| is NULL, it returns a newly-allocated |STACK_OF(CONF_VALUE)| +// containing the results. |method| is ignored. +// +// Do not use this function. This is an internal implementation detail of the +// human-readable print functions. If extracting a SAN list from a certificate, +// look at |gen| directly. +OPENSSL_EXPORT STACK_OF(CONF_VALUE) *i2v_GENERAL_NAMES( + const X509V3_EXT_METHOD *method, const GENERAL_NAMES *gen, + STACK_OF(CONF_VALUE) *extlist); + +// a2i_IPADDRESS decodes |ipasc| as the textual representation of an IPv4 or +// IPv6 address. On success, it returns a newly-allocated |ASN1_OCTET_STRING| +// containing the decoded IP address. IPv4 addresses are represented as 4-byte +// strings and IPv6 addresses as 16-byte strings. On failure, it returns NULL. +OPENSSL_EXPORT ASN1_OCTET_STRING *a2i_IPADDRESS(const char *ipasc); + +// a2i_IPADDRESS_NC decodes |ipasc| as the textual representation of an IPv4 or +// IPv6 address range. On success, it returns a newly-allocated +// |ASN1_OCTET_STRING| containing the decoded IP address, followed by the +// decoded mask. IPv4 ranges are represented as 8-byte strings and IPv6 ranges +// as 32-byte strings. On failure, it returns NULL. +// +// The text format decoded by this function is not the standard CIDR notiation. +// Instead, the mask after the "/" is represented as another IP address. For +// example, "192.168.0.0/16" would be written "192.168.0.0/255.255.0.0". +OPENSSL_EXPORT ASN1_OCTET_STRING *a2i_IPADDRESS_NC(const char *ipasc); + // Deprecated functions. @@ -2958,6 +3208,17 @@ OPENSSL_EXPORT STACK_OF(CONF_VALUE) *X509V3_parse_list(const char *line); #define X509_STORE_get1_certs X509_STORE_CTX_get1_certs #define X509_STORE_get1_crls X509_STORE_CTX_get1_crls +// The following constants are legacy aliases for |X509v3_KU_*|. +#define KU_DIGITAL_SIGNATURE X509v3_KU_DIGITAL_SIGNATURE +#define KU_NON_REPUDIATION X509v3_KU_NON_REPUDIATION +#define KU_KEY_ENCIPHERMENT X509v3_KU_KEY_ENCIPHERMENT +#define KU_DATA_ENCIPHERMENT X509v3_KU_DATA_ENCIPHERMENT +#define KU_KEY_AGREEMENT X509v3_KU_KEY_AGREEMENT +#define KU_KEY_CERT_SIGN X509v3_KU_KEY_CERT_SIGN +#define KU_CRL_SIGN X509v3_KU_CRL_SIGN +#define KU_ENCIPHER_ONLY X509v3_KU_ENCIPHER_ONLY +#define KU_DECIPHER_ONLY X509v3_KU_DECIPHER_ONLY + // Private structures. @@ -2969,17 +3230,6 @@ struct X509_algor_st { // Functions below this point have not yet been organized into sections. -#define X509v3_KU_DIGITAL_SIGNATURE 0x0080 -#define X509v3_KU_NON_REPUDIATION 0x0040 -#define X509v3_KU_KEY_ENCIPHERMENT 0x0020 -#define X509v3_KU_DATA_ENCIPHERMENT 0x0010 -#define X509v3_KU_KEY_AGREEMENT 0x0008 -#define X509v3_KU_KEY_CERT_SIGN 0x0004 -#define X509v3_KU_CRL_SIGN 0x0002 -#define X509v3_KU_ENCIPHER_ONLY 0x0001 -#define X509v3_KU_DECIPHER_ONLY 0x8000 -#define X509v3_KU_UNDEF 0xffff - // This stuff is certificate "auxiliary info" // it contains details which are useful in certificate // stores and databases. When used this is tagged onto @@ -3717,14 +3967,6 @@ OPENSSL_EXPORT int X509_VERIFY_PARAM_set1_ip_asc(X509_VERIFY_PARAM *param, OPENSSL_EXPORT int X509_VERIFY_PARAM_get_depth(const X509_VERIFY_PARAM *param); -// Forward reference -struct v3_ext_method; -struct v3_ext_ctx; - -// Useful typedefs - -typedef struct v3_ext_method X509V3_EXT_METHOD; - typedef void *(*X509V3_EXT_NEW)(void); typedef void (*X509V3_EXT_FREE)(void *); typedef void *(*X509V3_EXT_D2I)(void *, const unsigned char **, long); @@ -3837,10 +4079,6 @@ struct GENERAL_NAME_st { } d; } /* GENERAL_NAME */; -DEFINE_STACK_OF(GENERAL_NAME) - -typedef STACK_OF(GENERAL_NAME) GENERAL_NAMES; - DEFINE_STACK_OF(GENERAL_NAMES) typedef struct ACCESS_DESCRIPTION_st { @@ -3962,35 +4200,6 @@ struct ISSUING_DIST_POINT_st { // X509_PURPOSE stuff -#define EXFLAG_BCONS 0x1 -#define EXFLAG_KUSAGE 0x2 -#define EXFLAG_XKUSAGE 0x4 -#define EXFLAG_NSCERT 0x8 - -#define EXFLAG_CA 0x10 -// EXFLAG_SI indicates the certificate is self-issued, i.e. its subject and -// issuer names match. -#define EXFLAG_SI 0x20 -#define EXFLAG_V1 0x40 -#define EXFLAG_INVALID 0x80 -#define EXFLAG_SET 0x100 -#define EXFLAG_CRITICAL 0x200 -// EXFLAG_SS indicates the certificate is likely self-signed. That is, if it is -// self-issued, its authority key identifer (if any) matches itself, and its key -// usage extension (if any) allows certificate signatures. The signature itself -// is not checked in computing this bit. -#define EXFLAG_SS 0x2000 - -#define KU_DIGITAL_SIGNATURE 0x0080 -#define KU_NON_REPUDIATION 0x0040 -#define KU_KEY_ENCIPHERMENT 0x0020 -#define KU_DATA_ENCIPHERMENT 0x0010 -#define KU_KEY_AGREEMENT 0x0008 -#define KU_KEY_CERT_SIGN 0x0004 -#define KU_CRL_SIGN 0x0002 -#define KU_ENCIPHER_ONLY 0x0001 -#define KU_DECIPHER_ONLY 0x8000 - #define NS_SSL_CLIENT 0x80 #define NS_SSL_SERVER 0x40 #define NS_SMIME 0x20 @@ -4000,16 +4209,6 @@ struct ISSUING_DIST_POINT_st { #define NS_OBJSIGN_CA 0x01 #define NS_ANY_CA (NS_SSL_CA | NS_SMIME_CA | NS_OBJSIGN_CA) -#define XKU_SSL_SERVER 0x1 -#define XKU_SSL_CLIENT 0x2 -#define XKU_SMIME 0x4 -#define XKU_CODE_SIGN 0x8 -#define XKU_SGC 0x10 -#define XKU_OCSP_SIGN 0x20 -#define XKU_TIMESTAMP 0x40 -#define XKU_DVCS 0x80 -#define XKU_ANYEKU 0x100 - #define X509_PURPOSE_DYNAMIC 0x1 #define X509_PURPOSE_DYNAMIC_NAME 0x2 @@ -4049,39 +4248,10 @@ DECLARE_ASN1_FUNCTIONS(AUTHORITY_KEYID) DECLARE_ASN1_FUNCTIONS(GENERAL_NAME) OPENSSL_EXPORT GENERAL_NAME *GENERAL_NAME_dup(GENERAL_NAME *a); -// i2v_GENERAL_NAME serializes |gen| as a |CONF_VALUE|. If |ret| is non-NULL, it -// appends the value to |ret| and returns |ret| on success or NULL on error. If -// it returns NULL, the caller is still responsible for freeing |ret|. If |ret| -// is NULL, it returns a newly-allocated |STACK_OF(CONF_VALUE)| containing the -// result. |method| is ignored. -// -// Do not use this function. This is an internal implementation detail of the -// human-readable print functions. If extracting a SAN list from a certificate, -// look at |gen| directly. -OPENSSL_EXPORT STACK_OF(CONF_VALUE) *i2v_GENERAL_NAME( - const X509V3_EXT_METHOD *method, const GENERAL_NAME *gen, - STACK_OF(CONF_VALUE) *ret); - // TODO(https://crbug.com/boringssl/407): This is not const because it contains // an |X509_NAME|. DECLARE_ASN1_FUNCTIONS(GENERAL_NAMES) -// i2v_GENERAL_NAMES serializes |gen| as a list of |CONF_VALUE|s. If |ret| is -// non-NULL, it appends the values to |ret| and returns |ret| on success or NULL -// on error. If it returns NULL, the caller is still responsible for freeing -// |ret|. If |ret| is NULL, it returns a newly-allocated |STACK_OF(CONF_VALUE)| -// containing the results. |method| is ignored. -// -// Do not use this function. This is an internal implementation detail of the -// human-readable print functions. If extracting a SAN list from a certificate, -// look at |gen| directly. -OPENSSL_EXPORT STACK_OF(CONF_VALUE) *i2v_GENERAL_NAMES( - const X509V3_EXT_METHOD *method, const GENERAL_NAMES *gen, - STACK_OF(CONF_VALUE) *extlist); -OPENSSL_EXPORT GENERAL_NAMES *v2i_GENERAL_NAMES( - const X509V3_EXT_METHOD *method, const X509V3_CTX *ctx, - const STACK_OF(CONF_VALUE) *nval); - DECLARE_ASN1_FUNCTIONS_const(OTHERNAME) DECLARE_ASN1_FUNCTIONS_const(EDIPARTYNAME) OPENSSL_EXPORT void GENERAL_NAME_set0_value(GENERAL_NAME *a, int type, @@ -4094,17 +4264,7 @@ OPENSSL_EXPORT int GENERAL_NAME_get0_otherName(const GENERAL_NAME *gen, ASN1_OBJECT **poid, ASN1_TYPE **pvalue); -// i2s_ASN1_OCTET_STRING returns a human-readable representation of |oct| as a -// newly-allocated, NUL-terminated string, or NULL on error. |method| is -// ignored. The caller must release the result with |OPENSSL_free| when done. -OPENSSL_EXPORT char *i2s_ASN1_OCTET_STRING(const X509V3_EXT_METHOD *method, - const ASN1_OCTET_STRING *oct); - -OPENSSL_EXPORT ASN1_OCTET_STRING *s2i_ASN1_OCTET_STRING( - const X509V3_EXT_METHOD *method, const X509V3_CTX *ctx, const char *str); - DECLARE_ASN1_FUNCTIONS_const(EXTENDED_KEY_USAGE) -OPENSSL_EXPORT int i2a_ACCESS_DESCRIPTION(BIO *bp, const ACCESS_DESCRIPTION *a); DECLARE_ASN1_FUNCTIONS_const(CERTIFICATEPOLICIES) DECLARE_ASN1_FUNCTIONS_const(POLICYINFO) @@ -4150,27 +4310,6 @@ DECLARE_ASN1_ALLOC_FUNCTIONS(NAME_CONSTRAINTS) DECLARE_ASN1_ALLOC_FUNCTIONS(POLICY_CONSTRAINTS) DECLARE_ASN1_ITEM(POLICY_CONSTRAINTS) -OPENSSL_EXPORT GENERAL_NAME *a2i_GENERAL_NAME(GENERAL_NAME *out, - const X509V3_EXT_METHOD *method, - const X509V3_CTX *ctx, - int gen_type, const char *value, - int is_nc); - -OPENSSL_EXPORT GENERAL_NAME *v2i_GENERAL_NAME(const X509V3_EXT_METHOD *method, - const X509V3_CTX *ctx, - const CONF_VALUE *cnf); -OPENSSL_EXPORT GENERAL_NAME *v2i_GENERAL_NAME_ex( - GENERAL_NAME *out, const X509V3_EXT_METHOD *method, const X509V3_CTX *ctx, - const CONF_VALUE *cnf, int is_nc); -OPENSSL_EXPORT void X509V3_conf_free(CONF_VALUE *val); - -OPENSSL_EXPORT char *i2s_ASN1_INTEGER(const X509V3_EXT_METHOD *meth, - const ASN1_INTEGER *aint); -OPENSSL_EXPORT ASN1_INTEGER *s2i_ASN1_INTEGER(const X509V3_EXT_METHOD *meth, - const char *value); -OPENSSL_EXPORT char *i2s_ASN1_ENUMERATED(const X509V3_EXT_METHOD *meth, - const ASN1_ENUMERATED *aint); - // X509V3_EXT_add registers |ext| as a custom extension for the extension type // |ext->ext_nid|. |ext| must be valid for the remainder of the address space's // lifetime. It returns one on success and zero on error. @@ -4355,68 +4494,6 @@ OPENSSL_EXPORT int X509_PURPOSE_set(int *p, int purpose); OPENSSL_EXPORT int X509_check_issued(X509 *issuer, X509 *subject); OPENSSL_EXPORT int X509_check_akid(X509 *issuer, AUTHORITY_KEYID *akid); -OPENSSL_EXPORT uint32_t X509_get_extension_flags(X509 *x509); - -// X509_get_key_usage returns a bitmask of key usages (see Section 4.2.1.3 of -// RFC 5280) which |x509| is valid for. The result will be a combination of -// |KU_*| constants. -// -// If |x509| has no key usage extension, all key usages are valid and this -// function returns |UINT32_MAX|. If there was an error processing |x509|'s -// extensions, this function returns zero. -OPENSSL_EXPORT uint32_t X509_get_key_usage(X509 *x509); - -// X509_get_extended_key_usage returns a bitmask of extended key usages (see -// Section 4.2.1.12 of RFC 5280) which |x509| is valid for. The result will be -// a combination of |XKU_*| constants. -// -// If |x509| has no extended key usage extension, all extended key usages are -// valid and this function returns |UINT32_MAX|. If there was an error -// processing |x509|'s extensions, this function returns zero. -OPENSSL_EXPORT uint32_t X509_get_extended_key_usage(X509 *x509); - -// X509_get0_subject_key_id returns |x509|'s subject key identifier, if present. -// (See RFC 5280, section 4.2.1.2.) It returns NULL if the extension is not -// present or if some extension in |x509| was invalid. -// -// Note that decoding an |X509| object will not check for invalid extensions. To -// detect the error case, call |X509_get_extensions_flags| and check the -// |EXFLAG_INVALID| bit. -OPENSSL_EXPORT const ASN1_OCTET_STRING *X509_get0_subject_key_id(X509 *x509); - -// X509_get0_authority_key_id returns keyIdentifier of |x509|'s authority key -// identifier, if the extension and field are present. (See RFC 5280, -// section 4.2.1.1.) It returns NULL if the extension is not present, if it is -// present but lacks a keyIdentifier field, or if some extension in |x509| was -// invalid. -// -// Note that decoding an |X509| object will not check for invalid extensions. To -// detect the error case, call |X509_get_extensions_flags| and check the -// |EXFLAG_INVALID| bit. -OPENSSL_EXPORT const ASN1_OCTET_STRING *X509_get0_authority_key_id(X509 *x509); - -// X509_get0_authority_issuer returns the authorityCertIssuer of |x509|'s -// authority key identifier, if the extension and field are present. (See -// RFC 5280, section 4.2.1.1.) It returns NULL if the extension is not present, -// if it is present but lacks a authorityCertIssuer field, or if some extension -// in |x509| was invalid. -// -// Note that decoding an |X509| object will not check for invalid extensions. To -// detect the error case, call |X509_get_extensions_flags| and check the -// |EXFLAG_INVALID| bit. -OPENSSL_EXPORT const GENERAL_NAMES *X509_get0_authority_issuer(X509 *x509); - -// X509_get0_authority_serial returns the authorityCertSerialNumber of |x509|'s -// authority key identifier, if the extension and field are present. (See -// RFC 5280, section 4.2.1.1.) It returns NULL if the extension is not present, -// if it is present but lacks a authorityCertSerialNumber field, or if some -// extension in |x509| was invalid. -// -// Note that decoding an |X509| object will not check for invalid extensions. To -// detect the error case, call |X509_get_extensions_flags| and check the -// |EXFLAG_INVALID| bit. -OPENSSL_EXPORT const ASN1_INTEGER *X509_get0_authority_serial(X509 *x509); - OPENSSL_EXPORT int X509_PURPOSE_get_count(void); OPENSSL_EXPORT X509_PURPOSE *X509_PURPOSE_get0(int idx); OPENSSL_EXPORT int X509_PURPOSE_get_by_sname(const char *sname); @@ -4432,11 +4509,6 @@ OPENSSL_EXPORT int X509_PURPOSE_get_trust(const X509_PURPOSE *xp); OPENSSL_EXPORT void X509_PURPOSE_cleanup(void); OPENSSL_EXPORT int X509_PURPOSE_get_id(const X509_PURPOSE *); -OPENSSL_EXPORT STACK_OF(OPENSSL_STRING) *X509_get1_email(X509 *x); -OPENSSL_EXPORT STACK_OF(OPENSSL_STRING) *X509_REQ_get1_email(X509_REQ *x); -OPENSSL_EXPORT void X509_email_free(STACK_OF(OPENSSL_STRING) *sk); -OPENSSL_EXPORT STACK_OF(OPENSSL_STRING) *X509_get1_ocsp(X509 *x); - // X509_check_* functions // @@ -4504,9 +4576,6 @@ OPENSSL_EXPORT int X509_check_ip(X509 *x, const unsigned char *chk, OPENSSL_EXPORT int X509_check_ip_asc(X509 *x, const char *ipasc, unsigned int flags); -OPENSSL_EXPORT ASN1_OCTET_STRING *a2i_IPADDRESS(const char *ipasc); -OPENSSL_EXPORT ASN1_OCTET_STRING *a2i_IPADDRESS_NC(const char *ipasc); - #if defined(__cplusplus) } // extern C From c2ae13f88279fdb79e39c8dbd1b913f3a9177144 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 6 Dec 2023 13:43:40 -0500 Subject: [PATCH 07/12] Move KU_* back to It turns out NSS and OpenSSL defined the same constants! All this time, Chromium has inadvertently relied on KU_* being defined in and not . Once we merged the two headers, this broke. Since we just deprecated these in favor of X509v3_KU_*, just move these back into . Change-Id: I93453527f30eee6df7630dc68c052c814aaeda02 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64607 Reviewed-by: Bob Beck Commit-Queue: David Benjamin (cherry picked from commit 540fcce9a5a8d92e4686d5d266dc19c91724ea0b) --- include/openssl/x509v3.h | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h index c8e52be185..80edd940e2 100644 --- a/include/openssl/x509v3.h +++ b/include/openssl/x509v3.h @@ -12,6 +12,28 @@ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -// This header is provided in order to make compiling against code that expects -// OpenSSL easier. +#ifndef OPENSSL_HEADER_X509V3_H +#define OPENSSL_HEADER_X509V3_H + +// This header primarily exists in order to make compiling against code that +// expects OpenSSL easier. We have merged this header into . +// However, due to conflicts, some deprecated symbols are defined here. #include + +// Deprecated constants. + +// The following constants are legacy aliases for |X509v3_KU_*|. They are +// defined here instead of in because NSS's public headers use +// the same symbols. Some callers have inadvertently relied on the conflicts +// only being defined in this header. +#define KU_DIGITAL_SIGNATURE X509v3_KU_DIGITAL_SIGNATURE +#define KU_NON_REPUDIATION X509v3_KU_NON_REPUDIATION +#define KU_KEY_ENCIPHERMENT X509v3_KU_KEY_ENCIPHERMENT +#define KU_DATA_ENCIPHERMENT X509v3_KU_DATA_ENCIPHERMENT +#define KU_KEY_AGREEMENT X509v3_KU_KEY_AGREEMENT +#define KU_KEY_CERT_SIGN X509v3_KU_KEY_CERT_SIGN +#define KU_CRL_SIGN X509v3_KU_CRL_SIGN +#define KU_ENCIPHER_ONLY X509v3_KU_ENCIPHER_ONLY +#define KU_DECIPHER_ONLY X509v3_KU_DECIPHER_ONLY + +#endif // OPENSSL_HEADER_X509V3_H From 987521dc7ad089cfec3d1d892e20a6bdcaebab7c Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 6 Dec 2023 15:02:10 -0500 Subject: [PATCH 08/12] Actually remove KU_* from x509.h I forgot half of https://boringssl-review.googlesource.com/c/boringssl/+/64607 Change-Id: Idbb827c5298c08bb1344272353ba4740802c55de Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64627 Reviewed-by: Bob Beck Commit-Queue: David Benjamin (cherry picked from commit 85e2f2c655a13e29988df777ed680ddf0969434d) --- include/openssl/x509.h | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/include/openssl/x509.h b/include/openssl/x509.h index 6ad3a3190f..012da8ee8d 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -3208,17 +3208,6 @@ OPENSSL_EXPORT STACK_OF(CONF_VALUE) *X509V3_parse_list(const char *line); #define X509_STORE_get1_certs X509_STORE_CTX_get1_certs #define X509_STORE_get1_crls X509_STORE_CTX_get1_crls -// The following constants are legacy aliases for |X509v3_KU_*|. -#define KU_DIGITAL_SIGNATURE X509v3_KU_DIGITAL_SIGNATURE -#define KU_NON_REPUDIATION X509v3_KU_NON_REPUDIATION -#define KU_KEY_ENCIPHERMENT X509v3_KU_KEY_ENCIPHERMENT -#define KU_DATA_ENCIPHERMENT X509v3_KU_DATA_ENCIPHERMENT -#define KU_KEY_AGREEMENT X509v3_KU_KEY_AGREEMENT -#define KU_KEY_CERT_SIGN X509v3_KU_KEY_CERT_SIGN -#define KU_CRL_SIGN X509v3_KU_CRL_SIGN -#define KU_ENCIPHER_ONLY X509v3_KU_ENCIPHER_ONLY -#define KU_DECIPHER_ONLY X509v3_KU_DECIPHER_ONLY - // Private structures. From 74c1f2a2d130d50ca16af53b1e81832961cbb863 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sat, 2 Dec 2023 09:21:37 -0500 Subject: [PATCH 09/12] Remove dynamic X509_TRUST and X509_PURPOSE registration This is not thread-safe. Even if made thread-safe, it involves registering globals, so it's just not a good API. Note this means that there is no longer a way to configure custom trust OIDs or purpose checks. Evidently no one was doing that. Should a use case arise, I don't think it should be met by this API. The things one might want to configure here are: - Which OID to match against X509_add1_trust_object and X509_add1_reject_object - Whether self-signed certificates, if no trust objects are configured, also count as trust anchors - Which EKU OID to look for up the chain - Which legacy Netscape certificate type to look for (can we remove this?) - Which key usage bits to look for in the leaf We can simply add APIs for specifying those if we need them. Interestingly, there's a call to check_ca inside the purpose checks (which gets skipped if you don't configure a purpose!), but I think it may be redundant with the X509_check_ca call in the path verifier. Change-Id: If71ee3d0768b5fc71422852b4fcf7eb23e937dd2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64507 Reviewed-by: Bob Beck Commit-Queue: David Benjamin (cherry picked from commit 396f2ef0855fee63cd7fd2f60fc77b0a447b1dc7) --- crypto/x509/v3_purp.c | 137 ++--------------------------------------- crypto/x509/x509_trs.c | 126 ++----------------------------------- include/openssl/x509.h | 19 ------ 3 files changed, 11 insertions(+), 271 deletions(-) diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c index 6ef3cb1c94..7ec3ad1733 100644 --- a/crypto/x509/v3_purp.c +++ b/crypto/x509/v3_purp.c @@ -94,9 +94,6 @@ static int check_purpose_timestamp_sign(const X509_PURPOSE *xp, const X509 *x, static int no_check(const X509_PURPOSE *xp, const X509 *x, int ca); static int ocsp_helper(const X509_PURPOSE *xp, const X509 *x, int ca); -static int xp_cmp(const X509_PURPOSE *const *a, const X509_PURPOSE *const *b); -static void xptable_free(X509_PURPOSE *p); - static X509_PURPOSE xstandard[] = { {X509_PURPOSE_SSL_CLIENT, X509_TRUST_SSL_CLIENT, 0, check_purpose_ssl_client, (char *)"SSL client", (char *)"sslclient", NULL}, @@ -121,14 +118,6 @@ static X509_PURPOSE xstandard[] = { (char *)"timestampsign", NULL}, }; -#define X509_PURPOSE_COUNT (sizeof(xstandard) / sizeof(X509_PURPOSE)) - -static STACK_OF(X509_PURPOSE) *xptable = NULL; - -static int xp_cmp(const X509_PURPOSE *const *a, const X509_PURPOSE *const *b) { - return (*a)->purpose - (*b)->purpose; -} - // As much as I'd like to make X509_check_purpose use a "const" X509* I // really can't because it does recalculate hashes and do other non-const // things. If |id| is -1 it just calls |x509v3_cache_extensions| for its @@ -161,21 +150,13 @@ int X509_PURPOSE_set(int *p, int purpose) { return 1; } -int X509_PURPOSE_get_count(void) { - if (!xptable) { - return X509_PURPOSE_COUNT; - } - return sk_X509_PURPOSE_num(xptable) + X509_PURPOSE_COUNT; -} +int X509_PURPOSE_get_count(void) { return OPENSSL_ARRAY_SIZE(xstandard); } X509_PURPOSE *X509_PURPOSE_get0(int idx) { - if (idx < 0) { + if (idx < 0 || (size_t)idx >= OPENSSL_ARRAY_SIZE(xstandard)) { return NULL; } - if (idx < (int)X509_PURPOSE_COUNT) { - return xstandard + idx; - } - return sk_X509_PURPOSE_value(xptable, idx - X509_PURPOSE_COUNT); + return xstandard + idx; } int X509_PURPOSE_get_by_sname(const char *sname) { @@ -190,118 +171,10 @@ int X509_PURPOSE_get_by_sname(const char *sname) { } int X509_PURPOSE_get_by_id(int purpose) { - X509_PURPOSE tmp; - size_t idx; - - if ((purpose >= X509_PURPOSE_MIN) && (purpose <= X509_PURPOSE_MAX)) { + if (purpose >= X509_PURPOSE_MIN && purpose <= X509_PURPOSE_MAX) { return purpose - X509_PURPOSE_MIN; } - tmp.purpose = purpose; - if (!xptable) { - return -1; - } - - if (!sk_X509_PURPOSE_find_awslc(xptable, &idx, &tmp)) { - return -1; - } - return idx + X509_PURPOSE_COUNT; -} - -int X509_PURPOSE_add(int id, int trust, int flags, - int (*ck)(const X509_PURPOSE *, const X509 *, int), - const char *name, const char *sname, void *arg) { - X509_PURPOSE *ptmp; - char *name_dup, *sname_dup; - - // This is set according to what we change: application can't set it - flags &= ~X509_PURPOSE_DYNAMIC; - // This will always be set for application modified trust entries - flags |= X509_PURPOSE_DYNAMIC_NAME; - // Get existing entry if any - int idx = X509_PURPOSE_get_by_id(id); - // Need a new entry - if (idx == -1) { - if (!(ptmp = OPENSSL_malloc(sizeof(X509_PURPOSE)))) { - return 0; - } - ptmp->flags = X509_PURPOSE_DYNAMIC; - } else { - ptmp = X509_PURPOSE_get0(idx); - } - - // Duplicate the supplied names. - name_dup = OPENSSL_strdup(name); - sname_dup = OPENSSL_strdup(sname); - if (name_dup == NULL || sname_dup == NULL) { - if (name_dup != NULL) { - OPENSSL_free(name_dup); - } - if (sname_dup != NULL) { - OPENSSL_free(sname_dup); - } - if (idx == -1) { - OPENSSL_free(ptmp); - } - return 0; - } - - // OPENSSL_free existing name if dynamic - if (ptmp->flags & X509_PURPOSE_DYNAMIC_NAME) { - OPENSSL_free(ptmp->name); - OPENSSL_free(ptmp->sname); - } - // dup supplied name - ptmp->name = name_dup; - ptmp->sname = sname_dup; - // Keep the dynamic flag of existing entry - ptmp->flags &= X509_PURPOSE_DYNAMIC; - // Set all other flags - ptmp->flags |= flags; - - ptmp->purpose = id; - ptmp->trust = trust; - ptmp->check_purpose = ck; - ptmp->usr_data = arg; - - // If its a new entry manage the dynamic table - if (idx == -1) { - // TODO(davidben): This should be locked. Alternatively, remove the dynamic - // registration mechanism entirely. The trouble is there no way to pass in - // the various parameters into an |X509_VERIFY_PARAM| directly. You can only - // register it in the global table and get an ID. - if (!xptable && !(xptable = sk_X509_PURPOSE_new(xp_cmp))) { - xptable_free(ptmp); - return 0; - } - if (!sk_X509_PURPOSE_push(xptable, ptmp)) { - xptable_free(ptmp); - return 0; - } - sk_X509_PURPOSE_sort(xptable); - } - return 1; -} - -static void xptable_free(X509_PURPOSE *p) { - if (!p) { - return; - } - if (p->flags & X509_PURPOSE_DYNAMIC) { - if (p->flags & X509_PURPOSE_DYNAMIC_NAME) { - OPENSSL_free(p->name); - OPENSSL_free(p->sname); - } - OPENSSL_free(p); - } -} - -void X509_PURPOSE_cleanup(void) { - unsigned int i; - sk_X509_PURPOSE_pop_free(xptable, xptable_free); - for (i = 0; i < X509_PURPOSE_COUNT; i++) { - xptable_free(xstandard + i); - } - xptable = NULL; + return -1; } int X509_PURPOSE_get_id(const X509_PURPOSE *xp) { return xp->purpose; } diff --git a/crypto/x509/x509_trs.c b/crypto/x509/x509_trs.c index d8ab1dc229..ce4194b542 100644 --- a/crypto/x509/x509_trs.c +++ b/crypto/x509/x509_trs.c @@ -59,12 +59,10 @@ #include #include +#include "../internal.h" #include "internal.h" -static int tr_cmp(const X509_TRUST *const *a, const X509_TRUST *const *b); -static void trtable_free(X509_TRUST *p); - static int trust_1oidany(X509_TRUST *trust, X509 *x, int flags); static int trust_1oid(X509_TRUST *trust, X509 *x, int flags); static int trust_compat(X509_TRUST *trust, X509 *x, int flags); @@ -92,14 +90,6 @@ static X509_TRUST trstandard[] = { {X509_TRUST_TSA, 0, trust_1oidany, (char *)"TSA server", NID_time_stamp, NULL}}; -#define X509_TRUST_COUNT (sizeof(trstandard) / sizeof(X509_TRUST)) - -static STACK_OF(X509_TRUST) *trtable = NULL; - -static int tr_cmp(const X509_TRUST *const *a, const X509_TRUST *const *b) { - return (*a)->trust - (*b)->trust; -} - int X509_check_trust(X509 *x, int id, int flags) { X509_TRUST *pt; int idx; @@ -123,38 +113,20 @@ int X509_check_trust(X509 *x, int id, int flags) { return pt->check_trust(pt, x, flags); } -int X509_TRUST_get_count(void) { - if (!trtable) { - return X509_TRUST_COUNT; - } - return sk_X509_TRUST_num(trtable) + X509_TRUST_COUNT; -} +int X509_TRUST_get_count(void) { return OPENSSL_ARRAY_SIZE(trstandard); } X509_TRUST *X509_TRUST_get0(int idx) { - if (idx < 0) { + if (idx < 0 || (size_t)idx >= OPENSSL_ARRAY_SIZE(trstandard)) { return NULL; } - if (idx < (int)X509_TRUST_COUNT) { - return trstandard + idx; - } - return sk_X509_TRUST_value(trtable, idx - X509_TRUST_COUNT); + return trstandard + idx; } int X509_TRUST_get_by_id(int id) { - X509_TRUST tmp; - size_t idx; - - if ((id >= X509_TRUST_MIN) && (id <= X509_TRUST_MAX)) { + if (id >= X509_TRUST_MIN && id <= X509_TRUST_MAX) { return id - X509_TRUST_MIN; } - tmp.trust = id; - if (!trtable) { - return -1; - } - if (!sk_X509_TRUST_find_awslc(trtable, &idx, &tmp)) { - return -1; - } - return idx + X509_TRUST_COUNT; + return -1; } int X509_TRUST_set(int *t, int trust) { @@ -166,92 +138,6 @@ int X509_TRUST_set(int *t, int trust) { return 1; } -int X509_TRUST_add(int id, int flags, int (*ck)(X509_TRUST *, X509 *, int), - const char *name, int arg1, void *arg2) { - int idx; - X509_TRUST *trtmp; - char *name_dup; - - // This is set according to what we change: application can't set it - flags &= ~X509_TRUST_DYNAMIC; - // This will always be set for application modified trust entries - flags |= X509_TRUST_DYNAMIC_NAME; - // Get existing entry if any - idx = X509_TRUST_get_by_id(id); - // Need a new entry - if (idx == -1) { - if (!(trtmp = OPENSSL_malloc(sizeof(X509_TRUST)))) { - return 0; - } - trtmp->flags = X509_TRUST_DYNAMIC; - } else { - trtmp = X509_TRUST_get0(idx); - } - - // Duplicate the supplied name. - name_dup = OPENSSL_strdup(name); - if (name_dup == NULL) { - if (idx == -1) { - OPENSSL_free(trtmp); - } - return 0; - } - - // OPENSSL_free existing name if dynamic - if (trtmp->flags & X509_TRUST_DYNAMIC_NAME) { - OPENSSL_free(trtmp->name); - } - trtmp->name = name_dup; - // Keep the dynamic flag of existing entry - trtmp->flags &= X509_TRUST_DYNAMIC; - // Set all other flags - trtmp->flags |= flags; - - trtmp->trust = id; - trtmp->check_trust = ck; - trtmp->arg1 = arg1; - trtmp->arg2 = arg2; - - // If its a new entry manage the dynamic table - if (idx == -1) { - // TODO(davidben): This should be locked. Alternatively, remove the dynamic - // registration mechanism entirely. The trouble is there no way to pass in - // the various parameters into an |X509_VERIFY_PARAM| directly. You can only - // register it in the global table and get an ID. - if (!trtable && !(trtable = sk_X509_TRUST_new(tr_cmp))) { - trtable_free(trtmp); - return 0; - } - if (!sk_X509_TRUST_push(trtable, trtmp)) { - trtable_free(trtmp); - return 0; - } - sk_X509_TRUST_sort(trtable); - } - return 1; -} - -static void trtable_free(X509_TRUST *p) { - if (!p) { - return; - } - if (p->flags & X509_TRUST_DYNAMIC) { - if (p->flags & X509_TRUST_DYNAMIC_NAME) { - OPENSSL_free(p->name); - } - OPENSSL_free(p); - } -} - -void X509_TRUST_cleanup(void) { - unsigned int i; - for (i = 0; i < X509_TRUST_COUNT; i++) { - trtable_free(trstandard + i); - } - sk_X509_TRUST_pop_free(trtable, trtable_free); - trtable = NULL; -} - int X509_TRUST_get_flags(const X509_TRUST *xp) { return xp->flags; } char *X509_TRUST_get0_name(const X509_TRUST *xp) { return xp->name; } diff --git a/include/openssl/x509.h b/include/openssl/x509.h index 012da8ee8d..bb68109f60 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -3258,12 +3258,6 @@ DEFINE_STACK_OF(X509_TRUST) #define X509_TRUST_MIN 1 #define X509_TRUST_MAX 8 - -// trust_flags values - -#define X509_TRUST_DYNAMIC 1 -#define X509_TRUST_DYNAMIC_NAME 2 - // check_trust return codes #define X509_TRUST_TRUSTED 1 @@ -3329,10 +3323,6 @@ OPENSSL_EXPORT int X509_check_trust(X509 *x, int id, int flags); OPENSSL_EXPORT int X509_TRUST_get_count(void); OPENSSL_EXPORT X509_TRUST *X509_TRUST_get0(int idx); OPENSSL_EXPORT int X509_TRUST_get_by_id(int id); -OPENSSL_EXPORT int X509_TRUST_add(int id, int flags, - int (*ck)(X509_TRUST *, X509 *, int), - const char *name, int arg1, void *arg2); -OPENSSL_EXPORT void X509_TRUST_cleanup(void); OPENSSL_EXPORT int X509_TRUST_get_flags(const X509_TRUST *xp); OPENSSL_EXPORT char *X509_TRUST_get0_name(const X509_TRUST *xp); OPENSSL_EXPORT int X509_TRUST_get_trust(const X509_TRUST *xp); @@ -4198,9 +4188,6 @@ struct ISSUING_DIST_POINT_st { #define NS_OBJSIGN_CA 0x01 #define NS_ANY_CA (NS_SSL_CA | NS_SMIME_CA | NS_OBJSIGN_CA) -#define X509_PURPOSE_DYNAMIC 0x1 -#define X509_PURPOSE_DYNAMIC_NAME 0x2 - typedef struct x509_purpose_st { int purpose; int trust; // Default trust ID @@ -4487,15 +4474,9 @@ OPENSSL_EXPORT int X509_PURPOSE_get_count(void); OPENSSL_EXPORT X509_PURPOSE *X509_PURPOSE_get0(int idx); OPENSSL_EXPORT int X509_PURPOSE_get_by_sname(const char *sname); OPENSSL_EXPORT int X509_PURPOSE_get_by_id(int id); -OPENSSL_EXPORT int X509_PURPOSE_add(int id, int trust, int flags, - int (*ck)(const X509_PURPOSE *, - const X509 *, int), - const char *name, const char *sname, - void *arg); OPENSSL_EXPORT char *X509_PURPOSE_get0_name(const X509_PURPOSE *xp); OPENSSL_EXPORT char *X509_PURPOSE_get0_sname(const X509_PURPOSE *xp); OPENSSL_EXPORT int X509_PURPOSE_get_trust(const X509_PURPOSE *xp); -OPENSSL_EXPORT void X509_PURPOSE_cleanup(void); OPENSSL_EXPORT int X509_PURPOSE_get_id(const X509_PURPOSE *); From 01a46be3ce64e447adc0ef6d7685c60ab5208b59 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sat, 2 Dec 2023 17:20:35 -0500 Subject: [PATCH 10/12] Const-correct X509_TRUST and X509_PURPOSE This gets the built-in tables out of mutable data. Update-Note: No one uses these APIs except for rust-openssl. rust-openssl may need fixes because they seem to not quite handle C const correctly. Change-Id: Ia23c61e2fe6a5637e59044e10ea2048cfe46e502 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64508 Reviewed-by: Bob Beck Commit-Queue: David Benjamin (cherry picked from commit 6099ab92c2447d41ab9e95f5c1bc8cfbc331439f) --- crypto/x509/v3_purp.c | 6 +++--- crypto/x509/x509_trs.c | 19 +++++++++---------- crypto/x509/x509_vfy.c | 3 +-- include/openssl/x509.h | 6 +++--- 4 files changed, 16 insertions(+), 18 deletions(-) diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c index 7ec3ad1733..4eb4ee830d 100644 --- a/crypto/x509/v3_purp.c +++ b/crypto/x509/v3_purp.c @@ -94,7 +94,7 @@ static int check_purpose_timestamp_sign(const X509_PURPOSE *xp, const X509 *x, static int no_check(const X509_PURPOSE *xp, const X509 *x, int ca); static int ocsp_helper(const X509_PURPOSE *xp, const X509 *x, int ca); -static X509_PURPOSE xstandard[] = { +static const X509_PURPOSE xstandard[] = { {X509_PURPOSE_SSL_CLIENT, X509_TRUST_SSL_CLIENT, 0, check_purpose_ssl_client, (char *)"SSL client", (char *)"sslclient", NULL}, {X509_PURPOSE_SSL_SERVER, X509_TRUST_SSL_SERVER, 0, @@ -152,7 +152,7 @@ int X509_PURPOSE_set(int *p, int purpose) { int X509_PURPOSE_get_count(void) { return OPENSSL_ARRAY_SIZE(xstandard); } -X509_PURPOSE *X509_PURPOSE_get0(int idx) { +const X509_PURPOSE *X509_PURPOSE_get0(int idx) { if (idx < 0 || (size_t)idx >= OPENSSL_ARRAY_SIZE(xstandard)) { return NULL; } @@ -160,7 +160,7 @@ X509_PURPOSE *X509_PURPOSE_get0(int idx) { } int X509_PURPOSE_get_by_sname(const char *sname) { - X509_PURPOSE *xptmp; + const X509_PURPOSE *xptmp; for (int i = 0; i < X509_PURPOSE_get_count(); i++) { xptmp = X509_PURPOSE_get0(i); if (!strcmp(xptmp->sname, sname)) { diff --git a/crypto/x509/x509_trs.c b/crypto/x509/x509_trs.c index ce4194b542..122c20e775 100644 --- a/crypto/x509/x509_trs.c +++ b/crypto/x509/x509_trs.c @@ -63,9 +63,9 @@ #include "internal.h" -static int trust_1oidany(X509_TRUST *trust, X509 *x, int flags); -static int trust_1oid(X509_TRUST *trust, X509 *x, int flags); -static int trust_compat(X509_TRUST *trust, X509 *x, int flags); +static int trust_1oidany(const X509_TRUST *trust, X509 *x, int flags); +static int trust_1oid(const X509_TRUST *trust, X509 *x, int flags); +static int trust_compat(const X509_TRUST *trust, X509 *x, int flags); static int obj_trust(int id, X509 *x, int flags); @@ -73,7 +73,7 @@ static int obj_trust(int id, X509 *x, int flags); // any gaps so we can just subtract the minimum trust value to get an index // into the table -static X509_TRUST trstandard[] = { +static const X509_TRUST trstandard[] = { {X509_TRUST_COMPAT, 0, trust_compat, (char *)"compatible", 0, NULL}, {X509_TRUST_SSL_CLIENT, 0, trust_1oidany, (char *)"SSL Client", NID_client_auth, NULL}, @@ -91,7 +91,6 @@ static X509_TRUST trstandard[] = { NULL}}; int X509_check_trust(X509 *x, int id, int flags) { - X509_TRUST *pt; int idx; if (id == -1) { return 1; @@ -109,13 +108,13 @@ int X509_check_trust(X509 *x, int id, int flags) { if (idx == -1) { return obj_trust(id, x, flags); } - pt = X509_TRUST_get0(idx); + const X509_TRUST *pt = X509_TRUST_get0(idx); return pt->check_trust(pt, x, flags); } int X509_TRUST_get_count(void) { return OPENSSL_ARRAY_SIZE(trstandard); } -X509_TRUST *X509_TRUST_get0(int idx) { +const X509_TRUST *X509_TRUST_get0(int idx) { if (idx < 0 || (size_t)idx >= OPENSSL_ARRAY_SIZE(trstandard)) { return NULL; } @@ -144,7 +143,7 @@ char *X509_TRUST_get0_name(const X509_TRUST *xp) { return xp->name; } int X509_TRUST_get_trust(const X509_TRUST *xp) { return xp->trust; } -static int trust_1oidany(X509_TRUST *trust, X509 *x, int flags) { +static int trust_1oidany(const X509_TRUST *trust, X509 *x, int flags) { if (x->aux && (x->aux->trust || x->aux->reject)) { return obj_trust(trust->arg1, x, flags); } @@ -153,14 +152,14 @@ static int trust_1oidany(X509_TRUST *trust, X509 *x, int flags) { return trust_compat(trust, x, flags); } -static int trust_1oid(X509_TRUST *trust, X509 *x, int flags) { +static int trust_1oid(const X509_TRUST *trust, X509 *x, int flags) { if (x->aux) { return obj_trust(trust->arg1, x, flags); } return X509_TRUST_UNTRUSTED; } -static int trust_compat(X509_TRUST *trust, X509 *x, int flags) { +static int trust_compat(const X509_TRUST *trust, X509 *x, int flags) { if (!x509v3_cache_extensions(x)) { return X509_TRUST_UNTRUSTED; } diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 1cb706c0d2..19698f06cf 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -1611,13 +1611,12 @@ int X509_STORE_CTX_purpose_inherit(X509_STORE_CTX *ctx, int def_purpose, } // If we have a purpose then check it is valid if (purpose) { - X509_PURPOSE *ptmp; idx = X509_PURPOSE_get_by_id(purpose); if (idx == -1) { OPENSSL_PUT_ERROR(X509, X509_R_UNKNOWN_PURPOSE_ID); return 0; } - ptmp = X509_PURPOSE_get0(idx); + const X509_PURPOSE *ptmp = X509_PURPOSE_get0(idx); if (ptmp->trust == X509_TRUST_DEFAULT) { idx = X509_PURPOSE_get_by_id(def_purpose); if (idx == -1) { diff --git a/include/openssl/x509.h b/include/openssl/x509.h index bb68109f60..d724dfd657 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -3232,7 +3232,7 @@ DECLARE_STACK_OF(GENERAL_NAME) struct x509_trust_st { int trust; int flags; - int (*check_trust)(struct x509_trust_st *, X509 *, int); + int (*check_trust)(const X509_TRUST *, X509 *, int); char *name; int arg1; void *arg2; @@ -3321,7 +3321,7 @@ OPENSSL_EXPORT int X509_verify_cert(X509_STORE_CTX *ctx); OPENSSL_EXPORT int X509_check_trust(X509 *x, int id, int flags); OPENSSL_EXPORT int X509_TRUST_get_count(void); -OPENSSL_EXPORT X509_TRUST *X509_TRUST_get0(int idx); +OPENSSL_EXPORT const X509_TRUST *X509_TRUST_get0(int idx); OPENSSL_EXPORT int X509_TRUST_get_by_id(int id); OPENSSL_EXPORT int X509_TRUST_get_flags(const X509_TRUST *xp); OPENSSL_EXPORT char *X509_TRUST_get0_name(const X509_TRUST *xp); @@ -4471,7 +4471,7 @@ OPENSSL_EXPORT int X509_check_issued(X509 *issuer, X509 *subject); OPENSSL_EXPORT int X509_check_akid(X509 *issuer, AUTHORITY_KEYID *akid); OPENSSL_EXPORT int X509_PURPOSE_get_count(void); -OPENSSL_EXPORT X509_PURPOSE *X509_PURPOSE_get0(int idx); +OPENSSL_EXPORT const X509_PURPOSE *X509_PURPOSE_get0(int idx); OPENSSL_EXPORT int X509_PURPOSE_get_by_sname(const char *sname); OPENSSL_EXPORT int X509_PURPOSE_get_by_id(int id); OPENSSL_EXPORT char *X509_PURPOSE_get0_name(const X509_PURPOSE *xp); From 41039db4ee9f9d9a4b96a6ccdb07cb371d41ffe9 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 4 Dec 2023 11:39:59 -0500 Subject: [PATCH 11/12] Unexport some STACK_OF types. STACK_OF(GENERAL_NAMES) is STACK_OF(STACK_OF(GENERAL_NAMES)). Nothing uses this. It appears to be a remnant of CMS and indirect CRL support. May as well trim the header slightly. STACK_OF(X509_VERIFY_PARAM) is a remnant of (non-thread-safe) global registration of X509_VERIFY_PARAMs. STACK_OF(X509_LOOKUP) is only used internally. May as well prune them from the header so the file expands to be a bit less code. Update-Note: A few obscure STACK_OF(T) types are unexported. This is not expected to impact anyone. Change-Id: I03757c8522531132a31270b6dab055966b6e9070 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64527 Reviewed-by: Bob Beck Commit-Queue: David Benjamin (cherry picked from commit e6489902b7fb692875341b8ab5e57f0515f47bc1) --- crypto/x509/internal.h | 2 ++ include/openssl/x509.h | 6 ------ 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h index e178f0a289..4a0c5de9cd 100644 --- a/crypto/x509/internal.h +++ b/crypto/x509/internal.h @@ -284,6 +284,8 @@ struct x509_lookup_method_st { X509_OBJECT *ret); } /* X509_LOOKUP_METHOD */; +DEFINE_STACK_OF(X509_LOOKUP) + // This is used to hold everything. It is used for all certificate // validation. Once we have a certificate chain, the 'verify' // function is then called to actually check the cert chain. diff --git a/include/openssl/x509.h b/include/openssl/x509.h index d724dfd657..86c2f87562 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -3264,8 +3264,6 @@ DEFINE_STACK_OF(X509_TRUST) #define X509_TRUST_REJECTED 2 #define X509_TRUST_UNTRUSTED 3 -DECLARE_STACK_OF(GENERAL_NAMES) - // X509_verify_cert_error_string returns |err| as a human-readable string, where // |err| should be one of the |X509_V_*| values. If |err| is unknown, it returns // a default description. @@ -3350,9 +3348,7 @@ certificate chain. #define X509_LU_CRL 2 #define X509_LU_PKEY 3 -DEFINE_STACK_OF(X509_LOOKUP) DEFINE_STACK_OF(X509_OBJECT) -DEFINE_STACK_OF(X509_VERIFY_PARAM) typedef int (*X509_STORE_CTX_verify_cb)(int, X509_STORE_CTX *); typedef int (*X509_STORE_CTX_get_issuer_fn)(X509 **issuer, X509_STORE_CTX *ctx, @@ -4058,8 +4054,6 @@ struct GENERAL_NAME_st { } d; } /* GENERAL_NAME */; -DEFINE_STACK_OF(GENERAL_NAMES) - typedef struct ACCESS_DESCRIPTION_st { ASN1_OBJECT *method; GENERAL_NAME *location; From ec57e901aa2dfda5cfb5bf7cd8658a34f40086e5 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 6 Dec 2023 16:23:50 -0500 Subject: [PATCH 12/12] Document GENERAL_NAME-related APIs Update-Note: In the process, unexport the ASN1_ITEMs, and the d2i/i2d functions for OTHERNAME and EDIPARTYNAME. These do not appear to be used and removing them will cut down on the amount of compatibility glue needed when we rewrite the parsers with a safer calling convention. Bug: 426 Change-Id: Ifc45867c0a0c832e5ef72deaec5a2c88b8d8ac6a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64628 Reviewed-by: Bob Beck Commit-Queue: David Benjamin (cherry picked from commit e89d99af0e4d7fb1df4d961d7aafdfed30d08d41) --- crypto/ocsp/ocsp_asn.c | 1 + crypto/x509/internal.h | 8 ++ crypto/x509/v3_akeya.c | 2 + crypto/x509/v3_genn.c | 22 ++-- include/openssl/x509.h | 246 +++++++++++++++++++++++++++++------------ 5 files changed, 199 insertions(+), 80 deletions(-) diff --git a/crypto/ocsp/ocsp_asn.c b/crypto/ocsp/ocsp_asn.c index b013eb24f3..6236ed1e0d 100644 --- a/crypto/ocsp/ocsp_asn.c +++ b/crypto/ocsp/ocsp_asn.c @@ -11,6 +11,7 @@ // https://tools.ietf.org/html/rfc6960#section-4.2.1 #include "internal.h" +#include "../x509/internal.h" ASN1_SEQUENCE(OCSP_SIGNATURE) = { ASN1_SIMPLE(OCSP_SIGNATURE, signatureAlgorithm, X509_ALGOR), diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h index 4a0c5de9cd..54f2c1f438 100644 --- a/crypto/x509/internal.h +++ b/crypto/x509/internal.h @@ -246,6 +246,14 @@ struct X509_crl_st { unsigned char crl_hash[SHA256_DIGEST_LENGTH]; } /* X509_CRL */; +// GENERAL_NAME is an |ASN1_ITEM| whose ASN.1 type is GeneralName and C type is +// |GENERAL_NAME*|. +DECLARE_ASN1_ITEM(GENERAL_NAME) + +// GENERAL_NAMES is an |ASN1_ITEM| whose ASN.1 type is SEQUENCE OF GeneralName +// and C type is |GENERAL_NAMES*|, aka |STACK_OF(GENERAL_NAME)*|. +DECLARE_ASN1_ITEM(GENERAL_NAMES) + struct X509_VERIFY_PARAM_st { int64_t check_time; // POSIX time to use unsigned long flags; // Various verify flags diff --git a/crypto/x509/v3_akeya.c b/crypto/x509/v3_akeya.c index 4a05aae630..8614b86c80 100644 --- a/crypto/x509/v3_akeya.c +++ b/crypto/x509/v3_akeya.c @@ -61,6 +61,8 @@ #include #include +#include "internal.h" + ASN1_SEQUENCE(AUTHORITY_KEYID) = { ASN1_IMP_OPT(AUTHORITY_KEYID, keyid, ASN1_OCTET_STRING, 0), diff --git a/crypto/x509/v3_genn.c b/crypto/x509/v3_genn.c index 42cd787906..ec3a282d64 100644 --- a/crypto/x509/v3_genn.c +++ b/crypto/x509/v3_genn.c @@ -70,7 +70,7 @@ ASN1_SEQUENCE(OTHERNAME) = { ASN1_EXP(OTHERNAME, value, ASN1_ANY, 0), } ASN1_SEQUENCE_END(OTHERNAME) -IMPLEMENT_ASN1_FUNCTIONS_const(OTHERNAME) +IMPLEMENT_ASN1_ALLOC_FUNCTIONS(OTHERNAME) ASN1_SEQUENCE(EDIPARTYNAME) = { // DirectoryString is a CHOICE type, so use explicit tagging. @@ -78,7 +78,7 @@ ASN1_SEQUENCE(EDIPARTYNAME) = { ASN1_EXP(EDIPARTYNAME, partyName, DIRECTORYSTRING, 1), } ASN1_SEQUENCE_END(EDIPARTYNAME) -IMPLEMENT_ASN1_FUNCTIONS_const(EDIPARTYNAME) +IMPLEMENT_ASN1_ALLOC_FUNCTIONS(EDIPARTYNAME) ASN1_CHOICE(GENERAL_NAME) = { ASN1_IMP(GENERAL_NAME, d.otherName, OTHERNAME, GEN_OTHERNAME), @@ -208,9 +208,9 @@ void GENERAL_NAME_set0_value(GENERAL_NAME *a, int type, void *value) { a->type = type; } -void *GENERAL_NAME_get0_value(const GENERAL_NAME *a, int *ptype) { - if (ptype) { - *ptype = a->type; +void *GENERAL_NAME_get0_value(const GENERAL_NAME *a, int *out_type) { + if (out_type) { + *out_type = a->type; } switch (a->type) { case GEN_X400: @@ -255,16 +255,16 @@ int GENERAL_NAME_set0_othername(GENERAL_NAME *gen, ASN1_OBJECT *oid, return 1; } -int GENERAL_NAME_get0_otherName(const GENERAL_NAME *gen, ASN1_OBJECT **poid, - ASN1_TYPE **pvalue) { +int GENERAL_NAME_get0_otherName(const GENERAL_NAME *gen, ASN1_OBJECT **out_oid, + ASN1_TYPE **out_value) { if (gen->type != GEN_OTHERNAME) { return 0; } - if (poid) { - *poid = gen->d.otherName->type_id; + if (out_oid != NULL) { + *out_oid = gen->d.otherName->type_id; } - if (pvalue) { - *pvalue = gen->d.otherName->value; + if (out_value != NULL) { + *out_value = gen->d.otherName->value; } return 1; } diff --git a/include/openssl/x509.h b/include/openssl/x509.h index 86c2f87562..02e9fc87f4 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -1381,8 +1381,7 @@ DEFINE_STACK_OF(X509_NAME) // type is |X509_NAME*|. DECLARE_ASN1_ITEM(X509_NAME) -// X509_NAME_new returns a new, empty |X509_NAME_new|, or NULL on -// error. +// X509_NAME_new returns a new, empty |X509_NAME|, or NULL on error. OPENSSL_EXPORT X509_NAME *X509_NAME_new(void); // X509_NAME_free releases memory associated with |name|. @@ -1517,8 +1516,7 @@ OPENSSL_EXPORT int X509_NAME_add_entry_by_txt(X509_NAME *name, // (RFC 5280) and C type is |X509_NAME_ENTRY*|. DECLARE_ASN1_ITEM(X509_NAME_ENTRY) -// X509_NAME_ENTRY_new returns a new, empty |X509_NAME_ENTRY_new|, or NULL on -// error. +// X509_NAME_ENTRY_new returns a new, empty |X509_NAME_ENTRY|, or NULL on error. OPENSSL_EXPORT X509_NAME_ENTRY *X509_NAME_ENTRY_new(void); // X509_NAME_ENTRY_free releases memory associated with |entry|. @@ -1841,6 +1839,181 @@ OPENSSL_EXPORT STACK_OF(X509_EXTENSION) *X509v3_add_ext( STACK_OF(X509_EXTENSION) **x, const X509_EXTENSION *ex, int loc); +// General names. +// +// A |GENERAL_NAME| represents an X.509 GeneralName structure, defined in RFC +// 5280, Section 4.2.1.6. General names are distinct from names (|X509_NAME|). A +// general name is a CHOICE type which may contain one of several name types, +// most commonly a DNS name or an IP address. General names most commonly appear +// in the subject alternative name (SAN) extension, though they are also used in +// other extensions. +// +// Many extensions contain a SEQUENCE OF GeneralName, or GeneralNames, so +// |STACK_OF(GENERAL_NAME)| is defined and aliased to |GENERAL_NAMES|. + +typedef struct otherName_st { + ASN1_OBJECT *type_id; + ASN1_TYPE *value; +} OTHERNAME; + +typedef struct EDIPartyName_st { + ASN1_STRING *nameAssigner; + ASN1_STRING *partyName; +} EDIPARTYNAME; + +// GEN_* are constants for the |type| field of |GENERAL_NAME|, defined below. +#define GEN_OTHERNAME 0 +#define GEN_EMAIL 1 +#define GEN_DNS 2 +#define GEN_X400 3 +#define GEN_DIRNAME 4 +#define GEN_EDIPARTY 5 +#define GEN_URI 6 +#define GEN_IPADD 7 +#define GEN_RID 8 + +// A GENERAL_NAME_st, aka |GENERAL_NAME|, represents an X.509 GeneralName. The +// |type| field determines which member of |d| is active. A |GENERAL_NAME| may +// also be empty, in which case |type| is -1 and |d| is NULL. Empty +// |GENERAL_NAME|s are invalid and will never be returned from the parser, but +// may be created temporarily, e.g. by |GENERAL_NAME_new|. +struct GENERAL_NAME_st { + int type; + union { + char *ptr; + OTHERNAME *otherName; + ASN1_IA5STRING *rfc822Name; + ASN1_IA5STRING *dNSName; + ASN1_STRING *x400Address; + X509_NAME *directoryName; + EDIPARTYNAME *ediPartyName; + ASN1_IA5STRING *uniformResourceIdentifier; + ASN1_OCTET_STRING *iPAddress; + ASN1_OBJECT *registeredID; + + // Old names + ASN1_OCTET_STRING *ip; // iPAddress + X509_NAME *dirn; // dirn + ASN1_IA5STRING *ia5; // rfc822Name, dNSName, uniformResourceIdentifier + ASN1_OBJECT *rid; // registeredID + } d; +} /* GENERAL_NAME */; + +// GENERAL_NAME_new returns a new, empty |GENERAL_NAME|, or NULL on error. +OPENSSL_EXPORT GENERAL_NAME *GENERAL_NAME_new(void); + +// GENERAL_NAME_free releases memory associated with |gen|. +OPENSSL_EXPORT void GENERAL_NAME_free(GENERAL_NAME *gen); + +// d2i_GENERAL_NAME parses up to |len| bytes from |*inp| as a DER-encoded X.509 +// GeneralName (RFC 5280), as described in |d2i_SAMPLE|. +OPENSSL_EXPORT GENERAL_NAME *d2i_GENERAL_NAME(GENERAL_NAME **out, + const uint8_t **inp, long len); + +// i2d_GENERAL_NAME marshals |in| as a DER-encoded X.509 GeneralName (RFC 5280), +// as described in |i2d_SAMPLE|. +// +// TODO(https://crbug.com/boringssl/407): This function should be const and +// thread-safe but is currently neither in some cases, notably if |in| is an +// directoryName and the |X509_NAME| has been modified. +OPENSSL_EXPORT int i2d_GENERAL_NAME(GENERAL_NAME *in, uint8_t **outp); + +// GENERAL_NAME_dup returns a newly-allocated copy of |gen|, or NULL on error. +// This function works by serializing the structure, so it will fail if |gen| is +// empty. +// +// TODO(https://crbug.com/boringssl/407): This function should be const and +// thread-safe but is currently neither in some cases, notably if |gen| is an +// directoryName and the |X509_NAME| has been modified. +OPENSSL_EXPORT GENERAL_NAME *GENERAL_NAME_dup(GENERAL_NAME *gen); + +// GENERAL_NAMES_new returns a new, empty |GENERAL_NAMES|, or NULL on error. +OPENSSL_EXPORT GENERAL_NAMES *GENERAL_NAMES_new(void); + +// GENERAL_NAMES_free releases memory associated with |gens|. +OPENSSL_EXPORT void GENERAL_NAMES_free(GENERAL_NAMES *gens); + +// d2i_GENERAL_NAMES parses up to |len| bytes from |*inp| as a DER-encoded +// SEQUENCE OF GeneralName, as described in |d2i_SAMPLE|. +OPENSSL_EXPORT GENERAL_NAMES *d2i_GENERAL_NAMES(GENERAL_NAMES **out, + const uint8_t **inp, long len); + +// i2d_GENERAL_NAMES marshals |in| as a DER-encoded SEQUENCE OF GeneralName, as +// described in |i2d_SAMPLE|. +// +// TODO(https://crbug.com/boringssl/407): This function should be const and +// thread-safe but is currently neither in some cases, notably if some element +// of |in| is an directoryName and the |X509_NAME| has been modified. +OPENSSL_EXPORT int i2d_GENERAL_NAMES(GENERAL_NAMES *in, uint8_t **outp); + +// OTHERNAME_new returns a new, empty |OTHERNAME|, or NULL on error. +OPENSSL_EXPORT OTHERNAME *OTHERNAME_new(void); + +// OTHERNAME_free releases memory associated with |name|. +OPENSSL_EXPORT void OTHERNAME_free(OTHERNAME *name); + +// EDIPARTYNAME_new returns a new, empty |EDIPARTYNAME|, or NULL on error. +// EDIPartyName is rarely used in practice, so callers are unlikely to need this +// function. +OPENSSL_EXPORT EDIPARTYNAME *EDIPARTYNAME_new(void); + +// EDIPARTYNAME_free releases memory associated with |name|. EDIPartyName is +// rarely used in practice, so callers are unlikely to need this function. +OPENSSL_EXPORT void EDIPARTYNAME_free(EDIPARTYNAME *name); + +// GENERAL_NAME_set0_value set |gen|'s type and value to |type| and |value|. +// |type| must be a |GEN_*| constant and |value| must be an object of the +// corresponding type. |gen| takes ownership of |value|, so |value| must have +// been an allocated object. +// +// WARNING: |gen| must be empty (typically as returned from |GENERAL_NAME_new|) +// before calling this function. If |gen| already contained a value, the +// previous contents will be leaked. +OPENSSL_EXPORT void GENERAL_NAME_set0_value(GENERAL_NAME *gen, int type, + void *value); + +// GENERAL_NAME_get0_value returns the in-memory representation of |gen|'s +// contents and, |out_type| is not NULL, sets |*out_type| to the type of |gen|, +// which will be a |GEN_*| constant. If |gen| is incomplete, the return value +// will be NULL and the type will be -1. +// +// WARNING: Casting the result of this function to the wrong type is a +// potentially exploitable memory error. Callers must check |gen|'s type, either +// via |*out_type| or checking |gen->type| directly, before inspecting the +// result. +// +// WARNING: This function is not const-correct. The return value should be +// const. Callers shoudl not mutate the returned object. +OPENSSL_EXPORT void *GENERAL_NAME_get0_value(const GENERAL_NAME *gen, + int *out_type); + +// GENERAL_NAME_set0_othername sets |gen| to be an OtherName with type |oid| and +// value |value|. On success, it returns one and takes ownership of |oid| and +// |value|, which must be created in a way compatible with |ASN1_OBJECT_free| +// and |ASN1_TYPE_free|, respectively. On allocation failure, it returns zero. +// In the failure case, the caller retains ownership of |oid| and |value| and +// must release them when done. +// +// WARNING: |gen| must be empty (typically as returned from |GENERAL_NAME_new|) +// before calling this function. If |gen| already contained a value, the +// previously contents will be leaked. +OPENSSL_EXPORT int GENERAL_NAME_set0_othername(GENERAL_NAME *gen, + ASN1_OBJECT *oid, + ASN1_TYPE *value); + +// GENERAL_NAME_get0_otherName, if |gen| is an OtherName, sets |*out_oid| and +// |*out_value| to the OtherName's type-id and value, respectively, and returns +// one. If |gen| is not an OtherName, it returns zero and leaves |*out_oid| and +// |*out_value| unmodified. Either of |out_oid| or |out_value| may be NULL to +// ignore the value. +// +// WARNING: This function is not const-correct. |out_oid| and |out_value| are +// not const, but callers should not mutate the resulting objects. +OPENSSL_EXPORT int GENERAL_NAME_get0_otherName(const GENERAL_NAME *gen, + ASN1_OBJECT **out_oid, + ASN1_TYPE **out_value); + + // Algorithm identifiers. // // An |X509_ALGOR| represents an AlgorithmIdentifier structure, used in X.509 @@ -3225,7 +3398,6 @@ struct X509_algor_st { // the end of the certificate itself DECLARE_STACK_OF(DIST_POINT) -DECLARE_STACK_OF(GENERAL_NAME) // This is used for a table of trust checking functions @@ -4011,49 +4183,6 @@ struct BASIC_CONSTRAINTS_st { ASN1_INTEGER *pathlen; }; - -typedef struct otherName_st { - ASN1_OBJECT *type_id; - ASN1_TYPE *value; -} OTHERNAME; - -typedef struct EDIPartyName_st { - ASN1_STRING *nameAssigner; - ASN1_STRING *partyName; -} EDIPARTYNAME; - -struct GENERAL_NAME_st { -#define GEN_OTHERNAME 0 -#define GEN_EMAIL 1 -#define GEN_DNS 2 -#define GEN_X400 3 -#define GEN_DIRNAME 4 -#define GEN_EDIPARTY 5 -#define GEN_URI 6 -#define GEN_IPADD 7 -#define GEN_RID 8 - - int type; - union { - char *ptr; - OTHERNAME *otherName; // otherName - ASN1_IA5STRING *rfc822Name; - ASN1_IA5STRING *dNSName; - ASN1_STRING *x400Address; - X509_NAME *directoryName; - EDIPARTYNAME *ediPartyName; - ASN1_IA5STRING *uniformResourceIdentifier; - ASN1_OCTET_STRING *iPAddress; - ASN1_OBJECT *registeredID; - - // Old names - ASN1_OCTET_STRING *ip; // iPAddress - X509_NAME *dirn; // dirn - ASN1_IA5STRING *ia5; // rfc822Name, dNSName, uniformResourceIdentifier - ASN1_OBJECT *rid; // registeredID - } d; -} /* GENERAL_NAME */; - typedef struct ACCESS_DESCRIPTION_st { ASN1_OBJECT *method; GENERAL_NAME *location; @@ -4213,27 +4342,6 @@ DECLARE_ASN1_FUNCTIONS_const(BASIC_CONSTRAINTS) // an |X509_NAME|. DECLARE_ASN1_FUNCTIONS(AUTHORITY_KEYID) -// TODO(https://crbug.com/boringssl/407): This is not const because it contains -// an |X509_NAME|. -DECLARE_ASN1_FUNCTIONS(GENERAL_NAME) -OPENSSL_EXPORT GENERAL_NAME *GENERAL_NAME_dup(GENERAL_NAME *a); - -// TODO(https://crbug.com/boringssl/407): This is not const because it contains -// an |X509_NAME|. -DECLARE_ASN1_FUNCTIONS(GENERAL_NAMES) - -DECLARE_ASN1_FUNCTIONS_const(OTHERNAME) -DECLARE_ASN1_FUNCTIONS_const(EDIPARTYNAME) -OPENSSL_EXPORT void GENERAL_NAME_set0_value(GENERAL_NAME *a, int type, - void *value); -OPENSSL_EXPORT void *GENERAL_NAME_get0_value(const GENERAL_NAME *a, int *ptype); -OPENSSL_EXPORT int GENERAL_NAME_set0_othername(GENERAL_NAME *gen, - ASN1_OBJECT *oid, - ASN1_TYPE *value); -OPENSSL_EXPORT int GENERAL_NAME_get0_otherName(const GENERAL_NAME *gen, - ASN1_OBJECT **poid, - ASN1_TYPE **pvalue); - DECLARE_ASN1_FUNCTIONS_const(EXTENDED_KEY_USAGE) DECLARE_ASN1_FUNCTIONS_const(CERTIFICATEPOLICIES)