Re: [PATCH] certs: Restrict blacklist updates to the secondary trusted keyring

2023-09-12 Thread Eric Snowberg


> On Sep 12, 2023, at 4:47 PM, Mimi Zohar  wrote:
> 
> On Tue, 2023-09-12 at 17:11 +, Eric Snowberg wrote:
>> 
>>> On Sep 12, 2023, at 5:54 AM, Mimi Zohar  wrote:
>>> 
>>> On Tue, 2023-09-12 at 02:00 +, Eric Snowberg wrote:
 
> On Sep 11, 2023, at 5:08 PM, Mimi Zohar  wrote:
> 
> On Mon, 2023-09-11 at 22:17 +, Eric Snowberg wrote:
>> 
>>> On Sep 11, 2023, at 10:51 AM, Mickaël Salaün  wrote:
>>> 
>>> On Mon, Sep 11, 2023 at 09:29:07AM -0400, Mimi Zohar wrote:
 Hi Eric,
 
 On Fri, 2023-09-08 at 17:34 -0400, Eric Snowberg wrote:
> Currently root can dynamically update the blacklist keyring if the 
> hash
> being added is signed and vouched for by the builtin trusted keyring.
> Currently keys in the secondary trusted keyring can not be used.
> 
> Keys within the secondary trusted keyring carry the same capabilities 
> as
> the builtin trusted keyring.  Relax the current restriction for 
> updating
> the .blacklist keyring and allow the secondary to also be referenced 
> as
> a trust source.  Since the machine keyring is linked to the secondary
> trusted keyring, any key within it may also be used.
> 
> An example use case for this is IMA appraisal.  Now that IMA both
> references the blacklist keyring and allows the machine owner to add
> custom IMA CA certs via the machine keyring, this adds the additional
> capability for the machine owner to also do revocations on a running
> system.
> 
> IMA appraisal usage example to add a revocation for /usr/foo:
> 
> sha256sum /bin/foo | awk '{printf "bin:" $1}' > hash.txt
> 
> openssl smime -sign -in hash.txt -inkey machine-private-key.pem \
>-signer machine-certificate.pem -noattr -binary -outform DER \
>-out hash.p7s
> 
> keyctl padd blacklist "$(< hash.txt)" %:.blacklist < hash.p7s
> 
> Signed-off-by: Eric Snowberg 
 
 The secondary keyring may include both CA and code signing keys.  With
 this change any key loaded onto the secondary keyring may blacklist a
 hash.  Wouldn't it make more sense to limit blacklisting
 certificates/hashes to at least CA keys? 
>>> 
>>> Some operational constraints may limit what a CA can sign.
>> 
>> Agreed.  
>> 
>> Is there precedents for requiring this S/MIME to be signed by a CA? 
>> 
>>> This change is critical and should be tied to a dedicated kernel config
>>> (disabled by default), otherwise existing systems using this feature
>>> will have their threat model automatically changed without notice.
>> 
>> Today we have INTEGRITY_CA_MACHINE_KEYRING_MAX.  This can 
>> be enabled to enforce CA restrictions on the machine keyring.  Mimi, 
>> would 
>> this be a suitable solution for what you are after?
> 
> There needs to be some correlation between the file hashes being added
> to the blacklist and the certificate that signed them.  Without that
> correlation, any key on the secondary trusted keyring could add any
> file hashes it wants to the blacklist.
 
 Today any key in the secondary trusted keyring can be used to validate a 
 signed kernel module.  At a later time, if a new hash is added to the 
 blacklist 
 keyring to revoke loading a signed kernel module,  the ability to do the 
 revocation with this additional change would be more restrictive than 
 loading 
 the original module.
>>> 
>>> A public key on the secondary keyring is used to verify code that it
>>> signed, but does not impact any other code. Allowing any public key on
>>> the secondary keyring to blacklist any file hash is giving it more
>>> privileges than it originally had.
>>> 
>>> This requirement isn't different than how Certificate Revocation List
>>> (CRL) work.  Not any CA can revoke a certificate.
>> 
>> In UEFI Secure Boot we have the Forbidden Signature Database (DBX).  
>> Root can update the DBX on a host.  The requirement placed on updating 
>> it is the new DBX entry must be signed by any key contained within the 
>> KEK.  Following a reboot, all DBX entries load into the .blacklist keyring.  
>> There is not a requirement similar to how CRL’s work here, any KEK key 
>> can be used.
>> 
>> With architectures booted through a shim there is the MOKX.  Similar to 
>> DBX, MOKX have the same capabilities, however they do not need to be 
>> signed by any key, the machine owner must show they have physical 
>> presence (and potentially a MOK password) for inclusion.  Again there 
>> is not a requirement similar to how CRL’s work here either.  The machine 
>> owner can decide what is included.
>> 
>> Today when a kernel is built, any number of keys may be included within 
>> the builtin

Re: [PATCH] certs: Restrict blacklist updates to the secondary trusted keyring

2023-09-12 Thread Mimi Zohar
On Tue, 2023-09-12 at 17:11 +, Eric Snowberg wrote:
> 
> > On Sep 12, 2023, at 5:54 AM, Mimi Zohar  wrote:
> > 
> > On Tue, 2023-09-12 at 02:00 +, Eric Snowberg wrote:
> >> 
> >>> On Sep 11, 2023, at 5:08 PM, Mimi Zohar  wrote:
> >>> 
> >>> On Mon, 2023-09-11 at 22:17 +, Eric Snowberg wrote:
>  
> > On Sep 11, 2023, at 10:51 AM, Mickaël Salaün  wrote:
> > 
> > On Mon, Sep 11, 2023 at 09:29:07AM -0400, Mimi Zohar wrote:
> >> Hi Eric,
> >> 
> >> On Fri, 2023-09-08 at 17:34 -0400, Eric Snowberg wrote:
> >>> Currently root can dynamically update the blacklist keyring if the 
> >>> hash
> >>> being added is signed and vouched for by the builtin trusted keyring.
> >>> Currently keys in the secondary trusted keyring can not be used.
> >>> 
> >>> Keys within the secondary trusted keyring carry the same capabilities 
> >>> as
> >>> the builtin trusted keyring.  Relax the current restriction for 
> >>> updating
> >>> the .blacklist keyring and allow the secondary to also be referenced 
> >>> as
> >>> a trust source.  Since the machine keyring is linked to the secondary
> >>> trusted keyring, any key within it may also be used.
> >>> 
> >>> An example use case for this is IMA appraisal.  Now that IMA both
> >>> references the blacklist keyring and allows the machine owner to add
> >>> custom IMA CA certs via the machine keyring, this adds the additional
> >>> capability for the machine owner to also do revocations on a running
> >>> system.
> >>> 
> >>> IMA appraisal usage example to add a revocation for /usr/foo:
> >>> 
> >>> sha256sum /bin/foo | awk '{printf "bin:" $1}' > hash.txt
> >>> 
> >>> openssl smime -sign -in hash.txt -inkey machine-private-key.pem \
> >>> -signer machine-certificate.pem -noattr -binary -outform DER \
> >>> -out hash.p7s
> >>> 
> >>> keyctl padd blacklist "$(< hash.txt)" %:.blacklist < hash.p7s
> >>> 
> >>> Signed-off-by: Eric Snowberg 
> >> 
> >> The secondary keyring may include both CA and code signing keys.  With
> >> this change any key loaded onto the secondary keyring may blacklist a
> >> hash.  Wouldn't it make more sense to limit blacklisting
> >> certificates/hashes to at least CA keys? 
> > 
> > Some operational constraints may limit what a CA can sign.
>  
>  Agreed.  
>  
>  Is there precedents for requiring this S/MIME to be signed by a CA? 
>  
> > This change is critical and should be tied to a dedicated kernel config
> > (disabled by default), otherwise existing systems using this feature
> > will have their threat model automatically changed without notice.
>  
>  Today we have INTEGRITY_CA_MACHINE_KEYRING_MAX.  This can 
>  be enabled to enforce CA restrictions on the machine keyring.  Mimi, 
>  would 
>  this be a suitable solution for what you are after?
> >>> 
> >>> There needs to be some correlation between the file hashes being added
> >>> to the blacklist and the certificate that signed them.  Without that
> >>> correlation, any key on the secondary trusted keyring could add any
> >>> file hashes it wants to the blacklist.
> >> 
> >> Today any key in the secondary trusted keyring can be used to validate a 
> >> signed kernel module.  At a later time, if a new hash is added to the 
> >> blacklist 
> >> keyring to revoke loading a signed kernel module,  the ability to do the 
> >> revocation with this additional change would be more restrictive than 
> >> loading 
> >> the original module.
> > 
> > A public key on the secondary keyring is used to verify code that it
> > signed, but does not impact any other code. Allowing any public key on
> > the secondary keyring to blacklist any file hash is giving it more
> > privileges than it originally had.
> > 
> > This requirement isn't different than how Certificate Revocation List
> > (CRL) work.  Not any CA can revoke a certificate.
> 
> In UEFI Secure Boot we have the Forbidden Signature Database (DBX).  
> Root can update the DBX on a host.  The requirement placed on updating 
> it is the new DBX entry must be signed by any key contained within the 
> KEK.  Following a reboot, all DBX entries load into the .blacklist keyring.  
> There is not a requirement similar to how CRL’s work here, any KEK key 
> can be used.
> 
> With architectures booted through a shim there is the MOKX.  Similar to 
> DBX, MOKX have the same capabilities, however they do not need to be 
> signed by any key, the machine owner must show they have physical 
> presence (and potentially a MOK password) for inclusion.  Again there 
> is not a requirement similar to how CRL’s work here either.  The machine 
> owner can decide what is included.
> 
> Today when a kernel is built, any number of keys may be included within 
> the builtin trusted keyring.  The keys included in the kernel may not have 
> a sing

Re: [PATCH] certs: Restrict blacklist updates to the secondary trusted keyring

2023-09-12 Thread Eric Snowberg


> On Sep 12, 2023, at 5:54 AM, Mimi Zohar  wrote:
> 
> On Tue, 2023-09-12 at 02:00 +, Eric Snowberg wrote:
>> 
>>> On Sep 11, 2023, at 5:08 PM, Mimi Zohar  wrote:
>>> 
>>> On Mon, 2023-09-11 at 22:17 +, Eric Snowberg wrote:
 
> On Sep 11, 2023, at 10:51 AM, Mickaël Salaün  wrote:
> 
> On Mon, Sep 11, 2023 at 09:29:07AM -0400, Mimi Zohar wrote:
>> Hi Eric,
>> 
>> On Fri, 2023-09-08 at 17:34 -0400, Eric Snowberg wrote:
>>> Currently root can dynamically update the blacklist keyring if the hash
>>> being added is signed and vouched for by the builtin trusted keyring.
>>> Currently keys in the secondary trusted keyring can not be used.
>>> 
>>> Keys within the secondary trusted keyring carry the same capabilities as
>>> the builtin trusted keyring.  Relax the current restriction for updating
>>> the .blacklist keyring and allow the secondary to also be referenced as
>>> a trust source.  Since the machine keyring is linked to the secondary
>>> trusted keyring, any key within it may also be used.
>>> 
>>> An example use case for this is IMA appraisal.  Now that IMA both
>>> references the blacklist keyring and allows the machine owner to add
>>> custom IMA CA certs via the machine keyring, this adds the additional
>>> capability for the machine owner to also do revocations on a running
>>> system.
>>> 
>>> IMA appraisal usage example to add a revocation for /usr/foo:
>>> 
>>> sha256sum /bin/foo | awk '{printf "bin:" $1}' > hash.txt
>>> 
>>> openssl smime -sign -in hash.txt -inkey machine-private-key.pem \
>>> -signer machine-certificate.pem -noattr -binary -outform DER \
>>> -out hash.p7s
>>> 
>>> keyctl padd blacklist "$(< hash.txt)" %:.blacklist < hash.p7s
>>> 
>>> Signed-off-by: Eric Snowberg 
>> 
>> The secondary keyring may include both CA and code signing keys.  With
>> this change any key loaded onto the secondary keyring may blacklist a
>> hash.  Wouldn't it make more sense to limit blacklisting
>> certificates/hashes to at least CA keys? 
> 
> Some operational constraints may limit what a CA can sign.
 
 Agreed.  
 
 Is there precedents for requiring this S/MIME to be signed by a CA? 
 
> This change is critical and should be tied to a dedicated kernel config
> (disabled by default), otherwise existing systems using this feature
> will have their threat model automatically changed without notice.
 
 Today we have INTEGRITY_CA_MACHINE_KEYRING_MAX.  This can 
 be enabled to enforce CA restrictions on the machine keyring.  Mimi, would 
 this be a suitable solution for what you are after?
>>> 
>>> There needs to be some correlation between the file hashes being added
>>> to the blacklist and the certificate that signed them.  Without that
>>> correlation, any key on the secondary trusted keyring could add any
>>> file hashes it wants to the blacklist.
>> 
>> Today any key in the secondary trusted keyring can be used to validate a 
>> signed kernel module.  At a later time, if a new hash is added to the 
>> blacklist 
>> keyring to revoke loading a signed kernel module,  the ability to do the 
>> revocation with this additional change would be more restrictive than 
>> loading 
>> the original module.
> 
> A public key on the secondary keyring is used to verify code that it
> signed, but does not impact any other code. Allowing any public key on
> the secondary keyring to blacklist any file hash is giving it more
> privileges than it originally had.
> 
> This requirement isn't different than how Certificate Revocation List
> (CRL) work.  Not any CA can revoke a certificate.

In UEFI Secure Boot we have the Forbidden Signature Database (DBX).  
Root can update the DBX on a host.  The requirement placed on updating 
it is the new DBX entry must be signed by any key contained within the 
KEK.  Following a reboot, all DBX entries load into the .blacklist keyring.  
There is not a requirement similar to how CRL’s work here, any KEK key 
can be used.

With architectures booted through a shim there is the MOKX.  Similar to 
DBX, MOKX have the same capabilities, however they do not need to be 
signed by any key, the machine owner must show they have physical 
presence (and potentially a MOK password) for inclusion.  Again there 
is not a requirement similar to how CRL’s work here either.  The machine 
owner can decide what is included.

Today when a kernel is built, any number of keys may be included within 
the builtin trusted keyring.  The keys included in the kernel may not have 
a single usage field set or the CA bit set.  There are no requirements on 
how these keys get used later on.  Any key in the builtin trusted keyring 
can be used to sign a revocation that can be added to the blacklist keyring.  
Additionally, any key in the MOK can be used to sign this kernel and it will 
boot. 

Re: [PATCH] certs: Restrict blacklist updates to the secondary trusted keyring

2023-09-12 Thread Mimi Zohar
On Tue, 2023-09-12 at 02:00 +, Eric Snowberg wrote:
> 
> > On Sep 11, 2023, at 5:08 PM, Mimi Zohar  wrote:
> > 
> > On Mon, 2023-09-11 at 22:17 +, Eric Snowberg wrote:
> >> 
> >>> On Sep 11, 2023, at 10:51 AM, Mickaël Salaün  wrote:
> >>> 
> >>> On Mon, Sep 11, 2023 at 09:29:07AM -0400, Mimi Zohar wrote:
>  Hi Eric,
>  
>  On Fri, 2023-09-08 at 17:34 -0400, Eric Snowberg wrote:
> > Currently root can dynamically update the blacklist keyring if the hash
> > being added is signed and vouched for by the builtin trusted keyring.
> > Currently keys in the secondary trusted keyring can not be used.
> > 
> > Keys within the secondary trusted keyring carry the same capabilities as
> > the builtin trusted keyring.  Relax the current restriction for updating
> > the .blacklist keyring and allow the secondary to also be referenced as
> > a trust source.  Since the machine keyring is linked to the secondary
> > trusted keyring, any key within it may also be used.
> > 
> > An example use case for this is IMA appraisal.  Now that IMA both
> > references the blacklist keyring and allows the machine owner to add
> > custom IMA CA certs via the machine keyring, this adds the additional
> > capability for the machine owner to also do revocations on a running
> > system.
> > 
> > IMA appraisal usage example to add a revocation for /usr/foo:
> > 
> > sha256sum /bin/foo | awk '{printf "bin:" $1}' > hash.txt
> > 
> > openssl smime -sign -in hash.txt -inkey machine-private-key.pem \
> >  -signer machine-certificate.pem -noattr -binary -outform DER \
> >  -out hash.p7s
> > 
> > keyctl padd blacklist "$(< hash.txt)" %:.blacklist < hash.p7s
> > 
> > Signed-off-by: Eric Snowberg 
>  
>  The secondary keyring may include both CA and code signing keys.  With
>  this change any key loaded onto the secondary keyring may blacklist a
>  hash.  Wouldn't it make more sense to limit blacklisting
>  certificates/hashes to at least CA keys? 
> >>> 
> >>> Some operational constraints may limit what a CA can sign.
> >> 
> >> Agreed.  
> >> 
> >> Is there precedents for requiring this S/MIME to be signed by a CA? 
> >> 
> >>> This change is critical and should be tied to a dedicated kernel config
> >>> (disabled by default), otherwise existing systems using this feature
> >>> will have their threat model automatically changed without notice.
> >> 
> >> Today we have INTEGRITY_CA_MACHINE_KEYRING_MAX.  This can 
> >> be enabled to enforce CA restrictions on the machine keyring.  Mimi, would 
> >> this be a suitable solution for what you are after?
> > 
> > There needs to be some correlation between the file hashes being added
> > to the blacklist and the certificate that signed them.  Without that
> > correlation, any key on the secondary trusted keyring could add any
> > file hashes it wants to the blacklist.
> 
> Today any key in the secondary trusted keyring can be used to validate a 
> signed kernel module.  At a later time, if a new hash is added to the 
> blacklist 
> keyring to revoke loading a signed kernel module,  the ability to do the 
> revocation with this additional change would be more restrictive than loading 
> the original module.

A public key on the secondary keyring is used to verify code that it
signed, but does not impact any other code. Allowing any public key on
the secondary keyring to blacklist any file hash is giving it more
privileges than it originally had.

This requirement isn't different than how Certificate Revocation List
(CRL) work.  Not any CA can revoke a certificate.

> 
> But, if you think it would be appropriate, I could add a new Kconfig 
> (disabled 
> by default) that validates the key being used to vouch the S/MIME encoded 
> hash is a CA.  That would certainly make this more complicated.   With this 
> addition, would  the key usage field need to be referenced too?
> 
> Another idea I had was changing this patch to reference only the builtin and 
> the machine keyring (if configured), not the secondary keyring.   Then with
> INTEGRITY_CA_MACHINE_KEYRING_MAX, only CA keys could be 
> used. Let me know your thoughts on this approach.  Thanks.

Better, but it doesn't address the underlying problem.

-- 
thanks,

Mimi



Re: [PATCH] certs: Restrict blacklist updates to the secondary trusted keyring

2023-09-11 Thread Jarkko Sakkinen
On Sat Sep 9, 2023 at 12:34 AM EEST, Eric Snowberg wrote:
> Currently root can dynamically update the blacklist keyring if the hash
> being added is signed and vouched for by the builtin trusted keyring.
> Currently keys in the secondary trusted keyring can not be used.
>
> Keys within the secondary trusted keyring carry the same capabilities as
> the builtin trusted keyring.  Relax the current restriction for updating
> the .blacklist keyring and allow the secondary to also be referenced as
> a trust source.  Since the machine keyring is linked to the secondary
> trusted keyring, any key within it may also be used.
>
> An example use case for this is IMA appraisal.  Now that IMA both
> references the blacklist keyring and allows the machine owner to add
> custom IMA CA certs via the machine keyring, this adds the additional
> capability for the machine owner to also do revocations on a running
> system.
>
> IMA appraisal usage example to add a revocation for /usr/foo:
>
> sha256sum /bin/foo | awk '{printf "bin:" $1}' > hash.txt
>
> openssl smime -sign -in hash.txt -inkey machine-private-key.pem \
>-signer machine-certificate.pem -noattr -binary -outform DER \
>-out hash.p7s
>
> keyctl padd blacklist "$(< hash.txt)" %:.blacklist < hash.p7s
>
> Signed-off-by: Eric Snowberg 
> ---
>  certs/Kconfig | 2 +-
>  certs/blacklist.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/certs/Kconfig b/certs/Kconfig
> index 1f109b070877..23dc87c52aff 100644
> --- a/certs/Kconfig
> +++ b/certs/Kconfig
> @@ -134,7 +134,7 @@ config SYSTEM_BLACKLIST_AUTH_UPDATE
>   depends on SYSTEM_DATA_VERIFICATION
>   help
> If set, provide the ability to load new blacklist keys at run time if
> -   they are signed and vouched by a certificate from the builtin trusted
> +   they are signed and vouched by a certificate from the secondary 
> trusted
> keyring.  The PKCS#7 signature of the description is set in the key
> payload.  Blacklist keys cannot be removed.
>  
> diff --git a/certs/blacklist.c b/certs/blacklist.c
> index 675dd7a8f07a..0b346048ae2d 100644
> --- a/certs/blacklist.c
> +++ b/certs/blacklist.c
> @@ -102,12 +102,12 @@ static int blacklist_key_instantiate(struct key *key,
>  
>  #ifdef CONFIG_SYSTEM_BLACKLIST_AUTH_UPDATE
>   /*
> -  * Verifies the description's PKCS#7 signature against the builtin
> +  * Verifies the description's PKCS#7 signature against the secondary
>* trusted keyring.
>*/
>   err = verify_pkcs7_signature(key->description,
>   strlen(key->description), prep->data, prep->datalen,
> - NULL, VERIFYING_UNSPECIFIED_SIGNATURE, NULL, NULL);
> + VERIFY_USE_SECONDARY_KEYRING, 
> VERIFYING_UNSPECIFIED_SIGNATURE, NULL, NULL);
>   if (err)
>   return err;
>  #else
> -- 
> 2.39.3

What if a live system in the wild assumes the old policy? I feel that
this is "sort of" breaking backwards compatibility but please prove me
wrong.

BR, Jarkko


Re: [PATCH] certs: Restrict blacklist updates to the secondary trusted keyring

2023-09-11 Thread Jarkko Sakkinen
On Mon Sep 11, 2023 at 4:29 PM EEST, Mimi Zohar wrote:
> Hi Eric,
>
> On Fri, 2023-09-08 at 17:34 -0400, Eric Snowberg wrote:
> > Currently root can dynamically update the blacklist keyring if the hash
> > being added is signed and vouched for by the builtin trusted keyring.
> > Currently keys in the secondary trusted keyring can not be used.
> > 
> > Keys within the secondary trusted keyring carry the same capabilities as
> > the builtin trusted keyring.  Relax the current restriction for updating
> > the .blacklist keyring and allow the secondary to also be referenced as
> > a trust source.  Since the machine keyring is linked to the secondary
> > trusted keyring, any key within it may also be used.
> > 
> > An example use case for this is IMA appraisal.  Now that IMA both
> > references the blacklist keyring and allows the machine owner to add
> > custom IMA CA certs via the machine keyring, this adds the additional
> > capability for the machine owner to also do revocations on a running
> > system.
> > 
> > IMA appraisal usage example to add a revocation for /usr/foo:
> > 
> > sha256sum /bin/foo | awk '{printf "bin:" $1}' > hash.txt
> > 
> > openssl smime -sign -in hash.txt -inkey machine-private-key.pem \
> >-signer machine-certificate.pem -noattr -binary -outform DER \
> >-out hash.p7s
> > 
> > keyctl padd blacklist "$(< hash.txt)" %:.blacklist < hash.p7s
> > 
> > Signed-off-by: Eric Snowberg 
>
> The secondary keyring may include both CA and code signing keys.  With
> this change any key loaded onto the secondary keyring may blacklist a
> hash.  Wouldn't it make more sense to limit blacklisting
> certificates/hashes to at least CA keys? 

I think a bigger issue is that if a kernel is updated with this patch
it will change the behavior. It is nothing to do whether the "old" or
"new" is better but more like kind of backwards compatibility issue.

BR, Jarkko


Re: [PATCH] certs: Restrict blacklist updates to the secondary trusted keyring

2023-09-11 Thread Eric Snowberg


> On Sep 11, 2023, at 5:08 PM, Mimi Zohar  wrote:
> 
> On Mon, 2023-09-11 at 22:17 +, Eric Snowberg wrote:
>> 
>>> On Sep 11, 2023, at 10:51 AM, Mickaël Salaün  wrote:
>>> 
>>> On Mon, Sep 11, 2023 at 09:29:07AM -0400, Mimi Zohar wrote:
 Hi Eric,
 
 On Fri, 2023-09-08 at 17:34 -0400, Eric Snowberg wrote:
> Currently root can dynamically update the blacklist keyring if the hash
> being added is signed and vouched for by the builtin trusted keyring.
> Currently keys in the secondary trusted keyring can not be used.
> 
> Keys within the secondary trusted keyring carry the same capabilities as
> the builtin trusted keyring.  Relax the current restriction for updating
> the .blacklist keyring and allow the secondary to also be referenced as
> a trust source.  Since the machine keyring is linked to the secondary
> trusted keyring, any key within it may also be used.
> 
> An example use case for this is IMA appraisal.  Now that IMA both
> references the blacklist keyring and allows the machine owner to add
> custom IMA CA certs via the machine keyring, this adds the additional
> capability for the machine owner to also do revocations on a running
> system.
> 
> IMA appraisal usage example to add a revocation for /usr/foo:
> 
> sha256sum /bin/foo | awk '{printf "bin:" $1}' > hash.txt
> 
> openssl smime -sign -in hash.txt -inkey machine-private-key.pem \
>  -signer machine-certificate.pem -noattr -binary -outform DER \
>  -out hash.p7s
> 
> keyctl padd blacklist "$(< hash.txt)" %:.blacklist < hash.p7s
> 
> Signed-off-by: Eric Snowberg 
 
 The secondary keyring may include both CA and code signing keys.  With
 this change any key loaded onto the secondary keyring may blacklist a
 hash.  Wouldn't it make more sense to limit blacklisting
 certificates/hashes to at least CA keys? 
>>> 
>>> Some operational constraints may limit what a CA can sign.
>> 
>> Agreed.  
>> 
>> Is there precedents for requiring this S/MIME to be signed by a CA? 
>> 
>>> This change is critical and should be tied to a dedicated kernel config
>>> (disabled by default), otherwise existing systems using this feature
>>> will have their threat model automatically changed without notice.
>> 
>> Today we have INTEGRITY_CA_MACHINE_KEYRING_MAX.  This can 
>> be enabled to enforce CA restrictions on the machine keyring.  Mimi, would 
>> this be a suitable solution for what you are after?
> 
> There needs to be some correlation between the file hashes being added
> to the blacklist and the certificate that signed them.  Without that
> correlation, any key on the secondary trusted keyring could add any
> file hashes it wants to the blacklist.

Today any key in the secondary trusted keyring can be used to validate a 
signed kernel module.  At a later time, if a new hash is added to the blacklist 
keyring to revoke loading a signed kernel module,  the ability to do the 
revocation with this additional change would be more restrictive than loading 
the original module.

But, if you think it would be appropriate, I could add a new Kconfig (disabled 
by default) that validates the key being used to vouch the S/MIME encoded 
hash is a CA.  That would certainly make this more complicated.   With this 
addition, would  the key usage field need to be referenced too?

Another idea I had was changing this patch to reference only the builtin and 
the machine keyring (if configured), not the secondary keyring.   Then with
INTEGRITY_CA_MACHINE_KEYRING_MAX, only CA keys could be 
used. Let me know your thoughts on this approach.  Thanks.



Re: [PATCH] certs: Restrict blacklist updates to the secondary trusted keyring

2023-09-11 Thread Mimi Zohar
On Mon, 2023-09-11 at 22:17 +, Eric Snowberg wrote:
> 
> > On Sep 11, 2023, at 10:51 AM, Mickaël Salaün  wrote:
> > 
> > On Mon, Sep 11, 2023 at 09:29:07AM -0400, Mimi Zohar wrote:
> >> Hi Eric,
> >> 
> >> On Fri, 2023-09-08 at 17:34 -0400, Eric Snowberg wrote:
> >>> Currently root can dynamically update the blacklist keyring if the hash
> >>> being added is signed and vouched for by the builtin trusted keyring.
> >>> Currently keys in the secondary trusted keyring can not be used.
> >>> 
> >>> Keys within the secondary trusted keyring carry the same capabilities as
> >>> the builtin trusted keyring.  Relax the current restriction for updating
> >>> the .blacklist keyring and allow the secondary to also be referenced as
> >>> a trust source.  Since the machine keyring is linked to the secondary
> >>> trusted keyring, any key within it may also be used.
> >>> 
> >>> An example use case for this is IMA appraisal.  Now that IMA both
> >>> references the blacklist keyring and allows the machine owner to add
> >>> custom IMA CA certs via the machine keyring, this adds the additional
> >>> capability for the machine owner to also do revocations on a running
> >>> system.
> >>> 
> >>> IMA appraisal usage example to add a revocation for /usr/foo:
> >>> 
> >>> sha256sum /bin/foo | awk '{printf "bin:" $1}' > hash.txt
> >>> 
> >>> openssl smime -sign -in hash.txt -inkey machine-private-key.pem \
> >>>   -signer machine-certificate.pem -noattr -binary -outform DER \
> >>>   -out hash.p7s
> >>> 
> >>> keyctl padd blacklist "$(< hash.txt)" %:.blacklist < hash.p7s
> >>> 
> >>> Signed-off-by: Eric Snowberg 
> >> 
> >> The secondary keyring may include both CA and code signing keys.  With
> >> this change any key loaded onto the secondary keyring may blacklist a
> >> hash.  Wouldn't it make more sense to limit blacklisting
> >> certificates/hashes to at least CA keys? 
> > 
> > Some operational constraints may limit what a CA can sign.
> 
> Agreed.  
> 
> Is there precedents for requiring this S/MIME to be signed by a CA? 
> 
> > This change is critical and should be tied to a dedicated kernel config
> > (disabled by default), otherwise existing systems using this feature
> > will have their threat model automatically changed without notice.
> 
> Today we have INTEGRITY_CA_MACHINE_KEYRING_MAX.  This can 
> be enabled to enforce CA restrictions on the machine keyring.  Mimi, would 
> this be a suitable solution for what you are after?

There needs to be some correlation between the file hashes being added
to the blacklist and the certificate that signed them.  Without that
correlation, any key on the secondary trusted keyring could add any
file hashes it wants to the blacklist.

Mimi

> 
> I suppose root could add another key to the secondary keyring if it was 
> signed by a key in the machine keyring.  But then we are getting into an 
> area of key usage enforcement which really only exists for things added 
> to the .ima keyring.
> 
> >>> ---
> >>> certs/Kconfig | 2 +-
> >>> certs/blacklist.c | 4 ++--
> >>> 2 files changed, 3 insertions(+), 3 deletions(-)
> >>> 
> >>> diff --git a/certs/Kconfig b/certs/Kconfig
> >>> index 1f109b070877..23dc87c52aff 100644
> >>> --- a/certs/Kconfig
> >>> +++ b/certs/Kconfig
> >>> @@ -134,7 +134,7 @@ config SYSTEM_BLACKLIST_AUTH_UPDATE
> >>>   depends on SYSTEM_DATA_VERIFICATION
> >>>   help
> >>> If set, provide the ability to load new blacklist keys at run time if
> >>> -   they are signed and vouched by a certificate from the builtin trusted
> >>> +   they are signed and vouched by a certificate from the secondary 
> >>> trusted
> >> 
> >> If CONFIG_SECONDARY_TRUSTED_KEYRING is not enabled, it falls back to
> >> the builtin keyring.  Please update the comment accordingly.
> 
> I’ll fix these in the next round, thanks.
> 




Re: [PATCH] certs: Restrict blacklist updates to the secondary trusted keyring

2023-09-11 Thread Eric Snowberg



> On Sep 11, 2023, at 4:04 PM, Jarkko Sakkinen  wrote:
> 
> On Mon Sep 11, 2023 at 4:29 PM EEST, Mimi Zohar wrote:
>> Hi Eric,
>> 
>> On Fri, 2023-09-08 at 17:34 -0400, Eric Snowberg wrote:
>>> Currently root can dynamically update the blacklist keyring if the hash
>>> being added is signed and vouched for by the builtin trusted keyring.
>>> Currently keys in the secondary trusted keyring can not be used.
>>> 
>>> Keys within the secondary trusted keyring carry the same capabilities as
>>> the builtin trusted keyring.  Relax the current restriction for updating
>>> the .blacklist keyring and allow the secondary to also be referenced as
>>> a trust source.  Since the machine keyring is linked to the secondary
>>> trusted keyring, any key within it may also be used.
>>> 
>>> An example use case for this is IMA appraisal.  Now that IMA both
>>> references the blacklist keyring and allows the machine owner to add
>>> custom IMA CA certs via the machine keyring, this adds the additional
>>> capability for the machine owner to also do revocations on a running
>>> system.
>>> 
>>> IMA appraisal usage example to add a revocation for /usr/foo:
>>> 
>>> sha256sum /bin/foo | awk '{printf "bin:" $1}' > hash.txt
>>> 
>>> openssl smime -sign -in hash.txt -inkey machine-private-key.pem \
>>>   -signer machine-certificate.pem -noattr -binary -outform DER \
>>>   -out hash.p7s
>>> 
>>> keyctl padd blacklist "$(< hash.txt)" %:.blacklist < hash.p7s
>>> 
>>> Signed-off-by: Eric Snowberg 
>> 
>> The secondary keyring may include both CA and code signing keys.  With
>> this change any key loaded onto the secondary keyring may blacklist a
>> hash.  Wouldn't it make more sense to limit blacklisting
>> certificates/hashes to at least CA keys? 
> 
> I think a bigger issue is that if a kernel is updated with this patch
> it will change the behavior. It is nothing to do whether the "old" or
> "new" is better but more like kind of backwards compatibility issue.

For a kernel built without the secondary trusted keyring defined, there is 
no change to their security posture. For a kernel built with the secondary 
trusted keyring defined,  I would view the system as being more secure 
with this patch.   For any system using the secondary trusted keyring, 
root can add trusted keys.  However without this patch, root can not 
mitigate a security problem on a live system and do any type of revocation
for keys it owns.  Without the ability to do a revocation, we really only have 
authenticity, not integrity.



Re: [PATCH] certs: Restrict blacklist updates to the secondary trusted keyring

2023-09-11 Thread Eric Snowberg


> On Sep 11, 2023, at 10:51 AM, Mickaël Salaün  wrote:
> 
> On Mon, Sep 11, 2023 at 09:29:07AM -0400, Mimi Zohar wrote:
>> Hi Eric,
>> 
>> On Fri, 2023-09-08 at 17:34 -0400, Eric Snowberg wrote:
>>> Currently root can dynamically update the blacklist keyring if the hash
>>> being added is signed and vouched for by the builtin trusted keyring.
>>> Currently keys in the secondary trusted keyring can not be used.
>>> 
>>> Keys within the secondary trusted keyring carry the same capabilities as
>>> the builtin trusted keyring.  Relax the current restriction for updating
>>> the .blacklist keyring and allow the secondary to also be referenced as
>>> a trust source.  Since the machine keyring is linked to the secondary
>>> trusted keyring, any key within it may also be used.
>>> 
>>> An example use case for this is IMA appraisal.  Now that IMA both
>>> references the blacklist keyring and allows the machine owner to add
>>> custom IMA CA certs via the machine keyring, this adds the additional
>>> capability for the machine owner to also do revocations on a running
>>> system.
>>> 
>>> IMA appraisal usage example to add a revocation for /usr/foo:
>>> 
>>> sha256sum /bin/foo | awk '{printf "bin:" $1}' > hash.txt
>>> 
>>> openssl smime -sign -in hash.txt -inkey machine-private-key.pem \
>>>   -signer machine-certificate.pem -noattr -binary -outform DER \
>>>   -out hash.p7s
>>> 
>>> keyctl padd blacklist "$(< hash.txt)" %:.blacklist < hash.p7s
>>> 
>>> Signed-off-by: Eric Snowberg 
>> 
>> The secondary keyring may include both CA and code signing keys.  With
>> this change any key loaded onto the secondary keyring may blacklist a
>> hash.  Wouldn't it make more sense to limit blacklisting
>> certificates/hashes to at least CA keys? 
> 
> Some operational constraints may limit what a CA can sign.

Agreed.  

Is there precedents for requiring this S/MIME to be signed by a CA? 

> This change is critical and should be tied to a dedicated kernel config
> (disabled by default), otherwise existing systems using this feature
> will have their threat model automatically changed without notice.

Today we have INTEGRITY_CA_MACHINE_KEYRING_MAX.  This can 
be enabled to enforce CA restrictions on the machine keyring.  Mimi, would 
this be a suitable solution for what you are after?

I suppose root could add another key to the secondary keyring if it was 
signed by a key in the machine keyring.  But then we are getting into an 
area of key usage enforcement which really only exists for things added 
to the .ima keyring. 

>>> ---
>>> certs/Kconfig | 2 +-
>>> certs/blacklist.c | 4 ++--
>>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/certs/Kconfig b/certs/Kconfig
>>> index 1f109b070877..23dc87c52aff 100644
>>> --- a/certs/Kconfig
>>> +++ b/certs/Kconfig
>>> @@ -134,7 +134,7 @@ config SYSTEM_BLACKLIST_AUTH_UPDATE
>>> depends on SYSTEM_DATA_VERIFICATION
>>> help
>>>   If set, provide the ability to load new blacklist keys at run time if
>>> - they are signed and vouched by a certificate from the builtin trusted
>>> + they are signed and vouched by a certificate from the secondary 
>>> trusted
>> 
>> If CONFIG_SECONDARY_TRUSTED_KEYRING is not enabled, it falls back to
>> the builtin keyring.  Please update the comment accordingly.

I’ll fix these in the next round, thanks.



Re: [PATCH] certs: Restrict blacklist updates to the secondary trusted keyring

2023-09-11 Thread Mimi Zohar
Hi Eric,

On Fri, 2023-09-08 at 17:34 -0400, Eric Snowberg wrote:
> Currently root can dynamically update the blacklist keyring if the hash
> being added is signed and vouched for by the builtin trusted keyring.
> Currently keys in the secondary trusted keyring can not be used.
> 
> Keys within the secondary trusted keyring carry the same capabilities as
> the builtin trusted keyring.  Relax the current restriction for updating
> the .blacklist keyring and allow the secondary to also be referenced as
> a trust source.  Since the machine keyring is linked to the secondary
> trusted keyring, any key within it may also be used.
> 
> An example use case for this is IMA appraisal.  Now that IMA both
> references the blacklist keyring and allows the machine owner to add
> custom IMA CA certs via the machine keyring, this adds the additional
> capability for the machine owner to also do revocations on a running
> system.
> 
> IMA appraisal usage example to add a revocation for /usr/foo:
> 
> sha256sum /bin/foo | awk '{printf "bin:" $1}' > hash.txt
> 
> openssl smime -sign -in hash.txt -inkey machine-private-key.pem \
>-signer machine-certificate.pem -noattr -binary -outform DER \
>-out hash.p7s
> 
> keyctl padd blacklist "$(< hash.txt)" %:.blacklist < hash.p7s
> 
> Signed-off-by: Eric Snowberg 

The secondary keyring may include both CA and code signing keys.  With
this change any key loaded onto the secondary keyring may blacklist a
hash.  Wouldn't it make more sense to limit blacklisting
certificates/hashes to at least CA keys? 

> ---
>  certs/Kconfig | 2 +-
>  certs/blacklist.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/certs/Kconfig b/certs/Kconfig
> index 1f109b070877..23dc87c52aff 100644
> --- a/certs/Kconfig
> +++ b/certs/Kconfig
> @@ -134,7 +134,7 @@ config SYSTEM_BLACKLIST_AUTH_UPDATE
>   depends on SYSTEM_DATA_VERIFICATION
>   help
> If set, provide the ability to load new blacklist keys at run time if
> -   they are signed and vouched by a certificate from the builtin trusted
> +   they are signed and vouched by a certificate from the secondary 
> trusted

If CONFIG_SECONDARY_TRUSTED_KEYRING is not enabled, it falls back to
the builtin keyring.  Please update the comment accordingly.

> keyring.  The PKCS#7 signature of the description is set in the key
> payload.  Blacklist keys cannot be removed.
>  
> diff --git a/certs/blacklist.c b/certs/blacklist.c
> index 675dd7a8f07a..0b346048ae2d 100644
> --- a/certs/blacklist.c
> +++ b/certs/blacklist.c
> @@ -102,12 +102,12 @@ static int blacklist_key_instantiate(struct key *key,
>  
>  #ifdef CONFIG_SYSTEM_BLACKLIST_AUTH_UPDATE
>   /*
> -  * Verifies the description's PKCS#7 signature against the builtin
> +  * Verifies the description's PKCS#7 signature against the secondary
>* trusted keyring.
>*/

And similarly here ...

>   err = verify_pkcs7_signature(key->description,
>   strlen(key->description), prep->data, prep->datalen,
> - NULL, VERIFYING_UNSPECIFIED_SIGNATURE, NULL, NULL);
> + VERIFY_USE_SECONDARY_KEYRING, 
> VERIFYING_UNSPECIFIED_SIGNATURE, NULL, NULL);
>   if (err)
>   return err;
>  #else

-- 
thanks,

Mimi



Re: [PATCH] certs: Restrict blacklist updates to the secondary trusted keyring

2023-09-11 Thread Mickaël Salaün
On Mon, Sep 11, 2023 at 09:29:07AM -0400, Mimi Zohar wrote:
> Hi Eric,
> 
> On Fri, 2023-09-08 at 17:34 -0400, Eric Snowberg wrote:
> > Currently root can dynamically update the blacklist keyring if the hash
> > being added is signed and vouched for by the builtin trusted keyring.
> > Currently keys in the secondary trusted keyring can not be used.
> > 
> > Keys within the secondary trusted keyring carry the same capabilities as
> > the builtin trusted keyring.  Relax the current restriction for updating
> > the .blacklist keyring and allow the secondary to also be referenced as
> > a trust source.  Since the machine keyring is linked to the secondary
> > trusted keyring, any key within it may also be used.
> > 
> > An example use case for this is IMA appraisal.  Now that IMA both
> > references the blacklist keyring and allows the machine owner to add
> > custom IMA CA certs via the machine keyring, this adds the additional
> > capability for the machine owner to also do revocations on a running
> > system.
> > 
> > IMA appraisal usage example to add a revocation for /usr/foo:
> > 
> > sha256sum /bin/foo | awk '{printf "bin:" $1}' > hash.txt
> > 
> > openssl smime -sign -in hash.txt -inkey machine-private-key.pem \
> >-signer machine-certificate.pem -noattr -binary -outform DER \
> >-out hash.p7s
> > 
> > keyctl padd blacklist "$(< hash.txt)" %:.blacklist < hash.p7s
> > 
> > Signed-off-by: Eric Snowberg 
> 
> The secondary keyring may include both CA and code signing keys.  With
> this change any key loaded onto the secondary keyring may blacklist a
> hash.  Wouldn't it make more sense to limit blacklisting
> certificates/hashes to at least CA keys? 

Some operational constraints may limit what a CA can sign.

This change is critical and should be tied to a dedicated kernel config
(disabled by default), otherwise existing systems using this feature
will have their threat model automatically changed without notice.

> 
> > ---
> >  certs/Kconfig | 2 +-
> >  certs/blacklist.c | 4 ++--
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/certs/Kconfig b/certs/Kconfig
> > index 1f109b070877..23dc87c52aff 100644
> > --- a/certs/Kconfig
> > +++ b/certs/Kconfig
> > @@ -134,7 +134,7 @@ config SYSTEM_BLACKLIST_AUTH_UPDATE
> > depends on SYSTEM_DATA_VERIFICATION
> > help
> >   If set, provide the ability to load new blacklist keys at run time if
> > - they are signed and vouched by a certificate from the builtin trusted
> > + they are signed and vouched by a certificate from the secondary 
> > trusted
> 
> If CONFIG_SECONDARY_TRUSTED_KEYRING is not enabled, it falls back to
> the builtin keyring.  Please update the comment accordingly.
> 
> >   keyring.  The PKCS#7 signature of the description is set in the key
> >   payload.  Blacklist keys cannot be removed.
> >  
> > diff --git a/certs/blacklist.c b/certs/blacklist.c
> > index 675dd7a8f07a..0b346048ae2d 100644
> > --- a/certs/blacklist.c
> > +++ b/certs/blacklist.c
> > @@ -102,12 +102,12 @@ static int blacklist_key_instantiate(struct key *key,
> >  
> >  #ifdef CONFIG_SYSTEM_BLACKLIST_AUTH_UPDATE
> > /*
> > -* Verifies the description's PKCS#7 signature against the builtin
> > +* Verifies the description's PKCS#7 signature against the secondary
> >  * trusted keyring.
> >  */
> 
> And similarly here ...
> 
> > err = verify_pkcs7_signature(key->description,
> > strlen(key->description), prep->data, prep->datalen,
> > -   NULL, VERIFYING_UNSPECIFIED_SIGNATURE, NULL, NULL);
> > +   VERIFY_USE_SECONDARY_KEYRING, 
> > VERIFYING_UNSPECIFIED_SIGNATURE, NULL, NULL);
> > if (err)
> > return err;
> >  #else
> 
> -- 
> thanks,
> 
> Mimi
>