[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Wed Apr 26 13:17:24 UTC 2017
The branch master has been updated
via bf846a6d47a0f94b9771ead5ce52786045e58f49 (commit)
from d91b7423af79447df90cc0245b6944fce93302d1 (commit)
- Log -----------------------------------------------------------------
commit bf846a6d47a0f94b9771ead5ce52786045e58f49
Author: Matt Caswell <matt at openssl.org>
Date: Wed Apr 26 09:08:00 2017 +0100
Don't overwrite the alert value if there is no alert to send
The function tls_early_post_process_client_hello() was overwriting the
passed "al" parameter even if it was successful. The caller of that
function, tls_post_process_client_hello(), sets "al" to a sensible default
(HANDSHAKE_FAILURE), but this was being overwritten to be INTERNAL_ERROR.
The result is a "no shared cipher" error (and probably other similar errors)
were being reported back to the client with an incorrect INTERNAL_ERROR
alert.
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3314)
-----------------------------------------------------------------------
Summary of changes:
ssl/statem/statem_srvr.c | 42 +++++++++++++++++++++---------------------
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 919469f..d751502 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1444,10 +1444,10 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
return MSG_PROCESS_ERROR;
}
-static int tls_early_post_process_client_hello(SSL *s, int *al)
+static int tls_early_post_process_client_hello(SSL *s, int *pal)
{
unsigned int j;
- int i;
+ int i, al = SSL_AD_INTERNAL_ERROR;
int protverr;
size_t loop;
unsigned long id;
@@ -1460,13 +1460,12 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
CLIENTHELLO_MSG *clienthello = s->clienthello;
DOWNGRADE dgrd = DOWNGRADE_NONE;
- *al = SSL_AD_INTERNAL_ERROR;
/* Finished parsing the ClientHello, now we can start processing it */
/* Give the early callback a crack at things */
if (s->ctx->early_cb != NULL) {
int code;
/* A failure in the early callback terminates the connection. */
- code = s->ctx->early_cb(s, al, s->ctx->early_cb_arg);
+ code = s->ctx->early_cb(s, &al, s->ctx->early_cb_arg);
if (code == 0)
goto err;
if (code < 0) {
@@ -1513,13 +1512,13 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
/* like ssl3_get_record, send alert using remote version number */
s->version = s->client_version = clienthello->legacy_version;
}
- *al = SSL_AD_PROTOCOL_VERSION;
+ al = SSL_AD_PROTOCOL_VERSION;
goto err;
}
/* TLSv1.3 specifies that a ClientHello must end on a record boundary */
if (SSL_IS_TLS13(s) && RECORD_LAYER_processed_read_pending(&s->rlayer)) {
- *al = SSL_AD_UNEXPECTED_MESSAGE;
+ al = SSL_AD_UNEXPECTED_MESSAGE;
SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO,
SSL_R_NOT_ON_RECORD_BOUNDARY);
goto err;
@@ -1531,7 +1530,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
if (s->ctx->app_verify_cookie_cb != NULL) {
if (s->ctx->app_verify_cookie_cb(s, clienthello->dtls_cookie,
clienthello->dtls_cookie_len) == 0) {
- *al = SSL_AD_HANDSHAKE_FAILURE;
+ al = SSL_AD_HANDSHAKE_FAILURE;
SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO,
SSL_R_COOKIE_MISMATCH);
goto err;
@@ -1541,7 +1540,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
} else if (s->d1->cookie_len != clienthello->dtls_cookie_len
|| memcmp(clienthello->dtls_cookie, s->d1->cookie,
s->d1->cookie_len) != 0) {
- *al = SSL_AD_HANDSHAKE_FAILURE;
+ al = SSL_AD_HANDSHAKE_FAILURE;
SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, SSL_R_COOKIE_MISMATCH);
goto err;
}
@@ -1552,7 +1551,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
if (protverr != 0) {
SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, protverr);
s->version = s->client_version;
- *al = SSL_AD_PROTOCOL_VERSION;
+ al = SSL_AD_PROTOCOL_VERSION;
goto err;
}
}
@@ -1563,7 +1562,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
/* We need to do this before getting the session */
if (!tls_parse_extension(s, TLSEXT_IDX_extended_master_secret,
SSL_EXT_CLIENT_HELLO,
- clienthello->pre_proc_exts, NULL, 0, al)) {
+ clienthello->pre_proc_exts, NULL, 0, &al)) {
SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, SSL_R_CLIENTHELLO_TLSEXT);
goto err;
}
@@ -1590,7 +1589,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
if (!ssl_get_new_session(s, 1))
goto err;
} else {
- i = ssl_get_prev_session(s, clienthello, al);
+ i = ssl_get_prev_session(s, clienthello, &al);
if (i == 1) {
/* previous session */
s->hit = 1;
@@ -1604,9 +1603,9 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
}
if (!ssl_cache_cipherlist(s, &clienthello->ciphersuites,
- clienthello->isv2, al) ||
+ clienthello->isv2, &al) ||
!bytes_to_cipher_list(s, &clienthello->ciphersuites, &ciphers, &scsvs,
- clienthello->isv2, al)) {
+ clienthello->isv2, &al)) {
goto err;
}
@@ -1620,7 +1619,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
/* SCSV is fatal if renegotiating */
SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO,
SSL_R_SCSV_RECEIVED_WHEN_RENEGOTIATING);
- *al = SSL_AD_HANDSHAKE_FAILURE;
+ al = SSL_AD_HANDSHAKE_FAILURE;
goto err;
}
s->s3->send_connection_binding = 1;
@@ -1635,7 +1634,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
*/
SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO,
SSL_R_INAPPROPRIATE_FALLBACK);
- *al = SSL_AD_INAPPROPRIATE_FALLBACK;
+ al = SSL_AD_INAPPROPRIATE_FALLBACK;
goto err;
}
}
@@ -1665,7 +1664,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
* we need to have the cipher in the cipher list if we are asked
* to reuse it
*/
- *al = SSL_AD_ILLEGAL_PARAMETER;
+ al = SSL_AD_ILLEGAL_PARAMETER;
SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO,
SSL_R_REQUIRED_CIPHER_MISSING);
goto err;
@@ -1679,7 +1678,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
if (loop >= clienthello->compressions_len) {
/* no compress */
- *al = SSL_AD_DECODE_ERROR;
+ al = SSL_AD_DECODE_ERROR;
SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, SSL_R_NO_COMPRESSION_SPECIFIED);
goto err;
}
@@ -1691,7 +1690,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
/* TLS extensions */
if (!tls_parse_all_extensions(s, SSL_EXT_CLIENT_HELLO,
- clienthello->pre_proc_exts, NULL, 0, al)) {
+ clienthello->pre_proc_exts, NULL, 0, &al)) {
SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, SSL_R_PARSE_TLSEXT);
goto err;
}
@@ -1736,7 +1735,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
pref_cipher = ssl3_choose_cipher(s, s->session->ciphers,
SSL_get_ciphers(s));
if (pref_cipher == NULL) {
- *al = SSL_AD_HANDSHAKE_FAILURE;
+ al = SSL_AD_HANDSHAKE_FAILURE;
SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, SSL_R_NO_SHARED_CIPHER);
goto err;
}
@@ -1786,7 +1785,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
break;
}
if (k >= clienthello->compressions_len) {
- *al = SSL_AD_ILLEGAL_PARAMETER;
+ al = SSL_AD_ILLEGAL_PARAMETER;
SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO,
SSL_R_REQUIRED_COMPRESSION_ALGORITHM_MISSING);
goto err;
@@ -1836,7 +1835,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
sk_SSL_CIPHER_free(s->session->ciphers);
s->session->ciphers = ciphers;
if (ciphers == NULL) {
- *al = SSL_AD_INTERNAL_ERROR;
+ al = SSL_AD_INTERNAL_ERROR;
SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, ERR_R_INTERNAL_ERROR);
goto err;
}
@@ -1863,6 +1862,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *al)
return 1;
err:
ossl_statem_set_error(s);
+ *pal = al;
sk_SSL_CIPHER_free(ciphers);
sk_SSL_CIPHER_free(scsvs);
More information about the openssl-commits
mailing list