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

[Mimi Zohar]

On Tue, 2023-09-12 at 02:00 +0000, Eric Snowberg wrote:
> 
> > On Sep 11, 2023, at 5:08 PM, Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote:
> > 
> > On Mon, 2023-09-11 at 22:17 +0000, Eric Snowberg wrote:
> >> 
> >>> On Sep 11, 2023, at 10:51 AM, Mickaël Salaün <mic@xxxxxxxxxxx> 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 <eric.snowberg@xxxxxxxxxx>
> >>>> 
> >>>> 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