* [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring @ 2016-01-06 13:45 David Howells 2016-01-07 0:04 ` James Morris 2016-01-07 0:34 ` David Howells 0 siblings, 2 replies; 34+ messages in thread From: David Howells @ 2016-01-06 13:45 UTC (permalink / raw) To: petkan, jmorris Cc: dhowells, linux-security-module, keyrings, Mimi Zohar, linux-kernel Partially revert commit 41c89b64d7184a780f12f2cccdabe65cb2408893: Author: Petko Manolov <petkan@mip-labs.com> Date: Wed Dec 2 17:47:55 2015 +0200 IMA: create machine owner and blacklist keyrings The problem is that prep->trusted is a simple boolean and the additional x509_validate_trust() call doesn't therefore distinguish levels of trustedness, but is just OR'd with the result of validation against the system trusted keyring. However, setting the trusted flag means that this key may be added to *any* trusted-only keyring - including the system trusted keyring. Whilst I appreciate what the patch is trying to do, I don't think this is quite the right solution. Signed-off-by: David Howells <dhowells@redhat.com> cc: Petko Manolov <petkan@mip-labs.com> cc: Mimi Zohar <zohar@linux.vnet.ibm.com> cc: keyrings@vger.kernel.org --- crypto/asymmetric_keys/x509_public_key.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c index 9e9e5a6a9ed6..2a44b3752471 100644 --- a/crypto/asymmetric_keys/x509_public_key.c +++ b/crypto/asymmetric_keys/x509_public_key.c @@ -321,8 +321,6 @@ static int x509_key_preparse(struct key_preparsed_payload *prep) goto error_free_cert; } else if (!prep->trusted) { ret = x509_validate_trust(cert, get_system_trusted_keyring()); - if (ret) - ret = x509_validate_trust(cert, get_ima_mok_keyring()); if (!ret) prep->trusted = 1; } ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-06 13:45 [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring David Howells @ 2016-01-07 0:04 ` James Morris 2016-01-07 0:34 ` David Howells 1 sibling, 0 replies; 34+ messages in thread From: James Morris @ 2016-01-07 0:04 UTC (permalink / raw) To: David Howells Cc: petkan, linux-security-module, keyrings, Mimi Zohar, linux-kernel > Partially revert commit 41c89b64d7184a780f12f2cccdabe65cb2408893: > > Author: Petko Manolov <petkan@mip-labs.com> > Date: Wed Dec 2 17:47:55 2015 +0200 > IMA: create machine owner and blacklist keyrings > If you need this applied to a tree, please state which. -- James Morris <jmorris@namei.org> ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-06 13:45 [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring David Howells 2016-01-07 0:04 ` James Morris @ 2016-01-07 0:34 ` David Howells 2016-01-07 2:13 ` Mimi Zohar 2016-01-07 15:31 ` Mimi Zohar 1 sibling, 2 replies; 34+ messages in thread From: David Howells @ 2016-01-07 0:34 UTC (permalink / raw) Cc: dhowells, petkan, jmorris, linux-security-module, keyrings, Mimi Zohar, linux-kernel David Howells <dhowells@redhat.com> wrote: > Partially revert commit 41c89b64d7184a780f12f2cccdabe65cb2408893: > > Author: Petko Manolov <petkan@mip-labs.com> > Date: Wed Dec 2 17:47:55 2015 +0200 > IMA: create machine owner and blacklist keyrings > > The problem is that prep->trusted is a simple boolean and the additional > x509_validate_trust() call doesn't therefore distinguish levels of > trustedness, but is just OR'd with the result of validation against the > system trusted keyring. > > However, setting the trusted flag means that this key may be added to *any* > trusted-only keyring - including the system trusted keyring. > > Whilst I appreciate what the patch is trying to do, I don't think this is > quite the right solution. Please apply this to security/next. Thanks, David ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-07 0:34 ` David Howells @ 2016-01-07 2:13 ` Mimi Zohar 2016-01-07 3:28 ` Mimi Zohar 2016-01-07 15:31 ` Mimi Zohar 1 sibling, 1 reply; 34+ messages in thread From: Mimi Zohar @ 2016-01-07 2:13 UTC (permalink / raw) To: David Howells Cc: petkan, jmorris, linux-security-module, keyrings, linux-kernel On Thu, 2016-01-07 at 00:34 +0000, David Howells wrote: > David Howells <dhowells@redhat.com> wrote: > > > Partially revert commit 41c89b64d7184a780f12f2cccdabe65cb2408893: > > > > Author: Petko Manolov <petkan@mip-labs.com> > > Date: Wed Dec 2 17:47:55 2015 +0200 > > IMA: create machine owner and blacklist keyrings > > > > The problem is that prep->trusted is a simple boolean and the additional > > x509_validate_trust() call doesn't therefore distinguish levels of > > trustedness, but is just OR'd with the result of validation against the > > system trusted keyring. > > > > However, setting the trusted flag means that this key may be added to *any* > > trusted-only keyring - including the system trusted keyring. Hm, I'm not able to add a key to the system keyring that is signed by a key on either the system or the IMA MOK keyrings. The system keyring seems to be "locked". A key that is signed by either a key on the system or the IMA MOK keyring can be added to the IMA keyring. keyctl show %keyring:.system_keyring Keyring 973688077 ---lswrv 0 0 keyring: .system_keyring evmctl import m1-cert-signed.der 973688077 add_key failed errno: Permission denied (13) Mimi > > Whilst I appreciate what the patch is trying to do, I don't think this is > > quite the right solution. > Please apply this to security/next. > > Thanks, > David ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-07 2:13 ` Mimi Zohar @ 2016-01-07 3:28 ` Mimi Zohar 0 siblings, 0 replies; 34+ messages in thread From: Mimi Zohar @ 2016-01-07 3:28 UTC (permalink / raw) To: David Howells Cc: petkan, jmorris, linux-security-module, keyrings, linux-kernel On Wed, 2016-01-06 at 21:13 -0500, Mimi Zohar wrote: > On Thu, 2016-01-07 at 00:34 +0000, David Howells wrote: > > David Howells <dhowells@redhat.com> wrote: > > > > > Partially revert commit 41c89b64d7184a780f12f2cccdabe65cb2408893: > > > > > > Author: Petko Manolov <petkan@mip-labs.com> > > > Date: Wed Dec 2 17:47:55 2015 +0200 > > > IMA: create machine owner and blacklist keyrings > > > > > > The problem is that prep->trusted is a simple boolean and the additional > > > x509_validate_trust() call doesn't therefore distinguish levels of > > > trustedness, but is just OR'd with the result of validation against the > > > system trusted keyring. > > > > > > However, setting the trusted flag means that this key may be added to *any* > > > trusted-only keyring - including the system trusted keyring. > > Hm, I'm not able to add a key to the system keyring that is signed by a > key on either the system or the IMA MOK keyrings. The system keyring > seems to be "locked". A key that is signed by either a key on the > system or the IMA MOK keyring can be added to the IMA keyring. > > keyctl show %keyring:.system_keyring > Keyring > 973688077 ---lswrv 0 0 keyring: .system_keyring > > evmctl import m1-cert-signed.der 973688077 > add_key failed > errno: Permission denied (13) The "KEY_USR_WRITE" permission is required in order to add keys to a keyring. This permission is specified for the IMA keyring, but not for the system keyring, which explains why I could add a key to the IMA keyring, but not the system keyring. system_trusted_keyring = keyring_alloc(".system_keyring", KUIDT_INIT(0), KGIDT_INIT(0), current_cred(), ((KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH), KEY_ALLOC_NOT_IN_QUOTA, NULL); keyring[id] = keyring_alloc(keyring_name[id], KUIDT_INIT(0), KGIDT_INIT(0), cred, ((KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW | KEY_USR_READ | KEY_USR_WRITE | KEY_USR_SEARCH), KEY_ALLOC_NOT_IN_QUOTA, NULL); The only keys added to the system keyring should be those listed in the Kconfig SYSTEM_TRUSTED_KEYS specified file. Mimi ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-07 0:34 ` David Howells 2016-01-07 2:13 ` Mimi Zohar @ 2016-01-07 15:31 ` Mimi Zohar 2016-01-10 10:36 ` James Morris 1 sibling, 1 reply; 34+ messages in thread From: Mimi Zohar @ 2016-01-07 15:31 UTC (permalink / raw) To: David Howells Cc: petkan, jmorris, linux-security-module, keyrings, linux-kernel On Thu, 2016-01-07 at 00:34 +0000, David Howells wrote: > David Howells <dhowells@redhat.com> wrote: > > > Partially revert commit 41c89b64d7184a780f12f2cccdabe65cb2408893: > > > > Author: Petko Manolov <petkan@mip-labs.com> > > Date: Wed Dec 2 17:47:55 2015 +0200 > > IMA: create machine owner and blacklist keyrings > > > > The problem is that prep->trusted is a simple boolean and the additional > > x509_validate_trust() call doesn't therefore distinguish levels of > > trustedness, but is just OR'd with the result of validation against the > > system trusted keyring. > > > > However, setting the trusted flag means that this key may be added to *any* > > trusted-only keyring - including the system trusted keyring. > > > > Whilst I appreciate what the patch is trying to do, I don't think this is > > quite the right solution. > > Please apply this to security/next. The only upstreamed trusted keyrings are the system keyring, which does not permit user space to write to the keyring, and the 3 IMA keyrings. For those systems without the Kconfig IMA_MOK_KEYRING option enabled, get_ima_mok_keyring() does not change the existing behavior. For systems with IMA_MOK_KEYRING enabled, keys being added to the IMA keyring, can be validated against the system keyring or the IMA MOK keyring. Mimi ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-07 15:31 ` Mimi Zohar @ 2016-01-10 10:36 ` James Morris 2016-01-10 13:26 ` Mimi Zohar ` (2 more replies) 0 siblings, 3 replies; 34+ messages in thread From: James Morris @ 2016-01-10 10:36 UTC (permalink / raw) To: Mimi Zohar Cc: David Howells, petkan, linux-security-module, keyrings, linux-kernel On Thu, 7 Jan 2016, Mimi Zohar wrote: > On Thu, 2016-01-07 at 00:34 +0000, David Howells wrote: > > David Howells <dhowells@redhat.com> wrote: > > > > > Partially revert commit 41c89b64d7184a780f12f2cccdabe65cb2408893: > > > > > > Author: Petko Manolov <petkan@mip-labs.com> > > > Date: Wed Dec 2 17:47:55 2015 +0200 > > > IMA: create machine owner and blacklist keyrings > > > > > > The problem is that prep->trusted is a simple boolean and the additional > > > x509_validate_trust() call doesn't therefore distinguish levels of > > > trustedness, but is just OR'd with the result of validation against the > > > system trusted keyring. > > > > > > However, setting the trusted flag means that this key may be added to *any* > > > trusted-only keyring - including the system trusted keyring. > > > > > > Whilst I appreciate what the patch is trying to do, I don't think this is > > > quite the right solution. > > > > Please apply this to security/next. > > The only upstreamed trusted keyrings are the system keyring, which does > not permit user space to write to the keyring, and the 3 IMA keyrings. > > For those systems without the Kconfig IMA_MOK_KEYRING option enabled, > get_ima_mok_keyring() does not change the existing behavior. For > systems with IMA_MOK_KEYRING enabled, keys being added to the IMA > keyring, can be validated against the system keyring or the IMA MOK > keyring. > Is this a NAK on the patch? -- James Morris <jmorris@namei.org> ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-10 10:36 ` James Morris @ 2016-01-10 13:26 ` Mimi Zohar 2016-01-10 17:46 ` David Howells 2016-01-10 20:33 ` David Howells 2 siblings, 0 replies; 34+ messages in thread From: Mimi Zohar @ 2016-01-10 13:26 UTC (permalink / raw) To: James Morris Cc: David Howells, petkan, linux-security-module, keyrings, linux-kernel On Sun, 2016-01-10 at 21:36 +1100, James Morris wrote: > On Thu, 7 Jan 2016, Mimi Zohar wrote: > > > On Thu, 2016-01-07 at 00:34 +0000, David Howells wrote: > > > David Howells <dhowells@redhat.com> wrote: > > > > > > > Partially revert commit 41c89b64d7184a780f12f2cccdabe65cb2408893: > > > > > > > > Author: Petko Manolov <petkan@mip-labs.com> > > > > Date: Wed Dec 2 17:47:55 2015 +0200 > > > > IMA: create machine owner and blacklist keyrings > > > > > > > > The problem is that prep->trusted is a simple boolean and the additional > > > > x509_validate_trust() call doesn't therefore distinguish levels of > > > > trustedness, but is just OR'd with the result of validation against the > > > > system trusted keyring. > > > > > > > > However, setting the trusted flag means that this key may be added to *any* > > > > trusted-only keyring - including the system trusted keyring. > > > > > > > > Whilst I appreciate what the patch is trying to do, I don't think this is > > > > quite the right solution. > > > > > > Please apply this to security/next. > > > > The only upstreamed trusted keyrings are the system keyring, which does > > not permit user space to write to the keyring, and the 3 IMA keyrings. > > > > For those systems without the Kconfig IMA_MOK_KEYRING option enabled, > > get_ima_mok_keyring() does not change the existing behavior. For > > systems with IMA_MOK_KEYRING enabled, keys being added to the IMA > > keyring, can be validated against the system keyring or the IMA MOK > > keyring. > > > > Is this a NAK on the patch? Yes Mimi ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-10 10:36 ` James Morris 2016-01-10 13:26 ` Mimi Zohar @ 2016-01-10 17:46 ` David Howells 2016-01-10 20:33 ` Petko Manolov 2016-01-12 1:38 ` David Howells 2016-01-10 20:33 ` David Howells 2 siblings, 2 replies; 34+ messages in thread From: David Howells @ 2016-01-10 17:46 UTC (permalink / raw) To: Mimi Zohar, James Morris Cc: dhowells, petkan, linux-security-module, keyrings, linux-kernel 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 into the .ima_mok keyring. Thus the .ima_mok keyring is redundant and should be merged into the .system 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. (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. Again, I want to change this as per my recently posted patches. David ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-10 17:46 ` David Howells @ 2016-01-10 20:33 ` Petko Manolov 2016-01-12 1:38 ` David Howells 1 sibling, 0 replies; 34+ messages in thread From: Petko Manolov @ 2016-01-10 20:33 UTC (permalink / raw) To: David Howells Cc: Mimi Zohar, James Morris, linux-security-module, keyrings, linux-kernel, mdb 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-10 17:46 ` David Howells 2016-01-10 20:33 ` Petko Manolov @ 2016-01-12 1:38 ` David Howells 2016-01-12 16:14 ` Petko Manolov 2016-01-12 17:08 ` David Howells 1 sibling, 2 replies; 34+ messages in thread From: David Howells @ 2016-01-12 1:38 UTC (permalink / raw) To: Petko Manolov Cc: dhowells, Mimi Zohar, James Morris, linux-security-module, keyrings, linux-kernel, mdb 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-12 1:38 ` David Howells @ 2016-01-12 16:14 ` Petko Manolov 2016-01-12 17:08 ` David Howells 1 sibling, 0 replies; 34+ messages in thread From: Petko Manolov @ 2016-01-12 16:14 UTC (permalink / raw) To: David Howells Cc: Mimi Zohar, James Morris, linux-security-module, keyrings, linux-kernel, mdb 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-12 1:38 ` David Howells 2016-01-12 16:14 ` Petko Manolov @ 2016-01-12 17:08 ` David Howells 2016-01-13 16:31 ` Petko Manolov 1 sibling, 1 reply; 34+ messages in thread From: David Howells @ 2016-01-12 17:08 UTC (permalink / raw) To: Petko Manolov Cc: dhowells, Mimi Zohar, James Morris, linux-security-module, keyrings, linux-kernel, mdb 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-12 17:08 ` David Howells @ 2016-01-13 16:31 ` Petko Manolov 2016-01-13 17:51 ` Mimi Zohar 2016-01-13 18:19 ` David Howells 0 siblings, 2 replies; 34+ messages in thread From: Petko Manolov @ 2016-01-13 16:31 UTC (permalink / raw) To: David Howells Cc: Mimi Zohar, James Morris, linux-security-module, keyrings, linux-kernel, mdb 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-13 16:31 ` Petko Manolov @ 2016-01-13 17:51 ` Mimi Zohar 2016-01-13 18:01 ` Petko Manolov 2016-01-13 18:19 ` David Howells 1 sibling, 1 reply; 34+ messages in thread From: Mimi Zohar @ 2016-01-13 17:51 UTC (permalink / raw) To: Petko Manolov Cc: David Howells, James Morris, linux-security-module, keyrings, linux-kernel, mdb 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-13 17:51 ` Mimi Zohar @ 2016-01-13 18:01 ` Petko Manolov 0 siblings, 0 replies; 34+ messages in thread From: Petko Manolov @ 2016-01-13 18:01 UTC (permalink / raw) To: Mimi Zohar Cc: David Howells, James Morris, linux-security-module, keyrings, linux-kernel, mdb 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-13 16:31 ` Petko Manolov 2016-01-13 17:51 ` Mimi Zohar @ 2016-01-13 18:19 ` David Howells 2016-01-13 18:35 ` Petko Manolov 1 sibling, 1 reply; 34+ messages in thread From: David Howells @ 2016-01-13 18:19 UTC (permalink / raw) To: Mimi Zohar Cc: dhowells, Petko Manolov, James Morris, linux-security-module, keyrings, linux-kernel, mdb 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-13 18:19 ` David Howells @ 2016-01-13 18:35 ` Petko Manolov 2016-01-13 18:56 ` Mimi Zohar 0 siblings, 1 reply; 34+ messages in thread From: Petko Manolov @ 2016-01-13 18:35 UTC (permalink / raw) To: David Howells Cc: Mimi Zohar, James Morris, linux-security-module, keyrings, linux-kernel, mdb 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-13 18:35 ` Petko Manolov @ 2016-01-13 18:56 ` Mimi Zohar 2016-01-13 19:19 ` Petko Manolov 0 siblings, 1 reply; 34+ messages in thread From: Mimi Zohar @ 2016-01-13 18:56 UTC (permalink / raw) To: Petko Manolov Cc: David Howells, James Morris, linux-security-module, keyrings, linux-kernel, mdb 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-13 18:56 ` Mimi Zohar @ 2016-01-13 19:19 ` Petko Manolov 0 siblings, 0 replies; 34+ messages in thread From: Petko Manolov @ 2016-01-13 19:19 UTC (permalink / raw) To: Mimi Zohar Cc: David Howells, James Morris, linux-security-module, keyrings, linux-kernel, mdb 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-10 10:36 ` James Morris 2016-01-10 13:26 ` Mimi Zohar 2016-01-10 17:46 ` David Howells @ 2016-01-10 20:33 ` David Howells 2016-01-10 23:55 ` Mimi Zohar 2016-01-12 0:44 ` David Howells 2 siblings, 2 replies; 34+ messages in thread From: David Howells @ 2016-01-10 20:33 UTC (permalink / raw) To: Mimi Zohar, James Morris Cc: dhowells, Marcel Holtmann, petkan, linux-security-module, keyrings, linux-kernel David Howells <dhowells@redhat.com> 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 > into the .ima_mok keyring. Thus the .ima_mok keyring is redundant and > should be merged into the .system 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. > > (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. > > Again, I want to change this as per my recently posted patches. (4) Marcel asked to have user-based 'trusted' keyrings - where userspace can load a keyring up and then mark it as 'trusted' thereby limiting further additions - for the use with kernel-based TLS. These would *not* depend on the .system keyring. Unless we're willing to store the root CA certificate for the world in the kernel, we can't really do that. David ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-10 20:33 ` David Howells @ 2016-01-10 23:55 ` Mimi Zohar 2016-01-12 0:44 ` David Howells 1 sibling, 0 replies; 34+ messages in thread From: Mimi Zohar @ 2016-01-10 23:55 UTC (permalink / raw) To: David Howells Cc: James Morris, Marcel Holtmann, petkan, linux-security-module, keyrings, linux-kernel On Sun, 2016-01-10 at 20:33 +0000, David Howells wrote: > David Howells <dhowells@redhat.com> wrote: > (4) Marcel asked to have user-based 'trusted' keyrings - where userspace > can load a keyring up and then mark it as 'trusted' thereby limiting > further additions - for the use with kernel-based TLS. > > These would *not* depend on the .system keyring. Unless we're willing > to store the root CA certificate for the world in the kernel, we can't > really do that. Is this the primary use case scenario for your patches? Unfortunately, your posted patches would break the existing IMA trust model. Let's identify the different use case scenarios and work together to meet the different requirements. Mimi ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-10 20:33 ` David Howells 2016-01-10 23:55 ` Mimi Zohar @ 2016-01-12 0:44 ` David Howells 2016-01-12 1:28 ` Mark D. Baushke 2016-01-12 2:03 ` David Howells 1 sibling, 2 replies; 34+ messages in thread From: David Howells @ 2016-01-12 0:44 UTC (permalink / raw) To: Mimi Zohar Cc: dhowells, James Morris, Marcel Holtmann, petkan, linux-security-module, keyrings, linux-kernel Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > Is this the primary use case scenario for your patches? Unfortunately, > your posted patches would break the existing IMA trust model. Why so? The patches give you per-keyring control over restricting what is permitted in a keyring, allows you to use any criteria you like, whether it be just the contents of that keyring or CA certs in some other keyring(s) and a blacklist. In other words, it should be able to do everything one can do now - except that it controls linkage between trusted keyrings with the same restrictions as adding new keys. So if it breaks the IMA trust model, then doesn't that suggest that the trust model is broken anyway? David ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-12 0:44 ` David Howells @ 2016-01-12 1:28 ` Mark D. Baushke 2016-01-12 2:03 ` David Howells 1 sibling, 0 replies; 34+ messages in thread From: Mark D. Baushke @ 2016-01-12 1:28 UTC (permalink / raw) To: David Howells Cc: Mimi Zohar, James Morris, Marcel Holtmann, petkan, linux-security-module, keyrings, linux-kernel Hi David, David Howells <dhowells@redhat.com> writes: > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > Is this the primary use case scenario for your patches? > > Unfortunately, your posted patches would break the existing IMA > > trust model. > > Why so? The intent is that the 'vendor' of a software solution be able to provide a kernel with one or more .system keys as the root of trust. (In many cases, this root of trust may be tied to a system that does measured boot as well.) Further, that the 'vendor' provide a mechanism where a 'machine owner' is able to be delegated to add new certificate authorities to a machine owner keyring and those certificates may be used to sign signing keys which may in turn be added to the .ima and/or .evm system keyrings to ensure that IMA signed files are able to be run by the system. As you suggested in an earlier message thread, it is NOT intended that generic commercial Certificate Authorities get involved with this set of operations. The 'machine owner' key may also be used to add additional third-party certificate authorities or signing keys to appropriate keyrings in the system (generally the '.ima_mok' or the .ima keyrings). > The patches give you per-keyring control over restricting what is > permitted in a keyring, allows you to use any criteria you like, > whether it be just the contents of that keyring or CA certs in some > other keyring(s) and a blacklist. Could you ellaboriate on the identify of 'you' in this case? Is it the vendor the system which is trying to warranty the software provided to the machine owner? Or, is it the 'machine owner'? > In other words, it should be able to do everything one can do now - > except that it controls linkage between trusted keyrings with the same > restrictions as adding new keys. Hmmm... I suspect I may be missing something. If the 'vendor' selling the software desires that the 'machine owner' not extend the kernel with new kernel modules, how is that provided for in your linkage model? Another use case, is that the 'vendor' trusts a third-party to properly deliver both LKMs and user-land programs, but only if the 'machine owner' explicitly authorizes the keys for that 'third-party' explicitly. As you may see here, I am attempting to outline a use modle where it is possible to build up a hierarchy tree of trust rather than a flat set of keys that are all-powerful. Delegation to add a key into the .ima keyring is important. Keeping signing keys separate from delegated certificate authority keys is also important. > So if it breaks the IMA trust model, then doesn't that suggest that > the trust model is broken anyway? It is possible that I do not properly understand your new per-key control over a keyring. I would welcome enlightenment. One of the fundamental assumptions of a big server is that a kernel that might need to be running non-stop for many years, but might unload and reload LKMs more often than that and will certainly have the possibility of updating user-land software (hopefully without needing a reboot) on a periodic basis. I hope that my message provides some insight into the problem at hand. Thank you, -- Mark ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-12 0:44 ` David Howells 2016-01-12 1:28 ` Mark D. Baushke @ 2016-01-12 2:03 ` David Howells 2016-01-12 2:25 ` Mark D. Baushke ` (2 more replies) 1 sibling, 3 replies; 34+ messages in thread From: David Howells @ 2016-01-12 2:03 UTC (permalink / raw) To: Mark D. Baushke Cc: dhowells, Mimi Zohar, James Morris, Marcel Holtmann, petkan, linux-security-module, keyrings, linux-kernel Mark D. Baushke <mdb@juniper.net> wrote: > David Howells <dhowells@redhat.com> writes: > > > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > > > Is this the primary use case scenario for your patches? > > > Unfortunately, your posted patches would break the existing IMA > > > trust model. > > > > Why so? > > The intent is that the 'vendor' of a software solution be able to > provide a kernel with one or more .system keys as the root of trust. > (In many cases, this root of trust may be tied to a system that does > measured boot as well.) Yep. > Further, that the 'vendor' provide a mechanism where a 'machine owner' > is able to be delegated to add new certificate authorities to a machine > owner keyring Why not the .system keyring? > The 'machine owner' key may also be used to add additional third-party > certificate authorities or signing keys to appropriate keyrings in the > system (generally the '.ima_mok' or the .ima keyrings). The change I objected to makes the keys in the .system keyring and the .ima_mok keyring effectively equivalent - so why not merge .ima_mok into .system_keyring? > > The patches give you per-keyring control over restricting what is > > permitted in a keyring, allows you to use any criteria you like, > > whether it be just the contents of that keyring or CA certs in some > > other keyring(s) and a blacklist. > > Could you ellaboriate on the identify of 'you' in this case? Is it the > vendor the system which is trying to warranty the software provided to > the machine owner? Or, is it the 'machine owner'? Sorry, I meant you the kernel code author. When said author calls keyring_alloc(), they can supply a gatekeeper function as one of the parameters. This function gets to pass judgement on any link that userspace might try to make into a keyring. Note that *adding* a new key involves making a link in the keyring. See the patch ensubjected: [RFC PATCH 14/15] KEYS: Move the point of trust determination to __key_link() Search for keyring_alloc and particularly restrict_link_by_ima_mok. The restriction function cannot currently be cleared or modified by userspace - though I have an idea to make it possible to *impose* a restriction through keyctl() on any keyring that doesn't yet have a restriction imposed. The restriction function can impose any restrictions it likes, using the key's parsed payload, key type, the current keyring contents and any other keyring contents as it wishes in evaluating the trustworthiness of a key. > > In other words, it should be able to do everything one can do now - > > except that it controls linkage between trusted keyrings with the same > > restrictions as adding new keys. > > Hmmm... I suspect I may be missing something. Currently we're using the KEY_FLAG_TRUSTED and KEY_FLAG_TRUSTED_ONLY key flags to gatekeeper a additions to a keyring. KEY_FLAG_TRUSTED is set when a new proposed key is parsed, using the contents of .system_keyring - and now .ima_mok - as the authority source. KEY_FLAG_TRUSTED_ONLY is set on a keyring just after it is created (a window of opportunity which doesn't matter for pre-module keyrings). KEY_FLAG_TRUSTED_ONLY means that only keys marked KEY_FLAG_TRUSTED may be *added* to a keyring. It doesn't gate against linking trusted keys between keyrings. > If the 'vendor' selling the software desires that the 'machine owner' > not extend the kernel with new kernel modules, how is that provided for > in your linkage model? If .system_keyring was writable: keyctl clear %:.system_keyring would prevent any new keys from being added and would prevent signed modules from being loaded. > Another use case, is that the 'vendor' trusts a third-party to properly > deliver both LKMs and user-land programs, but only if the 'machine > owner' explicitly authorizes the keys for that 'third-party' explicitly. > > As you may see here, I am attempting to outline a use modle where it is > possible to build up a hierarchy tree of trust rather than a flat set of > keys that are all-powerful. However, given that the KEY_FLAG_TRUSTED mark is somewhat naive in its operation, .system_keyring and .ima_mok are effectively unioned. In the log message of the commit I want to partially revert, the .ima_mok is not self-allowing - which at least makes it worth differentiating, though that's not what the code change actually does. > Keeping signing keys separate from delegated certificate authority keys > is also important. Signing keys go in .ima only? > > So if it breaks the IMA trust model, then doesn't that suggest that > > the trust model is broken anyway? > > It is possible that I do not properly understand your new per-key > control over a keyring. I would welcome enlightenment. > > One of the fundamental assumptions of a big server is that a kernel that > might need to be running non-stop for many years, but might unload and > reload LKMs more often than that and will certainly have the possibility > of updating user-land software (hopefully without needing a reboot) on a > periodic basis. > > I hope that my message provides some insight into the problem at hand. It still doesn't clarify entirely why .ima_mok exists separately from .system_keyring from a technical point of view. David ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-12 2:03 ` David Howells @ 2016-01-12 2:25 ` Mark D. Baushke 2016-01-12 3:35 ` Mimi Zohar 2016-01-12 10:08 ` David Howells 2 siblings, 0 replies; 34+ messages in thread From: Mark D. Baushke @ 2016-01-12 2:25 UTC (permalink / raw) To: David Howells Cc: Mimi Zohar, James Morris, Marcel Holtmann, petkan, linux-security-module, keyrings, linux-kernel David Howells <dhowells@redhat.com> writes: > Signing keys go in .ima only? Yes, that is the current intent... it is not yet clear to me if it should also be in play for any .evm keys or not. > It still doesn't clarify entirely why .ima_mok exists separately from > .system_keyring from a technical point of view. IMA is an optional subsystem. Having a keyring for it to use as needed seems more flexible to me than getting all of the .system_keyring stakeholders to agree to any new semantics that might be needed. Rules on how .system_keyring mature and are managed would seem to be something that has multiple stakeholders. If a .ima_mok exists, then we can partition the rules associated with .ima_mok additions and deletions to being a chain of trust that is used to authorize a given .ima key entry to be permitted to be used and not fight with other .system_keyring semantics. For example, keyctl clear %:.sysmtem_keyring locks out adding keys to the primary keystore. That may be exactly the correct thing to do to stop adding new LKM keys. While still allowing new keys to be added to the .ima_mok that can in turn only authorize the addition/deletion of .ima keys. With regard to a .blacklist keyring type, I have no objections to sharing a blacklist accross the system. Sometime soon I need to hunt down the [RFC PATCH 14/15] KEYS:... and figure out how the restriction functions are supposed to work. I appreciate the time you have taken to write a response to my message. I hope that I have managed to provide you have a better understanding of the goals of a multi-tenant IMA system... Thanks, -- Mark ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-12 2:03 ` David Howells 2016-01-12 2:25 ` Mark D. Baushke @ 2016-01-12 3:35 ` Mimi Zohar 2016-01-12 10:08 ` David Howells 2 siblings, 0 replies; 34+ messages in thread From: Mimi Zohar @ 2016-01-12 3:35 UTC (permalink / raw) To: David Howells Cc: Mark D. Baushke, James Morris, Marcel Holtmann, petkan, linux-security-module, keyrings, linux-kernel On Tue, 2016-01-12 at 02:03 +0000, David Howells wrote: > See the patch ensubjected: > > [RFC PATCH 14/15] KEYS: Move the point of trust determination to __key_link() > > Search for keyring_alloc and particularly restrict_link_by_ima_mok. > > The restriction function cannot currently be cleared or modified by userspace > - though I have an idea to make it possible to *impose* a restriction through > keyctl() on any keyring that doesn't yet have a restriction imposed. > > The restriction function can impose any restrictions it likes, using the key's > parsed payload, key type, the current keyring contents and any other keyring > contents as it wishes in evaluating the trustworthiness of a key. One assumption is that ima-mok is always enabled, which isn't true and not the default. Depending on whether it is enabled, the ima keyring would need to be restricted by "restrict_link_by_ima_mok" or "restrict_link_by_system_trusted". The IMA MOK and blacklist are restricted to "public_key_restrict_link". Does this only allow keys signed by keys on the respective keyring or also by the system keyring? As long as the system keyring is limited to just the builtin keys, then this looks promising. Otherwise, perhaps a separate "builtin" keyring should be defined. Mimi ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-12 2:03 ` David Howells 2016-01-12 2:25 ` Mark D. Baushke 2016-01-12 3:35 ` Mimi Zohar @ 2016-01-12 10:08 ` David Howells 2016-01-12 13:21 ` Mimi Zohar ` (2 more replies) 2 siblings, 3 replies; 34+ messages in thread From: David Howells @ 2016-01-12 10:08 UTC (permalink / raw) To: Mimi Zohar Cc: dhowells, Mark D. Baushke, James Morris, Marcel Holtmann, petkan, linux-security-module, keyrings, linux-kernel Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > The IMA MOK and blacklist are restricted to "public_key_restrict_link". > Does this only allow keys signed by keys on the respective keyring or > also by the system keyring? As my patches stand, the following are implemented: (1) public_key_restrict_link() restricts to asymmetric keys that are signed by a CA in the specified keyring. It returns -ENOKEY if no matching key is found rather than -EKEYREJECTED, however, so you can call it several times for different keyrings. -EKEYREJECTED is only returned if a signature check fails. This is used by the following two functions. (2) restrict_link_by_system_trusted() restricts to asymmetric keys that are signed by a CA in the system keyring. This ignores the keyring argument it is given. Note that the system_trusted_keyring is then no longer exported because verify_pkcs7_signature() is also in certs/system_keyring.c and uses that by default if NULL is passed. (3) restrict_link_by_ima_mok() restricts to asymmetric keys signed by a CA in either .system_keyring or .ima_mok. So the trusted keyrings are then restricted as follows: (1) .system_keyring uses restrict_link_by_system_trusted() - though it lacks any sort of write permission, so it's currently moot. It could just as well be replaced with a function that just returns -EPERM. (2) .ima_mok should be using restrict_link_by_system_trusted(), but I failed to update this when I split the public_key_restrict_link() function. I've updated this in my patch. This would then be correct according to Petko's commit log: To successfully import a key into .ima_mok it must be signed by a key which CA is in .system keyring. However, from what Petko says, this is wrong and it should instead be using restrict_link_by_ima_mok(). (3) .ima_blacklist should be using restrict_link_by_system_trusted() also. I've no idea whether additions to this should be permitted by keys in .ima_mok also. (4) .ima uses restrict_link_by_ima_mok(), as per: On turn any key that needs to go in .ima keyring must be signed by CA in either .system or .ima_mok keyrings. (5) .evm is not restricted by my patches. This is a mistake on my part - but I'm not sure what the restriction actually needs to be as it's not mentioned in Petko's commit message. Presumably it needs the same as .ima. David ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-12 10:08 ` David Howells @ 2016-01-12 13:21 ` Mimi Zohar 2016-01-12 13:55 ` David Howells 2016-01-12 14:11 ` Petko Manolov 2 siblings, 0 replies; 34+ messages in thread From: Mimi Zohar @ 2016-01-12 13:21 UTC (permalink / raw) To: David Howells Cc: Mark D. Baushke, James Morris, Marcel Holtmann, petkan, linux-security-module, keyrings, linux-kernel On Tue, 2016-01-12 at 10:08 +0000, David Howells wrote: > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > The IMA MOK and blacklist are restricted to "public_key_restrict_link". > > Does this only allow keys signed by keys on the respective keyring or > > also by the system keyring? > > As my patches stand, the following are implemented: > > (1) public_key_restrict_link() restricts to asymmetric keys that are signed > by a CA in the specified keyring. It returns -ENOKEY if no matching key > is found rather than -EKEYREJECTED, however, so you can call it several > times for different keyrings. -EKEYREJECTED is only returned if a > signature check fails. This is used by the following two functions. Ok, so it is restricted it to CAs. Combining it with an option to limit it to the builtin CA keys based on the builtin flag would be nice. > (2) restrict_link_by_system_trusted() restricts to asymmetric keys that are > signed by a CA in the system keyring. This ignores the keyring argument > it is given. > Note that the system_trusted_keyring is then no longer exported because > verify_pkcs7_signature() is also in certs/system_keyring.c and uses that > by default if NULL is passed. Ok > (3) restrict_link_by_ima_mok() restricts to asymmetric keys signed by a CA in > either .system_keyring or .ima_mok. > So the trusted keyrings are then restricted as follows: > > (1) .system_keyring uses restrict_link_by_system_trusted() - though it lacks > any sort of write permission, so it's currently moot. It could just as > well be replaced with a function that just returns -EPERM. Why not retain the current semantics of the system keyring of not being writable and create a new keyring for new feature(s)? > (2) .ima_mok should be using restrict_link_by_system_trusted(), but I failed > to update this when I split the public_key_restrict_link() function. > I've updated this in my patch. This would then be correct according to > Petko's commit log: > > To successfully import a key into .ima_mok it must be signed by a > key which CA is in .system keyring. > > However, from what Petko says, this is wrong and it should instead be > using restrict_link_by_ima_mok(). The name "restrict_link_by_ima_mok()" doesn't reflect that it is either the system keyring or the IMA MOK keyring. > (3) .ima_blacklist should be using restrict_link_by_system_trusted() also. > I've no idea whether additions to this should be permitted by keys in > .ima_mok also. Petko/Mark, please correct me if I'm wrong. It would be the same as the IMA MOK keyring. > (4) .ima uses restrict_link_by_ima_mok(), as per: > > On turn any key that needs to go in .ima keyring must be signed by CA > in either .system or .ima_mok keyrings. Currently, if IMA MOK is not enabled, then the keyring isn't created and shouldn't be referenced. The default should be restrict_link_by_system_trusted. > (5) .evm is not restricted by my patches. This is a mistake on my part - but > I'm not sure what the restriction actually needs to be as it's not > mentioned in Petko's commit message. Presumably it needs the same as > .ima. Currently, security.evm xattrs are not portable across systems. On first access, security.evm xattrs containing file signatures are converted to an HMAC. This probably will change in the future, but for now .evm should be restrict_link_by_system_trusted as well. David, thank you for the detailed explanation. Mimi ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-12 10:08 ` David Howells 2016-01-12 13:21 ` Mimi Zohar @ 2016-01-12 13:55 ` David Howells 2016-01-12 15:17 ` Mimi Zohar 2016-01-12 15:56 ` David Howells 2016-01-12 14:11 ` Petko Manolov 2 siblings, 2 replies; 34+ messages in thread From: David Howells @ 2016-01-12 13:55 UTC (permalink / raw) To: Mimi Zohar Cc: dhowells, Mark D. Baushke, James Morris, Marcel Holtmann, petkan, linux-security-module, keyrings, linux-kernel Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > (1) public_key_restrict_link() restricts to asymmetric keys that are > > signed by a CA in the specified keyring. It returns -ENOKEY if no > > matching key is found rather than -EKEYREJECTED, however, so you can > > call it several times for different keyrings. -EKEYREJECTED is only > > returned if a signature check fails. This is used by the following > > two functions. > > Ok, so it is restricted it to CAs. Combining it with an option to limit > it to the builtin CA keys based on the builtin flag would be nice. Is there a point to the builtin flag if .system_keyring is closed? Currently all keys that go into .system_keyring are marked BUILTIN. But, yes, the restriction can include only using built in CAs. > > (1) .system_keyring uses restrict_link_by_system_trusted() - though it > > lacks any sort of write permission, so it's currently moot. It could > > just as well be replaced with a function that just returns -EPERM. > > Why not retain the current semantics of the system keyring of not being > writable and create a new keyring for new feature(s)? I think that the problem we have is that it can be argued either way. You would rather create a new keyring to hold additional keys, whereas I would prefer to use the keyring we already have. Do you have a technical reason why we can't just open the system keyring? It's not precisely a new feature, but rather an extension to an existing one that's been under consideration for a while. > The name "restrict_link_by_ima_mok()" doesn't reflect that it is either > the system keyring or the IMA MOK keyring. How about restrict_link_by_ima_trusted()? David ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-12 13:55 ` David Howells @ 2016-01-12 15:17 ` Mimi Zohar 2016-01-12 15:56 ` David Howells 1 sibling, 0 replies; 34+ messages in thread From: Mimi Zohar @ 2016-01-12 15:17 UTC (permalink / raw) To: David Howells Cc: Mark D. Baushke, James Morris, Marcel Holtmann, petkan, linux-security-module, keyrings, linux-kernel On Tue, 2016-01-12 at 13:55 +0000, David Howells wrote: > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > > (1) public_key_restrict_link() restricts to asymmetric keys that are > > > signed by a CA in the specified keyring. It returns -ENOKEY if no > > > matching key is found rather than -EKEYREJECTED, however, so you can > > > call it several times for different keyrings. -EKEYREJECTED is only > > > returned if a signature check fails. This is used by the following > > > two functions. > > > > Ok, so it is restricted it to CAs. Combining it with an option to limit > > it to the builtin CA keys based on the builtin flag would be nice. > > Is there a point to the builtin flag if .system_keyring is closed? Currently > all keys that go into .system_keyring are marked BUILTIN. It becomes a problem if the existing system keyring semantics of not being writable change or if non builtin keys are added to the system keyring. Discussion continued below. > But, yes, the restriction can include only using built in CAs. > > > (1) .system_keyring uses restrict_link_by_system_trusted() - though it > > > lacks any sort of write permission, so it's currently moot. It could > > > just as well be replaced with a function that just returns -EPERM. > > > > Why not retain the current semantics of the system keyring of not being > > writable and create a new keyring for new feature(s)? > > I think that the problem we have is that it can be argued either way. You > would rather create a new keyring to hold additional keys, whereas I would > prefer to use the keyring we already have. > > Do you have a technical reason why we can't just open the system keyring? > It's not precisely a new feature, but rather an extension to an existing one > that's been under consideration for a while. There are two main concerns: - Different keys are trusted for different things (eg. firmware, modules, regular files) and at different levels of the OS. I guess this could be addressed, as you've previously suggested, by identifiers. - The other issue is transitive trust (eg. A trusts B. B trusts C. Permit anything signed by C). In some use case scenarios this is desired, in others it is not. We would need the option to limit trust to just the builtin keys to support both use cases. Even with the identifier support and ability to limit transitive trust, I doubt it is as safe as limiting the system keyring to just the builtin keys. Refer to Petko's post. > > The name "restrict_link_by_ima_mok()" doesn't reflect that it is either > > the system keyring or the IMA MOK keyring. > > How about restrict_link_by_ima_trusted()? Good. restrict_link_by_ima_trusted would only check the IMA MOK keyring if it was configured. Mimi ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-12 13:55 ` David Howells 2016-01-12 15:17 ` Mimi Zohar @ 2016-01-12 15:56 ` David Howells 2016-01-12 16:02 ` Mimi Zohar 1 sibling, 1 reply; 34+ messages in thread From: David Howells @ 2016-01-12 15:56 UTC (permalink / raw) To: Mimi Zohar Cc: dhowells, Mark D. Baushke, James Morris, Marcel Holtmann, petkan, linux-security-module, keyrings, linux-kernel Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > The name "restrict_link_by_ima_mok()" doesn't reflect that it is either > > > the system keyring or the IMA MOK keyring. > > > > How about restrict_link_by_ima_trusted()? > > Good. restrict_link_by_ima_trusted would only check the IMA MOK keyring > if it was configured. And the system keyring? David ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-12 15:56 ` David Howells @ 2016-01-12 16:02 ` Mimi Zohar 0 siblings, 0 replies; 34+ messages in thread From: Mimi Zohar @ 2016-01-12 16:02 UTC (permalink / raw) To: David Howells Cc: Mark D. Baushke, James Morris, Marcel Holtmann, petkan, linux-security-module, keyrings, linux-kernel On Tue, 2016-01-12 at 15:56 +0000, David Howells wrote: > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > > > > The name "restrict_link_by_ima_mok()" doesn't reflect that it is either > > > > the system keyring or the IMA MOK keyring. > > > > > > How about restrict_link_by_ima_trusted()? > > > > Good. restrict_link_by_ima_trusted would only check the IMA MOK keyring > > if it was configured. > > And the system keyring? Only keys signed by the system keyring can be added to the .ima keyring, unless IMA MOK is enabled. In that case, keys signed by either the system or IMA MOK keyring can be added to the .ima keyring. Mimi ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring 2016-01-12 10:08 ` David Howells 2016-01-12 13:21 ` Mimi Zohar 2016-01-12 13:55 ` David Howells @ 2016-01-12 14:11 ` Petko Manolov 2 siblings, 0 replies; 34+ messages in thread From: Petko Manolov @ 2016-01-12 14:11 UTC (permalink / raw) To: David Howells Cc: Mimi Zohar, Mark D. Baushke, James Morris, Marcel Holtmann, linux-security-module, keyrings, linux-kernel 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2016-01-13 19:19 UTC | newest] Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-01-06 13:45 [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring David Howells 2016-01-07 0:04 ` James Morris 2016-01-07 0:34 ` David Howells 2016-01-07 2:13 ` Mimi Zohar 2016-01-07 3:28 ` Mimi Zohar 2016-01-07 15:31 ` Mimi Zohar 2016-01-10 10:36 ` James Morris 2016-01-10 13:26 ` Mimi Zohar 2016-01-10 17:46 ` David Howells 2016-01-10 20:33 ` Petko Manolov 2016-01-12 1:38 ` David Howells 2016-01-12 16:14 ` Petko Manolov 2016-01-12 17:08 ` David Howells 2016-01-13 16:31 ` Petko Manolov 2016-01-13 17:51 ` Mimi Zohar 2016-01-13 18:01 ` Petko Manolov 2016-01-13 18:19 ` David Howells 2016-01-13 18:35 ` Petko Manolov 2016-01-13 18:56 ` Mimi Zohar 2016-01-13 19:19 ` Petko Manolov 2016-01-10 20:33 ` David Howells 2016-01-10 23:55 ` Mimi Zohar 2016-01-12 0:44 ` David Howells 2016-01-12 1:28 ` Mark D. Baushke 2016-01-12 2:03 ` David Howells 2016-01-12 2:25 ` Mark D. Baushke 2016-01-12 3:35 ` Mimi Zohar 2016-01-12 10:08 ` David Howells 2016-01-12 13:21 ` Mimi Zohar 2016-01-12 13:55 ` David Howells 2016-01-12 15:17 ` Mimi Zohar 2016-01-12 15:56 ` David Howells 2016-01-12 16:02 ` Mimi Zohar 2016-01-12 14:11 ` Petko Manolov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.