From 77a942b7fedb58ae1afee042255b4b9267abebca Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 15 Jul 2014 01:22:50 -0400 Subject: [PATCH] Don't use the RSA key exchange with a signing-only key. This removes the last case where the server generates an RSA key for the ServerKeyExchange. Remove the code for this. Client support to accept them still remains. Leave the APIs for now, but they don't do anything anymore. Change-Id: I84439e034cc575719f5bc9b3e501165e12b62107 Reviewed-on: https://boringssl-review.googlesource.com/1286 Reviewed-by: Adam Langley --- ssl/d1_srvr.c | 7 +---- ssl/s3_lib.c | 72 +++++++------------------------------------------- ssl/s3_srvr.c | 44 ++++++------------------------ ssl/ssl_cert.c | 10 ------- ssl/ssl_lib.c | 5 ++-- ssl/ssl_locl.h | 2 -- 6 files changed, 21 insertions(+), 119 deletions(-) diff --git a/ssl/d1_srvr.c b/ssl/d1_srvr.c index 91b047a6d..3d30fcc13 100644 --- a/ssl/d1_srvr.c +++ b/ssl/d1_srvr.c @@ -163,7 +163,6 @@ int dtls1_accept(SSL *s) { BUF_MEM *buf; void (*cb)(const SSL *ssl,int type,int val)=NULL; - unsigned long alg_k; unsigned long alg_a; int ret= -1; int new_state,state,skip=0; @@ -375,23 +374,19 @@ int dtls1_accept(SSL *s) case SSL3_ST_SW_KEY_EXCH_A: case SSL3_ST_SW_KEY_EXCH_B: - alg_k = s->s3->tmp.new_cipher->algorithm_mkey; alg_a = s->s3->tmp.new_cipher->algorithm_auth; /* Send a ServerKeyExchange message if: * - The key exchange is ephemeral or anonymous * Diffie-Hellman. * - There is a PSK identity hint. - * - We have a signing-only RSA key. - * TODO(davidben): Remove this? * * TODO(davidben): This logic is currently duplicated * in s3_srvr.c. Fix this. In the meantime, keep them * in sync. */ if (ssl_cipher_requires_server_key_exchange(s->s3->tmp.new_cipher) || - ((alg_a & SSL_aPSK) && s->session->psk_identity_hint) || - ((alg_k & SSL_kRSA) && (s->cert->pkeys[SSL_PKEY_RSA_ENC].privatekey == NULL))) + ((alg_a & SSL_aPSK) && s->session->psk_identity_hint)) { dtls1_start_timer(s); ret=ssl3_send_server_key_exchange(s); diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index a30538224..721abb8d0 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -2478,29 +2478,12 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg) ret=(int)(s->s3->flags); break; case SSL_CTRL_NEED_TMP_RSA: - if ((s->cert != NULL) && (s->cert->rsa_tmp == NULL) && - ((s->cert->pkeys[SSL_PKEY_RSA_ENC].privatekey == NULL) || - (EVP_PKEY_size(s->cert->pkeys[SSL_PKEY_RSA_ENC].privatekey) > (512/8)))) - ret = 1; + /* Temporary RSA keys are never used. */ + ret = 0; break; case SSL_CTRL_SET_TMP_RSA: - { - RSA *rsa = (RSA *)parg; - if (rsa == NULL) - { - OPENSSL_PUT_ERROR(SSL, ssl3_ctrl, ERR_R_PASSED_NULL_PARAMETER); - return(ret); - } - if ((rsa = RSAPrivateKey_dup(rsa)) == NULL) - { - OPENSSL_PUT_ERROR(SSL, ssl3_ctrl, ERR_R_RSA_LIB); - return(ret); - } - if (s->cert->rsa_tmp != NULL) - RSA_free(s->cert->rsa_tmp); - s->cert->rsa_tmp = rsa; - ret = 1; - } + /* Temporary RSA keys are never used. */ + OPENSSL_PUT_ERROR(SSL, ssl3_ctrl, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); break; case SSL_CTRL_SET_TMP_RSA_CB: { @@ -2867,9 +2850,7 @@ long ssl3_callback_ctrl(SSL *s, int cmd, void (*fp)(void)) switch (cmd) { case SSL_CTRL_SET_TMP_RSA_CB: - { - s->cert->rsa_tmp_cb = (RSA *(*)(SSL *, int, int))fp; - } + /* Ignore the callback; temporary RSA keys are never used. */ break; #ifndef OPENSSL_NO_DH case SSL_CTRL_SET_TMP_DH_CB: @@ -2904,42 +2885,11 @@ long ssl3_ctx_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg) switch (cmd) { case SSL_CTRL_NEED_TMP_RSA: - if ( (cert->rsa_tmp == NULL) && - ((cert->pkeys[SSL_PKEY_RSA_ENC].privatekey == NULL) || - (EVP_PKEY_size(cert->pkeys[SSL_PKEY_RSA_ENC].privatekey) > (512/8))) - ) - return(1); - else - return(0); - /* break; */ + /* Temporary RSA keys are never used. */ + return 0; case SSL_CTRL_SET_TMP_RSA: - { - RSA *rsa; - int i; - - rsa=(RSA *)parg; - i=1; - if (rsa == NULL) - i=0; - else - { - if ((rsa=RSAPrivateKey_dup(rsa)) == NULL) - i=0; - } - if (!i) - { - OPENSSL_PUT_ERROR(SSL, ssl3_ctx_ctrl, ERR_R_RSA_LIB); - return(0); - } - else - { - if (cert->rsa_tmp != NULL) - RSA_free(cert->rsa_tmp); - cert->rsa_tmp=rsa; - return(1); - } - } - /* break; */ + OPENSSL_PUT_ERROR(SSL, ssl3_ctx_ctrl, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); + return 0; case SSL_CTRL_SET_TMP_RSA_CB: { OPENSSL_PUT_ERROR(SSL, ssl3_ctx_ctrl, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); @@ -3163,9 +3113,7 @@ long ssl3_ctx_callback_ctrl(SSL_CTX *ctx, int cmd, void (*fp)(void)) switch (cmd) { case SSL_CTRL_SET_TMP_RSA_CB: - { - cert->rsa_tmp_cb = (RSA *(*)(SSL *, int, int))fp; - } + /* Ignore the callback; temporary RSA keys are never used. */ break; #ifndef OPENSSL_NO_DH case SSL_CTRL_SET_TMP_DH_CB: diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c index 79ded6965..bc2f538ea 100644 --- a/ssl/s3_srvr.c +++ b/ssl/s3_srvr.c @@ -188,7 +188,6 @@ IMPLEMENT_ssl3_meth_func(SSLv3_server_method, int ssl3_accept(SSL *s) { BUF_MEM *buf; - unsigned long alg_k; unsigned long alg_a; void (*cb)(const SSL *ssl,int type,int val)=NULL; int ret= -1; @@ -369,23 +368,19 @@ int ssl3_accept(SSL *s) case SSL3_ST_SW_KEY_EXCH_A: case SSL3_ST_SW_KEY_EXCH_B: - alg_k = s->s3->tmp.new_cipher->algorithm_mkey; alg_a = s->s3->tmp.new_cipher->algorithm_auth; /* Send a ServerKeyExchange message if: * - The key exchange is ephemeral or anonymous * Diffie-Hellman. * - There is a PSK identity hint. - * - We have a signing-only RSA key. - * TODO(davidben): Remove this? * * TODO(davidben): This logic is currently duplicated * in d1_srvr.c. Fix this. In the meantime, keep them * in sync. */ if (ssl_cipher_requires_server_key_exchange(s->s3->tmp.new_cipher) || - ((alg_a & SSL_aPSK) && s->session->psk_identity_hint) || - ((alg_k & SSL_kRSA) && (s->cert->pkeys[SSL_PKEY_RSA_ENC].privatekey == NULL))) + ((alg_a & SSL_aPSK) && s->session->psk_identity_hint)) { ret=ssl3_send_server_key_exchange(s); if (ret <= 0) goto end; @@ -1399,7 +1394,6 @@ int ssl3_send_server_key_exchange(SSL *s) { unsigned char *q; int j,num; - RSA *rsa; unsigned char md_buf[MD5_DIGEST_LENGTH+SHA_DIGEST_LENGTH]; unsigned int u; #ifndef OPENSSL_NO_DH @@ -1412,8 +1406,8 @@ int ssl3_send_server_key_exchange(SSL *s) int curve_id = 0; BN_CTX *bn_ctx = NULL; #endif - const char* psk_identity_hint; - size_t psk_identity_hint_len; + const char* psk_identity_hint = NULL; + size_t psk_identity_hint_len = 0; EVP_PKEY *pkey; const EVP_MD *md = NULL; unsigned char *p,*d; @@ -1448,32 +1442,8 @@ int ssl3_send_server_key_exchange(SSL *s) psk_identity_hint_len = 0; n+=2+psk_identity_hint_len; } - if (alg_k & SSL_kRSA) - { - rsa=cert->rsa_tmp; - if ((rsa == NULL) && (s->cert->rsa_tmp_cb != NULL)) - { - rsa = s->cert->rsa_tmp_cb(s, 0, 1024); - if(rsa == NULL) - { - al=SSL_AD_HANDSHAKE_FAILURE; - OPENSSL_PUT_ERROR(SSL, ssl3_send_server_key_exchange, SSL_R_ERROR_GENERATING_TMP_RSA_KEY); - goto f_err; - } - RSA_up_ref(rsa); - cert->rsa_tmp=rsa; - } - if (rsa == NULL) - { - al=SSL_AD_HANDSHAKE_FAILURE; - OPENSSL_PUT_ERROR(SSL, ssl3_send_server_key_exchange, SSL_R_MISSING_TMP_RSA_KEY); - goto f_err; - } - r[0]=rsa->n; - r[1]=rsa->e; - } #ifndef OPENSSL_NO_DH - else if (alg_k & SSL_kEDH) + if (alg_k & SSL_kEDH) { dhp=cert->dh_tmp; if ((dhp == NULL) && (s->cert->dh_tmp_cb != NULL)) @@ -1523,9 +1493,10 @@ int ssl3_send_server_key_exchange(SSL *s) r[1]=dh->g; r[2]=dh->pub_key; } + else #endif #ifndef OPENSSL_NO_ECDH - else if (alg_k & SSL_kEECDH) + if (alg_k & SSL_kEECDH) { const EC_GROUP *group; @@ -1648,8 +1619,9 @@ int ssl3_send_server_key_exchange(SSL *s) r[2]=NULL; r[3]=NULL; } + else #endif /* !OPENSSL_NO_ECDH */ - else if (!(alg_k & SSL_kPSK)) + if (!(alg_k & SSL_kPSK)) { al=SSL_AD_HANDSHAKE_FAILURE; OPENSSL_PUT_ERROR(SSL, ssl3_send_server_key_exchange, SSL_R_UNKNOWN_KEY_EXCHANGE_TYPE); diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index 7c1227ca7..664db9d53 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -208,13 +208,6 @@ CERT *ssl_cert_dup(CERT *cert) ret->mask_k = cert->mask_k; ret->mask_a = cert->mask_a; - if (cert->rsa_tmp != NULL) - { - RSA_up_ref(cert->rsa_tmp); - ret->rsa_tmp = cert->rsa_tmp; - } - ret->rsa_tmp_cb = cert->rsa_tmp_cb; - #ifndef OPENSSL_NO_DH if (cert->dh_tmp != NULL) { @@ -389,8 +382,6 @@ CERT *ssl_cert_dup(CERT *cert) #if !defined(OPENSSL_NO_DH) || !defined(OPENSSL_NO_ECDH) err: #endif - if (ret->rsa_tmp != NULL) - RSA_free(ret->rsa_tmp); #ifndef OPENSSL_NO_DH if (ret->dh_tmp != NULL) DH_free(ret->dh_tmp); @@ -440,7 +431,6 @@ void ssl_cert_free(CERT *c) if(c == NULL) return; - if (c->rsa_tmp) RSA_free(c->rsa_tmp); #ifndef OPENSSL_NO_DH if (c->dh_tmp) DH_free(c->dh_tmp); #endif diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 93e2c2c84..a36007022 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -2209,7 +2209,7 @@ void SSL_set_cert_cb(SSL *s, int (*cb)(SSL *ssl, void *arg), void *arg) void ssl_set_cert_masks(CERT *c, const SSL_CIPHER *cipher) { CERT_PKEY *cpk; - int rsa_enc,rsa_tmp,rsa_sign,dh_tmp,dh_rsa,dh_dsa,dsa_sign; + int rsa_enc,rsa_sign,dh_tmp,dh_rsa,dh_dsa,dsa_sign; unsigned long mask_k,mask_a; #ifndef OPENSSL_NO_ECDSA int have_ecc_cert, ecdsa_ok; @@ -2224,7 +2224,6 @@ void ssl_set_cert_masks(CERT *c, const SSL_CIPHER *cipher) #endif if (c == NULL) return; - rsa_tmp=(c->rsa_tmp != NULL || c->rsa_tmp_cb != NULL); #ifndef OPENSSL_NO_DH dh_tmp=(c->dh_tmp != NULL || c->dh_tmp_cb != NULL); #else @@ -2258,7 +2257,7 @@ void ssl_set_cert_masks(CERT *c, const SSL_CIPHER *cipher) rsa_enc,rsa_enc_export,rsa_sign,dsa_sign,dh_rsa,dh_dsa); #endif - if (rsa_enc || (rsa_tmp && rsa_sign)) + if (rsa_enc) mask_k|=SSL_kRSA; #if 0 diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 2024a6b60..57d63c8c5 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -502,8 +502,6 @@ typedef struct cert_st unsigned long mask_a; /* Client only */ unsigned long mask_ssl; - RSA *rsa_tmp; - RSA *(*rsa_tmp_cb)(SSL *ssl,int is_export,int keysize); #ifndef OPENSSL_NO_DH DH *dh_tmp; DH *(*dh_tmp_cb)(SSL *ssl,int is_export,int keysize);