mbox series

[0/7] Add CA enforcement keyring restrictions

Message ID 20220406015337.4000739-1-eric.snowberg@oracle.com
Headers show
Series Add CA enforcement keyring restrictions | expand

Message

Eric Snowberg April 6, 2022, 1:53 a.m. UTC
A key added to the ima keyring must be signed by a key contained within 
either the builtin trusted or secondary trusted keyrings. Currently, there are 
CA restrictions described in IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY,
but these restrictions are not enforced within code. Therefore, keys within 
either the builtin or secondary may not be a CA and could be used to
vouch for an ima key.

The machine keyring can not be used as another trust anchor for adding keys 
to the ima keyring, since CA enforcement does not currently exist [1]. This 
would expand the current integrity gap.

Introduce a new root of trust key flag to close this integrity gap for
all keyrings.  The first key type to use this is X.509.  When a X.509 
certificate is self signed, contains kernCertSign Key Usage and contains 
the CA bit, the new flag is set.  Introduce new keyring restrictions 
that not only validates a key is signed by a key contained within the 
keyring, but also validates the key has the new root of trust key flag 
set.  Use this new restriction for keys added to the ima keyring.  Now 
that we have CA enforcement, allow the machine keyring to be used as another 
trust anchor for the ima keyring.

To recap, all keys that previously loaded into the builtin, secondary or
machine keyring will still load after applying this series.  Keys
contained within these keyrings may carry the root of trust flag. The
ima keyring will use the new root of trust restriction to validate
CA enforcement. Other keyrings that require a root of trust could also 
use this in the future.

[1] https://lore.kernel.org/lkml/2d681148b6ea57241f6a7c518dd331068a5f47b0.camel@linux.ibm.com/

Eric Snowberg (7):
  KEYS: Create static version of public_key_verify_signature
  KEYS: X.509: Parse Basic Constraints for CA
  KEYS: X.509: Parse Key Usage
  KEYS: Introduce a builtin root of trust key flag
  KEYS: Introduce sig restriction that validates root of trust
  KEYS: X.509: Flag Intermediate CA certs as built in
  integrity: Use root of trust signature restriction

 certs/system_keyring.c                    | 18 ++++++++++
 crypto/asymmetric_keys/restrict.c         | 42 +++++++++++++++++++++++
 crypto/asymmetric_keys/x509_cert_parser.c | 29 ++++++++++++++++
 crypto/asymmetric_keys/x509_parser.h      |  2 ++
 crypto/asymmetric_keys/x509_public_key.c  | 12 +++++++
 include/crypto/public_key.h               |  9 +++++
 include/keys/system_keyring.h             | 17 ++++++++-
 include/linux/ima.h                       | 16 +++++++++
 include/linux/key-type.h                  |  3 ++
 include/linux/key.h                       |  2 ++
 security/integrity/Kconfig                |  1 -
 security/integrity/digsig.c               |  4 +--
 security/keys/key.c                       | 13 +++++++
 13 files changed, 164 insertions(+), 4 deletions(-)


base-commit: 3123109284176b1532874591f7c81f3837bbdc17

Comments

Mimi Zohar April 6, 2022, 8:45 p.m. UTC | #1
Hi Eric,

On Tue, 2022-04-05 at 21:53 -0400, Eric Snowberg wrote:
> A key added to the ima keyring must be signed by a key contained within 
> either the builtin trusted or secondary trusted keyrings. Currently, there are 
> CA restrictions described in IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY,
> but these restrictions are not enforced within code. Therefore, keys within 
> either the builtin or secondary may not be a CA and could be used to
> vouch for an ima key.
> 
> The machine keyring can not be used as another trust anchor for adding keys 
> to the ima keyring, since CA enforcement does not currently exist [1]. This 
> would expand the current integrity gap.
> 
> Introduce a new root of trust key flag to close this integrity gap for
> all keyrings.  The first key type to use this is X.509.  When a X.509 
> certificate is self signed, contains kernCertSign Key Usage and contains 
> the CA bit, the new flag is set.  Introduce new keyring restrictions 
> that not only validates a key is signed by a key contained within the 
> keyring, but also validates the key has the new root of trust key flag 
> set.  Use this new restriction for keys added to the ima keyring.  Now 
> that we have CA enforcement, allow the machine keyring to be used as another 
> trust anchor for the ima keyring.
> 
> To recap, all keys that previously loaded into the builtin, secondary or
> machine keyring will still load after applying this series.  Keys
> contained within these keyrings may carry the root of trust flag. The
> ima keyring will use the new root of trust restriction to validate
> CA enforcement. Other keyrings that require a root of trust could also 
> use this in the future.

Your initial patch set indicated that you were addressing Linus'
request to allow end-users the ability "to add their own keys and sign
modules they trust".  However, from the design of the previous patch
set and now this one, everything indicates a lot more is going on than
just allowing end-users to add their own keys.  There would be no
reason for loading all the MOK keys, rather than just the CA keys, onto
the "machine" keyring.  Please provide the motivation for this design.

Please note that Patch 6/7 permits intermediary CA keys, without any
mention of it in the cover letter.  Please include this in the
motivation for this design.

thanks,

Mimi
Eric Snowberg April 6, 2022, 10:53 p.m. UTC | #2
> On Apr 6, 2022, at 2:45 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> Hi Eric,
> 
> On Tue, 2022-04-05 at 21:53 -0400, Eric Snowberg wrote:
>> A key added to the ima keyring must be signed by a key contained within 
>> either the builtin trusted or secondary trusted keyrings. Currently, there are 
>> CA restrictions described in IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY,
>> but these restrictions are not enforced within code. Therefore, keys within 
>> either the builtin or secondary may not be a CA and could be used to
>> vouch for an ima key.
>> 
>> The machine keyring can not be used as another trust anchor for adding keys 
>> to the ima keyring, since CA enforcement does not currently exist [1]. This 
>> would expand the current integrity gap.
>> 
>> Introduce a new root of trust key flag to close this integrity gap for
>> all keyrings.  The first key type to use this is X.509.  When a X.509 
>> certificate is self signed, contains kernCertSign Key Usage and contains 
>> the CA bit, the new flag is set.  Introduce new keyring restrictions 
>> that not only validates a key is signed by a key contained within the 
>> keyring, but also validates the key has the new root of trust key flag 
>> set.  Use this new restriction for keys added to the ima keyring.  Now 
>> that we have CA enforcement, allow the machine keyring to be used as another 
>> trust anchor for the ima keyring.
>> 
>> To recap, all keys that previously loaded into the builtin, secondary or
>> machine keyring will still load after applying this series.  Keys
>> contained within these keyrings may carry the root of trust flag. The
>> ima keyring will use the new root of trust restriction to validate
>> CA enforcement. Other keyrings that require a root of trust could also 
>> use this in the future.
> 
> Your initial patch set indicated that you were addressing Linus'
> request to allow end-users the ability "to add their own keys and sign
> modules they trust".  However, from the design of the previous patch
> set and now this one, everything indicates a lot more is going on than
> just allowing end-users to add their own keys.  There would be no
> reason for loading all the MOK keys, rather than just the CA keys, onto
> the "machine" keyring.  Please provide the motivation for this design.

The motivation is to satisfy both Linus and your requests. Linus requested 
the ability to allow users to add their own keys and sign modules they trust.  
A code signing CA certificate does not require kernCertSign in the usage. Adding 
this as a requirement for kernel modules would be a regression (or a bug).

This series addresses your request to only trust validly signed CA certs. 
As you pointed out in the Kconfig help for 
IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY:

help
  Keys may be added to the IMA or IMA blacklist keyrings, if the
  key is validly signed by a CA cert in the system built-in or
  secondary trusted keyrings.

  Intermediate keys between those the kernel has compiled in and the 
  IMA keys to be added may be added to the system secondary keyring,
  provided they are validly signed by a key already resident in the
  built-in or secondary trusted keyrings.

requires keys to be “validly” signed by a CA cert. Later the definition of a 
validly signed CA cert was defined as: self signed, contains kernCertSign 
key usage and contains the CA bit. While this help file states the CA restriction, 
nothing in code enforces it.  One can place any type of self signed cert in either 
keyring and ima will use it.  The motivation is for all keys added to the ima 
keyring to abide by the restriction defined in the Kconfig help.  With this series 
this can be accomplished without introducing a regression on keys placed in 
any of the system keyrings.

> Please note that Patch 6/7 permits intermediary CA keys, without any
> mention of it in the cover letter.  Please include this in the
> motivation for this design.

Ok, I’ll add that in the next round.
Mimi Zohar April 8, 2022, 2:41 p.m. UTC | #3
On Wed, 2022-04-06 at 22:53 +0000, Eric Snowberg wrote:
> 
> > On Apr 6, 2022, at 2:45 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > Hi Eric,
> > 
> > On Tue, 2022-04-05 at 21:53 -0400, Eric Snowberg wrote:
> >> A key added to the ima keyring must be signed by a key contained within 
> >> either the builtin trusted or secondary trusted keyrings. Currently, there are 
> >> CA restrictions described in IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY,
> >> but these restrictions are not enforced within code. Therefore, keys within 
> >> either the builtin or secondary may not be a CA and could be used to
> >> vouch for an ima key.
> >> 
> >> The machine keyring can not be used as another trust anchor for adding keys 
> >> to the ima keyring, since CA enforcement does not currently exist [1]. This 
> >> would expand the current integrity gap.
> >> 
> >> Introduce a new root of trust key flag to close this integrity gap for
> >> all keyrings.  The first key type to use this is X.509.  When a X.509 
> >> certificate is self signed, contains kernCertSign Key Usage and contains 
> >> the CA bit, the new flag is set.  Introduce new keyring restrictions 
> >> that not only validates a key is signed by a key contained within the 
> >> keyring, but also validates the key has the new root of trust key flag 
> >> set.  Use this new restriction for keys added to the ima keyring.  Now 
> >> that we have CA enforcement, allow the machine keyring to be used as another 
> >> trust anchor for the ima keyring.
> >> 
> >> To recap, all keys that previously loaded into the builtin, secondary or
> >> machine keyring will still load after applying this series.  Keys
> >> contained within these keyrings may carry the root of trust flag. The
> >> ima keyring will use the new root of trust restriction to validate
> >> CA enforcement. Other keyrings that require a root of trust could also 
> >> use this in the future.
> > 
> > Your initial patch set indicated that you were addressing Linus'
> > request to allow end-users the ability "to add their own keys and sign
> > modules they trust".  However, from the design of the previous patch
> > set and now this one, everything indicates a lot more is going on than
> > just allowing end-users to add their own keys.  There would be no
> > reason for loading all the MOK keys, rather than just the CA keys, onto
> > the "machine" keyring.  Please provide the motivation for this design.
> 
> The motivation is to satisfy both Linus and your requests. Linus requested 
> the ability to allow users to add their own keys and sign modules they trust.  
> A code signing CA certificate does not require kernCertSign in the usage. Adding 
> this as a requirement for kernel modules would be a regression (or a bug).

Of course a code signing CA certificate should not also be a
certificate signing key (keyCertSign).  Remember the
"builtin_trusted_keys" and "secondary_trusted_keys" keyrings are
special.  Their root of trust is based on a secure boot signature chain
of trust up to and including a signed kernel image.  The "machine"
keyring is totally different in this regard.  Establishing a new root
of trust is really difficult.  Requiring a root-CA to have key
certifcate signing usage is a level of indirection, which I would
consider a small price to pay for being able to establish a, hopefully
safe or at least safer, new root of trust for trusting "end-user" keys.

> 
> This series addresses your request to only trust validly signed CA certs. 
> As you pointed out in the Kconfig help for 
> IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY:
> 
> help
>   Keys may be added to the IMA or IMA blacklist keyrings, if the
>   key is validly signed by a CA cert in the system built-in or
>   secondary trusted keyrings.
> 
>   Intermediate keys between those the kernel has compiled in and the 
>   IMA keys to be added may be added to the system secondary keyring,
>   provided they are validly signed by a key already resident in the
>   built-in or secondary trusted keyrings.
> 
> requires keys to be “validly” signed by a CA cert. Later the definition of a 
> validly signed CA cert was defined as: self signed, contains kernCertSign 
> key usage and contains the CA bit. While this help file states the CA restriction, 
> nothing in code enforces it.  One can place any type of self signed cert in either 
> keyring and ima will use it.  The motivation is for all keys added to the ima 
> keyring to abide by the restriction defined in the Kconfig help.  With this series 
> this can be accomplished without introducing a regression on keys placed in 
> any of the system keyrings.
> 
> > Please note that Patch 6/7 permits intermediary CA keys, without any
> > mention of it in the cover letter.  Please include this in the
> > motivation for this design.
> 
> Ok, I’ll add that in the next round.

Your cover letter should say that this patch series enables
verification of 3rd party modules.

thanks,

Mimi
Coiby Xu Nov. 4, 2022, 1:20 p.m. UTC | #4
Hi Eric,

I wonder if there is any update on this work? I would be glad to do
anything that may be helpful including testing a new version of code.
Eric Snowberg Nov. 4, 2022, 9:06 p.m. UTC | #5
> On Nov 4, 2022, at 7:20 AM, Coiby Xu <coxu@redhat.com> wrote:
> 
> Hi Eric,
> 
> I wonder if there is any update on this work? I would be glad to do
> anything that may be helpful including testing a new version of code.
> 
> -- 
> Best regards,
> Coiby


The discussion on this topic briefly moved over to this thread [1].  I took 
the lack of response to mean an approach like this would not be considered.  
If it would be considered, I am willing to continue working on a solution 
to this problem.

1. https://lore.kernel.org/linux-integrity/8BB9D406-0394-4E2E-9B84-4A320AFDBDC4@oracle.com/
Elaine Palmer Nov. 9, 2022, 1:24 a.m. UTC | #6
On 2022/11/04 9:20 AM, Coiby Xu wrote:
> Hi Eric,
>
> I wonder if there is any update on this work? I would be glad to do
> anything that may be helpful including testing a new version of code.
>
Hi Coiby,

Yes, this discussion got stuck when we couldn't agree on one of the
following options:

(A) Filter which keys from MOK (or a management system) are loaded
    onto the .machine keyring. Specifically, load only keys with
    CA+keyCertSign attributes.

(B) Load all keys from MOK (or a management system) onto the
    .machine keyring. Then, subsequently filter those to restrict
    which ones can be loaded onto the .ima keyring specifically.

The objection to (A) was that distros would have to go through
two steps instead of one to load keys. The one-step method of
loading keys was supported by an out-of-tree patch and then by
the addition of the .machine keyring.

The objection to (B) was that, because the .machine keyring is now
linked to the .secondary keyring, it expands the scope of what the
kernel has trusted in the past. The effect is that keys in MOK
have the same broad scope as keys previously restricted to
.builtin and .secondary. It doesn't affect just IMA, but the rest
of the kernel as well.

I would suggest that we can get unstuck by considering:

(C) Defining a systemd (or dracut module) to load keys onto the
    .secondary keyring

(D) Using a configuration option to specify what types of
    .machine keys should be allowed to pass through to the
    .secondary keyring.
   
    The distro could choose (A) by allowing only
    CA+keyCertSign keys.

    The distro could choose (B) by allowing any kind
    of key.

We all seemed to agree that enforcing key usage should be
implemented and that a useful future effort is to add policies
to keys and keyrings, like, "This key can only be used for
verifying kernel modules."

I hope we can come to an agreement so work can proceed and IMA
can be re-enabled.

-Elaine Palmer
Eric Snowberg Nov. 9, 2022, 2:25 p.m. UTC | #7
> On Nov 8, 2022, at 6:24 PM, Elaine Palmer <erpalmerny@gmail.com> wrote:
> 
> 
> 
> On 2022/11/04 9:20 AM, Coiby Xu wrote:
>> Hi Eric,
>> 
>> I wonder if there is any update on this work? I would be glad to do
>> anything that may be helpful including testing a new version of code.
>> 
> Hi Coiby,
> 
> Yes, this discussion got stuck when we couldn't agree on one of the
> following options:
> 
> (A) Filter which keys from MOK (or a management system) are loaded
>     onto the .machine keyring. Specifically, load only keys with
>     CA+keyCertSign attributes.
> 
> (B) Load all keys from MOK (or a management system) onto the
>     .machine keyring. Then, subsequently filter those to restrict
>     which ones can be loaded onto the .ima keyring specifically.
> 
> The objection to (A) was that distros would have to go through
> two steps instead of one to load keys. The one-step method of
> loading keys was supported by an out-of-tree patch and then by
> the addition of the .machine keyring.
> 
> The objection to (B) was that, because the .machine keyring is now
> linked to the .secondary keyring, it expands the scope of what the
> kernel has trusted in the past. The effect is that keys in MOK
> have the same broad scope as keys previously restricted to
> .builtin and .secondary. It doesn't affect just IMA, but the rest
> of the kernel as well.
> 
> I would suggest that we can get unstuck by considering:
> 
> (C) Defining a systemd (or dracut module) to load keys onto the
>     .secondary keyring
> 
> (D) Using a configuration option to specify what types of
>     .machine keys should be allowed to pass through to the
>     .secondary keyring.
>    
>     The distro could choose (A) by allowing only
>     CA+keyCertSign keys.
> 
>     The distro could choose (B) by allowing any kind
>     of key.
> 
> We all seemed to agree that enforcing key usage should be
> implemented and that a useful future effort is to add policies
> to keys and keyrings, like, "This key can only be used for
> verifying kernel modules."
> 
> I hope we can come to an agreement so work can proceed and IMA
> can be re-enabled.

I would be open to making the changes necessary to support both (A and B) 
options.  What type of configuration option would be considered?  Would this 
be a compile time Kconfig, a Linux boot command line parameter, or another 
MOK variable?
Elaine Palmer Nov. 9, 2022, 2:58 p.m. UTC | #8
On 2022/11/09 9:25 AM, Eric Snowberg wrote:
>
>> On Nov 8, 2022, at 6:24 PM, Elaine Palmer <erpalmerny@gmail.com> wrote:
>>
>>
>>
>> On 2022/11/04 9:20 AM, Coiby Xu wrote:
>>> Hi Eric,
>>>
>>> I wonder if there is any update on this work? I would be glad to do
>>> anything that may be helpful including testing a new version of code.
>>>
>> Hi Coiby,
>>
>> Yes, this discussion got stuck when we couldn't agree on one of the
>> following options:
>>
>> (A) Filter which keys from MOK (or a management system) are loaded
>>     onto the .machine keyring. Specifically, load only keys with
>>     CA+keyCertSign attributes.
>>
>> (B) Load all keys from MOK (or a management system) onto the
>>     .machine keyring. Then, subsequently filter those to restrict
>>     which ones can be loaded onto the .ima keyring specifically.
>>
>> The objection to (A) was that distros would have to go through
>> two steps instead of one to load keys. The one-step method of
>> loading keys was supported by an out-of-tree patch and then by
>> the addition of the .machine keyring.
>>
>> The objection to (B) was that, because the .machine keyring is now
>> linked to the .secondary keyring, it expands the scope of what the
>> kernel has trusted in the past. The effect is that keys in MOK
>> have the same broad scope as keys previously restricted to
>> .builtin and .secondary. It doesn't affect just IMA, but the rest
>> of the kernel as well.
>>
>> I would suggest that we can get unstuck by considering:
>>
>> (C) Defining a systemd (or dracut module) to load keys onto the
>>     .secondary keyring
>>
>> (D) Using a configuration option to specify what types of
>>     .machine keys should be allowed to pass through to the
>>     .secondary keyring.
>>    
>>     The distro could choose (A) by allowing only
>>     CA+keyCertSign keys.
>>
>>     The distro could choose (B) by allowing any kind
>>     of key.
>>
>> We all seemed to agree that enforcing key usage should be
>> implemented and that a useful future effort is to add policies
>> to keys and keyrings, like, "This key can only be used for
>> verifying kernel modules."
>>
>> I hope we can come to an agreement so work can proceed and IMA
>> can be re-enabled.
> I would be open to making the changes necessary to support both (A and B) 
> options.  What type of configuration option would be considered?  Would this 
> be a compile time Kconfig, a Linux boot command line parameter, or another 
> MOK variable?
>
Thank you, Eric.  A compile time Kconfig would be the most secure, yet
would still support (B) when allowed.