diff options
| author | Viktor Dukhovni <openssl-users@dukhovni.org> | 2016-04-26 14:17:57 -0400 |
|---|---|---|
| committer | Viktor Dukhovni <openssl-users@dukhovni.org> | 2016-04-27 14:42:38 -0400 |
| commit | 69664d6af0cdd7738f55d10fbbe46cdf15f72e0e (patch) | |
| tree | fda55c65815fb5b568812d84059eefe1b18bed18 /crypto/x509 | |
| parent | 4c5e6b2cb95a4332829af140e5edba965c9685ce (diff) | |
Future proof build_chain() in x509_vfy.c
Coverity reports a potential NULL deref when "2 0 0" DANE trust-anchors
from DNS are configured via SSL_dane_tlsa_add() and X509_STORE_CTX_init()
is called with a NULL stack of untrusted certificates.
Since ssl_verify_cert_chain() always provideds a non-NULL stack of
untrusted certs, and no other code path enables DANE, the problem
can only happen in applications that use SSL_CTX_set_cert_verify_callback()
to implement their own wrappers around X509_verify_cert() passing
only the leaf certificate to the latter.
Regardless of the "improbability" of the problem, we do need to
ensure that build_chain() handles this case correctly.
Reviewed-by: Matt Caswell <matt@openssl.org>
Diffstat (limited to 'crypto/x509')
| -rw-r--r-- | crypto/x509/x509_vfy.c | 15 |
1 files changed, 14 insertions, 1 deletions
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index b895ffe33e..30eabcb558 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -2789,8 +2789,21 @@ static int build_chain(X509_STORE_CTX *ctx) return 0; } - /* Include any untrusted full certificates from DNS */ + /* + * If we got any "DANE-TA(2) Cert(0) Full(0)" trust-anchors from DNS, add + * them to our working copy of the untrusted certificate stack. Since the + * caller of X509_STORE_CTX_init() may have provided only a leaf cert with + * no corresponding stack of untrusted certificates, we may need to create + * an empty stack first. [ At present only the ssl library provides DANE + * support, and ssl_verify_cert_chain() always provides a non-null stack + * containing at least the leaf certificate, but we must be prepared for + * this to change. ] + */ if (DANETLS_ENABLED(dane) && dane->certs != NULL) { + if (sktmp == NULL && (sktmp = sk_X509_new_null()) == NULL) { + X509err(X509_F_BUILD_CHAIN, ERR_R_MALLOC_FAILURE); + return 0; + } for (i = 0; i < sk_X509_num(dane->certs); ++i) { if (!sk_X509_push(sktmp, sk_X509_value(dane->certs, i))) { sk_X509_free(sktmp); |