| Message ID | cover.1725972333.git.lukas@wunner.de |
|---|---|
| Headers | show |
| Series | Migrate to sig_alg and templatize ecdsa | expand |
Hi Herbert, On Tue, Sep 10, 2024 at 04:30:10PM +0200, Lukas Wunner wrote: > The original impetus of this series is to introduce P1363 signature > decoding for ecdsa (patch [18/19]), which is needed by the upcoming > SPDM library (Security Protocol and Data Model) for PCI device > authentication. > > To facilitate that, move X9.62 signature decoding out of ecdsa.c and > into a template (patch [15/19]). > > New in v2: Move the maximum signature size calculations for ecdsa > out of software_key_query() and into the X9.62 template so that > corresponding calculations can be added for P1363 without further > cluttering up software_key_query() (patch [16/19] - [17/19]). > > New in v2: Avoid inefficient copying from kernel buffers to sglists > in the new templates by introducing a sig_alg backend and migrating > all algorithms to it, per Herbert's advice (patch [02/19] - [12/19]). > > Clean up various smaller issues that caught my eye in ecdsa > (patch [01/19] and [14/19]), ecrdsa (patch [19/19]) and > ASN.1 headers (patch [13/19]). This series was submitted at the tail end of the v6.11 cycle. It still applies cleanly to v6.12-rc1 though, so I'm not sure whether to resubmit. Is there anything you want me to change? Thanks! Lukas
Hi, On 2024-09-10 16:30, Lukas Wunner wrote: > A sig_alg backend has just been introduced with the intent of moving all > asymmetric sign/verify algorithms to it one by one. > > Migrate the sign/verify operations from rsa-pkcs1pad.c to a separate > rsassa-pkcs1.c which uses the new backend. > > Consequently there are now two templates which build on the "rsa" > akcipher_alg: > > * The existing "pkcs1pad" template, which is instantiated as an > akcipher_instance and retains the encrypt/decrypt operations of > RSAES-PKCS1-v1_5 (RFC 8017 sec 7.2). > > * The new "pkcs1" template, which is instantiated as a sig_instance > and contains the sign/verify operations of RSASSA-PKCS1-v1_5 > (RFC 8017 sec 8.2). > > In a separate step, rsa-pkcs1pad.c could optionally be renamed to > rsaes-pkcs1.c for clarity. Additional "oaep" and "pss" templates > could be added for RSAES-OAEP and RSASSA-PSS. > > Note that it's currently allowed to allocate a "pkcs1pad(rsa)" transform > without specifying a hash algorithm. That makes sense if the transform > is only used for encrypt/decrypt and continues to be supported. But for > sign/verify, such transforms previously did not insert the Full Hash > Prefix into the padding. The resulting message encoding was incompliant > with EMSA-PKCS1-v1_5 (RFC 8017 sec 9.2) and therefore nonsensical. > >>From here on in, it is no longer allowed to allocate a transform without > specifying a hash algorithm if the transform is used for sign/verify > operations. This simplifies the code because the insertion of the Full > Hash Prefix is no longer optional, so various "if (digest_info)" clauses > can be removed. > > There has been a previous attempt to forbid transform allocation without > specifying a hash algorithm, namely by commit c0d20d22e0ad ("crypto: > rsa-pkcs1pad - Require hash to be present"). It had to be rolled back > with commit b3a8c8a5ebb5 ("crypto: rsa-pkcs1pad: Allow hash to be > optional [ver #2]"), presumably because it broke allocation of a > transform which was solely used for encrypt/decrypt, not sign/verify. > Avoid such breakage by allowing transform allocation for encrypt/decrypt > with and without specifying a hash algorithm (and simply ignoring the > hash algorithm in the former case). > > So again, specifying a hash algorithm is now mandatory for sign/verify, > but optional and ignored for encrypt/decrypt. > > The new sig_alg API uses kernel buffers instead of sglists, which > avoids the overhead of copying signature and digest from sglists back > into kernel buffers. rsassa-pkcs1.c is thus simplified quite a bit. > > sig_alg is always synchronous, whereas the underlying "rsa" akcipher_alg > may be asynchronous. So await the result of the akcipher_alg, similar > to crypto_akcipher_sync_{en,de}crypt(). > > As part of the migration, rename "rsa_digest_info" to "hash_prefix" to > adhere to the spec language in RFC 9580. Otherwise keep the code > unmodified wherever possible to ease reviewing and bisecting. Leave > several simplification and hardening opportunities to separate commits. > > rsassa-pkcs1.c uses modern __free() syntax for allocation of buffers > which need to be freed by kfree_sensitive(), hence a DEFINE_FREE() > clause for kfree_sensitive() is introduced herein as a byproduct. > > Signed-off-by: Lukas Wunner <lukas@wunner.de> This commit (1e562deacecca1f1bec7d23da526904a1e87525e in next-20241021) seems to break connecting to wpa2-enterprise with iwd. I've only tested with one such network (eduroam, EAP-PEAP MSCHAPv2) and not yet with wpa_supplicant. This appears in the kernel log repeatedly: [ 123.714646] wlan0: authenticate with b4:de:31:fa:2d:cc (local address=78:46:5c:01:28:85) [ 123.737991] wlan0: send auth to b4:de:31:fa:2d:cc (try 1/3) [ 123.763621] wlan0: authenticated [ 123.767600] wlan0: associate with b4:de:31:fa:2d:cc (try 1/3) [ 123.780873] wlan0: RX AssocResp from b4:de:31:fa:2d:cc (capab=0x1111 status=0 aid=1) [ 123.809668] wlan0: associated [ 123.882344] wlan0: Limiting TX power to 30 (30 - 0) dBm as advertised by b4:de:31:fa:2d:cc [ 126.895233] wlan0: deauthenticating from b4:de:31:fa:2d:cc by local choice (Reason: 23=IEEE8021X_FAILED) followed by this for a while: [ 127.214582] wlan0: authenticate with b4:de:31:fa:2d:cc (local address=78:46:5c:01:28:85) [ 127.237431] wlan0: send auth to b4:de:31:fa:2d:cc (try 1/3) [ 127.363430] wlan0: send auth to b4:de:31:fa:2d:cc (try 2/3) [ 127.467526] wlan0: send auth to b4:de:31:fa:2d:cc (try 3/3) [ 127.571506] wlan0: authentication with b4:de:31:fa:2d:cc timed out Please let me know if there's anything else you need. Regards, Klara Modin git bisect start # status: waiting for both good and bad commits # bad: [d49518711f816af793de9d4a1a0e13ad10b5ce91] i2c: spacemit: add support for SpacemiT K1 SoC git bisect bad d49518711f816af793de9d4a1a0e13ad10b5ce91 # status: waiting for good commit(s), bad commit known # good: [c55228220dd33e7627ad9736b6fce4df5e7eac98] Merge tag 'char-misc-6.12-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc git bisect good c55228220dd33e7627ad9736b6fce4df5e7eac98 # bad: [092d750e6edc08fdf25e858ac5aed09cfe4685be] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git git bisect bad 092d750e6edc08fdf25e858ac5aed09cfe4685be # good: [e0c1b92a36f6e500684f5e47d95eeb0719bad2ca] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/uml/linux.git git bisect good e0c1b92a36f6e500684f5e47d95eeb0719bad2ca # good: [a8a3d62d6fa4c374c9b1fc669ca1bc73f5370650] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git git bisect good a8a3d62d6fa4c374c9b1fc669ca1bc73f5370650 # good: [39ab20647d7b8516fcad91950d8491369ebd5ea3] Merge tag 'for-netdev' of https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next git bisect good 39ab20647d7b8516fcad91950d8491369ebd5ea3 # good: [2fe3f43cbfb72a5dd053663933542d190311210c] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git git bisect good 2fe3f43cbfb72a5dd053663933542d190311210c # good: [582173a1dcc0a38c210b20450a615d724026d18f] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git git bisect good 582173a1dcc0a38c210b20450a615d724026d18f # good: [07375e61c414d70a7332443e710e24f8dc5d6705] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/ath/ath.git git bisect good 07375e61c414d70a7332443e710e24f8dc5d6705 # bad: [98091a826873bc5c114455f474121b67907e98ab] crypto: drivers - Correct multiple typos in comments git bisect bad 98091a826873bc5c114455f474121b67907e98ab # bad: [d6793ff974e07e4eea151d1f0805e92d042825a1] crypto: ecdsa - Move X9.62 signature decoding into template git bisect bad d6793ff974e07e4eea151d1f0805e92d042825a1 # bad: [5e00481bf0a8b4dbd1588ae08f1ff82492011987] crypto: rsassa-pkcs1 - Harden digest length verification git bisect bad 5e00481bf0a8b4dbd1588ae08f1ff82492011987 # good: [ef132350a3c2ae15349b7f748ce0859f0c2861be] crypto: ecdsa - Migrate to sig_alg backend git bisect good ef132350a3c2ae15349b7f748ce0859f0c2861be # good: [7964b0d4bd1271f82d6b455366a200d320f7dbf8] crypto: rsa-pkcs1pad - Deduplicate set_{pub,priv}_key callbacks git bisect good 7964b0d4bd1271f82d6b455366a200d320f7dbf8 # bad: [1e562deacecca1f1bec7d23da526904a1e87525e] crypto: rsassa-pkcs1 - Migrate to sig_alg backend git bisect bad 1e562deacecca1f1bec7d23da526904a1e87525e # first bad commit: [1e562deacecca1f1bec7d23da526904a1e87525e] crypto: rsassa-pkcs1 - Migrate to sig_alg backend
On Mon, Oct 21, 2024 at 06:08:03PM +0200, Klara Modin wrote: > On 2024-09-10 16:30, Lukas Wunner wrote: > > A sig_alg backend has just been introduced with the intent of moving all > > asymmetric sign/verify algorithms to it one by one. > > > > Migrate the sign/verify operations from rsa-pkcs1pad.c to a separate > > rsassa-pkcs1.c which uses the new backend. [...] > This commit (1e562deacecca1f1bec7d23da526904a1e87525e in next-20241021) > seems to break connecting to wpa2-enterprise with iwd. Thanks for the report and sorry for the breakage. There is one pending fix for an issue I inadvertently introduced with my sig_alg rework: https://lore.kernel.org/r/ff7a28cddfc28e7a3fb8292c680510f35ec54391.1728898147.git.lukas@wunner.de/ However it fixes a different commit than the one you found through bisection, so I suspect it won't fix the problem, though it would still be good if you could test it. There is a *second* issue I discovered last week. I cooked up a fix this morning, but haven't written a commit message yet. The patch is included below and it could indeed solve the problem because it fixes an issue introduced by the commit you identified as culprit. So if you could test the patch below as well I'd be grateful. I'll now look at the config and dmesg output you've provided. Just wanted to get this e-mail out the door quickly to point you to potential fixes. Thanks! Lukas -- >8 -- diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index c98c158..af19f9c 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -165,14 +165,22 @@ static int software_key_query(const struct kernel_pkey_params *params, { struct crypto_akcipher *tfm; struct public_key *pkey = params->key->payload.data[asym_crypto]; + const char *hash_algo = params->hash_algo; char alg_name[CRYPTO_MAX_ALG_NAME]; struct crypto_sig *sig; u8 *key, *ptr; int ret, len; bool issig; + /* + * Specifying hash_algo has historically been optional for pkcs1, + * so use an arbitrary algorithm for backward compatibility. + */ + if (strcmp(params->encoding, "pkcs1") == 0 && !hash_algo) + hash_algo = "sha256"; + ret = software_key_determine_akcipher(pkey, params->encoding, - params->hash_algo, alg_name, + hash_algo, alg_name, &issig, kernel_pkey_sign); if (ret < 0) return ret;
On 2024-10-21 21:02, Lukas Wunner wrote: > On Mon, Oct 21, 2024 at 06:08:03PM +0200, Klara Modin wrote: >> On 2024-09-10 16:30, Lukas Wunner wrote: >>> A sig_alg backend has just been introduced with the intent of moving all >>> asymmetric sign/verify algorithms to it one by one. >>> >>> Migrate the sign/verify operations from rsa-pkcs1pad.c to a separate >>> rsassa-pkcs1.c which uses the new backend. > [...] >> This commit (1e562deacecca1f1bec7d23da526904a1e87525e in next-20241021) >> seems to break connecting to wpa2-enterprise with iwd. > > Thanks for the report and sorry for the breakage. > > There is one pending fix for an issue I inadvertently introduced > with my sig_alg rework: > > https://lore.kernel.org/r/ff7a28cddfc28e7a3fb8292c680510f35ec54391.1728898147.git.lukas@wunner.de/ > > However it fixes a different commit than the one you found through > bisection, so I suspect it won't fix the problem, though it would > still be good if you could test it. > > There is a *second* issue I discovered last week. I cooked up > a fix this morning, but haven't written a commit message yet. > The patch is included below and it could indeed solve the > problem because it fixes an issue introduced by the commit you > identified as culprit. So if you could test the patch below as well > I'd be grateful. > > I'll now look at the config and dmesg output you've provided. > Just wanted to get this e-mail out the door quickly to point you > to potential fixes. > > Thanks! > > Lukas > > -- >8 -- > > diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c > index c98c158..af19f9c 100644 > --- a/crypto/asymmetric_keys/public_key.c > +++ b/crypto/asymmetric_keys/public_key.c > @@ -165,14 +165,22 @@ static int software_key_query(const struct kernel_pkey_params *params, > { > struct crypto_akcipher *tfm; > struct public_key *pkey = params->key->payload.data[asym_crypto]; > + const char *hash_algo = params->hash_algo; > char alg_name[CRYPTO_MAX_ALG_NAME]; > struct crypto_sig *sig; > u8 *key, *ptr; > int ret, len; > bool issig; > > + /* > + * Specifying hash_algo has historically been optional for pkcs1, > + * so use an arbitrary algorithm for backward compatibility. > + */ > + if (strcmp(params->encoding, "pkcs1") == 0 && !hash_algo) > + hash_algo = "sha256"; > + > ret = software_key_determine_akcipher(pkey, params->encoding, > - params->hash_algo, alg_name, > + hash_algo, alg_name, > &issig, kernel_pkey_sign); > if (ret < 0) > return ret; > I don't think I have hit the first issue you mention but I'll apply the fix and see if it changes anything. I'll probably be able to test these two sometime tomorrow. Thanks, Klara Modin
On 2024-10-21 21:02, Lukas Wunner wrote: > On Mon, Oct 21, 2024 at 06:08:03PM +0200, Klara Modin wrote: >> On 2024-09-10 16:30, Lukas Wunner wrote: >>> A sig_alg backend has just been introduced with the intent of moving all >>> asymmetric sign/verify algorithms to it one by one. >>> >>> Migrate the sign/verify operations from rsa-pkcs1pad.c to a separate >>> rsassa-pkcs1.c which uses the new backend. > [...] >> This commit (1e562deacecca1f1bec7d23da526904a1e87525e in next-20241021) >> seems to break connecting to wpa2-enterprise with iwd. > > Thanks for the report and sorry for the breakage. > > There is one pending fix for an issue I inadvertently introduced > with my sig_alg rework: > > https://lore.kernel.org/r/ff7a28cddfc28e7a3fb8292c680510f35ec54391.1728898147.git.lukas@wunner.de/ > > However it fixes a different commit than the one you found through > bisection, so I suspect it won't fix the problem, though it would > still be good if you could test it. > > There is a *second* issue I discovered last week. I cooked up > a fix this morning, but haven't written a commit message yet. > The patch is included below and it could indeed solve the > problem because it fixes an issue introduced by the commit you > identified as culprit. So if you could test the patch below as well > I'd be grateful. > > I'll now look at the config and dmesg output you've provided. > Just wanted to get this e-mail out the door quickly to point you > to potential fixes. > > Thanks! > > Lukas > > -- >8 -- > > diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c > index c98c158..af19f9c 100644 > --- a/crypto/asymmetric_keys/public_key.c > +++ b/crypto/asymmetric_keys/public_key.c > @@ -165,14 +165,22 @@ static int software_key_query(const struct kernel_pkey_params *params, > { > struct crypto_akcipher *tfm; > struct public_key *pkey = params->key->payload.data[asym_crypto]; > + const char *hash_algo = params->hash_algo; > char alg_name[CRYPTO_MAX_ALG_NAME]; > struct crypto_sig *sig; > u8 *key, *ptr; > int ret, len; > bool issig; > > + /* > + * Specifying hash_algo has historically been optional for pkcs1, > + * so use an arbitrary algorithm for backward compatibility. > + */ > + if (strcmp(params->encoding, "pkcs1") == 0 && !hash_algo) > + hash_algo = "sha256"; > + > ret = software_key_determine_akcipher(pkey, params->encoding, > - params->hash_algo, alg_name, > + hash_algo, alg_name, > &issig, kernel_pkey_sign); > if (ret < 0) > return ret; > Tested on top of yesterday's next-20241022. With the first patch only there is no change, same behavior as previously. With the second patch only I get an oops, similar as the one you mentioned in the first fix With both patches everything seems to work as expected. Thanks! (for both fixes) Tested-by: Klara Modin <klarasmodin@gmail.com>
On Wed, Oct 23, 2024 at 12:19:45PM +0200, Klara Modin wrote: > On 2024-10-21 21:02, Lukas Wunner wrote: > > On Mon, Oct 21, 2024 at 06:08:03PM +0200, Klara Modin wrote: > > > This commit (1e562deacecca1f1bec7d23da526904a1e87525e in next-20241021) > > > seems to break connecting to wpa2-enterprise with iwd. [...] > > There is a *second* issue I discovered last week. I cooked up > > a fix this morning, but haven't written a commit message yet. > > The patch is included below and it could indeed solve the > > problem because it fixes an issue introduced by the commit you > > identified as culprit. [...] > Tested on top of yesterday's next-20241022. > > With the first patch only there is no change, same behavior as previously. > > With the second patch only I get an oops, similar as the one you mentioned > in the first fix > > With both patches everything seems to work as expected. Thanks! Thanks a lot for your testing efforts, this helps greatly! I've dug into the source code of iwd (Intel Wireless Daemon) and the ell library it uses (Embedded Linux Library). It turns out that the patch I sent you is sufficient when using TLS 1.2 or newer for EAP (which I assume is true in your case). But the patch is *not* sufficient for TLS 1.1 or earlier. Normally RSA PKCS#1 encoding requires that the hash is prepended by a Full Hash Prefix (an ASN.1 sequence which identifies the hash algorithm used). But it turns out there are legacy protocols such as TLS 1.1 or earlier as well as IKEv1 which omit the Full Hash Prefix. The kernel supported this prior to 1e562deacecc. Although TLS 1.1 was deprecated in 2021 by RFC 8996, I think we cannot just remove support without advance notice. So below is a new patch which reinstates support for these legacy protocols. It should also fix the issue you're seeing with TLS 1.2 or newer (which is caused by invoking KEYCTL_PKEY_QUERY without specifying a hash algorithm). The patch below replaces the one I sent on Monday. You'll still need the other pending fix: https://lore.kernel.org/r/ff7a28cddfc28e7a3fb8292c680510f35ec54391.1728898147.git.lukas@wunner.de/ Would you mind testing this combination? It did work in my own testing, but if you could test it as well that would raise the confidence. I've looked at the source code of wpa_supplicant as well as various IKEv1 daemons (strongswan, libreswan, isakmpd, raccoon) and none of them seems to use the kernel's Key Retention Service, so iwd is the only known user space application affected so far. Thanks, Lukas -- >8 -- diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index c98c158..bbd07a9 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -93,7 +93,7 @@ static void public_key_destroy(void *payload0, void *payload3) pkey->pkey_algo); } else { if (!hash_algo) - return -EINVAL; + hash_algo = "none"; n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME, "pkcs1(%s,%s)", pkey->pkey_algo, hash_algo); diff --git a/crypto/rsassa-pkcs1.c b/crypto/rsassa-pkcs1.c index 9c28f1c..4d077fc9 100644 --- a/crypto/rsassa-pkcs1.c +++ b/crypto/rsassa-pkcs1.c @@ -27,6 +27,8 @@ * https://www.rfc-editor.org/rfc/rfc9580#table-24 */ +static const u8 hash_prefix_none[] = { }; + static const u8 hash_prefix_md5[] = { 0x30, 0x20, 0x30, 0x0c, 0x06, 0x08, /* SEQUENCE (SEQUENCE (OID */ 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x02, 0x05, /* <algorithm>, */ @@ -93,6 +95,7 @@ size_t size; } hash_prefixes[] = { #define _(X) { #X, hash_prefix_##X, sizeof(hash_prefix_##X) } + _(none), _(md5), _(sha1), _(rmd160), @@ -119,9 +122,18 @@ static const struct hash_prefix *rsassa_pkcs1_find_hash_prefix(const char *name) return NULL; } -static unsigned int rsassa_pkcs1_hash_len(const struct hash_prefix *p) +static bool rsassa_pkcs1_invalid_hash_len(unsigned int len, + const struct hash_prefix *p) { /* + * Legacy protocols such as TLS 1.1 or earlier and IKE version 1 + * do not prepend a Full Hash Prefix to the hash. In that case, + * the size of the Full Hash Prefix is zero. + */ + if (p->data == hash_prefix_none) + return false; + + /* * The final byte of the Full Hash Prefix encodes the hash length. * * This needs to be revisited should hash algorithms with more than @@ -130,7 +142,7 @@ static unsigned int rsassa_pkcs1_hash_len(const struct hash_prefix *p) */ static_assert(HASH_MAX_DIGESTSIZE <= 127); - return p->data[p->size - 1]; + return len != p->data[p->size - 1]; } struct rsassa_pkcs1_ctx { @@ -167,7 +179,7 @@ static int rsassa_pkcs1_sign(struct crypto_sig *tfm, if (dlen < ctx->key_size) return -EOVERFLOW; - if (slen != rsassa_pkcs1_hash_len(hash_prefix)) + if (rsassa_pkcs1_invalid_hash_len(slen, hash_prefix)) return -EINVAL; if (slen + hash_prefix->size > ctx->key_size - 11) @@ -237,7 +249,7 @@ static int rsassa_pkcs1_verify(struct crypto_sig *tfm, /* RFC 8017 sec 8.2.2 step 1 - length checking */ if (!ctx->key_size || slen != ctx->key_size || - dlen != rsassa_pkcs1_hash_len(hash_prefix)) + rsassa_pkcs1_invalid_hash_len(dlen, hash_prefix)) return -EINVAL; /* RFC 8017 sec 8.2.2 step 2 - RSA verification */ diff --git a/crypto/testmgr.c b/crypto/testmgr.c index 7d768f0..86126be 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -5540,6 +5540,12 @@ static int alg_test_null(const struct alg_test_desc *desc, .cipher = __VECS(fcrypt_pcbc_tv_template) } }, { + .alg = "pkcs1(rsa,none)", + .test = alg_test_sig, + .suite = { + .sig = __VECS(pkcs1_rsa_none_tv_template) + } + }, { .alg = "pkcs1(rsa,sha224)", .test = alg_test_null, .fips_allowed = 1, diff --git a/crypto/testmgr.h b/crypto/testmgr.h index 55aae18..d4c232a 100644 --- a/crypto/testmgr.h +++ b/crypto/testmgr.h @@ -1983,6 +1983,61 @@ struct kpp_testvec { }; /* + * PKCS#1 RSA test vectors for hash algorithm "none" + * (i.e. the hash in "m" is not prepended by a Full Hash Prefix) + * + * Obtained from: + * https://vcsjones.dev/sometimes-valid-rsa-dotnet/ + * https://gist.github.com/vcsjones/ab4c2327b53ed018eada76b75ef4fd99 + */ +static const struct sig_testvec pkcs1_rsa_none_tv_template[] = { + { + .key = + "\x30\x82\x01\x0a\x02\x82\x01\x01\x00\xa2\x63\x0b\x39\x44\xb8\xbb" + "\x23\xa7\x44\x49\xbb\x0e\xff\xa1\xf0\x61\x0a\x53\x93\xb0\x98\xdb" + "\xad\x2c\x0f\x4a\xc5\x6e\xff\x86\x3c\x53\x55\x0f\x15\xce\x04\x3f" + "\x2b\xfd\xa9\x96\x96\xd9\xbe\x61\x79\x0b\x5b\xc9\x4c\x86\x76\xe5" + "\xe0\x43\x4b\x22\x95\xee\xc2\x2b\x43\xc1\x9f\xd8\x68\xb4\x8e\x40" + "\x4f\xee\x85\x38\xb9\x11\xc5\x23\xf2\x64\x58\xf0\x15\x32\x6f\x4e" + "\x57\xa1\xae\x88\xa4\x02\xd7\x2a\x1e\xcd\x4b\xe1\xdd\x63\xd5\x17" + "\x89\x32\x5b\xb0\x5e\x99\x5a\xa8\x9d\x28\x50\x0e\x17\xee\x96\xdb" + "\x61\x3b\x45\x51\x1d\xcf\x12\x56\x0b\x92\x47\xfc\xab\xae\xf6\x66" + "\x3d\x47\xac\x70\x72\xe7\x92\xe7\x5f\xcd\x10\xb9\xc4\x83\x64\x94" + "\x19\xbd\x25\x80\xe1\xe8\xd2\x22\xa5\xd0\xba\x02\x7a\xa1\x77\x93" + "\x5b\x65\xc3\xee\x17\x74\xbc\x41\x86\x2a\xdc\x08\x4c\x8c\x92\x8c" + "\x91\x2d\x9e\x77\x44\x1f\x68\xd6\xa8\x74\x77\xdb\x0e\x5b\x32\x8b" + "\x56\x8b\x33\xbd\xd9\x63\xc8\x49\x9d\x3a\xc5\xc5\xea\x33\x0b\xd2" + "\xf1\xa3\x1b\xf4\x8b\xbe\xd9\xb3\x57\x8b\x3b\xde\x04\xa7\x7a\x22" + "\xb2\x24\xae\x2e\xc7\x70\xc5\xbe\x4e\x83\x26\x08\xfb\x0b\xbd\xa9" + "\x4f\x99\x08\xe1\x10\x28\x72\xaa\xcd\x02\x03\x01\x00\x01", + .key_len = 294, + .m = + "\x68\xb4\xf9\x26\x34\x31\x25\xdd\x26\x50\x13\x68\xc1\x99\x26\x71" + "\x19\xa2\xde\x81", + .m_size = 20, + .c = + "\x6a\xdb\x39\xe5\x63\xb3\x25\xde\x58\xca\xc3\xf1\x36\x9c\x0b\x36" + "\xb7\xd6\x69\xf9\xba\xa6\x68\x14\x8c\x24\x52\xd3\x25\xa5\xf3\xad" + "\xc9\x47\x44\xde\x06\xd8\x0f\x56\xca\x2d\xfb\x0f\xe9\x99\xe2\x9d" + "\x8a\xe8\x7f\xfb\x9a\x99\x96\xf1\x2c\x4a\xe4\xc0\xae\x4d\x29\x47" + "\x38\x96\x51\x2f\x6d\x8e\xb8\x88\xbd\x1a\x0a\x70\xbc\x23\x38\x67" + "\x62\x22\x01\x23\x71\xe5\xbb\x95\xea\x6b\x8d\x31\x62\xbf\xf0\xc4" + "\xb9\x46\xd6\x67\xfc\x4c\xe6\x1f\xd6\x5d\xf7\xa9\xad\x3a\xf1\xbf" + "\xa2\xf9\x66\xde\xb6\x8e\xec\x8f\x81\x8d\x1e\x3a\x12\x27\x6a\xfc" + "\xae\x92\x9f\xc3\x87\xc3\xba\x8d\x04\xb8\x8f\x0f\x61\x68\x9a\x96" + "\x2c\x80\x2c\x32\x40\xde\x9d\xb9\x9b\xe2\xe4\x45\x2e\x91\x47\x5c" + "\x47\xa4\x9d\x02\x57\x59\xf7\x75\x5d\x5f\x32\x82\x75\x5d\xe5\x78" + "\xc9\x19\x61\x46\x06\x9d\xa5\x1d\xd6\x32\x48\x9a\xdb\x09\x29\x81" + "\x14\x2e\xf0\x27\xe9\x37\x13\x74\xec\xa5\xcd\x67\x6b\x19\xf6\x88" + "\xf0\xc2\x8b\xa8\x7f\x2f\x76\x5a\x3e\x0c\x47\x5d\xe8\x82\x50\x27" + "\x40\xce\x27\x41\x45\xa0\xcf\xaa\x2f\xd3\xad\x3c\xbf\x73\xff\x93" + "\xe3\x78\x49\xd9\xa9\x78\x22\x81\x9a\xe5\xe2\x94\xe9\x40\xab\xf1", + .c_size = 256, + .public_key_vec = true, + }, +}; + +/* * PKCS#1 RSA test vectors. Obtained from CAVS testing. */ static const struct sig_testvec pkcs1_rsa_tv_template[] = {
On Fri, Oct 25, 2024 at 09:17:02AM +0200, Lukas Wunner wrote: > So below is a new patch which reinstates support for these legacy > protocols. It should also fix the issue you're seeing with TLS 1.2 > or newer (which is caused by invoking KEYCTL_PKEY_QUERY without > specifying a hash algorithm). [...] > I've looked at the source code of wpa_supplicant as well as > various IKEv1 daemons (strongswan, libreswan, isakmpd, raccoon) > and none of them seems to use the kernel's Key Retention Service, > so iwd is the only known user space application affected so far. Yes, based on historical mailing list discussions it appears that KEYCTL_PKEY_* were added to the kernel for iwd, and iwd is their only user. This design is a huge mistake both on the part of iwd and the kernel community, for a variety of reasons that have already been covered extensively in the discussions that occur each time iwd breaks. iwd should be using a real crypto library, like all the other wireless daemons. - Eric
On 2024-10-25 09:17, Lukas Wunner wrote: > On Wed, Oct 23, 2024 at 12:19:45PM +0200, Klara Modin wrote: >> On 2024-10-21 21:02, Lukas Wunner wrote: >>> On Mon, Oct 21, 2024 at 06:08:03PM +0200, Klara Modin wrote: >>>> This commit (1e562deacecca1f1bec7d23da526904a1e87525e in next-20241021) >>>> seems to break connecting to wpa2-enterprise with iwd. > [...] >>> There is a *second* issue I discovered last week. I cooked up >>> a fix this morning, but haven't written a commit message yet. >>> The patch is included below and it could indeed solve the >>> problem because it fixes an issue introduced by the commit you >>> identified as culprit. > [...] >> Tested on top of yesterday's next-20241022. >> >> With the first patch only there is no change, same behavior as previously. >> >> With the second patch only I get an oops, similar as the one you mentioned >> in the first fix >> >> With both patches everything seems to work as expected. Thanks! > > Thanks a lot for your testing efforts, this helps greatly! > > I've dug into the source code of iwd (Intel Wireless Daemon) and > the ell library it uses (Embedded Linux Library). > > It turns out that the patch I sent you is sufficient when using > TLS 1.2 or newer for EAP (which I assume is true in your case). > But the patch is *not* sufficient for TLS 1.1 or earlier. > > Normally RSA PKCS#1 encoding requires that the hash is prepended > by a Full Hash Prefix (an ASN.1 sequence which identifies the > hash algorithm used). But it turns out there are legacy protocols > such as TLS 1.1 or earlier as well as IKEv1 which omit the > Full Hash Prefix. > > The kernel supported this prior to 1e562deacecc. Although TLS 1.1 > was deprecated in 2021 by RFC 8996, I think we cannot just remove > support without advance notice. > > So below is a new patch which reinstates support for these legacy > protocols. It should also fix the issue you're seeing with TLS 1.2 > or newer (which is caused by invoking KEYCTL_PKEY_QUERY without > specifying a hash algorithm). > > The patch below replaces the one I sent on Monday. You'll still > need the other pending fix: > > https://lore.kernel.org/r/ff7a28cddfc28e7a3fb8292c680510f35ec54391.1728898147.git.lukas@wunner.de/ > > Would you mind testing this combination? It did work in my own > testing, but if you could test it as well that would raise the > confidence. > > I've looked at the source code of wpa_supplicant as well as > various IKEv1 daemons (strongswan, libreswan, isakmpd, raccoon) > and none of them seems to use the kernel's Key Retention Service, > so iwd is the only known user space application affected so far. > > Thanks, > > Lukas > Thanks for the explanation. I'll probably be able to test this on Monday. (by the way, I don't seem to be receiving your emails. It could perhaps be something on the gmail side. I'll add klara@kasm.eu to the CC which is on my own server, maybe that'll work better) Regards, Klara Modin > -- >8 -- > > diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c > index c98c158..bbd07a9 100644 > --- a/crypto/asymmetric_keys/public_key.c > +++ b/crypto/asymmetric_keys/public_key.c > @@ -93,7 +93,7 @@ static void public_key_destroy(void *payload0, void *payload3) > pkey->pkey_algo); > } else { > if (!hash_algo) > - return -EINVAL; > + hash_algo = "none"; > n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME, > "pkcs1(%s,%s)", > pkey->pkey_algo, hash_algo); > diff --git a/crypto/rsassa-pkcs1.c b/crypto/rsassa-pkcs1.c > index 9c28f1c..4d077fc9 100644 > --- a/crypto/rsassa-pkcs1.c > +++ b/crypto/rsassa-pkcs1.c > @@ -27,6 +27,8 @@ > * https://www.rfc-editor.org/rfc/rfc9580#table-24 > */ > > +static const u8 hash_prefix_none[] = { }; > + > static const u8 hash_prefix_md5[] = { > 0x30, 0x20, 0x30, 0x0c, 0x06, 0x08, /* SEQUENCE (SEQUENCE (OID */ > 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x02, 0x05, /* <algorithm>, */ > @@ -93,6 +95,7 @@ > size_t size; > } hash_prefixes[] = { > #define _(X) { #X, hash_prefix_##X, sizeof(hash_prefix_##X) } > + _(none), > _(md5), > _(sha1), > _(rmd160), > @@ -119,9 +122,18 @@ static const struct hash_prefix *rsassa_pkcs1_find_hash_prefix(const char *name) > return NULL; > } > > -static unsigned int rsassa_pkcs1_hash_len(const struct hash_prefix *p) > +static bool rsassa_pkcs1_invalid_hash_len(unsigned int len, > + const struct hash_prefix *p) > { > /* > + * Legacy protocols such as TLS 1.1 or earlier and IKE version 1 > + * do not prepend a Full Hash Prefix to the hash. In that case, > + * the size of the Full Hash Prefix is zero. > + */ > + if (p->data == hash_prefix_none) > + return false; > + > + /* > * The final byte of the Full Hash Prefix encodes the hash length. > * > * This needs to be revisited should hash algorithms with more than > @@ -130,7 +142,7 @@ static unsigned int rsassa_pkcs1_hash_len(const struct hash_prefix *p) > */ > static_assert(HASH_MAX_DIGESTSIZE <= 127); > > - return p->data[p->size - 1]; > + return len != p->data[p->size - 1]; > } > > struct rsassa_pkcs1_ctx { > @@ -167,7 +179,7 @@ static int rsassa_pkcs1_sign(struct crypto_sig *tfm, > if (dlen < ctx->key_size) > return -EOVERFLOW; > > - if (slen != rsassa_pkcs1_hash_len(hash_prefix)) > + if (rsassa_pkcs1_invalid_hash_len(slen, hash_prefix)) > return -EINVAL; > > if (slen + hash_prefix->size > ctx->key_size - 11) > @@ -237,7 +249,7 @@ static int rsassa_pkcs1_verify(struct crypto_sig *tfm, > /* RFC 8017 sec 8.2.2 step 1 - length checking */ > if (!ctx->key_size || > slen != ctx->key_size || > - dlen != rsassa_pkcs1_hash_len(hash_prefix)) > + rsassa_pkcs1_invalid_hash_len(dlen, hash_prefix)) > return -EINVAL; > > /* RFC 8017 sec 8.2.2 step 2 - RSA verification */ > diff --git a/crypto/testmgr.c b/crypto/testmgr.c > index 7d768f0..86126be 100644 > --- a/crypto/testmgr.c > +++ b/crypto/testmgr.c > @@ -5540,6 +5540,12 @@ static int alg_test_null(const struct alg_test_desc *desc, > .cipher = __VECS(fcrypt_pcbc_tv_template) > } > }, { > + .alg = "pkcs1(rsa,none)", > + .test = alg_test_sig, > + .suite = { > + .sig = __VECS(pkcs1_rsa_none_tv_template) > + } > + }, { > .alg = "pkcs1(rsa,sha224)", > .test = alg_test_null, > .fips_allowed = 1, > diff --git a/crypto/testmgr.h b/crypto/testmgr.h > index 55aae18..d4c232a 100644 > --- a/crypto/testmgr.h > +++ b/crypto/testmgr.h > @@ -1983,6 +1983,61 @@ struct kpp_testvec { > }; > > /* > + * PKCS#1 RSA test vectors for hash algorithm "none" > + * (i.e. the hash in "m" is not prepended by a Full Hash Prefix) > + * > + * Obtained from: > + * https://vcsjones.dev/sometimes-valid-rsa-dotnet/ > + * https://gist.github.com/vcsjones/ab4c2327b53ed018eada76b75ef4fd99 > + */ > +static const struct sig_testvec pkcs1_rsa_none_tv_template[] = { > + { > + .key = > + "\x30\x82\x01\x0a\x02\x82\x01\x01\x00\xa2\x63\x0b\x39\x44\xb8\xbb" > + "\x23\xa7\x44\x49\xbb\x0e\xff\xa1\xf0\x61\x0a\x53\x93\xb0\x98\xdb" > + "\xad\x2c\x0f\x4a\xc5\x6e\xff\x86\x3c\x53\x55\x0f\x15\xce\x04\x3f" > + "\x2b\xfd\xa9\x96\x96\xd9\xbe\x61\x79\x0b\x5b\xc9\x4c\x86\x76\xe5" > + "\xe0\x43\x4b\x22\x95\xee\xc2\x2b\x43\xc1\x9f\xd8\x68\xb4\x8e\x40" > + "\x4f\xee\x85\x38\xb9\x11\xc5\x23\xf2\x64\x58\xf0\x15\x32\x6f\x4e" > + "\x57\xa1\xae\x88\xa4\x02\xd7\x2a\x1e\xcd\x4b\xe1\xdd\x63\xd5\x17" > + "\x89\x32\x5b\xb0\x5e\x99\x5a\xa8\x9d\x28\x50\x0e\x17\xee\x96\xdb" > + "\x61\x3b\x45\x51\x1d\xcf\x12\x56\x0b\x92\x47\xfc\xab\xae\xf6\x66" > + "\x3d\x47\xac\x70\x72\xe7\x92\xe7\x5f\xcd\x10\xb9\xc4\x83\x64\x94" > + "\x19\xbd\x25\x80\xe1\xe8\xd2\x22\xa5\xd0\xba\x02\x7a\xa1\x77\x93" > + "\x5b\x65\xc3\xee\x17\x74\xbc\x41\x86\x2a\xdc\x08\x4c\x8c\x92\x8c" > + "\x91\x2d\x9e\x77\x44\x1f\x68\xd6\xa8\x74\x77\xdb\x0e\x5b\x32\x8b" > + "\x56\x8b\x33\xbd\xd9\x63\xc8\x49\x9d\x3a\xc5\xc5\xea\x33\x0b\xd2" > + "\xf1\xa3\x1b\xf4\x8b\xbe\xd9\xb3\x57\x8b\x3b\xde\x04\xa7\x7a\x22" > + "\xb2\x24\xae\x2e\xc7\x70\xc5\xbe\x4e\x83\x26\x08\xfb\x0b\xbd\xa9" > + "\x4f\x99\x08\xe1\x10\x28\x72\xaa\xcd\x02\x03\x01\x00\x01", > + .key_len = 294, > + .m = > + "\x68\xb4\xf9\x26\x34\x31\x25\xdd\x26\x50\x13\x68\xc1\x99\x26\x71" > + "\x19\xa2\xde\x81", > + .m_size = 20, > + .c = > + "\x6a\xdb\x39\xe5\x63\xb3\x25\xde\x58\xca\xc3\xf1\x36\x9c\x0b\x36" > + "\xb7\xd6\x69\xf9\xba\xa6\x68\x14\x8c\x24\x52\xd3\x25\xa5\xf3\xad" > + "\xc9\x47\x44\xde\x06\xd8\x0f\x56\xca\x2d\xfb\x0f\xe9\x99\xe2\x9d" > + "\x8a\xe8\x7f\xfb\x9a\x99\x96\xf1\x2c\x4a\xe4\xc0\xae\x4d\x29\x47" > + "\x38\x96\x51\x2f\x6d\x8e\xb8\x88\xbd\x1a\x0a\x70\xbc\x23\x38\x67" > + "\x62\x22\x01\x23\x71\xe5\xbb\x95\xea\x6b\x8d\x31\x62\xbf\xf0\xc4" > + "\xb9\x46\xd6\x67\xfc\x4c\xe6\x1f\xd6\x5d\xf7\xa9\xad\x3a\xf1\xbf" > + "\xa2\xf9\x66\xde\xb6\x8e\xec\x8f\x81\x8d\x1e\x3a\x12\x27\x6a\xfc" > + "\xae\x92\x9f\xc3\x87\xc3\xba\x8d\x04\xb8\x8f\x0f\x61\x68\x9a\x96" > + "\x2c\x80\x2c\x32\x40\xde\x9d\xb9\x9b\xe2\xe4\x45\x2e\x91\x47\x5c" > + "\x47\xa4\x9d\x02\x57\x59\xf7\x75\x5d\x5f\x32\x82\x75\x5d\xe5\x78" > + "\xc9\x19\x61\x46\x06\x9d\xa5\x1d\xd6\x32\x48\x9a\xdb\x09\x29\x81" > + "\x14\x2e\xf0\x27\xe9\x37\x13\x74\xec\xa5\xcd\x67\x6b\x19\xf6\x88" > + "\xf0\xc2\x8b\xa8\x7f\x2f\x76\x5a\x3e\x0c\x47\x5d\xe8\x82\x50\x27" > + "\x40\xce\x27\x41\x45\xa0\xcf\xaa\x2f\xd3\xad\x3c\xbf\x73\xff\x93" > + "\xe3\x78\x49\xd9\xa9\x78\x22\x81\x9a\xe5\xe2\x94\xe9\x40\xab\xf1", > + .c_size = 256, > + .public_key_vec = true, > + }, > +}; > + > +/* > * PKCS#1 RSA test vectors. Obtained from CAVS testing. > */ > static const struct sig_testvec pkcs1_rsa_tv_template[] = { >
On 2024-10-25 09:17, Lukas Wunner wrote: > On Wed, Oct 23, 2024 at 12:19:45PM +0200, Klara Modin wrote: >> On 2024-10-21 21:02, Lukas Wunner wrote: >>> On Mon, Oct 21, 2024 at 06:08:03PM +0200, Klara Modin wrote: >>>> This commit (1e562deacecca1f1bec7d23da526904a1e87525e in next-20241021) >>>> seems to break connecting to wpa2-enterprise with iwd. > [...] >>> There is a *second* issue I discovered last week. I cooked up >>> a fix this morning, but haven't written a commit message yet. >>> The patch is included below and it could indeed solve the >>> problem because it fixes an issue introduced by the commit you >>> identified as culprit. > [...] >> Tested on top of yesterday's next-20241022. >> >> With the first patch only there is no change, same behavior as previously. >> >> With the second patch only I get an oops, similar as the one you mentioned >> in the first fix >> >> With both patches everything seems to work as expected. Thanks! > > Thanks a lot for your testing efforts, this helps greatly! > > I've dug into the source code of iwd (Intel Wireless Daemon) and > the ell library it uses (Embedded Linux Library). > > It turns out that the patch I sent you is sufficient when using > TLS 1.2 or newer for EAP (which I assume is true in your case). > But the patch is *not* sufficient for TLS 1.1 or earlier. > > Normally RSA PKCS#1 encoding requires that the hash is prepended > by a Full Hash Prefix (an ASN.1 sequence which identifies the > hash algorithm used). But it turns out there are legacy protocols > such as TLS 1.1 or earlier as well as IKEv1 which omit the > Full Hash Prefix. > > The kernel supported this prior to 1e562deacecc. Although TLS 1.1 > was deprecated in 2021 by RFC 8996, I think we cannot just remove > support without advance notice. > > So below is a new patch which reinstates support for these legacy > protocols. It should also fix the issue you're seeing with TLS 1.2 > or newer (which is caused by invoking KEYCTL_PKEY_QUERY without > specifying a hash algorithm). > > The patch below replaces the one I sent on Monday. You'll still > need the other pending fix: > > https://lore.kernel.org/r/ff7a28cddfc28e7a3fb8292c680510f35ec54391.1728898147.git.lukas@wunner.de/ > > Would you mind testing this combination? It did work in my own > testing, but if you could test it as well that would raise the > confidence. > > I've looked at the source code of wpa_supplicant as well as > various IKEv1 daemons (strongswan, libreswan, isakmpd, raccoon) > and none of them seems to use the kernel's Key Retention Service, > so iwd is the only known user space application affected so far. > > Thanks, > > Lukas This patch also fixes the issue for me (on top of next-20241028). Thanks, Tested-by: Klara Modin <klarasmodin@gmail.com> > > -- >8 -- > > diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c > index c98c158..bbd07a9 100644 > --- a/crypto/asymmetric_keys/public_key.c > +++ b/crypto/asymmetric_keys/public_key.c > @@ -93,7 +93,7 @@ static void public_key_destroy(void *payload0, void *payload3) > pkey->pkey_algo); > } else { > if (!hash_algo) > - return -EINVAL; > + hash_algo = "none"; > n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME, > "pkcs1(%s,%s)", > pkey->pkey_algo, hash_algo); > diff --git a/crypto/rsassa-pkcs1.c b/crypto/rsassa-pkcs1.c > index 9c28f1c..4d077fc9 100644 > --- a/crypto/rsassa-pkcs1.c > +++ b/crypto/rsassa-pkcs1.c > @@ -27,6 +27,8 @@ > * https://www.rfc-editor.org/rfc/rfc9580#table-24 > */ > > +static const u8 hash_prefix_none[] = { }; > + > static const u8 hash_prefix_md5[] = { > 0x30, 0x20, 0x30, 0x0c, 0x06, 0x08, /* SEQUENCE (SEQUENCE (OID */ > 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x02, 0x05, /* <algorithm>, */ > @@ -93,6 +95,7 @@ > size_t size; > } hash_prefixes[] = { > #define _(X) { #X, hash_prefix_##X, sizeof(hash_prefix_##X) } > + _(none), > _(md5), > _(sha1), > _(rmd160), > @@ -119,9 +122,18 @@ static const struct hash_prefix *rsassa_pkcs1_find_hash_prefix(const char *name) > return NULL; > } > > -static unsigned int rsassa_pkcs1_hash_len(const struct hash_prefix *p) > +static bool rsassa_pkcs1_invalid_hash_len(unsigned int len, > + const struct hash_prefix *p) > { > /* > + * Legacy protocols such as TLS 1.1 or earlier and IKE version 1 > + * do not prepend a Full Hash Prefix to the hash. In that case, > + * the size of the Full Hash Prefix is zero. > + */ > + if (p->data == hash_prefix_none) > + return false; > + > + /* > * The final byte of the Full Hash Prefix encodes the hash length. > * > * This needs to be revisited should hash algorithms with more than > @@ -130,7 +142,7 @@ static unsigned int rsassa_pkcs1_hash_len(const struct hash_prefix *p) > */ > static_assert(HASH_MAX_DIGESTSIZE <= 127); > > - return p->data[p->size - 1]; > + return len != p->data[p->size - 1]; > } > > struct rsassa_pkcs1_ctx { > @@ -167,7 +179,7 @@ static int rsassa_pkcs1_sign(struct crypto_sig *tfm, > if (dlen < ctx->key_size) > return -EOVERFLOW; > > - if (slen != rsassa_pkcs1_hash_len(hash_prefix)) > + if (rsassa_pkcs1_invalid_hash_len(slen, hash_prefix)) > return -EINVAL; > > if (slen + hash_prefix->size > ctx->key_size - 11) > @@ -237,7 +249,7 @@ static int rsassa_pkcs1_verify(struct crypto_sig *tfm, > /* RFC 8017 sec 8.2.2 step 1 - length checking */ > if (!ctx->key_size || > slen != ctx->key_size || > - dlen != rsassa_pkcs1_hash_len(hash_prefix)) > + rsassa_pkcs1_invalid_hash_len(dlen, hash_prefix)) > return -EINVAL; > > /* RFC 8017 sec 8.2.2 step 2 - RSA verification */ > diff --git a/crypto/testmgr.c b/crypto/testmgr.c > index 7d768f0..86126be 100644 > --- a/crypto/testmgr.c > +++ b/crypto/testmgr.c > @@ -5540,6 +5540,12 @@ static int alg_test_null(const struct alg_test_desc *desc, > .cipher = __VECS(fcrypt_pcbc_tv_template) > } > }, { > + .alg = "pkcs1(rsa,none)", > + .test = alg_test_sig, > + .suite = { > + .sig = __VECS(pkcs1_rsa_none_tv_template) > + } > + }, { > .alg = "pkcs1(rsa,sha224)", > .test = alg_test_null, > .fips_allowed = 1, > diff --git a/crypto/testmgr.h b/crypto/testmgr.h > index 55aae18..d4c232a 100644 > --- a/crypto/testmgr.h > +++ b/crypto/testmgr.h > @@ -1983,6 +1983,61 @@ struct kpp_testvec { > }; > > /* > + * PKCS#1 RSA test vectors for hash algorithm "none" > + * (i.e. the hash in "m" is not prepended by a Full Hash Prefix) > + * > + * Obtained from: > + * https://vcsjones.dev/sometimes-valid-rsa-dotnet/ > + * https://gist.github.com/vcsjones/ab4c2327b53ed018eada76b75ef4fd99 > + */ > +static const struct sig_testvec pkcs1_rsa_none_tv_template[] = { > + { > + .key = > + "\x30\x82\x01\x0a\x02\x82\x01\x01\x00\xa2\x63\x0b\x39\x44\xb8\xbb" > + "\x23\xa7\x44\x49\xbb\x0e\xff\xa1\xf0\x61\x0a\x53\x93\xb0\x98\xdb" > + "\xad\x2c\x0f\x4a\xc5\x6e\xff\x86\x3c\x53\x55\x0f\x15\xce\x04\x3f" > + "\x2b\xfd\xa9\x96\x96\xd9\xbe\x61\x79\x0b\x5b\xc9\x4c\x86\x76\xe5" > + "\xe0\x43\x4b\x22\x95\xee\xc2\x2b\x43\xc1\x9f\xd8\x68\xb4\x8e\x40" > + "\x4f\xee\x85\x38\xb9\x11\xc5\x23\xf2\x64\x58\xf0\x15\x32\x6f\x4e" > + "\x57\xa1\xae\x88\xa4\x02\xd7\x2a\x1e\xcd\x4b\xe1\xdd\x63\xd5\x17" > + "\x89\x32\x5b\xb0\x5e\x99\x5a\xa8\x9d\x28\x50\x0e\x17\xee\x96\xdb" > + "\x61\x3b\x45\x51\x1d\xcf\x12\x56\x0b\x92\x47\xfc\xab\xae\xf6\x66" > + "\x3d\x47\xac\x70\x72\xe7\x92\xe7\x5f\xcd\x10\xb9\xc4\x83\x64\x94" > + "\x19\xbd\x25\x80\xe1\xe8\xd2\x22\xa5\xd0\xba\x02\x7a\xa1\x77\x93" > + "\x5b\x65\xc3\xee\x17\x74\xbc\x41\x86\x2a\xdc\x08\x4c\x8c\x92\x8c" > + "\x91\x2d\x9e\x77\x44\x1f\x68\xd6\xa8\x74\x77\xdb\x0e\x5b\x32\x8b" > + "\x56\x8b\x33\xbd\xd9\x63\xc8\x49\x9d\x3a\xc5\xc5\xea\x33\x0b\xd2" > + "\xf1\xa3\x1b\xf4\x8b\xbe\xd9\xb3\x57\x8b\x3b\xde\x04\xa7\x7a\x22" > + "\xb2\x24\xae\x2e\xc7\x70\xc5\xbe\x4e\x83\x26\x08\xfb\x0b\xbd\xa9" > + "\x4f\x99\x08\xe1\x10\x28\x72\xaa\xcd\x02\x03\x01\x00\x01", > + .key_len = 294, > + .m = > + "\x68\xb4\xf9\x26\x34\x31\x25\xdd\x26\x50\x13\x68\xc1\x99\x26\x71" > + "\x19\xa2\xde\x81", > + .m_size = 20, > + .c = > + "\x6a\xdb\x39\xe5\x63\xb3\x25\xde\x58\xca\xc3\xf1\x36\x9c\x0b\x36" > + "\xb7\xd6\x69\xf9\xba\xa6\x68\x14\x8c\x24\x52\xd3\x25\xa5\xf3\xad" > + "\xc9\x47\x44\xde\x06\xd8\x0f\x56\xca\x2d\xfb\x0f\xe9\x99\xe2\x9d" > + "\x8a\xe8\x7f\xfb\x9a\x99\x96\xf1\x2c\x4a\xe4\xc0\xae\x4d\x29\x47" > + "\x38\x96\x51\x2f\x6d\x8e\xb8\x88\xbd\x1a\x0a\x70\xbc\x23\x38\x67" > + "\x62\x22\x01\x23\x71\xe5\xbb\x95\xea\x6b\x8d\x31\x62\xbf\xf0\xc4" > + "\xb9\x46\xd6\x67\xfc\x4c\xe6\x1f\xd6\x5d\xf7\xa9\xad\x3a\xf1\xbf" > + "\xa2\xf9\x66\xde\xb6\x8e\xec\x8f\x81\x8d\x1e\x3a\x12\x27\x6a\xfc" > + "\xae\x92\x9f\xc3\x87\xc3\xba\x8d\x04\xb8\x8f\x0f\x61\x68\x9a\x96" > + "\x2c\x80\x2c\x32\x40\xde\x9d\xb9\x9b\xe2\xe4\x45\x2e\x91\x47\x5c" > + "\x47\xa4\x9d\x02\x57\x59\xf7\x75\x5d\x5f\x32\x82\x75\x5d\xe5\x78" > + "\xc9\x19\x61\x46\x06\x9d\xa5\x1d\xd6\x32\x48\x9a\xdb\x09\x29\x81" > + "\x14\x2e\xf0\x27\xe9\x37\x13\x74\xec\xa5\xcd\x67\x6b\x19\xf6\x88" > + "\xf0\xc2\x8b\xa8\x7f\x2f\x76\x5a\x3e\x0c\x47\x5d\xe8\x82\x50\x27" > + "\x40\xce\x27\x41\x45\xa0\xcf\xaa\x2f\xd3\xad\x3c\xbf\x73\xff\x93" > + "\xe3\x78\x49\xd9\xa9\x78\x22\x81\x9a\xe5\xe2\x94\xe9\x40\xab\xf1", > + .c_size = 256, > + .public_key_vec = true, > + }, > +}; > + > +/* > * PKCS#1 RSA test vectors. Obtained from CAVS testing. > */ > static const struct sig_testvec pkcs1_rsa_tv_template[] = { >
The original impetus of this series is to introduce P1363 signature decoding for ecdsa (patch [18/19]), which is needed by the upcoming SPDM library (Security Protocol and Data Model) for PCI device authentication. To facilitate that, move X9.62 signature decoding out of ecdsa.c and into a template (patch [15/19]). New in v2: Move the maximum signature size calculations for ecdsa out of software_key_query() and into the X9.62 template so that corresponding calculations can be added for P1363 without further cluttering up software_key_query() (patch [16/19] - [17/19]). New in v2: Avoid inefficient copying from kernel buffers to sglists in the new templates by introducing a sig_alg backend and migrating all algorithms to it, per Herbert's advice (patch [02/19] - [12/19]). Clean up various smaller issues that caught my eye in ecdsa (patch [01/19] and [14/19]), ecrdsa (patch [19/19]) and ASN.1 headers (patch [13/19]). I've also accumulated various cleanups for crypto virtio on my development branch but will leave them for another day as this series is already nearing the "too big to review" threshold. ;) I've run selftests on every single commit, but further testing would be appreciated to raise the confidence. Link to v1: https://lore.kernel.org/all/cover.1722260176.git.lukas@wunner.de/ Changes v1 -> v2: * [PATCH 13/19] ASN.1: Clean up include statements in public headers * Drop "#include <linux/bug.h>" from <linux/asn1_encoder.h> (Jonathan) * [PATCH 14/19] crypto: ecdsa - Avoid signed integer overflow on signature decoding * Add code comment explaining why vlen may be larger than bufsize (Stefan) * [PATCH 15/19] crypto: ecdsa - Move X9.62 signature decoding into template * Drop unnecessary "params", "param_len" and "algo" definitions from ecdsa_nist_p{192,256,384,521}_tv_template[]. * Introduce and use struct ecdsa_raw_sig in <crypto/internal/ecc.h>. * [PATCH 18/19] crypto: ecdsa - Support P1363 signature decoding * Drop unnecessary "params", "param_len" and "algo" definitions from p1363_ecdsa_nist_p256_tv_template[]. Lukas Wunner (19): crypto: ecdsa - Drop unused test vector elements crypto: sig - Introduce sig_alg backend crypto: ecdsa - Migrate to sig_alg backend crypto: ecrdsa - Migrate to sig_alg backend crypto: rsa-pkcs1pad - Deduplicate set_{pub,priv}_key callbacks crypto: rsassa-pkcs1 - Migrate to sig_alg backend crypto: rsassa-pkcs1 - Harden digest length verification crypto: rsassa-pkcs1 - Avoid copying hash prefix crypto: virtio - Drop sign/verify operations crypto: drivers - Drop sign/verify operations crypto: akcipher - Drop sign/verify operations crypto: sig - Move crypto_sig_*() API calls to include file ASN.1: Clean up include statements in public headers crypto: ecdsa - Avoid signed integer overflow on signature decoding crypto: ecdsa - Move X9.62 signature decoding into template crypto: sig - Rename crypto_sig_maxsize() to crypto_sig_keysize() crypto: ecdsa - Move X9.62 signature size calculation into template crypto: ecdsa - Support P1363 signature decoding crypto: ecrdsa - Fix signature size calculation Documentation/crypto/api-akcipher.rst | 2 +- Documentation/crypto/api-sig.rst | 15 + Documentation/crypto/api.rst | 1 + Documentation/crypto/architecture.rst | 2 + crypto/Kconfig | 5 +- crypto/Makefile | 5 +- crypto/akcipher.c | 64 +- crypto/asymmetric_keys/public_key.c | 58 +- crypto/ecdsa-p1363.c | 159 ++++ crypto/ecdsa-x962.c | 237 +++++ crypto/ecdsa.c | 209 ++--- crypto/ecrdsa.c | 64 +- crypto/internal.h | 19 - crypto/rsa-pkcs1pad.c | 371 +------- crypto/rsa.c | 17 +- crypto/rsassa-pkcs1.c | 442 +++++++++ crypto/sig.c | 143 +-- crypto/testmgr.c | 320 +++++-- crypto/testmgr.h | 884 +++++++++++++++--- drivers/crypto/aspeed/aspeed-acry.c | 2 - drivers/crypto/hisilicon/hpre/hpre_crypto.c | 2 - drivers/crypto/starfive/jh7110-rsa.c | 2 - .../virtio/virtio_crypto_akcipher_algs.c | 65 +- include/crypto/akcipher.h | 69 +- include/crypto/internal/akcipher.h | 4 +- include/crypto/internal/ecc.h | 14 + include/crypto/internal/rsa.h | 29 + include/crypto/internal/sig.h | 80 ++ include/crypto/sig.h | 152 ++- include/linux/asn1_decoder.h | 1 + include/linux/asn1_encoder.h | 1 - include/linux/slab.h | 1 + include/uapi/linux/cryptouser.h | 5 + include/uapi/linux/virtio_crypto.h | 1 + security/integrity/ima/ima_main.c | 6 +- 35 files changed, 2398 insertions(+), 1053 deletions(-) create mode 100644 Documentation/crypto/api-sig.rst create mode 100644 crypto/ecdsa-p1363.c create mode 100644 crypto/ecdsa-x962.c create mode 100644 crypto/rsassa-pkcs1.c