diff mbox series

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

Message ID 20201005225258.200181-1-anatol.pomozov@gmail.com (mailing list archive)
State New
Headers show
Series [v2] encrypted-keys: Print more useful debug info if encryption algo is not available | expand

Commit Message

Anatol Pomozov Oct. 5, 2020, 10:52 p.m. UTC
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(-)

Comments

Jarkko Sakkinen Oct. 6, 2020, 3:56 p.m. UTC | #1
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
Jarkko Sakkinen Oct. 6, 2020, 3:56 p.m. UTC | #2
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
Anatol Pomozov Oct. 6, 2020, 5:18 p.m. UTC | #3
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.
Jarkko Sakkinen Oct. 6, 2020, 11:39 p.m. UTC | #4
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
Mimi Zohar Oct. 7, 2020, 12:33 a.m. UTC | #5
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
Jarkko Sakkinen Oct. 7, 2020, 3:15 a.m. UTC | #6
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
Mimi Zohar Oct. 12, 2020, 8:18 p.m. UTC | #7
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);
diff mbox series

Patch

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