From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 03C662117AE67 for ; Thu, 18 Oct 2018 14:45:48 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3D30D3082AF0; Thu, 18 Oct 2018 21:45:48 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-121-50.rdu2.redhat.com [10.10.121.50]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4AAA15D75E; Thu, 18 Oct 2018 21:45:45 +0000 (UTC) From: Laszlo Ersek To: edk2-devel@lists.01.org Cc: Star Zeng , Prasad Pandit , Jiewen Yao , Chao Zhang , "Leif Lindholm (Linaro address)" , Vincent Zimmer , Michael Kinney , Gary Lin , Ard Biesheuvel , Steve McIntyre <93sam@debian.org>, Peter Jones References: <1539657661-57656-1-git-send-email-star.zeng@intel.com> <75a8ff0b-dac9-dbb4-a792-1085a0b73699@redhat.com> Message-ID: <9e54939a-430e-7437-4388-d65f836e926b@redhat.com> Date: Thu, 18 Oct 2018 23:45:44 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <75a8ff0b-dac9-dbb4-a792-1085a0b73699@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.45]); Thu, 18 Oct 2018 21:45:48 +0000 (UTC) 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: Thu, 18 Oct 2018 21:45:49 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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. (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