]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
Distinguish between fatal and non-fatal errors when creating a record layer
authorMatt Caswell <matt@openssl.org>
Thu, 12 May 2022 15:35:52 +0000 (16:35 +0100)
committerMatt Caswell <matt@openssl.org>
Thu, 18 Aug 2022 15:38:12 +0000 (16:38 +0100)
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18132)

13 files changed:
ssl/record/methods/ktls_meth.c
ssl/record/methods/recmethod_local.h
ssl/record/methods/ssl3_meth.c
ssl/record/methods/tls13_meth.c
ssl/record/methods/tls1_meth.c
ssl/record/methods/tls_common.c
ssl/record/methods/tlsany_meth.c
ssl/record/rec_layer_s3.c
ssl/record/recordmethod.h
ssl/s3_enc.c
ssl/ssl_lib.c
ssl/t1_enc.c
ssl/tls13_enc.c

index 18576cee26337eb46d9dae9a92771d9b61681d48..767e2ed74cf9b65d81c6300965498865f6b00185 100644 (file)
@@ -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,
index b5d1bc230549a866d588141255f391a4fe4fd107..2e21a25b87272a9ff1b40d3f7e846dea3ebb0650 100644 (file)
 /* 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);
 };
index 8ad3e3221d73cbf5694de7f3d41928561c7be5a0..15f5d02c4174a5a9a03e7ba1d7b3972430f323d5 100644 (file)
@@ -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;
 }
 
 /*
index aaee322ae7ba92ad2c0f7f4d97742ba522ad8d7f..bc8d5f0a03b87dd7944059df7b52e46cfd3b2f59 100644 (file)
@@ -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,
index 9a77eec49245628748d776e25d7a0659853a3174..5c0a8f2cc3dcaa36137ef07192a29cd6c73152f0 100644 (file)
@@ -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
index 5106224c6c93b876f796a62b3fe7b81519b167c2..6189ab3a96cf741edb234b969d497de778252bf9 100644 (file)
@@ -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
 
index 12273549ce3d9649f5a9ec75fe1b5f23a10961fe..cde57a41cc05d761010199782d76b09feccf5659 100644 (file)
@@ -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,
index a9e0baa687440ee100ed442345549bd3a1c462bf..deb108cfc77d947ae01b7244408c75fa76bb3921 100644 (file)
@@ -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;
index 157b154805a6be6ead7411c84165469d6d62f573..43562e36b4def569ea84c71bb3dc5833d0f97740 100644 (file)
@@ -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? */
index fc9002b8e564f22db429ac2cdbf14df13003a878..1135392a846f3927c890f0f5c50906813dd3fd95 100644 (file)
@@ -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;
         }
 
index 4688aaa327c5bd28cc8ad424f03c4c464ba01978..9905a188b9996eca61674625f8ae829db1604017 100644 (file)
@@ -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;
     }
 
index 20964dfd610417d274e82e8b57a15f6d8aa526e4..9d3827adbcf56feb9a25579be74dd70131ebdee7 100644 (file)
@@ -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;
             }
 
index d6069c492ad66aa95a58abb32018f26761a3caaa..7e5f551aae9b4b48b9dd99344d5f996920447ec7 100644 (file)
@@ -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;
         }
     }