KEYS: revert requiring signature "encoding"
diff mbox series

Message ID 1541743166-5036-1-git-send-email-zohar@linux.ibm.com
State New
Headers show
Series
  • KEYS: revert requiring signature "encoding"
Related show

Commit Message

Mimi Zohar Nov. 9, 2018, 5:59 a.m. UTC
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(-)

Comments

David Howells Nov. 9, 2018, 1:16 p.m. UTC | #1
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
Mimi Zohar Nov. 9, 2018, 2:14 p.m. UTC | #2
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
David Howells Nov. 9, 2018, 3:05 p.m. UTC | #3
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
David Howells Nov. 9, 2018, 3:06 p.m. UTC | #4
And can you cc: Denis Kenzior <denkenz@gmail.com> too?

David
David Howells Nov. 9, 2018, 3:28 p.m. UTC | #5
Actually, a better solution would be to set the encoding in the caller, say in
asymmetric_verify() in digsig_asymmetric.c.

David

Patch
diff mbox series

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;