On 12/14/17 at 06:25pm, Dave Young wrote: > On 11/18/17 at 12:47pm, Dave Young wrote: > > Commit d3bfe84129f6 introduced secondary_trusted_keys keyring, current > > users of verify_pkcs7_signature are below: > > net/wireless/reg.c : uses its own trusted_keys > > kernel/module_signing.c : pass NULL trusted_keys > > crypto/asymmetric_keys/verify_pefile.c : pass NULL trusted_keys > > > > For both module and pefile verification, there is no reason to use builtin > > keys only. Actually in Fedora kernel module signing code passes 1UL, but > > kexec code does not pass 1UL for pefile verification thus we have below bug > > https://bugzilla.redhat.com/show_bug.cgi?id=1470995 > > > > Drop the hard code 1UL checking so that pefile verification can use > > secondary keyring as well. > > > > Signed-off-by: Dave Young <dyoung@xxxxxxxxxx> > > --- > > certs/system_keyring.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > --- linux-x86.orig/certs/system_keyring.c > > +++ linux-x86/certs/system_keyring.c > > @@ -229,8 +229,6 @@ int verify_pkcs7_signature(const void *d > > goto error; > > > > if (!trusted_keys) { > > - trusted_keys = builtin_trusted_keys; > > - } else if (trusted_keys == (void *)1UL) { > > #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING > > trusted_keys = secondary_trusted_keys; > > #else > > Another ping. > > If the (-1UL) is really needed, below file need update to use it > But I think it is ugly.. > crypto/asymmetric_keys/verify_pefile.c Ping again. Can anyone response to this issue? Let me describe more details about the problem: In Fedora kernel there is a patch below which is not upstreamed: https://src.fedoraproject.org/rpms/kernel/blob/master/f/Fix-for-module-sig-verification.patch It has below changes: --- diff --git a/kernel/module_signing.c b/kernel/module_signing.c index 937c844..d3d6f95 100644 --- a/kernel/module_signing.c +++ b/kernel/module_signing.c @@ -81,6 +81,6 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen) } return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len, - NULL, VERIFYING_MODULE_SIGNATURE, + (void *)1UL, VERIFYING_MODULE_SIGNATURE, NULL, NULL); } --- Above change is needed because the verify_pkcs7_signature is doing below: --- if (!trusted_keys) { trusted_keys = builtin_trusted_keys; } else if (trusted_keys == (void *)1UL) { #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING trusted_keys = secondary_trusted_keys; #else trusted_keys = builtin_trusted_keys; #endif } --- The trusted_keys is an argument passed to verify_pkcs7_signature function. We can see that users of this function must pass "-1UL" as trusted_keys to use secondary keyring. This "-1UL" is not documented and it looks a hardcode api. Besides of the module signing code, actually as I mentioned in the patch log kexec/kdump also need passing "-1UL" to use the secondary keyring. But why do we need that hack? If I understand it correctly if use secondary then builtin can still be used, see commit log of d3bfe84129f65e0af2450743ebdab33d161d01c9: If the secondary keyring is enabled, a link is created from that to .builtin_trusted_keys so that the the latter will automatically be searched too if the secondary keyring is searched. So why not directly use secondary in case trusted_keys == NULL? I'm not familar with the certs/keyring code, if I'm wrong please correct me. -- Thanks Dave _______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec
