| Message ID | 20181026175516.21251-2-seth.forshee@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2,Cosmic] UBUNTU: SAUCE: (efi-lockdown) module: trust keys from secondary keyring for module signing | expand |
On Fri, Oct 26, 2018 at 11:55:15AM -0600, Seth Forshee wrote: > BugLink: https://bugs.launchpad.net/bugs/1798863 > > For signing dkms modules we use a machine owner key whose public > half is enrolled into shim. This gets imported into the kernel's > secondary keyring, thus keys in this keyring need to be trusted > for module signing. > > Unfortunately the revision of the "secure boot lockdown" patches > imported into cosmic had a bug whereby keys in the secondary > keyring are not trusted for module signing. Another bug resulted > in the modules still being loaded under lockdown, so before > fixing that bug we need to fix the bug with trusting the MOK for > module signing so that dkms modules sigend with the MOK will > continue to load. > > CVE-2018-18653 Oops, this was included by mistake. Only patch 2 contains the CVE fix. This should be removed when applying, or I can resend if that would be preferable. > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > --- > kernel/module_signing.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/module_signing.c b/kernel/module_signing.c > index 937c844bee4a..d3d6f95a96b4 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); > } > -- > 2.19.1 >
On Fri, Oct 26, 2018 at 11:55:15AM -0600, Seth Forshee wrote: > BugLink: https://bugs.launchpad.net/bugs/1798863 > > For signing dkms modules we use a machine owner key whose public > half is enrolled into shim. This gets imported into the kernel's > secondary keyring, thus keys in this keyring need to be trusted > for module signing. > > Unfortunately the revision of the "secure boot lockdown" patches > imported into cosmic had a bug whereby keys in the secondary > keyring are not trusted for module signing. Another bug resulted > in the modules still being loaded under lockdown, so before > fixing that bug we need to fix the bug with trusting the MOK for > module signing so that dkms modules sigend with the MOK will > continue to load. > > CVE-2018-18653 > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > --- > kernel/module_signing.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/module_signing.c b/kernel/module_signing.c > index 937c844bee4a..d3d6f95a96b4 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); > } > -- > 2.19.1 Shouldn't we be using VERIFY_USE_SECONDARY_KEYRING instead? It's defined on cosmic. include/linux/verification.h:#define VERIFY_USE_SECONDARY_KEYRING ((struct key *)1UL)
On Fri, Oct 26, 2018 at 03:04:09PM -0300, Thadeu Lima de Souza Cascardo wrote: > On Fri, Oct 26, 2018 at 11:55:15AM -0600, Seth Forshee wrote: > > BugLink: https://bugs.launchpad.net/bugs/1798863 > > > > For signing dkms modules we use a machine owner key whose public > > half is enrolled into shim. This gets imported into the kernel's > > secondary keyring, thus keys in this keyring need to be trusted > > for module signing. > > > > Unfortunately the revision of the "secure boot lockdown" patches > > imported into cosmic had a bug whereby keys in the secondary > > keyring are not trusted for module signing. Another bug resulted > > in the modules still being loaded under lockdown, so before > > fixing that bug we need to fix the bug with trusting the MOK for > > module signing so that dkms modules sigend with the MOK will > > continue to load. > > > > CVE-2018-18653 > > > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > > --- > > kernel/module_signing.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/kernel/module_signing.c b/kernel/module_signing.c > > index 937c844bee4a..d3d6f95a96b4 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); > > } > > -- > > 2.19.1 > > Shouldn't we be using VERIFY_USE_SECONDARY_KEYRING instead? It's defined on cosmic. > > include/linux/verification.h:#define VERIFY_USE_SECONDARY_KEYRING ((struct key *)1UL) I suppose we can, I just used what we had in bionic. It doesn't seem that important to me either way, so I'm happy to change it if others would prefer it that way. Seth
On 2018-10-26 11:55:15, Seth Forshee wrote: > BugLink: https://bugs.launchpad.net/bugs/1798863 > > For signing dkms modules we use a machine owner key whose public > half is enrolled into shim. This gets imported into the kernel's > secondary keyring, thus keys in this keyring need to be trusted > for module signing. > > Unfortunately the revision of the "secure boot lockdown" patches > imported into cosmic had a bug whereby keys in the secondary > keyring are not trusted for module signing. Another bug resulted > in the modules still being loaded under lockdown, so before > fixing that bug we need to fix the bug with trusting the MOK for > module signing so that dkms modules sigend with the MOK will > continue to load. > > CVE-2018-18653 > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> After dropping the CVE ID from the commit message and switching over to VERIFY_USE_SECONDARY_KEYRING (because it is part of the kernel internal API and could change on us at some point in the future)... Acked-by: Tyler Hicks <tyhicks@canonical.com> Tyler > --- > kernel/module_signing.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/module_signing.c b/kernel/module_signing.c > index 937c844bee4a..d3d6f95a96b4 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); > } > -- > 2.19.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/kernel/module_signing.c b/kernel/module_signing.c index 937c844bee4a..d3d6f95a96b4 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); }
BugLink: https://bugs.launchpad.net/bugs/1798863 For signing dkms modules we use a machine owner key whose public half is enrolled into shim. This gets imported into the kernel's secondary keyring, thus keys in this keyring need to be trusted for module signing. Unfortunately the revision of the "secure boot lockdown" patches imported into cosmic had a bug whereby keys in the secondary keyring are not trusted for module signing. Another bug resulted in the modules still being loaded under lockdown, so before fixing that bug we need to fix the bug with trusting the MOK for module signing so that dkms modules sigend with the MOK will continue to load. CVE-2018-18653 Signed-off-by: Seth Forshee <seth.forshee@canonical.com> --- kernel/module_signing.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)