[RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work

[RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work

[Vitaly Chikunov]

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

Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work

[Stefan Berger]

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

Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work

[Vitaly Chikunov]

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
> 

Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work

[Vitaly Chikunov]

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
> > 

Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work

[Vitaly Chikunov]

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,

Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work

[Stefan Berger]

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 |

Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work

[Vitaly Chikunov]

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 |

Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work

[Stefan Berger]

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 |

Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work

[Jarkko Sakkinen]

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

Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work

[Jarkko Sakkinen]

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

Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work

[Jarkko Sakkinen]

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

Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work

[Vitaly Chikunov]

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

Re: [RFC PATCH] KEYS: Double max_size to make keyctl pkey_verify work

[Stefan Berger]

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