From: jsing Date: Tue, 25 Jun 2024 14:10:45 +0000 (+0000) Subject: Implement RSA key exchange in constant time. X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=b9e57b4b81ec7fe86a6dbf7b3b1bd36cdc3b0190;p=openbsd Implement RSA key exchange in constant time. RSA key exchange is known to have multiple security weaknesses, including being potentially susceptible to padding oracle and timing attacks. The RSA key exchange code that we inherited from OpenSSL was riddled with timing leaks, many of which we fixed (or minimised) early on. However, a number of issues still remained, particularly those related to libcrypto's RSA decryption and padding checks. Rework the RSA key exchange code such that we decrypt with RSA_NO_PADDING and then check the padding ourselves in constant time. In this case, the pre-master secret is of a known length, hence the padding is also a known length based on the size of the RSA key. This makes it easy to implement a check that is much safer than having RSA_private_decrypt() depad for us. Regardless, we still strongly recommend disabling RSA key exchange and using other key exchange methods that provide perfect forward secrecy and do not depend on client generated keys. Thanks to Marcel Maehren, Nurullah Erinola, Robert Merget, Juraj Somorovsky, Joerg Schwenk and Hubert Kario for raising these issues with us at various points in time. ok tb@ --- diff --git a/lib/libssl/Makefile b/lib/libssl/Makefile index 38e5ba30e04..a2b710922d7 100644 --- a/lib/libssl/Makefile +++ b/lib/libssl/Makefile @@ -1,4 +1,4 @@ -# $OpenBSD: Makefile,v 1.81 2023/11/22 15:55:28 tb Exp $ +# $OpenBSD: Makefile,v 1.82 2024/06/25 14:10:45 jsing Exp $ .include .ifndef NOMAN @@ -23,6 +23,7 @@ CFLAGS+= -DLIBRESSL_NAMESPACE CFLAGS+= -DTLS13_DEBUG .endif CFLAGS+= -I${.CURDIR} +CFLAGS+= -I${.CURDIR}/../libcrypto CFLAGS+= -I${.CURDIR}/../libcrypto/hidden CFLAGS+= -I${.CURDIR}/../libcrypto/bio CFLAGS+= -I${.CURDIR}/hidden diff --git a/lib/libssl/ssl_local.h b/lib/libssl/ssl_local.h index eeef569fa93..db102212a86 100644 --- a/lib/libssl/ssl_local.h +++ b/lib/libssl/ssl_local.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_local.h,v 1.16 2024/05/19 07:12:50 jsg Exp $ */ +/* $OpenBSD: ssl_local.h,v 1.17 2024/06/25 14:10:45 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -167,8 +167,10 @@ __BEGIN_HIDDEN_DECLS +#ifndef CTASSERT #define CTASSERT(x) extern char _ctassert[(x) ? 1 : -1 ] \ __attribute__((__unused__)) +#endif #ifndef LIBRESSL_HAS_DTLS1_2 #define LIBRESSL_HAS_DTLS1_2 diff --git a/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c index 6d61a4e4fa2..e9f14dc6107 100644 --- a/lib/libssl/ssl_srvr.c +++ b/lib/libssl/ssl_srvr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_srvr.c,v 1.160 2024/02/03 17:39:17 tb Exp $ */ +/* $OpenBSD: ssl_srvr.c,v 1.161 2024/06/25 14:10:45 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -163,6 +163,7 @@ #include #include "bytestring.h" +#include "crypto_internal.h" #include "dtls_local.h" #include "ssl_local.h" #include "ssl_sigalgs.h" @@ -1621,98 +1622,104 @@ ssl3_send_certificate_request(SSL *s) static int ssl3_get_client_kex_rsa(SSL *s, CBS *cbs) { - unsigned char fakekey[SSL_MAX_MASTER_KEY_LENGTH]; - unsigned char *pms = NULL; - unsigned char *p; + uint8_t fakepms[SSL_MAX_MASTER_KEY_LENGTH]; + uint8_t *pms = NULL; size_t pms_len = 0; + size_t pad_len; EVP_PKEY *pkey = NULL; RSA *rsa = NULL; CBS enc_pms; int decrypt_len; - int al = -1; + uint8_t mask; + size_t i; + int valid = 1; + int ret = 0; + + /* + * Handle key exchange in the form of an RSA-Encrypted Premaster Secret + * Message. See RFC 5246, section 7.4.7.1. + */ - arc4random_buf(fakekey, sizeof(fakekey)); + arc4random_buf(fakepms, sizeof(fakepms)); - fakekey[0] = s->s3->hs.peer_legacy_version >> 8; - fakekey[1] = s->s3->hs.peer_legacy_version & 0xff; + fakepms[0] = s->s3->hs.peer_legacy_version >> 8; + fakepms[1] = s->s3->hs.peer_legacy_version & 0xff; pkey = s->cert->pkeys[SSL_PKEY_RSA].privatekey; if (pkey == NULL || (rsa = EVP_PKEY_get0_RSA(pkey)) == NULL) { - al = SSL_AD_HANDSHAKE_FAILURE; SSLerror(s, SSL_R_MISSING_RSA_CERTIFICATE); - goto fatal_err; + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE); + goto err; } + /* + * The minimum size of an encrypted premaster secret is 11 bytes of + * padding (00 02 <8 or more non-zero bytes> 00) (RFC 8017, section + * 9.2) and 48 bytes of premaster secret (RFC 5246, section 7.4.7.1). + * This means an RSA key size of at least 472 bits. + */ pms_len = RSA_size(rsa); - if (pms_len < SSL_MAX_MASTER_KEY_LENGTH) - goto err; - if ((pms = malloc(pms_len)) == NULL) + if (pms_len < 11 + SSL_MAX_MASTER_KEY_LENGTH) { + SSLerror(s, SSL_R_DECRYPTION_FAILED); + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECRYPT_ERROR); goto err; - p = pms; + } + pad_len = pms_len - SSL_MAX_MASTER_KEY_LENGTH; - if (!CBS_get_u16_length_prefixed(cbs, &enc_pms)) - goto decode_err; - if (CBS_len(cbs) != 0 || CBS_len(&enc_pms) != RSA_size(rsa)) { + if (!CBS_get_u16_length_prefixed(cbs, &enc_pms)) { + SSLerror(s, SSL_R_BAD_PACKET_LENGTH); + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); + goto err; + } + if (CBS_len(&enc_pms) != pms_len || CBS_len(cbs) != 0) { SSLerror(s, SSL_R_TLS_RSA_ENCRYPTED_VALUE_LENGTH_IS_WRONG); + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); goto err; } - decrypt_len = RSA_private_decrypt(CBS_len(&enc_pms), CBS_data(&enc_pms), - pms, rsa, RSA_PKCS1_PADDING); + if ((pms = calloc(1, pms_len)) == NULL) + goto err; - ERR_clear_error(); + decrypt_len = RSA_private_decrypt(CBS_len(&enc_pms), CBS_data(&enc_pms), + pms, rsa, RSA_NO_PADDING); - if (decrypt_len != SSL_MAX_MASTER_KEY_LENGTH) { - al = SSL_AD_DECODE_ERROR; - /* SSLerror(s, SSL_R_BAD_RSA_DECRYPT); */ + if (decrypt_len != pms_len) { + SSLerror(s, SSL_R_DECRYPTION_FAILED); + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECRYPT_ERROR); + goto err; } - if ((al == -1) && !((pms[0] == (s->s3->hs.peer_legacy_version >> 8)) && - (pms[1] == (s->s3->hs.peer_legacy_version & 0xff)))) { - /* - * The premaster secret must contain the same version number - * as the ClientHello to detect version rollback attacks - * (strangely, the protocol does not offer such protection for - * DH ciphersuites). - * - * The Klima-Pokorny-Rosa extension of Bleichenbacher's attack - * (http://eprint.iacr.org/2003/052/) exploits the version - * number check as a "bad version oracle" -- an alert would - * reveal that the plaintext corresponding to some ciphertext - * made up by the adversary is properly formatted except that - * the version number is wrong. To avoid such attacks, we should - * treat this just like any other decryption error. - */ - al = SSL_AD_DECODE_ERROR; - /* SSLerror(s, SSL_R_BAD_PROTOCOL_VERSION_NUMBER); */ - } + /* + * All processing from here on needs to avoid leaking any information + * about the decrypted content, in order to prevent oracle attacks and + * minimise timing attacks. + */ - if (al != -1) { - /* - * Some decryption failure -- use random value instead - * as countermeasure against Bleichenbacher's attack - * on PKCS #1 v1.5 RSA padding (see RFC 2246, - * section 7.4.7.1). - */ - p = fakekey; - } + /* Check padding - 00 02 <8 or more non-zero bytes> 00 */ + valid &= crypto_ct_eq_u8(pms[0], 0x00); + valid &= crypto_ct_eq_u8(pms[1], 0x02); + for (i = 2; i < pad_len - 1; i++) + valid &= crypto_ct_ne_u8(pms[i], 0x00); + valid &= crypto_ct_eq_u8(pms[pad_len - 1], 0x00); - if (!tls12_derive_master_secret(s, p, SSL_MAX_MASTER_KEY_LENGTH)) - goto err; + /* Ensure client version in premaster secret matches ClientHello version. */ + valid &= crypto_ct_eq_u8(pms[pad_len + 0], s->s3->hs.peer_legacy_version >> 8); + valid &= crypto_ct_eq_u8(pms[pad_len + 1], s->s3->hs.peer_legacy_version & 0xff); - freezero(pms, pms_len); + /* Use the premaster secret if padding is correct, if not use the fake. */ + mask = crypto_ct_eq_mask_u8(valid, 1); + for (i = 0; i < SSL_MAX_MASTER_KEY_LENGTH; i++) + pms[i] = (pms[pad_len + i] & mask) | (fakepms[i] & ~mask); - return 1; + if (!tls12_derive_master_secret(s, pms, SSL_MAX_MASTER_KEY_LENGTH)) + goto err; + + ret = 1; - decode_err: - al = SSL_AD_DECODE_ERROR; - SSLerror(s, SSL_R_BAD_PACKET_LENGTH); - fatal_err: - ssl3_send_alert(s, SSL3_AL_FATAL, al); err: freezero(pms, pms_len); - return 0; + return ret; } static int