Obtain PSS salt length from provider

Rather than computing the PSS salt length again in core using ossl_rsa_ctx_to_pss_string, which calls rsa_ctx_to_pss and computes the salt length, obtain it from the provider using the OSSL_SIGNATURE_PARAM_ALGORITHM_ID param to handle the case where the interpretation of the magic constants in the provider differs from that of OpenSSL core. Add tests that verify that the rsa_pss_saltlen:max, rsa_pss_saltlen:<integer> and rsa_pss_saltlen:digest options work and put the computed digest length into the CMS_ContentInfo struct when using CMS. Do not add a test for the salt length generated by a provider when no specific rsa_pss_saltlen option is defined, since that number could change between providers and provider versions, and we want to preserve compatibility with older providers. Signed-off-by: Clemens Lang <cllang@redhat.com> Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#19724)
neverpanic · Dec 8, 2022 · 5a3bbe1 · 5a3bbe1
1 parent cae72ee
commit 5a3bbe1
Show file tree

Hide file tree

Showing 4 changed files with 122 additions and 27 deletions.
diff --git a/crypto/cms/cms_rsa.c b/crypto/cms/cms_rsa.c
@@ -10,6 +10,7 @@
 #include <assert.h>
 #include <openssl/cms.h>
 #include <openssl/err.h>
+#include <openssl/core_names.h>
 #include "crypto/asn1.h"
 #include "crypto/rsa.h"
 #include "cms_local.h"
@@ -190,7 +191,10 @@ static int rsa_cms_sign(CMS_SignerInfo *si)
     int pad_mode = RSA_PKCS1_PADDING;
     X509_ALGOR *alg;
     EVP_PKEY_CTX *pkctx = CMS_SignerInfo_get0_pkey_ctx(si);
-    ASN1_STRING *os = NULL;
+    unsigned char aid[128];
+    const unsigned char *pp = aid;
+    size_t aid_len = 0;
+    OSSL_PARAM params[2];
 
     CMS_SignerInfo_get0_algs(si, NULL, NULL, NULL, &alg);
     if (pkctx != NULL) {
@@ -204,14 +208,18 @@ static int rsa_cms_sign(CMS_SignerInfo *si)
     /* We don't support it */
     if (pad_mode != RSA_PKCS1_PSS_PADDING)
         return 0;
-    os = ossl_rsa_ctx_to_pss_string(pkctx);
-    if (os == NULL)
+
+    params[0] = OSSL_PARAM_construct_octet_string(
+        OSSL_SIGNATURE_PARAM_ALGORITHM_ID, aid, sizeof(aid));
+    params[1] = OSSL_PARAM_construct_end();
+
+    if (EVP_PKEY_CTX_get_params(pkctx, params) <= 0)
         return 0;
-    if (X509_ALGOR_set0(alg, OBJ_nid2obj(EVP_PKEY_RSA_PSS),
-                        V_ASN1_SEQUENCE, os))
-        return 1;
-    ASN1_STRING_free(os);
-    return 0;
+    if ((aid_len = params[0].return_size) == 0)
+        return 0;
+    if (d2i_X509_ALGOR(&alg, &pp, aid_len) == NULL)
+        return 0;
+    return 1;
 }
 
 static int rsa_cms_verify(CMS_SignerInfo *si)

diff --git a/crypto/rsa/rsa_ameth.c b/crypto/rsa/rsa_ameth.c
@@ -637,29 +637,31 @@ static int rsa_item_sign(EVP_MD_CTX *ctx, const ASN1_ITEM *it, const void *asn,
     if (pad_mode == RSA_PKCS1_PADDING)
         return 2;
     if (pad_mode == RSA_PKCS1_PSS_PADDING) {
-        ASN1_STRING *os1 = ossl_rsa_ctx_to_pss_string(pkctx);
+        unsigned char aid[128];
+        size_t aid_len = 0;
+        OSSL_PARAM params[2];
 
-        if (os1 == NULL)
+        params[0] = OSSL_PARAM_construct_octet_string(
+            OSSL_SIGNATURE_PARAM_ALGORITHM_ID, aid, sizeof(aid));
+        params[1] = OSSL_PARAM_construct_end();
+
+        if (EVP_PKEY_CTX_get_params(pkctx, params) <= 0)
+            return 0;
+        if ((aid_len = params[0].return_size) == 0)
             return 0;
-        /* Duplicate parameters if we have to */
-        if (alg2 != NULL) {
-            ASN1_STRING *os2 = ASN1_STRING_dup(os1);
 
-            if (os2 == NULL)
-                goto err;
-            if (!X509_ALGOR_set0(alg2, OBJ_nid2obj(EVP_PKEY_RSA_PSS),
-                                 V_ASN1_SEQUENCE, os2)) {
-                ASN1_STRING_free(os2);
-                goto err;
-            }
+        if (alg1 != NULL) {
+            const unsigned char *pp = aid;
+            if (d2i_X509_ALGOR(&alg1, &pp, aid_len) == NULL)
+                return 0;
         }
-        if (!X509_ALGOR_set0(alg1, OBJ_nid2obj(EVP_PKEY_RSA_PSS),
-                             V_ASN1_SEQUENCE, os1))
-            goto err;
+        if (alg2 != NULL) {
+            const unsigned char *pp = aid;
+            if (d2i_X509_ALGOR(&alg2, &pp, aid_len) == NULL)
+                return 0;
+        }
+
         return 3;
-    err:
-        ASN1_STRING_free(os1);
-        return 0;
     }
     return 2;
 }

diff --git a/test/recipes/15-test_rsapss.t b/test/recipes/15-test_rsapss.t
@@ -16,7 +16,7 @@ use OpenSSL::Test::Utils;
 
 setup("test_rsapss");
 
-plan tests => 10;
+plan tests => 11;
 
 #using test/testrsa.pem which happens to be a 512 bit RSA
 ok(run(app(['openssl', 'dgst', '-sign', srctop_file('test', 'testrsa.pem'), '-sha1',
@@ -58,6 +58,15 @@ ok(run(app(['openssl', 'dgst', '-prverify', srctop_file('test', 'testrsa.pem'),
             srctop_file('test', 'testrsa.pem')])),
    "openssl dgst -prverify [plain RSA key, PSS padding mode, PSS restrictions]");
 
+ok(run(app(['openssl', 'dgst', '-prverify', srctop_file('test', 'testrsa.pem'),
+            '-sha1',
+            '-sigopt', 'rsa_padding_mode:pss',
+            '-sigopt', 'rsa_pss_saltlen:42',
+            '-sigopt', 'rsa_mgf1_md:sha512',
+            '-signature', 'testrsapss-restricted.sig',
+            srctop_file('test', 'testrsa.pem')])),
+   "openssl dgst -sign rsa512bit.pem -sha1 -sigopt rsa_pss_saltlen:max produces 42 bits of PSS salt");
+
 ok(run(app(['openssl', 'dgst', '-prverify', srctop_file('test', 'testrsa.pem'),
             '-sha1',
             '-sigopt', 'rsa_padding_mode:pss',

diff --git a/test/recipes/80-test_cms.t b/test/recipes/80-test_cms.t
@@ -64,6 +64,7 @@ my @prov = ("-provider-path", $provpath,
             @config,
             "-provider", $provname);
 
+my $smrsa1024 = catfile($smdir, "smrsa1024.pem");
 my $smrsa1 = catfile($smdir, "smrsa1.pem");
 my $smroot = catfile($smdir, "smroot.pem");
 
@@ -498,6 +499,7 @@ my @smime_cms_param_tests = (
         "-signer", $smrsa1,
         "-keyopt", "rsa_padding_mode:pss", "-keyopt", "rsa_pss_saltlen:max",
         "-out", "{output}.cms" ],
+      sub { my %opts = @_; rsapssSaltlen("$opts{output}.cms") == 222; },
       [ "{cmd2}", @prov, "-verify", "-in", "{output}.cms", "-inform", "PEM",
         "-CAfile", $smroot, "-out", "{output}.txt" ],
       \&final_compare
@@ -523,6 +525,29 @@ my @smime_cms_param_tests = (
       \&final_compare
     ],
 
+    [ "signed content test streaming PEM format, RSA keys, PSS signature, saltlen=16",
+      [ "{cmd1}", @prov, "-sign", "-in", $smcont, "-outform", "PEM", "-nodetach",
+        "-signer", $smrsa1, "-md", "sha256",
+        "-keyopt", "rsa_padding_mode:pss", "-keyopt", "rsa_pss_saltlen:16",
+        "-out", "{output}.cms" ],
+      sub { my %opts = @_; rsapssSaltlen("$opts{output}.cms") == 16; },
+      [ "{cmd2}", @prov, "-verify", "-in", "{output}.cms", "-inform", "PEM",
+        "-CAfile", $smroot, "-out", "{output}.txt" ],
+      \&final_compare
+    ],
+
+    [ "signed content test streaming PEM format, RSA keys, PSS signature, saltlen=digest",
+      [ "{cmd1}", @prov, "-sign", "-in", $smcont, "-outform", "PEM", "-nodetach",
+        "-signer", $smrsa1, "-md", "sha256",
+        "-keyopt", "rsa_padding_mode:pss", "-keyopt", "rsa_pss_saltlen:digest",
+        "-out", "{output}.cms" ],
+      # digest is SHA-256, which produces 32 bytes of output
+      sub { my %opts = @_; rsapssSaltlen("$opts{output}.cms") == 32; },
+      [ "{cmd2}", @prov, "-verify", "-in", "{output}.cms", "-inform", "PEM",
+        "-CAfile", $smroot, "-out", "{output}.txt" ],
+      \&final_compare
+    ],
+
     [ "enveloped content test streaming S/MIME format, DES, OAEP default parameters",
       [ "{cmd1}", @defaultprov, "-encrypt", "-in", $smcont,
         "-stream", "-out", "{output}.cms",
@@ -738,6 +763,57 @@ sub contentType_matches {
   return scalar(@c);
 }
 
+sub rsapssSaltlen {
+  my ($in) = @_;
+  my $exit = 0;
+
+  my @asn1parse = run(app(["openssl", "asn1parse", "-in", $in, "-dump"]),
+                      capture => 1,
+                      statusvar => $exit);
+  return -1 if $exit != 0;
+
+  my $pssparam_offset = -1;
+  while ($_ = shift @asn1parse) {
+    chomp;
+    next unless /:rsassaPss$/;
+    # This line contains :rsassaPss, the next line contains a raw dump of the
+    # RSA_PSS_PARAMS sequence; obtain its offset
+    $_ = shift @asn1parse;
+    if (/^\s*(\d+):/) {
+      $pssparam_offset = int($1);
+    }
+  }
+
+  if ($pssparam_offset == -1) {
+    note "Failed to determine RSA_PSS_PARAM offset in CMS. " +
+         "Was the file correctly signed with RSASSA-PSS?";
+    return -1;
+  }
+
+  my @pssparam = run(app(["openssl", "asn1parse", "-in", $in,
+                          "-strparse", $pssparam_offset]),
+                     capture => 1,
+                     statusvar => $exit);
+  return -1 if $exit != 0;
+
+  my $saltlen = -1;
+  # Can't use asn1parse -item RSA_PSS_PARAMS here, because that's deprecated.
+  # This assumes the salt length is the last field, which may possibly be
+  # incorrect if there is a non-standard trailer field, but there almost never
+  # is in PSS.
+  if ($pssparam[-1] =~ /prim:\s+INTEGER\s+:([A-Fa-f0-9]+)$/) {
+    $saltlen = hex($1);
+  }
+
+  if ($saltlen == -1) {
+    note "Failed to determine salt length from RSA_PSS_PARAM struct. " +
+         "Was the file correctly signed with RSASSA-PSS?";
+    return -1;
+  }
+
+  return $saltlen;
+}
+
 subtest "CMS Check the content type attribute is added for additional signers\n" => sub {
     plan tests => (scalar @contenttype_cms_test);