* [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work @ 2022-02-02 6:59 Vitaly Chikunov 2022-02-02 12:55 ` Stefan Berger ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Vitaly Chikunov @ 2022-02-02 6:59 UTC (permalink / raw) To: keyrings, Jarkko Sakkinen, David Howells Cc: linux-crypto, linux-integrity, Stefan Berger, Eric Biggers Rarely used `keyctl pkey_verify' can verify raw signatures, but was failing, because ECDSA/EC-RDSA signature sizes are twice key sizes which does not pass in/out sizes check in keyctl_pkey_params_get_2. This in turn because these values cannot be distinguished by a single `max_size' callback return value. Also, `keyctl pkey_query` displays incorrect `max_sig_size' about these algorithms. Signed-off-by: Vitaly Chikunov <vt@altlinux.org> --- crypto/asymmetric_keys/public_key.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index 4fefb219bfdc..3ffbab07ed2a 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -143,8 +143,19 @@ static int software_key_query(const struct kernel_pkey_params *params, len = crypto_akcipher_maxsize(tfm); info->key_size = len * 8; - info->max_data_size = len; - info->max_sig_size = len; + if (strcmp(alg_name, "ecrdsa") == 0 || + strncmp(alg_name, "ecdsa-", 6) == 0) { + /* + * For these algos sig size is twice key size. + * keyctl uses max_sig_size as minimum input size, and + * max_data_size as minimum output size for a signature. + */ + info->max_data_size = len * 2; + info->max_sig_size = len * 2; + } else { + info->max_data_size = len; + info->max_sig_size = len; + } info->max_enc_size = len; info->max_dec_size = len; info->supported_ops = (KEYCTL_SUPPORTS_ENCRYPT | -- 2.33.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work 2022-02-02 6:59 [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work Vitaly Chikunov @ 2022-02-02 12:55 ` Stefan Berger 2022-02-02 21:24 ` Vitaly Chikunov 2022-02-20 23:31 ` Jarkko Sakkinen 2022-02-03 3:15 ` Stefan Berger 2022-02-20 23:25 ` Jarkko Sakkinen 2 siblings, 2 replies; 13+ messages in thread From: Stefan Berger @ 2022-02-02 12:55 UTC (permalink / raw) To: Vitaly Chikunov, keyrings, Jarkko Sakkinen, David Howells Cc: linux-crypto, linux-integrity, Eric Biggers On 2/2/22 01:59, Vitaly Chikunov wrote: > Rarely used `keyctl pkey_verify' can verify raw signatures, but was > failing, because ECDSA/EC-RDSA signature sizes are twice key sizes which > does not pass in/out sizes check in keyctl_pkey_params_get_2. > This in turn because these values cannot be distinguished by a single > `max_size' callback return value. > Also, `keyctl pkey_query` displays incorrect `max_sig_size' about these > algorithms. > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org> How do you use pkey_query? $ keyctl padd asymmetric testkey %keyring:test < cert.der 385037223 $ keyctl pkey_query 385037223 '' Password passing is not yet supported $ keyctl pkey_query 385037223 Format: keyctl --version keyctl add <type> <desc> <data> <keyring> [...] $ keyctl unlink 385037223 1 links removed ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work 2022-02-02 12:55 ` Stefan Berger @ 2022-02-02 21:24 ` Vitaly Chikunov 2022-02-02 22:38 ` Vitaly Chikunov 2022-02-03 0:07 ` Vitaly Chikunov 2022-02-20 23:31 ` Jarkko Sakkinen 1 sibling, 2 replies; 13+ messages in thread From: Vitaly Chikunov @ 2022-02-02 21:24 UTC (permalink / raw) To: Stefan Berger Cc: keyrings, Jarkko Sakkinen, David Howells, linux-crypto, linux-integrity, Eric Biggers Stefan, On Wed, Feb 02, 2022 at 07:55:43AM -0500, Stefan Berger wrote: > On 2/2/22 01:59, Vitaly Chikunov wrote: > > Rarely used `keyctl pkey_verify' can verify raw signatures, but was > > failing, because ECDSA/EC-RDSA signature sizes are twice key sizes which > > does not pass in/out sizes check in keyctl_pkey_params_get_2. > > This in turn because these values cannot be distinguished by a single > > `max_size' callback return value. > > Also, `keyctl pkey_query` displays incorrect `max_sig_size' about these > > algorithms. > > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org> > > How do you use pkey_query? > > $ keyctl padd asymmetric testkey %keyring:test < cert.der > 385037223 It should be (for RSA key): keyctl pkey_query 385037223 0 enc=pkcs1 hash=sha256 `0` is placeholder for a password. For example, I generated keys with your eckey-testing/generate.sh, and pkey_query after this patch is applied: # keyctl padd asymmetric "" @u < ecdsa-ca/ca.crt.der 66509339 # keyctl pkey_query 66509339 0 enc=x962 hash=sha256 key_size=256 max_data_size=64 max_sig_size=64 max_enc_size=32 max_dec_size=32 encrypt=y decrypt=n sign=n verify=y W/o patch max_data_size= and max_sig_size= will be 32. Thanks, > $ keyctl pkey_query 385037223 '' > Password passing is not yet supported > $ keyctl pkey_query 385037223 > Format: > keyctl --version > keyctl add <type> <desc> <data> <keyring> > [...] > > $ keyctl unlink 385037223 > 1 links removed > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work 2022-02-02 21:24 ` Vitaly Chikunov @ 2022-02-02 22:38 ` Vitaly Chikunov 2022-02-03 3:42 ` Stefan Berger 2022-02-03 0:07 ` Vitaly Chikunov 1 sibling, 1 reply; 13+ messages in thread From: Vitaly Chikunov @ 2022-02-02 22:38 UTC (permalink / raw) To: keyrings, Jarkko Sakkinen, David Howells, linux-crypto, linux-integrity, Eric Biggers Cc: Stefan Berger On Thu, Feb 03, 2022 at 12:24:37AM +0300, Vitaly Chikunov wrote: > Stefan, > > On Wed, Feb 02, 2022 at 07:55:43AM -0500, Stefan Berger wrote: > > On 2/2/22 01:59, Vitaly Chikunov wrote: > > > Rarely used `keyctl pkey_verify' can verify raw signatures, but was > > > failing, because ECDSA/EC-RDSA signature sizes are twice key sizes which > > > does not pass in/out sizes check in keyctl_pkey_params_get_2. > > > This in turn because these values cannot be distinguished by a single > > > `max_size' callback return value. > > > Also, `keyctl pkey_query` displays incorrect `max_sig_size' about these > > > algorithms. > > > > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org> > > > > How do you use pkey_query? > > > > $ keyctl padd asymmetric testkey %keyring:test < cert.der > > 385037223 > > It should be (for RSA key): > > keyctl pkey_query 385037223 0 enc=pkcs1 hash=sha256 > > `0` is placeholder for a password. > > For example, I generated keys with your eckey-testing/generate.sh, and > pkey_query after this patch is applied: > > # keyctl padd asymmetric "" @u < ecdsa-ca/ca.crt.der > 66509339 > # keyctl pkey_query 66509339 0 enc=x962 hash=sha256 > key_size=256 > max_data_size=64 > max_sig_size=64 > max_enc_size=32 > max_dec_size=32 I just thought, we can also set these to 0 if encrypt/decrypt is not enabled. Currently, there is no way to detect that encrypt is not possible, except by extending that if (strcmp...), but if we going to have it, why not correct other info too? Thanks, > encrypt=y > decrypt=n > sign=n > verify=y > > W/o patch max_data_size= and max_sig_size= will be 32. > > Thanks, > > > $ keyctl pkey_query 385037223 '' > > Password passing is not yet supported > > $ keyctl pkey_query 385037223 > > Format: > > keyctl --version > > keyctl add <type> <desc> <data> <keyring> > > [...] > > > > $ keyctl unlink 385037223 > > 1 links removed > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work 2022-02-02 22:38 ` Vitaly Chikunov @ 2022-02-03 3:42 ` Stefan Berger 0 siblings, 0 replies; 13+ messages in thread From: Stefan Berger @ 2022-02-03 3:42 UTC (permalink / raw) To: Vitaly Chikunov, keyrings, Jarkko Sakkinen, David Howells, linux-crypto, linux-integrity, Eric Biggers On 2/2/22 17:38, Vitaly Chikunov wrote: > On Thu, Feb 03, 2022 at 12:24:37AM +0300, Vitaly Chikunov wrote: >> Stefan, >> >> On Wed, Feb 02, 2022 at 07:55:43AM -0500, Stefan Berger wrote: >>> On 2/2/22 01:59, Vitaly Chikunov wrote: >>>> Rarely used `keyctl pkey_verify' can verify raw signatures, but was >>>> failing, because ECDSA/EC-RDSA signature sizes are twice key sizes which >>>> does not pass in/out sizes check in keyctl_pkey_params_get_2. >>>> This in turn because these values cannot be distinguished by a single >>>> `max_size' callback return value. >>>> Also, `keyctl pkey_query` displays incorrect `max_sig_size' about these >>>> algorithms. >>>> >>>> Signed-off-by: Vitaly Chikunov <vt@altlinux.org> >>> How do you use pkey_query? >>> >>> $ keyctl padd asymmetric testkey %keyring:test < cert.der >>> 385037223 >> It should be (for RSA key): >> >> keyctl pkey_query 385037223 0 enc=pkcs1 hash=sha256 >> >> `0` is placeholder for a password. >> >> For example, I generated keys with your eckey-testing/generate.sh, and >> pkey_query after this patch is applied: >> >> # keyctl padd asymmetric "" @u < ecdsa-ca/ca.crt.der >> 66509339 >> # keyctl pkey_query 66509339 0 enc=x962 hash=sha256 >> key_size=256 >> max_data_size=64 >> max_sig_size=64 >> max_enc_size=32 >> max_dec_size=32 > I just thought, we can also set these to 0 if encrypt/decrypt is not > enabled. Currently, there is no way to detect that encrypt is not > possible, except by extending that if (strcmp...), but if we going to > have it, why not correct other info too? Sounds good. This here works for signature verification for ECDSA now. [max_data_size = len * 2 was not enough] diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index 4fefb219bfdc..9e47a825b418 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -143,8 +143,26 @@ static int software_key_query(const struct kernel_pkey_params *params, len = crypto_akcipher_maxsize(tfm); info->key_size = len * 8; - info->max_data_size = len; - info->max_sig_size = len; + if (strcmp(alg_name, "ecrdsa") == 0) { + /* + * For these algos sig size is twice key size. + * keyctl uses max_sig_size as minimum input size, and + * max_data_size as minimum output size for a signature. + */ + info->max_data_size = len * 2; + info->max_sig_size = len * 2; + } else if (strncmp(alg_name, "ecdsa-", 6) == 0) { + /* + * ECDSA encodes the signature in ASN.1 sequence (2 bytes) + * with 2 bytes identifying each integer and 1 byte prefixed + * to each integer to make it a positive number. + */ + info->max_sig_size = 2 + (2 + 1 + len) * 2; + info->max_data_size = 2 + (2 + 1 + len) * 2; + } else { + info->max_data_size = len; + info->max_sig_size = len; + } info->max_enc_size = len; info->max_dec_size = len; info->supported_ops = (KEYCTL_SUPPORTS_ENCRYPT | ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work 2022-02-02 21:24 ` Vitaly Chikunov 2022-02-02 22:38 ` Vitaly Chikunov @ 2022-02-03 0:07 ` Vitaly Chikunov 1 sibling, 0 replies; 13+ messages in thread From: Vitaly Chikunov @ 2022-02-03 0:07 UTC (permalink / raw) To: keyrings, Jarkko Sakkinen, David Howells, linux-crypto, linux-integrity, Eric Biggers Cc: Stefan Berger JFYI, On Thu, Feb 03, 2022 at 12:24:37AM +0300, Vitaly Chikunov wrote: > On Wed, Feb 02, 2022 at 07:55:43AM -0500, Stefan Berger wrote: > > On 2/2/22 01:59, Vitaly Chikunov wrote: > > > Rarely used `keyctl pkey_verify' can verify raw signatures, but was > > > failing, because ECDSA/EC-RDSA signature sizes are twice key sizes which > > > does not pass in/out sizes check in keyctl_pkey_params_get_2. > > > This in turn because these values cannot be distinguished by a single > > > `max_size' callback return value. > > > Also, `keyctl pkey_query` displays incorrect `max_sig_size' about these > > > algorithms. > > > > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org> > > > > How do you use pkey_query? > > > > $ keyctl padd asymmetric testkey %keyring:test < cert.der > > 385037223 > > It should be (for RSA key): > > keyctl pkey_query 385037223 0 enc=pkcs1 hash=sha256 > > `0` is placeholder for a password. > > For example, I generated keys with your eckey-testing/generate.sh, and > pkey_query after this patch is applied: > > # keyctl padd asymmetric "" @u < ecdsa-ca/ca.crt.der > 66509339 > # keyctl pkey_query 66509339 0 enc=x962 hash=sha256 > key_size=256 > max_data_size=64 > max_sig_size=64 > max_enc_size=32 > max_dec_size=32 > encrypt=y > decrypt=n > sign=n > verify=y > > W/o patch max_data_size= and max_sig_size= will be 32. In case someone wants to reproduce `keyctl pkey_verify`, there are steps: 1. Keys are generated using ima-evm-utils tests suite, for example test-gost2012_512-A.cer: $ base64 test-gost2012_512-A.cer MIIB/DCCAWagAwIBAgIUK8+whWevr3FFkSdU9GLDAM7ure8wDAYIKoUDBwEBAwMFADARMQ8wDQYD VQQDDAZDQSBLZXkwIBcNMjIwMjAxMjIwOTQxWhgPMjA4MjEyMDUyMjA5NDFaMBExDzANBgNVBAMM BkNBIEtleTCBoDAXBggqhQMHAQEBAjALBgkqhQMHAQIBAgEDgYQABIGALXNrTJGgeErBUOov3Cfo IrHF9fcj8UjzwGeKCkbCcINzVUbdPmCopeJRHDJEvQBX1CQUPtlwDv6ANjTTRoq5nCk9L5PPFP1H z73JIXHT0eRBDVoWy0cWDRz1mmQlCnN2HThMtEloaQI81nTlKZOcEYDtDpi5WODmjEeRNQJMdqCj UDBOMAwGA1UdEwQFMAMBAf8wHQYDVR0OBBYEFCwfOITMbE9VisW1i2TYeu1tAo5QMB8GA1UdIwQY MBaAFCwfOITMbE9VisW1i2TYeu1tAo5QMAwGCCqFAwcBAQMDBQADgYEAmBfJCMTdC0/NSjz4BBiQ qDIEjomO7FEHYlkX5NGulcF8FaJW2jeyyXXtbpnub1IQ8af1KFIpwoS2e93LaaofxpWlpQLlju6m KYLOcO4xK3Whwa2hBAz9YbpUSFjvxnkS2/jpH2MsOSXuUEeCruG/RkHHB3ACef9umG6HCNQuAPY= 2. Hash and raw signature generated using openssl dgst: $ head -123c /dev/zero > foo.data $ openssl dgst -md_gost12_512 -out foo.hash512 -binary foo.data $ openssl dgst -md_gost12_512 -sign test-gost2012_512-A.key -out foo.sign512 -binary foo.data This may require configuring openssl to support engine, so there are resulting files: $ base64 foo.data AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA AAAAAAAAAAAA $ base64 foo.hash512 CZEpA3sP+swJIsahmsA2GX/GDp+4Ibt8c/Oli01NbTXmSyjbC0yivmouDW+LfqckewMKiKWCdNIE aY4cxfy4sQ== $ base64 foo.sign512 WNz9w7qmCvL/UG4uuCnj57C9udLRz1JXQLTOflpVa3fHPrp0qLBhoiLAdMtDr0AHPAsIlGS0vb9o vJIxtlGsimeqlAfffIpfmvu1oD/tqOT5NRa7xANT7tW2V9jiMRWt887dDSX+QBARcmXNwe07reoX Ko8xWMZ8xvOqWEuVPPw= 3. Then (with this patch applied, I run in virtme): # k=`keyctl padd asymmetric "" @u < test-gost2012_512-A.cer` # keyctl pkey_verify $k 0 foo.hash512 foo.sign512 enc=raw hash=streebog512 This gives exit 0. Test failure (due to wrong hash): # keyctl pkey_verify $k 0 foo.sign512 foo.sign512 enc=raw hash=streebog512 keyctl_pkey_verify: Bad message Thanks, ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work 2022-02-02 12:55 ` Stefan Berger 2022-02-02 21:24 ` Vitaly Chikunov @ 2022-02-20 23:31 ` Jarkko Sakkinen 1 sibling, 0 replies; 13+ messages in thread From: Jarkko Sakkinen @ 2022-02-20 23:31 UTC (permalink / raw) To: Stefan Berger Cc: Vitaly Chikunov, keyrings, David Howells, linux-crypto, linux-integrity, Eric Biggers On Wed, Feb 02, 2022 at 07:55:43AM -0500, Stefan Berger wrote: > > On 2/2/22 01:59, Vitaly Chikunov wrote: > > Rarely used `keyctl pkey_verify' can verify raw signatures, but was > > failing, because ECDSA/EC-RDSA signature sizes are twice key sizes which > > does not pass in/out sizes check in keyctl_pkey_params_get_2. > > This in turn because these values cannot be distinguished by a single > > `max_size' callback return value. > > Also, `keyctl pkey_query` displays incorrect `max_sig_size' about these > > algorithms. > > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org> > > How do you use pkey_query? > > $ keyctl padd asymmetric testkey %keyring:test < cert.der > 385037223 > $ keyctl pkey_query 385037223 '' > Password passing is not yet supported > $ keyctl pkey_query 385037223 > Format: > keyctl --version > keyctl add <type> <desc> <data> <keyring> > [...] > > $ keyctl unlink 385037223 > 1 links removed A keyctl transcript of the failing case would be really educating addition to the commit message (low-barrier to test this patch). BR, Jarkko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work 2022-02-02 6:59 [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work Vitaly Chikunov 2022-02-02 12:55 ` Stefan Berger @ 2022-02-03 3:15 ` Stefan Berger 2022-02-03 3:34 ` Vitaly Chikunov 2022-02-20 23:29 ` Jarkko Sakkinen 2022-02-20 23:25 ` Jarkko Sakkinen 2 siblings, 2 replies; 13+ messages in thread From: Stefan Berger @ 2022-02-03 3:15 UTC (permalink / raw) To: Vitaly Chikunov, keyrings, Jarkko Sakkinen, David Howells Cc: linux-crypto, linux-integrity, Eric Biggers On 2/2/22 01:59, Vitaly Chikunov wrote: > Rarely used `keyctl pkey_verify' can verify raw signatures, but was > failing, because ECDSA/EC-RDSA signature sizes are twice key sizes which > does not pass in/out sizes check in keyctl_pkey_params_get_2. > This in turn because these values cannot be distinguished by a single > `max_size' callback return value. > Also, `keyctl pkey_query` displays incorrect `max_sig_size' about these > algorithms. > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org> > --- > crypto/asymmetric_keys/public_key.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c > index 4fefb219bfdc..3ffbab07ed2a 100644 > --- a/crypto/asymmetric_keys/public_key.c > +++ b/crypto/asymmetric_keys/public_key.c > @@ -143,8 +143,19 @@ static int software_key_query(const struct kernel_pkey_params *params, > > len = crypto_akcipher_maxsize(tfm); > info->key_size = len * 8; > - info->max_data_size = len; > - info->max_sig_size = len; > + if (strcmp(alg_name, "ecrdsa") == 0 || > + strncmp(alg_name, "ecdsa-", 6) == 0) { > + /* > + * For these algos sig size is twice key size. > + * keyctl uses max_sig_size as minimum input size, and > + * max_data_size as minimum output size for a signature. > + */ > + info->max_data_size = len * 2; > + info->max_sig_size = len * 2; I don't know about the data size but following my tests this is not enough for ECDSA signature size. In ECDSA case the r and s components of the signature are encode in asn.1, not 'raw'. So there are 2 bytes at the beginning for sequence identifier , 2 bytes asn.1 for the r component, 1 additional 0-byte to make the r component always a positive number, then the r component, then 2 bytes asn.1 for the s component, 1 addition 0-byte to make the s component a positive number, then the s component. Phew. info->max_sig_size = 2 + (2 + 1 + len) * 2; so for NIST P384 it's: 2 + (2+1+48) * 2 = 104 Then it works for me as well. > + } else { > + info->max_data_size = len; > + info->max_sig_size = len; > + } > info->max_enc_size = len; > info->max_dec_size = len; > info->supported_ops = (KEYCTL_SUPPORTS_ENCRYPT | ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work 2022-02-03 3:15 ` Stefan Berger @ 2022-02-03 3:34 ` Vitaly Chikunov 2022-02-20 23:29 ` Jarkko Sakkinen 1 sibling, 0 replies; 13+ messages in thread From: Vitaly Chikunov @ 2022-02-03 3:34 UTC (permalink / raw) To: Stefan Berger Cc: keyrings, Jarkko Sakkinen, David Howells, linux-crypto, linux-integrity, Eric Biggers On Wed, Feb 02, 2022 at 10:15:24PM -0500, Stefan Berger wrote: > > On 2/2/22 01:59, Vitaly Chikunov wrote: > > Rarely used `keyctl pkey_verify' can verify raw signatures, but was > > failing, because ECDSA/EC-RDSA signature sizes are twice key sizes which > > does not pass in/out sizes check in keyctl_pkey_params_get_2. > > This in turn because these values cannot be distinguished by a single > > `max_size' callback return value. > > Also, `keyctl pkey_query` displays incorrect `max_sig_size' about these > > algorithms. > > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org> > > --- > > crypto/asymmetric_keys/public_key.c | 15 +++++++++++++-- > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c > > index 4fefb219bfdc..3ffbab07ed2a 100644 > > --- a/crypto/asymmetric_keys/public_key.c > > +++ b/crypto/asymmetric_keys/public_key.c > > @@ -143,8 +143,19 @@ static int software_key_query(const struct kernel_pkey_params *params, > > len = crypto_akcipher_maxsize(tfm); > > info->key_size = len * 8; > > - info->max_data_size = len; > > - info->max_sig_size = len; > > + if (strcmp(alg_name, "ecrdsa") == 0 || > > + strncmp(alg_name, "ecdsa-", 6) == 0) { > > + /* > > + * For these algos sig size is twice key size. > > + * keyctl uses max_sig_size as minimum input size, and > > + * max_data_size as minimum output size for a signature. > > + */ > > + info->max_data_size = len * 2; > > + info->max_sig_size = len * 2; > I don't know about the data size but following my tests this is not enough > for ECDSA signature size. In ECDSA case the r and s components of the > signature are encode in asn.1, not 'raw'. So there are 2 bytes at the > beginning for sequence identifier , 2 bytes asn.1 for the r component, 1 > additional 0-byte to make the r component always a positive number, then the > r component, then 2 bytes asn.1 for the s component, 1 addition 0-byte to > make the s component a positive number, then the s component. Phew. > > info->max_sig_size = 2 + (2 + 1 + len) * 2; > > so for NIST P384 it's: 2 + (2+1+48) * 2 = 104 > > Then it works for me as well. Well, another solution, without changing API, is that max_size() should return bigger size (to fit encoded signature), but in that case keyctl will think wrongly about key_size. Just for reference, keyctl_pkey_params_get_2 check that needs to be passed: case KEYCTL_PKEY_VERIFY: if (uparams.in_len > info.max_sig_size || uparams.out_len > info.max_data_size) return -EINVAL; So we can return arbitrarily big value, in theory. Thanks, > > > > + } else { > > + info->max_data_size = len; > > + info->max_sig_size = len; > > + } > > info->max_enc_size = len; > > info->max_dec_size = len; > > info->supported_ops = (KEYCTL_SUPPORTS_ENCRYPT | ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work 2022-02-03 3:15 ` Stefan Berger 2022-02-03 3:34 ` Vitaly Chikunov @ 2022-02-20 23:29 ` Jarkko Sakkinen 2022-02-21 2:43 ` Vitaly Chikunov 1 sibling, 1 reply; 13+ messages in thread From: Jarkko Sakkinen @ 2022-02-20 23:29 UTC (permalink / raw) To: Stefan Berger Cc: Vitaly Chikunov, keyrings, David Howells, linux-crypto, linux-integrity, Eric Biggers On Wed, Feb 02, 2022 at 10:15:24PM -0500, Stefan Berger wrote: > > On 2/2/22 01:59, Vitaly Chikunov wrote: > > Rarely used `keyctl pkey_verify' can verify raw signatures, but was > > failing, because ECDSA/EC-RDSA signature sizes are twice key sizes which > > does not pass in/out sizes check in keyctl_pkey_params_get_2. > > This in turn because these values cannot be distinguished by a single > > `max_size' callback return value. > > Also, `keyctl pkey_query` displays incorrect `max_sig_size' about these > > algorithms. > > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org> > > --- > > crypto/asymmetric_keys/public_key.c | 15 +++++++++++++-- > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c > > index 4fefb219bfdc..3ffbab07ed2a 100644 > > --- a/crypto/asymmetric_keys/public_key.c > > +++ b/crypto/asymmetric_keys/public_key.c > > @@ -143,8 +143,19 @@ static int software_key_query(const struct kernel_pkey_params *params, > > len = crypto_akcipher_maxsize(tfm); > > info->key_size = len * 8; > > - info->max_data_size = len; > > - info->max_sig_size = len; > > + if (strcmp(alg_name, "ecrdsa") == 0 || > > + strncmp(alg_name, "ecdsa-", 6) == 0) { > > + /* > > + * For these algos sig size is twice key size. > > + * keyctl uses max_sig_size as minimum input size, and > > + * max_data_size as minimum output size for a signature. > > + */ > > + info->max_data_size = len * 2; > > + info->max_sig_size = len * 2; > I don't know about the data size but following my tests this is not enough > for ECDSA signature size. In ECDSA case the r and s components of the > signature are encode in asn.1, not 'raw'. So there are 2 bytes at the > beginning for sequence identifier , 2 bytes asn.1 for the r component, 1 > additional 0-byte to make the r component always a positive number, then the > r component, then 2 bytes asn.1 for the s component, 1 addition 0-byte to > make the s component a positive number, then the s component. Phew. > > info->max_sig_size = 2 + (2 + 1 + len) * 2; > > so for NIST P384 it's: 2 + (2+1+48) * 2 = 104 > > Then it works for me as well. Thank you for the trouble of providing this great explanation. This reasoning should be included to the commit message for future reference. It would be also nice to encapsulate this calculation to an inline function. /Jarkko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work 2022-02-20 23:29 ` Jarkko Sakkinen @ 2022-02-21 2:43 ` Vitaly Chikunov 2022-02-25 19:47 ` Stefan Berger 0 siblings, 1 reply; 13+ messages in thread From: Vitaly Chikunov @ 2022-02-21 2:43 UTC (permalink / raw) To: Jarkko Sakkinen, Herbert Xu Cc: Stefan Berger, keyrings, David Howells, linux-crypto, linux-integrity, Eric Biggers On Mon, Feb 21, 2022 at 12:29:13AM +0100, Jarkko Sakkinen wrote: > On Wed, Feb 02, 2022 at 10:15:24PM -0500, Stefan Berger wrote: > > > > On 2/2/22 01:59, Vitaly Chikunov wrote: > > > Rarely used `keyctl pkey_verify' can verify raw signatures, but was > > > failing, because ECDSA/EC-RDSA signature sizes are twice key sizes which > > > does not pass in/out sizes check in keyctl_pkey_params_get_2. > > > This in turn because these values cannot be distinguished by a single > > > `max_size' callback return value. > > > Also, `keyctl pkey_query` displays incorrect `max_sig_size' about these > > > algorithms. > > > > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org> > > > --- > > > crypto/asymmetric_keys/public_key.c | 15 +++++++++++++-- > > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > > > diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c > > > index 4fefb219bfdc..3ffbab07ed2a 100644 > > > --- a/crypto/asymmetric_keys/public_key.c > > > +++ b/crypto/asymmetric_keys/public_key.c > > > @@ -143,8 +143,19 @@ static int software_key_query(const struct kernel_pkey_params *params, > > > len = crypto_akcipher_maxsize(tfm); > > > info->key_size = len * 8; > > > - info->max_data_size = len; > > > - info->max_sig_size = len; > > > + if (strcmp(alg_name, "ecrdsa") == 0 || > > > + strncmp(alg_name, "ecdsa-", 6) == 0) { > > > + /* > > > + * For these algos sig size is twice key size. > > > + * keyctl uses max_sig_size as minimum input size, and > > > + * max_data_size as minimum output size for a signature. > > > + */ > > > + info->max_data_size = len * 2; > > > + info->max_sig_size = len * 2; > > I don't know about the data size but following my tests this is not enough > > for ECDSA signature size. In ECDSA case the r and s components of the > > signature are encode in asn.1, not 'raw'. So there are 2 bytes at the > > beginning for sequence identifier , 2 bytes asn.1 for the r component, 1 > > additional 0-byte to make the r component always a positive number, then the > > r component, then 2 bytes asn.1 for the s component, 1 addition 0-byte to > > make the s component a positive number, then the s component. Phew. > > > > info->max_sig_size = 2 + (2 + 1 + len) * 2; > > > > so for NIST P384 it's: 2 + (2+1+48) * 2 = 104 > > > > Then it works for me as well. > > Thank you for the trouble of providing this great explanation. This > reasoning should be included to the commit message for future reference. > > It would be also nice to encapsulate this calculation to an inline > function. I wanted to discuss if there's a better way to do it. For example, instead of calculating algorithm specific information in software_key_query maybe we should extend akcipher_alg API with a pkey_params request (just for keyctl)? Also, there possible other solution - instead of setting info in software_key_query depending on algo (as in this patch), it's possible (in a hackish way) just to return larger value from akcipher_alg::max_size. But this will possible somewhat confuse keyctl users, as, for example, they will see arbitrary value for a key_size. Currently, this patch is the simplest non-confusing solution, and it's in accord with how we calculate algorithm specific things all around the code base (outside of algorithm implementation itself). Thanks, > > /Jarkko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work 2022-02-21 2:43 ` Vitaly Chikunov @ 2022-02-25 19:47 ` Stefan Berger 0 siblings, 0 replies; 13+ messages in thread From: Stefan Berger @ 2022-02-25 19:47 UTC (permalink / raw) To: Vitaly Chikunov, Jarkko Sakkinen, Herbert Xu Cc: keyrings, David Howells, linux-crypto, linux-integrity, Eric Biggers On 2/20/22 21:43, Vitaly Chikunov wrote: > On Mon, Feb 21, 2022 at 12:29:13AM +0100, Jarkko Sakkinen wrote: >> On Wed, Feb 02, 2022 at 10:15:24PM -0500, Stefan Berger wrote: >>> On 2/2/22 01:59, Vitaly Chikunov wrote: >>>> Rarely used `keyctl pkey_verify' can verify raw signatures, but was >>>> failing, because ECDSA/EC-RDSA signature sizes are twice key sizes which >>>> does not pass in/out sizes check in keyctl_pkey_params_get_2. >>>> This in turn because these values cannot be distinguished by a single >>>> `max_size' callback return value. >>>> Also, `keyctl pkey_query` displays incorrect `max_sig_size' about these >>>> algorithms. >>>> >>>> Signed-off-by: Vitaly Chikunov <vt@altlinux.org> >>>> --- >>>> crypto/asymmetric_keys/public_key.c | 15 +++++++++++++-- >>>> 1 file changed, 13 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c >>>> index 4fefb219bfdc..3ffbab07ed2a 100644 >>>> --- a/crypto/asymmetric_keys/public_key.c >>>> +++ b/crypto/asymmetric_keys/public_key.c >>>> @@ -143,8 +143,19 @@ static int software_key_query(const struct kernel_pkey_params *params, >>>> len = crypto_akcipher_maxsize(tfm); >>>> info->key_size = len * 8; >>>> - info->max_data_size = len; >>>> - info->max_sig_size = len; >>>> + if (strcmp(alg_name, "ecrdsa") == 0 || >>>> + strncmp(alg_name, "ecdsa-", 6) == 0) { >>>> + /* >>>> + * For these algos sig size is twice key size. >>>> + * keyctl uses max_sig_size as minimum input size, and >>>> + * max_data_size as minimum output size for a signature. >>>> + */ >>>> + info->max_data_size = len * 2; >>>> + info->max_sig_size = len * 2; >>> I don't know about the data size but following my tests this is not enough >>> for ECDSA signature size. In ECDSA case the r and s components of the >>> signature are encode in asn.1, not 'raw'. So there are 2 bytes at the >>> beginning for sequence identifier , 2 bytes asn.1 for the r component, 1 >>> additional 0-byte to make the r component always a positive number, then the >>> r component, then 2 bytes asn.1 for the s component, 1 addition 0-byte to >>> make the s component a positive number, then the s component. Phew. >>> >>> info->max_sig_size = 2 + (2 + 1 + len) * 2; >>> >>> so for NIST P384 it's: 2 + (2+1+48) * 2 = 104 >>> >>> Then it works for me as well. >> Thank you for the trouble of providing this great explanation. This >> reasoning should be included to the commit message for future reference. >> >> It would be also nice to encapsulate this calculation to an inline >> function. > I wanted to discuss if there's a better way to do it. For example, > instead of calculating algorithm specific information in > software_key_query maybe we should extend akcipher_alg API with a > pkey_params request (just for keyctl)? > > Also, there possible other solution - instead of setting info in > software_key_query depending on algo (as in this patch), it's possible > (in a hackish way) just to return larger value from > akcipher_alg::max_size. But this will possible somewhat confuse keyctl > users, as, for example, they will see arbitrary value for a key_size. > > Currently, this patch is the simplest non-confusing solution, and it's > in accord with how we calculate algorithm specific things all around > the code base (outside of algorithm implementation itself). I don't know the answer to the other questions, but I agree that your patch seem to be the simplest non-confusing solution. Are you going to repost it, possibly with my ECDSA modifications added? Stefan > > Thanks, > >> /Jarkko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work 2022-02-02 6:59 [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work Vitaly Chikunov 2022-02-02 12:55 ` Stefan Berger 2022-02-03 3:15 ` Stefan Berger @ 2022-02-20 23:25 ` Jarkko Sakkinen 2 siblings, 0 replies; 13+ messages in thread From: Jarkko Sakkinen @ 2022-02-20 23:25 UTC (permalink / raw) To: Vitaly Chikunov Cc: keyrings, David Howells, linux-crypto, linux-integrity, Stefan Berger, Eric Biggers On Wed, Feb 02, 2022 at 09:59:06AM +0300, Vitaly Chikunov wrote: > Rarely used `keyctl pkey_verify' can verify raw signatures, but was > failing, because ECDSA/EC-RDSA signature sizes are twice key sizes which > does not pass in/out sizes check in keyctl_pkey_params_get_2. > This in turn because these values cannot be distinguished by a single > `max_size' callback return value. > Also, `keyctl pkey_query` displays incorrect `max_sig_size' about these > algorithms. > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org> Please provide a fixes tag and describe your changes. BR, Jarkko ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-02-25 19:47 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-02 6:59 [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work Vitaly Chikunov 2022-02-02 12:55 ` Stefan Berger 2022-02-02 21:24 ` Vitaly Chikunov 2022-02-02 22:38 ` Vitaly Chikunov 2022-02-03 3:42 ` Stefan Berger 2022-02-03 0:07 ` Vitaly Chikunov 2022-02-20 23:31 ` Jarkko Sakkinen 2022-02-03 3:15 ` Stefan Berger 2022-02-03 3:34 ` Vitaly Chikunov 2022-02-20 23:29 ` Jarkko Sakkinen 2022-02-21 2:43 ` Vitaly Chikunov 2022-02-25 19:47 ` Stefan Berger 2022-02-20 23:25 ` Jarkko Sakkinen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.