EVP_CIPHER_CTX_set_keylen should not succeed if a bad keylen is passed

EVP_CIPHER_CTX_set_keylen() was succeeding even though a bad key length
is passed to it. This is because the set_ctx_params() were all accepting
this parameter and blindly changing the keylen even though the cipher did
not accept a variable key length. Even removing this didn't entirely
resolve the issue because set_ctx_params() functions succeed even if
passed a parameter they do not recognise.

This should fix various issues found by OSSfuzz/Cryptofuzz.

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/10449)

diff --git a/crypto/evp/evp_enc.c b/crypto/evp/evp_enc.c
index 3346e57dae99b89d8a11ce5119f4b997ef50c1b3..45e1dc67c292659f1c29f3bf1ce6e5caff7d87a3 100644 (file)
--- a/crypto/evp/evp_enc.c
+++ b/crypto/evp/evp_enc.c
@@ -1026,17 +1026,31 @@ int EVP_DecryptFinal_ex(EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl)
 
 int EVP_CIPHER_CTX_set_key_length(EVP_CIPHER_CTX *c, int keylen)
 {
-    int ok;
-    OSSL_PARAM params[2] = { OSSL_PARAM_END, OSSL_PARAM_END };
-    size_t len = keylen;
+    if (c->cipher->prov != NULL) {
+        int ok;
+        OSSL_PARAM params[2] = { OSSL_PARAM_END, OSSL_PARAM_END };
+        size_t len = keylen;
+
+        if (EVP_CIPHER_CTX_key_length(c) == keylen)
+            return 1;
+
+        /* Check the cipher actually understands this parameter */
+        if (OSSL_PARAM_locate_const(EVP_CIPHER_settable_ctx_params(c->cipher),
+                                    OSSL_CIPHER_PARAM_KEYLEN) == NULL)
+            return 0;
 
-    params[0] = OSSL_PARAM_construct_size_t(OSSL_CIPHER_PARAM_KEYLEN, &len);
-    ok = evp_do_ciph_ctx_setparams(c->cipher, c->provctx, params);
+        params[0] = OSSL_PARAM_construct_size_t(OSSL_CIPHER_PARAM_KEYLEN, &len);
+        ok = evp_do_ciph_ctx_setparams(c->cipher, c->provctx, params);
 
-    if (ok != EVP_CTRL_RET_UNSUPPORTED)
-        return ok;
+        return ok > 0 ? 1 : 0;
+    }
 
     /* TODO(3.0) legacy code follows */
+
+    /*
+     * Note there have never been any built-in ciphers that define this flag
+     * since it was first introduced.
+     */
     if (c->cipher->flags & EVP_CIPH_CUSTOM_KEY_LENGTH)
         return EVP_CIPHER_CTX_ctrl(c, EVP_CTRL_SET_KEY_LENGTH, keylen, NULL);
     if (EVP_CIPHER_CTX_key_length(c) == keylen)

diff --git a/providers/common/ciphers/cipher_common.c b/providers/common/ciphers/cipher_common.c
index d06e7b8004a0c6f1d365998a2c94e271674084b6..fe4560192d74db5578bec43c54299addb1014d76 100644 (file)
--- a/providers/common/ciphers/cipher_common.c
+++ b/providers/common/ciphers/cipher_common.c
@@ -71,6 +71,34 @@ CIPHER_DEFAULT_GETTABLE_CTX_PARAMS_END(cipher_generic)
 CIPHER_DEFAULT_SETTABLE_CTX_PARAMS_START(cipher_generic)
 CIPHER_DEFAULT_SETTABLE_CTX_PARAMS_END(cipher_generic)
 
+/*
+ * Variable key length cipher functions for OSSL_PARAM settables
+ */
+
+int cipher_var_keylen_set_ctx_params(void *vctx, const OSSL_PARAM params[])
+{
+    PROV_CIPHER_CTX *ctx = (PROV_CIPHER_CTX *)vctx;
+    const OSSL_PARAM *p;
+
+    if (!cipher_generic_set_ctx_params(vctx, params))
+        return 0;
+    p = OSSL_PARAM_locate_const(params, OSSL_CIPHER_PARAM_KEYLEN);
+    if (p != NULL) {
+        size_t keylen;
+
+        if (!OSSL_PARAM_get_size_t(p, &keylen)) {
+            ERR_raise(ERR_LIB_PROV, PROV_R_FAILED_TO_GET_PARAMETER);
+            return 0;
+        }
+        ctx->keylen = keylen;
+    }
+    return 1;
+}
+
+CIPHER_DEFAULT_SETTABLE_CTX_PARAMS_START(cipher_var_keylen)
+OSSL_PARAM_size_t(OSSL_CIPHER_PARAM_KEYLEN, NULL),
+CIPHER_DEFAULT_SETTABLE_CTX_PARAMS_END(cipher_var_keylen)
+
 /*-
  * AEAD cipher functions for OSSL_PARAM gettables and settables
  */
@@ -89,7 +117,6 @@ const OSSL_PARAM *cipher_aead_gettable_ctx_params(void)
 }
 
 static const OSSL_PARAM cipher_aead_known_settable_ctx_params[] = {
-    OSSL_PARAM_size_t(OSSL_CIPHER_PARAM_KEYLEN, NULL),
     OSSL_PARAM_size_t(OSSL_CIPHER_PARAM_AEAD_IVLEN, NULL),
     OSSL_PARAM_octet_string(OSSL_CIPHER_PARAM_AEAD_TAG, NULL, 0),
     OSSL_PARAM_octet_string(OSSL_CIPHER_PARAM_AEAD_TLS1_AAD, NULL, 0),
@@ -362,16 +389,6 @@ int cipher_generic_set_ctx_params(void *vctx, const OSSL_PARAM params[])
         }
         ctx->num = num;
     }
-    p = OSSL_PARAM_locate_const(params, OSSL_CIPHER_PARAM_KEYLEN);
-    if (p != NULL) {
-        size_t keylen;
-
-        if (!OSSL_PARAM_get_size_t(p, &keylen)) {
-            ERR_raise(ERR_LIB_PROV, PROV_R_FAILED_TO_GET_PARAMETER);
-            return 0;
-        }
-        ctx->keylen = keylen;
-    }
     return 1;
 }
 

diff --git a/providers/common/ciphers/cipher_gcm.c b/providers/common/ciphers/cipher_gcm.c
index d7c67e8b6b6d3de158faa274e39ce9d31f5c0edd..f479bc8fdabcd6cf717bb4b276f4693e1c0f7c80 100644 (file)
--- a/providers/common/ciphers/cipher_gcm.c
+++ b/providers/common/ciphers/cipher_gcm.c
@@ -202,25 +202,6 @@ int gcm_set_ctx_params(void *vctx, const OSSL_PARAM params[])
         }
     }
 
-    /*
-     * TODO(3.0) Temporary solution to address fuzz test crash, which will be
-     * reworked once the discussion in PR #9510 is resolved. i.e- We need a
-     * general solution for handling missing parameters inside set_params and
-     * get_params methods.
-     */
-    p = OSSL_PARAM_locate_const(params, OSSL_CIPHER_PARAM_KEYLEN);
-    if (p != NULL) {
-        size_t keylen;
-
-        if (!OSSL_PARAM_get_size_t(p, &keylen)) {
-            ERR_raise(ERR_LIB_PROV, PROV_R_FAILED_TO_GET_PARAMETER);
-            return 0;
-        }
-        /* The key length can not be modified for gcm mode */
-        if (keylen != ctx->keylen)
-            return 0;
-    }
-
     return 1;
 }
 

diff --git a/providers/common/include/prov/ciphercommon.h b/providers/common/include/prov/ciphercommon.h
index c9b00340172b775d76d352e8226bf787ddc679d5..1072b577fe41b79b25f75c84bccbd4b5e0eb5ea0 100644 (file)
--- a/providers/common/include/prov/ciphercommon.h
+++ b/providers/common/include/prov/ciphercommon.h
@@ -83,6 +83,8 @@ OSSL_OP_cipher_set_ctx_params_fn cipher_generic_set_ctx_params;
 OSSL_OP_cipher_gettable_params_fn     cipher_generic_gettable_params;
 OSSL_OP_cipher_gettable_ctx_params_fn cipher_generic_gettable_ctx_params;
 OSSL_OP_cipher_settable_ctx_params_fn cipher_generic_settable_ctx_params;
+OSSL_OP_cipher_set_ctx_params_fn cipher_var_keylen_set_ctx_params;
+OSSL_OP_cipher_settable_ctx_params_fn cipher_var_keylen_settable_ctx_params;
 OSSL_OP_cipher_gettable_ctx_params_fn cipher_aead_gettable_ctx_params;
 OSSL_OP_cipher_settable_ctx_params_fn cipher_aead_settable_ctx_params;
 int cipher_generic_get_params(OSSL_PARAM params[], unsigned int md,
@@ -119,8 +121,36 @@ const OSSL_DISPATCH alg##kbits##lcmode##_functions[] = {                       \
     { 0, NULL }                                                                \
 };
 
-#define IMPLEMENT_generic_cipher(alg, UCALG, lcmode, UCMODE, flags, kbits,     \
-                                 blkbits, ivbits, typ)                         \
+#define IMPLEMENT_var_keylen_cipher_func(alg, UCALG, lcmode, UCMODE, flags,    \
+                                         kbits, blkbits, ivbits, typ)          \
+const OSSL_DISPATCH alg##kbits##lcmode##_functions[] = {                       \
+    { OSSL_FUNC_CIPHER_NEWCTX,                                                 \
+      (void (*)(void)) alg##_##kbits##_##lcmode##_newctx },                    \
+    { OSSL_FUNC_CIPHER_FREECTX, (void (*)(void)) alg##_freectx },              \
+    { OSSL_FUNC_CIPHER_DUPCTX, (void (*)(void)) alg##_dupctx },                \
+    { OSSL_FUNC_CIPHER_ENCRYPT_INIT, (void (*)(void))cipher_generic_einit },   \
+    { OSSL_FUNC_CIPHER_DECRYPT_INIT, (void (*)(void))cipher_generic_dinit },   \
+    { OSSL_FUNC_CIPHER_UPDATE, (void (*)(void))cipher_generic_##typ##_update },\
+    { OSSL_FUNC_CIPHER_FINAL, (void (*)(void))cipher_generic_##typ##_final },  \
+    { OSSL_FUNC_CIPHER_CIPHER, (void (*)(void))cipher_generic_cipher },        \
+    { OSSL_FUNC_CIPHER_GET_PARAMS,                                             \
+      (void (*)(void)) alg##_##kbits##_##lcmode##_get_params },                \
+    { OSSL_FUNC_CIPHER_GET_CTX_PARAMS,                                         \
+      (void (*)(void))cipher_generic_get_ctx_params },                         \
+    { OSSL_FUNC_CIPHER_SET_CTX_PARAMS,                                         \
+      (void (*)(void))cipher_var_keylen_set_ctx_params },                      \
+    { OSSL_FUNC_CIPHER_GETTABLE_PARAMS,                                        \
+      (void (*)(void))cipher_generic_gettable_params },                        \
+    { OSSL_FUNC_CIPHER_GETTABLE_CTX_PARAMS,                                    \
+      (void (*)(void))cipher_generic_gettable_ctx_params },                    \
+    { OSSL_FUNC_CIPHER_SETTABLE_CTX_PARAMS,                                    \
+     (void (*)(void))cipher_var_keylen_settable_ctx_params },                  \
+    { 0, NULL }                                                                \
+};
+
+
+#define IMPLEMENT_generic_cipher_genfn(alg, UCALG, lcmode, UCMODE, flags,      \
+                                       kbits, blkbits, ivbits, typ)            \
 static OSSL_OP_cipher_get_params_fn alg##_##kbits##_##lcmode##_get_params;     \
 static int alg##_##kbits##_##lcmode##_get_params(OSSL_PARAM params[])          \
 {                                                                              \
@@ -138,9 +168,21 @@ static void * alg##_##kbits##_##lcmode##_newctx(void *provctx)                 \
      }                                                                         \
      return ctx;                                                               \
 }                                                                              \
+
+#define IMPLEMENT_generic_cipher(alg, UCALG, lcmode, UCMODE, flags, kbits,     \
+                                 blkbits, ivbits, typ)                         \
+IMPLEMENT_generic_cipher_genfn(alg, UCALG, lcmode, UCMODE, flags, kbits,       \
+                               blkbits, ivbits, typ)                           \
 IMPLEMENT_generic_cipher_func(alg, UCALG, lcmode, UCMODE, flags, kbits,        \
                               blkbits, ivbits, typ)
 
+#define IMPLEMENT_var_keylen_cipher(alg, UCALG, lcmode, UCMODE, flags, kbits,  \
+                                    blkbits, ivbits, typ)                      \
+IMPLEMENT_generic_cipher_genfn(alg, UCALG, lcmode, UCMODE, flags, kbits,       \
+                               blkbits, ivbits, typ)                           \
+IMPLEMENT_var_keylen_cipher_func(alg, UCALG, lcmode, UCMODE, flags, kbits,     \
+                                 blkbits, ivbits, typ)
+
 PROV_CIPHER_HW_FN cipher_hw_generic_cbc;
 PROV_CIPHER_HW_FN cipher_hw_generic_ecb;
 PROV_CIPHER_HW_FN cipher_hw_generic_ofb128;
@@ -262,7 +304,6 @@ const OSSL_PARAM * name##_gettable_ctx_params(void)                            \
 
 #define CIPHER_DEFAULT_SETTABLE_CTX_PARAMS_START(name)                         \
 static const OSSL_PARAM name##_known_settable_ctx_params[] = {                 \
-    OSSL_PARAM_size_t(OSSL_CIPHER_PARAM_KEYLEN, NULL),                         \
     OSSL_PARAM_uint(OSSL_CIPHER_PARAM_PADDING, NULL),                          \
     OSSL_PARAM_uint(OSSL_CIPHER_PARAM_NUM, NULL),
 #define CIPHER_DEFAULT_SETTABLE_CTX_PARAMS_END(name)                           \

diff --git a/providers/implementations/ciphers/cipher_blowfish.c b/providers/implementations/ciphers/cipher_blowfish.c
index 786cc1518909524bb5cb849e21da3c8cba822248..1ffb9452a8cb3ed50defa79e236cc7214c274902 100644 (file)
--- a/providers/implementations/ciphers/cipher_blowfish.c
+++ b/providers/implementations/ciphers/cipher_blowfish.c
@@ -39,10 +39,10 @@ static void *blowfish_dupctx(void *ctx)
 }
 
 /* bf_ecb_functions */
-IMPLEMENT_generic_cipher(blowfish, BLOWFISH, ecb, ECB, BF_FLAGS, 128, 64, 0, block)
+IMPLEMENT_var_keylen_cipher(blowfish, BLOWFISH, ecb, ECB, BF_FLAGS, 128, 64, 0, block)
 /* bf_cbc_functions */
-IMPLEMENT_generic_cipher(blowfish, BLOWFISH, cbc, CBC, BF_FLAGS, 128, 64, 64, block)
+IMPLEMENT_var_keylen_cipher(blowfish, BLOWFISH, cbc, CBC, BF_FLAGS, 128, 64, 64, block)
 /* bf_ofb_functions */
-IMPLEMENT_generic_cipher(blowfish, BLOWFISH, ofb64, OFB, BF_FLAGS, 64, 8, 64, stream)
+IMPLEMENT_var_keylen_cipher(blowfish, BLOWFISH, ofb64, OFB, BF_FLAGS, 64, 8, 64, stream)
 /* bf_cfb_functions */
-IMPLEMENT_generic_cipher(blowfish, BLOWFISH, cfb64,  CFB, BF_FLAGS, 64, 8, 64, stream)
+IMPLEMENT_var_keylen_cipher(blowfish, BLOWFISH, cfb64,  CFB, BF_FLAGS, 64, 8, 64, stream)

diff --git a/providers/implementations/ciphers/cipher_cast5.c b/providers/implementations/ciphers/cipher_cast5.c
index d049d836d09bca6d1887d7d9d9ee8377700c6534..473a7f02b9e563fa598e9621a864c4e12f9fe610 100644 (file)
--- a/providers/implementations/ciphers/cipher_cast5.c
+++ b/providers/implementations/ciphers/cipher_cast5.c
@@ -11,6 +11,7 @@
 
 #include "cipher_cast.h"
 #include "prov/implementations.h"
+#include "prov/providercommonerr.h"
 
 #define CAST5_FLAGS (EVP_CIPH_VARIABLE_LENGTH)
 
@@ -39,10 +40,10 @@ static void *cast5_dupctx(void *ctx)
 }
 
 /* cast5128ecb_functions */
-IMPLEMENT_generic_cipher(cast5, CAST, ecb, ECB, CAST5_FLAGS, 128, 64, 0, block)
+IMPLEMENT_var_keylen_cipher(cast5, CAST, ecb, ECB, CAST5_FLAGS, 128, 64, 0, block)
 /* cast5128cbc_functions */
-IMPLEMENT_generic_cipher(cast5, CAST, cbc, CBC, CAST5_FLAGS, 128, 64, 64, block)
+IMPLEMENT_var_keylen_cipher(cast5, CAST, cbc, CBC, CAST5_FLAGS, 128, 64, 64, block)
 /* cast564ofb64_functions */
-IMPLEMENT_generic_cipher(cast5, CAST, ofb64, OFB, CAST5_FLAGS, 64, 8, 64, stream)
+IMPLEMENT_var_keylen_cipher(cast5, CAST, ofb64, OFB, CAST5_FLAGS, 64, 8, 64, stream)
 /* cast564cfb64_functions */
-IMPLEMENT_generic_cipher(cast5, CAST, cfb64,  CFB, CAST5_FLAGS, 64, 8, 64, stream)
+IMPLEMENT_var_keylen_cipher(cast5, CAST, cfb64,  CFB, CAST5_FLAGS, 64, 8, 64, stream)

diff --git a/providers/implementations/ciphers/cipher_rc2.c b/providers/implementations/ciphers/cipher_rc2.c
index 135c6171ec6352c3a77f7b26d0340f7511d7ae56..604c7ed637eb8263078ba2d80a0e46b844745da9 100644 (file)
--- a/providers/implementations/ciphers/cipher_rc2.c
+++ b/providers/implementations/ciphers/cipher_rc2.c
@@ -130,7 +130,7 @@ static int rc2_set_ctx_params(void *vctx, OSSL_PARAM params[])
     PROV_RC2_CTX *ctx = (PROV_RC2_CTX *)vctx;
     const OSSL_PARAM *p;
 
-    if (!cipher_generic_set_ctx_params(vctx, params))
+    if (!cipher_var_keylen_set_ctx_params(vctx, params))
         return 0;
     p = OSSL_PARAM_locate_const(params, OSSL_CIPHER_PARAM_RC2_KEYBITS);
     if (p != NULL) {
@@ -176,6 +176,7 @@ OSSL_PARAM_size_t(OSSL_CIPHER_PARAM_RC2_KEYBITS, NULL),
 CIPHER_DEFAULT_GETTABLE_CTX_PARAMS_END(rc2)
 
 CIPHER_DEFAULT_SETTABLE_CTX_PARAMS_START(rc2)
+OSSL_PARAM_size_t(OSSL_CIPHER_PARAM_KEYLEN, NULL),
 OSSL_PARAM_size_t(OSSL_CIPHER_PARAM_RC2_KEYBITS, NULL),
 CIPHER_DEFAULT_SETTABLE_CTX_PARAMS_END(rc2)
 

diff --git a/providers/implementations/ciphers/cipher_rc4.c b/providers/implementations/ciphers/cipher_rc4.c
index c3da8b439733ad7632f678cad1b2795afeb7c6df..baf34f7b9393cf400631e9169d2ea096410cad3a 100644 (file)
--- a/providers/implementations/ciphers/cipher_rc4.c
+++ b/providers/implementations/ciphers/cipher_rc4.c
@@ -71,13 +71,13 @@ const OSSL_DISPATCH alg##kbits##_functions[] = {                               \
     { OSSL_FUNC_CIPHER_GET_CTX_PARAMS,                                         \
       (void (*)(void))cipher_generic_get_ctx_params },                         \
     { OSSL_FUNC_CIPHER_SET_CTX_PARAMS,                                         \
-      (void (*)(void))cipher_generic_set_ctx_params },                         \
+      (void (*)(void))cipher_var_keylen_set_ctx_params },                      \
     { OSSL_FUNC_CIPHER_GETTABLE_PARAMS,                                        \
       (void (*)(void))cipher_generic_gettable_params },                        \
     { OSSL_FUNC_CIPHER_GETTABLE_CTX_PARAMS,                                    \
       (void (*)(void))cipher_generic_gettable_ctx_params },                    \
     { OSSL_FUNC_CIPHER_SETTABLE_CTX_PARAMS,                                    \
-     (void (*)(void))cipher_generic_settable_ctx_params },                     \
+     (void (*)(void))cipher_var_keylen_settable_ctx_params },                  \
     { 0, NULL }                                                                \
 };
 

diff --git a/providers/implementations/ciphers/cipher_rc5.c b/providers/implementations/ciphers/cipher_rc5.c
index 7f5780f0626130d568b975dfe9548161986edda3..e2e1cb6a31007e848d3217865a338c877e841676 100644 (file)
--- a/providers/implementations/ciphers/cipher_rc5.c
+++ b/providers/implementations/ciphers/cipher_rc5.c
@@ -44,7 +44,7 @@ static int rc5_set_ctx_params(void *vctx, const OSSL_PARAM params[])
     PROV_RC5_CTX *ctx = (PROV_RC5_CTX *)vctx;
     const OSSL_PARAM *p;
 
-    if (!cipher_generic_set_ctx_params(vctx, params))
+    if (!cipher_var_keylen_set_ctx_params(vctx, params))
         return 0;
 
     p = OSSL_PARAM_locate_const(params, OSSL_CIPHER_PARAM_ROUNDS);
@@ -71,6 +71,7 @@ CIPHER_DEFAULT_GETTABLE_CTX_PARAMS_START(rc5)
 CIPHER_DEFAULT_GETTABLE_CTX_PARAMS_END(rc5)
 
 CIPHER_DEFAULT_SETTABLE_CTX_PARAMS_START(rc5)
+    OSSL_PARAM_size_t(OSSL_CIPHER_PARAM_KEYLEN, NULL),
     OSSL_PARAM_uint(OSSL_CIPHER_PARAM_ROUNDS, NULL),
 CIPHER_DEFAULT_SETTABLE_CTX_PARAMS_END(rc5)