linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] KEYS: revert requiring signature "encoding"
@ 2018-11-09  5:59 Mimi Zohar
  2018-11-09 13:16 ` David Howells
  2018-11-09 15:28 ` David Howells
  0 siblings, 2 replies; 6+ messages in thread
From: Mimi Zohar @ 2018-11-09  5:59 UTC (permalink / raw)
  To: David Howells
  Cc: Mimi Zohar, keyrings, linux-integrity, linux-security-module

Attempting to verify IMA signatures fail causing the system to hang.

Fixes: commit 82f94f24475c ("KEYS: Provide software public key query
       function [ver #2]")
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 crypto/asymmetric_keys/public_key.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index f5d85b47fcc6..f90360122090 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -239,9 +239,17 @@ int public_key_verify_signature(const struct public_key *pkey,
 	BUG_ON(!sig);
 	BUG_ON(!sig->s);
 
-	ret = software_key_determine_akcipher(sig->encoding,
-					      sig->hash_algo,
-					      pkey, alg_name);
+	if (!sig->digest)
+		return -ENOPKG;
+
+	if (!(sig->encoding) && strcmp(pkey->pkey_algo, "rsa") == 0)
+		ret = software_key_determine_akcipher("pkcs1",
+						      sig->hash_algo,
+						      pkey, alg_name);
+	else
+		ret = software_key_determine_akcipher(sig->encoding,
+						      sig->hash_algo,
+						      pkey, alg_name);
 	if (ret < 0)
 		return ret;
 
-- 
2.7.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] KEYS: revert requiring signature "encoding"
  2018-11-09  5:59 [PATCH] KEYS: revert requiring signature "encoding" Mimi Zohar
@ 2018-11-09 13:16 ` David Howells
  2018-11-09 14:14   ` Mimi Zohar
                     ` (2 more replies)
  2018-11-09 15:28 ` David Howells
  1 sibling, 3 replies; 6+ messages in thread
From: David Howells @ 2018-11-09 13:16 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: dhowells, keyrings, linux-integrity, linux-security-module

Mimi Zohar <zohar@linux.ibm.com> wrote:

> Attempting to verify IMA signatures fail causing the system to hang.

Can you say why?

> Fixes: commit 82f94f24475c ("KEYS: Provide software public key query
>        function [ver #2]")

Btw, no word "commit" after Fixes:.

David

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KEYS: revert requiring signature "encoding"
  2018-11-09 13:16 ` David Howells
@ 2018-11-09 14:14   ` Mimi Zohar
  2018-11-09 15:05   ` David Howells
  2018-11-09 15:06   ` David Howells
  2 siblings, 0 replies; 6+ messages in thread
From: Mimi Zohar @ 2018-11-09 14:14 UTC (permalink / raw)
  To: David Howells; +Cc: keyrings, linux-integrity, linux-security-module

On Fri, 2018-11-09 at 13:16 +0000, David Howells wrote:
> Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> > Attempting to verify IMA signatures fail causing the system to hang.
> 
> Can you say why?

On systems with IMA-appraisal enabled with a policy requiring file 
signatures, the "good" signature values are stored on the filesystem
as extended attributes (security.ima).  Signature verification failure
would normally be limited to just a particular file (eg. executable),
but during boot signature verification failure could result in a
system hang.

Removing existing signature formats breaks existing systems.  This
patch adds support for RSA signatures without an explicit "pkcs1" sig-
>encoding.

> 
> > Fixes: commit 82f94f24475c ("KEYS: Provide software public key query
> >        function [ver #2]")
> 
> Btw, no word "commit" after Fixes:.

Ok.

Looking the patch over again, do you prefer the duplicate call or
defining a local variable and using the ternary conditional operator
("?:") like this:

	bool rsa = false;

	if (!(sig->encoding) && strcmp(pkey->pkey_algo, "rsa") == 0)
                rsa = true;

        ret = software_key_determine_akcipher(rsa ? "pkcs1" : sig->encoding,
                                              sig->hash_algo,
                                              pkey, alg_name);

Mimi



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KEYS: revert requiring signature "encoding"
  2018-11-09 13:16 ` David Howells
  2018-11-09 14:14   ` Mimi Zohar
@ 2018-11-09 15:05   ` David Howells
  2018-11-09 15:06   ` David Howells
  2 siblings, 0 replies; 6+ messages in thread
From: David Howells @ 2018-11-09 15:05 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: dhowells, keyrings, linux-integrity, linux-security-module

Mimi Zohar <zohar@linux.ibm.com> wrote:

> Looking the patch over again, do you prefer the duplicate call or
> defining a local variable and using the ternary conditional operator
> ("?:") like this:
> 
> 	bool rsa = false;
> 
> 	if (!(sig->encoding) && strcmp(pkey->pkey_algo, "rsa") == 0)
>                 rsa = true;
> 
>         ret = software_key_determine_akcipher(rsa ? "pkcs1" : sig->encoding,
>                                               sig->hash_algo,
>                                               pkey, alg_name);

Might be better to do:

	const char *encoding = sig->encoding;

	if (!encoding && strcmp(pkey->pkey_algo, "rsa") == 0)
		encoding = "pkcs1";

	ret = software_key_determine_akcipher(encoding, ...

David

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KEYS: revert requiring signature "encoding"
  2018-11-09 13:16 ` David Howells
  2018-11-09 14:14   ` Mimi Zohar
  2018-11-09 15:05   ` David Howells
@ 2018-11-09 15:06   ` David Howells
  2 siblings, 0 replies; 6+ messages in thread
From: David Howells @ 2018-11-09 15:06 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: dhowells, keyrings, linux-integrity, linux-security-module,
	Denis Kenzior

And can you cc: Denis Kenzior <denkenz@gmail.com> too?

David

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KEYS: revert requiring signature "encoding"
  2018-11-09  5:59 [PATCH] KEYS: revert requiring signature "encoding" Mimi Zohar
  2018-11-09 13:16 ` David Howells
@ 2018-11-09 15:28 ` David Howells
  1 sibling, 0 replies; 6+ messages in thread
From: David Howells @ 2018-11-09 15:28 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: dhowells, keyrings, linux-integrity, linux-security-module,
	Denis Kenzior

Actually, a better solution would be to set the encoding in the caller, say in
asymmetric_verify() in digsig_asymmetric.c.

David

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-11-09 15:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-09  5:59 [PATCH] KEYS: revert requiring signature "encoding" Mimi Zohar
2018-11-09 13:16 ` David Howells
2018-11-09 14:14   ` Mimi Zohar
2018-11-09 15:05   ` David Howells
2018-11-09 15:06   ` David Howells
2018-11-09 15:28 ` David Howells

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).