mirrors/openssl
0
0
Fork 0
mirror of https://github.com/openssl/openssl.git synced 2024-12-12 11:33:41 +08:00
Code Issues Packages Projects Releases Wiki Activity

Add some more cleanups

Browse Source 
Follow up from the conversion to use SSLfatal() in the state machine to
clean things up a bit more.

[extended tests]

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4778)
This commit is contained in:
Matt Caswell 2017-11-23 13:11:42 +00:00
parent f9f674eb76
commit 29bfd5b79a
9 changed files with 70 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
crypto/err/openssl.txt
View File

@ -1079,7 +1079,9 @@ SSL_F_SSL_CERT_NEW:162:ssl_cert_new
SSL_F_SSL_CERT_SET0_CHAIN:340:ssl_cert_set0_chain
SSL_F_SSL_CHECK_PRIVATE_KEY:163:SSL_check_private_key
SSL_F_SSL_CHECK_SERVERHELLO_TLSEXT:280:*
SSL_F_SSL_CHECK_SRP_EXT_CLIENTHELLO:606:ssl_check_srp_ext_ClientHello
SSL_F_SSL_CHECK_SRVR_ECC_CERT_AND_ALG:279:ssl_check_srvr_ecc_cert_and_alg
SSL_F_SSL_CHOOSE_CLIENT_VERSION:607:ssl_choose_client_version
SSL_F_SSL_CIPHER_LIST_TO_BYTES:425:ssl_cipher_list_to_bytes
SSL_F_SSL_CIPHER_PROCESS_RULESTR:230:ssl_cipher_process_rulestr
SSL_F_SSL_CIPHER_STRENGTH_SORT:231:ssl_cipher_strength_sort

2
include/openssl/sslerr.h
View File

@ -132,7 +132,9 @@ int ERR_load_SSL_strings(void);
# define SSL_F_SSL_CERT_SET0_CHAIN 340
# define SSL_F_SSL_CHECK_PRIVATE_KEY 163
# define SSL_F_SSL_CHECK_SERVERHELLO_TLSEXT 280
# define SSL_F_SSL_CHECK_SRP_EXT_CLIENTHELLO 606
# define SSL_F_SSL_CHECK_SRVR_ECC_CERT_AND_ALG 279
# define SSL_F_SSL_CHOOSE_CLIENT_VERSION 607
# define SSL_F_SSL_CIPHER_LIST_TO_BYTES 425
# define SSL_F_SSL_CIPHER_PROCESS_RULESTR 230
# define SSL_F_SSL_CIPHER_STRENGTH_SORT 231

4
ssl/ssl_err.c
View File

@ -180,8 +180,12 @@ static const ERR_STRING_DATA SSL_str_functs[] = {
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_CHECK_PRIVATE_KEY, 0),
"SSL_check_private_key"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_CHECK_SERVERHELLO_TLSEXT, 0), ""},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_CHECK_SRP_EXT_CLIENTHELLO, 0),
"ssl_check_srp_ext_ClientHello"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_CHECK_SRVR_ECC_CERT_AND_ALG, 0),
"ssl_check_srvr_ecc_cert_and_alg"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_CHOOSE_CLIENT_VERSION, 0),
"ssl_choose_client_version"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_CIPHER_LIST_TO_BYTES, 0),
"ssl_cipher_list_to_bytes"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_CIPHER_PROCESS_RULESTR, 0),

3
ssl/ssl_locl.h
View File

@ -2266,8 +2266,7 @@ __owur int ssl_check_version_downgrade(SSL *s);
__owur int ssl_set_version_bound(int method_version, int version, int *bound);
__owur int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello,
DOWNGRADE *dgrd);
__owur int ssl_choose_client_version(SSL *s, int version, int checkdgrd,
int *al);
__owur int ssl_choose_client_version(SSL *s, int version, int checkdgrd);
int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version);
__owur long tls1_default_timeout(void);

22
ssl/statem/extensions.c
View File

@ -439,12 +439,11 @@ int extension_is_relevant(SSL *s, unsigned int extctx, unsigned int thisctx)
/*
* Gather a list of all the extensions from the data in |packet]. |context|
* tells us which message this extension is for. The raw extension data is
* stored in |*res| on success. In the event of an error the alert type to use
* is stored in |*al|. We don't actually process the content of the extensions
* yet, except to check their types. This function also runs the initialiser
* functions for all known extensions if |init| is nonzero (whether we have
* collected them or not). If successful the caller is responsible for freeing
* the contents of |*res|.
* stored in |*res| on success. We don't actually process the content of the
* extensions yet, except to check their types. This function also runs the
* initialiser functions for all known extensions if |init| is nonzero (whether
* we have collected them or not). If successful the caller is responsible for
* freeing the contents of |*res|.
*
* Per http://tools.ietf.org/html/rfc5246#section-7.4.1.4, there may not be
* more than one extension of the same type in a ClientHello or ServerHello.
@ -579,9 +578,8 @@ int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
* given |context| and the parser has not already been run. If this is for a
* Certificate message, then we also provide the parser with the relevant
* Certificate |x| and its position in the |chainidx| with 0 being the first
* Certificate. Returns 1 on success or 0 on failure. In the event of a failure
* |*al| is populated with a suitable alert code. If an extension is not present
* this counted as success.
* Certificate. Returns 1 on success or 0 on failure. If an extension is not
* present this counted as success.
*/
int tls_parse_extension(SSL *s, TLSEXT_INDEX idx, int context,
RAW_EXTENSION *exts, X509 *x, size_t chainidx)
@ -631,8 +629,7 @@ int tls_parse_extension(SSL *s, TLSEXT_INDEX idx, int context,
* finalisation for all extensions at the end if |fin| is nonzero, whether we
* collected them or not. Returns 1 for success or 0 for failure. If we are
* working on a Certificate message then we also pass the Certificate |x| and
* its position in the |chainidx|, with 0 being the first certificate. On
* failure, |*al| is populated with a suitable alert code.
* its position in the |chainidx|, with 0 being the first certificate.
*/
int tls_parse_all_extensions(SSL *s, int context, RAW_EXTENSION *exts, X509 *x,
size_t chainidx, int fin)
@ -782,8 +779,7 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
* Built in extension finalisation and initialisation functions. All initialise
* or finalise the associated extension type for the given |context|. For
* finalisers |sent| is set to 1 if we saw the extension during parsing, and 0
* otherwise. These functions return 1 on success or 0 on failure. In the event
* of a failure then |*al| is populated with a suitable error code.
* otherwise. These functions return 1 on success or 0 on failure.
*/
static int final_renegotiate(SSL *s, unsigned int context, int sent)

5
ssl/statem/extensions_srvr.c
View File

@ -400,8 +400,7 @@ int tls_parse_ctos_npn(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
/*
* Save the ALPN extension in a ClientHello.|pkt| holds the contents of the ALPN
* extension, not including type and length. |al| is a pointer to the alert
* value to send in the event of a failure. Returns: 1 on success, 0 on error.
* extension, not including type and length. Returns: 1 on success, 0 on error.
*/
int tls_parse_ctos_alpn(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
size_t chainidx)
@ -523,7 +522,6 @@ int tls_parse_ctos_etm(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
/*
* Process a psk_kex_modes extension received in the ClientHello. |pkt| contains
* the raw PACKET data for the extension. Returns 1 on success or 0 on failure.
* If a failure occurs then |*al| is set to an appropriate alert value.
*/
int tls_parse_ctos_psk_kex_modes(SSL *s, PACKET *pkt, unsigned int context,
X509 *x, size_t chainidx)
@ -554,7 +552,6 @@ int tls_parse_ctos_psk_kex_modes(SSL *s, PACKET *pkt, unsigned int context,
/*
* Process a key_share extension received in the ClientHello. |pkt| contains
* the raw PACKET data for the extension. Returns 1 on success or 0 on failure.
* If a failure occurs then |*al| is set to an appropriate alert value.
*/
int tls_parse_ctos_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
size_t chainidx)

9
ssl/statem/statem_clnt.c
View File

@ -1310,11 +1310,10 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
PACKET session_id, extpkt;
size_t session_id_len;
const unsigned char *cipherchars;
int al = SSL_AD_INTERNAL_ERROR;
unsigned int compression;
unsigned int sversion;
unsigned int context;
int protverr, discard;
int discard;
RAW_EXTENSION *extensions = NULL;
#ifndef OPENSSL_NO_COMP
SSL_COMP *comp;
@ -1338,10 +1337,8 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
* Must be done after reading the random data so we can check for the
* TLSv1.3 downgrade sentinels
*/
protverr = ssl_choose_client_version(s, sversion, 1, &al);
if (protverr != 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
protverr);
if (!ssl_choose_client_version(s, sversion, 1)) {
/* SSLfatal() already called */
goto err;
}

42
ssl/statem/statem_lib.c
View File

@ -1740,11 +1740,10 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd)
* @s: client SSL handle.
* @version: The proposed version from the server's HELLO.
* @checkdgrd: Whether to check the downgrade sentinels in the server_random
* @al: Where to store any alert value that may be generated
*
* Returns 0 on success or an SSL error reason number on failure.
* Returns 1 on success or 0 on error.
*/
int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al)
int ssl_choose_client_version(SSL *s, int version, int checkdgrd)
{
const version_info *vent;
const version_info *table;
@ -1755,15 +1754,18 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al)
version = TLS1_3_VERSION;
if (s->hello_retry_request && version != TLS1_3_VERSION) {
*al = SSL_AD_PROTOCOL_VERSION;
return SSL_R_WRONG_SSL_VERSION;
SSLfatal(s, SSL_AD_PROTOCOL_VERSION, SSL_F_SSL_CHOOSE_CLIENT_VERSION,
SSL_R_WRONG_SSL_VERSION);
return 0;
}
switch (s->method->version) {
default:
if (version != s->version) {
*al = SSL_AD_PROTOCOL_VERSION;
return SSL_R_WRONG_SSL_VERSION;
SSLfatal(s, SSL_AD_PROTOCOL_VERSION,
SSL_F_SSL_CHOOSE_CLIENT_VERSION,
SSL_R_WRONG_SSL_VERSION);
return 0;
}
/*
* If this SSL handle is not from a version flexible method we don't
@ -1772,7 +1774,7 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al)
* versions they don't want. If not, then easy to fix, just return
* ssl_method_error(s, s->method)
*/
return 0;
return 1;
case TLS_ANY_VERSION:
table = tls_version_table;
break;
@ -1795,8 +1797,9 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al)
err = ssl_method_error(s, method);
if (err != 0) {
if (version == vent->version) {
*al = SSL_AD_PROTOCOL_VERSION;
return err;
SSLfatal(s, SSL_AD_PROTOCOL_VERSION,
SSL_F_SSL_CHOOSE_CLIENT_VERSION, err);
return 0;
}
continue;
@ -1815,8 +1818,10 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al)
s->s3->server_random + SSL3_RANDOM_SIZE
- sizeof(tls12downgrade),
sizeof(tls12downgrade)) == 0) {
*al = SSL_AD_ILLEGAL_PARAMETER;
return SSL_R_INAPPROPRIATE_FALLBACK;
SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
SSL_F_SSL_CHOOSE_CLIENT_VERSION,
SSL_R_INAPPROPRIATE_FALLBACK);
return 0;
}
} else if (!SSL_IS_DTLS(s)
&& version < TLS1_2_VERSION
@ -1825,8 +1830,10 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al)
s->s3->server_random + SSL3_RANDOM_SIZE
- sizeof(tls11downgrade),
sizeof(tls11downgrade)) == 0) {
*al = SSL_AD_ILLEGAL_PARAMETER;
return SSL_R_INAPPROPRIATE_FALLBACK;
SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
SSL_F_SSL_CHOOSE_CLIENT_VERSION,
SSL_R_INAPPROPRIATE_FALLBACK);
return 0;
}
}
}
@ -1834,11 +1841,12 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al)
s->method = method;
s->version = version;
return 0;
return 1;
}
*al = SSL_AD_PROTOCOL_VERSION;
return SSL_R_UNSUPPORTED_PROTOCOL;
SSLfatal(s, SSL_AD_PROTOCOL_VERSION, SSL_F_SSL_CHOOSE_CLIENT_VERSION,
SSL_R_UNSUPPORTED_PROTOCOL);
return 0;
}
/*

45
ssl/statem/statem_srvr.c
View File

@ -1102,11 +1102,11 @@ WORK_STATE ossl_statem_server_post_process_message(SSL *s, WORK_STATE wst)
}
#ifndef OPENSSL_NO_SRP
static int ssl_check_srp_ext_ClientHello(SSL *s, int *al)
/* Returns 1 on success, 0 for retryable error, -1 for fatal error */
static int ssl_check_srp_ext_ClientHello(SSL *s)
{
int ret = SSL_ERROR_NONE;
*al = SSL_AD_UNRECOGNIZED_NAME;
int ret;
int al = SSL_AD_UNRECOGNIZED_NAME;
if ((s->s3->tmp.new_cipher->algorithm_mkey & SSL_kSRP) &&
(s->srp_ctx.TLS_ext_srp_username_callback != NULL)) {
@ -1115,13 +1115,24 @@ static int ssl_check_srp_ext_ClientHello(SSL *s, int *al)
* RFC 5054 says SHOULD reject, we do so if There is no srp
* login name
*/
ret = SSL3_AL_FATAL;
*al = SSL_AD_UNKNOWN_PSK_IDENTITY;
SSLfatal(s, SSL_AD_UNKNOWN_PSK_IDENTITY,
SSL_F_SSL_CHECK_SRP_EXT_CLIENTHELLO,
SSL_R_PSK_IDENTITY_NOT_FOUND);
return -1;
} else {
ret = SSL_srp_server_param_with_username(s, al);
ret = SSL_srp_server_param_with_username(s, &al);
if (ret < 0)
return 0;
if (ret == SSL3_AL_FATAL) {
SSLfatal(s, al, SSL_F_SSL_CHECK_SRP_EXT_CLIENTHELLO,
al == SSL_AD_UNKNOWN_PSK_IDENTITY
? SSL_R_PSK_IDENTITY_NOT_FOUND
: SSL_R_CLIENTHELLO_TLSEXT);
return -1;
}
}
}
return ret;
return 1;
}
#endif
@ -1986,7 +1997,7 @@ static int tls_handle_status_request(SSL *s)
/*
* Call the alpn_select callback if needed. Upon success, returns 1.
* Upon failure, returns 0 and sets |*al| to the appropriate fatal alert.
* Upon failure, returns 0.
*/
int tls_handle_alpn(SSL *s)
{
@ -2058,7 +2069,6 @@ int tls_handle_alpn(SSL *s)
WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst)
{
int al = SSL_AD_HANDSHAKE_FAILURE;
const SSL_CIPHER *cipher;
if (wst == WORK_MORE_A) {
@ -2158,24 +2168,15 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst)
#ifndef OPENSSL_NO_SRP
if (wst == WORK_MORE_C) {
int ret;
if ((ret = ssl_check_srp_ext_ClientHello(s, &al)) < 0) {
if ((ret = ssl_check_srp_ext_ClientHello(s)) == 0) {
/*
* callback indicates further work to be done
*/
s->rwstate = SSL_X509_LOOKUP;
return WORK_MORE_C;
}
if (ret != SSL_ERROR_NONE) {
/*
* This is not really an error but the only means to for
* a client to detect whether srp is supported.
*/
if (al != TLS1_AD_UNKNOWN_PSK_IDENTITY)
SSLfatal(s, al, SSL_F_TLS_POST_PROCESS_CLIENT_HELLO,
SSL_R_CLIENTHELLO_TLSEXT);
else
SSLfatal(s, al, SSL_F_TLS_POST_PROCESS_CLIENT_HELLO,
SSL_R_PSK_IDENTITY_NOT_FOUND);
if (ret < 0) {
/* SSLfatal() already called */
goto err;
}
}