[PATCH v2] encrypted-keys: Print more useful debug info if encryption algo is not available

[PATCH v2] encrypted-keys: Print more useful debug info if encryption algo is not available

[Anatol Pomozov]

It helps to improve a cryptic message "encrypted_key failed to alloc_cipher (-2)".
Adding algo name makes it easier to understand what cipher has failed.

Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
---
 security/keys/encrypted-keys/encrypted.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 192e531c146f..c09d48f53682 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -84,8 +84,8 @@ static int aes_get_sizes(void)
 
 	tfm = crypto_alloc_skcipher(blkcipher_alg, 0, CRYPTO_ALG_ASYNC);
 	if (IS_ERR(tfm)) {
-		pr_err("encrypted_key: failed to alloc_cipher (%ld)\n",
-		       PTR_ERR(tfm));
+		pr_err("encrypted_key: failed to alloc_cipher for %s (%ld)\n",
+		       blkcipher_alg, PTR_ERR(tfm));
 		return PTR_ERR(tfm);
 	}
 	ivsize = crypto_skcipher_ivsize(tfm);
-- 
2.28.0

Re: [PATCH v2] encrypted-keys: Print more useful debug info if encryption algo is not available

[Jarkko Sakkinen]

On Mon, Oct 05, 2020 at 03:52:58PM -0700, Anatol Pomozov wrote:
> It helps to improve a cryptic message "encrypted_key failed to alloc_cipher (-2)".
> Adding algo name makes it easier to understand what cipher has failed.
> 
> Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>

NAK, because you are missing David Howells from the CC list.

/Jarkko

Re: [PATCH v2] encrypted-keys: Print more useful debug info if encryption algo is not available

[Jarkko Sakkinen]

On Tue, Oct 06, 2020 at 06:56:28PM +0300, Jarkko Sakkinen wrote:
> On Mon, Oct 05, 2020 at 03:52:58PM -0700, Anatol Pomozov wrote:
> > It helps to improve a cryptic message "encrypted_key failed to alloc_cipher (-2)".
> > Adding algo name makes it easier to understand what cipher has failed.
> > 
> > Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
> 
> NAK, because you are missing David Howells from the CC list.

Oh and also me. You are essentially missing all the keyring maintainers.

/Jarkko

Re: [PATCH v2] encrypted-keys: Print more useful debug info if encryption algo is not available

[Anatol Pomozov]

Hi

On Tue, Oct 6, 2020 at 8:59 AM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Tue, Oct 06, 2020 at 06:56:28PM +0300, Jarkko Sakkinen wrote:
> > On Mon, Oct 05, 2020 at 03:52:58PM -0700, Anatol Pomozov wrote:
> > > It helps to improve a cryptic message "encrypted_key failed to alloc_cipher (-2)".
> > > Adding algo name makes it easier to understand what cipher has failed.
> > >
> > > Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
> >
> > NAK, because you are missing David Howells from the CC list.
>
> Oh and also me. You are essentially missing all the keyring maintainers.

The MAINTAINERS file states following:

KEYS-ENCRYPTED
M:      Mimi Zohar <zohar@linux.ibm.com>
L:      linux-integrity@vger.kernel.org
L:      keyrings@vger.kernel.org
S:      Supported
F:      Documentation/security/keys/trusted-encrypted.rst
F:      include/keys/encrypted-type.h
F:      security/keys/encrypted-keys/

Everything seems fine as I included the official maintainer and the
project maillist.

If David is not subscribed to the project maillist I'll be glad to CC
him as well.

Re: [PATCH v2] encrypted-keys: Print more useful debug info if encryption algo is not available

[Jarkko Sakkinen]

On Tue, Oct 06, 2020 at 10:18:43AM -0700, Anatol Pomozov wrote:
> Hi
> 
> On Tue, Oct 6, 2020 at 8:59 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Tue, Oct 06, 2020 at 06:56:28PM +0300, Jarkko Sakkinen wrote:
> > > On Mon, Oct 05, 2020 at 03:52:58PM -0700, Anatol Pomozov wrote:
> > > > It helps to improve a cryptic message "encrypted_key failed to alloc_cipher (-2)".
> > > > Adding algo name makes it easier to understand what cipher has failed.
> > > >
> > > > Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
> > >
> > > NAK, because you are missing David Howells from the CC list.
> >
> > Oh and also me. You are essentially missing all the keyring maintainers.
> 
> The MAINTAINERS file states following:
> 
> KEYS-ENCRYPTED
> M:      Mimi Zohar <zohar@linux.ibm.com>
> L:      linux-integrity@vger.kernel.org
> L:      keyrings@vger.kernel.org
> S:      Supported
> F:      Documentation/security/keys/trusted-encrypted.rst
> F:      include/keys/encrypted-type.h
> F:      security/keys/encrypted-keys/
> 
> Everything seems fine as I included the official maintainer and the
> project maillist.
> 
> If David is not subscribed to the project maillist I'll be glad to CC
> him as well.

Ugh, you are right then. Those two lists still confuse me thought
but that is not your fault.

Based on that I can give my ack because the change looks right
still.

Acked-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Thanks for explaining the situation.

/Jarkko

Re: [PATCH v2] encrypted-keys: Print more useful debug info if encryption algo is not available

[Mimi Zohar]

On Wed, 2020-10-07 at 02:39 +0300, Jarkko Sakkinen wrote:
> On Tue, Oct 06, 2020 at 10:18:43AM -0700, Anatol Pomozov wrote:
> > Hi
> > 
> > On Tue, Oct 6, 2020 at 8:59 AM Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com> wrote:
> > >
> > > On Tue, Oct 06, 2020 at 06:56:28PM +0300, Jarkko Sakkinen wrote:
> > > > On Mon, Oct 05, 2020 at 03:52:58PM -0700, Anatol Pomozov wrote:
> > > > > It helps to improve a cryptic message "encrypted_key failed to alloc_cipher (-2)".
> > > > > Adding algo name makes it easier to understand what cipher has failed.
> > > > >
> > > > > Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
> > > >
> > > > NAK, because you are missing David Howells from the CC list.
> > >
> > > Oh and also me. You are essentially missing all the keyring maintainers.
> > 
> > The MAINTAINERS file states following:
> > 
> > KEYS-ENCRYPTED
> > M:      Mimi Zohar <zohar@linux.ibm.com>
> > L:      linux-integrity@vger.kernel.org
> > L:      keyrings@vger.kernel.org
> > S:      Supported
> > F:      Documentation/security/keys/trusted-encrypted.rst
> > F:      include/keys/encrypted-type.h
> > F:      security/keys/encrypted-keys/
> > 
> > Everything seems fine as I included the official maintainer and the
> > project maillist.
> > 
> > If David is not subscribed to the project maillist I'll be glad to CC
> > him as well.
> 
> Ugh, you are right then. Those two lists still confuse me thought
> but that is not your fault.

Please refer to Documentation/security/keys/trusted-encrypted.rst for
an explanation.

> 
> Based on that I can give my ack because the change looks right
> still.
> 
> Acked-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> 
> Thanks for explaining the situation.

Thanks, Jarrko.  I'm on vacation, returning next week.

Mimi

Re: [PATCH v2] encrypted-keys: Print more useful debug info if encryption algo is not available

[Jarkko Sakkinen]

On Tue, Oct 06, 2020 at 08:33:24PM -0400, Mimi Zohar wrote:
> On Wed, 2020-10-07 at 02:39 +0300, Jarkko Sakkinen wrote:
> > On Tue, Oct 06, 2020 at 10:18:43AM -0700, Anatol Pomozov wrote:
> > > Hi
> > > 
> > > On Tue, Oct 6, 2020 at 8:59 AM Jarkko Sakkinen
> > > <jarkko.sakkinen@linux.intel.com> wrote:
> > > >
> > > > On Tue, Oct 06, 2020 at 06:56:28PM +0300, Jarkko Sakkinen wrote:
> > > > > On Mon, Oct 05, 2020 at 03:52:58PM -0700, Anatol Pomozov wrote:
> > > > > > It helps to improve a cryptic message "encrypted_key failed to alloc_cipher (-2)".
> > > > > > Adding algo name makes it easier to understand what cipher has failed.
> > > > > >
> > > > > > Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
> > > > >
> > > > > NAK, because you are missing David Howells from the CC list.
> > > >
> > > > Oh and also me. You are essentially missing all the keyring maintainers.
> > > 
> > > The MAINTAINERS file states following:
> > > 
> > > KEYS-ENCRYPTED
> > > M:      Mimi Zohar <zohar@linux.ibm.com>
> > > L:      linux-integrity@vger.kernel.org
> > > L:      keyrings@vger.kernel.org
> > > S:      Supported
> > > F:      Documentation/security/keys/trusted-encrypted.rst
> > > F:      include/keys/encrypted-type.h
> > > F:      security/keys/encrypted-keys/
> > > 
> > > Everything seems fine as I included the official maintainer and the
> > > project maillist.
> > > 
> > > If David is not subscribed to the project maillist I'll be glad to CC
> > > him as well.
> > 
> > Ugh, you are right then. Those two lists still confuse me thought
> > but that is not your fault.
> 
> Please refer to Documentation/security/keys/trusted-encrypted.rst for
> an explanation.

Yeah, I was not sure about the organization and just spotted keyrings
in the CC list :-)

> 
> > 
> > Based on that I can give my ack because the change looks right
> > still.
> > 
> > Acked-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > 
> > Thanks for explaining the situation.
> 
> Thanks, Jarrko.  I'm on vacation, returning next week.

Have a good one!

> Mimi

/Jarkko

Re: [PATCH v2] encrypted-keys: Print more useful debug info if encryption algo is not available

[Mimi Zohar]

Hi Anatol,

On Mon, 2020-10-05 at 15:52 -0700, Anatol Pomozov wrote:
> It helps to improve a cryptic message "encrypted_key failed to alloc_cipher (-2)".
> Adding algo name makes it easier to understand what cipher has failed.
> 
> Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>

The "if" clause in the Subject line doesn't belong there, but in the
patch description.  I would start the patch description with "Improve
the cryptic message ... by adding ..."

> ---
>  security/keys/encrypted-keys/encrypted.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
> index 192e531c146f..c09d48f53682 100644
> --- a/security/keys/encrypted-keys/encrypted.c
> +++ b/security/keys/encrypted-keys/encrypted.c
> @@ -84,8 +84,8 @@ static int aes_get_sizes(void)
>  
>  	tfm = crypto_alloc_skcipher(blkcipher_alg, 0, CRYPTO_ALG_ASYNC);
>  	if (IS_ERR(tfm)) {
> -		pr_err("encrypted_key: failed to alloc_cipher (%ld)\n",
> -		       PTR_ERR(tfm));
> +		pr_err("encrypted_key: failed to alloc_cipher for %s (%ld)\n",
> +		       blkcipher_alg, PTR_ERR(tfm));

I don't have a problem with including the blkcipher_alg in the error
message.   It is currently defined as "cbc(aes)".   Is it ever anything
else?

thanks,

Mimi

>  		return PTR_ERR(tfm);
>  	}
>  	ivsize = crypto_skcipher_ivsize(tfm);