From: "Long, Qin" <qin.long@intel.com>
To: "Zhang, Chao B" <chao.b.zhang@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Fu, Siyuan" <siyuan.fu@intel.com>
Subject: Re: [PATCH] SecureBoot UI Update
Date: Thu, 30 Mar 2017 08:26:20 +0000 [thread overview]
Message-ID: <BF2CCE9263284D428840004653A28B6E53F7E468@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <20170330020045.21452-1-chao.b.zhang@intel.com>
Reviewed-by: Long Qin <qin.long@intel.com>
And some typos need to be corrected when check-in:
+EnrollAuthenication2Descriptor (
--> EnrollAuthentication2Description (
+ return EnrollAuthenication2Descriptor(Private, VariableName);
--> EnrollAuthentication2Descriptor
+ IfrNvData->FileEnrollType = AUTHENCIATION_2_FILE_TYPE;
--> AUTHENTICATION_2_FILE_TYPE;
+#define AUTHENCIATION_2_FILE_TYPE 2
--> AUTHENTICATION_2_FILE_TYPE
+ UINT8 FileEnrollType; // File type of sigunature enroll
--> Signature
+#string STR_DBX_AUTH_2_FORMAT #language en-US "VARIABLE_AUTHENICATION_2"
--> VARIABLE_AUTHENTICATION_2"
> -----Original Message-----
> From: Zhang, Chao B
> Sent: Thursday, March 30, 2017 10:01 AM
> To: edk2-devel@lists.01.org
> Cc: Long, Qin; Fu, Siyuan
> Subject: [PATCH] SecureBoot UI Update
>
> ---
> .../SecureBootConfigDxe/SecureBootConfig.vfr | 38 +++-
> .../SecureBootConfigDxe/SecureBootConfigImpl.c | 196
> ++++++++++++++++++++-
> .../SecureBootConfigDxe/SecureBootConfigImpl.h | 32 ++++
> .../SecureBootConfigDxe/SecureBootConfigNvData.h | 5 +
> .../SecureBootConfigStrings.uni | 13 +-
> 5 files changed, 268 insertions(+), 16 deletions(-)
>
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> fig.vfr
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> fig.vfr
> index 02ddf4a..e153eca 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> fig.vfr
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
> +++ nfig.vfr
> @@ -455,15 +455,35 @@ formset
> maxsize = SECURE_BOOT_GUID_SIZE,
> endstring;
>
> - oneof name = SignatureFormatInDbx,
> - varid = SECUREBOOT_CONFIGURATION.CertificateFormat,
> - prompt =
> STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_PROMPT),
> - help = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_HELP),
> - option text =
> STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA256), value = 0x2,
> flags = DEFAULT;
> - option text =
> STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA384), value = 0x3,
> flags = 0;
> - option text =
> STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA512), value = 0x4,
> flags = 0;
> - option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_RAW),
> value = 0x5, flags = 0;
> - endoneof;
> + disableif NOT ideqval SECUREBOOT_CONFIGURATION.FileEnrollType == 1;
> + oneof name = X509SignatureFormatInDbx,
> + varid = SECUREBOOT_CONFIGURATION.CertificateFormat,
> + prompt =
> STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_PROMPT),
> + help = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_HELP),
> + option text =
> STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA256), value = 0x2,
> flags = DEFAULT;
> + option text =
> STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA384), value = 0x3,
> flags = 0;
> + option text =
> STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA512), value = 0x4,
> flags = 0;
> + option text =
> STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_RAW), value = 0x5, flags =
> 0;
> + endoneof;
> + endif;
> +
> + disableif NOT ideqval SECUREBOOT_CONFIGURATION.FileEnrollType == 2;
> + grayoutif TRUE;
> + text
> + help = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_PROMPT),
> // Help string
> + text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_HELP),
> // Prompt string
> + text = STRING_TOKEN(STR_DBX_PE_FORMAT_SHA256); //
> TextTwo
> + endif;
> + endif;
> +
> + suppressif ideqval SECUREBOOT_CONFIGURATION.FileEnrollType == 3;
> + grayoutif TRUE;
> + text
> + help = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_PROMPT),
> // Help string
> + text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_HELP),
> // Prompt string
> + text = STRING_TOKEN(STR_DBX_AUTH_2_FORMAT); //
> TextTwo
> + endif;
> + endif;
>
> suppressif ideqval SECUREBOOT_CONFIGURATION.CertificateFormat == 5;
> checkbox varid = SECUREBOOT_CONFIGURATION.AlwaysRevocation,
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figImpl.c
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figImpl.c
> index 6f58729..17fe120 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figImpl.c
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
> +++ nfigImpl.c
> @@ -120,6 +120,61 @@ IsDerEncodeCertificate ( }
>
> /**
> + This code checks if the file content complies with
> +EFI_VARIABLE_AUTHENTICATION_2 format The function reads file content
> but won't open/close given FileHandle.
> +
> + @param[in] FileHandle The FileHandle to be checked
> +
> + @retval TRUE The content is EFI_VARIABLE_AUTHENTICATION_2
> format.
> + @retval FALSE The content is NOT a
> EFI_VARIABLE_AUTHENTICATION_2 format.
> +
> +**/
> +BOOLEAN
> +IsAuthentication2Format (
> + IN EFI_FILE_HANDLE FileHandle
> +)
> +{
> + EFI_STATUS Status;
> + EFI_VARIABLE_AUTHENTICATION_2 *Auth2;
> + BOOLEAN IsAuth2Format;
> +
> + IsAuth2Format = FALSE;
> +
> + //
> + // Read the whole file content
> + //
> + Status = ReadFileContent(
> + FileHandle,
> + (VOID **) &mImageBase,
> + &mImageSize,
> + 0
> + );
> + if (EFI_ERROR (Status)) {
> + goto ON_EXIT;
> + }
> +
> + Auth2 = (EFI_VARIABLE_AUTHENTICATION_2 *)mImageBase; if
> + (Auth2->AuthInfo.Hdr.wCertificateType != WIN_CERT_TYPE_EFI_GUID) {
> + goto ON_EXIT;
> + }
> +
> + if (CompareGuid(&gEfiCertPkcs7Guid, &Auth2->AuthInfo.CertType)) {
> + IsAuth2Format = TRUE;
> + }
> +
> +ON_EXIT:
> + //
> + // Do not close File. simply check file content
> + //
> + if (mImageBase != NULL) {
> + FreePool (mImageBase);
> + mImageBase = NULL;
> + }
> +
> + return IsAuth2Format;
> +}
> +
> +/**
> Set Secure Boot option into variable space.
>
> @param[in] VarValue The option of Secure Boot.
> @@ -2081,6 +2136,115 @@ HashPeImageByType (
>
> **/
> EFI_STATUS
> +EnrollAuthenication2Descriptor (
> + IN SECUREBOOT_CONFIG_PRIVATE_DATA *Private,
> + IN CHAR16 *VariableName
> + )
> +{
> + EFI_STATUS Status;
> + VOID *Data;
> + UINTN DataSize;
> + UINT32 Attr;
> +
> + Data = NULL;
> +
> + //
> + // DBT only support DER-X509 Cert Enrollment // if (StrCmp
> + (VariableName, EFI_IMAGE_SECURITY_DATABASE2) == 0) {
> + return EFI_UNSUPPORTED;
> + }
> +
> + //
> + // Read the whole file content
> + //
> + Status = ReadFileContent(
> + Private->FileContext->FHandle,
> + (VOID **) &mImageBase,
> + &mImageSize,
> + 0
> + );
> + if (EFI_ERROR (Status)) {
> + goto ON_EXIT;
> + }
> + ASSERT (mImageBase != NULL);
> +
> + Attr = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS
> + | EFI_VARIABLE_BOOTSERVICE_ACCESS |
> + EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
> +
> + //
> + // Check if SigDB variable has been already existed.
> + // If true, use EFI_VARIABLE_APPEND_WRITE attribute to append the //
> + new signature data to original variable // DataSize = 0; Status =
> + gRT->GetVariable(
> + VariableName,
> + &gEfiImageSecurityDatabaseGuid,
> + NULL,
> + &DataSize,
> + NULL
> + );
> + if (Status == EFI_BUFFER_TOO_SMALL) {
> + Attr |= EFI_VARIABLE_APPEND_WRITE;
> + } else if (Status != EFI_NOT_FOUND) {
> + goto ON_EXIT;
> + }
> +
> +
> + DEBUG((DEBUG_ERROR, "DBX update binary %s %x %Attr
> + %x\n",VariableName, mImageSize, Attr)); // // Diretly set
> + AUTHENTICATION_2 data to SetVariable // Status = gRT->SetVariable(
> + VariableName,
> + &gEfiImageSecurityDatabaseGuid,
> + Attr,
> + mImageSize,
> + mImageBase
> + );
> +
> + DEBUG((DEBUG_ERROR, "DBX update binary status %x\n", Status));
> +
> +ON_EXIT:
> +
> + CloseFile (Private->FileContext->FHandle);
> + Private->FileContext->FHandle = NULL;
> +
> + if (Private->FileContext->FileName != NULL){
> + FreePool(Private->FileContext->FileName);
> + Private->FileContext->FileName = NULL; }
> +
> + if (Data != NULL) {
> + FreePool (Data);
> + }
> +
> + if (mImageBase != NULL) {
> + FreePool (mImageBase);
> + mImageBase = NULL;
> + }
> +
> + return Status;
> +
> +}
> +
> +
> +/**
> + Enroll a new executable's signature into Signature Database.
> +
> + @param[in] PrivateData The module's private data.
> + @param[in] VariableName Variable name of signature database, must be
> + EFI_IMAGE_SECURITY_DATABASE,
> EFI_IMAGE_SECURITY_DATABASE1
> + or EFI_IMAGE_SECURITY_DATABASE2.
> +
> + @retval EFI_SUCCESS New signature is enrolled successfully.
> + @retval EFI_INVALID_PARAMETER The parameter is invalid.
> + @retval EFI_UNSUPPORTED Unsupported command.
> + @retval EFI_OUT_OF_RESOURCES Could not allocate needed resources.
> +
> +**/
> +EFI_STATUS
> EnrollImageSignatureToSigDB (
> IN SECUREBOOT_CONFIG_PRIVATE_DATA *Private,
> IN CHAR16 *VariableName
> @@ -2305,10 +2469,12 @@ EnrollSignatureDatabase (
> // Supports DER-encoded X509 certificate.
> //
> return EnrollX509toSigDB (Private, VariableName);
> + } else if (IsAuthentication2Format(Private->FileContext->FHandle)){
> + return EnrollAuthenication2Descriptor(Private, VariableName); }
> + else {
> + return EnrollImageSignatureToSigDB (Private, VariableName);
> }
> -
> - return EnrollImageSignatureToSigDB (Private, VariableName); -}
> +}
>
> /**
> List all signatures in specified signature database (e.g. KEK/DB/DBX/DBT)
> @@ -2957,6 +3123,7 @@ SecureBootExtractConfigFromVariable (
> // Initilize the Date and Time using system time.
> //
> ConfigData->CertificateFormat = HASHALG_RAW;
> + ConfigData->FileEnrollType = UNKNOWN_FILE_TYPE;
> ConfigData->AlwaysRevocation = TRUE;
> gRT->GetTime (&CurrTime, NULL);
> ConfigData->RevocationDate.Year = CurrTime.Year;
> @@ -3258,6 +3425,8 @@ SecureBootCallback (
> UINT8 *SetupMode;
> CHAR16 PromptString[100];
> EFI_DEVICE_PATH_PROTOCOL *File;
> + UINTN NameLength;
> + UINT16 *FilePostFix;
>
> Status = EFI_SUCCESS;
> SecureBootEnable = NULL;
> @@ -3393,6 +3562,27 @@ SecureBootCallback (
>
> case SECUREBOOT_ENROLL_SIGNATURE_TO_DBX:
> ChooseFile (NULL, NULL, UpdateDBXFromFile, &File);
> + //
> + // Parse the file's postfix.
> + //
> + NameLength = StrLen (Private->FileContext->FileName);
> + if (NameLength <= 4) {
> + return FALSE;
> + }
> + FilePostFix = Private->FileContext->FileName + NameLength - 4;
> +
> + if (IsDerEncodeCertificate (FilePostFix)) {
> + //
> + // Supports DER-encoded X509 certificate.
> + //
> + IfrNvData->FileEnrollType = X509_CERT_FILE_TYPE;
> + } else if (IsAuthentication2Format(gSecureBootPrivateData-
> >FileContext->FHandle)){
> + IfrNvData->FileEnrollType = AUTHENCIATION_2_FILE_TYPE;
> + } else {
> + IfrNvData->FileEnrollType = PE_IMAGE_FILE_TYPE;
> + }
> + DEBUG((DEBUG_ERROR, "IfrNvData->FileEnrollType %d\n", IfrNvData-
> >FileEnrollType));
> + HiiSetBrowserData(&gSecureBootConfigFormSetGuid,
> + mSecureBootStorageName, sizeof
> (SECUREBOOT_CONFIGURATION),(UINT8
> + *)IfrNvData, NULL);
> break;
>
> case SECUREBOOT_ENROLL_SIGNATURE_TO_DBT:
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figImpl.h
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figImpl.h
> index bea9470..f9b75e6 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figImpl.h
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
> +++ nfigImpl.h
> @@ -47,6 +47,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
> #include <Guid/FileSystemVolumeLabelInfo.h>
> #include <Guid/ImageAuthentication.h>
> #include <Guid/FileInfo.h>
> +#include <Guid/WinCertificate.h>
>
> #include "SecureBootConfigNvData.h"
>
> @@ -582,4 +583,35 @@ UpdateDBTFromFile (
> IN EFI_DEVICE_PATH_PROTOCOL *FilePath
> );
>
> +/**
> + This code checks if the FileSuffix is one of the possible DER-encoded
> certificate suffix.
> +
> + @param[in] FileSuffix The suffix of the input certificate file
> +
> + @retval TRUE It's a DER-encoded certificate.
> + @retval FALSE It's NOT a DER-encoded certificate.
> +
> +**/
> +BOOLEAN
> +IsDerEncodeCertificate (
> + IN CONST CHAR16 *FileSuffix
> +);
> +
> +
> +/**
> + This code checks if the file content complies with
> +EFI_VARIABLE_AUTHENTICATION_2 format The function reads file content
> but won't open/close given FileHandle.
> +
> + @param[in] FileHandle The FileHandle to be checked
> +
> + @retval TRUE The content is EFI_VARIABLE_AUTHENTICATION_2
> format.
> + @retval FALSE The content is NOT a
> EFI_VARIABLE_AUTHENTICATION_2 format.
> +
> +**/
> +BOOLEAN
> +IsAuthentication2Format (
> + IN EFI_FILE_HANDLE FileHandle
> +);
> +
> +
> #endif
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figNvData.h
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figNvData.h
> index df4d72e..c3dc92c 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figNvData.h
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
> +++ nfigNvData.h
> @@ -107,6 +107,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
> #define SECURE_BOOT_GUID_SIZE 36
> #define SECURE_BOOT_GUID_STORAGE_SIZE 37
>
> +#define UNKNOWN_FILE_TYPE 0
> +#define X509_CERT_FILE_TYPE 1
> +#define AUTHENCIATION_2_FILE_TYPE 2
> +#define PE_IMAGE_FILE_TYPE 3
>
> //
> // Nv Data structure referenced by IFR
> @@ -123,6 +127,7 @@ typedef struct {
> UINT8 CertificateFormat; // The type of the certificate
> EFI_HII_DATE RevocationDate; // The revocation date of the certificate
> EFI_HII_TIME RevocationTime; // The revocation time of the certificate
> + UINT8 FileEnrollType; // File type of sigunature enroll
> } SECUREBOOT_CONFIGURATION;
>
> #endif
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figStrings.uni
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figStrings.uni
> index af6d83b..96a02b3 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figStrings.uni
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
> +++ nfigStrings.uni
> @@ -35,10 +35,15 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
>
> #string STR_DBX_CERTIFICATE_FORMAT_PROMPT #language en-US
> "Signature Format"
> #string STR_DBX_CERTIFICATE_FORMAT_HELP #language en-US "Select
> the certificate format used to enroll certificate into database."
> -#string STR_DBX_CERTIFICATE_FORMAT_SHA256 #language en-US
> "SHA256"
> -#string STR_DBX_CERTIFICATE_FORMAT_SHA384 #language en-US
> "SHA384"
> -#string STR_DBX_CERTIFICATE_FORMAT_SHA512 #language en-US
> "SHA512"
> -#string STR_DBX_CERTIFICATE_FORMAT_RAW #language en-US "RAW"
> +#string STR_DBX_CERTIFICATE_FORMAT_SHA256 #language en-US "X509
> CERT SHA256"
> +#string STR_DBX_CERTIFICATE_FORMAT_SHA384 #language en-US "X509
> CERT SHA384"
> +#string STR_DBX_CERTIFICATE_FORMAT_SHA512 #language en-US "X509
> CERT SHA512"
> +#string STR_DBX_CERTIFICATE_FORMAT_RAW #language en-US "X509
> CERT"
> +
> +#string STR_DBX_PE_FORMAT_SHA256 #language en-US "PE Image
> SHA256"
> +
> +#string STR_DBX_AUTH_2_FORMAT #language en-US
> "VARIABLE_AUTHENICATION_2"
> +
>
> #string STR_CERTIFICATE_REVOCATION_TIME_PROMPT #language en-US "
> Revocation Time"
> #string STR_CERTIFICATE_REVOCATION_TIME_HELP #language en-US
> "Input the revocation time of the certificate"
> --
> 1.9.5.msysgit.1
next prev parent reply other threads:[~2017-03-30 8:26 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-30 2:00 [PATCH] SecureBoot UI Update Zhang, Chao B
2017-03-30 8:26 ` Long, Qin [this message]
2017-03-30 16:31 ` Laszlo Ersek
2017-03-31 5:11 ` Zhang, Chao B
2017-03-31 8:47 ` Laszlo Ersek
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=BF2CCE9263284D428840004653A28B6E53F7E468@SHSMSX103.ccr.corp.intel.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