From: Matt Caswell Date: Thu, 12 May 2022 15:35:52 +0000 (+0100) Subject: Distinguish between fatal and non-fatal errors when creating a record layer X-Git-Tag: openssl-3.2.0-alpha1~2245 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7c2939999f8e43d996d846867ba326b052f821d6;p=thirdparty%2Fopenssl.git Distinguish between fatal and non-fatal errors when creating a record layer Reviewed-by: Hugo Landau Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/18132) --- diff --git a/ssl/record/methods/ktls_meth.c b/ssl/record/methods/ktls_meth.c index 18576cee263..767e2ed74cf 100644 --- a/ssl/record/methods/ktls_meth.c +++ b/ssl/record/methods/ktls_meth.c @@ -30,18 +30,22 @@ static int ktls_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, void *rl_sequence; ktls_crypto_info_t crypto_info; - /* Check if we are suitable for KTLS */ + /* + * Check if we are suitable for KTLS. If not suitable we return + * OSSL_RECORD_RETURN_NON_FATAL_ERR so that other record layers can be tried + * instead + */ if (comp != NULL) - return 0; + return OSSL_RECORD_RETURN_NON_FATAL_ERR; /* ktls supports only the maximum fragment size */ if (ssl_get_max_send_fragment(s) != SSL3_RT_MAX_PLAIN_LENGTH) - return 0; + return OSSL_RECORD_RETURN_NON_FATAL_ERR; /* check that cipher is supported */ if (!ktls_check_supported_cipher(s, ciph, taglen)) - return 0; + return OSSL_RECORD_RETURN_NON_FATAL_ERR; /* * TODO(RECLAYER): For the write side we need to add a check for @@ -51,7 +55,7 @@ static int ktls_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, /* All future data will get encrypted by ktls. Flush the BIO or skip ktls */ if (rl->direction == OSSL_RECORD_DIRECTION_WRITE) { if (BIO_flush(rl->bio) <= 0) - return 0; + return OSSL_RECORD_RETURN_NON_FATAL_ERR; } if (rl->direction == OSSL_RECORD_DIRECTION_WRITE) @@ -62,12 +66,12 @@ static int ktls_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, if (!ktls_configure_crypto(s, ciph, rl_sequence, &crypto_info, rl->direction == OSSL_RECORD_DIRECTION_WRITE, iv, ivlen, key, keylen, mackey, mackeylen)) - return 0; + return OSSL_RECORD_RETURN_NON_FATAL_ERR; if (!BIO_set_ktls(rl->bio, &crypto_info, rl->direction)) - return 0; + return OSSL_RECORD_RETURN_NON_FATAL_ERR; - return 1; + return OSSL_RECORD_RETURN_SUCCESS; } static int ktls_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *inrecs, size_t n_recs, diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h index b5d1bc23054..2e21a25b872 100644 --- a/ssl/record/methods/recmethod_local.h +++ b/ssl/record/methods/recmethod_local.h @@ -16,6 +16,11 @@ /* Protocol version specific function pointers */ struct record_functions_st { + /* + * Returns either OSSL_RECORD_RETURN_SUCCESS, OSSL_RECORD_RETURN_FATAL or + * OSSL_RECORD_RETURN_NON_FATAL_ERR if we can keep trying to find an + * alternative record layer. + */ int (*set_crypto_state)(OSSL_RECORD_LAYER *rl, int level, unsigned char *key, size_t keylen, unsigned char *iv, size_t ivlen, @@ -28,9 +33,16 @@ struct record_functions_st const SSL_COMP *comp, /* TODO(RECLAYER): Remove me */ SSL_CONNECTION *s); + /* + * Returns: + * 0: if the record is publicly invalid, or an internal error, or AEAD + * decryption failed, or EtM decryption failed. + * 1: Success or MtE decryption failed (MAC will be randomised) + */ int (*cipher)(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs, int sending, SSL_MAC_BUF *macs, size_t macsize, /* TODO(RECLAYER): Remove me */ SSL_CONNECTION *s); + /* Returns 1 for success or 0 for error */ int (*mac)(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec, unsigned char *md, int sending, /* TODO(RECLAYER): Remove me */SSL_CONNECTION *ssl); }; diff --git a/ssl/record/methods/ssl3_meth.c b/ssl/record/methods/ssl3_meth.c index 8ad3e3221d7..15f5d02c417 100644 --- a/ssl/record/methods/ssl3_meth.c +++ b/ssl/record/methods/ssl3_meth.c @@ -30,50 +30,48 @@ static int ssl3_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, EVP_CIPHER_CTX *ciph_ctx; if (md == NULL) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); + return OSSL_RECORD_RETURN_FATAL; } if ((rl->enc_read_ctx = EVP_CIPHER_CTX_new()) == NULL) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_MALLOC_FAILURE); - return 0; + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); + return OSSL_RECORD_RETURN_FATAL; } ciph_ctx = rl->enc_read_ctx; rl->read_hash = EVP_MD_CTX_new(); if (rl->read_hash == NULL) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); + return OSSL_RECORD_RETURN_FATAL; } #ifndef OPENSSL_NO_COMP if (comp != NULL) { rl->expand = COMP_CTX_new(comp->method); if (rl->expand == NULL) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, - SSL_R_COMPRESSION_LIBRARY_ERROR); - return 0; + ERR_raise(ERR_LIB_SSL, SSL_R_COMPRESSION_LIBRARY_ERROR); + return OSSL_RECORD_RETURN_FATAL; } } #endif if (!EVP_DecryptInit_ex(ciph_ctx, ciph, NULL, key, iv)) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); + return OSSL_RECORD_RETURN_FATAL; } if (EVP_CIPHER_get0_provider(ciph) != NULL && !ossl_set_tls_provider_parameters(rl, ciph_ctx, ciph, md, s)) { - /* RLAYERfatal already called */ - return 0; + return OSSL_RECORD_RETURN_FATAL; } if (mackeylen > sizeof(rl->mac_secret)) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); + return OSSL_RECORD_RETURN_FATAL; } memcpy(rl->mac_secret, mackey, mackeylen); - return 1; + return OSSL_RECORD_RETURN_SUCCESS; } /* diff --git a/ssl/record/methods/tls13_meth.c b/ssl/record/methods/tls13_meth.c index aaee322ae7b..bc8d5f0a03b 100644 --- a/ssl/record/methods/tls13_meth.c +++ b/ssl/record/methods/tls13_meth.c @@ -30,15 +30,15 @@ static int tls13_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, int mode; if (ivlen > sizeof(rl->iv)) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); + return OSSL_RECORD_RETURN_FATAL; } memcpy(rl->iv, iv, ivlen); ciph_ctx = rl->enc_read_ctx = EVP_CIPHER_CTX_new(); if (ciph_ctx == NULL) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_MALLOC_FAILURE); - return 0; + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); + return OSSL_RECORD_RETURN_FATAL; } RECORD_LAYER_reset_read_sequence(&s->rlayer); @@ -51,11 +51,11 @@ static int tls13_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, || (mode == EVP_CIPH_CCM_MODE && EVP_CIPHER_CTX_ctrl(ciph_ctx, EVP_CTRL_AEAD_SET_TAG, taglen, NULL) <= 0) || EVP_DecryptInit_ex(ciph_ctx, NULL, NULL, key, NULL) <= 0) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_EVP_LIB); - return 0; + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); + return OSSL_RECORD_RETURN_FATAL; } - return 1; + return OSSL_RECORD_RETURN_SUCCESS; } static int tls13_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs, diff --git a/ssl/record/methods/tls1_meth.c b/ssl/record/methods/tls1_meth.c index 9a77eec4924..5c0a8f2cc3d 100644 --- a/ssl/record/methods/tls1_meth.c +++ b/ssl/record/methods/tls1_meth.c @@ -32,7 +32,7 @@ static int tls1_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, EVP_PKEY *mac_key; if (level != OSSL_RECORD_PROTECTION_LEVEL_APPLICATION) - return 0; + return OSSL_RECORD_RETURN_FATAL; if (s->ext.use_etm) s->s3.flags |= TLS1_FLAGS_ENCRYPT_THEN_MAC_READ; @@ -51,7 +51,7 @@ static int tls1_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, if ((rl->enc_read_ctx = EVP_CIPHER_CTX_new()) == NULL) { RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_MALLOC_FAILURE); - return 0; + return OSSL_RECORD_RETURN_FATAL; } ciph_ctx = rl->enc_read_ctx; @@ -59,15 +59,14 @@ static int tls1_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, rl->read_hash = EVP_MD_CTX_new(); if (rl->read_hash == NULL) { RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; + return OSSL_RECORD_RETURN_FATAL; } #ifndef OPENSSL_NO_COMP if (comp != NULL) { rl->expand = COMP_CTX_new(comp->method); if (rl->expand == NULL) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, - SSL_R_COMPRESSION_LIBRARY_ERROR); - return 0; + ERR_raise(ERR_LIB_SSL, SSL_R_COMPRESSION_LIBRARY_ERROR); + return OSSL_RECORD_RETURN_FATAL; } } #endif @@ -100,8 +99,8 @@ static int tls1_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, rl->libctx, rl->propq, mac_key, NULL) <= 0) { EVP_PKEY_free(mac_key); - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); + return OSSL_RECORD_RETURN_FATAL; } EVP_PKEY_free(mac_key); } @@ -109,9 +108,9 @@ static int tls1_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, if (EVP_CIPHER_get_mode(ciph) == EVP_CIPH_GCM_MODE) { if (!EVP_DecryptInit_ex(ciph_ctx, ciph, NULL, key, NULL) || EVP_CIPHER_CTX_ctrl(ciph_ctx, EVP_CTRL_GCM_SET_IV_FIXED, - (int)ivlen, iv) <= 0) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; + (int)ivlen, iv) <= 0) { + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); + return OSSL_RECORD_RETURN_FATAL; } } else if (EVP_CIPHER_get_mode(ciph) == EVP_CIPH_CCM_MODE) { if (!EVP_DecryptInit_ex(ciph_ctx, ciph, NULL, NULL, NULL) @@ -126,13 +125,13 @@ static int tls1_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, * why not in the initial EVP_DecryptInit_ex() call? */ || !EVP_DecryptInit_ex(ciph_ctx, NULL, NULL, key, NULL)) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); + return OSSL_RECORD_RETURN_FATAL; } } else { if (!EVP_DecryptInit_ex(ciph_ctx, ciph, NULL, key, iv)) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); + return OSSL_RECORD_RETURN_FATAL; } } /* Needed for "composite" AEADs, such as RC4-HMAC-MD5 */ @@ -140,16 +139,14 @@ static int tls1_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, && mackeylen != 0 && EVP_CIPHER_CTX_ctrl(ciph_ctx, EVP_CTRL_AEAD_SET_MAC_KEY, (int)mackeylen, mackey) <= 0) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); + return OSSL_RECORD_RETURN_FATAL; } if (EVP_CIPHER_get0_provider(ciph) != NULL - && !ossl_set_tls_provider_parameters(rl, ciph_ctx, ciph, md, s)) { - /* RLAYERfatal already called */ - return 0; - } + && !ossl_set_tls_provider_parameters(rl, ciph_ctx, ciph, md, s)) + return OSSL_RECORD_RETURN_FATAL; - return 1; + return OSSL_RECORD_RETURN_SUCCESS; } #define MAX_PADDING 256 diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c index 5106224c6c9..6189ab3a96c 100644 --- a/ssl/record/methods/tls_common.c +++ b/ssl/record/methods/tls_common.c @@ -63,7 +63,7 @@ int ossl_set_tls_provider_parameters(OSSL_RECORD_LAYER *rl, *pprm = OSSL_PARAM_construct_end(); if (!EVP_CIPHER_CTX_set_params(ctx, params)) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); return 0; } @@ -1102,7 +1102,7 @@ static int tls_release_record(OSSL_RECORD_LAYER *rl, void *rechandle) return OSSL_RECORD_RETURN_SUCCESS; } -static OSSL_RECORD_LAYER * +static int tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, int role, int direction, int level, unsigned char *key, size_t keylen, unsigned char *iv, size_t ivlen, @@ -1113,15 +1113,18 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, const EVP_MD *md, const SSL_COMP *comp, BIO *transport, BIO_ADDR *local, BIO_ADDR *peer, const OSSL_PARAM *settings, const OSSL_PARAM *options, + OSSL_RECORD_LAYER **retrl, /* TODO(RECLAYER): Remove me */ SSL_CONNECTION *s) { OSSL_RECORD_LAYER *rl = OPENSSL_zalloc(sizeof(*rl)); const OSSL_PARAM *p; + *retrl = NULL; + if (rl == NULL) { RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_MALLOC_FAILURE); - return NULL; + return OSSL_RECORD_RETURN_FATAL; } if (transport != NULL && !BIO_up_ref(transport)) { @@ -1176,13 +1179,14 @@ tls_int_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, if (!tls_set1_bio(rl, transport)) goto err; - return rl; + *retrl = rl; + return OSSL_RECORD_RETURN_SUCCESS; err: OPENSSL_free(rl); - return NULL; + return OSSL_RECORD_RETURN_FATAL; } -static OSSL_RECORD_LAYER * +static int tls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, int role, int direction, int level, unsigned char *key, size_t keylen, unsigned char *iv, size_t ivlen, @@ -1193,54 +1197,55 @@ tls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, const EVP_MD *md, const SSL_COMP *comp, BIO *transport, BIO_ADDR *local, BIO_ADDR *peer, const OSSL_PARAM *settings, const OSSL_PARAM *options, + OSSL_RECORD_LAYER **retrl, /* TODO(RECLAYER): Remove me */ SSL_CONNECTION *s) { - OSSL_RECORD_LAYER *rl = tls_int_new_record_layer(libctx, propq, vers, role, - direction, level, key, - keylen, iv, ivlen, mackey, - mackeylen, ciph, taglen, - mactype, md, comp, - transport, local, peer, - settings, options, s); + int ret; + + ret = tls_int_new_record_layer(libctx, propq, vers, role, direction, level, + key, keylen, iv, ivlen, mackey, mackeylen, + ciph, taglen, mactype, md, comp, transport, + local, peer, settings, options, retrl, s); - if (rl == NULL) - return NULL; + if (ret != OSSL_RECORD_RETURN_SUCCESS) + return ret; switch (vers) { case TLS_ANY_VERSION: - rl->funcs = &tls_any_funcs; + (*retrl)->funcs = &tls_any_funcs; break; case TLS1_3_VERSION: - rl->funcs = &tls_1_3_funcs; + (*retrl)->funcs = &tls_1_3_funcs; break; case TLS1_2_VERSION: case TLS1_1_VERSION: case TLS1_VERSION: - rl->funcs = &tls_1_funcs; + (*retrl)->funcs = &tls_1_funcs; break; case SSL3_VERSION: - rl->funcs = &ssl_3_0_funcs; + (*retrl)->funcs = &ssl_3_0_funcs; break; default: /* Should not happen */ - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); + ret = OSSL_RECORD_RETURN_FATAL; goto err; } - if (!rl->funcs->set_crypto_state(rl, level, key, keylen, iv, ivlen, - mackey, mackeylen, ciph, taglen, - mactype, md, comp, s)) - goto err; + ret = (*retrl)->funcs->set_crypto_state(*retrl, level, key, keylen, iv, + ivlen, mackey, mackeylen, ciph, + taglen, mactype, md, comp, s); - return rl; err: - /* TODO(RECLAYER): How do we distinguish between fatal and non-fatal errors? */ - OPENSSL_free(rl); - return NULL; + if (ret != OSSL_RECORD_RETURN_SUCCESS) { + OPENSSL_free(*retrl); + *retrl = NULL; + } + return ret; } -static OSSL_RECORD_LAYER * +static int dtls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, int role, int direction, int level, unsigned char *key, size_t keylen, unsigned char *iv, size_t ivlen, @@ -1251,27 +1256,27 @@ dtls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, const EVP_MD *md, const SSL_COMP *comp, BIO *transport, BIO_ADDR *local, BIO_ADDR *peer, const OSSL_PARAM *settings, const OSSL_PARAM *options, + OSSL_RECORD_LAYER **retrl, /* TODO(RECLAYER): Remove me */ SSL_CONNECTION *s) { - OSSL_RECORD_LAYER *rl = tls_int_new_record_layer(libctx, propq, vers, role, - direction, level, key, - keylen, iv, ivlen, mackey, - mackeylen, ciph, taglen, - mactype, md, comp, - transport, local, peer, - settings, options, s); + int ret; + + ret = tls_int_new_record_layer(libctx, propq, vers, role, direction, level, + key, keylen, iv, ivlen, mackey, mackeylen, + ciph, taglen, mactype, md, comp, transport, + local, peer, settings, options, retrl, s); - if (rl == NULL) - return NULL; + if (ret != OSSL_RECORD_RETURN_SUCCESS) + return ret; - rl->isdtls = 1; + (*retrl)->isdtls = 1; - return rl; + return OSSL_RECORD_RETURN_SUCCESS; } #ifndef OPENSSL_NO_KTLS -static OSSL_RECORD_LAYER * +static int ktls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, int role, int direction, int level, unsigned char *key, size_t keylen, unsigned char *iv, size_t ivlen, @@ -1282,32 +1287,31 @@ ktls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers, const EVP_MD *md, const SSL_COMP *comp, BIO *transport, BIO_ADDR *local, BIO_ADDR *peer, const OSSL_PARAM *settings, const OSSL_PARAM *options, + OSSL_RECORD_LAYER **retrl, /* TODO(RECLAYER): Remove me */ SSL_CONNECTION *s) { - OSSL_RECORD_LAYER *rl = tls_int_new_record_layer(libctx, propq, vers, role, - direction, level, key, - keylen, iv, ivlen, mackey, - mackeylen, ciph, taglen, - mactype, md, comp, - transport, local, peer, - settings, options, s); - - if (rl == NULL) - return NULL; - - rl->funcs = &ossl_ktls_funcs; - - if (!rl->funcs->set_crypto_state(rl, level, key, keylen, iv, ivlen, - mackey, mackeylen, ciph, taglen, - mactype, md, comp, s)) - goto err; + int ret; - return rl; - err: - /* TODO(RECLAYER): How do we distinguish between fatal and non-fatal errors? */ - OPENSSL_free(rl); - return NULL; + ret = tls_int_new_record_layer(libctx, propq, vers, role, direction, level, + key, keylen, iv, ivlen, mackey, mackeylen, + ciph, taglen, mactype, md, comp, transport, + local, peer, settings, options, retrl, s); + + if (ret != OSSL_RECORD_RETURN_SUCCESS) + return ret; + + (*retrl)->funcs = &ossl_ktls_funcs; + + ret = (*retrl)->funcs->set_crypto_state(*retrl, level, key, keylen, iv, + ivlen, mackey, mackeylen, ciph, + taglen, mactype, md, comp, s); + + if (ret != OSSL_RECORD_RETURN_SUCCESS) { + OPENSSL_free(*retrl); + *retrl = NULL; + } + return ret; } #endif diff --git a/ssl/record/methods/tlsany_meth.c b/ssl/record/methods/tlsany_meth.c index 12273549ce3..cde57a41cc0 100644 --- a/ssl/record/methods/tlsany_meth.c +++ b/ssl/record/methods/tlsany_meth.c @@ -26,13 +26,13 @@ static int tls_any_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, SSL_CONNECTION *s) { if (level != OSSL_RECORD_PROTECTION_LEVEL_NONE) { - RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; + ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); + return OSSL_RECORD_RETURN_FATAL; } /* No crypto protection at the "NONE" level so nothing to be done */ - return 1; + return OSSL_RECORD_RETURN_SUCCESS; } static int tls_any_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index a9e0baa6874..deb108cfc77 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -1831,13 +1831,21 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version, } for (;;) { - s->rrl = s->rrlmethod->new_record_layer(sctx->libctx, sctx->propq, - version, s->server, direction, - level, key, keylen, iv, ivlen, - mackey, mackeylen, ciph, taglen, - mactype, md, comp, s->rbio, - NULL, NULL, NULL, options, s); - if (s->rrl == NULL) { + int rlret; + + rlret = s->rrlmethod->new_record_layer(sctx->libctx, sctx->propq, + version, s->server, direction, + level, key, keylen, iv, ivlen, + mackey, mackeylen, ciph, taglen, + mactype, md, comp, s->rbio, + NULL, NULL, NULL, options, + &s->rrl, s); + switch (rlret) { + case OSSL_RECORD_RETURN_FATAL: + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_RECORD_LAYER_FAILURE); + goto err; + + case OSSL_RECORD_RETURN_NON_FATAL_ERR: if (s->rrlmethod != origmeth && origmeth != NULL) { /* * We tried a new record layer method, but it didn't work out, @@ -1846,7 +1854,15 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version, s->rrlmethod = origmeth; continue; } - ERR_raise(ERR_LIB_SSL, SSL_R_NO_SUITABLE_RECORD_LAYER); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_NO_SUITABLE_RECORD_LAYER); + goto err; + + case OSSL_RECORD_RETURN_SUCCESS: + break; + + default: + /* Should not happen */ + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); goto err; } break; diff --git a/ssl/record/recordmethod.h b/ssl/record/recordmethod.h index 157b154805a..43562e36b4d 100644 --- a/ssl/record/recordmethod.h +++ b/ssl/record/recordmethod.h @@ -129,6 +129,13 @@ struct ossl_record_method_st { * force at any one time (one for reading and one for writing). In some * protocols more than 2 might be used (e.g. in DTLS for retransmitting * messages from an earlier epoch). + * + * The created OSSL_RECORD_LAYER object is stored in *ret on success (or + * NULL otherwise). The return value will be one of + * OSSL_RECORD_RETURN_SUCCESS, OSSL_RECORD_RETURN_FATAL or + * OSSL_RECORD_RETURN_NON_FATAL. A non-fatal return means that creation of + * the record layer has failed because it is unsuitable, but an alternative + * record layer can be tried instead. */ /* @@ -136,27 +143,28 @@ struct ossl_record_method_st { * make this fetchable * TODO(RECLAYER): mactype should not be an int */ - OSSL_RECORD_LAYER *(*new_record_layer)(OSSL_LIB_CTX *libctx, - const char *propq, int vers, - int role, int direction, - int level, unsigned char *key, - size_t keylen, - unsigned char *iv, - size_t ivlen, - unsigned char *mackey, - size_t mackeylen, - const EVP_CIPHER *ciph, - size_t taglen, - /* TODO(RECLAYER): This probably should not be an int */ - int mactype, - const EVP_MD *md, - const SSL_COMP *comp, - BIO *transport, BIO_ADDR *local, - BIO_ADDR *peer, - const OSSL_PARAM *settings, - const OSSL_PARAM *options, - /* TODO(RECLAYER): Remove me */ - SSL_CONNECTION *s); + int (*new_record_layer)(OSSL_LIB_CTX *libctx, + const char *propq, int vers, + int role, int direction, + int level, unsigned char *key, + size_t keylen, + unsigned char *iv, + size_t ivlen, + unsigned char *mackey, + size_t mackeylen, + const EVP_CIPHER *ciph, + size_t taglen, + /* TODO(RECLAYER): This probably should not be an int */ + int mactype, + const EVP_MD *md, + const SSL_COMP *comp, + BIO *transport, BIO_ADDR *local, + BIO_ADDR *peer, + const OSSL_PARAM *settings, + const OSSL_PARAM *options, + OSSL_RECORD_LAYER **ret, + /* TODO(RECLAYER): Remove me */ + SSL_CONNECTION *s); void (*free)(OSSL_RECORD_LAYER *rl); int (*reset)(OSSL_RECORD_LAYER *rl); /* Is this needed? */ diff --git a/ssl/s3_enc.c b/ssl/s3_enc.c index fc9002b8e56..1135392a846 100644 --- a/ssl/s3_enc.c +++ b/ssl/s3_enc.c @@ -151,7 +151,7 @@ int ssl3_change_cipher_state(SSL_CONNECTION *s, int which) OSSL_RECORD_PROTECTION_LEVEL_APPLICATION, key, key_len, iv, iv_len, mac_secret, md_len, ciph, 0, NID_undef, md, comp)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_NO_SUITABLE_RECORD_LAYER); + /* SSLfatal already called */ goto err; } diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 4688aaa327c..9905a188b99 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -668,7 +668,7 @@ int ossl_ssl_connection_reset(SSL *s) OSSL_RECORD_PROTECTION_LEVEL_NONE, NULL, 0, NULL, 0, NULL, 0, NULL, 0, NID_undef, NULL, NULL)) { - SSLfatal(sc, SSL_AD_INTERNAL_ERROR, SSL_R_NO_SUITABLE_RECORD_LAYER); + /* SSLfatal already called */ return 0; } diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c index 20964dfd610..9d3827adbcf 100644 --- a/ssl/t1_enc.c +++ b/ssl/t1_enc.c @@ -286,7 +286,7 @@ int tls1_change_cipher_state(SSL_CONNECTION *s, int which) key, cl, iv, (size_t)k, mac_secret, mac_secret_size, c, taglen, mac_type, m, comp)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_NO_SUITABLE_RECORD_LAYER); + /* SSLfatal already called */ goto err; } diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c index d6069c492ad..7e5f551aae9 100644 --- a/ssl/tls13_enc.c +++ b/ssl/tls13_enc.c @@ -721,7 +721,7 @@ int tls13_change_cipher_state(SSL_CONNECTION *s, int which) OSSL_RECORD_DIRECTION_READ, level, key, keylen, iv, ivlen, NULL, 0, cipher, taglen, NID_undef, NULL, NULL)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_NO_SUITABLE_RECORD_LAYER); + /* SSLfatal already called */ goto err; } /* TODO(RECLAYER): Remove me */ @@ -842,7 +842,7 @@ int tls13_update_key(SSL_CONNECTION *s, int sending) key, keylen, iv, ivlen, NULL, 0, s->s3.tmp.new_sym_enc, taglen, NID_undef, NULL, NULL)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_NO_SUITABLE_RECORD_LAYER); + /* SSLfatal already called */ goto err; } }