Re: [PATCH v4] certs: Add EFI_CERT_X509_GUID support for dbx entries

Re: [PATCH v4] certs: Add EFI_CERT_X509_GUID support for dbx entries

[Mimi Zohar]

[Cc'ing linux-integrity]

On Wed, 2021-01-27 at 11:46 +0000, David Howells wrote:
> Jarkko Sakkinen <jarkko@kernel.org> wrote:
> 
> > > I suppose a user space tool could be created. But wouldn’t what is
> > > currently done in the kernel in this area need to be removed?
> > 
> > Right. I don't think this was a great idea in the first place to
> > do to the kernel but since it exists, I guess the patch does make
> > sense.
> 
> This information needs to be loaded from the UEFI tables before the system
> starts loading any kernel modules or running any programs (if we do
> verification of such, which I think IMA can do).

There needs to a clear distinction between the pre-boot and post-boot
keys.  UEFI has its own trust model, which should be limited to UEFI. 
The .platform keyring was upstreamed and limited to verifying the kexec
kernel image.   Any other usage of the .platform keyring keys is
abusing its intended purpose.

The cover letter says,   "Anytime the .platform keyring is used, the
keys in the .blacklist keyring are referenced, if a matching key is
found, the key will be rejected."   I don't have a problem with loading
the UEFI X509 dbx entries as long as its usage is limited to verifying
the kexec kernel image.

Mimi

Re: [PATCH v4] certs: Add EFI_CERT_X509_GUID support for dbx entries

[Eric Snowberg]

> On Jan 27, 2021, at 7:03 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> [Cc'ing linux-integrity]
> 
> On Wed, 2021-01-27 at 11:46 +0000, David Howells wrote:
>> Jarkko Sakkinen <jarkko@kernel.org> wrote:
>> 
>>>> I suppose a user space tool could be created. But wouldn’t what is
>>>> currently done in the kernel in this area need to be removed?
>>> 
>>> Right. I don't think this was a great idea in the first place to
>>> do to the kernel but since it exists, I guess the patch does make
>>> sense.
>> 
>> This information needs to be loaded from the UEFI tables before the system
>> starts loading any kernel modules or running any programs (if we do
>> verification of such, which I think IMA can do).
> 
> There needs to a clear distinction between the pre-boot and post-boot
> keys.  UEFI has its own trust model, which should be limited to UEFI. 
> The .platform keyring was upstreamed and limited to verifying the kexec
> kernel image.   Any other usage of the .platform keyring keys is
> abusing its intended purpose.
> 
> The cover letter says,   "Anytime the .platform keyring is used, the
> keys in the .blacklist keyring are referenced, if a matching key is
> found, the key will be rejected."   I don't have a problem with loading
> the UEFI X509 dbx entries as long as its usage is limited to verifying
> the kexec kernel image.

Correct, with my patch, when EFI_CERT_X509_GUID entries are found in the
dbx, they will only be used during kexec.  I believe the latest dbx file on 
uefi.org contains three of these entires.

Based on my understanding of why the platform keyring was introduced, 
I intentionally only used these for kexec.  I do question the current 
upstream mainline code though.  Currently, when EFI_CERT_X509_SHA256_GUID
or EFI_CERT_SHA256_GUID entries are found in the dbx, they are applied 
everywhere.  It seems like there should be a dbx revocation keyring 
equivalent to the current platform keyring that is only used for pre-boot. 

If that is a direction you would like to see this go in the future, let
me know, I’d be happy to work on it.

Re: [PATCH v4] certs: Add EFI_CERT_X509_GUID support for dbx entries

[Nayna]

On 1/27/21 10:41 AM, Eric Snowberg wrote:
>> On Jan 27, 2021, at 7:03 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>
>> [Cc'ing linux-integrity]
>>
>> On Wed, 2021-01-27 at 11:46 +0000, David Howells wrote:
>>> Jarkko Sakkinen <jarkko@kernel.org> wrote:
>>>
>>>>> I suppose a user space tool could be created. But wouldn’t what is
>>>>> currently done in the kernel in this area need to be removed?
>>>> Right. I don't think this was a great idea in the first place to
>>>> do to the kernel but since it exists, I guess the patch does make
>>>> sense.
>>> This information needs to be loaded from the UEFI tables before the system
>>> starts loading any kernel modules or running any programs (if we do
>>> verification of such, which I think IMA can do).
>> There needs to a clear distinction between the pre-boot and post-boot
>> keys.  UEFI has its own trust model, which should be limited to UEFI.
>> The .platform keyring was upstreamed and limited to verifying the kexec
>> kernel image.   Any other usage of the .platform keyring keys is
>> abusing its intended purpose.
>>
>> The cover letter says,   "Anytime the .platform keyring is used, the
>> keys in the .blacklist keyring are referenced, if a matching key is
>> found, the key will be rejected."   I don't have a problem with loading
>> the UEFI X509 dbx entries as long as its usage is limited to verifying
>> the kexec kernel image.
> Correct, with my patch, when EFI_CERT_X509_GUID entries are found in the
> dbx, they will only be used during kexec.  I believe the latest dbx file on
> uefi.org contains three of these entires.
>
> Based on my understanding of why the platform keyring was introduced,
> I intentionally only used these for kexec.  I do question the current
> upstream mainline code though.  Currently, when EFI_CERT_X509_SHA256_GUID
> or EFI_CERT_SHA256_GUID entries are found in the dbx, they are applied
> everywhere.  It seems like there should be a dbx revocation keyring
> equivalent to the current platform keyring that is only used for pre-boot.
>
> If that is a direction you would like to see this go in the future, let
> me know, I’d be happy to work on it.
>
Yes, as you said, currently blacklist entries from dbx for 
EFI_CERT_X509_SHA256_GUID or EFI_CERT_SHA256_GUID are applied 
everywhere, and does not satisfy the trust model for .platform keyring. 
We should fix this, but changing now might break some existing systems. 
Probably it should be discussed as separate thread from this patchset.

Thanks & Regards,

       - Nayna

Re: [PATCH v4] certs: Add EFI_CERT_X509_GUID support for dbx entries

[Jarkko Sakkinen]

On Wed, Jan 27, 2021 at 09:03:59AM -0500, Mimi Zohar wrote:
> [Cc'ing linux-integrity]
> 
> On Wed, 2021-01-27 at 11:46 +0000, David Howells wrote:
> > Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > 
> > > > I suppose a user space tool could be created. But wouldn’t what is
> > > > currently done in the kernel in this area need to be removed?
> > > 
> > > Right. I don't think this was a great idea in the first place to
> > > do to the kernel but since it exists, I guess the patch does make
> > > sense.
> > 
> > This information needs to be loaded from the UEFI tables before the system
> > starts loading any kernel modules or running any programs (if we do
> > verification of such, which I think IMA can do).
> 
> There needs to a clear distinction between the pre-boot and post-boot
> keys.  UEFI has its own trust model, which should be limited to UEFI. 
> The .platform keyring was upstreamed and limited to verifying the kexec
> kernel image.   Any other usage of the .platform keyring keys is
> abusing its intended purpose.
> 
> The cover letter says,   "Anytime the .platform keyring is used, the
> keys in the .blacklist keyring are referenced, if a matching key is
> found, the key will be rejected."   I don't have a problem with loading
> the UEFI X509 dbx entries as long as its usage is limited to verifying
> the kexec kernel image.
> 
> Mimi

Thanks Mimi, this is a valid argument. I agree.

/Jarkko

Re: [PATCH v4] certs: Add EFI_CERT_X509_GUID support for dbx entries

[Jarkko Sakkinen]

On Wed, Jan 27, 2021 at 08:41:29AM -0700, Eric Snowberg wrote:
> 
> > On Jan 27, 2021, at 7:03 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > [Cc'ing linux-integrity]
> > 
> > On Wed, 2021-01-27 at 11:46 +0000, David Howells wrote:
> >> Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >> 
> >>>> I suppose a user space tool could be created. But wouldn’t what is
> >>>> currently done in the kernel in this area need to be removed?
> >>> 
> >>> Right. I don't think this was a great idea in the first place to
> >>> do to the kernel but since it exists, I guess the patch does make
> >>> sense.
> >> 
> >> This information needs to be loaded from the UEFI tables before the system
> >> starts loading any kernel modules or running any programs (if we do
> >> verification of such, which I think IMA can do).
> > 
> > There needs to a clear distinction between the pre-boot and post-boot
> > keys.  UEFI has its own trust model, which should be limited to UEFI. 
> > The .platform keyring was upstreamed and limited to verifying the kexec
> > kernel image.   Any other usage of the .platform keyring keys is
> > abusing its intended purpose.
> > 
> > The cover letter says,   "Anytime the .platform keyring is used, the
> > keys in the .blacklist keyring are referenced, if a matching key is
> > found, the key will be rejected."   I don't have a problem with loading
> > the UEFI X509 dbx entries as long as its usage is limited to verifying
> > the kexec kernel image.
> 
> Correct, with my patch, when EFI_CERT_X509_GUID entries are found in the
> dbx, they will only be used during kexec.  I believe the latest dbx file on 
> uefi.org contains three of these entires.
> 
> Based on my understanding of why the platform keyring was introduced, 
> I intentionally only used these for kexec.  I do question the current 
> upstream mainline code though.  Currently, when EFI_CERT_X509_SHA256_GUID
> or EFI_CERT_SHA256_GUID entries are found in the dbx, they are applied 
> everywhere.  It seems like there should be a dbx revocation keyring 
> equivalent to the current platform keyring that is only used for pre-boot. 
> 
> If that is a direction you would like to see this go in the future, let
> me know, I’d be happy to work on it.

I would tend to agree with this.

/Jarkko