Anothing ping. Comments/reviews/merge highly appreciated. Thank you, -Jan
Jan Bobek writes: > Ping. Can I get a review and/or some comments on this patch, please? > > Thanks, > -Jan > > Jan Bobek writes: > >> Based on whether the DER-encoded ContentInfo structure is present in >> authenticated SetVariable payload or not, the SHA-256 OID can be >> located at different places. >> >> UEFI specification explicitly states the driver shall support both >> cases, but the old code assumed ContentInfo was not present and >> incorrectly rejected authenticated variable updates when it were >> present. >> >> Cc: Jiewen Yao <jiewen....@intel.com> >> Cc: Jian J Wang <jian.j.w...@intel.com> >> Cc: Min Xu <min.m...@intel.com> >> Signed-off-by: Jan Bobek <jbo...@nvidia.com> >> --- >> .../Library/AuthVariableLib/AuthService.c | 18 +++++++++++------- >> 1 file changed, 11 insertions(+), 7 deletions(-) >> >> diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c >> b/SecurityPkg/Library/AuthVariableLib/AuthService.c >> index 054ee4d1d988..de8baccab410 100644 >> --- a/SecurityPkg/Library/AuthVariableLib/AuthService.c >> +++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c >> @@ -1933,15 +1933,19 @@ VerifyTimeBasedPayload ( >> // .... } >> // The DigestAlgorithmIdentifiers can be used to determine the hash >> algorithm >> // in VARIABLE_AUTHENTICATION_2 descriptor. >> - // This field has the fixed offset (+13) and be calculated based on >> two bytes of length encoding. >> + // This field has the fixed offset (+13) or (+32) based on whether the >> DER-encoded >> + // ContentInfo structure is present or not, and can be calculated >> based on two >> + // bytes of length encoding. >> // >> if ((Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != >> 0) { >> - if (SigDataSize >= (13 + sizeof (mSha256OidValue))) { >> - if (((*(SigData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) || >> - (CompareMem (SigData + 13, &mSha256OidValue, sizeof >> (mSha256OidValue)) != 0)) >> - { >> - return EFI_SECURITY_VIOLATION; >> - } >> + if ( ( (SigDataSize >= (13 + sizeof (mSha256OidValue))) >> + && ( ((*(SigData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) >> + || (CompareMem (SigData + 13, &mSha256OidValue, sizeof >> (mSha256OidValue)) != 0))) >> + && ( (SigDataSize >= (32 + sizeof (mSha256OidValue))) >> + && ( ((*(SigData + 20) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) >> + || (CompareMem (SigData + 32, &mSha256OidValue, sizeof >> (mSha256OidValue)) != 0)))) >> + { >> + return EFI_SECURITY_VIOLATION; >> } >> } -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#97892): https://edk2.groups.io/g/devel/message/97892 Mute This Topic: https://groups.io/mt/95419835/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-