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 <[email protected]>
---
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
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 <[email protected]>
> >
> > 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
> >
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 <[email protected]>
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
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 <[email protected]>
> >
> > 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,
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 <[email protected]>
>
> 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
>
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 <[email protected]>
>>> 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 |
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 <[email protected]>
> ---
> 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 |
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 <[email protected]>
> > ---
> > 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 |
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 <[email protected]>
>
> 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
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 <[email protected]>
> > ---
> > 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
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 <[email protected]>
Please provide a fixes tag and describe your changes.
BR, Jarkko
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 <[email protected]>
> > > ---
> > > 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
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 <[email protected]>
>>>> ---
>>>> 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