From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.88; helo=mga01.intel.com; envelope-from=star.zeng@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (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 948D321168203 for ; Fri, 19 Oct 2018 00:09:45 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 Oct 2018 00:09:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,399,1534834800"; d="scan'208";a="83864813" Received: from shzintpr02.sh.intel.com (HELO [10.7.209.51]) ([10.239.4.160]) by orsmga006.jf.intel.com with ESMTP; 19 Oct 2018 00:09:42 -0700 To: Laszlo Ersek , edk2-devel@lists.01.org Cc: Vincent Zimmer , Prasad Pandit , Steve McIntyre <93sam@debian.org>, Peter Jones , Jiewen Yao , Michael Kinney , Gary Lin , Chao Zhang , qin.long@intel.com References: <1539657661-57656-1-git-send-email-star.zeng@intel.com> <75a8ff0b-dac9-dbb4-a792-1085a0b73699@redhat.com> <9e54939a-430e-7437-4388-d65f836e926b@redhat.com> From: "Zeng, Star" Message-ID: <6903b57e-4547-723a-f21d-89c500df7801@intel.com> Date: Fri, 19 Oct 2018 15:09:11 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <9e54939a-430e-7437-4388-d65f836e926b@redhat.com> Subject: Re: CVE-2018-3613 [was: MdeModulePkg Variable: Fix Timestamp zeroing issue on APPEND_WRITE] X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Oct 2018 07:09:45 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Laszlo, Cc Qin also. Qin and Chao are secure boot experts, I also had some talk with them. On 2018/10/19 5:45, Laszlo Ersek wrote: > Hi All, > > On 10/16/18 04:41, Star Zeng wrote: >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=415 >> >> When SetVariable() to a time based auth variable with APPEND_WRITE >> attribute, and if the EFI_VARIABLE_AUTHENTICATION_2.TimeStamp in >> the input Data is earlier than current value, it will cause timestamp >> zeroing. >> >> This issue may bring time based auth variable downgrade problem. >> For example: >> A vendor released three certs at 2014, 2015, and 2016, and system >> integrated the 2016 cert. User can SetVariable() with 2015 cert and >> APPEND_WRITE attribute to cause timestamp zeroing first, then >> SetVariable() with 2014 cert to downgrade the cert. >> >> This patch fixes this issue. >> >> Cc: Jiewen Yao >> Cc: Chao Zhang >> Cc: Jian J Wang >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Star Zeng >> --- >> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c >> index a2d61c8cd618..8e8db71bd201 100644 >> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c >> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c >> @@ -2462,6 +2462,8 @@ UpdateVariable ( >> if (Variable->CurrPtr != NULL) { >> if (VariableCompareTimeStampInternal (&(((AUTHENTICATED_VARIABLE_HEADER *) CacheVariable->CurrPtr)->TimeStamp), TimeStamp)) { >> CopyMem (&AuthVariable->TimeStamp, TimeStamp, sizeof (EFI_TIME)); >> + } else { >> + CopyMem (&AuthVariable->TimeStamp, &(((AUTHENTICATED_VARIABLE_HEADER *) CacheVariable->CurrPtr)->TimeStamp), sizeof (EFI_TIME)); >> } >> } >> } >> > > I believe I found a significant mitigating factor for this > vulnerability. Very good analysis, I totally agree. :) Yes, if the dbx signature(includes the "attribute" information) was generated with "APPEND" attribute (that is the case you are seeing), it's infeasible to apply the downgrade write since the signature includes the "attribute" information, the PKCS7 verification will block the direct write without "APPEND" attribute. But there may be some initial dbx signature was generated without "APPEND" attribute. E.g. OEM may have some this kind of dbx. It should be rarely case, but I am not sure about that. Another, similar situation is also for other authenticated variables (not PK/KEK/DB/DBX/DBT). Thanks, Star > > (i) I tried to reproduce the issue (with this patch reverted). I indeed > managed to trigger the "timestamp zeroed" case, by *appending* a > hypothetical "2015" DBX update, to a virtual system that had the "2016" > DBX update installed first. > > However, when I tried to replay the hypothetical "2014" DBX update on > top, by *writing* it (not appending it), I found that it wouldn't work: > > > (ii) I confirmed that the timestamp check was in fact circumvented, due > to the zeroing above. That is, the following code snippet from > VerifyTimeBasedPayload() would *not* fire: > > [SecurityPkg/Library/AuthVariableLib/AuthService.c] > > 1869 if ((OrgTimeStamp != NULL) && ((Attributes & EFI_VARIABLE_APPEND_WRITE) == 0)) { > 1870 if (AuthServiceInternalCompareTimeStamp (&CertData->TimeStamp, OrgTimeStamp)) { > 1871 // > 1872 // TimeStamp check fail, suspicious replay attack, return EFI_SECURITY_VIOLATION. > 1873 // > 1874 return EFI_SECURITY_VIOLATION; > 1875 } > 1876 } > > (Line numbers correspond to commit 3a0329bed2a2). > > Yet the replay attempt was rejected. Why? > > > (iii) It was rejected on the following call chain: > > VariableServiceSetVariable() [MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c] > AuthVariableLibProcessVariable() [SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c] > ProcessVarWithKek() [SecurityPkg/Library/AuthVariableLib/AuthService.c] > VerifyTimeBasedPayloadAndUpdate() [SecurityPkg/Library/AuthVariableLib/AuthService.c] > VerifyTimeBasedPayload() [SecurityPkg/Library/AuthVariableLib/AuthService.c] > Pkcs7Verify() [CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c] > > [SecurityPkg/Library/AuthVariableLib/AuthService.c] > > 2032 // > 2033 // Ready to verify Pkcs7 SignedData. Go through KEK Signature Database to find out X.509 CertList. > 2034 // > 2035 KekDataSize = (UINT32) DataSize; > 2036 CertList = (EFI_SIGNATURE_LIST *) Data; > 2037 while ((KekDataSize > 0) && (KekDataSize >= CertList->SignatureListSize)) { > 2038 if (CompareGuid (&CertList->SignatureType, &gEfiCertX509Guid)) { > 2039 Cert = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertList + sizeof (EFI_SIGNATURE_LIST) + CertList->SignatureHeaderSize); > 2040 CertCount = (CertList->SignatureListSize - sizeof (EFI_SIGNATURE_LIST) - CertList->SignatureHeaderSize) / CertList->SignatureSize; > 2041 for (Index = 0; Index < CertCount; Index++) { > 2042 // > 2043 // Iterate each Signature Data Node within this CertList for a verify > 2044 // > 2045 TrustedCert = Cert->SignatureData; > 2046 TrustedCertSize = CertList->SignatureSize - (sizeof (EFI_SIGNATURE_DATA) - 1); > 2047 > 2048 // > 2049 // Verify Pkcs7 SignedData via Pkcs7Verify library. > 2050 // > 2051 VerifyStatus = Pkcs7Verify ( > 2052 SigData, > 2053 SigDataSize, > 2054 TrustedCert, > 2055 TrustedCertSize, > 2056 NewData, > 2057 NewDataSize > 2058 ); > 2059 if (VerifyStatus) { > 2060 goto Exit; > 2061 } > 2062 Cert = (EFI_SIGNATURE_DATA *) ((UINT8 *) Cert + CertList->SignatureSize); > 2063 } > 2064 } > 2065 KekDataSize -= CertList->SignatureListSize; > 2066 CertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList + CertList->SignatureListSize); > 2067 } > > The Pkcs7Verify() call on line 2051 would never return TRUE, even though > the CA certificate against which the "2014" DBX update had been released > was in KEK. > > So I hexdumped SigData, TrustedCert, and NewData, and compared them > against a (successful) *append* call. > > The difference is a single bit in NewData, and Pkcs7Verify() is right to > reject it. > > > (iv) It is explained in the UEFI-2.7 spec, section "8.2.2 Using the > EFI_VARIABLE_AUTHENTICATION_2 descriptor": > >> A caller that invokes the SetVariable() service with the >> EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS attribute set shall >> do the following prior to invoking the service: >> >> [...] >> >> 2. Hash the serialization of the values of the VariableName, >> VendorGuid and Attributes parameters of the SetVariable() call and >> the TimeStamp component of the EFI_VARIABLE_AUTHENTICATION_2 >> descriptor followed by the variable's new value (i.e. the Data >> parameter's new variable content). That is, digest = hash >> (VariableName, VendorGuid, Attributes, TimeStamp, >> DataNew_variable_content). [...] >> >> 3. Sign the resulting digest using a selected signature scheme (e.g. >> PKCS #1 v1.5) >> >> 4. Construct a DER-encoded PKCS #7 version 1.5 SignedData (see >> [RFC2315]) with the signed content as follows: >> >> [...] >> >> 5. Set AuthInfo.CertData to the DER-encoded PKCS #7 SignedData value. >> >> 6. Construct Data parameter: Construct the SetVariable()'s Data >> parameter by concatenating the complete, serialized >> EFI_VARIABLE_AUTHENTICATION_2 descriptor with the new value of the >> variable (DataNew_variable_content). > > Note that in step 2, the *Attributes* parameter of the SetVariable() > call is an input to the hash, and so it is an input to the signature > that the vendor places into the EFI_VARIABLE_AUTHENTICATION_2 > descriptor's AuthInfo field in step 5. > > In other words, when the vendor signs the DBX update, they sign the > expected Attributes value as well. > > > (v) Now compare the verification side: > >> Firmware that implements the SetVariable() service and supports the >> EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS attribute shall do >> the following in response to being called: >> >> [...] >> >> 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. > > That is, when recomputing the digest, the Attributes parameter of the > replay attempt is hashed *afresh* with the rest of the payload, and the > new digest is verified against the vendor's. Consider what the full > payload comprises, for the purpose of hashing: > - VariableName: fixed (otherwise the attack doesn't qualify as "replay") > - VendorGuid: fixed (ditto) > - Attributes: applied afresh, from the call parameters! > - TimeStamp: part of the vendor-released blob (from the auth descriptor) > - DataNew_variable_content: part of the vendor-released blob > > > >------------->----------->------------->---------v---------------< > | | | | | | > ^ ^ ^ ^ v ^ > | VariableName | VendorGuid | Attributes | TimeStamp | Signature | New Var Content | > | | | | > | +--- auth descriptor ---+ | > | | | > | +------------- vendor blob ---------------+ > | | > +----------------------------------- call params ----------------------------------+ > > > (vi) In practice, a vendor should always release a DBX update with > EFI_VARIABLE_APPEND_WRITE set, in Attributes. This is because Vendor1 > should never intend the user to lose Vendor2's DBX entries, when the > user applies Vendor1's DBX update. This means that, in practice, > Vendor1's signature should always depend on Attributes having > EFI_VARIABLE_APPEND_WRITE set. > > However, exploiting this vulnerability (= the replay of the "2014" DBX > update) depends on EFI_VARIABLE_APPEND_WRITE being *clear*. And when the > "2014" DBX update is replayed like that, then the vendor's signature on > the same "2014" update will no longer match. > > This makes me wonder if this vulnerability is exploitable at all in > practice. Please share your thoughts. > > Thanks > Laszlo >