From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 95F6221A0482E for ; Thu, 30 Mar 2017 22:11:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=intel; t=1490937081; x=1522473081; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=FphVlOu9eC1Dk0gRI9kiODmvPTM48N1xxQ8cISEaIbo=; b=NZ+By5OtOfn0TJACQVRaFZLYL46dWxqoKribF5axD9MhCcuOsqf70Ukt Hii2gsxg+NN/ZZjBR2BlNhgEPdi4bQ==; Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 30 Mar 2017 22:11:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,250,1486454400"; d="scan'208";a="840283699" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by FMSMGA003.fm.intel.com with ESMTP; 30 Mar 2017 22:11:20 -0700 Received: from fmsmsx116.amr.corp.intel.com (10.18.116.20) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 30 Mar 2017 22:11:20 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx116.amr.corp.intel.com (10.18.116.20) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 30 Mar 2017 22:11:20 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.212]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.46]) with mapi id 14.03.0248.002; Fri, 31 Mar 2017 13:11:18 +0800 From: "Zhang, Chao B" To: Laszlo Ersek , "edk2-devel@lists.01.org" CC: "Fu, Siyuan" , "Long, Qin" Thread-Topic: [edk2] [PATCH] SecureBoot UI Update Thread-Index: AQHSqPl8/KbxyRmvN0StMIE0Z/NZ7qGtDgyAgAFZjAA= Date: Fri, 31 Mar 2017 05:11:17 +0000 Message-ID: References: <20170330020045.21452-1-chao.b.zhang@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] SecureBoot UI Update X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 31 Mar 2017 05:11:21 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Laszlo: I checked my patch. It did contains such info. I use index 1, 2 in fron= t of some comment line.=20 Not sure it is the reason why these info are filtered. I will send patch a= gain. =20 -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Lasz= lo Ersek Sent: Friday, March 31, 2017 12:31 AM To: Zhang, Chao B ; edk2-devel@lists.01.org Cc: Fu, Siyuan ; Long, Qin Subject: Re: [edk2] [PATCH] SecureBoot UI Update On 03/30/17 04:00, Zhang, Chao B wrote: > --- > .../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(-) (I hope this patch has not been committed yet.) Can you guys please describe the UI update in the commit message? The commi= t message is useless in this form. What is being changed? And why? Worse, there is no Contributed-under line, and no Signed-off-by line. This patch does not conform to the edk2 contribution rules. Thanks Laszlo >=20 > diff --git=20 > a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf > ig.vfr=20 > b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf > ig.vfr > index 02ddf4a..e153eca 100644 > ---=20 > a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf > ig.vfr > +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBoot > +++ Config.vfr > @@ -455,15 +455,35 @@ formset > maxsize =3D SECURE_BOOT_GUID_SIZE, > endstring; > =20 > - oneof name =3D SignatureFormatInDbx, > - varid =3D SECUREBOOT_CONFIGURATION.CertificateFormat, > - prompt =3D STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_PROMPT= ), > - help =3D STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_HELP), > - option text =3D STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA256= ), value =3D 0x2, flags =3D DEFAULT; > - option text =3D STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA384= ), value =3D 0x3, flags =3D 0; > - option text =3D STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA512= ), value =3D 0x4, flags =3D 0; > - option text =3D STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_RAW), = value =3D 0x5, flags =3D 0; > - endoneof; > + disableif NOT ideqval SECUREBOOT_CONFIGURATION.FileEnrollType =3D=3D= 1; > + oneof name =3D X509SignatureFormatInDbx, > + varid =3D SECUREBOOT_CONFIGURATION.CertificateFormat, > + prompt =3D STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_PROM= PT), > + help =3D STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_HELP= ), > + option text =3D STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA2= 56), value =3D 0x2, flags =3D DEFAULT; > + option text =3D STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA3= 84), value =3D 0x3, flags =3D 0; > + option text =3D STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA5= 12), value =3D 0x4, flags =3D 0; > + option text =3D STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_RAW)= , value =3D 0x5, flags =3D 0; > + endoneof; > + endif; > + > + disableif NOT ideqval SECUREBOOT_CONFIGURATION.FileEnrollType =3D=3D= 2; > + grayoutif TRUE; > + text > + help =3D STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_PROMPT), = // Help string > + text =3D STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_HELP), = // Prompt string > + text =3D STRING_TOKEN(STR_DBX_PE_FORMAT_SHA256); = // TextTwo > + endif; > + endif; > + > + suppressif ideqval SECUREBOOT_CONFIGURATION.FileEnrollType =3D=3D 3; > + grayoutif TRUE; > + text > + help =3D STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_PROMPT), = // Help string > + text =3D STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_HELP), = // Prompt string > + text =3D STRING_TOKEN(STR_DBX_AUTH_2_FORMAT); = // TextTwo > + endif; > + endif; > =20 > suppressif ideqval SECUREBOOT_CONFIGURATION.CertificateFormat =3D=3D= 5; > checkbox varid =3D SECUREBOOT_CONFIGURATION.AlwaysRevocation, > diff --git=20 > a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf > igImpl.c=20 > b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf > igImpl.c > index 6f58729..17fe120 100644 > ---=20 > a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf > igImpl.c > +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBoot > +++ ConfigImpl.c > @@ -120,6 +120,61 @@ IsDerEncodeCertificate ( } > =20 > /** > + This code checks if the file content complies with=20 > +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_AUTHENTICA= TION_2 format. > + > +**/ > +BOOLEAN > +IsAuthentication2Format ( > + IN EFI_FILE_HANDLE FileHandle > +) > +{ > + EFI_STATUS Status; > + EFI_VARIABLE_AUTHENTICATION_2 *Auth2; > + BOOLEAN IsAuth2Format; > + > + IsAuth2Format =3D FALSE; > + > + // > + // Read the whole file content > + // > + Status =3D ReadFileContent( > + FileHandle, > + (VOID **) &mImageBase, > + &mImageSize, > + 0 > + ); > + if (EFI_ERROR (Status)) { > + goto ON_EXIT; > + } > + > + Auth2 =3D (EFI_VARIABLE_AUTHENTICATION_2 *)mImageBase; if=20 > + (Auth2->AuthInfo.Hdr.wCertificateType !=3D WIN_CERT_TYPE_EFI_GUID) { > + goto ON_EXIT; > + } > + > + if (CompareGuid(&gEfiCertPkcs7Guid, &Auth2->AuthInfo.CertType)) { > + IsAuth2Format =3D TRUE; > + } > + > +ON_EXIT: > + // > + // Do not close File. simply check file content > + // > + if (mImageBase !=3D NULL) { > + FreePool (mImageBase); > + mImageBase =3D NULL; > + } > + > + return IsAuth2Format; > +} > + > +/** > Set Secure Boot option into variable space. > =20 > @param[in] VarValue The option of Secure Boot. > @@ -2081,6 +2136,115 @@ HashPeImageByType ( > =20 > **/ > EFI_STATUS > +EnrollAuthenication2Descriptor ( > + IN SECUREBOOT_CONFIG_PRIVATE_DATA *Private, > + IN CHAR16 *VariableName > + ) > +{ > + EFI_STATUS Status; > + VOID *Data; > + UINTN DataSize; > + UINT32 Attr; > + > + Data =3D NULL; > + > + // > + // DBT only support DER-X509 Cert Enrollment // if (StrCmp=20 > + (VariableName, EFI_IMAGE_SECURITY_DATABASE2) =3D=3D 0) { > + return EFI_UNSUPPORTED; > + } > + > + // > + // Read the whole file content > + // > + Status =3D ReadFileContent( > + Private->FileContext->FHandle, > + (VOID **) &mImageBase, > + &mImageSize, > + 0 > + ); > + if (EFI_ERROR (Status)) { > + goto ON_EXIT; > + } > + ASSERT (mImageBase !=3D NULL); > + > + Attr =3D EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS > + | EFI_VARIABLE_BOOTSERVICE_ACCESS |=20 > + 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 =20 > + // new signature data to original variable // DataSize =3D 0; =20 > + Status =3D gRT->GetVariable( > + VariableName, > + &gEfiImageSecurityDatabaseGuid, > + NULL, > + &DataSize, > + NULL > + ); > + if (Status =3D=3D EFI_BUFFER_TOO_SMALL) { > + Attr |=3D EFI_VARIABLE_APPEND_WRITE; } else if (Status !=3D=20 > + EFI_NOT_FOUND) { > + goto ON_EXIT; > + } > + > + =20 > + DEBUG((DEBUG_ERROR, "DBX update binary %s %x %Attr=20 > + %x\n",VariableName, mImageSize, Attr)); // // Diretly set=20 > + AUTHENTICATION_2 data to SetVariable // Status =3D gRT->SetVariable( > + VariableName, > + &gEfiImageSecurityDatabaseGuid, > + Attr, > + mImageSize, > + mImageBase > + ); > + > + DEBUG((DEBUG_ERROR, "DBX update binary status %x\n", Status)); > + > +ON_EXIT: > + > + CloseFile (Private->FileContext->FHandle); =20 > + Private->FileContext->FHandle =3D NULL; > + > + if (Private->FileContext->FileName !=3D NULL){ > + FreePool(Private->FileContext->FileName); > + Private->FileContext->FileName =3D NULL; } > + > + if (Data !=3D NULL) { > + FreePool (Data); > + } > + > + if (mImageBase !=3D NULL) { > + FreePool (mImageBase); > + mImageBase =3D 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 b= e > + EFI_IMAGE_SECURITY_DATABASE, EFI_IMAGE_SECU= RITY_DATABASE1 > + or EFI_IMAGE_SECURITY_DATABASE2. > + > + @retval EFI_SUCCESS New signature is enrolled successfull= y. > + @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); }=20 > + else { > + return EnrollImageSignatureToSigDB (Private, VariableName); > } > - > - return EnrollImageSignatureToSigDB (Private, VariableName); -} > +} > =20 > /** > List all signatures in specified signature database (e.g.=20 > KEK/DB/DBX/DBT) @@ -2957,6 +3123,7 @@ SecureBootExtractConfigFromVariable= ( > // Initilize the Date and Time using system time. > // > ConfigData->CertificateFormat =3D HASHALG_RAW; > + ConfigData->FileEnrollType =3D UNKNOWN_FILE_TYPE; > ConfigData->AlwaysRevocation =3D TRUE; > gRT->GetTime (&CurrTime, NULL); > ConfigData->RevocationDate.Year =3D CurrTime.Year; > @@ -3258,6 +3425,8 @@ SecureBootCallback ( > UINT8 *SetupMode; > CHAR16 PromptString[100]; > EFI_DEVICE_PATH_PROTOCOL *File; > + UINTN NameLength; > + UINT16 *FilePostFix; > =20 > Status =3D EFI_SUCCESS; > SecureBootEnable =3D NULL; > @@ -3393,6 +3562,27 @@ SecureBootCallback ( > =20 > case SECUREBOOT_ENROLL_SIGNATURE_TO_DBX: > ChooseFile (NULL, NULL, UpdateDBXFromFile, &File); > + // > + // Parse the file's postfix. > + // > + NameLength =3D StrLen (Private->FileContext->FileName); > + if (NameLength <=3D 4) { > + return FALSE; > + } > + FilePostFix =3D Private->FileContext->FileName + NameLength - 4; > + > + if (IsDerEncodeCertificate (FilePostFix)) { > + // > + // Supports DER-encoded X509 certificate. > + // > + IfrNvData->FileEnrollType =3D X509_CERT_FILE_TYPE; > + } else if (IsAuthentication2Format(gSecureBootPrivateData->FileCon= text->FHandle)){ > + IfrNvData->FileEnrollType =3D AUTHENCIATION_2_FILE_TYPE; > + } else { > + IfrNvData->FileEnrollType =3D PE_IMAGE_FILE_TYPE; > + } > + DEBUG((DEBUG_ERROR, "IfrNvData->FileEnrollType %d\n", IfrNvData->F= ileEnrollType)); > + HiiSetBrowserData(&gSecureBootConfigFormSetGuid,=20 > + mSecureBootStorageName, sizeof (SECUREBOOT_CONFIGURATION),(UINT8=20 > + *)IfrNvData, NULL); > break; > =20 > case SECUREBOOT_ENROLL_SIGNATURE_TO_DBT: > diff --git=20 > a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf > igImpl.h=20 > b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf > igImpl.h > index bea9470..f9b75e6 100644 > ---=20 > a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf > igImpl.h > +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBoot > +++ ConfigImpl.h > @@ -47,6 +47,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITH= ER EXPRESS OR IMPLIED. > #include > #include #include > +#include > =20 > #include "SecureBootConfigNvData.h" > =20 > @@ -582,4 +583,35 @@ UpdateDBTFromFile ( > IN EFI_DEVICE_PATH_PROTOCOL *FilePath > ); > =20 > +/** > + This code checks if the FileSuffix is one of the possible DER-encoded = certificate suffix. > + > + @param[in] FileSuffix The suffix of the input certificate f= ile > + > + @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=20 > +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_AUTHENTICA= TION_2 format. > + > +**/ > +BOOLEAN > +IsAuthentication2Format ( > + IN EFI_FILE_HANDLE FileHandle > +); > + > + > #endif > diff --git=20 > a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf > igNvData.h=20 > b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf > igNvData.h > index df4d72e..c3dc92c 100644 > ---=20 > a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf > igNvData.h > +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBoot > +++ ConfigNvData.h > @@ -107,6 +107,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, E= ITHER EXPRESS OR IMPLIED. > #define SECURE_BOOT_GUID_SIZE 36 > #define SECURE_BOOT_GUID_STORAGE_SIZE 37 > =20 > +#define UNKNOWN_FILE_TYPE 0 > +#define X509_CERT_FILE_TYPE 1 > +#define AUTHENCIATION_2_FILE_TYPE 2 > +#define PE_IMAGE_FILE_TYPE 3 > =20 > // > // Nv Data structure referenced by IFR @@ -123,6 +127,7 @@ typedef=20 > 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=20 > certificate > + UINT8 FileEnrollType; // File type of sigunature enroll > } SECUREBOOT_CONFIGURATION; > =20 > #endif > diff --git=20 > a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf > igStrings.uni=20 > b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf > igStrings.uni > index af6d83b..96a02b3 100644 > ---=20 > a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf > igStrings.uni > +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBoot > +++ ConfigStrings.uni > @@ -35,10 +35,15 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EI= THER EXPRESS OR IMPLIED. > =20 > #string STR_DBX_CERTIFICATE_FORMAT_PROMPT #language en-US "Signature Fo= rmat" > #string STR_DBX_CERTIFICATE_FORMAT_HELP #language en-US "Select the c= ertificate 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 SH= A256" > +#string STR_DBX_CERTIFICATE_FORMAT_SHA384 #language en-US "X509 CERT SH= A384" > +#string STR_DBX_CERTIFICATE_FORMAT_SHA512 #language en-US "X509 CERT SH= A512" > +#string STR_DBX_CERTIFICATE_FORMAT_RAW #language en-US "X509 CERT" > + > +#string STR_DBX_PE_FORMAT_SHA256 #language en-US "PE Image SHA= 256" > + > +#string STR_DBX_AUTH_2_FORMAT #language en-US "VARIABLE_AUT= HENICATION_2" > + > =20 > #string STR_CERTIFICATE_REVOCATION_TIME_PROMPT #language en-US " Revoca= tion Time" > #string STR_CERTIFICATE_REVOCATION_TIME_HELP #language en-US "Input th= e revocation time of the certificate" >=20 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel