OERV-RUNTIME/boringssl
2
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Be strict about expecting a server Certificate message.

Browse Source 
Introduce a ssl_cipher_has_server_public_key to save the repeated
NULL/PSK/RSA_PSK[*] check. Don't allow skipping to ServerKeyExchange when
expecting Certificate; the messages expected are determined by the cipher
suite. The ssl3_get_server_public_key call is already guarded.

As the previous test demonstrates, this is safe because of the
ssl3_check_cert_and_algorithm call, but avoid the looseness in the parsing
there.

[*] NB: we don't implement RSA_PSK, and OpenSSL has never implemented it.

Change-Id: I0571e6bcbeb8eb883f77878bdc98d1aa3a287cf3
Reviewed-on: https://boringssl-review.googlesource.com/1156
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
David Benjamin
2014-07-14 20:50:30 +00:00
committed by Adam Langley
parent 1c375dda8a
commit e8f3d666cc
5 changed files with 28 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ssl/s3_clnt.c
+7 -25
View File
@@ -313,11 +313,7 @@ int ssl3_connect(SSL *s)
s->init_num=0;
break;
}
/* Check if it is anon DH/ECDH */
/* or non-RSA PSK */
if (!(s->s3->tmp.new_cipher->algorithm_auth & SSL_aNULL) &&
!((s->s3->tmp.new_cipher->algorithm_auth & SSL_aPSK) &&
!(s->s3->tmp.new_cipher->algorithm_mkey & SSL_kRSA)))
if (ssl_cipher_has_server_public_key(s->s3->tmp.new_cipher))
{
ret=ssl3_get_server_certificate(s);
if (ret <= 0) goto end;
@@ -1118,24 +1114,12 @@ int ssl3_get_server_certificate(SSL *s)
n=s->method->ssl_get_message(s,
SSL3_ST_CR_CERT_A,
SSL3_ST_CR_CERT_B,
-1,
SSL3_MT_CERTIFICATE,
s->max_cert_list,
&ok);
if (!ok) return((int)n);
if (s->s3->tmp.message_type == SSL3_MT_SERVER_KEY_EXCHANGE)
{
s->s3->tmp.reuse_message=1;
return(1);
}
if (s->s3->tmp.message_type != SSL3_MT_CERTIFICATE)
{
al=SSL_AD_UNEXPECTED_MESSAGE;
OPENSSL_PUT_ERROR(SSL, ssl3_get_server_certificate, SSL_R_BAD_MESSAGE_TYPE);
goto f_err;
}
CBS_init(&cbs, (uint8_t *)s->init_msg, n);
if ((sk=sk_X509_new_null()) == NULL)
@@ -1732,9 +1716,7 @@ fprintf(stderr, "USING TLSv1.2 HASH %s\n", EVP_MD_name(md));
}
else
{
if (!(alg_a & SSL_aNULL) &&
/* Among PSK ciphers only RSA_PSK needs a public key */
!((alg_a & SSL_aPSK) && !(alg_k & SSL_kRSA)))
if (ssl_cipher_has_server_public_key(s->s3->tmp.new_cipher))
{
/* Might be wrong key type, check it */
if (ssl3_check_cert_and_algorithm(s))
@@ -2916,13 +2898,13 @@ int ssl3_check_cert_and_algorithm(SSL *s)
DH *dh;
#endif
/* we don't have a certificate */
if (!ssl_cipher_has_server_public_key(s->s3->tmp.new_cipher))
return 1;
alg_k=s->s3->tmp.new_cipher->algorithm_mkey;
alg_a=s->s3->tmp.new_cipher->algorithm_auth;
/* we don't have a certificate */
if ((alg_a & SSL_aNULL) || ((alg_a & SSL_aPSK) && !(alg_k & SSL_kRSA)))
return(1);
sc=s->session->sess_cert;
if (sc == NULL)
{
ssl/s3_srvr.c
+2 -9
View File
@@ -351,12 +351,7 @@ int ssl3_accept(SSL *s)
case SSL3_ST_SW_CERT_A:
case SSL3_ST_SW_CERT_B:
/* Check if it is anon DH or anon ECDH, */
/* non-RSA PSK or SRP */
if (!(s->s3->tmp.new_cipher->algorithm_auth & SSL_aNULL)
/* Among PSK ciphersuites only RSA_PSK uses server certificate */
&& !(s->s3->tmp.new_cipher->algorithm_auth & SSL_aPSK &&
!(s->s3->tmp.new_cipher->algorithm_mkey & SSL_kRSA)))
if (ssl_cipher_has_server_public_key(s->s3->tmp.new_cipher))
{
ret=ssl3_send_server_certificate(s);
if (ret <= 0) goto end;
@@ -1776,9 +1771,7 @@ int ssl3_send_server_key_exchange(SSL *s)
n+=2+nr[i];
}
if (!(alg_a & SSL_aNULL)
/* Among PSK ciphersuites only RSA uses a certificate */
&& !((alg_a & SSL_aPSK) && !(alg_k & SSL_kRSA)))
if (ssl_cipher_has_server_public_key(s->s3->tmp.new_cipher))
{
if ((pkey=ssl_get_sign_pkey(s,s->s3->tmp.new_cipher,&md))
== NULL)
ssl/ssl_ciph.c
+16
View File
@@ -1867,3 +1867,19 @@ const SSL_CIPHER *SSL_CIPHER_find(SSL *ssl, const unsigned char *ptr)
{
return ssl->method->get_cipher_by_char(ptr);
}
/* ssl_cipher_has_server_public_key returns 1 if |cipher| involves a
* server public key in the key exchange, sent in a server Certificate
* message. Otherwise it returns 0. */
int ssl_cipher_has_server_public_key(const SSL_CIPHER *cipher)
{
/* Anonymous ciphers do not include a server certificate. */
if (cipher->algorithm_auth & SSL_aNULL)
return 0;
/* Neither do PSK ciphers, except for RSA_PSK. */
if ((cipher->algorithm_auth & SSL_aPSK) &&
!(cipher->algorithm_mkey & SSL_kRSA))
return 0;
/* All other ciphers include it. */
return 1;
}
ssl/ssl_locl.h
+2
View File
@@ -993,6 +993,8 @@ int ssl_get_handshake_digest(int i,long *mask,const EVP_MD **md);
int ssl_get_handshake_digest(int i,long *mask,const EVP_MD **md);
int ssl_cipher_get_cert_index(const SSL_CIPHER *c);
const SSL_CIPHER *ssl_get_cipher_by_char(SSL *ssl, const unsigned char *ptr);
int ssl_cipher_has_server_public_key(const SSL_CIPHER *cipher);
int ssl_cert_set0_chain(CERT *c, STACK_OF(X509) *chain);
int ssl_cert_set1_chain(CERT *c, STACK_OF(X509) *chain);
int ssl_cert_add0_chain_cert(CERT *c, X509 *x);
ssl/test/runner/runner.go
+1 -1
View File
@@ -198,7 +198,7 @@ var testCases = []testCase{
},
},
shouldFail: true,
expectedError: ":MISSING_RSA_SIGNING_CERT:",
expectedError: ":UNEXPECTED_MESSAGE:",
},
}