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 <jiewen....@intel.com>
Cc: Chao Zhang <chao.b.zh...@intel.com>
Cc: Jian J Wang <jian.j.w...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.z...@intel.com>
---
  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


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to