lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 15 Jan 2022 21:55:47 -0500 From: Mimi Zohar <zohar@...ux.ibm.com> To: Jarkko Sakkinen <jarkko@...nel.org>, Eric Snowberg <eric.snowberg@...cle.com> Cc: David Howells <dhowells@...hat.com>, "dwmw2@...radead.org" <dwmw2@...radead.org>, "ardb@...nel.org" <ardb@...nel.org>, "jmorris@...ei.org" <jmorris@...ei.org>, "serge@...lyn.com" <serge@...lyn.com>, "nayna@...ux.ibm.com" <nayna@...ux.ibm.com>, "keescook@...omium.org" <keescook@...omium.org>, "torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>, "weiyongjun1@...wei.com" <weiyongjun1@...wei.com>, "keyrings@...r.kernel.org" <keyrings@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>, "linux-security-module@...r.kernel.org" <linux-security-module@...r.kernel.org>, "James.Bottomley@...senpartnership.com" <James.Bottomley@...senpartnership.com>, "pjones@...hat.com" <pjones@...hat.com>, Konrad Wilk <konrad.wilk@...cle.com> Subject: Re: [PATCH v9 2/8] integrity: Introduce a Linux keyring called machine On Sat, 2022-01-15 at 21:15 +0200, Jarkko Sakkinen wrote: > On Sat, Jan 15, 2022 at 09:14:45PM +0200, Jarkko Sakkinen wrote: > > On Sat, Jan 15, 2022 at 07:12:35PM +0000, Eric Snowberg wrote: > > > > > > > > > > On Jan 15, 2022, at 10:11 AM, Jarkko Sakkinen <jarkko@...nel.org> wrote: > > > > > > > > On Wed, Jan 12, 2022 at 02:41:47PM -0500, Mimi Zohar wrote: > > > >> On Tue, 2022-01-11 at 20:14 -0500, Mimi Zohar wrote: > > > >>> On Tue, 2022-01-11 at 21:26 +0000, Eric Snowberg wrote: > > > >>>> > > > >>>>> On Jan 11, 2022, at 11:16 AM, Mimi Zohar <zohar@...ux.ibm.com> wrote: > > > >>>>> > > > >>>>> On Mon, 2022-01-10 at 23:25 +0000, Eric Snowberg wrote: > > > >>>>>>> Jarkko, my concern is that once this version of the patch set is > > > >>>>>>> upstreamed, would limiting which keys may be loaded onto the .machine > > > >>>>>>> keyring be considered a regression? > > > >>>>>> > > > >>>>>> > > > >>>>>> Currently certificates built into the kernel do not have a CA restriction on them. > > > >>>>>> IMA will trust anything in this keyring even if the CA bit is not set. While it would > > > >>>>>> be advisable for a kernel to be built with a CA, nothing currently enforces it. > > > >>>>>> > > > >>>>>> My thinking for the dropped CA restriction patches was to introduce a new Kconfig. > > > >>>>>> This Kconfig would do the CA enforcement on the machine keyring. However if the > > > >>>>>> Kconfig option was not set for enforcement, it would work as it does in this series, > > > >>>>>> plus it would allow IMA to work with non-CA keys. This would be done by removing > > > >>>>>> the restriction placed in this patch. Let me know your thoughts on whether this would > > > >>>>>> be an appropriate solution. I believe this would get around what you are identifying as > > > >>>>>> a possible regression. > > > >>>>> > > > >>>>> True the problem currently exists with the builtin keys, but there's a > > > >>>>> major difference between trusting the builtin keys and those being > > > >>>>> loading via MOK. This is an integrity gap that needs to be closed and > > > >>>>> shouldn't be expanded to keys on the .machine keyring. > > > >>>>> > > > >>>>> "plus it would allow IMA to work with non-CA keys" is unacceptable. > > > >>>> > > > >>>> Ok, I’ll leave that part out. Could you clarify the wording I should include in the future > > > >>>> cover letter, which adds IMA support, on why it is unacceptable for the end-user to > > > >>>> make this decision? > > > >>> > > > >>> The Kconfig IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY > > > >>> "help" is very clear: > > > >> > > > >> [Reposting the text due to email formatting issues.] > > > >> > > > >> help > > > >> Keys may be added to the IMA or IMA blacklist keyrings, if the > > > >> key is validly signed by a CA cert in the system built-in or > > > >> secondary trusted keyrings. > > > >> > > > >> Intermediate keys between those the kernel has compiled in and the > > > >> IMA keys to be added may be added to the system secondary keyring, > > > >> provided they are validly signed by a key already resident in the > > > >> built-in or secondary trusted keyrings. > > > >> > > > >> > > > >> The first paragraph requires "validly signed by a CA cert in the system > > > >> built-in or secondary trusted keyrings" for keys to be loaded onto the > > > >> IMA keyring. This Kconfig is limited to just the builtin and secondary > > > >> keyrings. Changing this silently to include the ".machine" keyring > > > >> introduces integrity risks that previously did not exist. A new IMA > > > >> Kconfig needs to be defined to allow all three keyrings - builtin, > > > >> machine, and secondary. > > > >> > > > >> The second paragraph implies that only CA and intermediate CA keys are > > > >> on secondary keyring, or as in our case the ".machine" keyring linked > > > >> to the secondary keyring. > > > >> > > > >> Mimi > > > >> > > > > I have also now test environment for this patch set but if there are > > > > any possible changes, I'm waiting for a new version, as it is anyway > > > > for 5.18 cycle earliest. > > > > > > Other than the two sentence changes, I have not seen anything identified > > > code wise requiring a change. If you’d like me to respin a v10 with the sentence > > > changes let me know. Or if you want to remove the ima reference, that works > > > too. Just let me know how you want to handle this. Thanks. > > > > I'm basically waiting also Mimi to test this as I do not have IMA test > > environment. > > > > From my side: > > > > Tested-by: Jarkko Sakkinen <jarkko@...nel.org> > > I can pick the whole thing at the time when I get green light. The MOK keys are not loaded onto the .machine keyring if CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY is enabled. >From an IMA perspective nothing has changed. After the IMA references in the patch descriptions are removed, feel free to add Tested-by: Mimi Zohar <zohar@...ux.ibm.com> on patches 1 - 5. thanks, Mimi
Powered by blists - more mailing lists