Blame SOURCES/0090-signature-Clamp-PSS-salt-len-to-MD-len.patch

3da501
From 9cc914ff3e1fda124bdc76d72ebc9349ec19f8ae Mon Sep 17 00:00:00 2001
3da501
From: Clemens Lang <cllang@redhat.com>
3da501
Date: Fri, 18 Nov 2022 12:35:33 +0100
3da501
Subject: [PATCH 3/3] signature: Clamp PSS salt len to MD len
3da501
MIME-Version: 1.0
3da501
Content-Type: text/plain; charset=UTF-8
3da501
Content-Transfer-Encoding: 8bit
3da501
3da501
FIPS 186-4 section 5 "The RSA Digital Signature Algorithm", subsection
3da501
5.5 "PKCS #1" says: "For RSASSA-PSS […] the length (in bytes) of the
3da501
salt (sLen) shall satisfy 0 <= sLen <= hLen, where hLen is the length of
3da501
the hash function output block (in bytes)."
3da501
3da501
Introduce a new option RSA_PSS_SALTLEN_AUTO_DIGEST_MAX and make it the
3da501
default. The new value will behave like RSA_PSS_SALTLEN_AUTO, but will
3da501
not use more than the digest legth when signing, so that FIPS 186-4 is
3da501
not violated. This value has two advantages when compared with
3da501
RSA_PSS_SALTLEN_DIGEST: (1) It will continue to do auto-detection when
3da501
verifying signatures for maximum compatibility, where
3da501
RSA_PSS_SALTLEN_DIGEST would fail for other digest sizes. (2) It will
3da501
work for combinations where the maximum salt length is smaller than the
3da501
digest size, which typically happens with large digest sizes (e.g.,
3da501
SHA-512) and small RSA keys.
3da501
3da501
Signed-off-by: Clemens Lang <cllang@redhat.com>
3da501
---
3da501
 crypto/rsa/rsa_ameth.c                        | 18 ++++++++-
3da501
 crypto/rsa/rsa_pss.c                          | 26 ++++++++++--
3da501
 doc/man3/EVP_PKEY_CTX_ctrl.pod                | 11 ++++-
3da501
 doc/man7/EVP_SIGNATURE-RSA.pod                |  5 +++
3da501
 include/openssl/core_names.h                  |  1 +
3da501
 include/openssl/rsa.h                         |  3 ++
3da501
 providers/implementations/signature/rsa_sig.c | 40 ++++++++++++++-----
3da501
 test/recipes/25-test_req.t                    |  2 +-
3da501
 8 files changed, 87 insertions(+), 19 deletions(-)
3da501
3da501
diff --git a/crypto/rsa/rsa_ameth.c b/crypto/rsa/rsa_ameth.c
3da501
index 61ec53d424..e69a98d116 100644
3da501
--- a/crypto/rsa/rsa_ameth.c
3da501
+++ b/crypto/rsa/rsa_ameth.c
3da501
@@ -450,6 +450,7 @@ static RSA_PSS_PARAMS *rsa_ctx_to_pss(EVP_PKEY_CTX *pkctx)
3da501
     const EVP_MD *sigmd, *mgf1md;
3da501
     EVP_PKEY *pk = EVP_PKEY_CTX_get0_pkey(pkctx);
3da501
     int saltlen;
3da501
+    int saltlenMax = -1;
3da501
 
3da501
     if (EVP_PKEY_CTX_get_signature_md(pkctx, &sigmd) <= 0)
3da501
         return NULL;
3da501
@@ -457,14 +458,27 @@ static RSA_PSS_PARAMS *rsa_ctx_to_pss(EVP_PKEY_CTX *pkctx)
3da501
         return NULL;
d8c783
     if (EVP_PKEY_CTX_get_rsa_pss_saltlen(pkctx, &saltlen) <= 0)
3da501
         return NULL;
3da501
-    if (saltlen == -1) {
3da501
+    if (saltlen == RSA_PSS_SALTLEN_DIGEST) {
3da501
         saltlen = EVP_MD_get_size(sigmd);
3da501
-    } else if (saltlen == -2 || saltlen == -3) {
3da501
+    } else if (saltlen == RSA_PSS_SALTLEN_AUTO_DIGEST_MAX) {
3da501
+        /* FIPS 186-4 section 5 "The RSA Digital Signature Algorithm",
3da501
+         * subsection 5.5 "PKCS #1" says: "For RSASSA-PSS […] the length (in
3da501
+         * bytes) of the salt (sLen) shall satisfy 0 <= sLen <= hLen, where
3da501
+         * hLen is the length of the hash function output block (in bytes)."
3da501
+         *
3da501
+         * Provide a way to use at most the digest length, so that the default
3da501
+         * does not violate FIPS 186-4. */
3da501
+        saltlen = RSA_PSS_SALTLEN_MAX;
3da501
+        saltlenMax = EVP_MD_get_size(sigmd);
3da501
+    }
3da501
+    if (saltlen == RSA_PSS_SALTLEN_MAX || saltlen == RSA_PSS_SALTLEN_AUTO) {
3da501
         saltlen = EVP_PKEY_get_size(pk) - EVP_MD_get_size(sigmd) - 2;
3da501
         if ((EVP_PKEY_get_bits(pk) & 0x7) == 1)
3da501
             saltlen--;
3da501
         if (saltlen < 0)
3da501
             return NULL;
3da501
+        if (saltlenMax >= 0 && saltlen > saltlenMax)
3da501
+            saltlen = saltlenMax;
3da501
     }
3da501
 
3da501
     return ossl_rsa_pss_params_create(sigmd, mgf1md, saltlen);
3da501
diff --git a/crypto/rsa/rsa_pss.c b/crypto/rsa/rsa_pss.c
3da501
index 33874bfef8..430c36eb2a 100644
3da501
--- a/crypto/rsa/rsa_pss.c
3da501
+++ b/crypto/rsa/rsa_pss.c
3da501
@@ -61,11 +61,12 @@ int RSA_verify_PKCS1_PSS_mgf1(RSA *rsa, const unsigned char *mHash,
3da501
      *      -1      sLen == hLen
3da501
      *      -2      salt length is autorecovered from signature
3da501
      *      -3      salt length is maximized
3da501
+     *      -4      salt length is autorecovered from signature
3da501
      *      -N      reserved
3da501
      */
3da501
     if (sLen == RSA_PSS_SALTLEN_DIGEST) {
3da501
         sLen = hLen;
3da501
-    } else if (sLen < RSA_PSS_SALTLEN_MAX) {
3da501
+    } else if (sLen < RSA_PSS_SALTLEN_AUTO_DIGEST_MAX) {
3da501
         ERR_raise(ERR_LIB_RSA, RSA_R_SLEN_CHECK_FAILED);
3da501
         goto err;
3da501
     }
3da501
@@ -112,7 +113,9 @@ int RSA_verify_PKCS1_PSS_mgf1(RSA *rsa, const unsigned char *mHash,
3da501
         ERR_raise(ERR_LIB_RSA, RSA_R_SLEN_RECOVERY_FAILED);
3da501
         goto err;
3da501
     }
3da501
-    if (sLen != RSA_PSS_SALTLEN_AUTO && (maskedDBLen - i) != sLen) {
3da501
+    if (sLen != RSA_PSS_SALTLEN_AUTO
3da501
+            && sLen != RSA_PSS_SALTLEN_AUTO_DIGEST_MAX
3da501
+            && (maskedDBLen - i) != sLen) {
3da501
         ERR_raise_data(ERR_LIB_RSA, RSA_R_SLEN_CHECK_FAILED,
3da501
                        "expected: %d retrieved: %d", sLen,
3da501
                        maskedDBLen - i);
3da501
@@ -160,6 +163,7 @@ int RSA_padding_add_PKCS1_PSS_mgf1(RSA *rsa, unsigned char *EM,
3da501
     int hLen, maskedDBLen, MSBits, emLen;
3da501
     unsigned char *H, *salt = NULL, *p;
3da501
     EVP_MD_CTX *ctx = NULL;
3da501
+    int sLenMax = -1;
3da501
 
3da501
     if (mgf1Hash == NULL)
3da501
         mgf1Hash = Hash;
3da501
@@ -172,13 +176,25 @@ int RSA_padding_add_PKCS1_PSS_mgf1(RSA *rsa, unsigned char *EM,
3da501
      *      -1      sLen == hLen
3da501
      *      -2      salt length is maximized
3da501
      *      -3      same as above (on signing)
3da501
+     *      -4      salt length is min(hLen, maximum salt length)
3da501
      *      -N      reserved
3da501
      */
3da501
+    /* FIPS 186-4 section 5 "The RSA Digital Signature Algorithm", subsection
3da501
+     * 5.5 "PKCS #1" says: "For RSASSA-PSS […] the length (in bytes) of the
3da501
+     * salt (sLen) shall satisfy 0 <= sLen <= hLen, where hLen is the length of
3da501
+     * the hash function output block (in bytes)."
3da501
+     *
3da501
+     * Provide a way to use at most the digest length, so that the default does
3da501
+     * not violate FIPS 186-4. */
3da501
     if (sLen == RSA_PSS_SALTLEN_DIGEST) {
3da501
         sLen = hLen;
3da501
-    } else if (sLen == RSA_PSS_SALTLEN_MAX_SIGN) {
3da501
+    } else if (sLen == RSA_PSS_SALTLEN_MAX_SIGN
3da501
+            || sLen == RSA_PSS_SALTLEN_AUTO) {
3da501
         sLen = RSA_PSS_SALTLEN_MAX;
3da501
-    } else if (sLen < RSA_PSS_SALTLEN_MAX) {
3da501
+    } else if (sLen == RSA_PSS_SALTLEN_AUTO_DIGEST_MAX) {
3da501
+        sLen = RSA_PSS_SALTLEN_MAX;
3da501
+        sLenMax = hLen;
3da501
+    } else if (sLen < RSA_PSS_SALTLEN_AUTO_DIGEST_MAX) {
3da501
         ERR_raise(ERR_LIB_RSA, RSA_R_SLEN_CHECK_FAILED);
3da501
         goto err;
3da501
     }
3da501
@@ -195,6 +211,8 @@ int RSA_padding_add_PKCS1_PSS_mgf1(RSA *rsa, unsigned char *EM,
3da501
     }
3da501
     if (sLen == RSA_PSS_SALTLEN_MAX) {
3da501
         sLen = emLen - hLen - 2;
3da501
+        if (sLenMax >= 0 && sLen > sLenMax)
3da501
+            sLen = sLenMax;
3da501
     } else if (sLen > emLen - hLen - 2) {
3da501
         ERR_raise(ERR_LIB_RSA, RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE);
3da501
         goto err;
3da501
diff --git a/doc/man3/EVP_PKEY_CTX_ctrl.pod b/doc/man3/EVP_PKEY_CTX_ctrl.pod
3da501
index 3075eaafd6..9b96f42dbc 100644
3da501
--- a/doc/man3/EVP_PKEY_CTX_ctrl.pod
3da501
+++ b/doc/man3/EVP_PKEY_CTX_ctrl.pod
3da501
@@ -270,8 +270,8 @@ EVP_PKEY_CTX_get_rsa_padding() gets the RSA padding mode for I<ctx>.
3da501
 
3da501
 EVP_PKEY_CTX_set_rsa_pss_saltlen() sets the RSA PSS salt length to I<saltlen>.
3da501
 As its name implies it is only supported for PSS padding. If this function is
3da501
-not called then the maximum salt length is used when signing and auto detection
3da501
-when verifying. Three special values are supported:
3da501
+not called then the salt length is maximized up to the digest length when
3da501
+signing and auto detection when verifying. Four special values are supported:
3da501
 
3da501
 =over 4
3da501
 
3da501
@@ -289,6 +289,13 @@ causes the salt length to be automatically determined based on the
3da501
 B<PSS> block structure when verifying.  When signing, it has the same
3da501
 meaning as B<RSA_PSS_SALTLEN_MAX>.
3da501
 
3da501
+=item B<RSA_PSS_SALTLEN_AUTO_DIGEST_MAX>
3da501
+
3da501
+causes the salt length to be automatically determined based on the B<PSS> block
3da501
+structure when verifying, like B<RSA_PSS_SALTLEN_AUTO>.  When signing, the salt
3da501
+length is maximized up to a maximum of the digest length to comply with FIPS
3da501
+186-4 section 5.5.
3da501
+
3da501
 =back
3da501
 
3da501
 EVP_PKEY_CTX_get_rsa_pss_saltlen() gets the RSA PSS salt length for I<ctx>.
3da501
diff --git a/doc/man7/EVP_SIGNATURE-RSA.pod b/doc/man7/EVP_SIGNATURE-RSA.pod
3da501
index 1ce32cc443..13d053e262 100644
3da501
--- a/doc/man7/EVP_SIGNATURE-RSA.pod
3da501
+++ b/doc/man7/EVP_SIGNATURE-RSA.pod
3da501
@@ -68,6 +68,11 @@ Use the maximum salt length.
3da501
 
3da501
 Auto detect the salt length.
3da501
 
3da501
+=item "auto-digestmax" (B<OSSL_PKEY_RSA_PSS_SALT_LEN_AUTO_DIGEST_MAX>)
3da501
+
3da501
+Auto detect the salt length when verifying.  Maximize the salt length up to the
3da501
+digest size when signing to comply with FIPS 186-4 section 5.5.
3da501
+
3da501
 =back
3da501
 
3da501
 =back
3da501
diff --git a/include/openssl/core_names.h b/include/openssl/core_names.h
3da501
index 69c59f0b46..5779f41427 100644
3da501
--- a/include/openssl/core_names.h
3da501
+++ b/include/openssl/core_names.h
3da501
@@ -399,6 +399,7 @@ extern "C" {
3da501
 #define OSSL_PKEY_RSA_PSS_SALT_LEN_DIGEST "digest"
3da501
 #define OSSL_PKEY_RSA_PSS_SALT_LEN_MAX    "max"
3da501
 #define OSSL_PKEY_RSA_PSS_SALT_LEN_AUTO   "auto"
3da501
+#define OSSL_PKEY_RSA_PSS_SALT_LEN_AUTO_DIGEST_MAX "auto-digestmax"
3da501
 
3da501
 /* Key generation parameters */
3da501
 #define OSSL_PKEY_PARAM_RSA_BITS             OSSL_PKEY_PARAM_BITS
3da501
diff --git a/include/openssl/rsa.h b/include/openssl/rsa.h
3da501
index a55c9727c6..daf55bc6d4 100644
3da501
--- a/include/openssl/rsa.h
3da501
+++ b/include/openssl/rsa.h
3da501
@@ -137,6 +137,9 @@ int EVP_PKEY_CTX_set_rsa_keygen_pubexp(EVP_PKEY_CTX *ctx, BIGNUM *pubexp);
3da501
 # define RSA_PSS_SALTLEN_AUTO   -2
3da501
 /* Set salt length to maximum possible */
3da501
 # define RSA_PSS_SALTLEN_MAX    -3
3da501
+/* Auto-detect on verify, set salt length to min(maximum possible, digest
3da501
+ * length) on sign */
3da501
+# define RSA_PSS_SALTLEN_AUTO_DIGEST_MAX  -4
3da501
 /* Old compatible max salt length for sign only */
3da501
 # define RSA_PSS_SALTLEN_MAX_SIGN    -2
3da501
 
3da501
diff --git a/providers/implementations/signature/rsa_sig.c b/providers/implementations/signature/rsa_sig.c
3da501
index 0c45008a00..1a787d77db 100644
3da501
--- a/providers/implementations/signature/rsa_sig.c
3da501
+++ b/providers/implementations/signature/rsa_sig.c
3da501
@@ -191,8 +191,8 @@ static void *rsa_newctx(void *provctx, const char *propq)
3da501
     prsactx->libctx = PROV_LIBCTX_OF(provctx);
3da501
     prsactx->flag_allow_md = 1;
3da501
     prsactx->propq = propq_copy;
3da501
-    /* Maximum for sign, auto for verify */
3da501
-    prsactx->saltlen = RSA_PSS_SALTLEN_AUTO;
3da501
+    /* Maximum up to digest length for sign, auto for verify */
3da501
+    prsactx->saltlen = RSA_PSS_SALTLEN_AUTO_DIGEST_MAX;
3da501
     prsactx->min_saltlen = -1;
3da501
     return prsactx;
3da501
 }
3da501
@@ -200,13 +200,27 @@ static void *rsa_newctx(void *provctx, const char *propq)
3da501
 static int rsa_pss_compute_saltlen(PROV_RSA_CTX *ctx)
3da501
 {
3da501
     int saltlen = ctx->saltlen;
3da501
- 
3da501
+    int saltlenMax = -1;
3da501
+
3da501
+    /* FIPS 186-4 section 5 "The RSA Digital Signature Algorithm", subsection
3da501
+     * 5.5 "PKCS #1" says: "For RSASSA-PSS […] the length (in bytes) of the
3da501
+     * salt (sLen) shall satisfy 0 <= sLen <= hLen, where hLen is the length of
3da501
+     * the hash function output block (in bytes)."
3da501
+     *
3da501
+     * Provide a way to use at most the digest length, so that the default does
3da501
+     * not violate FIPS 186-4. */
3da501
     if (saltlen == RSA_PSS_SALTLEN_DIGEST) {
3da501
         saltlen = EVP_MD_get_size(ctx->md);
3da501
-    } else if (saltlen == RSA_PSS_SALTLEN_AUTO || saltlen == RSA_PSS_SALTLEN_MAX) {
3da501
+    } else if (saltlen == RSA_PSS_SALTLEN_AUTO_DIGEST_MAX) {
3da501
+        saltlen = RSA_PSS_SALTLEN_MAX;
3da501
+        saltlenMax = EVP_MD_get_size(ctx->md);
3da501
+    }
3da501
+    if (saltlen == RSA_PSS_SALTLEN_MAX || saltlen == RSA_PSS_SALTLEN_AUTO) {
3da501
         saltlen = RSA_size(ctx->rsa) - EVP_MD_get_size(ctx->md) - 2;
3da501
         if ((RSA_bits(ctx->rsa) & 0x7) == 1)
3da501
             saltlen--;
3da501
+        if (saltlenMax >= 0 && saltlen > saltlenMax)
3da501
+            saltlen = saltlenMax;
3da501
     }
3da501
     if (saltlen < 0) {
3da501
         ERR_raise(ERR_LIB_PROV, ERR_R_INTERNAL_ERROR);
3da501
@@ -411,8 +425,8 @@ static int rsa_signverify_init(void *vprsactx, void *vrsa,
3da501
 
3da501
     prsactx->operation = operation;
3da501
 
3da501
-    /* Maximum for sign, auto for verify */
3da501
-    prsactx->saltlen = RSA_PSS_SALTLEN_AUTO;
3da501
+    /* Maximize up to digest length for sign, auto for verify */
3da501
+    prsactx->saltlen = RSA_PSS_SALTLEN_AUTO_DIGEST_MAX;
3da501
     prsactx->min_saltlen = -1;
3da501
 
3da501
     switch (RSA_test_flags(prsactx->rsa, RSA_FLAG_TYPE_MASK)) {
3da501
@@ -1110,6 +1124,9 @@ static int rsa_get_ctx_params(void *vprsactx, OSSL_PARAM *params)
3da501
             case RSA_PSS_SALTLEN_AUTO:
3da501
                 value = OSSL_PKEY_RSA_PSS_SALT_LEN_AUTO;
3da501
                 break;
3da501
+            case RSA_PSS_SALTLEN_AUTO_DIGEST_MAX:
3da501
+                value = OSSL_PKEY_RSA_PSS_SALT_LEN_AUTO_DIGEST_MAX;
3da501
+                break;
3da501
             default:
3da501
                 {
3da501
                     int len = BIO_snprintf(p->data, p->data_size, "%d",
3da501
@@ -1297,6 +1314,8 @@ static int rsa_set_ctx_params(void *vprsactx, const OSSL_PARAM params[])
3da501
                 saltlen = RSA_PSS_SALTLEN_MAX;
3da501
             else if (strcmp(p->data, OSSL_PKEY_RSA_PSS_SALT_LEN_AUTO) == 0)
3da501
                 saltlen = RSA_PSS_SALTLEN_AUTO;
3da501
+            else if (strcmp(p->data, OSSL_PKEY_RSA_PSS_SALT_LEN_AUTO_DIGEST_MAX) == 0)
3da501
+                saltlen = RSA_PSS_SALTLEN_AUTO_DIGEST_MAX;
3da501
             else
3da501
                 saltlen = atoi(p->data);
3da501
             break;
3da501
@@ -1305,11 +1324,11 @@ static int rsa_set_ctx_params(void *vprsactx, const OSSL_PARAM params[])
3da501
         }
3da501
 
3da501
         /*
3da501
-         * RSA_PSS_SALTLEN_MAX seems curiously named in this check.
3da501
-         * Contrary to what it's name suggests, it's the currently
3da501
-         * lowest saltlen number possible.
3da501
+         * RSA_PSS_SALTLEN_AUTO_DIGEST_MAX seems curiously named in this check.
3da501
+         * Contrary to what it's name suggests, it's the currently lowest
3da501
+         * saltlen number possible.
3da501
          */
3da501
-        if (saltlen < RSA_PSS_SALTLEN_MAX) {
3da501
+        if (saltlen < RSA_PSS_SALTLEN_AUTO_DIGEST_MAX) {
3da501
             ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_SALT_LENGTH);
3da501
             return 0;
3da501
         }
3da501
@@ -1317,6 +1336,7 @@ static int rsa_set_ctx_params(void *vprsactx, const OSSL_PARAM params[])
3da501
         if (rsa_pss_restricted(prsactx)) {
3da501
             switch (saltlen) {
3da501
             case RSA_PSS_SALTLEN_AUTO:
3da501
+            case RSA_PSS_SALTLEN_AUTO_DIGEST_MAX:
3da501
                 if (prsactx->operation == EVP_PKEY_OP_VERIFY) {
3da501
                     ERR_raise_data(ERR_LIB_PROV, PROV_R_INVALID_SALT_LENGTH,
3da501
                                    "Cannot use autodetected salt length");
3da501
diff --git a/test/recipes/25-test_req.t b/test/recipes/25-test_req.t
3da501
index e615f1b338..35541aed12 100644
3da501
--- a/test/recipes/25-test_req.t
3da501
+++ b/test/recipes/25-test_req.t
3da501
@@ -199,7 +199,7 @@ subtest "generating certificate requests with RSA-PSS" => sub {
3da501
         ok(!run(app(["openssl", "req",
3da501
                      "-config", srctop_file("test", "test.cnf"),
3da501
                      "-new", "-out", "testreq-rsapss3.pem", "-utf8",
3da501
-                     "-sigopt", "rsa_pss_saltlen:-4",
3da501
+                     "-sigopt", "rsa_pss_saltlen:-5",
3da501
                      "-key", srctop_file("test", "testrsapss.pem")])),
3da501
            "Generating request with expected failure");
3da501
 
3da501
-- 
3da501
2.38.1
3da501