diff options
author | Clemens Lang <cllang@redhat.com> | 2022-11-21 14:33:57 +0100 |
---|---|---|
committer | Tomas Mraz <tomas@openssl.org> | 2022-12-08 11:02:52 +0100 |
commit | 5a3bbe1712435d577bbc5ec046906979e8471d8b (patch) | |
tree | 0baeafcfd65f2db8dc64c27689f3b63d51421ef2 | |
parent | cae72eefc3fbdd2f7a1a065f237bf3943619bca2 (diff) |
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 https://github.com/openssl/openssl/pull/19724)
-rw-r--r-- | crypto/cms/cms_rsa.c | 24 | ||||
-rw-r--r-- | crypto/rsa/rsa_ameth.c | 38 | ||||
-rw-r--r-- | test/recipes/15-test_rsapss.t | 11 | ||||
-rw-r--r-- | test/recipes/80-test_cms.t | 76 |
4 files changed, 122 insertions, 27 deletions
diff --git a/crypto/cms/cms_rsa.c b/crypto/cms/cms_rsa.c index 0675369192..e997e6eec1 100644 --- 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 index 03bbeecee0..08207184ed 100644 --- 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 index aba7e16b8f..c566ade933 100644 --- 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', @@ -61,6 +61,15 @@ ok(run(app(['openssl', 'dgst', '-prverify', srctop_file('test', 'testrsa.pem'), 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', '-signature', 'testrsapss-unrestricted.sig', srctop_file('test', 'testrsa.pem')])), "openssl dgst -prverify [plain RSA key, PSS padding mode, no PSS restrictions]"); diff --git a/test/recipes/80-test_cms.t b/test/recipes/80-test_cms.t index f9d19df6b9..f6794be891 100644 --- 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); |