Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1302769 > unrolled thread

[PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring

Back to article view | Back to linux.kernel

Contents

  [PATCH] X.509: Partially revert patch to add validation against IMA  MOK keyring David Howells <dhowells@redhat.com> - 2016-01-06 14:50 +0100
    Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring James Morris <jmorris@namei.org> - 2016-01-07 01:10 +0100
    Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring David Howells <dhowells@redhat.com> - 2016-01-07 01:40 +0100
      Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-01-07 03:20 +0100
        Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-01-07 04:30 +0100
      Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-01-07 16:40 +0100
        Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring James Morris <jmorris@namei.org> - 2016-01-10 11:40 +0100
          Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-01-10 14:30 +0100
            Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring David Howells <dhowells@redhat.com> - 2016-01-10 18:50 +0100
              Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring David Howells <dhowells@redhat.com> - 2016-01-10 21:40 +0100
                Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-01-11 01:00 +0100
                  Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring David Howells <dhowells@redhat.com> - 2016-01-12 01:50 +0100
                    Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring  David Howells <dhowells@redhat.com> - 2016-01-12 03:10 +0100
                      Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-01-12 04:40 +0100
                        Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring David Howells <dhowells@redhat.com> - 2016-01-12 11:10 +0100
                          Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-01-12 14:30 +0100
                            Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring David Howells <dhowells@redhat.com> - 2016-01-12 15:00 +0100
                              Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-01-12 16:20 +0100
                                Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring David Howells <dhowells@redhat.com> - 2016-01-12 17:00 +0100
                                  Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-01-12 17:10 +0100
                          Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Petko Manolov <petkan@mip-labs.com> - 2016-01-12 15:20 +0100
              Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Petko Manolov <petkan@mip-labs.com> - 2016-01-10 21:40 +0100
                Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring David Howells <dhowells@redhat.com> - 2016-01-12 02:40 +0100
                  Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Petko Manolov <petkan@mip-labs.com> - 2016-01-12 17:20 +0100
                    Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring David Howells <dhowells@redhat.com> - 2016-01-12 18:10 +0100
                      Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Petko Manolov <petkan@mip-labs.com> - 2016-01-13 17:40 +0100
                        Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-01-13 19:00 +0100
                          Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Petko Manolov <petkan@mip-labs.com> - 2016-01-13 19:10 +0100
                          Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring David Howells <dhowells@redhat.com> - 2016-01-13 19:20 +0100
                            Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Petko Manolov <petkan@mip-labs.com> - 2016-01-13 19:40 +0100
                              Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-01-13 20:00 +0100
                                Re: [PATCH] X.509: Partially revert patch to add validation against  IMA MOK keyring Petko Manolov <petkan@mip-labs.com> - 2016-01-13 20:20 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1307464 — Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring

Guys, i'd like to take a step back, make a short keyring overview (the way i see 
it) and see if we're all on the same page.  Implementation details matters, but 
only if we've agreed on the architecture first.  This is how i see the _trusted_ 
keyrings hierarchy in the kernel.  I don't claim i am right... :)


(1) I really think the system keyring should remain static.  The only time you 
can really trust your kernel is at it's build time.  Later on, when it is 
running, it may fail you in a great many and spectacular ways, thanks to the 
bugs we all introduce.

Unless the kernel is terribly broken the system keyring should not contain 
anything else but the built-in certificates.  This will serve as foothold of 
sanity in an extremely dynamic and sometimes quite messy world.  Most production 
grade kernels are built in a clean room environment with restricted physical 
access.  You can't get as good level of security once they are released in the 
wild.


(2) IMA MOK and blacklist keyrings - right now they are implemented as an aid to 
the IMA subsystem, but really should be system-wide keyrings.  Assuming the 
.system is read-only the only way you can dynamically add _trusted_ certificates 
to your kernel is via some sort of read-write keyring that is _dependent_ on 
the .system.

MOK (or, again, whatever the name) should be the second level of _trusted_ 
keyrings.  .ima and .evm are the obvious users, but there may be others in the 
future and represents the leafs of the trusted keyring graph.


(3) the blacklist keyring - it is at the same hierarchy level as the system 
keyring, but for obvious reasons, can't be read-only.  Since it should be the 
first keyring we check we can't allow stuff to go there randomly.  One thing i 
can think of is the standard CA signature verification.  Only keys that are 
properly signed should go there.  This check should not be enough to guarantee 
entry in .blacklist otherwise it will be very easy to DoS IMA-appraisal enabled 
system.  Additional criteria should be introduced when moving keys to this 
keyring.


I would like to once again state that .ima_mok and .ima_blacklist should really 
be system wide keyrings.  It so happened that i worked with Mimi Zohar and Mark 
Baushke and my focus was the IMA/EVM subsystem.  I (maybe naively) thought IMA 
would serve as a test ground for the concept and once it mature it may move up.  
It now seems that we need to agree on an architecture that will serve all newly 
introduced use-cases.



		Petko

[toc] | [prev] | [next] | [standalone]

#1305688 — Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring

On 16-01-10 17:46:30, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > > Is this a NAK on the patch?
> > 
> > Yes
> 
> I would like to counter Mimi's NAK:
> 
>  (1) Commit 41c89b64d7184a780f12f2cccdabe65cb2408893 doesn't do what it
>      says.  Given the change I want to revert, this bit of the description:
> 
> 
> 	To successfully import a key into .ima_mok it must be signed by a
> 	key which CA is in .system keyring.
> 
>      is *not* true.  A key in the .ima_mok keyring will *also* allow a key

This is correct, but is also the desired result.  Assume that you have multi 
tenant machine where the manufacturer signs the owner's/tenant's key and those 
also need to sign other sub-tenant keys.  One can't put them on the system 
keyring so they end up in .ima_mok.

>      into the .ima_mok keyring.  Thus the .ima_mok keyring is redundant and
>      should be merged into the .system keyring.

I share Mimi's opinion that .system keyring must be static and ultimately 
trusted.  Since .ima_mok is a dynamic keyring, merging them will break the 
semantics.

>  (2) You can use KEYCTL_LINK to link trusted keys between trusted keyrings
>      if the key being linked grants permission.  Add a new key to one open
>      keyring and you can then link it across to another.
> 
>      Keyrings need to guard against *link* as per my recently posted
>      patches.

I'd rather rely on a certificate being properly signed in order to land in a 
particular keyring, rather than being linked based on permissions model.

>  (3) In the current model, the trusted-only keyring and trusted-key concept
>      ought really to apply only to the .system keyring as the concept of
>      'trust' is boolean in this implementation.

The .system keyring should be read-only, IOW static.  Only keys present at build 
time should go there.  Everything else goes to the machine owner keyring (MOK) 
or whatever the name.  MOK should be read-write and (maybe) hold only second 
level CAs, signed by CA in the system keyring.

I introduced .ima_mok just because my work had limited scope at the time and i 
consider the name as misleading.

The way i see kernel's keyrings:

                           /---> .ima
            /----> MOK ---<
.system ---<               \---> .evm
            \----> BL       \---> .whatever need to be "trusted"

The graph could be a lot more complex, but to wrap your head around the idea 
think of big ass machine with years of uptime and multiple simultaneous users, 
all pre-installed files IMA signed, ability to add other IMA signed packages on 
the fly.  The machine must be FIPS certifiable, etc.  A terabit switching 
machine should be able to do that and there are real users for this scenario out 
there.

The black-list keyring is equally important so one can revoke CAs if need be.  
On the fly.


		Petko

[toc] | [prev] | [next] | [standalone]

#1306900 — Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring

Petko Manolov <petkan@mip-labs.com> wrote:

> > I would like to counter Mimi's NAK:
> > 
> >  (1) Commit 41c89b64d7184a780f12f2cccdabe65cb2408893 doesn't do what it
> >      says.  Given the change I want to revert, this bit of the description:
> > 
> > 	To successfully import a key into .ima_mok it must be signed by a
> > 	key which CA is in .system keyring.
> > 
> >      is *not* true.  A key in the .ima_mok keyring will *also* allow a key
> 
> This is correct, but is also the desired result.

Not really.  What if you want a trusted keyring that is not governed by the
.ima_mok keyring?  You can't have it.

> Assume that you have multi tenant machine where the manufacturer signs the
> owner's/tenant's key and those also need to sign other sub-tenant keys.  One
> can't put them on the system keyring

Why not?  We can enable user-write on the system keyring.  Are you saying that
there should be multiple .ima_mok keyrings?  If so, your change is wrong.

> so they end up in .ima_mok.
> 
> >      into the .ima_mok keyring.  Thus the .ima_mok keyring is redundant and
> >      should be merged into the .system keyring.
> 
> I share Mimi's opinion that .system keyring must be static and ultimately 
> trusted.

I don't necessarily share the opinion that it must remain static.  If you can
change the .ima_mok keyring, then it's as good as being able to change the
.system keyring by your change that I want to revert.

The one thing I grant that enabling the .system keyring will allow is deletion
of trusted keys - and once you've deleted them, you can't necessarily get them
back without rebooting.

> Since .ima_mok is a dynamic keyring, merging them will break the 
> semantics.

So what you're saying is that .ima_mok is just a staging area for changes
that would otherwise go in .system?  In which case, why is it IMA-related at
all?  Why isn't it called ".mok" or ".system_overrides" or something like that
and in certs/system_keyring.c?

Remember: IMA is optional.  We want to be able to disable it.  So if you want
this feature, it really needs to be separated from IMA.

And why shouldn't we change these semantics?

And why can't .system be a dynamic keyring?

> >  (2) You can use KEYCTL_LINK to link trusted keys between trusted keyrings
> >      if the key being linked grants permission.  Add a new key to one open
> >      keyring and you can then link it across to another.
> > 
> >      Keyrings need to guard against *link* as per my recently posted
> >      patches.
> 
> I'd rather rely on a certificate being properly signed in order to land in a 
> particular keyring, rather than being linked based on permissions model.

Then the patches I posted *are* necessary.  Currently KEYCTL_LINK will let you
link a "trusted" key between trusted-only keyrings if the Link permission is
set because trustedness is currently evaluated only once - at key preparse
time - because we currently throw away all the metadata you need to do further
checks.

The patches I posted validate the certificate signature on addition of a
*link* into a keyring (add_key(), if it doesn't update a key, creates a key
and then does a link operation).  The gatekeeper function is settable
per-keyring.

> >  (3) In the current model, the trusted-only keyring and trusted-key concept
> >      ought really to apply only to the .system keyring as the concept of
> >      'trust' is boolean in this implementation.
> 
> The .system keyring should be read-only, IOW static.  Only keys present at
> build time should go there.

Why?  You haven't given a reason why .ima_mok shouldn't be integrated into
.system and that opened up.  Because you prefer it the way you've done it
isn't necessarily a good reason.

> Everything else goes to the machine owner keyring (MOK) or whatever the
> name.  MOK should be read-write and (maybe) hold only second level CAs,
> signed by CA in the system keyring.  I introduced .ima_mok just because my
> work had limited scope at the time and i consider the name as misleading.

What would you call it then, if not .ima_mok?  Given its current name, it
seems that it should relate to the UEFI BIOS data if such is available.

> The way i see kernel's keyrings:
> 
>                            /---> .ima
>             /----> MOK ---<
> .system ---<               \---> .evm
>             \----> BL       \---> .whatever need to be "trusted"
> 
> The graph could be a lot more complex, but to wrap your head around the idea
> think of big ass machine with years of uptime and multiple simultaneous
> users, all pre-installed files IMA signed, ability to add other IMA signed
> packages on the fly.  The machine must be FIPS certifiable, etc.  A terabit
> switching machine should be able to do that and there are real users for
> this scenario out there.

Given you seem to have one .system ring and one MOK ring, why do you need
separate rings?

> The black-list keyring is equally important so one can revoke CAs if need
> be.  On the fly.

I've no objection to a blacklist.  I can see that you might want it to be a
separate keyring to have the no-removal clause in place.  I would consider
making a blacklist key type and have it contain a bundle of key IDs to reduce
resource consumption, but that's an implementation detail (the match function
can check against all the IDs in a bundle).

However, the blacklist also should be separated from the IMA subsystem and
moved to system_keyring.c.  I can probably backport the code we use in Fedora
for this.

David

[toc] | [prev] | [next] | [standalone]

#1307597 — Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring

On 16-01-12 01:38:13, David Howells wrote:
> Petko Manolov <petkan@mip-labs.com> wrote:
> 
> > > I would like to counter Mimi's NAK:
> > > 
> > >  (1) Commit 41c89b64d7184a780f12f2cccdabe65cb2408893 doesn't do what it
> > >      says.  Given the change I want to revert, this bit of the description:
> > > 
> > > 	To successfully import a key into .ima_mok it must be signed by a
> > > 	key which CA is in .system keyring.
> > > 
> > >      is *not* true.  A key in the .ima_mok keyring will *also* allow a key
> > 
> > This is correct, but is also the desired result.
> 
> Not really.  What if you want a trusted keyring that is not governed by the 
> .ima_mok keyring?  You can't have it.

There is no need for .ima_mok if here is .mok, which should be system wide 
keyring.  I'm trying to say that once we have .mok (you're more than welcome to 
suggest better name) we'll get rid of .ima_mok.

> > Assume that you have multi tenant machine where the manufacturer signs the
> > owner's/tenant's key and those also need to sign other sub-tenant keys.  One
> > can't put them on the system keyring
> 
> Why not?  We can enable user-write on the system keyring.  Are you saying that 
> there should be multiple .ima_mok keyrings?  If so, your change is wrong.

Nope, no multiple MOK keyrings.  One, read-write trusted keyring should be 
enough.

> > so they end up in .ima_mok.
> > 
> > >      into the .ima_mok keyring.  Thus the .ima_mok keyring is redundant and
> > >      should be merged into the .system keyring.
> > 
> > I share Mimi's opinion that .system keyring must be static and ultimately 
> > trusted.
> 
> I don't necessarily share the opinion that it must remain static.  If you can 
> change the .ima_mok keyring, then it's as good as being able to change the 
> .system keyring by your change that I want to revert.

I still see value in immutable system keyring.  Being able to reboot to a known 
state is only one of the reasons.  The other is the ultimate trust one should 
have in .system...

> The one thing I grant that enabling the .system keyring will allow is deletion 
> of trusted keys - and once you've deleted them, you can't necessarily get them 
> back without rebooting.

Can't we incorporate this functionality in .blacklist and avoid rebooting.

> > Since .ima_mok is a dynamic keyring, merging them will break the semantics.
> 
> So what you're saying is that .ima_mok is just a staging area for changes that 
> would otherwise go in .system?  In which case, why is it IMA-related at all?  
> Why isn't it called ".mok" or ".system_overrides" or something like that and 
> in certs/system_keyring.c?

.ima_mok was designed at a time when i did not see it as a system-level trusted 
keyring.  It later occurred that it should be moved out of the IMA subsystem as 
there are potentially other users.

> Remember: IMA is optional.  We want to be able to disable it.  So if you want 
> this feature, it really needs to be separated from IMA.

Moving it out, or being enabled if IMA (or other sybsystem) is selected, will do 
the trick

> And why shouldn't we change these semantics?
> 
> And why can't .system be a dynamic keyring?

Because this makes me uneasy.  What are we saving?  A few pages of memory?..

> > >  (2) You can use KEYCTL_LINK to link trusted keys between trusted keyrings
> > >      if the key being linked grants permission.  Add a new key to one open
> > >      keyring and you can then link it across to another.
> > > 
> > >      Keyrings need to guard against *link* as per my recently posted
> > >      patches.
> > 
> > I'd rather rely on a certificate being properly signed in order to land in a 
> > particular keyring, rather than being linked based on permissions model.
> 
> Then the patches I posted *are* necessary.  Currently KEYCTL_LINK will let you 
> link a "trusted" key between trusted-only keyrings if the Link permission is 
> set because trustedness is currently evaluated only once - at key preparse 
> time - because we currently throw away all the metadata you need to do further 
> checks.

I don't mind linking in general as long as the permission check is supplementary 
to the keys CA hierarchy verification.

> The patches I posted validate the certificate signature on addition of a 
> *link* into a keyring (add_key(), if it doesn't update a key, creates a key 
> and then does a link operation).  The gatekeeper function is settable 
> per-keyring.

Splendid.  I've no issue with this.

> > >  (3) In the current model, the trusted-only keyring and trusted-key concept
> > >      ought really to apply only to the .system keyring as the concept of
> > >      'trust' is boolean in this implementation.
> > 
> > The .system keyring should be read-only, IOW static.  Only keys present at
> > build time should go there.
> 
> Why?  You haven't given a reason why .ima_mok shouldn't be integrated into 
> .system and that opened up.  Because you prefer it the way you've done it 
> isn't necessarily a good reason.

Err...  I've stated my motive numerous times.  Please let me know if i miss the 
point somewhere.

> > Everything else goes to the machine owner keyring (MOK) or whatever the
> > name.  MOK should be read-write and (maybe) hold only second level CAs,
> > signed by CA in the system keyring.  I introduced .ima_mok just because my
> > work had limited scope at the time and i consider the name as misleading.
> 
> What would you call it then, if not .ima_mok?  Given its current name, it 
> seems that it should relate to the UEFI BIOS data if such is available.

Nope, the name is indeed misleading.  I don't insist on MOK in the name as long 
as the semantics is preserved.  I also do not want to involve UEFI-embedded 
certificates.  Just because they are in my machine's NV memory does not mean i 
trust them.  I'd rather trust the kernel and certificates that i've made.

There are companies that make their own hardware and build all of their 
software.  UEFI may or may not be present.  Again, think of big machines.

> > The way i see kernel's keyrings:
> > 
> >                            /---> .ima
> >             /----> MOK ---<
> > .system ---<               \---> .evm
> >             \----> BL       \---> .whatever need to be "trusted"
> > 
> > The graph could be a lot more complex, but to wrap your head around the idea
> > think of big ass machine with years of uptime and multiple simultaneous
> > users, all pre-installed files IMA signed, ability to add other IMA signed
> > packages on the fly.  The machine must be FIPS certifiable, etc.  A terabit
> > switching machine should be able to do that and there are real users for
> > this scenario out there.
> 
> Given you seem to have one .system ring and one MOK ring, why do you need
> separate rings?

If .system is RO, then we need something that is both second level CA hierarchy 
and is RW.

> > The black-list keyring is equally important so one can revoke CAs if need
> > be.  On the fly.
> 
> I've no objection to a blacklist.  I can see that you might want it to be a 
> separate keyring to have the no-removal clause in place.  I would consider 
> making a blacklist key type and have it contain a bundle of key IDs to reduce 
> resource consumption, but that's an implementation detail (the match function 
> can check against all the IDs in a bundle).

I think blacklist keyring is important.  Being write-only is also important.  
Once this situation is resolved i'll have to post a patch that makes use of 
.blacklist as we already have users for this scenario.

> However, the blacklist also should be separated from the IMA subsystem and 
> moved to system_keyring.c.  I can probably backport the code we use in Fedora 
> for this.

I totally agree with you here.  If the certificate used to sign certain kernel 
modules is revoked we would very much like to forbid loading those modules.


		Petko

[toc] | [prev] | [next] | [standalone]

#1307639 — Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring

Petko Manolov <petkan@mip-labs.com> wrote:

> There is no need for .ima_mok if here is .mok, which should be system wide
> keyring.  I'm trying to say that once we have .mok (you're more than welcome
> to suggest better name) we'll get rid of .ima_mok.

If we really must have separate keyrings - and I still don't see that it's
necessary - then maybe:

	.builtin_trusted_keys (RO)
	.secondarily_trusted_keys (RW).

I'd prefer to avoid "mok" as that might be misconstrued in a UEFI system.

> I still see value in immutable system keyring.  Being able to reboot to a
> known state is only one of the reasons.

I'm not sure what you mean.  Changes to .system_keyring would not be
persistent across reboot.

> The other is the ultimate trust one should have in .system...

Do you have a use case where you would use an immutable set of keys
exclusively?

> > The one thing I grant that enabling the .system keyring will allow is
> > deletion of trusted keys - and once you've deleted them, you can't
> > necessarily get them back without rebooting.
> 
> Can't we incorporate this functionality in .blacklist and avoid rebooting.

I think you misunderstood.  Once you've discarded a builtin keyring you cannot
get it back without rebooting (unless you hold another trusted key that signed
it).  Once you blacklist a builtin keyring you cannot get it back without
rebooting (unless you can remove things from a blacklist).

Clearing .system_keyring would be equivalent to blacklisting all the keys held
therein.  However, I presume you would have it that you cannot add to
.blacklist unless your bundle of keys to be blacklisted is appropriately
signed.

> > And why can't .system be a dynamic keyring?
> 
> Because this makes me uneasy.  What are we saving?  A few pages of memory?..

A key struct and some associative array metadata plus the cost of looking up
in a second keyring.

I'm not sure why it makes you any more uneasy than having .ima_mok at all.
However, if it makes you able to sleep at night (;-)), and you're willing to
accept modification of the trust model along the lines of the patchset I
posted (which will need a couple of alterations) and move the new trust
keyring and blacklist keyring to the core, then okay, we can do that.

> I don't mind linking in general as long as the permission check is
> supplementary to the keys CA hierarchy verification.

Which it currently isn't really.  As things stand, the CA hierarchy
verification takes place once at key creation and is assumed applicable to all
trusted keyrings thereafter.  KEY_FLAG_TRUSTED_ONLY was only really supposed
to apply to the system keyring.

David

[toc] | [prev] | [next] | [standalone]

#1308585 — Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring

On 16-01-12 17:08:44, David Howells wrote:
> Petko Manolov <petkan@mip-labs.com> wrote:
> 
> > There is no need for .ima_mok if here is .mok, which should be system wide
> > keyring.  I'm trying to say that once we have .mok (you're more than welcome
> > to suggest better name) we'll get rid of .ima_mok.
> 
> If we really must have separate keyrings - and I still don't see that it's
> necessary - then maybe:
>
> 	.builtin_trusted_keys (RO)
> 	.secondarily_trusted_keys (RW).

Yep, that's the basic idea.

> I'd prefer to avoid "mok" as that might be misconstrued in a UEFI system.

It is a good idea to avoid confusion.

> > I still see value in immutable system keyring.  Being able to reboot to a
> > known state is only one of the reasons.
> 
> I'm not sure what you mean.  Changes to .system_keyring would not be 
> persistent across reboot.

Among other things it is nice to have a keyring with a known good state without 
the need to reboot.  You (the root user and only under certain circumstances) 
should always be able to zap the secondary keyring, but not the .system.

> > The other is the ultimate trust one should have in .system...
> 
> Do you have a use case where you would use an immutable set of keys 
> exclusively?

Yep.  Certain government structures will not use the machine in multi tenant 
mode so they'll settle with the vendor keys.

> > Can't we incorporate this functionality in .blacklist and avoid rebooting.
> 
> I think you misunderstood.  Once you've discarded a builtin keyring you cannot 
> get it back without rebooting (unless you hold another trusted key that signed 
> it).  Once you blacklist a builtin keyring you cannot get it back without 
> rebooting (unless you can remove things from a blacklist).

I do not get that part about blacklisting an entire keyring.  I hope we will be 
able to blacklist particular key, not the whole keyring.

> Clearing .system_keyring would be equivalent to blacklisting all the keys held 
> therein.  However, I presume you would have it that you cannot add to 
> .blacklist unless your bundle of keys to be blacklisted is appropriately 
> signed.
> 
> > > And why can't .system be a dynamic keyring?
> > 
> > Because this makes me uneasy.  What are we saving?  A few pages of memory?..
> 
> A key struct and some associative array metadata plus the cost of looking up
> in a second keyring.

Hopefully something that will be done only when importing new trusted key.

> I'm not sure why it makes you any more uneasy than having .ima_mok at all. 
> However, if it makes you able to sleep at night (;-)), and you're willing to 
> accept modification of the trust model along the lines of the patchset I 
> posted (which will need a couple of alterations) and move the new trust 
> keyring and blacklist keyring to the core, then okay, we can do that.

I'll sleep much better once i grasp your entire patch-set, although it will 
require some lack of sleep on my part. ;-)

I am also afraid these "couple of alternations" should be communicated to all 
stakeholders (Mimi, for one), especially if we decide to make .ima_mok to be 
system wide .secondarily_trusted_keys.  I volunteer to be excluded from the name 
giving competition. :-)

It is also not clear to me how we'll move keys to the blacklist keyring.  They 
should obviously be signed by CA in primary/secondary trusted keyring(s), the 
permissions should be set correctly, but i somehow feel this is not enough.

Mark, would you please comment on the above?

> > I don't mind linking in general as long as the permission check is 
> > supplementary to the keys CA hierarchy verification.
> 
> Which it currently isn't really.  As things stand, the CA hierarchy 
> verification takes place once at key creation and is assumed applicable to all 
> trusted keyrings thereafter.  KEY_FLAG_TRUSTED_ONLY was only really supposed 
> to apply to the system keyring.

I am not opposed to everything what you suggest.  Since we did that work in 
parallel (your stuff and the IMA keyring additions) with no communication 
between us, we ended up with broken IMA model.  I see three possibilities:

  - dump the IMA changes for this release (not happy about it);

  - try to quickly adapt the IMA system to your changes (not sure if it can be 
    done easily and/or quickly) and do it properly for 4.6;

  - elevate .ima_mok/blacklist to system wide RW keyrings (we may miss the merge 
    window);



		Petko

[toc] | [prev] | [next] | [standalone]

#1308688 — Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring

On Wed, 2016-01-13 at 18:31 +0200, Petko Manolov wrote:
> On 16-01-12 17:08:44, David Howells wrote:
> > Petko Manolov <petkan@mip-labs.com> wrote:
> > 
> > > There is no need for .ima_mok if here is .mok, which should be system wide
> > > keyring.  I'm trying to say that once we have .mok (you're more than welcome
> > > to suggest better name) we'll get rid of .ima_mok.
> > 
> > If we really must have separate keyrings - and I still don't see that it's
> > necessary - then maybe:
> >
> > 	.builtin_trusted_keys (RO)
> > 	.secondarily_trusted_keys (RW).
> 
> Yep, that's the basic idea.

The "builtin_trusted_keys" is fine.  It would be
"secondary_trusted_keys".  This secondary keyring needs to be Kconfig
optional.

> > I'm not sure why it makes you any more uneasy than having .ima_mok at all. 
> > However, if it makes you able to sleep at night (;-)), and you're willing to 
> > accept modification of the trust model along the lines of the patchset I 
> > posted (which will need a couple of alterations) and move the new trust 
> > keyring and blacklist keyring to the core, then okay, we can do that.
> 
> I'll sleep much better once i grasp your entire patch-set, although it will 
> require some lack of sleep on my part. ;-)
> 
> I am also afraid these "couple of alternations" should be communicated to all 
> stakeholders (Mimi, for one), especially if we decide to make .ima_mok to be 
> system wide .secondarily_trusted_keys.  I volunteer to be excluded from the name 
> giving competition. :-)
> 
> It is also not clear to me how we'll move keys to the blacklist keyring.  They 
> should obviously be signed by CA in primary/secondary trusted keyring(s), the 
> permissions should be set correctly, but i somehow feel this is not enough.

> Mark, would you please comment on the above?
> 
> > > I don't mind linking in general as long as the permission check is 
> > > supplementary to the keys CA hierarchy verification.
> > 
> > Which it currently isn't really.  As things stand, the CA hierarchy 
> > verification takes place once at key creation and is assumed applicable to all 
> > trusted keyrings thereafter.  KEY_FLAG_TRUSTED_ONLY was only really supposed 
> > to apply to the system keyring.
> 
> I am not opposed to everything what you suggest.  Since we did that work in 
> parallel (your stuff and the IMA keyring additions) with no communication 
> between us, we ended up with broken IMA model.  I see three possibilities:
> 
>   - dump the IMA changes for this release (not happy about it);
> 
>   - try to quickly adapt the IMA system to your changes (not sure if it can be 
>     done easily and/or quickly) and do it properly for 4.6;
> 
>   - elevate .ima_mok/blacklist to system wide RW keyrings (we may miss the merge 
>     window);

I beg to differ.  The IMA model is not broken with the current patches
being upstreamed.  The basic concepts developed will continue to be
used, perhaps not directly by IMA.

David's proposal is a major redesign of keyrings and the system keyring
in particular.  It looks promising, but will need to be reviewed.

Mimi

[toc] | [prev] | [next] | [standalone]

#1308696 — Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring

On 16-01-13 12:51:36, Mimi Zohar wrote:
> On Wed, 2016-01-13 at 18:31 +0200, Petko Manolov wrote:
> > 
> > I am not opposed to everything what you suggest.  Since we did that work in 
> > parallel (your stuff and the IMA keyring additions) with no communication 
> > between us, we ended up with broken IMA model.  I see three possibilities:
> > 
> >   - dump the IMA changes for this release (not happy about it);
> > 
> >   - try to quickly adapt the IMA system to your changes (not sure if it can be 
> >     done easily and/or quickly) and do it properly for 4.6;
> > 
> >   - elevate .ima_mok/blacklist to system wide RW keyrings (we may miss the merge 
> >     window);
> 
> I beg to differ.  The IMA model is not broken with the current patches being 
> upstreamed.  The basic concepts developed will continue to be used, perhaps 
> not directly by IMA.
> 
> David's proposal is a major redesign of keyrings and the system keyring in 
> particular.  It looks promising, but will need to be reviewed.

Due to time limitations i was not able to study David's changes in detail.  I 
only commented on what (i thought) i understood. :)

I assume a wider discussion will clean out the details.


		Petko

[toc] | [prev] | [next] | [standalone]

#1308707 — Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring

Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> I beg to differ.  The IMA model is not broken with the current patches
> being upstreamed.  The basic concepts developed will continue to be
> used, perhaps not directly by IMA.

I still object to the change to x509_key_preparse() and still want it
reverting or removing.  It affects module signing too.

David

[toc] | [prev] | [next] | [standalone]

#1308716 — Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring

On 16-01-13 18:19:10, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > I beg to differ.  The IMA model is not broken with the current patches
> > being upstreamed.  The basic concepts developed will continue to be
> > used, perhaps not directly by IMA.
> 
> I still object to the change to x509_key_preparse() and still want it 
> reverting or removing.  It affects module signing too.

The only problem i see with the code is that in case .ima_mok is not configured 
x509_validate_trust() returns NULL, which falsely set the key as trusted.  This 
could easily be fixed.

Some users do want to be able to load kernel modules signed by other trusted 
parties.  Think of .ima_mok as system wide keyring in this case.  It is 
semantically broken, but it does the right thing.


		Petko

[toc] | [prev] | [next] | [standalone]

#1308731 — Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring

On Wed, 2016-01-13 at 20:35 +0200, Petko Manolov wrote:
> On 16-01-13 18:19:10, David Howells wrote:
> > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > 
> > > I beg to differ.  The IMA model is not broken with the current patches
> > > being upstreamed.  The basic concepts developed will continue to be
> > > used, perhaps not directly by IMA.
> > 
> > I still object to the change to x509_key_preparse() and still want it 
> > reverting or removing.  It affects module signing too.
> 
> The only problem i see with the code is that in case .ima_mok is not configured 
> x509_validate_trust() returns NULL, which falsely set the key as trusted.  This 
> could easily be fixed.

When IMA_MOK_KEYRING  is not enabled, get_ima_mok_keyring() will return
NULL.  x509_validate_trust() will return -EOPNOTSUPP.

The code is fine.

Mimi

> Some users do want to be able to load kernel modules signed by other trusted 
> parties.  Think of .ima_mok as system wide keyring in this case.  It is 
> semantically broken, but it does the right thing.
> 
> 
> 		Petko

[toc] | [prev] | [next] | [standalone]

#1308746 — Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring

On 16-01-13 13:56:39, Mimi Zohar wrote:
> On Wed, 2016-01-13 at 20:35 +0200, Petko Manolov wrote:
> > On 16-01-13 18:19:10, David Howells wrote:
> > > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > > 
> > > > I beg to differ.  The IMA model is not broken with the current patches
> > > > being upstreamed.  The basic concepts developed will continue to be
> > > > used, perhaps not directly by IMA.
> > > 
> > > I still object to the change to x509_key_preparse() and still want it 
> > > reverting or removing.  It affects module signing too.
> > 
> > The only problem i see with the code is that in case .ima_mok is not configured 
> > x509_validate_trust() returns NULL, which falsely set the key as trusted.  This 
> > could easily be fixed.
> 
> When IMA_MOK_KEYRING is not enabled, get_ima_mok_keyring() will return NULL.  
> x509_validate_trust() will return -EOPNOTSUPP.
> 
> The code is fine.

Oops, my bad.  It's been a while since i wrote that code... :)


		Petko

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web