Remove SSL_OP_EPHEMERAL_RSA quirk.

Also fix a place where fixes for the condition for sending ServerKeyExchange in s3_srvr.c were never propogated to d1_srvr.c. Tidy up that logic to use ssl_cipher_requires_server_key_exchange and simplify the PSK check. Change-Id: Ie36d378f733e59a8df405bc869f2346af59bd574 Reviewed-on: https://boringssl-review.googlesource.com/1283 Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:11:50 +00:00
parent ff175b4a46
commit b9cc33a4d6
5 changed files with 37 additions and 84 deletions
@@ -547,9 +547,6 @@ struct ssl_session_st
 #define SSL_OP_SINGLE_DH_USE				0x00100000L
 /* Set to always use the tmp_rsa key when doing RSA operations,
 * even when this violates protocol specs */
-#define SSL_OP_EPHEMERAL_RSA				0x00200000L
-/* Set on servers to choose the cipher according to the server's
- * preferences */
 #define SSL_OP_CIPHER_SERVER_PREFERENCE			0x00400000L
 /* If set, a server will allow a client to issue a SSLv3.0 version number
 * as latest version supported in the premaster secret, even when TLSv1.0
@@ -475,8 +475,6 @@ typedef struct ssl3_state_st
 		unsigned char *certificate_types;
 		size_t num_certificate_types;

-		int use_rsa_tmp;
-
 		int key_block_length;
 		unsigned char *key_block;

@@ -164,6 +164,7 @@ 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;
 	int listen;
@@ -375,28 +376,22 @@ 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;

-			/* clear this, it may get reset by
-			 * send_server_key_exchange */
-			if ((s->options & SSL_OP_EPHEMERAL_RSA)
-				)
-				/* option SSL_OP_EPHEMERAL_RSA sends temporary RSA key
-				 * even when forbidden by protocol specs
-				 * (handshake may fail as clients are not required to
-				 * be able to handle this) */
-				s->s3->tmp.use_rsa_tmp=1;
-			else
-				s->s3->tmp.use_rsa_tmp=0;
-
-			/* only send if a DH key exchange or
-			 * RSA but we have a sign only certificate */
-			if (s->s3->tmp.use_rsa_tmp
-			/* PSK: send ServerKeyExchange if PSK identity
-			 * hint if provided */
-			    || ((alg_k & SSL_kPSK) && s->session->psk_identity_hint)
-			    || (alg_k & (SSL_kEDH|SSL_kDHr|SSL_kDHd))
-			    || (alg_k & SSL_kEECDH)
-			    || ((alg_k & SSL_kRSA)
+			/* 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
 				    || (SSL_C_IS_EXPORT(s->s3->tmp.new_cipher)
 					&& EVP_PKEY_size(s->cert->pkeys[SSL_PKEY_RSA_ENC].privatekey)*8 > SSL_C_EXPORT_PKEYLENGTH(s->s3->tmp.new_cipher)
@@ -374,38 +374,20 @@ int ssl3_accept(SSL *s)
 			alg_k = s->s3->tmp.new_cipher->algorithm_mkey;
 			alg_a = s->s3->tmp.new_cipher->algorithm_auth;

-			/* clear this, it may get reset by
-			 * send_server_key_exchange */
-			if ((s->options & SSL_OP_EPHEMERAL_RSA)
-				)
-				/* option SSL_OP_EPHEMERAL_RSA sends temporary RSA key
-				 * even when forbidden by protocol specs
-				 * (handshake may fail as clients are not required to
-				 * be able to handle this) */
-				s->s3->tmp.use_rsa_tmp=1;
-			else
-				s->s3->tmp.use_rsa_tmp=0;
-
-
-			/* only send if a DH key exchange, fortezza or
-			 * RSA but we have a sign only certificate
+			/* 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?
 			 *
-			 * PSK: may send PSK identity hints
-			 *
-			 * For ECC ciphersuites, we send a serverKeyExchange
-			 * message only if the cipher suite is either
-			 * ECDH-anon or ECDHE. In other cases, the
-			 * server certificate contains the server's
-			 * public key for key exchange.
+			 * TODO(davidben): This logic is currently duplicated
+			 * in d1_srvr.c. Fix this. In the meantime, keep them
+			 * in sync.
 			 */
-			if (s->s3->tmp.use_rsa_tmp
-			/* PSK: send ServerKeyExchange if either:
-			 *   - PSK identity hint is provided, or
-			 *   - the key exchange is kEECDH. */
-			    || ((alg_a & SSL_aPSK) && ((alg_k & SSL_kEECDH) || s->session->psk_identity_hint))
-			    || (alg_k & SSL_kEDH)
-			    || (alg_k & SSL_kEECDH)
-			    || ((alg_k & SSL_kRSA)
+			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
 				    || (SSL_C_IS_EXPORT(s->s3->tmp.new_cipher)
 					&& EVP_PKEY_size(s->cert->pkeys[SSL_PKEY_RSA_ENC].privatekey)*8 > SSL_C_EXPORT_PKEYLENGTH(s->s3->tmp.new_cipher)
@@ -1528,7 +1510,6 @@ int ssl3_send_server_key_exchange(SSL *s)
 				}
 			r[0]=rsa->n;
 			r[1]=rsa->e;
-			s->s3->tmp.use_rsa_tmp=1;
 			}
 #ifndef OPENSSL_NO_DH
 		else if (alg_k & SSL_kEDH)
@@ -2083,34 +2064,16 @@ int ssl3_get_client_key_exchange(SSL *s)
 		unsigned char version_good;
 		size_t j;

-		/* FIX THIS UP EAY EAY EAY EAY */
-		if (s->s3->tmp.use_rsa_tmp)
+		pkey=s->cert->pkeys[SSL_PKEY_RSA_ENC].privatekey;
+		if (	(pkey == NULL) ||
+			(pkey->type != EVP_PKEY_RSA) ||
+			(pkey->pkey.rsa == NULL))
 			{
-			if ((s->cert != NULL) && (s->cert->rsa_tmp != NULL))
-				rsa=s->cert->rsa_tmp;
-			/* Don't do a callback because rsa_tmp should
-			 * be sent already */
-			if (rsa == NULL)
-				{
-				al=SSL_AD_HANDSHAKE_FAILURE;
-				OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, SSL_R_MISSING_TMP_RSA_PKEY);
-				goto f_err;
-
-				}
-			}
-		else
-			{
-			pkey=s->cert->pkeys[SSL_PKEY_RSA_ENC].privatekey;
-			if (	(pkey == NULL) ||
-				(pkey->type != EVP_PKEY_RSA) ||
-				(pkey->pkey.rsa == NULL))
-				{
-				al=SSL_AD_HANDSHAKE_FAILURE;
-				OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, SSL_R_MISSING_RSA_CERTIFICATE);
-				goto f_err;
-				}
-			rsa=pkey->pkey.rsa;
+			al=SSL_AD_HANDSHAKE_FAILURE;
+			OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, SSL_R_MISSING_RSA_CERTIFICATE);
+			goto f_err;
 			}
+		rsa=pkey->pkey.rsa;

 		/* TLS and [incidentally] DTLS{0xFEFF}
 		 *
@@ -1842,7 +1842,7 @@ int ssl_cipher_has_server_public_key(const SSL_CIPHER *cipher)
 * TODO(davidben): Can we remove the RSA one? This is a remnant of
 * RSA_EXPORT ciphers which required this (it was used to generate an
 * ephemeral 512-bit RSA encryption key), but it's allowed for all RSA
- * ciphers. There's even a SSL_OP_EPHEMERAL_RSA to always use it. */
+ * ciphers. */
 int ssl_cipher_requires_server_key_exchange(const SSL_CIPHER *cipher)
 	{
 	/* Ephemeral Diffie-Hellman key exchanges require a