From d515722d2291703589b55661f805e73193524914 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Mon, 12 Dec 2016 11:37:43 -0800 Subject: [PATCH] Don't depend on the X509 code for getting public keys. This change removes the use of |X509_get_pubkey| from the TLS <= 1.2 code. That function is replaced with a shallow parse of the certificate to extract the public key instead. Change-Id: I8938c6c5a01b32038c6b6fa58eb065e5b44ca6d2 Reviewed-on: https://boringssl-review.googlesource.com/12707 Reviewed-by: Adam Langley Commit-Queue: Adam Langley CQ-Verified: CQ bot account: commit-bot@chromium.org --- crypto/err/ssl.errordata | 1 + include/openssl/ssl.h | 1 + ssl/handshake_client.c | 30 +++++-------------- ssl/handshake_server.c | 42 ++++++++++---------------- ssl/internal.h | 19 ++++++++++-- ssl/s3_both.c | 1 + ssl/ssl_cert.c | 65 +++++++++++++++++++++++++++++++++++++--- 7 files changed, 104 insertions(+), 55 deletions(-) diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata index e9b20664..bac2f39e 100644 --- a/crypto/err/ssl.errordata +++ b/crypto/err/ssl.errordata @@ -22,6 +22,7 @@ SSL,261,BLOCK_CIPHER_PAD_IS_WRONG SSL,120,BN_LIB SSL,255,BUFFERED_MESSAGES_ON_CIPHER_CHANGE SSL,121,BUFFER_TOO_SMALL +SSL,272,CANNOT_PARSE_LEAF_CERT SSL,122,CA_DN_LENGTH_MISMATCH SSL,123,CA_DN_TOO_LONG SSL,124,CCS_RECEIVED_EARLY diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 49a3ffdf..58f8d64d 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -4587,6 +4587,7 @@ BORINGSSL_MAKE_DELETER(SSL_SESSION, SSL_SESSION_free) #define SSL_R_INVALID_SCT_LIST 269 #define SSL_R_TOO_MUCH_SKIPPED_EARLY_DATA 270 #define SSL_R_PSK_IDENTITY_BINDER_COUNT_MISMATCH 271 +#define SSL_R_CANNOT_PARSE_LEAF_CERT 272 #define SSL_R_SSLV3_ALERT_CLOSE_NOTIFY 1000 #define SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE 1010 #define SSL_R_SSLV3_ALERT_BAD_RECORD_MAC 1020 diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c index 59490204..352ddc97 100644 --- a/ssl/handshake_client.c +++ b/ssl/handshake_client.c @@ -1044,8 +1044,10 @@ static int ssl3_get_server_certificate(SSL_HANDSHAKE *hs) { uint8_t alert; sk_CRYPTO_BUFFER_pop_free(ssl->s3->new_session->certs, CRYPTO_BUFFER_free); - ssl->s3->new_session->certs = - ssl_parse_cert_chain(&alert, NULL, &cbs, ssl->ctx->pool); + EVP_PKEY_free(hs->peer_pubkey); + hs->peer_pubkey = NULL; + ssl->s3->new_session->certs = ssl_parse_cert_chain( + &alert, &hs->peer_pubkey, NULL, &cbs, ssl->ctx->pool); if (ssl->s3->new_session->certs == NULL) { ssl3_send_alert(ssl, SSL3_AL_FATAL, alert); return -1; @@ -1123,7 +1125,6 @@ static int ssl3_verify_server_cert(SSL_HANDSHAKE *hs) { static int ssl3_get_server_key_exchange(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; int al; - EVP_PKEY *pkey = NULL; DH *dh = NULL; EC_KEY *ecdh = NULL; EC_POINT *srvr_ecpoint = NULL; @@ -1276,11 +1277,6 @@ static int ssl3_get_server_key_exchange(SSL_HANDSHAKE *hs) { /* ServerKeyExchange should be signed by the server's public key. */ if (ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) { - pkey = X509_get_pubkey(ssl->s3->new_session->x509_peer); - if (pkey == NULL) { - goto err; - } - uint16_t signature_algorithm = 0; if (ssl3_protocol_version(ssl) >= TLS1_2_VERSION) { if (!CBS_get_u16(&server_key_exchange, &signature_algorithm)) { @@ -1292,9 +1288,9 @@ static int ssl3_get_server_key_exchange(SSL_HANDSHAKE *hs) { goto f_err; } ssl->s3->tmp.peer_signature_algorithm = signature_algorithm; - } else if (pkey->type == EVP_PKEY_RSA) { + } else if (hs->peer_pubkey->type == EVP_PKEY_RSA) { signature_algorithm = SSL_SIGN_RSA_PKCS1_MD5_SHA1; - } else if (pkey->type == EVP_PKEY_EC) { + } else if (hs->peer_pubkey->type == EVP_PKEY_EC) { signature_algorithm = SSL_SIGN_ECDSA_SHA1; } else { al = SSL_AD_UNSUPPORTED_CERTIFICATE; @@ -1327,7 +1323,7 @@ static int ssl3_get_server_key_exchange(SSL_HANDSHAKE *hs) { int sig_ok = ssl_public_key_verify( ssl, CBS_data(&signature), CBS_len(&signature), signature_algorithm, - pkey, transcript_data, transcript_len); + hs->peer_pubkey, transcript_data, transcript_len); OPENSSL_free(transcript_data); #if defined(BORINGSSL_UNSAFE_FUZZER_MODE) @@ -1350,13 +1346,11 @@ static int ssl3_get_server_key_exchange(SSL_HANDSHAKE *hs) { goto f_err; } } - EVP_PKEY_free(pkey); return 1; f_err: ssl3_send_alert(ssl, SSL3_AL_FATAL, al); err: - EVP_PKEY_free(pkey); DH_free(dh); EC_POINT_free(srvr_ecpoint); EC_KEY_free(ecdh); @@ -1557,20 +1551,12 @@ static int ssl3_send_client_key_exchange(SSL_HANDSHAKE *hs) { goto err; } - EVP_PKEY *pkey = X509_get_pubkey(ssl->s3->new_session->x509_peer); - if (pkey == NULL) { - goto err; - } - - RSA *rsa = EVP_PKEY_get0_RSA(pkey); + RSA *rsa = EVP_PKEY_get0_RSA(hs->peer_pubkey); if (rsa == NULL) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - EVP_PKEY_free(pkey); goto err; } - EVP_PKEY_free(pkey); - pms[0] = hs->client_version >> 8; pms[1] = hs->client_version & 0xff; if (!RAND_bytes(&pms[2], SSL_MAX_MASTER_KEY_LENGTH - 2)) { diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c index 88472518..7f4a627f 100644 --- a/ssl/handshake_server.c +++ b/ssl/handshake_server.c @@ -1468,11 +1468,14 @@ static int ssl3_get_client_certificate(SSL_HANDSHAKE *hs) { CBS_init(&certificate_msg, ssl->init_msg, ssl->init_num); sk_CRYPTO_BUFFER_pop_free(ssl->s3->new_session->certs, CRYPTO_BUFFER_free); + EVP_PKEY_free(hs->peer_pubkey); + hs->peer_pubkey = NULL; uint8_t alert; ssl->s3->new_session->certs = - ssl_parse_cert_chain(&alert, ssl->retain_only_sha256_of_client_certs - ? ssl->s3->new_session->peer_sha256 - : NULL, + ssl_parse_cert_chain(&alert, &hs->peer_pubkey, + ssl->retain_only_sha256_of_client_certs + ? ssl->s3->new_session->peer_sha256 + : NULL, &certificate_msg, ssl->ctx->pool); if (ssl->s3->new_session->certs == NULL) { ssl3_send_alert(ssl, SSL3_AL_FATAL, alert); @@ -1794,15 +1797,13 @@ err: static int ssl3_get_cert_verify(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; - int al, ret = 0; + int al; CBS certificate_verify, signature; - X509 *peer = ssl->s3->new_session->x509_peer; - EVP_PKEY *pkey = NULL; /* Only RSA and ECDSA client certificates are supported, so a * CertificateVerify is required if and only if there's a client certificate. * */ - if (peer == NULL) { + if (hs->peer_pubkey == NULL) { ssl3_free_handshake_buffer(ssl); return 1; } @@ -1813,12 +1814,6 @@ static int ssl3_get_cert_verify(SSL_HANDSHAKE *hs) { return msg_ret; } - /* Filter out unsupported certificate types. */ - pkey = X509_get_pubkey(peer); - if (pkey == NULL) { - goto err; - } - CBS_init(&certificate_verify, ssl->init_msg, ssl->init_num); /* Determine the digest type if needbe. */ @@ -1833,9 +1828,9 @@ static int ssl3_get_cert_verify(SSL_HANDSHAKE *hs) { goto f_err; } ssl->s3->tmp.peer_signature_algorithm = signature_algorithm; - } else if (pkey->type == EVP_PKEY_RSA) { + } else if (hs->peer_pubkey->type == EVP_PKEY_RSA) { signature_algorithm = SSL_SIGN_RSA_PKCS1_MD5_SHA1; - } else if (pkey->type == EVP_PKEY_EC) { + } else if (hs->peer_pubkey->type == EVP_PKEY_EC) { signature_algorithm = SSL_SIGN_ECDSA_SHA1; } else { al = SSL_AD_UNSUPPORTED_CERTIFICATE; @@ -1863,7 +1858,7 @@ static int ssl3_get_cert_verify(SSL_HANDSHAKE *hs) { goto err; } - EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new(pkey, NULL); + EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new(hs->peer_pubkey, NULL); sig_ok = pctx != NULL && EVP_PKEY_verify_init(pctx) && EVP_PKEY_CTX_set_signature_md(pctx, md) && @@ -1873,7 +1868,7 @@ static int ssl3_get_cert_verify(SSL_HANDSHAKE *hs) { } else { sig_ok = ssl_public_key_verify( ssl, CBS_data(&signature), CBS_len(&signature), signature_algorithm, - pkey, (const uint8_t *)ssl->s3->handshake_buffer->data, + hs->peer_pubkey, (const uint8_t *)ssl->s3->handshake_buffer->data, ssl->s3->handshake_buffer->length); } @@ -1894,17 +1889,12 @@ static int ssl3_get_cert_verify(SSL_HANDSHAKE *hs) { goto err; } - ret = 1; - - if (0) { - f_err: - ssl3_send_alert(ssl, SSL3_AL_FATAL, al); - } + return 1; +f_err: + ssl3_send_alert(ssl, SSL3_AL_FATAL, al); err: - EVP_PKEY_free(pkey); - - return ret; + return 0; } /* ssl3_get_next_proto reads a Next Protocol Negotiation handshake message. It diff --git a/ssl/internal.h b/ssl/internal.h index 3640b65c..98c5cdf1 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -757,10 +757,15 @@ int ssl_session_x509_cache_objects(SSL_SESSION *sess); /* ssl_parse_cert_chain parses a certificate list from |cbs| in the format used * by a TLS Certificate message. On success, it returns a newly-allocated * |CRYPTO_BUFFER| list and advances |cbs|. Otherwise, it returns NULL and sets - * |*out_alert| to an alert to send to the peer. If the list is non-empty and - * |out_leaf_sha256| is non-NULL, it writes the SHA-256 hash of the leaf to - * |out_leaf_sha256|. */ + * |*out_alert| to an alert to send to the peer. + * + * If the list is non-empty then |*out_pubkey| will be set to a freshly + * allocated public-key from the leaf certificate. + * + * If the list is non-empty and |out_leaf_sha256| is non-NULL, it writes the + * SHA-256 hash of the leaf to |out_leaf_sha256|. */ STACK_OF(CRYPTO_BUFFER) *ssl_parse_cert_chain(uint8_t *out_alert, + EVP_PKEY **out_pubkey, uint8_t *out_leaf_sha256, CBS *cbs, CRYPTO_BUFFER_POOL *pool); @@ -774,6 +779,11 @@ int ssl_add_cert_to_cbb(CBB *cbb, X509 *x509); * empty certificate list. It returns one on success and zero on error. */ int ssl_add_cert_chain(SSL *ssl, CBB *cbb); +/* ssl_cert_parse_pubkey extracts the public key from the DER-encoded, X.509 + * certificate in |in|. It returns an allocated |EVP_PKEY| or else returns NULL + * and pushes to the error queue. */ +EVP_PKEY *ssl_cert_parse_pubkey(const CBS *in); + /* ssl_parse_client_CA_list parses a CA list from |cbs| in the format used by a * TLS CertificateRequest message. On success, it returns a newly-allocated * |X509_NAME| list and advances |cbs|. Otherwise, it returns NULL and sets @@ -980,6 +990,9 @@ struct ssl_handshake_st { /* hostname, on the server, is the value of the SNI extension. */ char *hostname; + /* peer_pubkey is the public key parsed from the peer's leaf certificate. */ + EVP_PKEY *peer_pubkey; + /* key_block is the record-layer key block for TLS 1.2 and earlier. */ uint8_t *key_block; uint8_t key_block_len; diff --git a/ssl/s3_both.c b/ssl/s3_both.c index eeccab15..4800f920 100644 --- a/ssl/s3_both.c +++ b/ssl/s3_both.c @@ -171,6 +171,7 @@ void ssl_handshake_free(SSL_HANDSHAKE *hs) { } OPENSSL_free(hs->hostname); + EVP_PKEY_free(hs->peer_pubkey); OPENSSL_free(hs); } diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index 7711cd12..f9ee7932 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -120,8 +120,9 @@ #include #include -#include +#include #include +#include #include #include #include @@ -463,9 +464,12 @@ X509 *ssl_parse_x509(CBS *cbs) { } STACK_OF(CRYPTO_BUFFER) *ssl_parse_cert_chain(uint8_t *out_alert, + EVP_PKEY **out_pubkey, uint8_t *out_leaf_sha256, CBS *cbs, CRYPTO_BUFFER_POOL *pool) { + *out_pubkey = NULL; + STACK_OF(CRYPTO_BUFFER) *ret = sk_CRYPTO_BUFFER_new_null(); if (ret == NULL) { *out_alert = SSL_AD_INTERNAL_ERROR; @@ -489,9 +493,16 @@ STACK_OF(CRYPTO_BUFFER) *ssl_parse_cert_chain(uint8_t *out_alert, goto err; } - /* Retain the hash of the leaf certificate if requested. */ - if (sk_CRYPTO_BUFFER_num(ret) == 0 && out_leaf_sha256 != NULL) { - SHA256(CBS_data(&certificate), CBS_len(&certificate), out_leaf_sha256); + if (sk_CRYPTO_BUFFER_num(ret) == 0) { + *out_pubkey = ssl_cert_parse_pubkey(&certificate); + if (*out_pubkey == NULL) { + goto err; + } + + /* Retain the hash of the leaf certificate if requested. */ + if (out_leaf_sha256 != NULL) { + SHA256(CBS_data(&certificate), CBS_len(&certificate), out_leaf_sha256); + } } CRYPTO_BUFFER *buf = @@ -512,6 +523,8 @@ STACK_OF(CRYPTO_BUFFER) *ssl_parse_cert_chain(uint8_t *out_alert, return ret; err: + EVP_PKEY_free(*out_pubkey); + *out_pubkey = NULL; sk_CRYPTO_BUFFER_pop_free(ret, CRYPTO_BUFFER_free); return NULL; } @@ -594,6 +607,50 @@ int ssl_add_cert_chain(SSL *ssl, CBB *cbb) { return CBB_flush(cbb); } +EVP_PKEY *ssl_cert_parse_pubkey(const CBS *in) { + /* From RFC 5280, section 4.1 + * Certificate ::= SEQUENCE { + * tbsCertificate TBSCertificate, + * signatureAlgorithm AlgorithmIdentifier, + * signatureValue BIT STRING } + + * TBSCertificate ::= SEQUENCE { + * version [0] EXPLICIT Version DEFAULT v1, + * serialNumber CertificateSerialNumber, + * signature AlgorithmIdentifier, + * issuer Name, + * validity Validity, + * subject Name, + * subjectPublicKeyInfo SubjectPublicKeyInfo, + * ... } */ + CBS buf = *in; + + CBS toplevel, tbs_cert, spki; + if (!CBS_get_asn1(&buf, &toplevel, CBS_ASN1_SEQUENCE) || + CBS_len(&buf) != 0 || + !CBS_get_asn1(&toplevel, &tbs_cert, CBS_ASN1_SEQUENCE) || + /* version */ + !CBS_get_optional_asn1( + &tbs_cert, NULL, NULL, + CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 0) || + /* serialNumber */ + !CBS_get_asn1(&tbs_cert, NULL, CBS_ASN1_INTEGER) || + /* signature algorithm */ + !CBS_get_asn1(&tbs_cert, NULL, CBS_ASN1_SEQUENCE) || + /* issuer */ + !CBS_get_asn1(&tbs_cert, NULL, CBS_ASN1_SEQUENCE) || + /* validity */ + !CBS_get_asn1(&tbs_cert, NULL, CBS_ASN1_SEQUENCE) || + /* subject */ + !CBS_get_asn1(&tbs_cert, NULL, CBS_ASN1_SEQUENCE) || + !CBS_get_asn1_element(&tbs_cert, &spki, CBS_ASN1_SEQUENCE)) { + OPENSSL_PUT_ERROR(SSL, SSL_R_CANNOT_PARSE_LEAF_CERT); + return NULL; + } + + return EVP_parse_public_key(&spki); +} + static int ca_dn_cmp(const X509_NAME **a, const X509_NAME **b) { return X509_NAME_cmp(*a, *b); }