|
Re: [PATCH v2 2/2] ArmVirtPkg/ArmVirtQemu: Avoid early ID map on ThunderX
Marc Zyngier <maz@...>
On Thu, 19 Jan 2023 11:11:34 +0000,
Ard Biesheuvel <ardb@...> wrote: Oliver seems to have a vintage ThunderX (aka the worst arm64 implementation in history!), so it is indeed a very different beat from TX2. Without the kernel patch[1], I can trigger the issue pretty reliably, specially in the absence of THP. It all depends on the layout of the EDK2 object and the order in which pages get mapped. The first course of action would be to make sure that the patch is applied to the host kernel. If this still fails to boot, I'm happy to help investigating it. M. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/patch?id=406504c7b0405d74d74c15a667cd4c4620c3e7a9 -- Without deviation from the norm, progress is not possible. |
|
|
|
Re: [PATCH V2] MdeModulePkg/Decompress: Add missing source file to Brotli library
Timothy Lin
This patch tried to fix following build failure when linking the Brotli decompress lib.
Steps to replicate the issue:
1 - Attach a NULL lib to DxeIpl.inf in any project's .DSC, say OvmfPkgX64.dsc
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
<LibraryClasses>
NULL|MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf
}
2 - Follow the EDKII'2 package build steps to build that project. Build.py would compliain with following errors.
...
/home/edk2/edk2-stable202211/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli/c/dec/decode.c:879: undefined reference to `_kBrotliPrefixCodeRanges' /usr/bin/ld: /home/edk2/edk2-stable202211/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli/c/dec/decode.c:1193: undefined reference to `_kBrotliContextLookupTable' /usr/bin/ld: /tmp/ccKzzHN0.ltrans0.ltrans.o: in function `DecodeCommandBlockSwitch': ... /usr/bin/ld: /tmp/ccKzzHN0.ltrans0.ltrans.o: in function `BrotliUefiDecompress': /home/edk2/edk2-stable202211/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli/c/dec/decode.c:898: undefined reference to `_kBrotliPrefixCodeRanges' /usr/bin/ld: /home/edk2/edk2-stable202211/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli/c/dec/decode.c:1193: undefined reference to `_kBrotliContextLookupTable' |
|
|
|
Re: [PATCH v1 1/1] .devcontainer/devcontainer.json: Add devcontainer file
Ard Biesheuvel
On Mon, 23 Jan 2023 at 10:23, Oliver Steffen <osteffen@...> wrote:
Yes, please, this would be very useful. |
|
|
|
Re: [PATCH v1 1/1] .devcontainer/devcontainer.json: Add devcontainer file
Oliver Steffen
Quoting Rebecca Cran (2023-01-20 22:22:55)
Related to this, I've been wondering if we might want to commit a .vscode directory with project configuration file(s)?As long as project settings for other IDEs (should there be a reasonable user base) would be accepted in the future, why not? I like the idea, because it is understood by many IDEs and text editors. -Oliver |
|
|
|
Re: [PATCH edk2-platforms v1 1/1] Platform/Hisilicon: Fix missing dependency on VariableFlashInfoLib
PierreGondois
Hi Sami,
toggle quoted message
Show quoted text
I think other platforms have the same library missing: - Platform/Hisilicon/D03/D03.dsc - Platform/Hisilicon/D06/D06.dsc - Platform/Hisilicon/HiKey960/HiKey960.dsc - Platform/Hisilicon/HiKey/HiKey.dsc If the library definition is done in Silicon/Hisilicon/Hisilicon.dsc.inc, I think it solves all the definitions at once, Regards, Pierre On 1/19/23 17:14, Sami Mujawar wrote:
From: Guillaume Gardet <Guillaume.Gardet@...> |
|
|
|
Re: [PATCH v1 0/4] Don't require self-signed PK in setup mode
Yao, Jiewen
Hi Sean
toggle quoted message
Show quoted text
I would like to hear your feedback, since it is a little different from the original MSFT patch. Would you please take a look? Thank you Yao, Jiewen -----Original Message----- |
|
|
|
Re: [PATCH v2 1/1] SecurityPkg/AuthVariableLib: Check SHA-256 OID with ContentInfo present
Yao, Jiewen
toggle quoted message
Show quoted text
From: Yao, Jiewen <jiewen.yao@...>
Sent: Monday, January 23, 2023 10:38 AM To: Jan Bobek <jbobek@...>; devel@edk2.groups.io Cc: Jan Bobek <jbobek@...>; Wang, Jian J <jian.j.wang@...>; Xu, Min M <min.m.xu@...> Subject: Re: [PATCH v2 1/1] SecurityPkg/AuthVariableLib: Check SHA-256 OID with ContentInfo present
reviewed-by: Jiewen Yao <Jiewen.yao@...> 发件人:
Jan Bobek <jbobek@...>
REF:
https://bugzilla.tianocore.org/show_bug.cgi?id=4305 |
|
|
|
Re: [PATCH 1/1] MdeModulePkg: Fix typo of "event" in a comment in Core/Dxe/Event/Event.c
Michael D Kinney
Reviewed-by: Michael D Kinney <michael.d.kinney@...>
toggle quoted message
Show quoted text
-----Original Message----- |
|
|
|
[PATCH 1/1] MdeModulePkg: Fix typo of "event" in a comment in Core/Dxe/Event/Event.c
Rebecca Cran
Signed-off-by: Rebecca Cran <rebecca@...>
--- MdeModulePkg/Core/Dxe/Event/Event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MdeModulePkg/Core/Dxe/Event/Event.c b/MdeModulePkg/Core/Dxe/Event/Event.c index dc82abb02130..7506803b0b38 100644 --- a/MdeModulePkg/Core/Dxe/Event/Event.c +++ b/MdeModulePkg/Core/Dxe/Event/Event.c @@ -606,7 +606,7 @@ CoreCheckEvent ( } // - // If the even looks signalled, get the lock and clear it + // If the event looks signalled, get the lock and clear it // if (Event->SignalCount != 0) { -- 2.34.1 |
|
|
|
Re: [PATCH v2 1/1] SecurityPkg/AuthVariableLib: Check SHA-256 OID with ContentInfo present
Yao, Jiewen
reviewed-by: Jiewen Yao <Jiewen.yao@...>
发件人: Jan Bobek <jbobek@...>
发送时间: Monday, January 23, 2023 5:53:48 AM 收件人: devel@edk2.groups.io <devel@edk2.groups.io> 抄送: Jan Bobek <jbobek@...>; Yao, Jiewen <jiewen.yao@...>; Wang, Jian J <jian.j.wang@...>; Xu, Min M <min.m.xu@...> 主题: [PATCH v2 1/1] SecurityPkg/AuthVariableLib: Check SHA-256 OID with ContentInfo present REF:
https://bugzilla.tianocore.org/show_bug.cgi?id=4305
Based on whether the DER-encoded ContentInfo structure is present in authenticated SetVariable payload or not, the SHA-256 OID can be located at different places. UEFI specification explicitly states the driver shall support both cases, but the old code assumed ContentInfo was not present and incorrectly rejected authenticated variable updates when it were present. Cc: Jiewen Yao <jiewen.yao@...> Cc: Jian J Wang <jian.j.wang@...> Cc: Min Xu <min.m.xu@...> Signed-off-by: Jan Bobek <jbobek@...> --- .../Library/AuthVariableLib/AuthService.c | 50 ++++++++++++++++--- 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c b/SecurityPkg/Library/AuthVariableLib/AuthService.c index 054ee4d1d988..9beeca09aeba 100644 --- a/SecurityPkg/Library/AuthVariableLib/AuthService.c +++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c @@ -1925,7 +1925,7 @@ VerifyTimeBasedPayload ( // SignedData.digestAlgorithms shall contain the digest algorithm used when preparing the // signature. Only a digest algorithm of SHA-256 is accepted. // - // According to PKCS#7 Definition: + // According to PKCS#7 Definition (https://www.rfc-editor.org/rfc/rfc2315): // SignedData ::= SEQUENCE { // version Version, // digestAlgorithms DigestAlgorithmIdentifiers, @@ -1933,15 +1933,49 @@ VerifyTimeBasedPayload ( // .... } // The DigestAlgorithmIdentifiers can be used to determine the hash algorithm // in VARIABLE_AUTHENTICATION_2 descriptor. - // This field has the fixed offset (+13) and be calculated based on two bytes of length encoding. + // This field has the fixed offset (+13) or (+32) based on whether the DER-encoded + // ContentInfo structure is present or not, and can be calculated based on two + // bytes of length encoding. + // + // Both condition can be handled in WrapPkcs7Data() in CryptPkcs7VerifyCommon.c. + // + // See below examples: + // + // 1. Without ContentInfo + // 30 82 0c da // SEQUENCE (5 element) (3294 BYTES) -- SignedData + // 02 01 01 // INTEGER 1 -- Version + // 31 0f // SET (1 element) (15 BYTES) -- DigestAlgorithmIdentifiers + // 30 0d // SEQUENCE (2 element) (13 BYTES) -- AlgorithmIdentifier + // 06 09 // OBJECT-IDENTIFIER (9 BYTES) -- algorithm + // 60 86 48 01 65 03 04 02 01 // sha256 [2.16.840.1.101.3.4.2.1] + // 05 00 // NULL (0 BYTES) -- parameters + // + // Example from: https://uefi.org/revocationlistfile + // + // 2. With ContentInfo + // 30 82 05 90 // SEQUENCE (1424 BYTES) -- ContentInfo + // 06 09 // OBJECT-IDENTIFIER (9 BYTES) -- ContentType + // 2a 86 48 86 f7 0d 01 07 02 // signedData [1.2.840.113549.1.7.2] + // a0 82 05 81 // CONTEXT-SPECIFIC CONSTRUCTED TAG 0 (1409 BYTES) -- content + // 30 82 05 7d // SEQUENCE (1405 BYTES) -- SignedData + // 02 01 01 // INTEGER 1 -- Version + // 31 0f // SET (1 element) (15 BYTES) -- DigestAlgorithmIdentifiers + // 30 0d // SEQUENCE (13 BYTES) -- AlgorithmIdentifier + // 06 09 // OBJECT-IDENTIFIER (9 BYTES) -- algorithm + // 60 86 48 01 65 03 04 02 01 // sha256 [2.16.840.1.101.3.4.2.1] + // 05 00 // NULL (0 BYTES) -- parameters + // + // Example generated with: https://wiki.archlinux.org/title/Unified_Extensible_Firmware_Interface/Secure_Boot#Manual_process // if ((Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != 0) { - if (SigDataSize >= (13 + sizeof (mSha256OidValue))) { - if (((*(SigData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) || - (CompareMem (SigData + 13, &mSha256OidValue, sizeof (mSha256OidValue)) != 0)) - { - return EFI_SECURITY_VIOLATION; - } + if ( ( (SigDataSize >= (13 + sizeof (mSha256OidValue))) + && ( ((*(SigData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) + || (CompareMem (SigData + 13, &mSha256OidValue, sizeof (mSha256OidValue)) != 0))) + && ( (SigDataSize >= (32 + sizeof (mSha256OidValue))) + && ( ((*(SigData + 20) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) + || (CompareMem (SigData + 32, &mSha256OidValue, sizeof (mSha256OidValue)) != 0)))) + { + return EFI_SECURITY_VIOLATION; } } -- 2.30.2 |
|
|
|
Event: Tools, CI, Code base construction meeting series - Monday, January 23, 2023
#cal-reminder
Group Notification <noreply@...>
Reminder: Tools, CI, Code base construction meeting series When: Where: Description: TianoCore community, Microsoft and Intel will be hosting a series of open meetings to discuss build, CI, tools, and other related topics. If you are interested, have ideas/opinions please join us. These meetings will be Monday 4:30pm Pacific Time on Microsoft Teams. MS Teams Link in following discussion: * https://github.com/tianocore/edk2/discussions/2614 Anyone is welcome to join.
MS Teams Browser Clients * https://docs.microsoft.com/en-us/microsoftteams/get-clients?tabs=Windows#browser-client |
|
|
|
[PATCH v2 1/1] SecurityPkg/AuthVariableLib: Check SHA-256 OID with ContentInfo present
Jan Bobek
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4305
Based on whether the DER-encoded ContentInfo structure is present in authenticated SetVariable payload or not, the SHA-256 OID can be located at different places. UEFI specification explicitly states the driver shall support both cases, but the old code assumed ContentInfo was not present and incorrectly rejected authenticated variable updates when it were present. Cc: Jiewen Yao <jiewen.yao@...> Cc: Jian J Wang <jian.j.wang@...> Cc: Min Xu <min.m.xu@...> Signed-off-by: Jan Bobek <jbobek@...> --- .../Library/AuthVariableLib/AuthService.c | 50 ++++++++++++++++--- 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c b/SecurityPkg/Library/AuthVariableLib/AuthService.c index 054ee4d1d988..9beeca09aeba 100644 --- a/SecurityPkg/Library/AuthVariableLib/AuthService.c +++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c @@ -1925,7 +1925,7 @@ VerifyTimeBasedPayload ( // SignedData.digestAlgorithms shall contain the digest algorithm used when preparing the // signature. Only a digest algorithm of SHA-256 is accepted. // - // According to PKCS#7 Definition: + // According to PKCS#7 Definition (https://www.rfc-editor.org/rfc/rfc2315): // SignedData ::= SEQUENCE { // version Version, // digestAlgorithms DigestAlgorithmIdentifiers, @@ -1933,15 +1933,49 @@ VerifyTimeBasedPayload ( // .... } // The DigestAlgorithmIdentifiers can be used to determine the hash algorithm // in VARIABLE_AUTHENTICATION_2 descriptor. - // This field has the fixed offset (+13) and be calculated based on two bytes of length encoding. + // This field has the fixed offset (+13) or (+32) based on whether the DER-encoded + // ContentInfo structure is present or not, and can be calculated based on two + // bytes of length encoding. + // + // Both condition can be handled in WrapPkcs7Data() in CryptPkcs7VerifyCommon.c. + // + // See below examples: + // + // 1. Without ContentInfo + // 30 82 0c da // SEQUENCE (5 element) (3294 BYTES) -- SignedData + // 02 01 01 // INTEGER 1 -- Version + // 31 0f // SET (1 element) (15 BYTES) -- DigestAlgorithmIdentifiers + // 30 0d // SEQUENCE (2 element) (13 BYTES) -- AlgorithmIdentifier + // 06 09 // OBJECT-IDENTIFIER (9 BYTES) -- algorithm + // 60 86 48 01 65 03 04 02 01 // sha256 [2.16.840.1.101.3.4.2.1] + // 05 00 // NULL (0 BYTES) -- parameters + // + // Example from: https://uefi.org/revocationlistfile + // + // 2. With ContentInfo + // 30 82 05 90 // SEQUENCE (1424 BYTES) -- ContentInfo + // 06 09 // OBJECT-IDENTIFIER (9 BYTES) -- ContentType + // 2a 86 48 86 f7 0d 01 07 02 // signedData [1.2.840.113549.1.7.2] + // a0 82 05 81 // CONTEXT-SPECIFIC CONSTRUCTED TAG 0 (1409 BYTES) -- content + // 30 82 05 7d // SEQUENCE (1405 BYTES) -- SignedData + // 02 01 01 // INTEGER 1 -- Version + // 31 0f // SET (1 element) (15 BYTES) -- DigestAlgorithmIdentifiers + // 30 0d // SEQUENCE (13 BYTES) -- AlgorithmIdentifier + // 06 09 // OBJECT-IDENTIFIER (9 BYTES) -- algorithm + // 60 86 48 01 65 03 04 02 01 // sha256 [2.16.840.1.101.3.4.2.1] + // 05 00 // NULL (0 BYTES) -- parameters + // + // Example generated with: https://wiki.archlinux.org/title/Unified_Extensible_Firmware_Interface/Secure_Boot#Manual_process // if ((Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != 0) { - if (SigDataSize >= (13 + sizeof (mSha256OidValue))) { - if (((*(SigData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) || - (CompareMem (SigData + 13, &mSha256OidValue, sizeof (mSha256OidValue)) != 0)) - { - return EFI_SECURITY_VIOLATION; - } + if ( ( (SigDataSize >= (13 + sizeof (mSha256OidValue))) + && ( ((*(SigData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) + || (CompareMem (SigData + 13, &mSha256OidValue, sizeof (mSha256OidValue)) != 0))) + && ( (SigDataSize >= (32 + sizeof (mSha256OidValue))) + && ( ((*(SigData + 20) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) + || (CompareMem (SigData + 32, &mSha256OidValue, sizeof (mSha256OidValue)) != 0)))) + { + return EFI_SECURITY_VIOLATION; } } -- 2.30.2 |
|
|
|
Re: [PATCH V2 04/10] OvmfPkg/IntelTdx: Implement other helper functions in SecTdxHelperLib
Min Xu
On January 20, 2023 9:51 PM, Gerd Hoffmann wrote:
Hi, Gerdimplementation can choose its own length. Below are some examples.As Jiewen mentioned TCG defines the field to be variable length. The[Jiewen] No. We cannot move to MdePkg.Why doesn't follow TDX standard TCG practices here? Actually I tried to find some common header file to define the events (the firmware volume). But it seems there is no such header file. Let me check the code again and see if there is such header file. Thanks Min |
|
|
|
[PATCH v1 4/4] SecurityPkg: don't require PK to be self-signed by default
Jan Bobek
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2506
Change the default value of PcdRequireSelfSignedPk to FALSE in accordance with UEFI spec, which states that PK need not be self-signed when enrolling in setup mode. Note that this relaxes the legacy behavior, which required the PK to be self-signed in this case. Cc: Jiewen Yao <jiewen.yao@...> Cc: Jian J Wang <jian.j.wang@...> Signed-off-by: Jan Bobek <jbobek@...> --- SecurityPkg/SecurityPkg.dec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec index d3b7ad7ff6fb..0382090f4e75 100644 --- a/SecurityPkg/SecurityPkg.dec +++ b/SecurityPkg/SecurityPkg.dec @@ -585,7 +585,7 @@ [PcdsFeatureFlag] # TRUE - Require PK to be self-signed. # FALSE - Do not require PK to be self-signed. # @Prompt Require PK to be self-signed - gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE|BOOLEAN|0x00010027 + gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|FALSE|BOOLEAN|0x00010027 [UserExtensions.TianoCore."ExtraFiles"] SecurityPkgExtra.uni -- 2.30.2 |
|
|
|
[PATCH v1 2/4] OvmfPkg: require self-signed PK when secure boot is enabled
Jan Bobek
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2506
In all DSC files that define SECURE_BOOT_ENABLE, opt-in into requiring self-signed PK when SECURE_BOOT_ENABLE is TRUE. Cc: Ard Biesheuvel <ardb+tianocore@...> Cc: Jiewen Yao <jiewen.yao@...> Cc: Jordan Justen <jordan.l.justen@...> Cc: Gerd Hoffmann <kraxel@...> Cc: Rebecca Cran <rebecca@...> Cc: Peter Grehan <grehan@...> Cc: Sebastien Boeuf <sebastien.boeuf@...> Signed-off-by: Jan Bobek <jbobek@...> --- OvmfPkg/Bhyve/BhyveX64.dsc | 3 +++ OvmfPkg/CloudHv/CloudHvX64.dsc | 3 +++ OvmfPkg/IntelTdx/IntelTdxX64.dsc | 3 +++ OvmfPkg/Microvm/MicrovmX64.dsc | 3 +++ OvmfPkg/OvmfPkgIa32.dsc | 3 +++ OvmfPkg/OvmfPkgIa32X64.dsc | 3 +++ OvmfPkg/OvmfPkgX64.dsc | 3 +++ 7 files changed, 21 insertions(+) diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc index befec670d4f3..66a2ae8868e5 100644 --- a/OvmfPkg/Bhyve/BhyveX64.dsc +++ b/OvmfPkg/Bhyve/BhyveX64.dsc @@ -422,6 +422,9 @@ [PcdsFeatureFlag] gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE +!if $(SECURE_BOOT_ENABLE) == TRUE + gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE +!endif [PcdsFixedAtBuild] gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|TRUE diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc index 7326417eab62..9cb267f98942 100644 --- a/OvmfPkg/CloudHv/CloudHvX64.dsc +++ b/OvmfPkg/CloudHv/CloudHvX64.dsc @@ -480,6 +480,9 @@ [PcdsFeatureFlag] gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugSupport|TRUE gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE !endif +!if $(SECURE_BOOT_ENABLE) == TRUE + gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE +!endif [PcdsFixedAtBuild] gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc index 0f1e970fbbb3..93918b55b1a5 100644 --- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc +++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc @@ -390,6 +390,9 @@ [PcdsFeatureFlag] !ifdef $(CSM_ENABLE) gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable|TRUE !endif +!if $(SECURE_BOOT_ENABLE) == TRUE + gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE +!endif [PcdsFixedAtBuild] gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc index 2d53b5c2950d..3c988f3e65e0 100644 --- a/OvmfPkg/Microvm/MicrovmX64.dsc +++ b/OvmfPkg/Microvm/MicrovmX64.dsc @@ -476,6 +476,9 @@ [PcdsFeatureFlag] gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE +!if $(SECURE_BOOT_ENABLE) == TRUE + gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE +!endif [PcdsFixedAtBuild] gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index f232de13a7b6..22dc29330d2d 100644 --- a/OvmfPkg/OvmfPkgIa32.dsc +++ b/OvmfPkg/OvmfPkgIa32.dsc @@ -488,6 +488,9 @@ [PcdsFeatureFlag] gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugSupport|TRUE gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE !endif +!if $(SECURE_BOOT_ENABLE) == TRUE + gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE +!endif [PcdsFixedAtBuild] gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc index a9d422bd9169..6b539814bdb0 100644 --- a/OvmfPkg/OvmfPkgIa32X64.dsc +++ b/OvmfPkg/OvmfPkgIa32X64.dsc @@ -493,6 +493,9 @@ [PcdsFeatureFlag] gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugSupport|TRUE gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE !endif +!if $(SECURE_BOOT_ENABLE) == TRUE + gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE +!endif [PcdsFixedAtBuild] gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index 3f970a79a08a..f6b8b342c4ed 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -513,6 +513,9 @@ [PcdsFeatureFlag] gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugSupport|TRUE gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE !endif +!if $(SECURE_BOOT_ENABLE) == TRUE + gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE +!endif [PcdsFixedAtBuild] gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 -- 2.30.2 |
|
|
|
[PATCH v1 3/4] ArmVirtPkg: require self-signed PK when secure boot is enabled
Jan Bobek
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2506
In all DSC files that define SECURE_BOOT_ENABLE, opt-in into requiring self-signed PK when SECURE_BOOT_ENABLE is TRUE. Cc: Ard Biesheuvel <ardb+tianocore@...> Cc: Leif Lindholm <quic_llindhol@...> Cc: Sami Mujawar <sami.mujawar@...> Cc: Gerd Hoffmann <kraxel@...> Signed-off-by: Jan Bobek <jbobek@...> --- ArmVirtPkg/ArmVirtCloudHv.dsc | 4 ++++ ArmVirtPkg/ArmVirtQemu.dsc | 4 ++++ ArmVirtPkg/ArmVirtQemuKernel.dsc | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc index 7ca7a391d9cf..dc33936d6f03 100644 --- a/ArmVirtPkg/ArmVirtCloudHv.dsc +++ b/ArmVirtPkg/ArmVirtCloudHv.dsc @@ -85,6 +85,10 @@ [PcdsFeatureFlag.common] gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE +!if $(SECURE_BOOT_ENABLE) == TRUE + gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE +!endif + [PcdsFixedAtBuild.common] !if $(ARCH) == AARCH64 gArmTokenSpaceGuid.PcdVFPEnabled|1 diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc index 0f1c6395488a..31fd0e5279ab 100644 --- a/ArmVirtPkg/ArmVirtQemu.dsc +++ b/ArmVirtPkg/ArmVirtQemu.dsc @@ -145,6 +145,10 @@ [PcdsFeatureFlag.common] gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled|$(TPM2_ENABLE) +!if $(SECURE_BOOT_ENABLE) == TRUE + gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE +!endif + [PcdsFixedAtBuild.common] !if $(ARCH) == AARCH64 gArmTokenSpaceGuid.PcdVFPEnabled|1 diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc index 807c85d48285..1e0f06c91137 100644 --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc @@ -114,6 +114,10 @@ [PcdsFeatureFlag.common] gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE +!if $(SECURE_BOOT_ENABLE) == TRUE + gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE +!endif + [PcdsFixedAtBuild.common] !if $(ARCH) == AARCH64 gArmTokenSpaceGuid.PcdVFPEnabled|1 -- 2.30.2 |
|
|
|
[PATCH v1 1/4] SecurityPkg: limit verification of enrolled PK in setup mode
Jan Bobek
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2506
Per UEFI spec, enrolling a new PK in setup mode should not require a self-signature. Introduce a feature PCD called PcdRequireSelfSignedPk to control this requirement. Default to TRUE in order to preserve the legacy behavior. Cc: Jiewen Yao <jiewen.yao@...> Cc: Jian J Wang <jian.j.wang@...> Cc: Min Xu <min.m.xu@...> Co-authored-by: Matthew Carlson <macarl@...> Signed-off-by: Jan Bobek <jbobek@...> --- SecurityPkg/SecurityPkg.dec | 7 +++++++ SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf | 3 +++ SecurityPkg/Library/AuthVariableLib/AuthService.c | 9 +++++++-- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec index 8257f11d17c7..d3b7ad7ff6fb 100644 --- a/SecurityPkg/SecurityPkg.dec +++ b/SecurityPkg/SecurityPkg.dec @@ -580,5 +580,12 @@ [PcdsDynamic, PcdsDynamicEx] ## This PCD records LASA field in CC EVENTLOG ACPI table. gEfiSecurityPkgTokenSpaceGuid.PcdCcEventlogAcpiTableLasa|0|UINT64|0x00010026 +[PcdsFeatureFlag] + ## Indicates if the platform requires PK to be self-signed when setting the PK in setup mode. + # TRUE - Require PK to be self-signed. + # FALSE - Do not require PK to be self-signed. + # @Prompt Require PK to be self-signed + gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE|BOOLEAN|0x00010027 + [UserExtensions.TianoCore."ExtraFiles"] SecurityPkgExtra.uni diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf index 8eadeebcebd7..e5985c5f8b60 100644 --- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf +++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf @@ -86,3 +86,6 @@ [Guids] gEfiCertTypeRsa2048Sha256Guid ## SOMETIMES_CONSUMES ## GUID # Unique ID for the type of the certificate. gEfiCertPkcs7Guid ## SOMETIMES_CONSUMES ## GUID # Unique ID for the type of the certificate. gEfiCertX509Guid ## SOMETIMES_CONSUMES ## GUID # Unique ID for the type of the signature. + +[FeaturePcd] + gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c b/SecurityPkg/Library/AuthVariableLib/AuthService.c index 054ee4d1d988..e9989695626e 100644 --- a/SecurityPkg/Library/AuthVariableLib/AuthService.c +++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c @@ -603,7 +603,10 @@ 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) + && !(FeaturePcdGet (PcdRequireSelfSignedPk) && IsPk))) + { Payload = (UINT8 *)Data + AUTHINFO2_SIZE (Data); PayloadSize = DataSize - AUTHINFO2_SIZE (Data); if (PayloadSize == 0) { @@ -627,7 +630,9 @@ ProcessVarWithPk ( return Status; } - if ((mPlatformMode != SETUP_MODE) || IsPk) { + if ( (mPlatformMode != SETUP_MODE) + || (FeaturePcdGet (PcdRequireSelfSignedPk) && IsPk)) + { Status = VendorKeyIsModified (); } } else if (mPlatformMode == USER_MODE) { -- 2.30.2 |
|
|
|
[PATCH v1 0/4] Don't require self-signed PK in setup mode
Jan Bobek
Hi all,
I'm sending out v1 of my patch series that addresses a UEFI spec non-compliance when enrolling PK in setup mode. Additional info can be found in bugzilla [1]; the changes are split into 4 patches as suggested by Laszlo Ersek in comment #4. I've based my work on the patch by Matthew Carlson; I've credited him with co-authorship of the first patch even though in the end I decided to do the implementation a bit differently. Comments & reviews welcome! Cheers, -Jan References: 1. https://bugzilla.tianocore.org/show_bug.cgi?id=2506 Jan Bobek (4): SecurityPkg: limit verification of enrolled PK in setup mode OvmfPkg: require self-signed PK when secure boot is enabled ArmVirtPkg: require self-signed PK when secure boot is enabled SecurityPkg: don't require PK to be self-signed by default SecurityPkg/SecurityPkg.dec | 7 +++++++ ArmVirtPkg/ArmVirtCloudHv.dsc | 4 ++++ ArmVirtPkg/ArmVirtQemu.dsc | 4 ++++ ArmVirtPkg/ArmVirtQemuKernel.dsc | 4 ++++ OvmfPkg/Bhyve/BhyveX64.dsc | 3 +++ OvmfPkg/CloudHv/CloudHvX64.dsc | 3 +++ OvmfPkg/IntelTdx/IntelTdxX64.dsc | 3 +++ OvmfPkg/Microvm/MicrovmX64.dsc | 3 +++ OvmfPkg/OvmfPkgIa32.dsc | 3 +++ OvmfPkg/OvmfPkgIa32X64.dsc | 3 +++ OvmfPkg/OvmfPkgX64.dsc | 3 +++ SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf | 3 +++ SecurityPkg/Library/AuthVariableLib/AuthService.c | 9 +++++++-- 13 files changed, 50 insertions(+), 2 deletions(-) -- 2.30.2 |
|
|
|
Re: [PATCH v1 1/1] .devcontainer/devcontainer.json: Add devcontainer file
Michael Kubacki
I think that's a good idea. edk2-pytool-extensions has a .vscode/settings.json file that has been useful for local python linting and testing that helps catch issues before CI.
toggle quoted message
Show quoted text
For example: https://github.com/tianocore/edk2-pytool-extensions/blob/master/.vscode/settings.json On 1/20/2023 4:22 PM, Rebecca Cran wrote:
Related to this, I've been wondering if we might want to commit a .vscode directory with project configuration file(s)? |
|
|
|
Re: [PATCH 1/3] UsbNetworkPkg/UsbRndis: Add USB RNDIS devices support
Rebecca Cran
Hi Richard,
toggle quoted message
Show quoted text
I've continued working on my USB EEM support, and just pushed my changes to https://github.com/bcran/edk2/tree/usb-net . I was wondering when you think you might have the next set of patches ready for review? Thanks. Rebecca Cran On 1/12/23 01:36, RichardHo [何明忠] via groups.io wrote:
Hi Michael, |
|
|