* Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256 before further verification
@ 2017-12-11 10:40 Wim Vervoorn
2017-12-11 15:55 ` Long, Qin
0 siblings, 1 reply; 3+ messages in thread
From: Wim Vervoorn @ 2017-12-11 10:40 UTC (permalink / raw)
To: edk2-devel@lists.01.org
Hello,
We ran into issues with the Timebased Authenticated variable handling.
In commit: c035e37335ae43229d7e68de74a65f2c01ebc0af
This was added. This assumed the very first tag will be the Sha256 Oid. We have noticed situations where this is the case.
The question is if the check below represents the specification and the tools generating the databuffer should be changed. Or if this check is not correct. It seems to me that the data should be parsed to check for the correct OID and not assume this is the first one
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;
}
}
}
----
Modified: SecurityPkg/Library/AuthVariableLib/AuthService.c
Modified: SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
Best Regards,
Wim Vervoorn
Eltan B.V.
Ambachtstraat 23
5481 SM Schijndel
The Netherlands
T : +31-(0)73-594 46 64
E : wvervoorn@eltan.com
W : http://www.eltan.com
"THIS MESSAGE CONTAINS CONFIDENTIAL INFORMATION. UNLESS YOU ARE THE INTENDED RECIPIENT OF THIS MESSAGE, ANY USE OF THIS MESSAGE IS STRICTLY PROHIBITED. IF YOU HAVE RECEIVED THIS MESSAGE IN ERROR, PLEASE IMMEDIATELY NOTIFY THE SENDER BY TELEPHONE +31-(0)73-5944664 OR REPLY EMAIL, AND IMMEDIATELY DELETE THIS MESSAGE AND ALL COPIES."
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256 before further verification 2017-12-11 10:40 Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256 before further verification Wim Vervoorn @ 2017-12-11 15:55 ` Long, Qin 2017-12-12 14:14 ` Wim Vervoorn 0 siblings, 1 reply; 3+ messages in thread From: Long, Qin @ 2017-12-11 15:55 UTC (permalink / raw) To: Wim Vervoorn, edk2-devel@lists.01.org Hi, Wim Vervoorn, Yes, the logic here is a little tricky. We wouldn't like to introduce the full ASN.1 parse interfaces to handle the encoding data check. So as the comments states, the digestAlgorithms field usually has the fixed offset (based on two bytes of length encoding) in one PKCS#7 signedData structure. So the new codes (added by that commit) used this assumption to check the Sha256 OID directly. // // SignedData.digestAlgorithms shall contain the digest algorithm used when preparing the // signature. Only a digest algorithm of SHA-256 is accepted. // // According to PKCS#7 Definition: // SignedData ::= SEQUENCE { // version Version, // digestAlgorithms DigestAlgorithmIdentifiers, // contentInfo ContentInfo, // .... } // 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. // ...... One typical ASN.1 structure of PKCS7 Signature is ContentInfo { contentType = 1.2.840.113549.1.7.2 //(signedData) content { SignedData { version = 1 ... } } } But please note, the PKCS#7 signedData definition for Authenticated Variable in UEFI spec didn't include the contentType fields. So if you used some third-party tool (e.g. OpenSSL) to generate the signedData, you need to strip-off some bytes. See more discussion & clarifications from https://bugzilla.tianocore.org/show_bug.cgi?id=586 And share us the binary data for more analysis if you still have verification issues. Best Regards & Thanks, LONG, Qin -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Wim Vervoorn Sent: Monday, December 11, 2017 6:40 PM To: edk2-devel@lists.01.org Subject: [edk2] Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256 before further verification Hello, We ran into issues with the Timebased Authenticated variable handling. In commit: c035e37335ae43229d7e68de74a65f2c01ebc0af This was added. This assumed the very first tag will be the Sha256 Oid. We have noticed situations where this is the case. The question is if the check below represents the specification and the tools generating the databuffer should be changed. Or if this check is not correct. It seems to me that the data should be parsed to check for the correct OID and not assume this is the first one 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; } } } ---- Modified: SecurityPkg/Library/AuthVariableLib/AuthService.c Modified: SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h Best Regards, Wim Vervoorn Eltan B.V. Ambachtstraat 23 5481 SM Schijndel The Netherlands T : +31-(0)73-594 46 64 E : wvervoorn@eltan.com W : http://www.eltan.com "THIS MESSAGE CONTAINS CONFIDENTIAL INFORMATION. UNLESS YOU ARE THE INTENDED RECIPIENT OF THIS MESSAGE, ANY USE OF THIS MESSAGE IS STRICTLY PROHIBITED. IF YOU HAVE RECEIVED THIS MESSAGE IN ERROR, PLEASE IMMEDIATELY NOTIFY THE SENDER BY TELEPHONE +31-(0)73-5944664 OR REPLY EMAIL, AND IMMEDIATELY DELETE THIS MESSAGE AND ALL COPIES." _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256 before further verification 2017-12-11 15:55 ` Long, Qin @ 2017-12-12 14:14 ` Wim Vervoorn 0 siblings, 0 replies; 3+ messages in thread From: Wim Vervoorn @ 2017-12-12 14:14 UTC (permalink / raw) To: Long, Qin, edk2-devel@lists.01.org Hello LONG, Quin, Thank you very much for the quick response. From the discussion it is clear to me where the problem is and how the data can be signed using signtool to prevent this. Do you know if there are any updates to the Linux tools (e.g. efitools) that allow supporting UEFI 2.6 in an easy way? Best Regards, Wim Vervoorn Eltan B.V. Ambachtstraat 23 5481 SM Schijndel The Netherlands T : +31-(0)73-594 46 64 E : wvervoorn@eltan.com W : http://www.eltan.com "THIS MESSAGE CONTAINS CONFIDENTIAL INFORMATION. UNLESS YOU ARE THE INTENDED RECIPIENT OF THIS MESSAGE, ANY USE OF THIS MESSAGE IS STRICTLY PROHIBITED. IF YOU HAVE RECEIVED THIS MESSAGE IN ERROR, PLEASE IMMEDIATELY NOTIFY THE SENDER BY TELEPHONE +31-(0)73-5944664 OR REPLY EMAIL, AND IMMEDIATELY DELETE THIS MESSAGE AND ALL COPIES." -----Original Message----- From: Long, Qin [mailto:qin.long@intel.com] Sent: Monday, December 11, 2017 4:56 PM To: Wim Vervoorn <wvervoorn@eltan.com>; edk2-devel@lists.01.org Subject: RE: Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256 before further verification Hi, Wim Vervoorn, Yes, the logic here is a little tricky. We wouldn't like to introduce the full ASN.1 parse interfaces to handle the encoding data check. So as the comments states, the digestAlgorithms field usually has the fixed offset (based on two bytes of length encoding) in one PKCS#7 signedData structure. So the new codes (added by that commit) used this assumption to check the Sha256 OID directly. // // SignedData.digestAlgorithms shall contain the digest algorithm used when preparing the // signature. Only a digest algorithm of SHA-256 is accepted. // // According to PKCS#7 Definition: // SignedData ::= SEQUENCE { // version Version, // digestAlgorithms DigestAlgorithmIdentifiers, // contentInfo ContentInfo, // .... } // 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. // ...... One typical ASN.1 structure of PKCS7 Signature is ContentInfo { contentType = 1.2.840.113549.1.7.2 //(signedData) content { SignedData { version = 1 ... } } } But please note, the PKCS#7 signedData definition for Authenticated Variable in UEFI spec didn't include the contentType fields. So if you used some third-party tool (e.g. OpenSSL) to generate the signedData, you need to strip-off some bytes. See more discussion & clarifications from https://bugzilla.tianocore.org/show_bug.cgi?id=586 And share us the binary data for more analysis if you still have verification issues. Best Regards & Thanks, LONG, Qin -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Wim Vervoorn Sent: Monday, December 11, 2017 6:40 PM To: edk2-devel@lists.01.org Subject: [edk2] Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256 before further verification Hello, We ran into issues with the Timebased Authenticated variable handling. In commit: c035e37335ae43229d7e68de74a65f2c01ebc0af This was added. This assumed the very first tag will be the Sha256 Oid. We have noticed situations where this is the case. The question is if the check below represents the specification and the tools generating the databuffer should be changed. Or if this check is not correct. It seems to me that the data should be parsed to check for the correct OID and not assume this is the first one 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; } } } ---- Modified: SecurityPkg/Library/AuthVariableLib/AuthService.c Modified: SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h Best Regards, Wim Vervoorn Eltan B.V. Ambachtstraat 23 5481 SM Schijndel The Netherlands T : +31-(0)73-594 46 64 E : wvervoorn@eltan.com W : http://www.eltan.com "THIS MESSAGE CONTAINS CONFIDENTIAL INFORMATION. UNLESS YOU ARE THE INTENDED RECIPIENT OF THIS MESSAGE, ANY USE OF THIS MESSAGE IS STRICTLY PROHIBITED. IF YOU HAVE RECEIVED THIS MESSAGE IN ERROR, PLEASE IMMEDIATELY NOTIFY THE SENDER BY TELEPHONE +31-(0)73-5944664 OR REPLY EMAIL, AND IMMEDIATELY DELETE THIS MESSAGE AND ALL COPIES." _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-12-12 14:09 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-12-11 10:40 Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256 before further verification Wim Vervoorn 2017-12-11 15:55 ` Long, Qin 2017-12-12 14:14 ` Wim Vervoorn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox