Move some deprecated X.509 functions into the deprecated section In particular, deprecate get_crl and check_crl. Only gRPC uses them, and does so incorrectly. It is impossible to implement those callbacks correctly. Also remove X509_STORE_CTX_set_cert. No one uses it, and it's redundant with X509_STORE_CTX_init. We should remove X509_STORE_CTX_set_chain too, but there are some callers to cleanup. Bug: 426 Change-Id: I846f8292a5f5d6cc3b5d2a576bfaf86e9ea84bdc Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65147 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 4f6c4ad..a19dd44 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c
@@ -1467,8 +1467,6 @@ return NULL; } -void X509_STORE_CTX_set_cert(X509_STORE_CTX *ctx, X509 *x) { ctx->cert = x; } - void X509_STORE_CTX_set_chain(X509_STORE_CTX *ctx, STACK_OF(X509) *sk) { ctx->untrusted = sk; }
diff --git a/include/openssl/x509.h b/include/openssl/x509.h index 3cf7514..9a51f11 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h
@@ -3429,6 +3429,88 @@ #define X509_STORE_get1_certs X509_STORE_CTX_get1_certs #define X509_STORE_get1_crls X509_STORE_CTX_get1_crls +// X509_STORE_CTX_get_chain is a legacy alias for |X509_STORE_CTX_get0_chain|. +OPENSSL_EXPORT STACK_OF(X509) *X509_STORE_CTX_get_chain(X509_STORE_CTX *ctx); + +// X509_STORE_CTX_trusted_stack is a deprecated alias for +// |X509_STORE_CTX_set0_trusted_stack|. +OPENSSL_EXPORT void X509_STORE_CTX_trusted_stack(X509_STORE_CTX *ctx, + STACK_OF(X509) *sk); + +typedef int (*X509_STORE_CTX_verify_cb)(int, X509_STORE_CTX *); + +// X509_STORE_CTX_set_verify_cb configures a callback function for |ctx| that is +// called multiple times during |X509_verify_cert|. The callback returns zero to +// fail verification and one to proceed. Typically, it will return |ok|, which +// preserves the default behavior. Returning one when |ok| is zero will proceed +// past some error. The callback may inspect |ctx| and the error queue to +// attempt to determine the current stage of certificate verification, but this +// is often unreliable. When synthesizing an error, callbacks should use +// |X509_STORE_CTX_set_error| to set a corresponding error. +// +// WARNING: Do not use this function. It is extremely fragile and unpredictable. +// This callback exposes implementation details of certificate verification, +// which change as the library evolves. Attempting to use it for security checks +// can introduce vulnerabilities if making incorrect assumptions about when the +// callback is called. Some errors, when suppressed, may implicitly suppress +// other errors due to internal implementation details. Additionally, overriding +// |ok| may leave |ctx| in an inconsistent state and break invariants. +// +// Instead, customize certificate verification by configuring options on the +// |X509_STORE_CTX| before verification, or applying additional checks after +// |X509_verify_cert| completes successfully. +OPENSSL_EXPORT void X509_STORE_CTX_set_verify_cb( + X509_STORE_CTX *ctx, int (*verify_cb)(int ok, X509_STORE_CTX *ctx)); + +// X509_STORE_set_verify_cb acts like |X509_STORE_CTX_set_verify_cb| but sets +// the verify callback for any |X509_STORE_CTX| created from this |X509_STORE| +// +// Do not use this function. See |X509_STORE_CTX_set_verify_cb| for details. +OPENSSL_EXPORT void X509_STORE_set_verify_cb( + X509_STORE *store, X509_STORE_CTX_verify_cb verify_cb); + +// X509_STORE_set_verify_cb_func is a deprecated alias for +// |X509_STORE_set_verify_cb|. +#define X509_STORE_set_verify_cb_func(store, func) \ + X509_STORE_set_verify_cb((store), (func)) + +typedef int (*X509_STORE_CTX_get_crl_fn)(X509_STORE_CTX *ctx, X509_CRL **crl, + X509 *x); +typedef int (*X509_STORE_CTX_check_crl_fn)(X509_STORE_CTX *ctx, X509_CRL *crl); + +// X509_STORE_set_get_crl override's |store|'s logic for looking up CRLs. +// +// Do not use this function. It is temporarily retained to support one caller +// and will be removed after that caller is fixed. It is not possible for +// external callers to correctly implement this callback. The real +// implementation sets some inaccessible internal state on |X509_STORE_CTX|. +OPENSSL_EXPORT void X509_STORE_set_get_crl(X509_STORE *store, + X509_STORE_CTX_get_crl_fn get_crl); + +// X509_STORE_set_check_crl override's |store|'s logic for checking CRL +// validity. +// +// Do not use this function. It is temporarily retained to support one caller +// and will be removed after that caller is fixed. It is not possible for +// external callers to correctly implement this callback. The real +// implementation relies some inaccessible internal state on |X509_STORE_CTX|. +OPENSSL_EXPORT void X509_STORE_set_check_crl( + X509_STORE *store, X509_STORE_CTX_check_crl_fn check_crl); + +// X509_STORE_CTX_set_chain configures |ctx| to use |sk| for untrusted +// intermediate certificates to use in verification. This function is redundant +// with the |chain| parameter of |X509_STORE_CTX_init|. Use the parameter +// instead. +// +// WARNING: Despite the similar name, this function is unrelated to +// |X509_STORE_CTX_get0_chain|. +// +// WARNING: This function saves a pointer to |sk| without copying or +// incrementing reference counts. |sk| must outlive |ctx| and may not be mutated +// for the duration of the certificate verification. +OPENSSL_EXPORT void X509_STORE_CTX_set_chain(X509_STORE_CTX *ctx, + STACK_OF(X509) *sk); + // Private structures. @@ -3560,10 +3642,6 @@ DEFINE_STACK_OF(X509_OBJECT) -typedef int (*X509_STORE_CTX_verify_cb)(int, X509_STORE_CTX *); -typedef int (*X509_STORE_CTX_get_crl_fn)(X509_STORE_CTX *ctx, X509_CRL **crl, - X509 *x); -typedef int (*X509_STORE_CTX_check_crl_fn)(X509_STORE_CTX *ctx, X509_CRL *crl); // X509_STORE_set_depth configures |store| to, by default, limit certificate // chains to |depth| intermediate certificates. This count excludes both the @@ -3791,19 +3869,6 @@ // |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 -// the verify callback for any |X509_STORE_CTX| created from this |X509_STORE| -// -// Do not use this funciton. see |X509_STORE_CTX_set_verify_cb|. -OPENSSL_EXPORT void X509_STORE_set_verify_cb( - X509_STORE *ctx, X509_STORE_CTX_verify_cb verify_cb); -#define X509_STORE_set_verify_cb_func(ctx, func) \ - X509_STORE_set_verify_cb((ctx), (func)) -OPENSSL_EXPORT void X509_STORE_set_get_crl(X509_STORE *ctx, - X509_STORE_CTX_get_crl_fn get_crl); -OPENSSL_EXPORT void X509_STORE_set_check_crl( - X509_STORE *ctx, X509_STORE_CTX_check_crl_fn check_crl); - // X509_STORE_CTX_new returns a newly-allocated, empty |X509_STORE_CTX|, or NULL // on error. OPENSSL_EXPORT X509_STORE_CTX *X509_STORE_CTX_new(void); @@ -3833,11 +3898,6 @@ OPENSSL_EXPORT void X509_STORE_CTX_set0_trusted_stack(X509_STORE_CTX *ctx, STACK_OF(X509) *sk); -// X509_STORE_CTX_trusted_stack is a deprecated alias for -// |X509_STORE_CTX_set0_trusted_stack|. -OPENSSL_EXPORT void X509_STORE_CTX_trusted_stack(X509_STORE_CTX *ctx, - STACK_OF(X509) *sk); - // X509_STORE_CTX_get0_store returns the |X509_STORE| that |ctx| uses. OPENSSL_EXPORT X509_STORE *X509_STORE_CTX_get0_store(X509_STORE_CTX *ctx); @@ -3898,9 +3958,6 @@ OPENSSL_EXPORT X509 *X509_STORE_CTX_get_current_cert(X509_STORE_CTX *ctx); OPENSSL_EXPORT X509_CRL *X509_STORE_CTX_get0_current_crl(X509_STORE_CTX *ctx); -// X509_STORE_CTX_get_chain is a legacy alias for |X509_STORE_CTX_get0_chain|. -OPENSSL_EXPORT STACK_OF(X509) *X509_STORE_CTX_get_chain(X509_STORE_CTX *ctx); - // X509_STORE_CTX_get0_chain, after a successful |X509_verify_cert| call, // returns the verified certificate chain. The chain begins with the leaf and // ends with trust anchor. @@ -3917,9 +3974,6 @@ // result with |sk_X509_pop_free| and |X509_free| when done. OPENSSL_EXPORT STACK_OF(X509) *X509_STORE_CTX_get1_chain(X509_STORE_CTX *ctx); -OPENSSL_EXPORT void X509_STORE_CTX_set_cert(X509_STORE_CTX *c, X509 *x); -OPENSSL_EXPORT void X509_STORE_CTX_set_chain(X509_STORE_CTX *c, - STACK_OF(X509) *sk); OPENSSL_EXPORT STACK_OF(X509) *X509_STORE_CTX_get0_untrusted( X509_STORE_CTX *ctx); OPENSSL_EXPORT void X509_STORE_CTX_set0_crls(X509_STORE_CTX *c, @@ -3947,28 +4001,6 @@ unsigned long flags, int64_t t); -// X509_STORE_CTX_set_verify_cb configures a callback function for |ctx| that is -// called multiple times during |X509_verify_cert|. The callback returns zero to -// fail verification and one to proceed. Typically, it will return |ok|, which -// preserves the default behavior. Returning one when |ok| is zero will proceed -// past some error. The callback may inspect |ctx| and the error queue to -// attempt to determine the current stage of certificate verification, but this -// is often unreliable. When synthesizing an error, callbacks should use -// |X509_STORE_CTX_set_error| to set a corresponding error. -// -// WARNING: Do not use this function. It is extremely fragile and unpredictable. -// This callback exposes implementation details of certificate verification, -// which change as the library evolves. Attempting to use it for security checks -// can introduce vulnerabilities if making incorrect assumptions about when the -// callback is called. Additionally, overriding |ok| may leave |ctx| in an -// inconsistent state and break invariants. -// -// Instead, customize certificate verification by configuring options on the -// |X509_STORE_CTX| before verification, or applying additional checks after -// |X509_verify_cert| completes successfully. -OPENSSL_EXPORT void X509_STORE_CTX_set_verify_cb( - X509_STORE_CTX *ctx, int (*verify_cb)(int ok, X509_STORE_CTX *ctx)); - // X509_STORE_CTX_get0_param returns |ctx|'s verification parameters. This // object is mutable and may be modified by the caller. OPENSSL_EXPORT X509_VERIFY_PARAM *X509_STORE_CTX_get0_param(