From: "Liming Sun" <lsun@mellanox.com>
To: Wei6 Xu <wei6.xu@intel.com>, Liming Gao <liming.gao@intel.com>,
Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Sun <lsun@mellanox.com>,
devel@edk2.groups.io, Sean Brogan <sean.brogan@microsoft.com>
Subject: [PATCH] FmpDevicePkg: Enhance capsule verification with secure boot keys
Date: Fri, 19 Jun 2020 13:48:17 -0400 [thread overview]
Message-ID: <5b42e8e089fb961766c639b733284413ccf03272.1592587621.git.lsun@mellanox.com> (raw)
This commit enhances the FmpDevicePkg package to optionally verify
capsule with the secure boot keys when PcdFmpDevicePkcs7CertBufferXdr
is not set and the new PCD variable PcdFmpDeviceAllowSecureBootKeys
is configured. Below is the check logic:
- Pass if verified with PK key, or PK key not set yet;
- Deny if verified with the DBX keys;
- Verified it against the DB keys;
One purpose for this change is to auto-deploy the UEFI secure boot
keys with UEFI capsule. Initially it's done in trusted environment.
Once secure boot is enabled, the same keys will be used to verify
the signed capsules as well for further updates.
Signed-off-by: Liming Sun <lsun@mellanox.com>
---
FmpDevicePkg/FmpDevicePkg.dec | 6 +++
FmpDevicePkg/FmpDxe/FmpDxe.c | 109 ++++++++++++++++++++++++++++++++++++--
FmpDevicePkg/FmpDxe/FmpDxe.h | 1 +
FmpDevicePkg/FmpDxe/FmpDxe.inf | 3 ++
FmpDevicePkg/FmpDxe/FmpDxeLib.inf | 1 +
5 files changed, 117 insertions(+), 3 deletions(-)
diff --git a/FmpDevicePkg/FmpDevicePkg.dec b/FmpDevicePkg/FmpDevicePkg.dec
index cab63f5..3aeb89c 100644
--- a/FmpDevicePkg/FmpDevicePkg.dec
+++ b/FmpDevicePkg/FmpDevicePkg.dec
@@ -126,6 +126,12 @@
# @Prompt Firmware Device Image Type ID
gFmpDevicePkgTokenSpaceGuid.PcdFmpDeviceImageTypeIdGuid|{0}|VOID*|0x40000010
+ ## This option is used to verify the capsule using secure boot keys if the
+ # PcdFmpDevicePkcs7CertBufferXdr is not configured. In such case, the check
+ # will pass if secure boot hasn't been enabled yet.
+ # @A flag to tell whether to use secure boot keys when PcdFmpDevicePkcs7CertBufferXdr is not set.
+ gFmpDevicePkgTokenSpaceGuid.PcdFmpDeviceAllowSecureBootKeys|0x0|UINT8|0x40000012
+
[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
## One or more PKCS7 certificates used to verify a firmware device capsule
# update image. Encoded using the Variable-Length Opaque Data format of RFC
diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index 5884177..6f82aee 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -682,6 +682,102 @@ GetAllHeaderSize (
return CalculatedSize;
}
+EFI_STATUS
+CheckTheImageWithSecureBootVariable (
+ IN CONST CHAR16 *Name,
+ IN CONST EFI_GUID *Guid,
+ IN CONST VOID *Image,
+ IN UINTN ImageSize
+ )
+{
+ EFI_STATUS Status;
+ VOID *Data;
+ UINTN Length;
+ EFI_SIGNATURE_LIST *CertList;
+ EFI_SIGNATURE_DATA *CertData;
+ UINTN CertCount;
+ UINTN Index;
+
+ Status = GetVariable2 (Name, Guid, &Data, &Length);
+ if (EFI_ERROR (Status)) {
+ return EFI_NOT_FOUND;
+ }
+
+ CertList = (EFI_SIGNATURE_LIST *) Data;
+ while ((Length > 0) && (Length >= CertList->SignatureListSize)) {
+ if (CompareGuid (&CertList->SignatureType, &gEfiCertX509Guid)) {
+ CertData = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertList +
+ sizeof (EFI_SIGNATURE_LIST) + CertList->SignatureHeaderSize);
+ CertCount = (CertList->SignatureListSize - sizeof (EFI_SIGNATURE_LIST) -
+ CertList->SignatureHeaderSize) / CertList->SignatureSize;
+
+ for (Index = 0; Index < CertCount; Index++) {
+ Status = AuthenticateFmpImage (
+ (EFI_FIRMWARE_IMAGE_AUTHENTICATION *)Image,
+ ImageSize,
+ CertData->SignatureData,
+ CertList->SignatureSize - sizeof (EFI_GUID)
+ );
+ if (!EFI_ERROR (Status))
+ goto Done;
+
+ CertData = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertData + CertList->SignatureSize);
+ }
+ }
+
+ Length -= CertList->SignatureListSize;
+ CertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList + CertList->SignatureListSize);
+ }
+
+Done:
+ FreePool (Data);
+ return Status;
+}
+
+EFI_STATUS
+CheckTheImageWithSecureBootKeys (
+ IN CONST VOID *Image,
+ IN UINTN ImageSize
+ )
+{
+ EFI_STATUS Status;
+
+ // PK check.
+ Status = CheckTheImageWithSecureBootVariable(
+ EFI_PLATFORM_KEY_NAME,
+ &gEfiGlobalVariableGuid,
+ Image,
+ ImageSize
+ );
+ if (!EFI_ERROR (Status) || Status == EFI_NOT_FOUND) {
+ // Return SUCCESS if verified by PK key or PK key not configured.
+ DEBUG ((DEBUG_INFO, "FmpDxe: Verified capsule with PK key.\n"));
+ return EFI_SUCCESS;
+ }
+
+ // DBX check.
+ Status = CheckTheImageWithSecureBootVariable(
+ EFI_IMAGE_SECURITY_DATABASE1,
+ &gEfiImageSecurityDatabaseGuid,
+ Image,
+ ImageSize
+ );
+ if (!EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "FmpDxe: Reject capsule with DBX key.\n"));
+ return EFI_SECURITY_VIOLATION;
+ }
+
+ // DB check.
+ DEBUG ((DEBUG_INFO, "FmpDxe: Verify capsule with DB key.\n"));
+ Status = CheckTheImageWithSecureBootVariable(
+ EFI_IMAGE_SECURITY_DATABASE,
+ &gEfiImageSecurityDatabaseGuid,
+ Image,
+ ImageSize
+ );
+ return Status;
+}
+
/**
Checks if the firmware image is valid for the device.
@@ -728,6 +824,7 @@ CheckTheImage (
UINT8 *PublicKeyDataXdrEnd;
EFI_FIRMWARE_IMAGE_DEP *Dependencies;
UINT32 DependenciesSize;
+ UINT8 AllowSecureBootKeys;
Status = EFI_SUCCESS;
RawSize = 0;
@@ -782,9 +879,15 @@ CheckTheImage (
PublicKeyDataXdr = PcdGetPtr (PcdFmpDevicePkcs7CertBufferXdr);
PublicKeyDataXdrEnd = PublicKeyDataXdr + PcdGetSize (PcdFmpDevicePkcs7CertBufferXdr);
- if (PublicKeyDataXdr == NULL || (PublicKeyDataXdr == PublicKeyDataXdrEnd)) {
- DEBUG ((DEBUG_ERROR, "FmpDxe(%s): Invalid certificate, skipping it.\n", mImageIdName));
- Status = EFI_ABORTED;
+ if (PublicKeyDataXdr == NULL || (PublicKeyDataXdrEnd - PublicKeyDataXdr < sizeof (UINT32))) {
+ AllowSecureBootKeys = PcdGet8 (PcdFmpDeviceAllowSecureBootKeys);
+ if (AllowSecureBootKeys) {
+ DEBUG ((DEBUG_INFO, "FmpDxe: Use secure boot certs.\n"));
+ Status = CheckTheImageWithSecureBootKeys (Image, ImageSize);
+ } else {
+ DEBUG ((DEBUG_ERROR, "FmpDxe(%s): Invalid certificate, skipping it.\n", mImageIdName));
+ Status = EFI_ABORTED;
+ }
} else {
//
// Try each key from PcdFmpDevicePkcs7CertBufferXdr
diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.h b/FmpDevicePkg/FmpDxe/FmpDxe.h
index 30754de..72a6ce6 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.h
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.h
@@ -34,6 +34,7 @@
#include <Protocol/FirmwareManagement.h>
#include <Protocol/FirmwareManagementProgress.h>
#include <Protocol/VariableLock.h>
+#include <Guid/ImageAuthentication.h>
#include <Guid/SystemResourceTable.h>
#include <Guid/EventGroup.h>
diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.inf b/FmpDevicePkg/FmpDxe/FmpDxe.inf
index eeb904a..60b02d4 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.inf
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.inf
@@ -58,6 +58,8 @@
[Guids]
gEfiEndOfDxeEventGroupGuid
+ gEfiCertX509Guid
+ gEfiImageSecurityDatabaseGuid
[Protocols]
gEdkiiVariableLockProtocolGuid ## CONSUMES
@@ -74,6 +76,7 @@
gFmpDevicePkgTokenSpaceGuid.PcdFmpDevicePkcs7CertBufferXdr ## CONSUMES
gFmpDevicePkgTokenSpaceGuid.PcdFmpDeviceTestKeySha256Digest ## CONSUMES
gFmpDevicePkgTokenSpaceGuid.PcdFmpDeviceImageTypeIdGuid ## CONSUMES
+ gFmpDevicePkgTokenSpaceGuid.PcdFmpDeviceAllowSecureBootKeys ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdTestKeyUsed ## SOMETIMES_PRODUCES
[Depex]
diff --git a/FmpDevicePkg/FmpDxe/FmpDxeLib.inf b/FmpDevicePkg/FmpDxe/FmpDxeLib.inf
index 9a93b5e..1308cae 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxeLib.inf
+++ b/FmpDevicePkg/FmpDxe/FmpDxeLib.inf
@@ -74,6 +74,7 @@
gFmpDevicePkgTokenSpaceGuid.PcdFmpDevicePkcs7CertBufferXdr ## CONSUMES
gFmpDevicePkgTokenSpaceGuid.PcdFmpDeviceTestKeySha256Digest ## CONSUMES
gFmpDevicePkgTokenSpaceGuid.PcdFmpDeviceImageTypeIdGuid ## CONSUMES
+ gFmpDevicePkgTokenSpaceGuid.PcdFmpDeviceAllowSecureBootKeys ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdTestKeyUsed ## SOMETIMES_PRODUCES
[Depex]
--
1.8.3.1
next reply other threads:[~2020-06-19 17:48 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-19 17:48 Liming Sun [this message]
2020-06-29 3:18 ` [edk2-devel] [PATCH] FmpDevicePkg: Enhance capsule verification with secure boot keys Guomin Jiang
2020-06-30 3:33 ` Liming Sun
2020-06-30 7:32 ` Guomin Jiang
2020-06-30 12:47 ` Liming Sun
2020-07-01 0:56 ` Guomin Jiang
2020-07-01 16:26 ` Liming Sun
2020-07-01 17:42 ` Michael D Kinney
2020-07-06 20:58 ` Liming Sun
2020-07-07 15:42 ` Michael D Kinney
2020-07-22 0:37 ` Guomin Jiang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5b42e8e089fb961766c639b733284413ccf03272.1592587621.git.lsun@mellanox.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox