[PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

[PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

[derek.lin2]

[-- Attachment #1.1: Type: text/plain, Size: 923 bytes --]

Patch is attached from group.io.
Since ECR785, which is added UEFI 2.3.1 errata A, enrolling a PK in setup mode doesn't need to verify the PK.
Below is the sentence about it in UEFI spec
```
3. If the firmware is in setup mode and the variable is one of:
- The global PK variable;
- The global KEK variable;
- The "db" variable with GUID EFI_IMAGE_SECURITY_DATABASE_GUID; or
- The "dbx" variable with GUID EFI_IMAGE_SECURITY_DATABASE_GUID,
then the firmware implementation shall consider the checks in the following steps 4 and 5 to
have passed, and proceed with updating the variable value as outlined below.
```
The step 4 is to verify the signature and the step 5 is to verify the cert.

After this change, when system is in Setup mode, setting a PK does not require authenticated variable descriptor.

Signed-off-by: Derek Lin <derek.lin2@hpe.com>
Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com>

[-- Attachment #1.2: Type: text/html, Size: 1164 bytes --]

[-- Attachment #2: 0001-SecurityPkg-Don-t-Verify-the-enrolled-PK-in-setup-mo.patch --]
[-- Type: application/octet-stream, Size: 2977 bytes --]

From 4333f078f3d06a9332bf7220a1112b482a1671fe Mon Sep 17 00:00:00 2001
From: Derek Lin <derek.lin2@hpe.com>
Date: Tue, 2 Jul 2019 11:00:51 +0800
Subject: [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

Since ECR785, which is added UEFI 2.3.1 errata A, enrolling a PK
in setup mode doesn't need to verify the PK.
Below is the sentence about it in UEFI spec
```
3. If the firmware is in setup mode and the variable is one of:
- The global PK variable;
- The global KEK variable;
- The "db" variable with GUID EFI_IMAGE_SECURITY_DATABASE_GUID; or
- The "dbx" variable with GUID EFI_IMAGE_SECURITY_DATABASE_GUID,
then the firmware implementation shall consider the checks in the following
steps 4 and 5 to have passed, and proceed with updating the variable value
as outlined below.
```
The step 4 is to verify the signature and the step 5 is to verify the cert.

After this change, when system is in Setup mode, setting a PK does not require
authenticated variable descriptor.

Signed-off-by: Derek Lin <derek.lin2@hpe.com>
Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com>
---
 .../Library/AuthVariableLib/AuthService.c      | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c b/SecurityPkg/Library/AuthVariableLib/AuthService.c
index 486df55bed..30347e2089 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -19,6 +19,7 @@
   to verify the signature.
 
 Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
+(C) Copyright 2019 Hewlett Packard Enterprise Development LP<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -583,7 +584,7 @@ ProcessVarWithPk (
   // Init state of Del. State may change due to secure check
   //
   Del = FALSE;
-  if ((InCustomMode() && UserPhysicalPresent()) || (mPlatformMode == SETUP_MODE && !IsPk)) {
+  if ((InCustomMode() && UserPhysicalPresent()) || (mPlatformMode == SETUP_MODE)) {
     Payload = (UINT8 *) Data + AUTHINFO2_SIZE (Data);
     PayloadSize = DataSize - AUTHINFO2_SIZE (Data);
     if (PayloadSize == 0) {
@@ -610,7 +611,7 @@ ProcessVarWithPk (
     if ((mPlatformMode != SETUP_MODE) || IsPk) {
       Status = VendorKeyIsModified ();
     }
-  } else if (mPlatformMode == USER_MODE) {
+  } else {
     //
     // Verify against X509 Cert in PK database.
     //
@@ -623,19 +624,6 @@ ProcessVarWithPk (
                AuthVarTypePk,
                &Del
                );
-  } else {
-    //
-    // Verify against the certificate in data payload.
-    //
-    Status = VerifyTimeBasedPayloadAndUpdate (
-               VariableName,
-               VendorGuid,
-               Data,
-               DataSize,
-               Attributes,
-               AuthVarTypePayload,
-               &Del
-               );
   }
 
   if (!EFI_ERROR(Status) && IsPk) {
-- 
2.20.1.windows.1

Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

[derek.lin2]

[-- Attachment #1.1: Type: text/plain, Size: 1276 bytes --]

Add SecurityPkg maintainers.

Thanks,
Derek

From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of derek.lin2@hpe.com
Sent: Tuesday, July 2, 2019 1:25 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

Patch is attached from group.io.
Since ECR785, which is added UEFI 2.3.1 errata A, enrolling a PK in setup mode doesn't need to verify the PK.
Below is the sentence about it in UEFI spec
```
3. If the firmware is in setup mode and the variable is one of:
- The global PK variable;
- The global KEK variable;
- The "db" variable with GUID EFI_IMAGE_SECURITY_DATABASE_GUID; or
- The "dbx" variable with GUID EFI_IMAGE_SECURITY_DATABASE_GUID,
then the firmware implementation shall consider the checks in the following steps 4 and 5 to
have passed, and proceed with updating the variable value as outlined below.
```
The step 4 is to verify the signature and the step 5 is to verify the cert.

After this change, when system is in Setup mode, setting a PK does not require authenticated variable descriptor.

Signed-off-by: Derek Lin <derek.lin2@hpe.com<mailto:derek.lin2@hpe.com>>
Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com<mailto:cinnamon.shia@hpe.com>>



[-- Attachment #1.2: Type: text/html, Size: 5421 bytes --]

[-- Attachment #2: 0001-SecurityPkg-Don-t-Verify-the-enrolled-PK-in-setup-mo.patch --]
[-- Type: application/octet-stream, Size: 2977 bytes --]

From 4333f078f3d06a9332bf7220a1112b482a1671fe Mon Sep 17 00:00:00 2001
From: Derek Lin <derek.lin2@hpe.com>
Date: Tue, 2 Jul 2019 11:00:51 +0800
Subject: [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

Since ECR785, which is added UEFI 2.3.1 errata A, enrolling a PK
in setup mode doesn't need to verify the PK.
Below is the sentence about it in UEFI spec
```
3. If the firmware is in setup mode and the variable is one of:
- The global PK variable;
- The global KEK variable;
- The "db" variable with GUID EFI_IMAGE_SECURITY_DATABASE_GUID; or
- The "dbx" variable with GUID EFI_IMAGE_SECURITY_DATABASE_GUID,
then the firmware implementation shall consider the checks in the following
steps 4 and 5 to have passed, and proceed with updating the variable value
as outlined below.
```
The step 4 is to verify the signature and the step 5 is to verify the cert.

After this change, when system is in Setup mode, setting a PK does not require
authenticated variable descriptor.

Signed-off-by: Derek Lin <derek.lin2@hpe.com>
Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com>
---
 .../Library/AuthVariableLib/AuthService.c      | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c b/SecurityPkg/Library/AuthVariableLib/AuthService.c
index 486df55bed..30347e2089 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -19,6 +19,7 @@
   to verify the signature.
 
 Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
+(C) Copyright 2019 Hewlett Packard Enterprise Development LP<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -583,7 +584,7 @@ ProcessVarWithPk (
   // Init state of Del. State may change due to secure check
   //
   Del = FALSE;
-  if ((InCustomMode() && UserPhysicalPresent()) || (mPlatformMode == SETUP_MODE && !IsPk)) {
+  if ((InCustomMode() && UserPhysicalPresent()) || (mPlatformMode == SETUP_MODE)) {
     Payload = (UINT8 *) Data + AUTHINFO2_SIZE (Data);
     PayloadSize = DataSize - AUTHINFO2_SIZE (Data);
     if (PayloadSize == 0) {
@@ -610,7 +611,7 @@ ProcessVarWithPk (
     if ((mPlatformMode != SETUP_MODE) || IsPk) {
       Status = VendorKeyIsModified ();
     }
-  } else if (mPlatformMode == USER_MODE) {
+  } else {
     //
     // Verify against X509 Cert in PK database.
     //
@@ -623,19 +624,6 @@ ProcessVarWithPk (
                AuthVarTypePk,
                &Del
                );
-  } else {
-    //
-    // Verify against the certificate in data payload.
-    //
-    Status = VerifyTimeBasedPayloadAndUpdate (
-               VariableName,
-               VendorGuid,
-               Data,
-               DataSize,
-               Attributes,
-               AuthVarTypePayload,
-               &Del
-               );
   }
 
   if (!EFI_ERROR(Status) && IsPk) {
-- 
2.20.1.windows.1

Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

[Zhang, Chao B]

[-- Attachment #1: Type: text/plain, Size: 1322 bytes --]

Hi Derek:
   The patch is good to me.
   Reviewed-by : Chao Zhang <chao.b.zhang@intel.com>

From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of derek.lin2@hpe.com
Sent: Tuesday, July 2, 2019 1:25 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

Patch is attached from group.io.
Since ECR785, which is added UEFI 2.3.1 errata A, enrolling a PK in setup mode doesn't need to verify the PK.
Below is the sentence about it in UEFI spec
```
3. If the firmware is in setup mode and the variable is one of:
- The global PK variable;
- The global KEK variable;
- The "db" variable with GUID EFI_IMAGE_SECURITY_DATABASE_GUID; or
- The "dbx" variable with GUID EFI_IMAGE_SECURITY_DATABASE_GUID,
then the firmware implementation shall consider the checks in the following steps 4 and 5 to
have passed, and proceed with updating the variable value as outlined below.
```
The step 4 is to verify the signature and the step 5 is to verify the cert.

After this change, when system is in Setup mode, setting a PK does not require authenticated variable descriptor.

Signed-off-by: Derek Lin <derek.lin2@hpe.com<mailto:derek.lin2@hpe.com>>
Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com<mailto:cinnamon.shia@hpe.com>>



[-- Attachment #2: Type: text/html, Size: 5438 bytes --]

Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

[Wang, Jian J]

[-- Attachment #1: Type: text/plain, Size: 1874 bytes --]

Hi Derek,

Please file a Bugzilla for this issue. With it addressed,

    Reviewed-by: Jian J Wang jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>

Thanks,
Jian
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Zhang, Chao B
Sent: Tuesday, July 09, 2019 11:39 PM
To: devel@edk2.groups.io; derek.lin2@hpe.com
Subject: Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

Hi Derek:
   The patch is good to me.
   Reviewed-by : Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of derek.lin2@hpe.com<mailto:derek.lin2@hpe.com>
Sent: Tuesday, July 2, 2019 1:25 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

Patch is attached from group.io.
Since ECR785, which is added UEFI 2.3.1 errata A, enrolling a PK in setup mode doesn't need to verify the PK.
Below is the sentence about it in UEFI spec
```
3. If the firmware is in setup mode and the variable is one of:
- The global PK variable;
- The global KEK variable;
- The "db" variable with GUID EFI_IMAGE_SECURITY_DATABASE_GUID; or
- The "dbx" variable with GUID EFI_IMAGE_SECURITY_DATABASE_GUID,
then the firmware implementation shall consider the checks in the following steps 4 and 5 to
have passed, and proceed with updating the variable value as outlined below.
```
The step 4 is to verify the signature and the step 5 is to verify the cert.

After this change, when system is in Setup mode, setting a PK does not require authenticated variable descriptor.

Signed-off-by: Derek Lin <derek.lin2@hpe.com<mailto:derek.lin2@hpe.com>>
Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com<mailto:cinnamon.shia@hpe.com>>



[-- Attachment #2: Type: text/html, Size: 7844 bytes --]

Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

[Laszlo Ersek]

Hi,

On 07/10/19 10:50, Wang, Jian J wrote:
> Hi Derek,
> 
> Please file a Bugzilla for this issue. With it addressed,
> 
>     Reviewed-by: Jian J Wang jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>

I saw this patch as soon as it was posted, and I've been hoping for a
deeper discussion on the list. (I didn't ask my questions immediately
because I felt they might have been somewhat naive.) So I guess I have
to ask them now :)


(1) What is the exact failure (or spec non-conformance) scenario?

Is it the problem that, currently, even if SetupMode is 1, CustomMode
also needs to be set, for enrolling a self-signed PK?

(Looking at the patch itself, it looks like the subcondition that is
*not* the CustomMode check is relaxed; so that seems to support my
question.)


(2) Can we please confirm that the code will continue to enforce
self-signedness?

I checked the spec, and I'm a bit worried about the spec language
proper. Page 246 in UEFI-2.8 says,

  3. If the variable SetupMode==1, and the variable is a secure boot
     policy variable, then the firmware implementation shall consider
     the checks in the following steps 4 and 5 to have passed, and
     proceed with updating the variable value as outlined below.

  4. Verify the signature by:
     — extracting the EFI_VARIABLE_AUTHENTICATION_2 descriptor from the
       Data buffer;
     — using the descriptor contents and other parameters to (a)
       construct the input to the digest algorithm; (b) computing the
       digest; and (c) comparing the digest with the result of applying
       the signer’s public key to the signature.

In other words, step#4 seems to stand for verifying self-signedness, and
step#3 appears to require that *not even* self-signedness be enforced,
when in SetupMode.

Honestly, I think that step#4 should never be skipped. In other words,
self-signedness should be unconditional.

A certificate is basically a signed statement that a particular public
key belongs to a particular entity (such as "FooBar"). If *not even*
FooBar signs that statement, then the statement has *zero* credibility.
So why should we accept it for any purpose at all?

Speaking in terms of "PK" (UEFI Platform Key), specifically: what good
would a platform vendor be that failed to sign its own PK?

So, I'd like to understand:

(2a) whether skipping step#4 in SetupMode is a bug in the spec --
because, I think it is;

(2b) whether the edk2 code would continue enforcing self-signedness on
X509 certificates, if the proposed patch were applied.

Thanks!
Laszlo

> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Zhang, Chao B
> Sent: Tuesday, July 09, 2019 11:39 PM
> To: devel@edk2.groups.io; derek.lin2@hpe.com
> Subject: Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode
> 
> Hi Derek:
>    The patch is good to me.
>    Reviewed-by : Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
> 
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of derek.lin2@hpe.com<mailto:derek.lin2@hpe.com>
> Sent: Tuesday, July 2, 2019 1:25 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Subject: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode
> 
> Patch is attached from group.io.
> Since ECR785, which is added UEFI 2.3.1 errata A, enrolling a PK in setup mode doesn't need to verify the PK.
> Below is the sentence about it in UEFI spec
> ```
> 3. If the firmware is in setup mode and the variable is one of:
> - The global PK variable;
> - The global KEK variable;
> - The "db" variable with GUID EFI_IMAGE_SECURITY_DATABASE_GUID; or
> - The "dbx" variable with GUID EFI_IMAGE_SECURITY_DATABASE_GUID,
> then the firmware implementation shall consider the checks in the following steps 4 and 5 to
> have passed, and proceed with updating the variable value as outlined below.
> ```
> The step 4 is to verify the signature and the step 5 is to verify the cert.
> 
> After this change, when system is in Setup mode, setting a PK does not require authenticated variable descriptor.
> 
> Signed-off-by: Derek Lin <derek.lin2@hpe.com<mailto:derek.lin2@hpe.com>>
> Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com<mailto:cinnamon.shia@hpe.com>>
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

[Zhang, Chao B]

[-- Attachment #1: Type: text/plain, Size: 6787 bytes --]

HI Laszlo:
   There is a discussion over this issue in UEFI Manits https://mantis.uefi.org/mantis/view.php?id=1983
The justification lies here:
  Spec perspective:
    Section 8.2.2  : In SetupMode Secure Boot Policy variables shall consider step 3 and 4 check to be successful.
    Section 32.3.1 : If in the platform is in stepup mode, then the new PKpub may be signed with PKpriv
Customer needs:
     1) PK update process is complex based on current implementation – self signed PK is required. 2 PK images are required
- new PK signed with the old PKprivate, to be used if system is in user mode
- new self-sighed PK (new PK signed with new PKprivate) to be used if system is in setup mode
   2) PKpub Default can be easily updated to PK

Back to Laszlo’s question
  (1) What is the exact failure (or spec non-conformance) scenario?
     A:    Please see above explanation
     (2a) whether skipping step#4 in SetupMode is a bug in the spec -- because, I think it is;
     A:   Please see customer needs part in above explanation
     (2b) whether the edk2 code would continue enforcing self-signedness on
          X509 certificates, if the proposed patch were applied.
      A:  After this patch, there is no self-encrypted PK check. Per discussion, we believe that is not a new security hole
         Even with self-proofed PK check, attacker can easily spoof a faked PK attack in setup mode.   Generally speaking,
         PK provision should happen in Build phase or with Physical Presence Asserted.


From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Thursday, July 11, 2019 1:04 AM
To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>; Derek Lin <derek.lin2@hpe.com>; Cinnamon Shia <cinnamon.shia@hpe.com>
Subject: Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

Hi,

On 07/10/19 10:50, Wang, Jian J wrote:
> Hi Derek,
>
> Please file a Bugzilla for this issue. With it addressed,
>
>     Reviewed-by: Jian J Wang jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang@intel.com%3cmailto:jian.j.wang@intel.com>>

I saw this patch as soon as it was posted, and I've been hoping for a
deeper discussion on the list. (I didn't ask my questions immediately
because I felt they might have been somewhat naive.) So I guess I have
to ask them now :)


(1) What is the exact failure (or spec non-conformance) scenario?

Is it the problem that, currently, even if SetupMode is 1, CustomMode
also needs to be set, for enrolling a self-signed PK?

(Looking at the patch itself, it looks like the subcondition that is
*not* the CustomMode check is relaxed; so that seems to support my
question.)


(2) Can we please confirm that the code will continue to enforce
self-signedness?

I checked the spec, and I'm a bit worried about the spec language
proper. Page 246 in UEFI-2.8 says,

  3. If the variable SetupMode==1, and the variable is a secure boot
     policy variable, then the firmware implementation shall consider
     the checks in the following steps 4 and 5 to have passed, and
     proceed with updating the variable value as outlined below.

  4. Verify the signature by:
     — extracting the EFI_VARIABLE_AUTHENTICATION_2 descriptor from the
       Data buffer;
     — using the descriptor contents and other parameters to (a)
       construct the input to the digest algorithm; (b) computing the
       digest; and (c) comparing the digest with the result of applying
       the signer’s public key to the signature.

In other words, step#4 seems to stand for verifying self-signedness, and
step#3 appears to require that *not even* self-signedness be enforced,
when in SetupMode.

Honestly, I think that step#4 should never be skipped. In other words,
self-signedness should be unconditional.

A certificate is basically a signed statement that a particular public
key belongs to a particular entity (such as "FooBar"). If *not even*
FooBar signs that statement, then the statement has *zero* credibility.
So why should we accept it for any purpose at all?

Speaking in terms of "PK" (UEFI Platform Key), specifically: what good
would a platform vendor be that failed to sign its own PK?

So, I'd like to understand:

(2a) whether skipping step#4 in SetupMode is a bug in the spec --
because, I think it is;

(2b) whether the edk2 code would continue enforcing self-signedness on
X509 certificates, if the proposed patch were applied.

Thanks!
Laszlo

> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of Zhang, Chao B
> Sent: Tuesday, July 09, 2019 11:39 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; derek.lin2@hpe.com<mailto:derek.lin2@hpe.com>
> Subject: Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode
>
> Hi Derek:
>    The patch is good to me.
>    Reviewed-by : Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com%3cmailto:chao.b.zhang@intel.com>>>
>
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>> [mailto:devel@edk2.groups.io] On Behalf Of derek.lin2@hpe.com<mailto:derek.lin2@hpe.com<mailto:derek.lin2@hpe.com%3cmailto:derek.lin2@hpe.com>>
> Sent: Tuesday, July 2, 2019 1:25 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>
> Subject: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode
>
> Patch is attached from group.io.
> Since ECR785, which is added UEFI 2.3.1 errata A, enrolling a PK in setup mode doesn't need to verify the PK.
> Below is the sentence about it in UEFI spec
> ```
> 3. If the firmware is in setup mode and the variable is one of:
> - The global PK variable;
> - The global KEK variable;
> - The "db" variable with GUID EFI_IMAGE_SECURITY_DATABASE_GUID; or
> - The "dbx" variable with GUID EFI_IMAGE_SECURITY_DATABASE_GUID,
> then the firmware implementation shall consider the checks in the following steps 4 and 5 to
> have passed, and proceed with updating the variable value as outlined below.
> ```
> The step 4 is to verify the signature and the step 5 is to verify the cert.
>
> After this change, when system is in Setup mode, setting a PK does not require authenticated variable descriptor.
>
> Signed-off-by: Derek Lin <derek.lin2@hpe.com<mailto:derek.lin2@hpe.com<mailto:derek.lin2@hpe.com%3cmailto:derek.lin2@hpe.com>>>
> Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com<mailto:cinnamon.shia@hpe.com<mailto:cinnamon.shia@hpe.com%3cmailto:cinnamon.shia@hpe.com>>>
>
>
>
> 
>

[-- Attachment #2: Type: text/html, Size: 20338 bytes --]

Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

[Laszlo Ersek]

On 07/11/19 05:20, Zhang, Chao B wrote:
> HI Laszlo:
>    There is a discussion over this issue in UEFI Manits https://mantis.uefi.org/mantis/view.php?id=1983
> The justification lies here:
>   Spec perspective:
>     Section 8.2.2  : In SetupMode Secure Boot Policy variables shall consider step 3 and 4 check to be successful.
>     Section 32.3.1 : If in the platform is in stepup mode, then the new PKpub may be signed with PKpriv
> Customer needs:
>      1) PK update process is complex based on current implementation – self signed PK is required. 2 PK images are required
> - new PK signed with the old PKprivate, to be used if system is in user mode
> - new self-sighed PK (new PK signed with new PKprivate) to be used if system is in setup mode
>    2) PKpub Default can be easily updated to PK
> 
> Back to Laszlo’s question
>   (1) What is the exact failure (or spec non-conformance) scenario?
>      A:    Please see above explanation
>      (2a) whether skipping step#4 in SetupMode is a bug in the spec -- because, I think it is;
>      A:   Please see customer needs part in above explanation
>      (2b) whether the edk2 code would continue enforcing self-signedness on
>           X509 certificates, if the proposed patch were applied.
>       A:  After this patch, there is no self-encrypted PK check. Per discussion, we believe that is not a new security hole
>          Even with self-proofed PK check, attacker can easily spoof a faked PK attack in setup mode.   Generally speaking,
>          PK provision should happen in Build phase or with Physical Presence Asserted.

Thank you for the explanation.

The discussion in Mantis#1983 makes the situation clear.

In Mantis#1983, it was requested that self-signedness be strictly
enforced when a PK is enrolled in Setup Mode. That matches my opinion fully.

Mantis#1983 was ultimately revoked because "it would complicate the PK
update process". Namely, platform vendors would have to provide
different blobs, for enrollment in setup mode (self-signed) vs.
enrollment in user mode (signed with the currently in-place PK's private
half).

Well, I think that said process would be exactly right. The
counter-argument:

  it would complicate the PK update process

is poor, in my opinion, when it comes to the root of trust in Secure
Boot. There is no failure scenario in fact, it's just that vendors would
have to provide multiple update images, and users (or the update tools)
would have to download the image that would be accepted by the current
state. In my opinion, that should be fine.

Multiple PK update images cannot be avoided anyway, if a platform vendor
releases at least two PK updates, over time. (So that we have the
initial PK0, and two updates, PK1, PK2.) When PK2 is released, it must
be made available to systems running in User Mode
- both as signed with PK0,
- and as signed with PK1,
anyway.

So I don't see why it's a burden to release PK2 as signed with PK2 --
i.e., with itself -- as well.

Mantis#1983 also highlights that it should always be possible to revert
PK to PKDefault. IMO that should not be a problem if PKDefault is
self-signed, and the physically present user switches the SB mode to
Setup Mode first. In that case, PKDefault can be re-enrolled -- in the
firmware setup utility, at boot time only -- like any other brand new
self-signed PK.

The point of requiring a self-signed certificate, and not just a public
key, is *not* to increase security. The point is to perform a sanity
check. If the certificate is correctly self-signed, it proves that,
whoever generated the certificate, had access to the counterpart private
key at least at that time. Conversely, if it's just a standalone pubkey,
or the signature on the cert is from some other entity, then the
end-user has no evidence that a matching private key was *ever* saved by
the creator. Again, the point is not to prevent spoofing, but to
sanity-check that the creator of the pubkey was capable of signing *at
all* with the matching private key.

And so I disagree with the rejection / revocation of Mantis#1983.

Regarding the edk2 patch:

If the patch indeed matches the UEFI-2.8 spec, then I don't have a valid
case against the proposed patch.

(Again, I think it's the spec that is problematic, and that Mantis#1983
had merit.)

However, can we please introduce a FeaturePCD for sticking with the
present in-tree behavior?

I'm OK if the default value for the new FeaturePCD is such that the new
(proposed, spec-conformant) behavior becomes the default. I'd just like
if platforms could opt out (i.e. if they could enforce self-signedness
in SetupMode).

Thanks
Laszlo

Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

[Zhang, Chao B]

[-- Attachment #1: Type: text/plain, Size: 5430 bytes --]

Hi Laszlo:
   Your investigation makes sense to me. For Keep old logic as an option will be a mild evolution.
HI Derek,
  Would you please update your patch based on Laszlo’s comments? Please don’t forget to CC Laszlo.

From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek
Sent: Thursday, July 11, 2019 7:47 PM
To: Zhang, Chao B <chao.b.zhang@intel.com>; devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Derek Lin <derek.lin2@hpe.com>; Cinnamon Shia <cinnamon.shia@hpe.com>; Felix Poludov <Felixp@ami.com>; Peter Jones <pjones@redhat.com>
Subject: Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

On 07/11/19 05:20, Zhang, Chao B wrote:
> HI Laszlo:
>    There is a discussion over this issue in UEFI Manits https://mantis.uefi.org/mantis/view.php?id=1983
> The justification lies here:
>   Spec perspective:
>     Section 8.2.2  : In SetupMode Secure Boot Policy variables shall consider step 3 and 4 check to be successful.
>     Section 32.3.1 : If in the platform is in stepup mode, then the new PKpub may be signed with PKpriv
> Customer needs:
>      1) PK update process is complex based on current implementation – self signed PK is required. 2 PK images are required
> - new PK signed with the old PKprivate, to be used if system is in user mode
> - new self-sighed PK (new PK signed with new PKprivate) to be used if system is in setup mode
>    2) PKpub Default can be easily updated to PK
>
> Back to Laszlo’s question
>   (1) What is the exact failure (or spec non-conformance) scenario?
>      A:    Please see above explanation
>      (2a) whether skipping step#4 in SetupMode is a bug in the spec -- because, I think it is;
>      A:   Please see customer needs part in above explanation
>      (2b) whether the edk2 code would continue enforcing self-signedness on
>           X509 certificates, if the proposed patch were applied.
>       A:  After this patch, there is no self-encrypted PK check. Per discussion, we believe that is not a new security hole
>          Even with self-proofed PK check, attacker can easily spoof a faked PK attack in setup mode.   Generally speaking,
>          PK provision should happen in Build phase or with Physical Presence Asserted.

Thank you for the explanation.

The discussion in Mantis#1983 makes the situation clear.

In Mantis#1983, it was requested that self-signedness be strictly
enforced when a PK is enrolled in Setup Mode. That matches my opinion fully.

Mantis#1983 was ultimately revoked because "it would complicate the PK
update process". Namely, platform vendors would have to provide
different blobs, for enrollment in setup mode (self-signed) vs.
enrollment in user mode (signed with the currently in-place PK's private
half).

Well, I think that said process would be exactly right. The
counter-argument:

  it would complicate the PK update process

is poor, in my opinion, when it comes to the root of trust in Secure
Boot. There is no failure scenario in fact, it's just that vendors would
have to provide multiple update images, and users (or the update tools)
would have to download the image that would be accepted by the current
state. In my opinion, that should be fine.

Multiple PK update images cannot be avoided anyway, if a platform vendor
releases at least two PK updates, over time. (So that we have the
initial PK0, and two updates, PK1, PK2.) When PK2 is released, it must
be made available to systems running in User Mode
- both as signed with PK0,
- and as signed with PK1,
anyway.

So I don't see why it's a burden to release PK2 as signed with PK2 --
i.e., with itself -- as well.

Mantis#1983 also highlights that it should always be possible to revert
PK to PKDefault. IMO that should not be a problem if PKDefault is
self-signed, and the physically present user switches the SB mode to
Setup Mode first. In that case, PKDefault can be re-enrolled -- in the
firmware setup utility, at boot time only -- like any other brand new
self-signed PK.

The point of requiring a self-signed certificate, and not just a public
key, is *not* to increase security. The point is to perform a sanity
check. If the certificate is correctly self-signed, it proves that,
whoever generated the certificate, had access to the counterpart private
key at least at that time. Conversely, if it's just a standalone pubkey,
or the signature on the cert is from some other entity, then the
end-user has no evidence that a matching private key was *ever* saved by
the creator. Again, the point is not to prevent spoofing, but to
sanity-check that the creator of the pubkey was capable of signing *at
all* with the matching private key.

And so I disagree with the rejection / revocation of Mantis#1983.

Regarding the edk2 patch:

If the patch indeed matches the UEFI-2.8 spec, then I don't have a valid
case against the proposed patch.

(Again, I think it's the spec that is problematic, and that Mantis#1983
had merit.)

However, can we please introduce a FeaturePCD for sticking with the
present in-tree behavior?

I'm OK if the default value for the new FeaturePCD is such that the new
(proposed, spec-conformant) behavior becomes the default. I'd just like
if platforms could opt out (i.e. if they could enforce self-signedness
in SetupMode).

Thanks
Laszlo



[-- Attachment #2: Type: text/html, Size: 15689 bytes --]

Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

[Lin, Derek (HPS SW)]

[-- Attachment #1: Type: text/plain, Size: 570 bytes --]

Hi Laszlo, Chao,

Sorry for late response in this thread.

I review Mantis#1983 and this discussion again. I agree with Laszlo.
1. UEFI spec 2.8 is not very clear about PK validation in Setup mode.
2. This patch only reduce the complexity of update PK process.

Having a FeaturePCD to control this kind of behavior in EDK2 is weird. That only make things more complicated to me.
To simplify and make things clear, updating PK shall always be signed in both Setup Mode and User Mode.

Anyway, I agree with Laszlo and I'm good with current implementation now.

[-- Attachment #2: Type: text/html, Size: 635 bytes --]