Reduce caller requirements on X509V3_CTX.

This relaxes two caller requirements:

First, although one needs to initialize X509V3_CTX in two parts, some
callers forget to this. This works some of the time on accident,
because most codepaths read ctx->db. But if one were to read it, it'd
be uninitialized. Since all the entrypoints take a CONF anyway, and
always match them, just implicitly initialize the CONF half of the
X509V3_CTX with the provided one.

Second, allow X509V3_CTX to be NULL. Some codepaths in the library
check for NULL (or don't use it) and some do not. Enough codepaths
don't check that it really cannot be considered to work, but enough
do that a caller could mistakenly pass in NULL and have it mostly
work. I've seen one caller mistakenly do this. Since we have to copy
the X509V3_CTX for the first relaxation anyway, allow it to be NULL
and fill in an empty one when omitted.

Update-Note: If using different CONFs in the X509V3_CTX and the function
parameter, the function parameter is now always used. No callers do
this, and it's somewhat arbitrary which is used. (The generic code
always uses the one in ctx. The @section syntax uses the parameter. Then
the per-extension callbacks use the ctx.)

Change-Id: I9fc15a581ea375ea06c4b082dcf0d6360be8144f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56109
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>

1 files changed, 11 insertions, 15 deletions

diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h
index a3d0d33..25a72d4 100644
--- a/include/openssl/x509v3.h
+++ b/include/openssl/x509v3.h
@@ -586,8 +586,9 @@ struct v3_ext_ctx {
 //
 // |issuer|, |subject|, |req|, and |crl|, if non-NULL, must outlive |ctx|.
 //
-// WARNING: This function only partially initializes |ctx|. Callers must also
-// call |X509V3_set_nconf| or |X509V3_set_ctx_nodb|.
+// WARNING: This function only partially initializes |ctx|. Unless otherwise
+// documented, callers must also call |X509V3_set_nconf| or
+// |X509V3_set_ctx_nodb|.
 OPENSSL_EXPORT void X509V3_set_ctx(X509V3_CTX *ctx, const X509 *issuer,
                                    const X509 *subject, const X509_REQ *req,
                                    const X509_CRL *crl, int flags);
@@ -597,8 +598,9 @@ OPENSSL_EXPORT void X509V3_set_ctx(X509V3_CTX *ctx, const X509 *issuer,
 // incomplete and should be discarded. This can be used to partially validate
 // syntax.
 //
-// WARNING: This function only partially initializes |ctx|. Callers must also
-// call |X509V3_set_nconf| or |X509V3_set_ctx_nodb|.
+// WARNING: This function only partially initializes |ctx|. Unless otherwise
+// documented, callers must also call |X509V3_set_nconf| or
+// |X509V3_set_ctx_nodb|.
 //
 // TODO(davidben): Can we remove this?
 #define X509V3_set_ctx_test(ctx) \
@@ -611,13 +613,6 @@ OPENSSL_EXPORT void X509V3_set_ctx(X509V3_CTX *ctx, const X509 *issuer,
 //
 // WARNING: This function only partially initializes |ctx|. Callers must also
 // call |X509V3_set_ctx| or |X509V3_set_ctx_test|.
-//
-// TODO(davidben): All the public entrypoints take a |CONF| already. OpenSSL
-// does not document the relationship between |db| in this structure and the
-// parameter, but all callers either match them, or use NULL and forget to call
-// |X509V3_set_ctx_nodb|. The latter results in reading an uninitialized pointer
-// if an applicable format is ever accidentally used. Perhaps this should be
-// automatically initialized by |X509V3_EXT_nconf|, etc.
 OPENSSL_EXPORT void X509V3_set_nconf(X509V3_CTX *ctx, const CONF *conf);
 
 // X509V3_set_ctx_nodb calls |X509V3_set_nconf| with no config database.
@@ -626,11 +621,12 @@ OPENSSL_EXPORT void X509V3_set_nconf(X509V3_CTX *ctx, const CONF *conf);
 // X509V3_EXT_nconf constructs an extension of type specified by |name|, and
 // value specified by |value|. It returns a newly-allocated |X509_EXTENSION|
 // object on success, or NULL on error. |conf| and |ctx| specify additional
-// information referenced by some formats. |conf| may be NULL, in which case
-// features which use it will be disabled.
+// information referenced by some formats. Either |conf| or |ctx| may be NULL,
+// in which case features which use it will be disabled.
 //
-// TODO(davidben): Allow |ctx| to be NULL. One caller seems to do it, even
-// though it doesn't really work.
+// If non-NULL, |ctx| must be initialized with |X509V3_set_ctx| or
+// |X509V3_set_ctx_test|. This function implicitly calls |X509V3_set_nconf| with
+// |conf|, so it is safe to only call |X509V3_set_ctx|.
 OPENSSL_EXPORT X509_EXTENSION *X509V3_EXT_nconf(const CONF *conf,
                                                 const X509V3_CTX *ctx,
                                                 const char *name,