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

Groups > linux.kernel > #1325538 > unrolled thread

Re: [RFC PATCH 02/20] KEYS: Add a system blacklist keyring [ver #2]

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH 02/20] KEYS: Add a system blacklist keyring [ver #2] David Howells <dhowells@redhat.com> - 2016-02-03 16:30 +0100
    Re: [RFC PATCH 02/20] KEYS: Add a system blacklist keyring [ver #2] Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-02-08 14:40 +0100
      Re: [RFC PATCH 02/20] KEYS: Add a system blacklist keyring [ver #2] David Howells <dhowells@redhat.com> - 2016-02-08 15:00 +0100
        Re: [RFC PATCH 02/20] KEYS: Add a system blacklist keyring [ver #2] Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-02-08 16:10 +0100
          How to add additional blacklist entries? David Howells <dhowells@redhat.com> - 2016-02-08 17:00 +0100
            Re: How to add additional blacklist entries? Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-02-08 17:40 +0100
              Re: How to add additional blacklist entries? David Howells <dhowells@redhat.com> - 2016-02-08 17:50 +0100
                Re: How to add additional blacklist entries? Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-02-08 20:30 +0100
      Re: [RFC PATCH 02/20] KEYS: Add a system blacklist keyring [ver #2] David Howells <dhowells@redhat.com> - 2016-02-08 16:00 +0100
        Re: [RFC PATCH 02/20] KEYS: Add a system blacklist keyring [ver #2] Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-02-08 17:50 +0100

#1325538 — Re: [RFC PATCH 02/20] KEYS: Add a system blacklist keyring [ver #2]

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

> >  (3) The ability to configure a list of blacklisted hashes into the kernel
> >      at build time.  This is done by setting
> >      CONFIG_SYSTEM_BLACKLIST_HASH_LIST to the filename of a list of hashes
> >      that are in the form:
> > 
> > 	"<hash>", "<hash>", ..., "<hash>"
> > 
> >      where each <hash> is a hex string representation of the hash and must
> >      include all necessary leading zeros to pad the hash to the right size.
> 
> Is the output of "keyctl print" the hex string representation?

No, there is no payload and no read method.  "keyctl desc" will return the hex
string representation.

>  Update keys documentation?

Not a bad idea, but it should probably go in a separate document, along with
info about asymmetric keys.

> > The blacklist cannot currently be modified by userspace, but it will be
> > possible to load it, for example, from the UEFI blacklist database.
> 
> When loading the UEFI blacklist database is enabled, it should be
> configurable.

Probably.  That patch isn't added yet though.

> > In the future, it should also be made possible to load blacklisted
> > asymmetric keys in here too.
> 
> Please update to reflect patch 3/20 "X.509: Allow X.509 certs to be
> blacklisted" adds this support.

Changed to:

	A later commit will make it possible to load blacklisted asymmetric
	keys in here too.
 
David

[toc] | [next] | [standalone]

#1329044

On Wed, 2016-02-03 at 15:27 +0000, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > >  (3) The ability to configure a list of blacklisted hashes into the kernel
> > >      at build time.  This is done by setting
> > >      CONFIG_SYSTEM_BLACKLIST_HASH_LIST to the filename of a list of hashes
> > >      that are in the form:
> > > 
> > > 	"<hash>", "<hash>", ..., "<hash>"
> > > 
> > >      where each <hash> is a hex string representation of the hash and must
> > >      include all necessary leading zeros to pad the hash to the right size.
> > 
> > Is the output of "keyctl print" the hex string representation?
> 
> No, there is no payload and no read method.  "keyctl desc" will return the hex
> string representation.
> 
> >  Update keys documentation?
> 
> Not a bad idea, but it should probably go in a separate document, along with
> info about asymmetric keys.
> 
> > > The blacklist cannot currently be modified by userspace, but it will be
> > > possible to load it, for example, from the UEFI blacklist database.
> > 
> > When loading the UEFI blacklist database is enabled, it should be
> > configurable.
> 
> Probably.  That patch isn't added yet though.
> 
> > > In the future, it should also be made possible to load blacklisted
> > > asymmetric keys in here too.
> > 
> > Please update to reflect patch 3/20 "X.509: Allow X.509 certs to be
> > blacklisted" adds this support.
> 
> Changed to:
> 
> 	A later commit will make it possible to load blacklisted asymmetric
> 	keys in here too.

As you said, only the kernel can load keys on the blacklist, not
userspace, and the patch for loading the UEFI blacklist keys on the
system blacklist keyring is not included in this patch set, wouldn't it
be better to separate the concept of a general blacklist keyring from
the concept of trust?   In addition, this patch set removes the IMA
blacklist without any method for adding blacklisted IMA keys to the
system blacklist keyring.   I suggest a separate blacklist patch set
that adds support: for a system blacklist keyring, different types of
keys being black listed, allow userspace to load blacklisted keys on the
system blacklist keyring,  convert the ima_blacklist to use the general
system blacklist keyring and, optionally, include the UEFI blacklist
keys on the blacklist keyring.

This patch set would then be limited to "how certificates/keys are
determined to be trusted."  By separating out the blacklist keyring from
the issue of trust, you'll have smaller patch sets that can more easily
be reviewed. (Reviewing anything having to do with certificates is
difficult enough.)  It would also allow you to upstream the two patch
sets independently of each other.

Mimi

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

#1329073

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

> In addition, this patch set removes the IMA blacklist without any method for
> adding blacklisted IMA keys to the system blacklist keyring.

That's not true.

Patch 18 enables userspace to add keys to the system blacklist keyring,
provided those keys are validly signed:

-			      KEY_USR_SEARCH,
+			      KEY_USR_SEARCH | KEY_USR_WRITE,
 			      KEY_ALLOC_NOT_IN_QUOTA |
 			      KEY_FLAG_KEEP,
-			      NULL, NULL);
+			      restrict_link_by_system_trusted, NULL);

After this commit, you can do everything with the system blacklist keyring
that you can currently do with the IMA blacklist keyring.

David

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

#1329146

On Mon, 2016-02-08 at 13:55 +0000, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > In addition, this patch set removes the IMA blacklist without any method for
> > adding blacklisted IMA keys to the system blacklist keyring.
> 
> That's not true.
> 
> Patch 18 enables userspace to add keys to the system blacklist keyring,
> provided those keys are validly signed:
> 
> -			      KEY_USR_SEARCH,
> +			      KEY_USR_SEARCH | KEY_USR_WRITE,
>  			      KEY_ALLOC_NOT_IN_QUOTA |
>  			      KEY_FLAG_KEEP,
> -			      NULL, NULL);
> +			      restrict_link_by_system_trusted, NULL);
> 
> After this commit, you can do everything with the system blacklist keyring
> that you can currently do with the IMA blacklist keyring.

Right, this patch makes the system blacklist keyring writable by
userspace and removes the IMA blacklist.  What I don't understand is how
to add a key that is currently on the IMA keyring to the system
blacklist? 

With the IMA blacklist, the same certificate that was added to the IMA
keyring could be added to the blacklist. (Probably not the best idea.)

The system black list currently only supports the  TBSCertificate hash,
not the key-id.   I have the signed certificate being added to the IMA
keyring.  I'm missing the step of getting the TBSCertificate hash based
on the certificate.

Mimi

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

#1329238 — How to add additional blacklist entries?

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

> Right, this patch makes the system blacklist keyring writable by
> userspace and removes the IMA blacklist.  What I don't understand is how
> to add a key that is currently on the IMA keyring to the system
> blacklist? 

You can do this from userspace with "keyctl link".  Admittedly, this attaches
the entire key to the blacklist keyring, not just the ID.  But that's
basically what you're doing at the moment, right.

To simply list the SKID of the key you want to blacklist, another patch will
be required, but the question is as to what the interface should look like.

Let's start at the beginning.  First of all, let me ask the following:

 (1) How is the key-to-be-blacklisted specified?  A copy of the X.509 cert to
     be blocked?  A signed list of SKIDs to be blocked?  A CRL?

 (2) How is the blacklist addition to be verified?

David

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

#1329287 — Re: How to add additional blacklist entries?

On Mon, 2016-02-08 at 15:53 +0000, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > Right, this patch makes the system blacklist keyring writable by
> > userspace and removes the IMA blacklist.  What I don't understand is how
> > to add a key that is currently on the IMA keyring to the system
> > blacklist? 
> 
> You can do this from userspace with "keyctl link".  Admittedly, this attaches
> the entire key to the blacklist keyring, not just the ID.  But that's
> basically what you're doing at the moment, right.

Does this imply that the key already has to be loaded onto a keyring in
order to link it to the blacklist?   Currently the key doesn't need to
be on the IMA keyring in order for it to be black listed.  The cert can
be verified, that it is signed by a key on the system trusted (or
ima_mok) keyring(s), before directly being added to the IMA blacklist
keyring.

> To simply list the SKID of the key you want to blacklist, another patch will
> be required, but the question is as to what the interface should look like.
> 
> Let's start at the beginning.  First of all, let me ask the following:
> 
>  (1) How is the key-to-be-blacklisted specified?  A copy of the X.509 cert to
>      be blocked?  A signed list of SKIDs to be blocked?  A CRL?

Similar to the TBScertificate hash list, there should be support for a
SKIDs list, either in the same file or separately.

>  (2) How is the blacklist addition to be verified?

As I recall without going back and looking at the patches, you've
defined a new key type for just the TBScertficate hash without a
payload.  Is it possible to do the equivalent for SKIDs?  In both cases,
these new key type(s) would need to be signed by a key on the system
keyring (now called the builtin keyring) for it to be added to the
blacklist.

Mimi

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

#1329313 — Re: How to add additional blacklist entries?

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

> > > Right, this patch makes the system blacklist keyring writable by
> > > userspace and removes the IMA blacklist.  What I don't understand is how
> > > to add a key that is currently on the IMA keyring to the system
> > > blacklist? 
> > 
> > You can do this from userspace with "keyctl link".  Admittedly, this
> > attaches the entire key to the blacklist keyring, not just the ID.  But
> > that's basically what you're doing at the moment, right.
> 
> Does this imply that the key already has to be loaded onto a keyring in
> order to link it to the blacklist?   Currently the key doesn't need to
> be on the IMA keyring in order for it to be black listed.  The cert can
> be verified, that it is signed by a key on the system trusted (or
> ima_mok) keyring(s), before directly being added to the IMA blacklist
> keyring.

You can link from any key you have LINK permission on.  Further, add_key() can
add directly.

> > To simply list the SKID of the key you want to blacklist, another patch
> > will be required, but the question is as to what the interface should look
> > like.
> > 
> > Let's start at the beginning.  First of all, let me ask the following:
> > 
> >  (1) How is the key-to-be-blacklisted specified?  A copy of the X.509 cert
> >      to be blocked?  A signed list of SKIDs to be blocked?  A CRL?
> 
> Similar to the TBScertificate hash list, there should be support for a
> SKIDs list, either in the same file or separately.

Separately probably makes sense - and marking the blacklist keys with
something that says what is to be checked.

> >  (2) How is the blacklist addition to be verified?
> 
> As I recall without going back and looking at the patches, you've
> defined a new key type for just the TBScertficate hash without a
> payload.

Sort of.  It carries a hash string as a description.  One of the patches
matches this with the X.509 TBScertficate hash.  I should look at adding
another patch to check the PE file content hash for kexec also.

> Is it possible to do the equivalent for SKIDs?

Yes.

> In both cases, these new key type(s) would need to be signed by a key on the
> system keyring (now called the builtin keyring) for it to be added to the
> blacklist.

I think you may have misunderstood the point of the question.  Assuming we're
loading a SKID list from userspace, how do we validate the list?  Is it
wrapped in an X.509 cert, a PKCS#7 message or is it a binary blob with an
associated signature?

Or are you proposing the SKID list be built into the kernel at compile time
and not modifiable at runtime?

David

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

#1329495 — Re: How to add additional blacklist entries?

On Mon, 2016-02-08 at 16:43 +0000, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > > > Right, this patch makes the system blacklist keyring writable by
> > > > userspace and removes the IMA blacklist.  What I don't understand is how
> > > > to add a key that is currently on the IMA keyring to the system
> > > > blacklist? 
> > > 
> > > You can do this from userspace with "keyctl link".  Admittedly, this
> > > attaches the entire key to the blacklist keyring, not just the ID.  But
> > > that's basically what you're doing at the moment, right.
> > 
> > Does this imply that the key already has to be loaded onto a keyring in
> > order to link it to the blacklist?   Currently the key doesn't need to
> > be on the IMA keyring in order for it to be black listed.  The cert can
> > be verified, that it is signed by a key on the system trusted (or
> > ima_mok) keyring(s), before directly being added to the IMA blacklist
> > keyring.
> 
> You can link from any key you have LINK permission on.  Further, add_key() can
> add directly.

Oh, for some reason I thought the system blacklist keyring was limited
to the new key type with just a description.   I was able to add, but
also remove a key from the system blacklist.   I guess the KEY_FLAG_KEEP
is not set on the system blacklist.

> > > To simply list the SKID of the key you want to blacklist, another patch
> > > will be required, but the question is as to what the interface should look
> > > like.
> > > 
> > > Let's start at the beginning.  First of all, let me ask the following:
> > > 
> > >  (1) How is the key-to-be-blacklisted specified?  A copy of the X.509 cert
> > >      to be blocked?  A signed list of SKIDs to be blocked?  A CRL?
> > 
> > Similar to the TBScertificate hash list, there should be support for a
> > SKIDs list, either in the same file or separately.
> 
> Separately probably makes sense - and marking the blacklist keys with
> something that says what is to be checked.
> 
> > >  (2) How is the blacklist addition to be verified?
> > 
> > As I recall without going back and looking at the patches, you've
> > defined a new key type for just the TBScertficate hash without a
> > payload.
> 
> Sort of.  It carries a hash string as a description.  One of the patches
> matches this with the X.509 TBScertficate hash.  I should look at adding
> another patch to check the PE file content hash for kexec also.
> 
> > Is it possible to do the equivalent for SKIDs?
> 
> Yes.
> 
> > In both cases, these new key type(s) would need to be signed by a key on the
> > system keyring (now called the builtin keyring) for it to be added to the
> > blacklist.
> 
> I think you may have misunderstood the point of the question.  Assuming we're
> loading a SKID list from userspace, how do we validate the list?  Is it
> wrapped in an X.509 cert, a PKCS#7 message or is it a binary blob with an
> associated signature?

The keys being added to the IMA keyring are signed x509 certs (eg.
openssl ca -ss_cert).  It would be nice to be able to include the skid
in the description, without a payload, and sign that.  I have no idea if
that Is possible or if it makes sense.  I'm open to suggestions.

> Or are you proposing the SKID list be built into the kernel at compile time
> and not modifiable at runtime?

No, we definitely want to be able to blacklist keys at run time.

Mimi

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

#1329142

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

> By separating out the blacklist keyring from the issue of trust, you'll have
> smaller patch sets that can more easily be reviewed. (Reviewing anything
> having to do with certificates is difficult enough.)  It would also allow
> you to upstream the two patch sets independently of each other.

Unfortunately, there's a dependency between the subsets you're talking about
in the form of the restriction function passed to keyring_alloc() - an
argument that's only made available in the other subset, so they cannot be
completely independent.

That said, the trust changes don't require the blacklist changes.

David

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

#1329317

On Mon, 2016-02-08 at 14:55 +0000, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > By separating out the blacklist keyring from the issue of trust, you'll have
> > smaller patch sets that can more easily be reviewed. (Reviewing anything
> > having to do with certificates is difficult enough.)  It would also allow
> > you to upstream the two patch sets independently of each other.
> 
> Unfortunately, there's a dependency between the subsets you're talking about
> in the form of the restriction function passed to keyring_alloc() - an
> argument that's only made available in the other subset, so they cannot be
> completely independent.
> 
> That said, the trust changes don't require the blacklist changes.

Right, I should have said the blacklist changes are dependent on the
trusted keyring changes.

In the "trust" patch set, could you please include Mehmet's patch?

Thanks!

Mimi

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web