* [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode @ 2019-07-02 5:25 derek.lin2 2019-07-04 6:29 ` [edk2-devel] " derek.lin2 2019-07-09 15:39 ` Zhang, Chao B 0 siblings, 2 replies; 9+ messages in thread From: derek.lin2 @ 2019-07-02 5:25 UTC (permalink / raw) To: devel [-- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode 2019-07-02 5:25 [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode derek.lin2 @ 2019-07-04 6:29 ` derek.lin2 2019-07-09 15:39 ` Zhang, Chao B 1 sibling, 0 replies; 9+ messages in thread From: derek.lin2 @ 2019-07-04 6:29 UTC (permalink / raw) To: devel@edk2.groups.io, Lin, Derek (HPS SW), chao.b.zhang@intel.com, jiewen.yao@intel.com, jian.j.wang@intel.com [-- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode 2019-07-02 5:25 [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode derek.lin2 2019-07-04 6:29 ` [edk2-devel] " derek.lin2 @ 2019-07-09 15:39 ` Zhang, Chao B 2019-07-10 8:50 ` Wang, Jian J 1 sibling, 1 reply; 9+ messages in thread From: Zhang, Chao B @ 2019-07-09 15:39 UTC (permalink / raw) To: devel@edk2.groups.io, derek.lin2@hpe.com [-- 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode 2019-07-09 15:39 ` Zhang, Chao B @ 2019-07-10 8:50 ` Wang, Jian J 2019-07-10 17:04 ` Laszlo Ersek 0 siblings, 1 reply; 9+ messages in thread From: Wang, Jian J @ 2019-07-10 8:50 UTC (permalink / raw) To: devel@edk2.groups.io, Zhang, Chao B, derek.lin2@hpe.com [-- 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode 2019-07-10 8:50 ` Wang, Jian J @ 2019-07-10 17:04 ` Laszlo Ersek 2019-07-11 3:20 ` Zhang, Chao B 0 siblings, 1 reply; 9+ messages in thread From: Laszlo Ersek @ 2019-07-10 17:04 UTC (permalink / raw) To: devel, jian.j.wang, Zhang, Chao B, Derek Lin, Cinnamon Shia 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>> > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode 2019-07-10 17:04 ` Laszlo Ersek @ 2019-07-11 3:20 ` Zhang, Chao B 2019-07-11 11:47 ` Laszlo Ersek 0 siblings, 1 reply; 9+ messages in thread From: Zhang, Chao B @ 2019-07-11 3:20 UTC (permalink / raw) To: Laszlo Ersek, devel@edk2.groups.io, Wang, Jian J, Derek Lin, Cinnamon Shia [-- 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode 2019-07-11 3:20 ` Zhang, Chao B @ 2019-07-11 11:47 ` Laszlo Ersek 2019-07-12 1:41 ` Zhang, Chao B 0 siblings, 1 reply; 9+ messages in thread From: Laszlo Ersek @ 2019-07-11 11:47 UTC (permalink / raw) To: Zhang, Chao B, devel@edk2.groups.io, Wang, Jian J, Derek Lin, Cinnamon Shia, Felix Poludov, Peter Jones 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode 2019-07-11 11:47 ` Laszlo Ersek @ 2019-07-12 1:41 ` Zhang, Chao B 2019-08-23 3:20 ` Lin, Derek (HPS SW) 0 siblings, 1 reply; 9+ messages in thread From: Zhang, Chao B @ 2019-07-12 1:41 UTC (permalink / raw) To: devel@edk2.groups.io, lersek@redhat.com, Wang, Jian J, Derek Lin, Cinnamon Shia, Felix Poludov, Peter Jones [-- 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode 2019-07-12 1:41 ` Zhang, Chao B @ 2019-08-23 3:20 ` Lin, Derek (HPS SW) 0 siblings, 0 replies; 9+ messages in thread From: Lin, Derek (HPS SW) @ 2019-08-23 3:20 UTC (permalink / raw) To: Zhang, Chao B, devel [-- 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-08-23 3:20 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-07-02 5:25 [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode derek.lin2 2019-07-04 6:29 ` [edk2-devel] " derek.lin2 2019-07-09 15:39 ` Zhang, Chao B 2019-07-10 8:50 ` Wang, Jian J 2019-07-10 17:04 ` Laszlo Ersek 2019-07-11 3:20 ` Zhang, Chao B 2019-07-11 11:47 ` Laszlo Ersek 2019-07-12 1:41 ` Zhang, Chao B 2019-08-23 3:20 ` Lin, Derek (HPS SW)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox