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 1 of 2  [1] 2  Next page →

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

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;
 	}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

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

> 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>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

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

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

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

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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

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

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>

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web