From c76a4aead2660f417608eead5cdff81f04021220 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 6 Jul 2016 11:02:32 +0100 Subject: [PATCH] Errors fix up following break up of CKE processing Reviewed-by: Richard Levitte --- include/openssl/ssl.h | 6 +++ ssl/ssl_err.c | 7 +++ ssl/statem/statem_srvr.c | 100 +++++++++++++++++---------------------- 3 files changed, 57 insertions(+), 56 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index ce7110da59..1e50e7e4b3 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -2222,6 +2222,12 @@ void ERR_load_SSL_strings(void); # define SSL_F_TLS_PROCESS_CERT_STATUS 362 # define SSL_F_TLS_PROCESS_CERT_VERIFY 379 # define SSL_F_TLS_PROCESS_CHANGE_CIPHER_SPEC 363 +# define SSL_F_TLS_PROCESS_CKE_DHE 404 +# define SSL_F_TLS_PROCESS_CKE_ECDHE 405 +# define SSL_F_TLS_PROCESS_CKE_GOST 406 +# define SSL_F_TLS_PROCESS_CKE_PSK_PREAMBLE 407 +# define SSL_F_TLS_PROCESS_CKE_RSA 409 +# define SSL_F_TLS_PROCESS_CKE_SRP 410 # define SSL_F_TLS_PROCESS_CLIENT_CERTIFICATE 380 # define SSL_F_TLS_PROCESS_CLIENT_HELLO 381 # define SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE 382 diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index aa4e5a4cb5..b69e91c58e 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -263,6 +263,13 @@ static ERR_STRING_DATA SSL_str_functs[] = { {ERR_FUNC(SSL_F_TLS_PROCESS_CERT_VERIFY), "tls_process_cert_verify"}, {ERR_FUNC(SSL_F_TLS_PROCESS_CHANGE_CIPHER_SPEC), "tls_process_change_cipher_spec"}, + {ERR_FUNC(SSL_F_TLS_PROCESS_CKE_DHE), "tls_process_cke_dhe"}, + {ERR_FUNC(SSL_F_TLS_PROCESS_CKE_ECDHE), "tls_process_cke_ecdhe"}, + {ERR_FUNC(SSL_F_TLS_PROCESS_CKE_GOST), "tls_process_cke_gost"}, + {ERR_FUNC(SSL_F_TLS_PROCESS_CKE_PSK_PREAMBLE), + "tls_process_cke_psk_preamble"}, + {ERR_FUNC(SSL_F_TLS_PROCESS_CKE_RSA), "tls_process_cke_rsa"}, + {ERR_FUNC(SSL_F_TLS_PROCESS_CKE_SRP), "tls_process_cke_srp"}, {ERR_FUNC(SSL_F_TLS_PROCESS_CLIENT_CERTIFICATE), "tls_process_client_certificate"}, {ERR_FUNC(SSL_F_TLS_PROCESS_CLIENT_HELLO), "tls_process_client_hello"}, diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index ddea83371e..82fced51dc 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -2024,25 +2024,24 @@ static int tls_process_cke_psk_preamble(SSL *s, PACKET *pkt, int *al) if (!PACKET_get_length_prefixed_2(pkt, &psk_identity)) { *al = SSL_AD_DECODE_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, SSL_R_LENGTH_MISMATCH); + SSLerr(SSL_F_TLS_PROCESS_CKE_PSK_PREAMBLE, SSL_R_LENGTH_MISMATCH); return 0; } if (PACKET_remaining(&psk_identity) > PSK_MAX_IDENTITY_LEN) { *al = SSL_AD_DECODE_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, - SSL_R_DATA_LENGTH_TOO_LONG); + SSLerr(SSL_F_TLS_PROCESS_CKE_PSK_PREAMBLE, SSL_R_DATA_LENGTH_TOO_LONG); return 0; } if (s->psk_server_callback == NULL) { *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, + SSLerr(SSL_F_TLS_PROCESS_CKE_PSK_PREAMBLE, SSL_R_PSK_NO_SERVER_CB); return 0; } if (!PACKET_strndup(&psk_identity, &s->session->psk_identity)) { *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_PSK_PREAMBLE, ERR_R_INTERNAL_ERROR); return 0; } @@ -2051,14 +2050,14 @@ static int tls_process_cke_psk_preamble(SSL *s, PACKET *pkt, int *al) if (psklen > PSK_MAX_PSK_LEN) { *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_PSK_PREAMBLE, ERR_R_INTERNAL_ERROR); return 0; } else if (psklen == 0) { /* * PSK related to the given identity not found */ *al = SSL_AD_UNKNOWN_PSK_IDENTITY; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, + SSLerr(SSL_F_TLS_PROCESS_CKE_PSK_PREAMBLE, SSL_R_PSK_IDENTITY_NOT_FOUND); return 0; } @@ -2069,7 +2068,7 @@ static int tls_process_cke_psk_preamble(SSL *s, PACKET *pkt, int *al) if (s->s3->tmp.psk == NULL) { *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE); + SSLerr(SSL_F_TLS_PROCESS_CKE_PSK_PREAMBLE, ERR_R_MALLOC_FAILURE); return 0; } @@ -2079,7 +2078,7 @@ static int tls_process_cke_psk_preamble(SSL *s, PACKET *pkt, int *al) #else /* Should never happen */ *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_PSK_PREAMBLE, ERR_R_INTERNAL_ERROR); return 0; #endif } @@ -2100,8 +2099,7 @@ static int tls_process_cke_rsa(SSL *s, PACKET *pkt, int *al) rsa = EVP_PKEY_get0_RSA(s->cert->pkeys[SSL_PKEY_RSA_ENC].privatekey); if (rsa == NULL) { *al = SSL_AD_HANDSHAKE_FAILURE; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, - SSL_R_MISSING_RSA_CERTIFICATE); + SSLerr(SSL_F_TLS_PROCESS_CKE_RSA, SSL_R_MISSING_RSA_CERTIFICATE); return 0; } @@ -2112,8 +2110,7 @@ static int tls_process_cke_rsa(SSL *s, PACKET *pkt, int *al) if (!PACKET_get_length_prefixed_2(pkt, &enc_premaster) || PACKET_remaining(pkt) != 0) { *al = SSL_AD_DECODE_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, - SSL_R_LENGTH_MISMATCH); + SSLerr(SSL_F_TLS_PROCESS_CKE_RSA, SSL_R_LENGTH_MISMATCH); return 0; } } @@ -2126,15 +2123,14 @@ static int tls_process_cke_rsa(SSL *s, PACKET *pkt, int *al) */ if (RSA_size(rsa) < SSL_MAX_MASTER_KEY_LENGTH) { *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, - RSA_R_KEY_SIZE_TOO_SMALL); + SSLerr(SSL_F_TLS_PROCESS_CKE_RSA, RSA_R_KEY_SIZE_TOO_SMALL); return 0; } rsa_decrypt = OPENSSL_malloc(RSA_size(rsa)); if (rsa_decrypt == NULL) { *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE); + SSLerr(SSL_F_TLS_PROCESS_CKE_RSA, ERR_R_MALLOC_FAILURE); return 0; } @@ -2169,7 +2165,7 @@ static int tls_process_cke_rsa(SSL *s, PACKET *pkt, int *al) */ if (decrypt_len < 11 + SSL_MAX_MASTER_KEY_LENGTH) { *al = SSL_AD_DECRYPT_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, SSL_R_DECRYPTION_FAILED); + SSLerr(SSL_F_TLS_PROCESS_CKE_RSA, SSL_R_DECRYPTION_FAILED); goto err; } @@ -2237,7 +2233,7 @@ static int tls_process_cke_rsa(SSL *s, PACKET *pkt, int *al) if (!ssl_generate_master_secret(s, rsa_decrypt + padding_len, sizeof(rand_premaster_secret), 0)) { *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_RSA, ERR_R_INTERNAL_ERROR); goto err; } @@ -2248,7 +2244,7 @@ static int tls_process_cke_rsa(SSL *s, PACKET *pkt, int *al) #else /* Should never happen */ *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_RSA, ERR_R_INTERNAL_ERROR); return 0; #endif } @@ -2266,46 +2262,43 @@ static int tls_process_cke_dhe(SSL *s, PACKET *pkt, int *al) if (!PACKET_get_net_2(pkt, &i)) { *al = SSL_AD_HANDSHAKE_FAILURE; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, + SSLerr(SSL_F_TLS_PROCESS_CKE_DHE, SSL_R_DH_PUBLIC_VALUE_LENGTH_IS_WRONG); goto err; } if (PACKET_remaining(pkt) != i) { - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, + SSLerr(SSL_F_TLS_PROCESS_CKE_DHE, SSL_R_DH_PUBLIC_VALUE_LENGTH_IS_WRONG); goto err; } skey = s->s3->tmp.pkey; if (skey == NULL) { *al = SSL_AD_HANDSHAKE_FAILURE; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, - SSL_R_MISSING_TMP_DH_KEY); + SSLerr(SSL_F_TLS_PROCESS_CKE_DHE, SSL_R_MISSING_TMP_DH_KEY); goto err; } if (PACKET_remaining(pkt) == 0L) { *al = SSL_AD_HANDSHAKE_FAILURE; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, - SSL_R_MISSING_TMP_DH_KEY); + SSLerr(SSL_F_TLS_PROCESS_CKE_DHE, SSL_R_MISSING_TMP_DH_KEY); goto err; } if (!PACKET_get_bytes(pkt, &data, i)) { /* We already checked we have enough data */ *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, - ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_DHE, ERR_R_INTERNAL_ERROR); goto err; } ckey = EVP_PKEY_new(); if (ckey == NULL || EVP_PKEY_copy_parameters(ckey, skey) == 0) { - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, SSL_R_BN_LIB); + SSLerr(SSL_F_TLS_PROCESS_CKE_DHE, SSL_R_BN_LIB); goto err; } cdh = EVP_PKEY_get0_DH(ckey); pub_key = BN_bin2bn(data, i, NULL); if (pub_key == NULL || !DH_set0_key(cdh, pub_key, NULL)) { - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_DHE, ERR_R_INTERNAL_ERROR); if (pub_key != NULL) BN_free(pub_key); goto err; @@ -2313,7 +2306,7 @@ static int tls_process_cke_dhe(SSL *s, PACKET *pkt, int *al) if (ssl_derive(s, skey, ckey) == 0) { *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_DHE, ERR_R_INTERNAL_ERROR); goto err; } @@ -2326,7 +2319,7 @@ static int tls_process_cke_dhe(SSL *s, PACKET *pkt, int *al) #else /* Should never happen */ *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_DHE, ERR_R_INTERNAL_ERROR); return 0; #endif } @@ -2341,8 +2334,7 @@ static int tls_process_cke_ecdhe(SSL *s, PACKET *pkt, int *al) if (PACKET_remaining(pkt) == 0L) { /* We don't support ECDH client auth */ *al = SSL_AD_HANDSHAKE_FAILURE; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, - SSL_R_MISSING_TMP_ECDH_KEY); + SSLerr(SSL_F_TLS_PROCESS_CKE_ECDHE, SSL_R_MISSING_TMP_ECDH_KEY); goto err; } else { unsigned int i; @@ -2356,30 +2348,29 @@ static int tls_process_cke_ecdhe(SSL *s, PACKET *pkt, int *al) /* Get encoded point length */ if (!PACKET_get_1(pkt, &i)) { *al = SSL_AD_DECODE_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, - SSL_R_LENGTH_MISMATCH); + SSLerr(SSL_F_TLS_PROCESS_CKE_ECDHE, SSL_R_LENGTH_MISMATCH); goto err; } if (!PACKET_get_bytes(pkt, &data, i) || PACKET_remaining(pkt) != 0) { - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_EC_LIB); + SSLerr(SSL_F_TLS_PROCESS_CKE_ECDHE, ERR_R_EC_LIB); goto err; } ckey = EVP_PKEY_new(); if (ckey == NULL || EVP_PKEY_copy_parameters(ckey, skey) <= 0) { - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_EVP_LIB); + SSLerr(SSL_F_TLS_PROCESS_CKE_ECDHE, ERR_R_EVP_LIB); goto err; } if (EC_KEY_oct2key(EVP_PKEY_get0_EC_KEY(ckey), data, i, NULL) == 0) { - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_EC_LIB); + SSLerr(SSL_F_TLS_PROCESS_CKE_ECDHE, ERR_R_EC_LIB); goto err; } } if (ssl_derive(s, skey, ckey) == 0) { *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_ECDHE, ERR_R_INTERNAL_ERROR); goto err; } @@ -2393,7 +2384,7 @@ static int tls_process_cke_ecdhe(SSL *s, PACKET *pkt, int *al) #else /* Should never happen */ *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_ECDHE, ERR_R_INTERNAL_ERROR); return 0; #endif } @@ -2407,29 +2398,28 @@ static int tls_process_cke_srp(SSL *s, PACKET *pkt, int *al) if (!PACKET_get_net_2(pkt, &i) || !PACKET_get_bytes(pkt, &data, i)) { *al = SSL_AD_DECODE_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, SSL_R_BAD_SRP_A_LENGTH); + SSLerr(SSL_F_TLS_PROCESS_CKE_SRP, SSL_R_BAD_SRP_A_LENGTH); return 0; } if ((s->srp_ctx.A = BN_bin2bn(data, i, NULL)) == NULL) { - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_BN_LIB); + SSLerr(SSL_F_TLS_PROCESS_CKE_SRP, ERR_R_BN_LIB); return 0; } if (BN_ucmp(s->srp_ctx.A, s->srp_ctx.N) >= 0 || BN_is_zero(s->srp_ctx.A)) { *al = SSL_AD_ILLEGAL_PARAMETER; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, - SSL_R_BAD_SRP_PARAMETERS); + SSLerr(SSL_F_TLS_PROCESS_CKE_SRP, SSL_R_BAD_SRP_PARAMETERS); return 0; } OPENSSL_free(s->session->srp_username); s->session->srp_username = OPENSSL_strdup(s->srp_ctx.login); if (s->session->srp_username == NULL) { - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE); + SSLerr(SSL_F_TLS_PROCESS_CKE_SRP, ERR_R_MALLOC_FAILURE); return 0; } if (!srp_generate_server_master_secret(s)) { - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_SRP, ERR_R_INTERNAL_ERROR); return 0; } @@ -2437,7 +2427,7 @@ static int tls_process_cke_srp(SSL *s, PACKET *pkt, int *al) #else /* Should never happen */ *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_SRP, ERR_R_INTERNAL_ERROR); return 0; #endif } @@ -2477,12 +2467,12 @@ static int tls_process_cke_gost(SSL *s, PACKET *pkt, int *al) pkey_ctx = EVP_PKEY_CTX_new(pk, NULL); if (pkey_ctx == NULL) { *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE); + SSLerr(SSL_F_TLS_PROCESS_CKE_GOST, ERR_R_MALLOC_FAILURE); return 0; } if (EVP_PKEY_decrypt_init(pkey_ctx) <= 0) { *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_GOST, ERR_R_INTERNAL_ERROR); return 0; } /* @@ -2500,7 +2490,7 @@ static int tls_process_cke_gost(SSL *s, PACKET *pkt, int *al) sess_key_len = PACKET_remaining(pkt); if (!PACKET_get_bytes(pkt, &data, sess_key_len)) { *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_GOST, ERR_R_INTERNAL_ERROR); goto err; } if (ASN1_get_object ((const unsigned char **)&data, &Tlen, &Ttag, @@ -2508,8 +2498,7 @@ static int tls_process_cke_gost(SSL *s, PACKET *pkt, int *al) || Ttag != V_ASN1_SEQUENCE || Tclass != V_ASN1_UNIVERSAL) { *al = SSL_AD_DECODE_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, - SSL_R_DECRYPTION_FAILED); + SSLerr(SSL_F_TLS_PROCESS_CKE_GOST, SSL_R_DECRYPTION_FAILED); goto err; } start = data; @@ -2517,15 +2506,14 @@ static int tls_process_cke_gost(SSL *s, PACKET *pkt, int *al) if (EVP_PKEY_decrypt (pkey_ctx, premaster_secret, &outlen, start, inlen) <= 0) { *al = SSL_AD_DECODE_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, - SSL_R_DECRYPTION_FAILED); + SSLerr(SSL_F_TLS_PROCESS_CKE_GOST, SSL_R_DECRYPTION_FAILED); goto err; } /* Generate master secret */ if (!ssl_generate_master_secret(s, premaster_secret, sizeof(premaster_secret), 0)) { *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_GOST, ERR_R_INTERNAL_ERROR); goto err; } /* Check if pubkey from client certificate was used */ @@ -2540,7 +2528,7 @@ static int tls_process_cke_gost(SSL *s, PACKET *pkt, int *al) #else /* Should never happen */ *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_PROCESS_CKE_GOST, ERR_R_INTERNAL_ERROR); return 0; #endif } -- 2.25.1