Commit c0a445a9 authored by Viktor Dukhovni's avatar Viktor Dukhovni
Browse files

Suppress DANE TLSA reflection when verification fails 



As documented both SSL_get0_dane_authority() and SSL_get0_dane_tlsa()
are expected to return a negative match depth and nothing else when
verification fails.  However, this only happened when verification
failed during chain construction.  Errors in verification of the
constructed chain did not have the intended effect on these functions.

This commit updates the functions to check for verify_result ==
X509_V_OK, and no longer erases any accumulated match information
when chain construction fails.  Sophisticated developers can, with
care, use SSL_set_verify_result(ssl, X509_V_OK) to "peek" at TLSA
info even when verification fail.  They must of course first check
and save the real error, and restore the original error as quickly
as possible.  Hiding by default seems to be the safer interface.

Introduced X509_V_ERR_DANE_NO_MATCH code to signal failure to find
matching TLSA records.  Previously reported via X509_V_ERR_CERT_UNTRUSTED.

This also changes the "-brief" output from s_client to include
verification results and TLSA match information.

Mentioned session resumption in code example in SSL_CTX_dane_enable(3).
Also mentioned that depths returned are relative to the verified chain
which is now available via SSL_get0_verified_chain(3).

Added a few more test-cases to danetest, that exercise the new
code.

Resolved thread safety issue in use of static buffer in
X509_verify_cert_error_string().

Fixed long-stating issue in apps/s_cb.c which always sets verify_error
to either X509_V_OK or "chain to long", code elsewhere (e.g.
s_time.c), seems to expect the actual error.  [ The new chain
construction code is expected to correctly generate "chain
too long" errors, so at some point we need to drop the
work-arounds, once SSL_set_verify_depth() is also fixed to
propagate the depth to X509_STORE_CTX reliably. ]

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
parent 2d9a9d8a

apps/s_apps.h

+1 −0
Original line number Diff line number Diff line
@@ -192,6 +192,7 @@ void ssl_ctx_set_excert(SSL_CTX *ctx, SSL_EXCERT *exc); 
void ssl_excert_free(SSL_EXCERT *exc);

int args_excert(int option, SSL_EXCERT **pexc);

int load_excert(SSL_EXCERT **pexc);

void print_verify_detail(SSL *s, BIO *bio);

void print_ssl_summary(SSL *s);

#ifdef HEADER_SSL_H

int config_ctx(SSL_CONF_CTX *cctx, STACK_OF(OPENSSL_STRING) *str,

apps/s_cb.c

+77 −1
Original line number Diff line number Diff line
@@ -167,7 +167,7 @@ int verify_callback(int ok, X509_STORE_CTX *ctx) 
        if (verify_depth >= depth) {

            if (!verify_return_error)

                ok = 1;

            verify_error = X509_V_OK;

            verify_error = err;

        } else {

            ok = 0;

            verify_error = X509_V_ERR_CERT_CHAIN_TOO_LONG;
@@ -1086,6 +1086,80 @@ static void print_raw_cipherlist(SSL *s) 
    BIO_puts(bio_err, "\n");

}



/*

 * Hex encoder for TLSA RRdata, not ':' delimited.

 */

static char *hexencode(const unsigned char *data, size_t len)

{

    static const char *hex = "0123456789abcdef";

    char *out;

    char *cp;

    size_t outlen = 2 * len + 1;

    int ilen = (int) outlen;



    if (outlen < len || ilen < 0 || outlen != (size_t)ilen) {

        BIO_printf(bio_err, "%s: %" PRIu64 "-byte buffer too large to hexencode\n",

                   opt_getprog(), (uint64_t)len);

        exit(1);

    }

    cp = out = app_malloc(ilen, "TLSA hex data buffer");



    while (ilen-- > 0) {

        *cp++ = hex[(*data >> 4) & 0x0f];

        *cp++ = hex[*data++ & 0x0f];

    }

    *cp = '\0';

    return out;

}



void print_verify_detail(SSL *s, BIO *bio)

{

    int mdpth;

    EVP_PKEY *mspki;

    long verify_err = SSL_get_verify_result(s);



    if (verify_err == X509_V_OK) {

        const char *peername = SSL_get0_peername(s);



        BIO_printf(bio, "Verification: OK\n");

        if (peername != NULL)

            BIO_printf(bio, "Verified peername: %s\n", peername);

    } else {

        const char *reason = X509_verify_cert_error_string(verify_err);



        BIO_printf(bio, "Verification error: %s\n", reason);

    }



    if ((mdpth = SSL_get0_dane_authority(s, NULL, &mspki)) >= 0) {

        uint8_t usage, selector, mtype;

        const unsigned char *data = NULL;

        size_t dlen = 0;

        char *hexdata;



        mdpth = SSL_get0_dane_tlsa(s, &usage, &selector, &mtype, &data, &dlen);



        /*

         * The TLSA data field can be quite long when it is a certificate,

         * public key or even a SHA2-512 digest.  Because the initial octets of

         * ASN.1 certificates and public keys contain mostly boilerplate OIDs

         * and lengths, we show the last 12 bytes of the data instead, as these

         * are more likely to distinguish distinct TLSA records.

         */

#define TLSA_TAIL_SIZE 12

        if (dlen > TLSA_TAIL_SIZE)

            hexdata = hexencode(data + dlen - TLSA_TAIL_SIZE, TLSA_TAIL_SIZE);

        else

            hexdata = hexencode(data, dlen);

        BIO_printf(bio, "DANE TLSA %d %d %d %s%s %s at depth %d\n",

                   usage, selector, mtype,

                   (dlen > TLSA_TAIL_SIZE) ? "..." : "", hexdata,

                   (mspki != NULL) ? "signed the certificate" :

                   mdpth ? "matched TA certificate" : "matched EE certificate",

                   mdpth);

        OPENSSL_free(hexdata);

    }

}



void print_ssl_summary(SSL *s)

{

    const SSL_CIPHER *c;
@@ -1100,12 +1174,14 @@ void print_ssl_summary(SSL *s) 
    peer = SSL_get_peer_certificate(s);

    if (peer) {

        int nid;



        BIO_puts(bio_err, "Peer certificate: ");

        X509_NAME_print_ex(bio_err, X509_get_subject_name(peer),

                           0, XN_FLAG_ONELINE);

        BIO_puts(bio_err, "\n");

        if (SSL_get_peer_signature_nid(s, &nid))

            BIO_printf(bio_err, "Hash used: %s\n", OBJ_nid2sn(nid));

        print_verify_detail(s, bio_err);

    } else

        BIO_puts(bio_err, "No peer certificate\n");

    X509_free(peer);

apps/s_client.c

+1 −15
Original line number Diff line number Diff line
@@ -2461,9 +2461,6 @@ static void print_stuff(BIO *bio, SSL *s, int full) 
    const SSL_CIPHER *c;

    X509_NAME *xn;

    int i;

    int mdpth;

    EVP_PKEY *mspki;

    const char *peername;

#ifndef OPENSSL_NO_COMP

    const COMP_METHOD *comp, *expansion;

#endif
@@ -2525,18 +2522,7 @@ static void print_stuff(BIO *bio, SSL *s, int full) 
                   BIO_number_read(SSL_get_rbio(s)),

                   BIO_number_written(SSL_get_wbio(s)));

    }

    if ((mdpth = SSL_get0_dane_authority(s, NULL, &mspki)) >= 0) {

        uint8_t usage, selector, mtype;

        mdpth = SSL_get0_dane_tlsa(s, &usage, &selector, &mtype, NULL, NULL);

        BIO_printf(bio, "DANE TLSA %d %d %d %s at depth %d\n",

                   usage, selector, mtype,

                   (mspki != NULL) ? "TA public key verified certificate" :

                   mdpth ? "matched TA certificate" : "matched EE certificate",

                   mdpth);

    }

    if (SSL_get_verify_result(s) == X509_V_OK &&

        (peername = SSL_get0_peername(s)) != NULL)

        BIO_printf(bio, "Verified peername: %s\n", peername);

    print_verify_detail(s, bio);

    BIO_printf(bio, (SSL_session_reused(s) ? "---\nReused, " : "---\nNew, "));

    c = SSL_get_current_cipher(s);

    BIO_printf(bio, "%s, Cipher is %s\n",

crypto/x509/x509_txt.c

+6 −4
Original line number Diff line number Diff line
@@ -69,11 +69,11 @@ 


const char *X509_verify_cert_error_string(long n)

{

    static char buf[100];



    switch ((int)n) {

    case X509_V_OK:

        return ("ok");

    case X509_V_ERR_UNSPECIFIED:

        return ("unspecified certificate verification error");

    case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT:

        return ("unable to get issuer certificate");

    case X509_V_ERR_UNABLE_TO_GET_CRL:
@@ -202,9 +202,11 @@ const char *X509_verify_cert_error_string(long n) 
        return ("Email address mismatch");

    case X509_V_ERR_IP_ADDRESS_MISMATCH:

        return ("IP address mismatch");

    case X509_V_ERR_DANE_NO_MATCH:

        return ("No matching DANE TLSA records");



    default:

        BIO_snprintf(buf, sizeof buf, "error number %ld", n);

        return (buf);

        /* Printing an error number into a static buffer is not thread-safe */

        return ("unknown certificate verification error");

    }

}

crypto/x509/x509_vfy.c

+3 −5
Original line number Diff line number Diff line
@@ -2631,7 +2631,7 @@ static int check_dane_pkeys(X509_STORE_CTX *ctx) 
            X509_verify(cert, t->spki) <= 0)

            continue;



        /* Clear PKIX-?? matches that failed to panned out to a full chain */

        /* Clear any PKIX-?? matches that failed to extend to a full chain */

        X509_free(dane->mcert);

        dane->mcert = NULL;
@@ -2711,7 +2711,7 @@ static int dane_verify(X509_STORE_CTX *ctx) 
            return 0;

        ctx->current_cert = cert;

        ctx->error_depth = 0;

        ctx->error = X509_V_ERR_CERT_UNTRUSTED;

        ctx->error = X509_V_ERR_DANE_NO_MATCH;

        return ctx->verify_cb(0, ctx);

    }
@@ -3026,7 +3026,7 @@ static int build_chain(X509_STORE_CTX *ctx) 
            ctx->error = X509_V_ERR_CERT_CHAIN_TOO_LONG;

        else if (DANETLS_ENABLED(dane) &&

                 (!DANETLS_HAS_PKIX(dane) || dane->pdpth >= 0))

            ctx->error = X509_V_ERR_CERT_UNTRUSTED;

            ctx->error = X509_V_ERR_DANE_NO_MATCH;

        else if (ss && sk_X509_num(ctx->chain) == 1)

            ctx->error = X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT;

        else if (ss)
@@ -3035,8 +3035,6 @@ static int build_chain(X509_STORE_CTX *ctx) 
            ctx->error = X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY;

        else

            ctx->error = X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT;

        if (DANETLS_ENABLED(dane))

            dane_reset(dane);

        return ctx->verify_cb(0, ctx);

    }

}