System may break during time based AuthVariable update, causing certdb inconsistent. 2 ways are used to ensure update atomic. 1. Delete cert in certdb after variable is deleted 2. Clean up certdb on variable initialization
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chao Zhang <chao.b.zh...@intel.com> --- SecurityPkg/Library/AuthVariableLib/AuthService.c | 151 ++++++++++++++++++--- .../Library/AuthVariableLib/AuthServiceInternal.h | 16 +++ .../Library/AuthVariableLib/AuthVariableLib.c | 9 ++ 3 files changed, 154 insertions(+), 22 deletions(-) diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c b/SecurityPkg/Library/AuthVariableLib/AuthService.c index b3e8933..aa23c88 100644 --- a/SecurityPkg/Library/AuthVariableLib/AuthService.c +++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c @@ -1965,6 +1965,101 @@ InsertCertsToDb ( } /** + Clean up signer's certificates for common authenticated variable + by corresponding VariableName and VendorGuid from "certdb". + Sytem may break down during Timebased Variable update & certdb update, + make them inconsistent, this function is called in AuthVariable Init to ensure + consistency + + @retval EFI_NOT_FOUND Fail to find matching certs. + @retval EFI_SUCCESS Find matching certs and output parameters. + +**/ +EFI_STATUS +CleanCertsFromDb ( + VOID + ){ + UINT32 Offset; + AUTH_CERT_DB_DATA *Ptr; + UINT32 NameSize; + UINT32 NodeSize; + CHAR16 *VariableName; + EFI_STATUS Status; + BOOLEAN CertCleaned; + UINT8 *Data; + UINTN DataSize; + UINT8 *AuthVarData; + UINTN AuthVarDataSize; + + CertCleaned = FALSE; + Status = EFI_SUCCESS; + + // + // Get corresponding certificates by VendorGuid and VariableName. + // + while (CertCleaned) { + CertCleaned = FALSE; + + // + // Get latest variable "certdb" + // + Status = AuthServiceInternalFindVariable ( + EFI_CERT_DB_NAME, + &gEfiCertDbGuid, + (VOID **) &Data, + &DataSize + ); + if (EFI_ERROR (Status)) { + return Status; + } + + if ((DataSize == 0) || (Data == NULL)) { + ASSERT (FALSE); + return EFI_NOT_FOUND; + } + + Offset = sizeof (UINT32); + + while (Offset < (UINT32) DataSize) { + Ptr = (AUTH_CERT_DB_DATA *) (Data + Offset); + // + // Check whether VendorGuid matches. + // + NodeSize = ReadUnaligned32 (&Ptr->CertNodeSize); + NameSize = ReadUnaligned32 (&Ptr->NameSize); + + VariableName = AllocateZeroPool((NameSize + 1) * sizeof(CHAR16)); + if (VariableName == NULL) { + return EFI_OUT_OF_RESOURCES; + } + CopyMem (VariableName, (UINT8 *) Ptr + sizeof (AUTH_CERT_DB_DATA), NameSize * sizeof(CHAR16)); + // + // Find corresponding time auth variable + // + Status = AuthServiceInternalFindVariable ( + VariableName, + &Ptr->VendorGuid, + (VOID **) &AuthVarData, + &AuthVarDataSize + ); + + if (EFI_ERROR(Status)) { + Status = DeleteCertsFromDb(VariableName, &Ptr->VendorGuid); + CertCleaned = TRUE; + DEBUG((EFI_D_INFO, "Recovery!! Cert for Auth Variable %s Guid %g is removed for consistency\n", VariableName, Ptr->VendorGuid)); + FreePool(VariableName); + break; + } + + FreePool(VariableName); + Offset = Offset + NodeSize; + } + } + + return Status; +} + +/** Process variable with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS set Caution: This function may receive untrusted input. @@ -2285,16 +2380,7 @@ VerifyTimeBasedPayload ( goto Exit; } - // - // Delete signer's certificates when delete the common authenticated variable. - // - if ((PayloadSize == 0) && (OrgTimeStamp != NULL) && ((Attributes & EFI_VARIABLE_APPEND_WRITE) == 0)) { - Status = DeleteCertsFromDb (VariableName, VendorGuid); - if (EFI_ERROR (Status)) { - VerifyStatus = FALSE; - goto Exit; - } - } else if ((OrgTimeStamp == NULL) && (PayloadSize != 0)) { + if ((OrgTimeStamp == NULL) && (PayloadSize != 0)) { // // Insert signer's certificates when adding a new common authenticated variable. // @@ -2412,21 +2498,42 @@ VerifyTimeBasedPayloadAndUpdate ( return Status; } - if ((PayloadSize == 0) && (VarDel != NULL)) { - *VarDel = TRUE; - } - CertData = (EFI_VARIABLE_AUTHENTICATION_2 *) Data; // // Final step: Update/Append Variable if it pass Pkcs7Verify // - return AuthServiceInternalUpdateVariableWithTimeStamp ( - VariableName, - VendorGuid, - PayloadPtr, - PayloadSize, - Attributes, - &CertData->TimeStamp - ); + Status = AuthServiceInternalUpdateVariableWithTimeStamp ( + VariableName, + VendorGuid, + PayloadPtr, + PayloadSize, + Attributes, + &CertData->TimeStamp + ); + + // + // Delete signer's certificates when delete the common authenticated variable. + // + if (EFI_ERROR(Status) + && (AuthVarType == AuthVarTypePriv) + && (PayloadSize == 0) + && !EFI_ERROR(FindStatus) + && ((Attributes & EFI_VARIABLE_APPEND_WRITE) == 0)) { + Status = DeleteCertsFromDb (VariableName, VendorGuid); + } + + + if ((VarDel != NULL)) { + if (!EFI_ERROR(Status) + && (PayloadSize == 0) + && !EFI_ERROR(FindStatus) + && ((Attributes & EFI_VARIABLE_APPEND_WRITE) == 0) ) { + *VarDel = TRUE; + } else { + *VarDel = FALSE; + } + } + + return Status; } diff --git a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h index 08fff3f..add05c2 100644 --- a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h +++ b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h @@ -187,6 +187,22 @@ DeleteCertsFromDb ( ); /** + Clean up signer's certificates for common authenticated variable + by corresponding VariableName and VendorGuid from "certdb". + Sytem may break down during Timebased Variable update & certdb update, + make them inconsistent, this function is called in AuthVariable Init to ensure + consistency + + @retval EFI_NOT_FOUND Fail to find matching certs. + @retval EFI_SUCCESS Find matching certs and output parameters. + +**/ +EFI_STATUS +CleanCertsFromDb ( + VOID + ); + +/** Filter out the duplicated EFI_SIGNATURE_DATA from the new data by comparing to the original data. @param[in] Data Pointer to original EFI_SIGNATURE_LIST. diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c index 0bb0918..02df309 100644 --- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c +++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c @@ -352,6 +352,15 @@ AuthVariableLibInitialize ( if (EFI_ERROR (Status)) { return Status; } + } else { + // + // Clean up Certs to make certDB & Time based auth variable consistent + // + Status = CleanCertsFromDb(); + if (EFI_ERROR (Status)) { + DEBUG ((EFI_D_INFO, "Clean up CertDB fail! Status %x\n", Status)); + return Status; + } } // -- 1.9.5.msysgit.1 ------------------------------------------------------------------------------ Don't Limit Your Business. Reach for the Cloud. GigeNET's Cloud Solutions provide you with the tools and support that you need to offload your IT needs and focus on growing your business. Configured For All Businesses. Start Your Cloud Today. https://www.gigenetcloud.com/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel