| Message ID | 1541743166-5036-1-git-send-email-zohar@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KEYS: revert requiring signature "encoding" | expand |
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
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
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
And can you cc: Denis Kenzior <denkenz@gmail.com> too? David
Actually, a better solution would be to set the encoding in the caller, say in asymmetric_verify() in digsig_asymmetric.c. David
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;
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(-)