From 56d250321ea9dfa66ea9afa599f12c83a4147c86 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Tue, 23 Jun 2015 16:20:13 -0700 Subject: Fixes for CVE-2015-1791. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a NewSessionTicket is received by a multi-threaded client when attempting to reuse a previous ticket then a race condition can occur potentially leading to a double free of the ticket data. This change cherry-picks the following BoringSSL changes: b31040d0 – Get rid of CERT_PKEY slots in SESS_CERT. fd67aa8c – Add SSL_SESSION_from_bytes. 95d31825 – Duplicate SSL_SESSIONs when renewing them. d65bb78c – Add SSL_initial_handshake_complete. 680ca961 – Preserve session->sess_cert on ticket renewal. Change-Id: I474065330842e4ab0066b2485c1489a50e4dfd5b --- src/crypto/err/ssl.errordata | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'src/crypto') diff --git a/src/crypto/err/ssl.errordata b/src/crypto/err/ssl.errordata index 4ae0a51..9464c3d 100644 --- a/src/crypto/err/ssl.errordata +++ b/src/crypto/err/ssl.errordata @@ -20,7 +20,11 @@ SSL,function,112,SSL_CTX_use_certificate_ASN1 SSL,function,113,SSL_CTX_use_certificate_chain_file SSL,function,114,SSL_CTX_use_certificate_file SSL,function,115,SSL_CTX_use_psk_identity_hint +SSL,function,280,SSL_SESSION_from_bytes SSL,function,116,SSL_SESSION_new +SSL,function,281,SSL_SESSION_parse +SSL,function,150,SSL_SESSION_parse_octet_string +SSL,function,151,SSL_SESSION_parse_string SSL,function,117,SSL_SESSION_print_fp SSL,function,118,SSL_SESSION_set1_id_context SSL,function,119,SSL_SESSION_to_bytes_full @@ -58,8 +62,6 @@ SSL,function,146,SSL_use_certificate_file SSL,function,147,SSL_use_psk_identity_hint SSL,function,148,SSL_write SSL,function,149,d2i_SSL_SESSION -SSL,function,150,d2i_SSL_SESSION_get_octet_string -SSL,function,151,d2i_SSL_SESSION_get_string SSL,function,152,do_ssl3_write SSL,function,153,dtls1_accept SSL,function,154,dtls1_buffer_record @@ -82,6 +84,7 @@ SSL,function,166,i2d_SSL_SESSION SSL,function,167,ssl3_accept SSL,function,169,ssl3_cert_verify_hash SSL,function,170,ssl3_check_cert_and_algorithm +SSL,function,282,ssl3_check_certificate_for_cipher SSL,function,171,ssl3_connect SSL,function,172,ssl3_ctrl SSL,function,173,ssl3_ctx_ctrl -- cgit v1.1 From 98856d4b9dc1a59a576816dbb097aa9d9e6de47a Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Tue, 23 Jun 2015 16:23:41 -0700 Subject: Fix for CVE-2015-1789. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X509_cmp_time does not properly check the length of the ASN1_TIME string and can read a few bytes out of bounds. In addition, X509_cmp_time accepts an arbitrary number of fractional seconds in the time string. An attacker can use this to craft malformed certificates and CRLs of various sizes and potentially cause a segmentation fault, resulting in a DoS on applications that verify certificates or CRLs. TLS clients that verify CRLs are affected. TLS clients and servers with client authentication enabled may be affected if they use custom verification callbacks. This change cherry-picks the following changes from BoringSSL: d87021d2 – Fix length checks in X509_cmp_time to avoid out-of-bounds reads. Change-Id: Ia7d0c5d889f61a3c4be6ea79a5ab41f67bc3c65c --- src/crypto/x509/x509_vfy.c | 54 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 7 deletions(-) (limited to 'src/crypto') diff --git a/src/crypto/x509/x509_vfy.c b/src/crypto/x509/x509_vfy.c index 2ba9c84..f53f279 100644 --- a/src/crypto/x509/x509_vfy.c +++ b/src/crypto/x509/x509_vfy.c @@ -1829,49 +1829,89 @@ int X509_cmp_time(const ASN1_TIME *ctm, time_t *cmp_time) ASN1_TIME atm; long offset; char buff1[24],buff2[24],*p; - int i,j; + int i, j, remaining; p=buff1; - i=ctm->length; + remaining = ctm->length; str=(char *)ctm->data; + /* Note that the following (historical) code allows much more slack in + * the time format than RFC5280. In RFC5280, the representation is + * fixed: + * UTCTime: YYMMDDHHMMSSZ + * GeneralizedTime: YYYYMMDDHHMMSSZ */ if (ctm->type == V_ASN1_UTCTIME) { - if ((i < 11) || (i > 17)) return 0; + /* YYMMDDHHMM[SS]Z or YYMMDDHHMM[SS](+-)hhmm */ + int min_length = sizeof("YYMMDDHHMMZ") - 1; + int max_length = sizeof("YYMMDDHHMMSS+hhmm") - 1; + if (remaining < min_length || remaining > max_length) + return 0; memcpy(p,str,10); p+=10; str+=10; + remaining -= 10; } else { - if (i < 13) return 0; + /* YYYYMMDDHHMM[SS[.fff]]Z or YYYYMMDDHHMM[SS[.f[f[f]]]](+-)hhmm */ + int min_length = sizeof("YYYYMMDDHHMMZ") - 1; + int max_length = sizeof("YYYYMMDDHHMMSS.fff+hhmm") - 1; + if (remaining < min_length || remaining > max_length) + return 0; memcpy(p,str,12); p+=12; str+=12; + remaining -= 12; } if ((*str == 'Z') || (*str == '-') || (*str == '+')) { *(p++)='0'; *(p++)='0'; } else { + /* SS (seconds) */ + if (remaining < 2) + return 0; *(p++)= *(str++); *(p++)= *(str++); - /* Skip any fractional seconds... */ - if (*str == '.') + remaining -= 2; + /* Skip any (up to three) fractional seconds... + * TODO(emilia): in RFC5280, fractional seconds are forbidden. + * Can we just kill them altogether? */ + if (remaining && *str == '.') { str++; - while ((*str >= '0') && (*str <= '9')) str++; + remaining--; + for (i = 0; i < 3 && remaining; i++, str++, remaining--) + { + if (*str < '0' || *str > '9') + break; + } } } *(p++)='Z'; *(p++)='\0'; + /* We now need either a terminating 'Z' or an offset. */ + if (!remaining) + return 0; if (*str == 'Z') + { + if (remaining != 1) + return 0; offset=0; + } else { + /* (+-)HHMM */ if ((*str != '+') && (*str != '-')) return 0; + /* Historical behaviour: the (+-)hhmm offset is forbidden in RFC5280. */ + if (remaining != 5) + return 0; + if (str[1] < '0' || str[1] > '9' || str[2] < '0' || str[2] > '9' || + str[3] < '0' || str[3] > '9' || str[4] < '0' || str[4] > '9') + return 0; offset=((str[1]-'0')*10+(str[2]-'0'))*60; offset+=(str[3]-'0')*10+(str[4]-'0'); if (*str == '-') -- cgit v1.1 From e0846beeb321f7d3170e4e389950b12fce69ab10 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Tue, 23 Jun 2015 16:25:33 -0700 Subject: dsa_pub_encode: Write out DSA parameters (p, q, g) in addition to key. This change cherry-picks BoringSSL's e65886a5. Change-Id: I63d5dc280d420b64b658bfd85f180a01adb8a18b --- src/crypto/evp/p_dsa_asn1.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) (limited to 'src/crypto') diff --git a/src/crypto/evp/p_dsa_asn1.c b/src/crypto/evp/p_dsa_asn1.c index 0ac7da7..826d4e4 100644 --- a/src/crypto/evp/p_dsa_asn1.c +++ b/src/crypto/evp/p_dsa_asn1.c @@ -129,21 +129,37 @@ err: static int dsa_pub_encode(X509_PUBKEY *pk, const EVP_PKEY *pkey) { DSA *dsa; - void *pval = NULL; + ASN1_STRING *pval = NULL; uint8_t *penc = NULL; int penclen; dsa = pkey->pkey.dsa; dsa->write_params = 0; - penclen = i2d_DSAPublicKey(dsa, &penc); + int ptype; + if (dsa->p && dsa->q && dsa->g) { + pval = ASN1_STRING_new(); + if (!pval) { + OPENSSL_PUT_ERROR(EVP, dsa_pub_encode, ERR_R_MALLOC_FAILURE); + goto err; + } + pval->length = i2d_DSAparams(dsa, &pval->data); + if (pval->length <= 0) { + OPENSSL_PUT_ERROR(EVP, dsa_pub_encode, ERR_R_MALLOC_FAILURE); + goto err; + } + ptype = V_ASN1_SEQUENCE; + } else { + ptype = V_ASN1_UNDEF; + } + penclen = i2d_DSAPublicKey(dsa, &penc); if (penclen <= 0) { OPENSSL_PUT_ERROR(EVP, dsa_pub_encode, ERR_R_MALLOC_FAILURE); goto err; } - if (X509_PUBKEY_set0_param(pk, OBJ_nid2obj(EVP_PKEY_DSA), V_ASN1_UNDEF, pval, + if (X509_PUBKEY_set0_param(pk, OBJ_nid2obj(EVP_PKEY_DSA), ptype, pval, penc, penclen)) { return 1; } -- cgit v1.1 From bd9957e6e28506c4431ce8d3cadbc0a04905b15e Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Tue, 23 Jun 2015 16:28:07 -0700 Subject: Handle RDRAND failures. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I mistakenly believed that only RDSEED could fail. However, the Intel manuals state that RDRAND can fail too. This change cherry-picks the following BoringSSL changes: 2cac3506 – Handle RDRAND failures. 248abbd7 – Add missing comma in .type pragma for rdrand code. Change-Id: Icdc56a50ce36e9c525063583882c676a5312d313 --- src/crypto/rand/asm/rdrand-x86_64.pl | 52 +++++++++++++++++++++++++++++++++++- src/crypto/rand/hwrand.c | 30 +++++++++++++-------- src/crypto/rand/internal.h | 5 ++-- src/crypto/rand/rand.c | 5 ++-- 4 files changed, 75 insertions(+), 17 deletions(-) (limited to 'src/crypto') diff --git a/src/crypto/rand/asm/rdrand-x86_64.pl b/src/crypto/rand/asm/rdrand-x86_64.pl index a917611..c32a55c 100644 --- a/src/crypto/rand/asm/rdrand-x86_64.pl +++ b/src/crypto/rand/asm/rdrand-x86_64.pl @@ -1,5 +1,19 @@ #!/usr/bin/env perl +# Copyright (c) 2015, Google Inc. +# +# Permission to use, copy, modify, and/or distribute this software for any +# purpose with or without fee is hereby granted, provided that the above +# copyright notice and this permission notice appear in all copies. +# +# THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES +# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY +# SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES +# WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION +# OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN +# CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ + $flavour = shift; $output = shift; if ($flavour =~ /\./) { $output = $flavour; undef $flavour; } @@ -14,11 +28,47 @@ open OUT,"| \"$^X\" $xlate $flavour $output"; print<<___; .text +# CRYPTO_rdrand writes eight bytes of random data from the hardware RNG to +# |out|. It returns one on success or zero on hardware failure. +# int CRYPTO_rdrand(uint8_t out[8]); .globl CRYPTO_rdrand .type CRYPTO_rdrand,\@function,1 .align 16 CRYPTO_rdrand: - .byte 0x48, 0x0f, 0xc7, 0xf0 + xorq %rax, %rax + # This is rdrand %rcx. It sets rcx to a random value and sets the carry + # flag on success. + .byte 0x48, 0x0f, 0xc7, 0xf1 + # An add-with-carry of zero effectively sets %rax to the carry flag. + adcq %rax, %rax + movq %rcx, 0(%rdi) + retq + +# CRYPTO_rdrand_multiple8_buf fills |len| bytes at |buf| with random data from +# the hardware RNG. The |len| argument must be a multiple of eight. It returns +# one on success and zero on hardware failure. +# int CRYPTO_rdrand_multiple8_buf(uint8_t *buf, size_t len); +.globl CRYPTO_rdrand_multiple8_buf +.type CRYPTO_rdrand_multiple8_buf,\@function,2 +.align 16 +CRYPTO_rdrand_multiple8_buf: + test %rsi, %rsi + jz .Lout + movq \$8, %rdx +.Lloop: + # This is rdrand %rcx. It sets rcx to a random value and sets the carry + # flag on success. + .byte 0x48, 0x0f, 0xc7, 0xf1 + jnc .Lerr + movq %rcx, 0(%rdi) + addq %rdx, %rdi + subq %rdx, %rsi + jnz .Lloop +.Lout: + movq \$1, %rax + retq +.Lerr: + xorq %rax, %rax retq ___ diff --git a/src/crypto/rand/hwrand.c b/src/crypto/rand/hwrand.c index 73d3de7..5f81f09 100644 --- a/src/crypto/rand/hwrand.c +++ b/src/crypto/rand/hwrand.c @@ -14,6 +14,7 @@ #include +#include #include #include @@ -26,21 +27,28 @@ int CRYPTO_have_hwrand(void) { return (OPENSSL_ia32cap_P[1] & (1u << 30)) != 0; } -/* CRYPTO_rdrand is defined in asm/rdrand-x86_64.pl */ -extern uint64_t CRYPTO_rdrand(void); +/* These functions are defined in asm/rdrand-x86_64.pl */ +extern int CRYPTO_rdrand(uint8_t out[8]); +extern int CRYPTO_rdrand_multiple8_buf(uint8_t *buf, size_t len); -void CRYPTO_hwrand(uint8_t *buf, size_t len) { - while (len >= 8) { - uint64_t rand = CRYPTO_rdrand(); - memcpy(buf, &rand, sizeof(rand)); - len -= sizeof(rand); - buf += sizeof(rand); +int CRYPTO_hwrand(uint8_t *buf, size_t len) { + const size_t len_multiple8 = len & ~7; + if (!CRYPTO_rdrand_multiple8_buf(buf, len_multiple8)) { + return 0; } + len -= len_multiple8; + + if (len != 0) { + assert(len < 8); - if (len > 0) { - uint64_t rand = CRYPTO_rdrand(); - memcpy(buf, &rand, len); + uint8_t rand_buf[8]; + if (!CRYPTO_rdrand(rand_buf)) { + return 0; + } + memcpy(buf + len_multiple8, rand_buf, len); } + + return 1; } #else diff --git a/src/crypto/rand/internal.h b/src/crypto/rand/internal.h index 1cca7f3..5e6ea11 100644 --- a/src/crypto/rand/internal.h +++ b/src/crypto/rand/internal.h @@ -29,8 +29,9 @@ void CRYPTO_sysrand(uint8_t *buf, size_t len); int CRYPTO_have_hwrand(void); /* CRYPTO_hwrand fills |len| bytes at |buf| with entropy from the hardware. - * This function can only be called if |CRYPTO_have_hwrand| returns one. */ -void CRYPTO_hwrand(uint8_t *buf, size_t len); + * This function can only be called if |CRYPTO_have_hwrand| returns one. + * It returns one on success or zero on hardware failure. */ +int CRYPTO_hwrand(uint8_t *buf, size_t len); #if defined(__cplusplus) diff --git a/src/crypto/rand/rand.c b/src/crypto/rand/rand.c index a647b6a..a96ac48 100644 --- a/src/crypto/rand/rand.c +++ b/src/crypto/rand/rand.c @@ -78,7 +78,8 @@ int RAND_bytes(uint8_t *buf, size_t len) { return 1; } - if (!CRYPTO_have_hwrand()) { + if (!CRYPTO_have_hwrand() || + !CRYPTO_hwrand(buf, len)) { /* Without a hardware RNG to save us from address-space duplication, the OS * entropy is used directly. */ CRYPTO_sysrand(buf, len); @@ -108,8 +109,6 @@ int RAND_bytes(uint8_t *buf, size_t len) { state->partial_block_used = sizeof(state->partial_block); } - CRYPTO_hwrand(buf, len); - if (len >= sizeof(state->partial_block)) { size_t remaining = len; while (remaining > 0) { -- cgit v1.1