rfc@edk2.groups.io | Unified API for Hashing Algorithms in EDK2

Home Messages Hashtags

Date Date 1 - 17 of 17

Unified API for Hashing Algorithms in EDK2

Sukerkar, Amol N #194 Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://bugzilla.tianocore.org/show_bug.cgi?id=2151. You can also provide the feedback in the Bugzilla. Thanks, Amol PDF Hash Unified API Support in EDK2.pdf
More All Messages By This Member
Nate DeSimone #195 Hi Amol, With regard to verifying code hashes, it makes much more sense to me to have a GUID defined that represents the hash function to use versus a PCD. The reason for this is the method for signing Firmware Volumes typically involves using nested FVs where the GUID for the nested FV type indicates the algorithm needed to "extract" the nested FV. Extraction in this context is used to verify code hashes as well as decompress compressed code. The "extractor" is usually a statically linked library attached to either SignedSectionExtractPei and/or DxeIpl. For example, it is very possible for a platform to use more than one hashing algorithm, I can think of more than one closed source platform at Intel that actually does this. Now, one thing that I think your document implies is that we should have better APIs for crypto operations in general. If that was the intent then I totally agree with you. There are several reasons that I think this should be done. First, the API for OpenSSL has a tendency to change often enough to make integrating new versions into older codebases a headache, putting an abstraction layer on it would be helpful. Second, the method used to consume OpenSSL in edk2 causes some technical issues. Specifically, OpenSSL is statically linked and it defines a crazy number of global variables, enough so that on a debug build I have noticed roughly a 20KB increase in binary size even if you don't call a single function in OpenSSL. Wrapping the crypto operations in a PPI/Protocol so that only 1 driver needs to link to OpenSSL would probably help reduce binary size for most implementations. Your current document does not define a PPI/Protocol, that is something I would recommend. One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol have run into is that all the crypto functions need to be linked, even the ones that a platform does not use, which results in a net negative binary size impact. Accordingly, one would likely need to break the crypto functionality into multiple PPIs/Protocols so that only the functions a platform needs are actually included. I think some experiments need to be done to figure out the most optimal division of crypto APIs. Hope that helps, Nate toggle quoted message Show quoted text -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 20, 2019 8:28 AM To: rfc@edk2.groups.io Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Importance: High Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://bugzilla.tianocore.org/show_bug.cgi?id=2151. You can also provide the feedback in the Bugzilla. Thanks, Amol
More All Messages By This Member
Michael D Kinney #196 Nate and Amol, There is some work already started by Sean and Matt that implements a PEIM, DXE Driver, and SMM Driver to produce Protocol/PPI that wraps the BaseCryptLib services. This content broken out into its own package is available here: https://github.com/microsoft/mu_plus/tree/dev/201908/SharedCryptoPkg I have ported and simplified this content into a proposed set of patches to the CryptoPkg. It uses a structured PCD to configure the services mapped into the Protocols/PPIs and avoids the issue Nate notes below with protocols and PPIs including all of the BaseCryptLib services. The structured PCD allows families of crypto services or individual services within a family to be enabled/disabled. https://github.com/mdkinney/edk2/tree/CryptoPkg_PPI_Protocol_Proposal_V5 For example, the DSC file PCD statements to enable the SHA1 family and SHA256 family of hash services with the HashAll service disabled is: [PcdsFixedAtBuild] gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha1.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha1.Services.HashAll \| FALSE gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha256.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha256.Services.HashAll \| FALSE Please take a look at this proposal and let me know if this can be used to address. https://bugzilla.tianocore.org/show_bug.cgi?id=2151 There is currently a limitation in the structured PCD feature that does not allow the structured PCD field values to be set in the scope of a module in a <PcdsFixedAtBuild> section. To work around this limitation, the CryptoPkg DSC file has a define called CRYPTO_SERVICES that can be set to ALL, NONE, MIN_PEI, or MIN_DXE_MIN_SMM. The default is ALL. Building with each of these values will build the modules with different sets of enabled services that matches the services enabled using multiple modules in the work from Sean and Matt. If this limitation is addressed in BaseTools, then CryptoPkg could remove the CRYPTO_SERVIES define and all !if statements. Thanks, Mike toggle quoted message Show quoted text -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Nate DeSimone Sent: Wednesday, November 20, 2019 10:31 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Amol, With regard to verifying code hashes, it makes much more sense to me to have a GUID defined that represents the hash function to use versus a PCD. The reason for this is the method for signing Firmware Volumes typically involves using nested FVs where the GUID for the nested FV type indicates the algorithm needed to "extract" the nested FV. Extraction in this context is used to verify code hashes as well as decompress compressed code. The "extractor" is usually a statically linked library attached to either SignedSectionExtractPei and/or DxeIpl. For example, it is very possible for a platform to use more than one hashing algorithm, I can think of more than one closed source platform at Intel that actually does this. Now, one thing that I think your document implies is that we should have better APIs for crypto operations in general. If that was the intent then I totally agree with you. There are several reasons that I think this should be done. First, the API for OpenSSL has a tendency to change often enough to make integrating new versions into older codebases a headache, putting an abstraction layer on it would be helpful. Second, the method used to consume OpenSSL in edk2 causes some technical issues. Specifically, OpenSSL is statically linked and it defines a crazy number of global variables, enough so that on a debug build I have noticed roughly a 20KB increase in binary size even if you don't call a single function in OpenSSL. Wrapping the crypto operations in a PPI/Protocol so that only 1 driver needs to link to OpenSSL would probably help reduce binary size for most implementations. Your current document does not define a PPI/Protocol, that is something I would recommend. One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol have run into is that all the crypto functions need to be linked, even the ones that a platform does not use, which results in a net negative binary size impact. Accordingly, one would likely need to break the crypto functionality into multiple PPIs/Protocols so that only the functions a platform needs are actually included. I think some experiments need to be done to figure out the most optimal division of crypto APIs. Hope that helps, Nate -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 20, 2019 8:28 AM To: rfc@edk2.groups.io Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Importance: High Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://bugzilla.tianocore.org/show_bug.cgi?id=2151. You can also provide the feedback in the Bugzilla. Thanks, Amol
More All Messages By This Member
Sukerkar, Amol N #197 Thanks, Nate and Mike! I am going through the code and comments and will respond shortly. In the meantime, here is the GitHub link to my PoC for the community to look at and comment: https://github.com/ansukerk/edk2. Best regards, Amol toggle quoted message Show quoted text -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael D Kinney Sent: Thursday, November 21, 2019 4:37 PM To: rfc@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Nate and Amol, There is some work already started by Sean and Matt that implements a PEIM, DXE Driver, and SMM Driver to produce Protocol/PPI that wraps the BaseCryptLib services. This content broken out into its own package is available here: https://github.com/microsoft/mu_plus/tree/dev/201908/SharedCryptoPkg I have ported and simplified this content into a proposed set of patches to the CryptoPkg. It uses a structured PCD to configure the services mapped into the Protocols/PPIs and avoids the issue Nate notes below with protocols and PPIs including all of the BaseCryptLib services. The structured PCD allows families of crypto services or individual services within a family to be enabled/disabled. https://github.com/mdkinney/edk2/tree/CryptoPkg_PPI_Protocol_Proposal_V5 For example, the DSC file PCD statements to enable the SHA1 family and SHA256 family of hash services with the HashAll service disabled is: [PcdsFixedAtBuild] gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha1.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha1.Services.HashAll \| FALSE gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha256.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha256.Services.HashAll \| FALSE Please take a look at this proposal and let me know if this can be used to address. https://bugzilla.tianocore.org/show_bug.cgi?id=2151 There is currently a limitation in the structured PCD feature that does not allow the structured PCD field values to be set in the scope of a module in a <PcdsFixedAtBuild> section. To work around this limitation, the CryptoPkg DSC file has a define called CRYPTO_SERVICES that can be set to ALL, NONE, MIN_PEI, or MIN_DXE_MIN_SMM. The default is ALL. Building with each of these values will build the modules with different sets of enabled services that matches the services enabled using multiple modules in the work from Sean and Matt. If this limitation is addressed in BaseTools, then CryptoPkg could remove the CRYPTO_SERVIES define and all !if statements. Thanks, Mike -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Nate DeSimone Sent: Wednesday, November 20, 2019 10:31 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Amol, With regard to verifying code hashes, it makes much more sense to me to have a GUID defined that represents the hash function to use versus a PCD. The reason for this is the method for signing Firmware Volumes typically involves using nested FVs where the GUID for the nested FV type indicates the algorithm needed to "extract" the nested FV. Extraction in this context is used to verify code hashes as well as decompress compressed code. The "extractor" is usually a statically linked library attached to either SignedSectionExtractPei and/or DxeIpl. For example, it is very possible for a platform to use more than one hashing algorithm, I can think of more than one closed source platform at Intel that actually does this. Now, one thing that I think your document implies is that we should have better APIs for crypto operations in general. If that was the intent then I totally agree with you. There are several reasons that I think this should be done. First, the API for OpenSSL has a tendency to change often enough to make integrating new versions into older codebases a headache, putting an abstraction layer on it would be helpful. Second, the method used to consume OpenSSL in edk2 causes some technical issues. Specifically, OpenSSL is statically linked and it defines a crazy number of global variables, enough so that on a debug build I have noticed roughly a 20KB increase in binary size even if you don't call a single function in OpenSSL. Wrapping the crypto operations in a PPI/Protocol so that only 1 driver needs to link to OpenSSL would probably help reduce binary size for most implementations. Your current document does not define a PPI/Protocol, that is something I would recommend. One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol have run into is that all the crypto functions need to be linked, even the ones that a platform does not use, which results in a net negative binary size impact. Accordingly, one would likely need to break the crypto functionality into multiple PPIs/Protocols so that only the functions a platform needs are actually included. I think some experiments need to be done to figure out the most optimal division of crypto APIs. Hope that helps, Nate -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 20, 2019 8:28 AM To: rfc@edk2.groups.io Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Importance: High Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://bugzilla.tianocore.org/show_bug.cgi?id=2151. You can also provide the feedback in the Bugzilla. Thanks, Amol
More All Messages By This Member
Sukerkar, Amol N #198 Hi Mike and Nate, With our implementation we are trying to address the following in EDKII: 1. A common Hashing API for UEFI drivers to consume instead of the current API that directly calls into the specific hashing algorithm, for instance, SHA1_Init, SHA256_Update, etc. Due to a stronger hashing algorithm requirement, certain drivers need to be upgraded to SHA384 from SHA256 and the unified API is proposed, so, while upgrading we remove the hard-coded dependency on each hashing algorithm API. 2. The size issue as a result of statically linking openssl/cryptolib API to the consuming driver. There are two ways we tried to address this issue, although, we didn't explicitly defined a PPI to address it. a. Introduced a fixed PCD per hashing algorithm to include/exclude a particular algorithm from BIOS. b. Use a register mechanism to link the instance of a particular hashing algorithm to Unified API library. This is configurable using PCD. Note that 2.a and 2.b implementation is very similar to HashLibBaseCryptoRouter used for TPM2 operations. From the size and static linking perspective, I believe we can have a discussion between 2.a and 2.b with Mike's implementation(https://github.com/mdkinney/edk2/tree/CryptoPkg_PPI_Protocol_Proposal_V5) below. We still need a common hashing API so the hashing algorithm strength for a particular driver can be increased without having to modify the consumer driver code. Comments are welcome. Thanks, Amol toggle quoted message Show quoted text -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Thursday, November 21, 2019 4:45 PM To: rfc@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Nate and Mike! I am going through the code and comments and will respond shortly. In the meantime, here is the GitHub link to my PoC for the community to look at and comment: https://github.com/ansukerk/edk2. Best regards, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael D Kinney Sent: Thursday, November 21, 2019 4:37 PM To: rfc@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Nate and Amol, There is some work already started by Sean and Matt that implements a PEIM, DXE Driver, and SMM Driver to produce Protocol/PPI that wraps the BaseCryptLib services. This content broken out into its own package is available here: https://github.com/microsoft/mu_plus/tree/dev/201908/SharedCryptoPkg I have ported and simplified this content into a proposed set of patches to the CryptoPkg. It uses a structured PCD to configure the services mapped into the Protocols/PPIs and avoids the issue Nate notes below with protocols and PPIs including all of the BaseCryptLib services. The structured PCD allows families of crypto services or individual services within a family to be enabled/disabled. https://github.com/mdkinney/edk2/tree/CryptoPkg_PPI_Protocol_Proposal_V5 For example, the DSC file PCD statements to enable the SHA1 family and SHA256 family of hash services with the HashAll service disabled is: [PcdsFixedAtBuild] gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha1.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha1.Services.HashAll \| FALSE gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha256.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha256.Services.HashAll \| FALSE Please take a look at this proposal and let me know if this can be used to address. https://bugzilla.tianocore.org/show_bug.cgi?id=2151 There is currently a limitation in the structured PCD feature that does not allow the structured PCD field values to be set in the scope of a module in a <PcdsFixedAtBuild> section. To work around this limitation, the CryptoPkg DSC file has a define called CRYPTO_SERVICES that can be set to ALL, NONE, MIN_PEI, or MIN_DXE_MIN_SMM. The default is ALL. Building with each of these values will build the modules with different sets of enabled services that matches the services enabled using multiple modules in the work from Sean and Matt. If this limitation is addressed in BaseTools, then CryptoPkg could remove the CRYPTO_SERVIES define and all !if statements. Thanks, Mike -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Nate DeSimone Sent: Wednesday, November 20, 2019 10:31 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Amol, With regard to verifying code hashes, it makes much more sense to me to have a GUID defined that represents the hash function to use versus a PCD. The reason for this is the method for signing Firmware Volumes typically involves using nested FVs where the GUID for the nested FV type indicates the algorithm needed to "extract" the nested FV. Extraction in this context is used to verify code hashes as well as decompress compressed code. The "extractor" is usually a statically linked library attached to either SignedSectionExtractPei and/or DxeIpl. For example, it is very possible for a platform to use more than one hashing algorithm, I can think of more than one closed source platform at Intel that actually does this. Now, one thing that I think your document implies is that we should have better APIs for crypto operations in general. If that was the intent then I totally agree with you. There are several reasons that I think this should be done. First, the API for OpenSSL has a tendency to change often enough to make integrating new versions into older codebases a headache, putting an abstraction layer on it would be helpful. Second, the method used to consume OpenSSL in edk2 causes some technical issues. Specifically, OpenSSL is statically linked and it defines a crazy number of global variables, enough so that on a debug build I have noticed roughly a 20KB increase in binary size even if you don't call a single function in OpenSSL. Wrapping the crypto operations in a PPI/Protocol so that only 1 driver needs to link to OpenSSL would probably help reduce binary size for most implementations. Your current document does not define a PPI/Protocol, that is something I would recommend. One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol have run into is that all the crypto functions need to be linked, even the ones that a platform does not use, which results in a net negative binary size impact. Accordingly, one would likely need to break the crypto functionality into multiple PPIs/Protocols so that only the functions a platform needs are actually included. I think some experiments need to be done to figure out the most optimal division of crypto APIs. Hope that helps, Nate -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 20, 2019 8:28 AM To: rfc@edk2.groups.io Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Importance: High Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://bugzilla.tianocore.org/show_bug.cgi?id=2151. You can also provide the feedback in the Bugzilla. Thanks, Amol
More All Messages By This Member
Sean #199 Amol, I am interested to hear more about the actual use cases. I like the idea of abstracting the API so that a calling driver doesn't have to change when the hashing algorithm requirements change but am not convinced that this will actually be helpful (or at least worth the introduction of more complexity in security sensitive code). For the TPM and measured boot I agree. Random driver needs to extend a measurement then it can use the existing hashlib and it will be hashed and extended per platform policy. This is because the random driver doesn't need to understand the result. Only the TPM needs to and that is handled within the extend operation. For the other drivers that I have seen using hashing, they need to understand the result and make driver specific decisions based on the result. For example if I am verifying the OBB or some FV then I must make sure the known good and computed match. This driver must locate the known good and thus this process generally must know the format of the result. For this I think it is best to know what algorithm I am using and I don't see a strong use case to change algorithm to an abstracted algorithm. Do you have a set of example drivers/use cases that could use abstracted hash support? Thanks Sean toggle quoted message Show quoted text -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N via Groups.Io Sent: Monday, November 25, 2019 12:08 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sean Brogan <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [EXTERNAL] Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike and Nate, With our implementation we are trying to address the following in EDKII: 1. A common Hashing API for UEFI drivers to consume instead of the current API that directly calls into the specific hashing algorithm, for instance, SHA1_Init, SHA256_Update, etc. Due to a stronger hashing algorithm requirement, certain drivers need to be upgraded to SHA384 from SHA256 and the unified API is proposed, so, while upgrading we remove the hard-coded dependency on each hashing algorithm API. 2. The size issue as a result of statically linking openssl/cryptolib API to the consuming driver. There are two ways we tried to address this issue, although, we didn't explicitly defined a PPI to address it. a. Introduced a fixed PCD per hashing algorithm to include/exclude a particular algorithm from BIOS. b. Use a register mechanism to link the instance of a particular hashing algorithm to Unified API library. This is configurable using PCD. Note that 2.a and 2.b implementation is very similar to HashLibBaseCryptoRouter used for TPM2 operations. From the size and static linking perspective, I believe we can have a discussion between 2.a and 2.b with Mike's implementation(https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Proposal_V5&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5dW8TEhs9USyDM%3D&reserved=0) below. We still need a common hashing API so the hashing algorithm strength for a particular driver can be increased without having to modify the consumer driver code. Comments are welcome. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Thursday, November 21, 2019 4:45 PM To: rfc@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Nate and Mike! I am going through the code and comments and will respond shortly. In the meantime, here is the GitHub link to my PoC for the community to look at and comment: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fansukerk%2Fedk2&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&sdata=keRdfU%2BxffkBsyvYHwvSOIA8ryexOigiKhR%2B%2F1q7DjE%3D&reserved=0. Best regards, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael D Kinney Sent: Thursday, November 21, 2019 4:37 PM To: rfc@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Nate and Amol, There is some work already started by Sean and Matt that implements a PEIM, DXE Driver, and SMM Driver to produce Protocol/PPI that wraps the BaseCryptLib services. This content broken out into its own package is available here: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fmu_plus%2Ftree%2Fdev%2F201908%2FSharedCryptoPkg&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&sdata=88TRhUqsVrj7sx8P4hjbdLy2%2BDKnrBoi7HAI3GUhpPo%3D&reserved=0 I have ported and simplified this content into a proposed set of patches to the CryptoPkg. It uses a structured PCD to configure the services mapped into the Protocols/PPIs and avoids the issue Nate notes below with protocols and PPIs including all of the BaseCryptLib services. The structured PCD allows families of crypto services or individual services within a family to be enabled/disabled. https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Proposal_V5&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5dW8TEhs9USyDM%3D&reserved=0 For example, the DSC file PCD statements to enable the SHA1 family and SHA256 family of hash services with the HashAll service disabled is: [PcdsFixedAtBuild] gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha1.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha1.Services.HashAll \| FALSE gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha256.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha256.Services.HashAll \| FALSE Please take a look at this proposal and let me know if this can be used to address. https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&sdata=oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&reserved=0 There is currently a limitation in the structured PCD feature that does not allow the structured PCD field values to be set in the scope of a module in a <PcdsFixedAtBuild> section. To work around this limitation, the CryptoPkg DSC file has a define called CRYPTO_SERVICES that can be set to ALL, NONE, MIN_PEI, or MIN_DXE_MIN_SMM. The default is ALL. Building with each of these values will build the modules with different sets of enabled services that matches the services enabled using multiple modules in the work from Sean and Matt. If this limitation is addressed in BaseTools, then CryptoPkg could remove the CRYPTO_SERVIES define and all !if statements. Thanks, Mike -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Nate DeSimone Sent: Wednesday, November 20, 2019 10:31 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Amol, With regard to verifying code hashes, it makes much more sense to me to have a GUID defined that represents the hash function to use versus a PCD. The reason for this is the method for signing Firmware Volumes typically involves using nested FVs where the GUID for the nested FV type indicates the algorithm needed to "extract" the nested FV. Extraction in this context is used to verify code hashes as well as decompress compressed code. The "extractor" is usually a statically linked library attached to either SignedSectionExtractPei and/or DxeIpl. For example, it is very possible for a platform to use more than one hashing algorithm, I can think of more than one closed source platform at Intel that actually does this. Now, one thing that I think your document implies is that we should have better APIs for crypto operations in general. If that was the intent then I totally agree with you. There are several reasons that I think this should be done. First, the API for OpenSSL has a tendency to change often enough to make integrating new versions into older codebases a headache, putting an abstraction layer on it would be helpful. Second, the method used to consume OpenSSL in edk2 causes some technical issues. Specifically, OpenSSL is statically linked and it defines a crazy number of global variables, enough so that on a debug build I have noticed roughly a 20KB increase in binary size even if you don't call a single function in OpenSSL. Wrapping the crypto operations in a PPI/Protocol so that only 1 driver needs to link to OpenSSL would probably help reduce binary size for most implementations. Your current document does not define a PPI/Protocol, that is something I would recommend. One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol have run into is that all the crypto functions need to be linked, even the ones that a platform does not use, which results in a net negative binary size impact. Accordingly, one would likely need to break the crypto functionality into multiple PPIs/Protocols so that only the functions a platform needs are actually included. I think some experiments need to be done to figure out the most optimal division of crypto APIs. Hope that helps, Nate -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 20, 2019 8:28 AM To: rfc@edk2.groups.io Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Importance: High Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&sdata=oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&reserved=0. You can also provide the feedback in the Bugzilla. Thanks, Amol
More All Messages By This Member
Sukerkar, Amol N #200 Hi Sean, A few examples that we want to target are - HDD Password verification - Capsule Verification (Platform recovery) - Bios Guard Platform Data Table - More importantly, Boot Guard Event Log driver that has a bunch of redundant code today calculating various hashes. I don't think this is open source though. Currently, the hash verification mechanism in the above features is hard-coded to SHA256 which we will need to change to SHA384 to support a more robust hashing mechanism. This is a good opportunity to replace the existing SHA256 API with an abstracted unified API. And if necessary, the abstracted unified API still supports driver specific hashing algorithm if needed (as opposed to the strongest hashing algorithm supported by the system). We are not targeting verification of OBB or for that matter any PE/COFF image with this mechanism, mainly because it has it's own wrapper and verification mechanism in DxeImageVerificationLib. Thanks, Amol toggle quoted message Show quoted text -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sean via Groups.Io Sent: Monday, November 25, 2019 1:24 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, I am interested to hear more about the actual use cases. I like the idea of abstracting the API so that a calling driver doesn't have to change when the hashing algorithm requirements change but am not convinced that this will actually be helpful (or at least worth the introduction of more complexity in security sensitive code). For the TPM and measured boot I agree. Random driver needs to extend a measurement then it can use the existing hashlib and it will be hashed and extended per platform policy. This is because the random driver doesn't need to understand the result. Only the TPM needs to and that is handled within the extend operation. For the other drivers that I have seen using hashing, they need to understand the result and make driver specific decisions based on the result. For example if I am verifying the OBB or some FV then I must make sure the known good and computed match. This driver must locate the known good and thus this process generally must know the format of the result. For this I think it is best to know what algorithm I am using and I don't see a strong use case to change algorithm to an abstracted algorithm. Do you have a set of example drivers/use cases that could use abstracted hash support? Thanks Sean -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N via Groups.Io Sent: Monday, November 25, 2019 12:08 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sean Brogan <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [EXTERNAL] Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike and Nate, With our implementation we are trying to address the following in EDKII: 1. A common Hashing API for UEFI drivers to consume instead of the current API that directly calls into the specific hashing algorithm, for instance, SHA1_Init, SHA256_Update, etc. Due to a stronger hashing algorithm requirement, certain drivers need to be upgraded to SHA384 from SHA256 and the unified API is proposed, so, while upgrading we remove the hard-coded dependency on each hashing algorithm API. 2. The size issue as a result of statically linking openssl/cryptolib API to the consuming driver. There are two ways we tried to address this issue, although, we didn't explicitly defined a PPI to address it. a. Introduced a fixed PCD per hashing algorithm to include/exclude a particular algorithm from BIOS. b. Use a register mechanism to link the instance of a particular hashing algorithm to Unified API library. This is configurable using PCD. Note that 2.a and 2.b implementation is very similar to HashLibBaseCryptoRouter used for TPM2 operations. From the size and static linking perspective, I believe we can have a discussion between 2.a and 2.b with Mike's implementation(https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Proposal_V5&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5dW8TEhs9USyDM%3D&reserved=0) below. We still need a common hashing API so the hashing algorithm strength for a particular driver can be increased without having to modify the consumer driver code. Comments are welcome. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Thursday, November 21, 2019 4:45 PM To: rfc@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Nate and Mike! I am going through the code and comments and will respond shortly. In the meantime, here is the GitHub link to my PoC for the community to look at and comment: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fansukerk%2Fedk2&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&sdata=keRdfU%2BxffkBsyvYHwvSOIA8ryexOigiKhR%2B%2F1q7DjE%3D&reserved=0. Best regards, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael D Kinney Sent: Thursday, November 21, 2019 4:37 PM To: rfc@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Nate and Amol, There is some work already started by Sean and Matt that implements a PEIM, DXE Driver, and SMM Driver to produce Protocol/PPI that wraps the BaseCryptLib services. This content broken out into its own package is available here: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fmu_plus%2Ftree%2Fdev%2F201908%2FSharedCryptoPkg&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&sdata=88TRhUqsVrj7sx8P4hjbdLy2%2BDKnrBoi7HAI3GUhpPo%3D&reserved=0 I have ported and simplified this content into a proposed set of patches to the CryptoPkg. It uses a structured PCD to configure the services mapped into the Protocols/PPIs and avoids the issue Nate notes below with protocols and PPIs including all of the BaseCryptLib services. The structured PCD allows families of crypto services or individual services within a family to be enabled/disabled. https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Proposal_V5&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5dW8TEhs9USyDM%3D&reserved=0 For example, the DSC file PCD statements to enable the SHA1 family and SHA256 family of hash services with the HashAll service disabled is: [PcdsFixedAtBuild] gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha1.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha1.Services.HashAll \| FALSE gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha256.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha256.Services.HashAll \| FALSE Please take a look at this proposal and let me know if this can be used to address. https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&sdata=oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&reserved=0 There is currently a limitation in the structured PCD feature that does not allow the structured PCD field values to be set in the scope of a module in a <PcdsFixedAtBuild> section. To work around this limitation, the CryptoPkg DSC file has a define called CRYPTO_SERVICES that can be set to ALL, NONE, MIN_PEI, or MIN_DXE_MIN_SMM. The default is ALL. Building with each of these values will build the modules with different sets of enabled services that matches the services enabled using multiple modules in the work from Sean and Matt. If this limitation is addressed in BaseTools, then CryptoPkg could remove the CRYPTO_SERVIES define and all !if statements. Thanks, Mike -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Nate DeSimone Sent: Wednesday, November 20, 2019 10:31 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Amol, With regard to verifying code hashes, it makes much more sense to me to have a GUID defined that represents the hash function to use versus a PCD. The reason for this is the method for signing Firmware Volumes typically involves using nested FVs where the GUID for the nested FV type indicates the algorithm needed to "extract" the nested FV. Extraction in this context is used to verify code hashes as well as decompress compressed code. The "extractor" is usually a statically linked library attached to either SignedSectionExtractPei and/or DxeIpl. For example, it is very possible for a platform to use more than one hashing algorithm, I can think of more than one closed source platform at Intel that actually does this. Now, one thing that I think your document implies is that we should have better APIs for crypto operations in general. If that was the intent then I totally agree with you. There are several reasons that I think this should be done. First, the API for OpenSSL has a tendency to change often enough to make integrating new versions into older codebases a headache, putting an abstraction layer on it would be helpful. Second, the method used to consume OpenSSL in edk2 causes some technical issues. Specifically, OpenSSL is statically linked and it defines a crazy number of global variables, enough so that on a debug build I have noticed roughly a 20KB increase in binary size even if you don't call a single function in OpenSSL. Wrapping the crypto operations in a PPI/Protocol so that only 1 driver needs to link to OpenSSL would probably help reduce binary size for most implementations. Your current document does not define a PPI/Protocol, that is something I would recommend. One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol have run into is that all the crypto functions need to be linked, even the ones that a platform does not use, which results in a net negative binary size impact. Accordingly, one would likely need to break the crypto functionality into multiple PPIs/Protocols so that only the functions a platform needs are actually included. I think some experiments need to be done to figure out the most optimal division of crypto APIs. Hope that helps, Nate -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 20, 2019 8:28 AM To: rfc@edk2.groups.io Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Importance: High Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&sdata=oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&reserved=0. You can also provide the feedback in the Bugzilla. Thanks, Amol
More All Messages By This Member
Wang, Jian J #201 Hi Sean and Amol, I believe the OBB and FV/PE image verification can still benefit from the unified API, if we add one more parameter to HashApiIinit() to force using a hash algorithm or add a PCD to choose one or link only required HashApiInstanceXxx. (Just for example) HashApiInit ( IN EFI_GUID HashAlg, OUT HASH_HANDLE HashHandle ) In this way, we can avoid hard-code hash API in the verification module. Although it's slow, I think the OBB verification or FV/PE image signature algorithm is still subject to change per security requirement change. During a transition period, we don't have to add a new code to handle a different algorithm in verification module. Instead we just need to link a different hash lib or set PCD to use a different algorithm. I think this makes things simpler and easier to change. The BaseCryptLib implemented in PPI/Protocol is a good idea to save image size. I think the Unified hash API (BaseHashLib) can make use of it to save extra hash algorithm library instances (HashApiInstanceSha1, Sha256,...). The only problem is that the structured PCD, like Mike mentioned before, doesn't support module override, which prevents different modules from using different algorithms and then cannot save module size utter mostly. Regards, Jian toggle quoted message Show quoted text -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, November 26, 2019 5:53 AM To: rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean, A few examples that we want to target are - HDD Password verification - Capsule Verification (Platform recovery) - Bios Guard Platform Data Table - More importantly, Boot Guard Event Log driver that has a bunch of redundant code today calculating various hashes. I don't think this is open source though. Currently, the hash verification mechanism in the above features is hard-coded to SHA256 which we will need to change to SHA384 to support a more robust hashing mechanism. This is a good opportunity to replace the existing SHA256 API with an abstracted unified API. And if necessary, the abstracted unified API still supports driver specific hashing algorithm if needed (as opposed to the strongest hashing algorithm supported by the system). We are not targeting verification of OBB or for that matter any PE/COFF image with this mechanism, mainly because it has it's own wrapper and verification mechanism in DxeImageVerificationLib. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sean via Groups.Io Sent: Monday, November 25, 2019 1:24 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, I am interested to hear more about the actual use cases. I like the idea of abstracting the API so that a calling driver doesn't have to change when the hashing algorithm requirements change but am not convinced that this will actually be helpful (or at least worth the introduction of more complexity in security sensitive code). For the TPM and measured boot I agree. Random driver needs to extend a measurement then it can use the existing hashlib and it will be hashed and extended per platform policy. This is because the random driver doesn't need to understand the result. Only the TPM needs to and that is handled within the extend operation. For the other drivers that I have seen using hashing, they need to understand the result and make driver specific decisions based on the result. For example if I am verifying the OBB or some FV then I must make sure the known good and computed match. This driver must locate the known good and thus this process generally must know the format of the result. For this I think it is best to know what algorithm I am using and I don't see a strong use case to change algorithm to an abstracted algorithm. Do you have a set of example drivers/use cases that could use abstracted hash support? Thanks Sean -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N via Groups.Io Sent: Monday, November 25, 2019 12:08 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sean Brogan <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [EXTERNAL] Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike and Nate, With our implementation we are trying to address the following in EDKII: 1. A common Hashing API for UEFI drivers to consume instead of the current API that directly calls into the specific hashing algorithm, for instance, SHA1_Init, SHA256_Update, etc. Due to a stronger hashing algorithm requirement, certain drivers need to be upgraded to SHA384 from SHA256 and the unified API is proposed, so, while upgrading we remove the hard-coded dependency on each hashing algorithm API. 2. The size issue as a result of statically linking openssl/cryptolib API to the consuming driver. There are two ways we tried to address this issue, although, we didn't explicitly defined a PPI to address it. a. Introduced a fixed PCD per hashing algorithm to include/exclude a particular algorithm from BIOS. b. Use a register mechanism to link the instance of a particular hashing algorithm to Unified API library. This is configurable using PCD. Note that 2.a and 2.b implementation is very similar to HashLibBaseCryptoRouter used for TPM2 operations. From the size and static linking perspective, I believe we can have a discussion between 2.a and 2.b with Mike's implementation(https://nam06.safelinks.protection.outlook.com/?url=https%3A %2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_ Proposal_V5&data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110f db2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1 %7C0%7C637103092953247213&sdata=2acK0dmJZ85aZ3LvOOqD3e3w44V EM5dW8TEhs9USyDM%3D&reserved=0) below. We still need a common hashing API so the hashing algorithm strength for a particular driver can be increased without having to modify the consumer driver code. Comments are welcome. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Thursday, November 21, 2019 4:45 PM To: rfc@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Nate and Mike! I am going through the code and comments and will respond shortly. In the meantime, here is the GitHub link to my PoC for the community to look at and comment: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.c om%2Fansukerk%2Fedk2&data=02%7C01%7Csean.brogan%40microsoft.c om%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd 011db47%7C1%7C0%7C637103092953247213&sdata=keRdfU%2BxffkBsyv YHwvSOIA8ryexOigiKhR%2B%2F1q7DjE%3D&reserved=0. Best regards, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael D Kinney Sent: Thursday, November 21, 2019 4:37 PM To: rfc@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Nate and Amol, There is some work already started by Sean and Matt that implements a PEIM, DXE Driver, and SMM Driver to produce Protocol/PPI that wraps the BaseCryptLib services. This content broken out into its own package is available here: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.c om%2Fmicrosoft%2Fmu_plus%2Ftree%2Fdev%2F201908%2FSharedCryptoPkg& amp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2 acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637 103092953247213&sdata=88TRhUqsVrj7sx8P4hjbdLy2%2BDKnrBoi7HAI3G UhpPo%3D&reserved=0 I have ported and simplified this content into a proposed set of patches to the CryptoPkg. It uses a structured PCD to configure the services mapped into the Protocols/PPIs and avoids the issue Nate notes below with protocols and PPIs including all of the BaseCryptLib services. The structured PCD allows families of crypto services or individual services within a family to be enabled/disabled. https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.c om%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Proposal_V5&a mp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb2a91f4e8c2a cb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6371 03092953247213&sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5dW8TEhs9 USyDM%3D&reserved=0 For example, the DSC file PCD statements to enable the SHA1 family and SHA256 family of hash services with the HashAll service disabled is: [PcdsFixedAtBuild] gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha1.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha1.Services.Has hAll \| FALSE gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha256.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable.Sha256.Services. HashAll \| FALSE Please take a look at this proposal and let me know if this can be used to address. https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&reserved=0 There is currently a limitation in the structured PCD feature that does not allow the structured PCD field values to be set in the scope of a module in a <PcdsFixedAtBuild> section. To work around this limitation, the CryptoPkg DSC file has a define called CRYPTO_SERVICES that can be set to ALL, NONE, MIN_PEI, or MIN_DXE_MIN_SMM. The default is ALL. Building with each of these values will build the modules with different sets of enabled services that matches the services enabled using multiple modules in the work from Sean and Matt. If this limitation is addressed in BaseTools, then CryptoPkg could remove the CRYPTO_SERVIES define and all !if statements. Thanks, Mike -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Nate DeSimone Sent: Wednesday, November 20, 2019 10:31 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Amol, With regard to verifying code hashes, it makes much more sense to me to have a GUID defined that represents the hash function to use versus a PCD. The reason for this is the method for signing Firmware Volumes typically involves using nested FVs where the GUID for the nested FV type indicates the algorithm needed to "extract" the nested FV. Extraction in this context is used to verify code hashes as well as decompress compressed code. The "extractor" is usually a statically linked library attached to either SignedSectionExtractPei and/or DxeIpl. For example, it is very possible for a platform to use more than one hashing algorithm, I can think of more than one closed source platform at Intel that actually does this. Now, one thing that I think your document implies is that we should have better APIs for crypto operations in general. If that was the intent then I totally agree with you. There are several reasons that I think this should be done. First, the API for OpenSSL has a tendency to change often enough to make integrating new versions into older codebases a headache, putting an abstraction layer on it would be helpful. Second, the method used to consume OpenSSL in edk2 causes some technical issues. Specifically, OpenSSL is statically linked and it defines a crazy number of global variables, enough so that on a debug build I have noticed roughly a 20KB increase in binary size even if you don't call a single function in OpenSSL. Wrapping the crypto operations in a PPI/Protocol so that only 1 driver needs to link to OpenSSL would probably help reduce binary size for most implementations. Your current document does not define a PPI/Protocol, that is something I would recommend. One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol have run into is that all the crypto functions need to be linked, even the ones that a platform does not use, which results in a net negative binary size impact. Accordingly, one would likely need to break the crypto functionality into multiple PPIs/Protocols so that only the functions a platform needs are actually included. I think some experiments need to be done to figure out the most optimal division of crypto APIs. Hope that helps, Nate -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 20, 2019 8:28 AM To: rfc@edk2.groups.io Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Importance: High Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213&sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&reserved=0. You can also provide the feedback in the Bugzilla. Thanks, Amol
More All Messages By This Member
Michael D Kinney #202 As Sean points out, different components in a platform may have different hash requirements. If we want to go down the path where we have a few modules that produce Protocols/PPIs for the BaseCryptLib so all modules can share the same crypto services, then each module need to decide which specific hash algorithm it requires. Instead of defining another protocol/PPI for a unified hash service, we could consider a couple of other options: 1) Each module that uses hash services could use a FixedAtBuild PCD to decide which hash algorithm it should use. Based on this PCD value, different APIs in BaseCryptLib can be called. Since it is a FixedAtBuild PCD, the compiler/linker optimizations will only include the one hash algorithm selected. by using BaseCryptLib, this is compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs. 2) If we have many modules that need to make a hash algorithm selection, then define a new library class with the unified hash APIs, and the library instance uses a FixedAtBuild PCD to configure the hash algorithm used. This is also compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs and minimizes the sources in modules that need a design that supports flexible hash algorithm selection. If new hash algorithm is added, then only the lib instance and PCD need to be updated. Each module sets the FixedAtBuild PCD value in the scope of the module in the platform DSC file. If most or all modules in a platform need the same hash algorithm, then the FixedAtBuild PCD can be set at the platform scope in the platform DSC file. Best regards, Mike toggle quoted message Show quoted text -----Original Message----- From: Wang, Jian J <jian.j.wang@...> Sent: Monday, November 25, 2019 9:09 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean and Amol, I believe the OBB and FV/PE image verification can still benefit from the unified API, if we add one more parameter to HashApiIinit() to force using a hash algorithm or add a PCD to choose one or link only required HashApiInstanceXxx. (Just for example) HashApiInit ( IN EFI_GUID HashAlg, OUT HASH_HANDLE HashHandle ) In this way, we can avoid hard-code hash API in the verification module. Although it's slow, I think the OBB verification or FV/PE image signature algorithm is still subject to change per security requirement change. During a transition period, we don't have to add a new code to handle a different algorithm in verification module. Instead we just need to link a different hash lib or set PCD to use a different algorithm. I think this makes things simpler and easier to change. The BaseCryptLib implemented in PPI/Protocol is a good idea to save image size. I think the Unified hash API (BaseHashLib) can make use of it to save extra hash algorithm library instances (HashApiInstanceSha1, Sha256,...). The only problem is that the structured PCD, like Mike mentioned before, doesn't support module override, which prevents different modules from using different algorithms and then cannot save module size utter mostly. Regards, Jian -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, November 26, 2019 5:53 AM To: rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean, A few examples that we want to target are - HDD Password verification - Capsule Verification (Platform recovery) - Bios Guard Platform Data Table - More importantly, Boot Guard Event Log driver that has a bunch of redundant code today calculating various hashes. I don't think this is open source though. Currently, the hash verification mechanism in the above features is hard-coded to SHA256 which we will need to change to SHA384 to support a more robust hashing mechanism. This is a good opportunity to replace the existing SHA256 API with an abstracted unified API. And if necessary, the abstracted unified API still supports driver specific hashing algorithm if needed (as opposed to the strongest hashing algorithm supported by the system). We are not targeting verification of OBB or for that matter any PE/COFF image with this mechanism, mainly because it has it's own wrapper and verification mechanism in DxeImageVerificationLib. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sean via Groups.Io Sent: Monday, November 25, 2019 1:24 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, I am interested to hear more about the actual use cases. I like the idea of abstracting the API so that a calling driver doesn't have to change when the hashing algorithm requirements change but am not convinced that this will actually be helpful (or at least worth the introduction of more complexity in security sensitive code). For the TPM and measured boot I agree. Random driver needs to extend a measurement then it can use the existing hashlib and it will be hashed and extended per platform policy. This is because the random driver doesn't need to understand the result. Only the TPM needs to and that is handled within the extend operation. For the other drivers that I have seen using hashing, they need to understand the result and make driver specific decisions based on the result. For example if I am verifying the OBB or some FV then I must make sure the known good and computed match. This driver must locate the known good and thus this process generally must know the format of the result. For this I think it is best to know what algorithm I am using and I don't see a strong use case to change algorithm to an abstracted algorithm. Do you have a set of example drivers/use cases that could use abstracted hash support? Thanks Sean -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N via Groups.Io Sent: Monday, November 25, 2019 12:08 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sean Brogan <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [EXTERNAL] Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike and Nate, With our implementation we are trying to address the following in EDKII: 1. A common Hashing API for UEFI drivers to consume instead of the current API that directly calls into the specific hashing algorithm, for instance, SHA1_Init, SHA256_Update, etc. Due to a stronger hashing algorithm requirement, certain drivers need to be upgraded to SHA384 from SHA256 and the unified API is proposed, so, while upgrading we remove the hard-coded dependency on each hashing algorithm API. 2. The size issue as a result of statically linking openssl/cryptolib API to the consuming driver. There are two ways we tried to address this issue, although, we didn't explicitly defined a PPI to address it. a. Introduced a fixed PCD per hashing algorithm to include/exclude a particular algorithm from BIOS. b. Use a register mechanism to link the instance of a particular hashing algorithm to Unified API library. This is configurable using PCD. Note that 2.a and 2.b implementation is very similar to HashLibBaseCryptoRouter used for TPM2 operations. From the size and static linking perspective, I believe we can have a discussion between 2.a and 2.b with Mike's implementation(https://nam06.safelinks.protection.outlo ok.com/?url=htt ps%3A %2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_P PI_Protocol_ Proposal_V5&data=02%7C01%7Csean.brogan%40microsoft. com%7Cd110f db2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd 011db47%7C1 %7C0%7C637103092953247213&sdata=2acK0dmJZ85aZ3LvOOq D3e3w44V EM5dW8TEhs9USyDM%3D&reserved=0) below. We still need a common hashing API so the hashing algorithm strength for a particular driver can be increased without having to modify the consumer driver code. Comments are welcome. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Thursday, November 21, 2019 4:45 PM To: rfc@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Nate and Mike! I am going through the code and comments and will respond shortly. In the meantime, here is the GitHub link to my PoC for the community to look at and comment: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fansukerk%2Fedk2&data=02%7C01%7Csean.brogan%40m icrosoft.c om%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141a f91ab2d7cd 011db47%7C1%7C0%7C637103092953247213&sdata=keRdfU%2 BxffkBsyv YHwvSOIA8ryexOigiKhR%2B%2F1q7DjE%3D&reserved=0. Best regards, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael D Kinney Sent: Thursday, November 21, 2019 4:37 PM To: rfc@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Nate and Amol, There is some work already started by Sean and Matt that implements a PEIM, DXE Driver, and SMM Driver to produce Protocol/PPI that wraps the BaseCryptLib services. This content broken out into its own package is available here: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmicrosoft%2Fmu_plus%2Ftree%2Fdev%2F201908%2FShared CryptoPkg& amp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fd b2a91f4e8c2 acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1% 7C0%7C637 103092953247213&sdata=88TRhUqsVrj7sx8P4hjbdLy2%2BDK nrBoi7HAI3G UhpPo%3D&reserved=0 I have ported and simplified this content into a proposed set of patches to the CryptoPkg. It uses a structured PCD to configure the services mapped into the Protocols/PPIs and avoids the issue Nate notes below with protocols and PPIs including all of the BaseCryptLib services. The structured PCD allows families of crypto services or individual services within a family to be enabled/disabled. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Pr oposal_V5&a mp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb 2a91f4e8c2a cb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7 C0%7C6371 03092953247213&sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5 dW8TEhs9 USyDM%3D&reserved=0 For example, the DSC file PCD statements to enable the SHA1 family and SHA256 family of hash services with the HashAll service disabled is: [PcdsFixedAtBuild] gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Services.Has hAll \| FALSE gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Services. HashAll \| FALSE Please take a look at this proposal and let me know if this can be used to address. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0 There is currently a limitation in the structured PCD feature that does not allow the structured PCD field values to be set in the scope of a module in a <PcdsFixedAtBuild> section. To work around this limitation, the CryptoPkg DSC file has a define called CRYPTO_SERVICES that can be set to ALL, NONE, MIN_PEI, or MIN_DXE_MIN_SMM. The default is ALL. Building with each of these values will build the modules with different sets of enabled services that matches the services enabled using multiple modules in the work from Sean and Matt. If this limitation is addressed in BaseTools, then CryptoPkg could remove the CRYPTO_SERVIES define and all !if statements. Thanks, Mike -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Nate DeSimone Sent: Wednesday, November 20, 2019 10:31 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Amol, With regard to verifying code hashes, it makes much more sense to me to have a GUID defined that represents the hash function to use versus a PCD. The reason for this is the method for signing Firmware Volumes typically involves using nested FVs where the GUID for the nested FV type indicates the algorithm needed to "extract" the nested FV. Extraction in this context is used to verify code hashes as well as decompress compressed code. The "extractor" is usually a statically linked library attached to either SignedSectionExtractPei and/or DxeIpl. For example, it is very possible for a platform to use more than one hashing algorithm, I can think of more than one closed source platform at Intel that actually does this. Now, one thing that I think your document implies is that we should have better APIs for crypto operations in general. If that was the intent then I totally agree with you. There are several reasons that I think this should be done. First, the API for OpenSSL has a tendency to change often enough to make integrating new versions into older codebases a headache, putting an abstraction layer on it would be helpful. Second, the method used to consume OpenSSL in edk2 causes some technical issues. Specifically, OpenSSL is statically linked and it defines a crazy number of global variables, enough so that on a debug build I have noticed roughly a 20KB increase in binary size even if you don't call a single function in OpenSSL. Wrapping the crypto operations in a PPI/Protocol so that only 1 driver needs to link to OpenSSL would probably help reduce binary size for most implementations. Your current document does not define a PPI/Protocol, that is something I would recommend. One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol have run into is that all the crypto functions need to be linked, even the ones that a platform does not use, which results in a net negative binary size impact. Accordingly, one would likely need to break the crypto functionality into multiple PPIs/Protocols so that only the functions a platform needs are actually included. I think some experiments need to be done to figure out the most optimal division of crypto APIs. Hope that helps, Nate -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 20, 2019 8:28 AM To: rfc@edk2.groups.io Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Importance: High Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0. You can also provide the feedback in the Bugzilla. Thanks, Amol
More All Messages By This Member
Sukerkar, Amol N #203 Hi Mike, I think the point # 2 you are making is also the implementation we have proposed. Also, if we can have a two tier approach then we may be in a position to improve code flexibility: 1. Define FixedAtBuild PCD at SecurityPkg DSC file that can be used by each module intending to use system level hashing algorithm. 2. For modules that need a module-specific hashing algorithm, override the PCD at SecurityPkg DSC file with a FIxedAtBuild PCD at platform level DSC file. This can eliminate the need for each module to be forced to define the PCD at platform level. Thanks, Amol toggle quoted message Show quoted text -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Tuesday, November 26, 2019 6:24 PM To: Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 As Sean points out, different components in a platform may have different hash requirements. If we want to go down the path where we have a few modules that produce Protocols/PPIs for the BaseCryptLib so all modules can share the same crypto services, then each module need to decide which specific hash algorithm it requires. Instead of defining another protocol/PPI for a unified hash service, we could consider a couple of other options: 1) Each module that uses hash services could use a FixedAtBuild PCD to decide which hash algorithm it should use. Based on this PCD value, different APIs in BaseCryptLib can be called. Since it is a FixedAtBuild PCD, the compiler/linker optimizations will only include the one hash algorithm selected. by using BaseCryptLib, this is compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs. 2) If we have many modules that need to make a hash algorithm selection, then define a new library class with the unified hash APIs, and the library instance uses a FixedAtBuild PCD to configure the hash algorithm used. This is also compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs and minimizes the sources in modules that need a design that supports flexible hash algorithm selection. If new hash algorithm is added, then only the lib instance and PCD need to be updated. Each module sets the FixedAtBuild PCD value in the scope of the module in the platform DSC file. If most or all modules in a platform need the same hash algorithm, then the FixedAtBuild PCD can be set at the platform scope in the platform DSC file. Best regards, Mike -----Original Message----- From: Wang, Jian J <jian.j.wang@...> Sent: Monday, November 25, 2019 9:09 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean and Amol, I believe the OBB and FV/PE image verification can still benefit from the unified API, if we add one more parameter to HashApiIinit() to force using a hash algorithm or add a PCD to choose one or link only required HashApiInstanceXxx. (Just for example) HashApiInit ( IN EFI_GUID HashAlg, OUT HASH_HANDLE HashHandle ) In this way, we can avoid hard-code hash API in the verification module. Although it's slow, I think the OBB verification or FV/PE image signature algorithm is still subject to change per security requirement change. During a transition period, we don't have to add a new code to handle a different algorithm in verification module. Instead we just need to link a different hash lib or set PCD to use a different algorithm. I think this makes things simpler and easier to change. The BaseCryptLib implemented in PPI/Protocol is a good idea to save image size. I think the Unified hash API (BaseHashLib) can make use of it to save extra hash algorithm library instances (HashApiInstanceSha1, Sha256,...). The only problem is that the structured PCD, like Mike mentioned before, doesn't support module override, which prevents different modules from using different algorithms and then cannot save module size utter mostly. Regards, Jian -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, November 26, 2019 5:53 AM To: rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean, A few examples that we want to target are - HDD Password verification - Capsule Verification (Platform recovery) - Bios Guard Platform Data Table - More importantly, Boot Guard Event Log driver that has a bunch of redundant code today calculating various hashes. I don't think this is open source though. Currently, the hash verification mechanism in the above features is hard-coded to SHA256 which we will need to change to SHA384 to support a more robust hashing mechanism. This is a good opportunity to replace the existing SHA256 API with an abstracted unified API. And if necessary, the abstracted unified API still supports driver specific hashing algorithm if needed (as opposed to the strongest hashing algorithm supported by the system). We are not targeting verification of OBB or for that matter any PE/COFF image with this mechanism, mainly because it has it's own wrapper and verification mechanism in DxeImageVerificationLib. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sean via Groups.Io Sent: Monday, November 25, 2019 1:24 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, I am interested to hear more about the actual use cases. I like the idea of abstracting the API so that a calling driver doesn't have to change when the hashing algorithm requirements change but am not convinced that this will actually be helpful (or at least worth the introduction of more complexity in security sensitive code). For the TPM and measured boot I agree. Random driver needs to extend a measurement then it can use the existing hashlib and it will be hashed and extended per platform policy. This is because the random driver doesn't need to understand the result. Only the TPM needs to and that is handled within the extend operation. For the other drivers that I have seen using hashing, they need to understand the result and make driver specific decisions based on the result. For example if I am verifying the OBB or some FV then I must make sure the known good and computed match. This driver must locate the known good and thus this process generally must know the format of the result. For this I think it is best to know what algorithm I am using and I don't see a strong use case to change algorithm to an abstracted algorithm. Do you have a set of example drivers/use cases that could use abstracted hash support? Thanks Sean -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N via Groups.Io Sent: Monday, November 25, 2019 12:08 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sean Brogan <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [EXTERNAL] Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike and Nate, With our implementation we are trying to address the following in EDKII: 1. A common Hashing API for UEFI drivers to consume instead of the current API that directly calls into the specific hashing algorithm, for instance, SHA1_Init, SHA256_Update, etc. Due to a stronger hashing algorithm requirement, certain drivers need to be upgraded to SHA384 from SHA256 and the unified API is proposed, so, while upgrading we remove the hard-coded dependency on each hashing algorithm API. 2. The size issue as a result of statically linking openssl/cryptolib API to the consuming driver. There are two ways we tried to address this issue, although, we didn't explicitly defined a PPI to address it. a. Introduced a fixed PCD per hashing algorithm to include/exclude a particular algorithm from BIOS. b. Use a register mechanism to link the instance of a particular hashing algorithm to Unified API library. This is configurable using PCD. Note that 2.a and 2.b implementation is very similar to HashLibBaseCryptoRouter used for TPM2 operations. From the size and static linking perspective, I believe we can have a discussion between 2.a and 2.b with Mike's implementation(https://nam06.safelinks.protection.outlo ok.com/?url=htt ps%3A %2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_P PI_Protocol_ Proposal_V5&data=02%7C01%7Csean.brogan%40microsoft. com%7Cd110f db2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd 011db47%7C1 %7C0%7C637103092953247213&sdata=2acK0dmJZ85aZ3LvOOq D3e3w44V EM5dW8TEhs9USyDM%3D&reserved=0) below. We still need a common hashing API so the hashing algorithm strength for a particular driver can be increased without having to modify the consumer driver code. Comments are welcome. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Thursday, November 21, 2019 4:45 PM To: rfc@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Nate and Mike! I am going through the code and comments and will respond shortly. In the meantime, here is the GitHub link to my PoC for the community to look at and comment: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fansukerk%2Fedk2&data=02%7C01%7Csean.brogan%40m icrosoft.c om%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141a f91ab2d7cd 011db47%7C1%7C0%7C637103092953247213&sdata=keRdfU%2 BxffkBsyv YHwvSOIA8ryexOigiKhR%2B%2F1q7DjE%3D&reserved=0. Best regards, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael D Kinney Sent: Thursday, November 21, 2019 4:37 PM To: rfc@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Nate and Amol, There is some work already started by Sean and Matt that implements a PEIM, DXE Driver, and SMM Driver to produce Protocol/PPI that wraps the BaseCryptLib services. This content broken out into its own package is available here: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmicrosoft%2Fmu_plus%2Ftree%2Fdev%2F201908%2FShared CryptoPkg& amp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fd b2a91f4e8c2 acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1% 7C0%7C637 103092953247213&sdata=88TRhUqsVrj7sx8P4hjbdLy2%2BDK nrBoi7HAI3G UhpPo%3D&reserved=0 I have ported and simplified this content into a proposed set of patches to the CryptoPkg. It uses a structured PCD to configure the services mapped into the Protocols/PPIs and avoids the issue Nate notes below with protocols and PPIs including all of the BaseCryptLib services. The structured PCD allows families of crypto services or individual services within a family to be enabled/disabled. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Pr oposal_V5&a mp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb 2a91f4e8c2a cb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7 C0%7C6371 03092953247213&sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5 dW8TEhs9 USyDM%3D&reserved=0 For example, the DSC file PCD statements to enable the SHA1 family and SHA256 family of hash services with the HashAll service disabled is: [PcdsFixedAtBuild] gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Services.Has hAll \| FALSE gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Services. HashAll \| FALSE Please take a look at this proposal and let me know if this can be used to address. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0 There is currently a limitation in the structured PCD feature that does not allow the structured PCD field values to be set in the scope of a module in a <PcdsFixedAtBuild> section. To work around this limitation, the CryptoPkg DSC file has a define called CRYPTO_SERVICES that can be set to ALL, NONE, MIN_PEI, or MIN_DXE_MIN_SMM. The default is ALL. Building with each of these values will build the modules with different sets of enabled services that matches the services enabled using multiple modules in the work from Sean and Matt. If this limitation is addressed in BaseTools, then CryptoPkg could remove the CRYPTO_SERVIES define and all !if statements. Thanks, Mike -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Nate DeSimone Sent: Wednesday, November 20, 2019 10:31 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Amol, With regard to verifying code hashes, it makes much more sense to me to have a GUID defined that represents the hash function to use versus a PCD. The reason for this is the method for signing Firmware Volumes typically involves using nested FVs where the GUID for the nested FV type indicates the algorithm needed to "extract" the nested FV. Extraction in this context is used to verify code hashes as well as decompress compressed code. The "extractor" is usually a statically linked library attached to either SignedSectionExtractPei and/or DxeIpl. For example, it is very possible for a platform to use more than one hashing algorithm, I can think of more than one closed source platform at Intel that actually does this. Now, one thing that I think your document implies is that we should have better APIs for crypto operations in general. If that was the intent then I totally agree with you. There are several reasons that I think this should be done. First, the API for OpenSSL has a tendency to change often enough to make integrating new versions into older codebases a headache, putting an abstraction layer on it would be helpful. Second, the method used to consume OpenSSL in edk2 causes some technical issues. Specifically, OpenSSL is statically linked and it defines a crazy number of global variables, enough so that on a debug build I have noticed roughly a 20KB increase in binary size even if you don't call a single function in OpenSSL. Wrapping the crypto operations in a PPI/Protocol so that only 1 driver needs to link to OpenSSL would probably help reduce binary size for most implementations. Your current document does not define a PPI/Protocol, that is something I would recommend. One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol have run into is that all the crypto functions need to be linked, even the ones that a platform does not use, which results in a net negative binary size impact. Accordingly, one would likely need to break the crypto functionality into multiple PPIs/Protocols so that only the functions a platform needs are actually included. I think some experiments need to be done to figure out the most optimal division of crypto APIs. Hope that helps, Nate -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 20, 2019 8:28 AM To: rfc@edk2.groups.io Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Importance: High Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0. You can also provide the feedback in the Bugzilla. Thanks, Amol
More All Messages By This Member
Sukerkar, Amol N #219 Hello everyone, Checking if there are any review comments for https://github.com/ansukerk/edk2. Do I need to submit patches in any other format? Thanks, Amol toggle quoted message Show quoted text -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 27, 2019 11:39 AM To: Kinney, Michael D <michael.d.kinney@...>; Wang, Jian J <jian.j.wang@...>; rfc@edk2.groups.io; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike, I think the point # 2 you are making is also the implementation we have proposed. Also, if we can have a two tier approach then we may be in a position to improve code flexibility: 1. Define FixedAtBuild PCD at SecurityPkg DSC file that can be used by each module intending to use system level hashing algorithm. 2. For modules that need a module-specific hashing algorithm, override the PCD at SecurityPkg DSC file with a FIxedAtBuild PCD at platform level DSC file. This can eliminate the need for each module to be forced to define the PCD at platform level. Thanks, Amol -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Tuesday, November 26, 2019 6:24 PM To: Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 As Sean points out, different components in a platform may have different hash requirements. If we want to go down the path where we have a few modules that produce Protocols/PPIs for the BaseCryptLib so all modules can share the same crypto services, then each module need to decide which specific hash algorithm it requires. Instead of defining another protocol/PPI for a unified hash service, we could consider a couple of other options: 1) Each module that uses hash services could use a FixedAtBuild PCD to decide which hash algorithm it should use. Based on this PCD value, different APIs in BaseCryptLib can be called. Since it is a FixedAtBuild PCD, the compiler/linker optimizations will only include the one hash algorithm selected. by using BaseCryptLib, this is compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs. 2) If we have many modules that need to make a hash algorithm selection, then define a new library class with the unified hash APIs, and the library instance uses a FixedAtBuild PCD to configure the hash algorithm used. This is also compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs and minimizes the sources in modules that need a design that supports flexible hash algorithm selection. If new hash algorithm is added, then only the lib instance and PCD need to be updated. Each module sets the FixedAtBuild PCD value in the scope of the module in the platform DSC file. If most or all modules in a platform need the same hash algorithm, then the FixedAtBuild PCD can be set at the platform scope in the platform DSC file. Best regards, Mike -----Original Message----- From: Wang, Jian J <jian.j.wang@...> Sent: Monday, November 25, 2019 9:09 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean and Amol, I believe the OBB and FV/PE image verification can still benefit from the unified API, if we add one more parameter to HashApiIinit() to force using a hash algorithm or add a PCD to choose one or link only required HashApiInstanceXxx. (Just for example) HashApiInit ( IN EFI_GUID HashAlg, OUT HASH_HANDLE HashHandle ) In this way, we can avoid hard-code hash API in the verification module. Although it's slow, I think the OBB verification or FV/PE image signature algorithm is still subject to change per security requirement change. During a transition period, we don't have to add a new code to handle a different algorithm in verification module. Instead we just need to link a different hash lib or set PCD to use a different algorithm. I think this makes things simpler and easier to change. The BaseCryptLib implemented in PPI/Protocol is a good idea to save image size. I think the Unified hash API (BaseHashLib) can make use of it to save extra hash algorithm library instances (HashApiInstanceSha1, Sha256,...). The only problem is that the structured PCD, like Mike mentioned before, doesn't support module override, which prevents different modules from using different algorithms and then cannot save module size utter mostly. Regards, Jian -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, November 26, 2019 5:53 AM To: rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean, A few examples that we want to target are - HDD Password verification - Capsule Verification (Platform recovery) - Bios Guard Platform Data Table - More importantly, Boot Guard Event Log driver that has a bunch of redundant code today calculating various hashes. I don't think this is open source though. Currently, the hash verification mechanism in the above features is hard-coded to SHA256 which we will need to change to SHA384 to support a more robust hashing mechanism. This is a good opportunity to replace the existing SHA256 API with an abstracted unified API. And if necessary, the abstracted unified API still supports driver specific hashing algorithm if needed (as opposed to the strongest hashing algorithm supported by the system). We are not targeting verification of OBB or for that matter any PE/COFF image with this mechanism, mainly because it has it's own wrapper and verification mechanism in DxeImageVerificationLib. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sean via Groups.Io Sent: Monday, November 25, 2019 1:24 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, I am interested to hear more about the actual use cases. I like the idea of abstracting the API so that a calling driver doesn't have to change when the hashing algorithm requirements change but am not convinced that this will actually be helpful (or at least worth the introduction of more complexity in security sensitive code). For the TPM and measured boot I agree. Random driver needs to extend a measurement then it can use the existing hashlib and it will be hashed and extended per platform policy. This is because the random driver doesn't need to understand the result. Only the TPM needs to and that is handled within the extend operation. For the other drivers that I have seen using hashing, they need to understand the result and make driver specific decisions based on the result. For example if I am verifying the OBB or some FV then I must make sure the known good and computed match. This driver must locate the known good and thus this process generally must know the format of the result. For this I think it is best to know what algorithm I am using and I don't see a strong use case to change algorithm to an abstracted algorithm. Do you have a set of example drivers/use cases that could use abstracted hash support? Thanks Sean -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N via Groups.Io Sent: Monday, November 25, 2019 12:08 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sean Brogan <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [EXTERNAL] Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike and Nate, With our implementation we are trying to address the following in EDKII: 1. A common Hashing API for UEFI drivers to consume instead of the current API that directly calls into the specific hashing algorithm, for instance, SHA1_Init, SHA256_Update, etc. Due to a stronger hashing algorithm requirement, certain drivers need to be upgraded to SHA384 from SHA256 and the unified API is proposed, so, while upgrading we remove the hard-coded dependency on each hashing algorithm API. 2. The size issue as a result of statically linking openssl/cryptolib API to the consuming driver. There are two ways we tried to address this issue, although, we didn't explicitly defined a PPI to address it. a. Introduced a fixed PCD per hashing algorithm to include/exclude a particular algorithm from BIOS. b. Use a register mechanism to link the instance of a particular hashing algorithm to Unified API library. This is configurable using PCD. Note that 2.a and 2.b implementation is very similar to HashLibBaseCryptoRouter used for TPM2 operations. From the size and static linking perspective, I believe we can have a discussion between 2.a and 2.b with Mike's implementation(https://nam06.safelinks.protection.outlo ok.com/?url=htt ps%3A %2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_P PI_Protocol_ Proposal_V5&data=02%7C01%7Csean.brogan%40microsoft. com%7Cd110f db2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd 011db47%7C1 %7C0%7C637103092953247213&sdata=2acK0dmJZ85aZ3LvOOq D3e3w44V EM5dW8TEhs9USyDM%3D&reserved=0) below. We still need a common hashing API so the hashing algorithm strength for a particular driver can be increased without having to modify the consumer driver code. Comments are welcome. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Thursday, November 21, 2019 4:45 PM To: rfc@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Nate and Mike! I am going through the code and comments and will respond shortly. In the meantime, here is the GitHub link to my PoC for the community to look at and comment: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fansukerk%2Fedk2&data=02%7C01%7Csean.brogan%40m icrosoft.c om%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141a f91ab2d7cd 011db47%7C1%7C0%7C637103092953247213&sdata=keRdfU%2 BxffkBsyv YHwvSOIA8ryexOigiKhR%2B%2F1q7DjE%3D&reserved=0. Best regards, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael D Kinney Sent: Thursday, November 21, 2019 4:37 PM To: rfc@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Nate and Amol, There is some work already started by Sean and Matt that implements a PEIM, DXE Driver, and SMM Driver to produce Protocol/PPI that wraps the BaseCryptLib services. This content broken out into its own package is available here: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmicrosoft%2Fmu_plus%2Ftree%2Fdev%2F201908%2FShared CryptoPkg& amp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fd b2a91f4e8c2 acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1% 7C0%7C637 103092953247213&sdata=88TRhUqsVrj7sx8P4hjbdLy2%2BDK nrBoi7HAI3G UhpPo%3D&reserved=0 I have ported and simplified this content into a proposed set of patches to the CryptoPkg. It uses a structured PCD to configure the services mapped into the Protocols/PPIs and avoids the issue Nate notes below with protocols and PPIs including all of the BaseCryptLib services. The structured PCD allows families of crypto services or individual services within a family to be enabled/disabled. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Pr oposal_V5&a mp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb 2a91f4e8c2a cb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7 C0%7C6371 03092953247213&sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5 dW8TEhs9 USyDM%3D&reserved=0 For example, the DSC file PCD statements to enable the SHA1 family and SHA256 family of hash services with the HashAll service disabled is: [PcdsFixedAtBuild] gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Services.Has hAll \| FALSE gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Services. HashAll \| FALSE Please take a look at this proposal and let me know if this can be used to address. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0 There is currently a limitation in the structured PCD feature that does not allow the structured PCD field values to be set in the scope of a module in a <PcdsFixedAtBuild> section. To work around this limitation, the CryptoPkg DSC file has a define called CRYPTO_SERVICES that can be set to ALL, NONE, MIN_PEI, or MIN_DXE_MIN_SMM. The default is ALL. Building with each of these values will build the modules with different sets of enabled services that matches the services enabled using multiple modules in the work from Sean and Matt. If this limitation is addressed in BaseTools, then CryptoPkg could remove the CRYPTO_SERVIES define and all !if statements. Thanks, Mike -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Nate DeSimone Sent: Wednesday, November 20, 2019 10:31 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Amol, With regard to verifying code hashes, it makes much more sense to me to have a GUID defined that represents the hash function to use versus a PCD. The reason for this is the method for signing Firmware Volumes typically involves using nested FVs where the GUID for the nested FV type indicates the algorithm needed to "extract" the nested FV. Extraction in this context is used to verify code hashes as well as decompress compressed code. The "extractor" is usually a statically linked library attached to either SignedSectionExtractPei and/or DxeIpl. For example, it is very possible for a platform to use more than one hashing algorithm, I can think of more than one closed source platform at Intel that actually does this. Now, one thing that I think your document implies is that we should have better APIs for crypto operations in general. If that was the intent then I totally agree with you. There are several reasons that I think this should be done. First, the API for OpenSSL has a tendency to change often enough to make integrating new versions into older codebases a headache, putting an abstraction layer on it would be helpful. Second, the method used to consume OpenSSL in edk2 causes some technical issues. Specifically, OpenSSL is statically linked and it defines a crazy number of global variables, enough so that on a debug build I have noticed roughly a 20KB increase in binary size even if you don't call a single function in OpenSSL. Wrapping the crypto operations in a PPI/Protocol so that only 1 driver needs to link to OpenSSL would probably help reduce binary size for most implementations. Your current document does not define a PPI/Protocol, that is something I would recommend. One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol have run into is that all the crypto functions need to be linked, even the ones that a platform does not use, which results in a net negative binary size impact. Accordingly, one would likely need to break the crypto functionality into multiple PPIs/Protocols so that only the functions a platform needs are actually included. I think some experiments need to be done to figure out the most optimal division of crypto APIs. Hope that helps, Nate -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 20, 2019 8:28 AM To: rfc@edk2.groups.io Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Importance: High Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0. You can also provide the feedback in the Bugzilla. Thanks, Amol
More All Messages By This Member
Michael D Kinney #220 Amol, I would recommend a different set of APIs in the HashLib class. Instead of doing a registration, define a PCD to select the hash type and have the generic HashLib function call the BaseCryptLib APIs based on the PCD value. If the PCD is type FixedAtBuild only the hash functions specified by the PCD are included in the executable. I would also recommend the HashLib and the PCD be added to the CryptoPkg instead of the SecurityPkg. The BaseCryptLib hash families today are: MD4, MD5, SHA1, SHA256, SHA384, SHA512 SM3 Proposed FixedAtBuild PCD to declare the hash family of APIs Provided by the HashLib. [PcdsFixedAtBuild] # Family of hash API provided by the HashLib. Can be set at # global platform scope or module specific scope. # # 0x00 - No hash family selected. All HashLib APIs ASSERT(). # 0x01 - HashLib provides MD4 family of APIs. # 0x02 - HashLib provides MD5 family of APIs. # 0x03 - HashLib provides SHA1 family of APIs. # 0x04 - HashLib provides SHA256 family of APIs. # 0x05 - HashLib provides SHA384 family of APIs. # 0x06 - HashLib provides SHA512 family of APIs. # gEfiCryptoPkgTokenSpaceGuid.PcdHashLibFamily\|0x00\|UINT8\|0x00000002 The proposed HashLib APIs are shown below and are based on the BaseCryptLib hash APIs. This would allow existing modules that are currently using BaseCryptLib to be easily updated to use the HashLib APIs and can be easily configured with a PCD to select a specific Family of hash services. Can add #defines for the families to remove use of hard coded values. UINTN EFIAPI HashGetContextSize ( VOID ) { switch (PcdGet8 (PcdHashLibFamily)) { case 0x01: return Md4GetContextSize (); case 0x02: return Md5GetContextSize (); case 0x03: return Sha1GetContextSize (); case 0x04: return Sha256GetContextSize (); case 0x05: return Sha384GetContextSize (); case 0x06: return Sha512GetContextSize (); case 0x00: default: ASSERT (FALSE); return 0; } BOOLEAN EFIAPI HashInit ( OUT VOID HashContext ); BOOLEAN EFIAPI HashDuplicate ( IN CONST VOID HashContext, OUT VOID NewHashContext ); BOOLEAN EFIAPI HashUpdate ( IN OUT VOID HashContext, IN CONST VOID Data, IN UINTN DataSize ); BOOLEAN EFIAPI HashFinal ( IN OUT VOID HashContext, OUT UINT8 HashValue ); BOOLEAN EFIAPI HashAll ( IN CONST VOID Data, IN UINTN DataSize, OUT UINT8 HashValue ); Mike toggle quoted message Show quoted text -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, December 17, 2019 10:23 AM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hello everyone, Checking if there are any review comments for https://github.com/ansukerk/edk2. Do I need to submit patches in any other format? Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 27, 2019 11:39 AM To: Kinney, Michael D <michael.d.kinney@...>; Wang, Jian J <jian.j.wang@...>; rfc@edk2.groups.io; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike, I think the point # 2 you are making is also the implementation we have proposed. Also, if we can have a two tier approach then we may be in a position to improve code flexibility: 1. Define FixedAtBuild PCD at SecurityPkg DSC file that can be used by each module intending to use system level hashing algorithm. 2. For modules that need a module-specific hashing algorithm, override the PCD at SecurityPkg DSC file with a FIxedAtBuild PCD at platform level DSC file. This can eliminate the need for each module to be forced to define the PCD at platform level. Thanks, Amol -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Tuesday, November 26, 2019 6:24 PM To: Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 As Sean points out, different components in a platform may have different hash requirements. If we want to go down the path where we have a few modules that produce Protocols/PPIs for the BaseCryptLib so all modules can share the same crypto services, then each module need to decide which specific hash algorithm it requires. Instead of defining another protocol/PPI for a unified hash service, we could consider a couple of other options: 1) Each module that uses hash services could use a FixedAtBuild PCD to decide which hash algorithm it should use. Based on this PCD value, different APIs in BaseCryptLib can be called. Since it is a FixedAtBuild PCD, the compiler/linker optimizations will only include the one hash algorithm selected. by using BaseCryptLib, this is compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs. 2) If we have many modules that need to make a hash algorithm selection, then define a new library class with the unified hash APIs, and the library instance uses a FixedAtBuild PCD to configure the hash algorithm used. This is also compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs and minimizes the sources in modules that need a design that supports flexible hash algorithm selection. If new hash algorithm is added, then only the lib instance and PCD need to be updated. Each module sets the FixedAtBuild PCD value in the scope of the module in the platform DSC file. If most or all modules in a platform need the same hash algorithm, then the FixedAtBuild PCD can be set at the platform scope in the platform DSC file. Best regards, Mike -----Original Message----- From: Wang, Jian J <jian.j.wang@...> Sent: Monday, November 25, 2019 9:09 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean and Amol, I believe the OBB and FV/PE image verification can still benefit from the unified API, if we add one more parameter to HashApiIinit() to force using a hash algorithm or add a PCD to choose one or link only required HashApiInstanceXxx. (Just for example) HashApiInit ( IN EFI_GUID HashAlg, OUT HASH_HANDLE *HashHandle ) In this way, we can avoid hard-code hash API in the verification module. Although it's slow, I think the OBB verification or FV/PE image signature algorithm is still subject to change per security requirement change. During a transition period, we don't have to add a new code to handle a different algorithm in verification module. Instead we just need to link a different hash lib or set PCD to use a different algorithm. I think this makes things simpler and easier to change. The BaseCryptLib implemented in PPI/Protocol is a good idea to save image size. I think the Unified hash API (BaseHashLib) can make use of it to save extra hash algorithm library instances (HashApiInstanceSha1, Sha256,...). The only problem is that the structured PCD, like Mike mentioned before, doesn't support module override, which prevents different modules from using different algorithms and then cannot save module size utter mostly. Regards, Jian -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, November 26, 2019 5:53 AM To: rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean, A few examples that we want to target are - HDD Password verification - Capsule Verification (Platform recovery) - Bios Guard Platform Data Table - More importantly, Boot Guard Event Log driver that has a bunch of redundant code today calculating various hashes. I don't think this is open source though. Currently, the hash verification mechanism in the above features is hard-coded to SHA256 which we will need to change to SHA384 to support a more robust hashing mechanism. This is a good opportunity to replace the existing SHA256 API with an abstracted unified API. And if necessary, the abstracted unified API still supports driver specific hashing algorithm if needed (as opposed to the strongest hashing algorithm supported by the system). We are not targeting verification of OBB or for that matter any PE/COFF image with this mechanism, mainly because it has it's own wrapper and verification mechanism in DxeImageVerificationLib. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sean via Groups.Io Sent: Monday, November 25, 2019 1:24 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, I am interested to hear more about the actual use cases. I like the idea of abstracting the API so that a calling driver doesn't have to change when the hashing algorithm requirements change but am not convinced that this will actually be helpful (or at least worth the introduction of more complexity in security sensitive code). For the TPM and measured boot I agree. Random driver needs to extend a measurement then it can use the existing hashlib and it will be hashed and extended per platform policy. This is because the random driver doesn't need to understand the result. Only the TPM needs to and that is handled within the extend operation. For the other drivers that I have seen using hashing, they need to understand the result and make driver specific decisions based on the result. For example if I am verifying the OBB or some FV then I must make sure the known good and computed match. This driver must locate the known good and thus this process generally must know the format of the result. For this I think it is best to know what algorithm I am using and I don't see a strong use case to change algorithm to an abstracted algorithm. Do you have a set of example drivers/use cases that could use abstracted hash support? Thanks Sean -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N via Groups.Io Sent: Monday, November 25, 2019 12:08 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sean Brogan <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [EXTERNAL] Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike and Nate, With our implementation we are trying to address the following in EDKII: 1. A common Hashing API for UEFI drivers to consume instead of the current API that directly calls into the specific hashing algorithm, for instance, SHA1_Init, SHA256_Update, etc. Due to a stronger hashing algorithm requirement, certain drivers need to be upgraded to SHA384 from SHA256 and the unified API is proposed, so, while upgrading we remove the hard-coded dependency on each hashing algorithm API. 2. The size issue as a result of statically linking openssl/cryptolib API to the consuming driver. There are two ways we tried to address this issue, although, we didn't explicitly defined a PPI to address it. a. Introduced a fixed PCD per hashing algorithm to include/exclude a particular algorithm from BIOS. b. Use a register mechanism to link the instance of a particular hashing algorithm to Unified API library. This is configurable using PCD. Note that 2.a and 2.b implementation is very similar to HashLibBaseCryptoRouter used for TPM2 operations. From the size and static linking perspective, I believe we can have a discussion between 2.a and 2.b with Mike's implementation(https://nam06.safelinks.protection.outlo ok.com/?url=htt ps%3A %2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_P PI_Protocol_ Proposal_V5&data=02%7C01%7Csean.brogan%40microsoft. com%7Cd110f db2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd 011db47%7C1 %7C0%7C637103092953247213&sdata=2acK0dmJZ85aZ3LvOOq D3e3w44V EM5dW8TEhs9USyDM%3D&reserved=0) below. We still need a common hashing API so the hashing algorithm strength for a particular driver can be increased without having to modify the consumer driver code. Comments are welcome. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Thursday, November 21, 2019 4:45 PM To: rfc@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Nate and Mike! I am going through the code and comments and will respond shortly. In the meantime, here is the GitHub link to my PoC for the community to look at and comment: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fansukerk%2Fedk2&data=02%7C01%7Csean.brogan%40m icrosoft.c om%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141a f91ab2d7cd 011db47%7C1%7C0%7C637103092953247213&sdata=keRdfU%2 BxffkBsyv YHwvSOIA8ryexOigiKhR%2B%2F1q7DjE%3D&reserved=0. Best regards, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael D Kinney Sent: Thursday, November 21, 2019 4:37 PM To: rfc@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Nate and Amol, There is some work already started by Sean and Matt that implements a PEIM, DXE Driver, and SMM Driver to produce Protocol/PPI that wraps the BaseCryptLib services. This content broken out into its own package is available here: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmicrosoft%2Fmu_plus%2Ftree%2Fdev%2F201908%2FShared CryptoPkg& amp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fd b2a91f4e8c2 acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1% 7C0%7C637 103092953247213&sdata=88TRhUqsVrj7sx8P4hjbdLy2%2BDK nrBoi7HAI3G UhpPo%3D&reserved=0 I have ported and simplified this content into a proposed set of patches to the CryptoPkg. It uses a structured PCD to configure the services mapped into the Protocols/PPIs and avoids the issue Nate notes below with protocols and PPIs including all of the BaseCryptLib services. The structured PCD allows families of crypto services or individual services within a family to be enabled/disabled. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Pr oposal_V5&a mp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb 2a91f4e8c2a cb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7 C0%7C6371 03092953247213&sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5 dW8TEhs9 USyDM%3D&reserved=0 For example, the DSC file PCD statements to enable the SHA1 family and SHA256 family of hash services with the HashAll service disabled is: [PcdsFixedAtBuild] gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Services.Has hAll \| FALSE gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Services. HashAll \| FALSE Please take a look at this proposal and let me know if this can be used to address. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0 There is currently a limitation in the structured PCD feature that does not allow the structured PCD field values to be set in the scope of a module in a <PcdsFixedAtBuild> section. To work around this limitation, the CryptoPkg DSC file has a define called CRYPTO_SERVICES that can be set to ALL, NONE, MIN_PEI, or MIN_DXE_MIN_SMM. The default is ALL. Building with each of these values will build the modules with different sets of enabled services that matches the services enabled using multiple modules in the work from Sean and Matt. If this limitation is addressed in BaseTools, then CryptoPkg could remove the CRYPTO_SERVIES define and all !if statements. Thanks, Mike -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Nate DeSimone Sent: Wednesday, November 20, 2019 10:31 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Amol, With regard to verifying code hashes, it makes much more sense to me to have a GUID defined that represents the hash function to use versus a PCD. The reason for this is the method for signing Firmware Volumes typically involves using nested FVs where the GUID for the nested FV type indicates the algorithm needed to "extract" the nested FV. Extraction in this context is used to verify code hashes as well as decompress compressed code. The "extractor" is usually a statically linked library attached to either SignedSectionExtractPei and/or DxeIpl. For example, it is very possible for a platform to use more than one hashing algorithm, I can think of more than one closed source platform at Intel that actually does this. Now, one thing that I think your document implies is that we should have better APIs for crypto operations in general. If that was the intent then I totally agree with you. There are several reasons that I think this should be done. First, the API for OpenSSL has a tendency to change often enough to make integrating new versions into older codebases a headache, putting an abstraction layer on it would be helpful. Second, the method used to consume OpenSSL in edk2 causes some technical issues. Specifically, OpenSSL is statically linked and it defines a crazy number of global variables, enough so that on a debug build I have noticed roughly a 20KB increase in binary size even if you don't call a single function in OpenSSL. Wrapping the crypto operations in a PPI/Protocol so that only 1 driver needs to link to OpenSSL would probably help reduce binary size for most implementations. Your current document does not define a PPI/Protocol, that is something I would recommend. One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol have run into is that all the crypto functions need to be linked, even the ones that a platform does not use, which results in a net negative binary size impact. Accordingly, one would likely need to break the crypto functionality into multiple PPIs/Protocols so that only the functions a platform needs are actually included. I think some experiments need to be done to figure out the most optimal division of crypto APIs. Hope that helps, Nate -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 20, 2019 8:28 AM To: rfc@edk2.groups.io Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Importance: High Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0. You can also provide the feedback in the Bugzilla. Thanks, Amol
More All Messages By This Member
Sukerkar, Amol N #221 Thanks, Mike! Based on your suggestion below and keeping in line with our implementation, I would like to recommend the following changes. I will make those and upload for review: The PCD defined (this one at SecurityPkg level) will be as follows. Note that to keep it aligned with our implementation, we are keeping the bitmap "mutually exclusive" for each algorithm. [PcdsFixedAtBuild] ## This PCD indicates the HASH algorithm to verify unsigned PE/COFF image # Based on the value set, the required algorithm is chosen to verify # the unsigned image during Secure Boot.<BR> # The hashing algorithm selected must match the hashing algorithm used to # hash the image to be added to DB using tools such as KeyEnroll.<BR> # 0x00000001 - SHA1.<BR> # 0x00000002 - SHA256.<BR> # 0x00000004 - SHA384.<BR> # 0x00000008 - SHA512.<BR> # 0x00000010 - SM3_256.<BR> # 0x00000020 - MD4.<BR> # 0x00000040 - MD5.<BR> # @Prompt Set policy for hashing unsigned image for Secure Boot. # @ValidRange 0x80000001 \| 0x00000000 - 0x00000005 gEfiSecurityPkgTokenSpaceGuid.PcdSystemHashPolicy\|0x02\|UINT32\|0x00010024 the BaseHashLib API will provide the wrapper. As an example, for HashInit, the function definition would be EFI_STATUS EFIAPI HashApiInit ( OUT HASH_HANDLE HashHandle ) { . . . HashPolicy = PcdGet32 (PcdSystemHashPolicy); if ((mBaseHashInterfaceCount == 0) \|\| !(mCurrentHashMask & HashPolicy)) { return EFI_UNSUPPORTED; } HashCtx = AllocatePool (sizeof(HashCtx)); ASSERT (HashCtx != NULL); for (Index = 0; Index < mBaseHashInterfaceCount; Index++) { HashMask = GetApiHashMaskFromAlgo (&mHashOps[Index].HashGuid); if ((HashMask & HashPolicy) != 0) { mHashOps[Index].HashInit (HashCtx); break; } } HashHandle = (HASH_HANDLE)HashCtx; return EFI_SUCCESS; } And mHashOps[Index].HashInit will be registered to the PCD value of PcdSystemHashPolicy. For example, if SHA384 is selected as value of PcdSystemHashPolicy, The HashInit call will look like, EFI_STATUS EFIAPI Sha384_Init ( OUT HASH_HANDLE HashHandle ) { VOID Sha384Ctx; UINTN CtxSize; CtxSize = Sha384GetContextSize (); Sha384Ctx = AllocatePool (CtxSize); ASSERT (Sha384Ctx != NULL); Sha384Init (Sha384Ctx); HashHandle = (HASH_HANDLE)Sha384Ctx; return EFI_SUCCESS; } Do you see any issues/challenges with this implementation? Thanks, Amol toggle quoted message Show quoted text -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Tuesday, December 17, 2019 12:47 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, I would recommend a different set of APIs in the HashLib class. Instead of doing a registration, define a PCD to select the hash type and have the generic HashLib function call the BaseCryptLib APIs based on the PCD value. If the PCD is type FixedAtBuild only the hash functions specified by the PCD are included in the executable. I would also recommend the HashLib and the PCD be added to the CryptoPkg instead of the SecurityPkg. The BaseCryptLib hash families today are: MD4, MD5, SHA1, SHA256, SHA384, SHA512 SM3 Proposed FixedAtBuild PCD to declare the hash family of APIs Provided by the HashLib. [PcdsFixedAtBuild] # Family of hash API provided by the HashLib. Can be set at # global platform scope or module specific scope. # # 0x00 - No hash family selected. All HashLib APIs ASSERT(). # 0x01 - HashLib provides MD4 family of APIs. # 0x02 - HashLib provides MD5 family of APIs. # 0x03 - HashLib provides SHA1 family of APIs. # 0x04 - HashLib provides SHA256 family of APIs. # 0x05 - HashLib provides SHA384 family of APIs. # 0x06 - HashLib provides SHA512 family of APIs. # gEfiCryptoPkgTokenSpaceGuid.PcdHashLibFamily\|0x00\|UINT8\|0x00000002 The proposed HashLib APIs are shown below and are based on the BaseCryptLib hash APIs. This would allow existing modules that are currently using BaseCryptLib to be easily updated to use the HashLib APIs and can be easily configured with a PCD to select a specific Family of hash services. Can add #defines for the families to remove use of hard coded values. UINTN EFIAPI HashGetContextSize ( VOID ) { switch (PcdGet8 (PcdHashLibFamily)) { case 0x01: return Md4GetContextSize (); case 0x02: return Md5GetContextSize (); case 0x03: return Sha1GetContextSize (); case 0x04: return Sha256GetContextSize (); case 0x05: return Sha384GetContextSize (); case 0x06: return Sha512GetContextSize (); case 0x00: default: ASSERT (FALSE); return 0; } BOOLEAN EFIAPI HashInit ( OUT VOID HashContext ); BOOLEAN EFIAPI HashDuplicate ( IN CONST VOID HashContext, OUT VOID NewHashContext ); BOOLEAN EFIAPI HashUpdate ( IN OUT VOID HashContext, IN CONST VOID Data, IN UINTN DataSize ); BOOLEAN EFIAPI HashFinal ( IN OUT VOID HashContext, OUT UINT8 HashValue ); BOOLEAN EFIAPI HashAll ( IN CONST VOID Data, IN UINTN DataSize, OUT UINT8 HashValue ); Mike -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, December 17, 2019 10:23 AM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hello everyone, Checking if there are any review comments for https://github.com/ansukerk/edk2. Do I need to submit patches in any other format? Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 27, 2019 11:39 AM To: Kinney, Michael D <michael.d.kinney@...>; Wang, Jian J <jian.j.wang@...>; rfc@edk2.groups.io; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike, I think the point # 2 you are making is also the implementation we have proposed. Also, if we can have a two tier approach then we may be in a position to improve code flexibility: 1. Define FixedAtBuild PCD at SecurityPkg DSC file that can be used by each module intending to use system level hashing algorithm. 2. For modules that need a module-specific hashing algorithm, override the PCD at SecurityPkg DSC file with a FIxedAtBuild PCD at platform level DSC file. This can eliminate the need for each module to be forced to define the PCD at platform level. Thanks, Amol -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Tuesday, November 26, 2019 6:24 PM To: Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 As Sean points out, different components in a platform may have different hash requirements. If we want to go down the path where we have a few modules that produce Protocols/PPIs for the BaseCryptLib so all modules can share the same crypto services, then each module need to decide which specific hash algorithm it requires. Instead of defining another protocol/PPI for a unified hash service, we could consider a couple of other options: 1) Each module that uses hash services could use a FixedAtBuild PCD to decide which hash algorithm it should use. Based on this PCD value, different APIs in BaseCryptLib can be called. Since it is a FixedAtBuild PCD, the compiler/linker optimizations will only include the one hash algorithm selected. by using BaseCryptLib, this is compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs. 2) If we have many modules that need to make a hash algorithm selection, then define a new library class with the unified hash APIs, and the library instance uses a FixedAtBuild PCD to configure the hash algorithm used. This is also compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs and minimizes the sources in modules that need a design that supports flexible hash algorithm selection. If new hash algorithm is added, then only the lib instance and PCD need to be updated. Each module sets the FixedAtBuild PCD value in the scope of the module in the platform DSC file. If most or all modules in a platform need the same hash algorithm, then the FixedAtBuild PCD can be set at the platform scope in the platform DSC file. Best regards, Mike -----Original Message----- From: Wang, Jian J <jian.j.wang@...> Sent: Monday, November 25, 2019 9:09 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean and Amol, I believe the OBB and FV/PE image verification can still benefit from the unified API, if we add one more parameter to HashApiIinit() to force using a hash algorithm or add a PCD to choose one or link only required HashApiInstanceXxx. (Just for example) HashApiInit ( IN EFI_GUID HashAlg, OUT HASH_HANDLE *HashHandle ) In this way, we can avoid hard-code hash API in the verification module. Although it's slow, I think the OBB verification or FV/PE image signature algorithm is still subject to change per security requirement change. During a transition period, we don't have to add a new code to handle a different algorithm in verification module. Instead we just need to link a different hash lib or set PCD to use a different algorithm. I think this makes things simpler and easier to change. The BaseCryptLib implemented in PPI/Protocol is a good idea to save image size. I think the Unified hash API (BaseHashLib) can make use of it to save extra hash algorithm library instances (HashApiInstanceSha1, Sha256,...). The only problem is that the structured PCD, like Mike mentioned before, doesn't support module override, which prevents different modules from using different algorithms and then cannot save module size utter mostly. Regards, Jian -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, November 26, 2019 5:53 AM To: rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean, A few examples that we want to target are - HDD Password verification - Capsule Verification (Platform recovery) - Bios Guard Platform Data Table - More importantly, Boot Guard Event Log driver that has a bunch of redundant code today calculating various hashes. I don't think this is open source though. Currently, the hash verification mechanism in the above features is hard-coded to SHA256 which we will need to change to SHA384 to support a more robust hashing mechanism. This is a good opportunity to replace the existing SHA256 API with an abstracted unified API. And if necessary, the abstracted unified API still supports driver specific hashing algorithm if needed (as opposed to the strongest hashing algorithm supported by the system). We are not targeting verification of OBB or for that matter any PE/COFF image with this mechanism, mainly because it has it's own wrapper and verification mechanism in DxeImageVerificationLib. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sean via Groups.Io Sent: Monday, November 25, 2019 1:24 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, I am interested to hear more about the actual use cases. I like the idea of abstracting the API so that a calling driver doesn't have to change when the hashing algorithm requirements change but am not convinced that this will actually be helpful (or at least worth the introduction of more complexity in security sensitive code). For the TPM and measured boot I agree. Random driver needs to extend a measurement then it can use the existing hashlib and it will be hashed and extended per platform policy. This is because the random driver doesn't need to understand the result. Only the TPM needs to and that is handled within the extend operation. For the other drivers that I have seen using hashing, they need to understand the result and make driver specific decisions based on the result. For example if I am verifying the OBB or some FV then I must make sure the known good and computed match. This driver must locate the known good and thus this process generally must know the format of the result. For this I think it is best to know what algorithm I am using and I don't see a strong use case to change algorithm to an abstracted algorithm. Do you have a set of example drivers/use cases that could use abstracted hash support? Thanks Sean -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N via Groups.Io Sent: Monday, November 25, 2019 12:08 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sean Brogan <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [EXTERNAL] Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike and Nate, With our implementation we are trying to address the following in EDKII: 1. A common Hashing API for UEFI drivers to consume instead of the current API that directly calls into the specific hashing algorithm, for instance, SHA1_Init, SHA256_Update, etc. Due to a stronger hashing algorithm requirement, certain drivers need to be upgraded to SHA384 from SHA256 and the unified API is proposed, so, while upgrading we remove the hard-coded dependency on each hashing algorithm API. 2. The size issue as a result of statically linking openssl/cryptolib API to the consuming driver. There are two ways we tried to address this issue, although, we didn't explicitly defined a PPI to address it. a. Introduced a fixed PCD per hashing algorithm to include/exclude a particular algorithm from BIOS. b. Use a register mechanism to link the instance of a particular hashing algorithm to Unified API library. This is configurable using PCD. Note that 2.a and 2.b implementation is very similar to HashLibBaseCryptoRouter used for TPM2 operations. From the size and static linking perspective, I believe we can have a discussion between 2.a and 2.b with Mike's implementation(https://nam06.safelinks.protection.outlo ok.com/?url=htt ps%3A %2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_P PI_Protocol_ Proposal_V5&data=02%7C01%7Csean.brogan%40microsoft. com%7Cd110f db2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd 011db47%7C1 %7C0%7C637103092953247213&sdata=2acK0dmJZ85aZ3LvOOq D3e3w44V EM5dW8TEhs9USyDM%3D&reserved=0) below. We still need a common hashing API so the hashing algorithm strength for a particular driver can be increased without having to modify the consumer driver code. Comments are welcome. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Thursday, November 21, 2019 4:45 PM To: rfc@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Nate and Mike! I am going through the code and comments and will respond shortly. In the meantime, here is the GitHub link to my PoC for the community to look at and comment: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fansukerk%2Fedk2&data=02%7C01%7Csean.brogan%40m icrosoft.c om%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141a f91ab2d7cd 011db47%7C1%7C0%7C637103092953247213&sdata=keRdfU%2 BxffkBsyv YHwvSOIA8ryexOigiKhR%2B%2F1q7DjE%3D&reserved=0. Best regards, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael D Kinney Sent: Thursday, November 21, 2019 4:37 PM To: rfc@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Nate and Amol, There is some work already started by Sean and Matt that implements a PEIM, DXE Driver, and SMM Driver to produce Protocol/PPI that wraps the BaseCryptLib services. This content broken out into its own package is available here: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmicrosoft%2Fmu_plus%2Ftree%2Fdev%2F201908%2FShared CryptoPkg& amp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fd b2a91f4e8c2 acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1% 7C0%7C637 103092953247213&sdata=88TRhUqsVrj7sx8P4hjbdLy2%2BDK nrBoi7HAI3G UhpPo%3D&reserved=0 I have ported and simplified this content into a proposed set of patches to the CryptoPkg. It uses a structured PCD to configure the services mapped into the Protocols/PPIs and avoids the issue Nate notes below with protocols and PPIs including all of the BaseCryptLib services. The structured PCD allows families of crypto services or individual services within a family to be enabled/disabled. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Pr oposal_V5&a mp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb 2a91f4e8c2a cb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7 C0%7C6371 03092953247213&sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5 dW8TEhs9 USyDM%3D&reserved=0 For example, the DSC file PCD statements to enable the SHA1 family and SHA256 family of hash services with the HashAll service disabled is: [PcdsFixedAtBuild] gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Services.Has hAll \| FALSE gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Services. HashAll \| FALSE Please take a look at this proposal and let me know if this can be used to address. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0 There is currently a limitation in the structured PCD feature that does not allow the structured PCD field values to be set in the scope of a module in a <PcdsFixedAtBuild> section. To work around this limitation, the CryptoPkg DSC file has a define called CRYPTO_SERVICES that can be set to ALL, NONE, MIN_PEI, or MIN_DXE_MIN_SMM. The default is ALL. Building with each of these values will build the modules with different sets of enabled services that matches the services enabled using multiple modules in the work from Sean and Matt. If this limitation is addressed in BaseTools, then CryptoPkg could remove the CRYPTO_SERVIES define and all !if statements. Thanks, Mike -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Nate DeSimone Sent: Wednesday, November 20, 2019 10:31 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Amol, With regard to verifying code hashes, it makes much more sense to me to have a GUID defined that represents the hash function to use versus a PCD. The reason for this is the method for signing Firmware Volumes typically involves using nested FVs where the GUID for the nested FV type indicates the algorithm needed to "extract" the nested FV. Extraction in this context is used to verify code hashes as well as decompress compressed code. The "extractor" is usually a statically linked library attached to either SignedSectionExtractPei and/or DxeIpl. For example, it is very possible for a platform to use more than one hashing algorithm, I can think of more than one closed source platform at Intel that actually does this. Now, one thing that I think your document implies is that we should have better APIs for crypto operations in general. If that was the intent then I totally agree with you. There are several reasons that I think this should be done. First, the API for OpenSSL has a tendency to change often enough to make integrating new versions into older codebases a headache, putting an abstraction layer on it would be helpful. Second, the method used to consume OpenSSL in edk2 causes some technical issues. Specifically, OpenSSL is statically linked and it defines a crazy number of global variables, enough so that on a debug build I have noticed roughly a 20KB increase in binary size even if you don't call a single function in OpenSSL. Wrapping the crypto operations in a PPI/Protocol so that only 1 driver needs to link to OpenSSL would probably help reduce binary size for most implementations. Your current document does not define a PPI/Protocol, that is something I would recommend. One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol have run into is that all the crypto functions need to be linked, even the ones that a platform does not use, which results in a net negative binary size impact. Accordingly, one would likely need to break the crypto functionality into multiple PPIs/Protocols so that only the functions a platform needs are actually included. I think some experiments need to be done to figure out the most optimal division of crypto APIs. Hope that helps, Nate -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 20, 2019 8:28 AM To: rfc@edk2.groups.io Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Importance: High Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0. You can also provide the feedback in the Bugzilla. Thanks, Amol
More All Messages By This Member
Michael D Kinney #222 Amol, Your style requires bigger source changes to exiting modules to switch from BaseCryptLib to HashLib. Your style also required a loop to search for matching API on every call. Mine will optimize away the case statement into a direct call to the select hash family API. I do not prefer the mutually exclusive bitmap. An enumeration guarantees no ambiguity. Mike toggle quoted message Show quoted text -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, December 17, 2019 1:13 PM To: Kinney, Michael D <michael.d.kinney@...>; rfc@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Mike! Based on your suggestion below and keeping in line with our implementation, I would like to recommend the following changes. I will make those and upload for review: The PCD defined (this one at SecurityPkg level) will be as follows. Note that to keep it aligned with our implementation, we are keeping the bitmap "mutually exclusive" for each algorithm. [PcdsFixedAtBuild] ## This PCD indicates the HASH algorithm to verify unsigned PE/COFF image # Based on the value set, the required algorithm is chosen to verify # the unsigned image during Secure Boot.<BR> # The hashing algorithm selected must match the hashing algorithm used to # hash the image to be added to DB using tools such as KeyEnroll.<BR> # 0x00000001 - SHA1.<BR> # 0x00000002 - SHA256.<BR> # 0x00000004 - SHA384.<BR> # 0x00000008 - SHA512.<BR> # 0x00000010 - SM3_256.<BR> # 0x00000020 - MD4.<BR> # 0x00000040 - MD5.<BR> # @Prompt Set policy for hashing unsigned image for Secure Boot. # @ValidRange 0x80000001 \| 0x00000000 - 0x00000005 gEfiSecurityPkgTokenSpaceGuid.PcdSystemHashPolicy\|0x02\| UINT32\|0x00010024 the BaseHashLib API will provide the wrapper. As an example, for HashInit, the function definition would be EFI_STATUS EFIAPI HashApiInit ( OUT HASH_HANDLE HashHandle ) { . . . HashPolicy = PcdGet32 (PcdSystemHashPolicy); if ((mBaseHashInterfaceCount == 0) \|\| !(mCurrentHashMask & HashPolicy)) { return EFI_UNSUPPORTED; } HashCtx = AllocatePool (sizeof(HashCtx)); ASSERT (HashCtx != NULL); for (Index = 0; Index < mBaseHashInterfaceCount; Index++) { HashMask = GetApiHashMaskFromAlgo (&mHashOps[Index].HashGuid); if ((HashMask & HashPolicy) != 0) { mHashOps[Index].HashInit (HashCtx); break; } } HashHandle = (HASH_HANDLE)HashCtx; return EFI_SUCCESS; } And mHashOps[Index].HashInit will be registered to the PCD value of PcdSystemHashPolicy. For example, if SHA384 is selected as value of PcdSystemHashPolicy, The HashInit call will look like, EFI_STATUS EFIAPI Sha384_Init ( OUT HASH_HANDLE HashHandle ) { VOID Sha384Ctx; UINTN CtxSize; CtxSize = Sha384GetContextSize (); Sha384Ctx = AllocatePool (CtxSize); ASSERT (Sha384Ctx != NULL); Sha384Init (Sha384Ctx); HashHandle = (HASH_HANDLE)Sha384Ctx; return EFI_SUCCESS; } Do you see any issues/challenges with this implementation? Thanks, Amol -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Tuesday, December 17, 2019 12:47 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, I would recommend a different set of APIs in the HashLib class. Instead of doing a registration, define a PCD to select the hash type and have the generic HashLib function call the BaseCryptLib APIs based on the PCD value. If the PCD is type FixedAtBuild only the hash functions specified by the PCD are included in the executable. I would also recommend the HashLib and the PCD be added to the CryptoPkg instead of the SecurityPkg. The BaseCryptLib hash families today are: MD4, MD5, SHA1, SHA256, SHA384, SHA512 SM3 Proposed FixedAtBuild PCD to declare the hash family of APIs Provided by the HashLib. [PcdsFixedAtBuild] # Family of hash API provided by the HashLib. Can be set at # global platform scope or module specific scope. # # 0x00 - No hash family selected. All HashLib APIs ASSERT(). # 0x01 - HashLib provides MD4 family of APIs. # 0x02 - HashLib provides MD5 family of APIs. # 0x03 - HashLib provides SHA1 family of APIs. # 0x04 - HashLib provides SHA256 family of APIs. # 0x05 - HashLib provides SHA384 family of APIs. # 0x06 - HashLib provides SHA512 family of APIs. # gEfiCryptoPkgTokenSpaceGuid.PcdHashLibFamily\|0x00\|UINT8 \|0x00000002 The proposed HashLib APIs are shown below and are based on the BaseCryptLib hash APIs. This would allow existing modules that are currently using BaseCryptLib to be easily updated to use the HashLib APIs and can be easily configured with a PCD to select a specific Family of hash services. Can add #defines for the families to remove use of hard coded values. UINTN EFIAPI HashGetContextSize ( VOID ) { switch (PcdGet8 (PcdHashLibFamily)) { case 0x01: return Md4GetContextSize (); case 0x02: return Md5GetContextSize (); case 0x03: return Sha1GetContextSize (); case 0x04: return Sha256GetContextSize (); case 0x05: return Sha384GetContextSize (); case 0x06: return Sha512GetContextSize (); case 0x00: default: ASSERT (FALSE); return 0; } BOOLEAN EFIAPI HashInit ( OUT VOID HashContext ); BOOLEAN EFIAPI HashDuplicate ( IN CONST VOID HashContext, OUT VOID NewHashContext ); BOOLEAN EFIAPI HashUpdate ( IN OUT VOID HashContext, IN CONST VOID Data, IN UINTN DataSize ); BOOLEAN EFIAPI HashFinal ( IN OUT VOID HashContext, OUT UINT8 HashValue ); BOOLEAN EFIAPI HashAll ( IN CONST VOID Data, IN UINTN DataSize, OUT UINT8 HashValue ); Mike -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, December 17, 2019 10:23 AM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hello everyone, Checking if there are any review comments for https://github.com/ansukerk/edk2. Do I need to submit patches in any other format? Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 27, 2019 11:39 AM To: Kinney, Michael D <michael.d.kinney@...>; Wang, Jian J <jian.j.wang@...>; rfc@edk2.groups.io; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike, I think the point # 2 you are making is also the implementation we have proposed. Also, if we can have a two tier approach then we may be in a position to improve code flexibility: 1. Define FixedAtBuild PCD at SecurityPkg DSC file that can be used by each module intending to use system level hashing algorithm. 2. For modules that need a module-specific hashing algorithm, override the PCD at SecurityPkg DSC file with a FIxedAtBuild PCD at platform level DSC file. This can eliminate the need for each module to be forced to define the PCD at platform level. Thanks, Amol -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Tuesday, November 26, 2019 6:24 PM To: Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 As Sean points out, different components in a platform may have different hash requirements. If we want to go down the path where we have a few modules that produce Protocols/PPIs for the BaseCryptLib so all modules can share the same crypto services, then each module need to decide which specific hash algorithm it requires. Instead of defining another protocol/PPI for a unified hash service, we could consider a couple of other options: 1) Each module that uses hash services could use a FixedAtBuild PCD to decide which hash algorithm it should use. Based on this PCD value, different APIs in BaseCryptLib can be called. Since it is a FixedAtBuild PCD, the compiler/linker optimizations will only include the one hash algorithm selected. by using BaseCryptLib, this is compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs. 2) If we have many modules that need to make a hash algorithm selection, then define a new library class with the unified hash APIs, and the library instance uses a FixedAtBuild PCD to configure the hash algorithm used. This is also compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs and minimizes the sources in modules that need a design that supports flexible hash algorithm selection. If new hash algorithm is added, then only the lib instance and PCD need to be updated. Each module sets the FixedAtBuild PCD value in the scope of the module in the platform DSC file. If most or all modules in a platform need the same hash algorithm, then the FixedAtBuild PCD can be set at the platform scope in the platform DSC file. Best regards, Mike -----Original Message----- From: Wang, Jian J <jian.j.wang@...> Sent: Monday, November 25, 2019 9:09 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean and Amol, I believe the OBB and FV/PE image verification can still benefit from the unified API, if we add one more parameter to HashApiIinit() to force using a hash algorithm or add a PCD to choose one or link only required HashApiInstanceXxx. (Just for example) HashApiInit ( IN EFI_GUID HashAlg, OUT HASH_HANDLE *HashHandle ) In this way, we can avoid hard-code hash API in the verification module. Although it's slow, I think the OBB verification or FV/PE image signature algorithm is still subject to change per security requirement change. During a transition period, we don't have to add a new code to handle a different algorithm in verification module. Instead we just need to link a different hash lib or set PCD to use a different algorithm. I think this makes things simpler and easier to change. The BaseCryptLib implemented in PPI/Protocol is a good idea to save image size. I think the Unified hash API (BaseHashLib) can make use of it to save extra hash algorithm library instances (HashApiInstanceSha1, Sha256,...). The only problem is that the structured PCD, like Mike mentioned before, doesn't support module override, which prevents different modules from using different algorithms and then cannot save module size utter mostly. Regards, Jian -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, November 26, 2019 5:53 AM To: rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean, A few examples that we want to target are - HDD Password verification - Capsule Verification (Platform recovery) - Bios Guard Platform Data Table - More importantly, Boot Guard Event Log driver that has a bunch of redundant code today calculating various hashes. I don't think this is open source though. Currently, the hash verification mechanism in the above features is hard-coded to SHA256 which we will need to change to SHA384 to support a more robust hashing mechanism. This is a good opportunity to replace the existing SHA256 API with an abstracted unified API. And if necessary, the abstracted unified API still supports driver specific hashing algorithm if needed (as opposed to the strongest hashing algorithm supported by the system). We are not targeting verification of OBB or for that matter any PE/COFF image with this mechanism, mainly because it has it's own wrapper and verification mechanism in DxeImageVerificationLib. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sean via Groups.Io Sent: Monday, November 25, 2019 1:24 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, I am interested to hear more about the actual use cases. I like the idea of abstracting the API so that a calling driver doesn't have to change when the hashing algorithm requirements change but am not convinced that this will actually be helpful (or at least worth the introduction of more complexity in security sensitive code). For the TPM and measured boot I agree. Random driver needs to extend a measurement then it can use the existing hashlib and it will be hashed and extended per platform policy. This is because the random driver doesn't need to understand the result. Only the TPM needs to and that is handled within the extend operation. For the other drivers that I have seen using hashing, they need to understand the result and make driver specific decisions based on the result. For example if I am verifying the OBB or some FV then I must make sure the known good and computed match. This driver must locate the known good and thus this process generally must know the format of the result. For this I think it is best to know what algorithm I am using and I don't see a strong use case to change algorithm to an abstracted algorithm. Do you have a set of example drivers/use cases that could use abstracted hash support? Thanks Sean -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N via Groups.Io Sent: Monday, November 25, 2019 12:08 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sean Brogan <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [EXTERNAL] Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike and Nate, With our implementation we are trying to address the following in EDKII: 1. A common Hashing API for UEFI drivers to consume instead of the current API that directly calls into the specific hashing algorithm, for instance, SHA1_Init, SHA256_Update, etc. Due to a stronger hashing algorithm requirement, certain drivers need to be upgraded to SHA384 from SHA256 and the unified API is proposed, so, while upgrading we remove the hard-coded dependency on each hashing algorithm API. 2. The size issue as a result of statically linking openssl/cryptolib API to the consuming driver. There are two ways we tried to address this issue, although, we didn't explicitly defined a PPI to address it. a. Introduced a fixed PCD per hashing algorithm to include/exclude a particular algorithm from BIOS. b. Use a register mechanism to link the instance of a particular hashing algorithm to Unified API library. This is configurable using PCD. Note that 2.a and 2.b implementation is very similar to HashLibBaseCryptoRouter used for TPM2 operations. From the size and static linking perspective, I believe we can have a discussion between 2.a and 2.b with Mike's implementation(https://nam06.safelinks.protection.outlo ok.com/?url=htt ps%3A %2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_P PI_Protocol_ Proposal_V5&data=02%7C01%7Csean.brogan%40microsoft. com%7Cd110f db2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd 011db47%7C1 %7C0%7C637103092953247213&sdata=2acK0dmJZ85aZ3LvOOq D3e3w44V EM5dW8TEhs9USyDM%3D&reserved=0) below. We still need a common hashing API so the hashing algorithm strength for a particular driver can be increased without having to modify the consumer driver code. Comments are welcome. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Thursday, November 21, 2019 4:45 PM To: rfc@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Nate and Mike! I am going through the code and comments and will respond shortly. In the meantime, here is the GitHub link to my PoC for the community to look at and comment: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fansukerk%2Fedk2&data=02%7C01%7Csean.brogan%40m icrosoft.c om%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141a f91ab2d7cd 011db47%7C1%7C0%7C637103092953247213&sdata=keRdfU%2 BxffkBsyv YHwvSOIA8ryexOigiKhR%2B%2F1q7DjE%3D&reserved=0. Best regards, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael D Kinney Sent: Thursday, November 21, 2019 4:37 PM To: rfc@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Nate and Amol, There is some work already started by Sean and Matt that implements a PEIM, DXE Driver, and SMM Driver to produce Protocol/PPI that wraps the BaseCryptLib services. This content broken out into its own package is available here: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmicrosoft%2Fmu_plus%2Ftree%2Fdev%2F201908%2FShared CryptoPkg& amp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fd b2a91f4e8c2 acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1% 7C0%7C637 103092953247213&sdata=88TRhUqsVrj7sx8P4hjbdLy2%2BDK nrBoi7HAI3G UhpPo%3D&reserved=0 I have ported and simplified this content into a proposed set of patches to the CryptoPkg. It uses a structured PCD to configure the services mapped into the Protocols/PPIs and avoids the issue Nate notes below with protocols and PPIs including all of the BaseCryptLib services. The structured PCD allows families of crypto services or individual services within a family to be enabled/disabled. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Pr oposal_V5&a mp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb 2a91f4e8c2a cb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7 C0%7C6371 03092953247213&sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5 dW8TEhs9 USyDM%3D&reserved=0 For example, the DSC file PCD statements to enable the SHA1 family and SHA256 family of hash services with the HashAll service disabled is: [PcdsFixedAtBuild] gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Services.Has hAll \| FALSE gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Services. HashAll \| FALSE Please take a look at this proposal and let me know if this can be used to address. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0 There is currently a limitation in the structured PCD feature that does not allow the structured PCD field values to be set in the scope of a module in a <PcdsFixedAtBuild> section. To work around this limitation, the CryptoPkg DSC file has a define called CRYPTO_SERVICES that can be set to ALL, NONE, MIN_PEI, or MIN_DXE_MIN_SMM. The default is ALL. Building with each of these values will build the modules with different sets of enabled services that matches the services enabled using multiple modules in the work from Sean and Matt. If this limitation is addressed in BaseTools, then CryptoPkg could remove the CRYPTO_SERVIES define and all !if statements. Thanks, Mike -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Nate DeSimone Sent: Wednesday, November 20, 2019 10:31 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Amol, With regard to verifying code hashes, it makes much more sense to me to have a GUID defined that represents the hash function to use versus a PCD. The reason for this is the method for signing Firmware Volumes typically involves using nested FVs where the GUID for the nested FV type indicates the algorithm needed to "extract" the nested FV. Extraction in this context is used to verify code hashes as well as decompress compressed code. The "extractor" is usually a statically linked library attached to either SignedSectionExtractPei and/or DxeIpl. For example, it is very possible for a platform to use more than one hashing algorithm, I can think of more than one closed source platform at Intel that actually does this. Now, one thing that I think your document implies is that we should have better APIs for crypto operations in general. If that was the intent then I totally agree with you. There are several reasons that I think this should be done. First, the API for OpenSSL has a tendency to change often enough to make integrating new versions into older codebases a headache, putting an abstraction layer on it would be helpful. Second, the method used to consume OpenSSL in edk2 causes some technical issues. Specifically, OpenSSL is statically linked and it defines a crazy number of global variables, enough so that on a debug build I have noticed roughly a 20KB increase in binary size even if you don't call a single function in OpenSSL. Wrapping the crypto operations in a PPI/Protocol so that only 1 driver needs to link to OpenSSL would probably help reduce binary size for most implementations. Your current document does not define a PPI/Protocol, that is something I would recommend. One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol have run into is that all the crypto functions need to be linked, even the ones that a platform does not use, which results in a net negative binary size impact. Accordingly, one would likely need to break the crypto functionality into multiple PPIs/Protocols so that only the functions a platform needs are actually included. I think some experiments need to be done to figure out the most optimal division of crypto APIs. Hope that helps, Nate -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 20, 2019 8:28 AM To: rfc@edk2.groups.io Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Importance: High Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0. You can also provide the feedback in the Bugzilla. Thanks, Amol
More All Messages By This Member
Sukerkar, Amol N #223 Hi Mike, In my proposal, only the API that is selected by PcdSystemHashPolicy will be registered and it will not matter whether other hashing algorithms are supported or not. Whereas, in your proposal, every platform will need to have API support for all the hashing algorithms supported in EDK2 (returning actual output or ASSERT()). Wouldn't this be a limitation? I have not done this analysis, but, I will analyze if there is significant penalty of iterating through 5 or 6 GUIDs. My design can accommodate mutually exclusive bitmap as well as enumeration, so, this should not be an issue. If you can comment on my question above, it will be helpful. Thanks, Amol toggle quoted message Show quoted text -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Tuesday, December 17, 2019 3:46 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, Your style requires bigger source changes to exiting modules to switch from BaseCryptLib to HashLib. Your style also required a loop to search for matching API on every call. Mine will optimize away the case statement into a direct call to the select hash family API. I do not prefer the mutually exclusive bitmap. An enumeration guarantees no ambiguity. Mike -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, December 17, 2019 1:13 PM To: Kinney, Michael D <michael.d.kinney@...>; rfc@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Mike! Based on your suggestion below and keeping in line with our implementation, I would like to recommend the following changes. I will make those and upload for review: The PCD defined (this one at SecurityPkg level) will be as follows. Note that to keep it aligned with our implementation, we are keeping the bitmap "mutually exclusive" for each algorithm. [PcdsFixedAtBuild] ## This PCD indicates the HASH algorithm to verify unsigned PE/COFF image # Based on the value set, the required algorithm is chosen to verify # the unsigned image during Secure Boot.<BR> # The hashing algorithm selected must match the hashing algorithm used to # hash the image to be added to DB using tools such as KeyEnroll.<BR> # 0x00000001 - SHA1.<BR> # 0x00000002 - SHA256.<BR> # 0x00000004 - SHA384.<BR> # 0x00000008 - SHA512.<BR> # 0x00000010 - SM3_256.<BR> # 0x00000020 - MD4.<BR> # 0x00000040 - MD5.<BR> # @Prompt Set policy for hashing unsigned image for Secure Boot. # @ValidRange 0x80000001 \| 0x00000000 - 0x00000005 gEfiSecurityPkgTokenSpaceGuid.PcdSystemHashPolicy\|0x02\| UINT32\|0x00010024 the BaseHashLib API will provide the wrapper. As an example, for HashInit, the function definition would be EFI_STATUS EFIAPI HashApiInit ( OUT HASH_HANDLE HashHandle ) { . . . HashPolicy = PcdGet32 (PcdSystemHashPolicy); if ((mBaseHashInterfaceCount == 0) \|\| !(mCurrentHashMask & HashPolicy)) { return EFI_UNSUPPORTED; } HashCtx = AllocatePool (sizeof(HashCtx)); ASSERT (HashCtx != NULL); for (Index = 0; Index < mBaseHashInterfaceCount; Index++) { HashMask = GetApiHashMaskFromAlgo (&mHashOps[Index].HashGuid); if ((HashMask & HashPolicy) != 0) { mHashOps[Index].HashInit (HashCtx); break; } } HashHandle = (HASH_HANDLE)HashCtx; return EFI_SUCCESS; } And mHashOps[Index].HashInit will be registered to the PCD value of PcdSystemHashPolicy. For example, if SHA384 is selected as value of PcdSystemHashPolicy, The HashInit call will look like, EFI_STATUS EFIAPI Sha384_Init ( OUT HASH_HANDLE HashHandle ) { VOID Sha384Ctx; UINTN CtxSize; CtxSize = Sha384GetContextSize (); Sha384Ctx = AllocatePool (CtxSize); ASSERT (Sha384Ctx != NULL); Sha384Init (Sha384Ctx); HashHandle = (HASH_HANDLE)Sha384Ctx; return EFI_SUCCESS; } Do you see any issues/challenges with this implementation? Thanks, Amol -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Tuesday, December 17, 2019 12:47 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, I would recommend a different set of APIs in the HashLib class. Instead of doing a registration, define a PCD to select the hash type and have the generic HashLib function call the BaseCryptLib APIs based on the PCD value. If the PCD is type FixedAtBuild only the hash functions specified by the PCD are included in the executable. I would also recommend the HashLib and the PCD be added to the CryptoPkg instead of the SecurityPkg. The BaseCryptLib hash families today are: MD4, MD5, SHA1, SHA256, SHA384, SHA512 SM3 Proposed FixedAtBuild PCD to declare the hash family of APIs Provided by the HashLib. [PcdsFixedAtBuild] # Family of hash API provided by the HashLib. Can be set at # global platform scope or module specific scope. # # 0x00 - No hash family selected. All HashLib APIs ASSERT(). # 0x01 - HashLib provides MD4 family of APIs. # 0x02 - HashLib provides MD5 family of APIs. # 0x03 - HashLib provides SHA1 family of APIs. # 0x04 - HashLib provides SHA256 family of APIs. # 0x05 - HashLib provides SHA384 family of APIs. # 0x06 - HashLib provides SHA512 family of APIs. # gEfiCryptoPkgTokenSpaceGuid.PcdHashLibFamily\|0x00\|UINT8 \|0x00000002 The proposed HashLib APIs are shown below and are based on the BaseCryptLib hash APIs. This would allow existing modules that are currently using BaseCryptLib to be easily updated to use the HashLib APIs and can be easily configured with a PCD to select a specific Family of hash services. Can add #defines for the families to remove use of hard coded values. UINTN EFIAPI HashGetContextSize ( VOID ) { switch (PcdGet8 (PcdHashLibFamily)) { case 0x01: return Md4GetContextSize (); case 0x02: return Md5GetContextSize (); case 0x03: return Sha1GetContextSize (); case 0x04: return Sha256GetContextSize (); case 0x05: return Sha384GetContextSize (); case 0x06: return Sha512GetContextSize (); case 0x00: default: ASSERT (FALSE); return 0; } BOOLEAN EFIAPI HashInit ( OUT VOID HashContext ); BOOLEAN EFIAPI HashDuplicate ( IN CONST VOID HashContext, OUT VOID NewHashContext ); BOOLEAN EFIAPI HashUpdate ( IN OUT VOID HashContext, IN CONST VOID Data, IN UINTN DataSize ); BOOLEAN EFIAPI HashFinal ( IN OUT VOID HashContext, OUT UINT8 HashValue ); BOOLEAN EFIAPI HashAll ( IN CONST VOID Data, IN UINTN DataSize, OUT UINT8 HashValue ); Mike -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, December 17, 2019 10:23 AM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hello everyone, Checking if there are any review comments for https://github.com/ansukerk/edk2. Do I need to submit patches in any other format? Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 27, 2019 11:39 AM To: Kinney, Michael D <michael.d.kinney@...>; Wang, Jian J <jian.j.wang@...>; rfc@edk2.groups.io; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike, I think the point # 2 you are making is also the implementation we have proposed. Also, if we can have a two tier approach then we may be in a position to improve code flexibility: 1. Define FixedAtBuild PCD at SecurityPkg DSC file that can be used by each module intending to use system level hashing algorithm. 2. For modules that need a module-specific hashing algorithm, override the PCD at SecurityPkg DSC file with a FIxedAtBuild PCD at platform level DSC file. This can eliminate the need for each module to be forced to define the PCD at platform level. Thanks, Amol -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Tuesday, November 26, 2019 6:24 PM To: Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 As Sean points out, different components in a platform may have different hash requirements. If we want to go down the path where we have a few modules that produce Protocols/PPIs for the BaseCryptLib so all modules can share the same crypto services, then each module need to decide which specific hash algorithm it requires. Instead of defining another protocol/PPI for a unified hash service, we could consider a couple of other options: 1) Each module that uses hash services could use a FixedAtBuild PCD to decide which hash algorithm it should use. Based on this PCD value, different APIs in BaseCryptLib can be called. Since it is a FixedAtBuild PCD, the compiler/linker optimizations will only include the one hash algorithm selected. by using BaseCryptLib, this is compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs. 2) If we have many modules that need to make a hash algorithm selection, then define a new library class with the unified hash APIs, and the library instance uses a FixedAtBuild PCD to configure the hash algorithm used. This is also compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs and minimizes the sources in modules that need a design that supports flexible hash algorithm selection. If new hash algorithm is added, then only the lib instance and PCD need to be updated. Each module sets the FixedAtBuild PCD value in the scope of the module in the platform DSC file. If most or all modules in a platform need the same hash algorithm, then the FixedAtBuild PCD can be set at the platform scope in the platform DSC file. Best regards, Mike -----Original Message----- From: Wang, Jian J <jian.j.wang@...> Sent: Monday, November 25, 2019 9:09 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean and Amol, I believe the OBB and FV/PE image verification can still benefit from the unified API, if we add one more parameter to HashApiIinit() to force using a hash algorithm or add a PCD to choose one or link only required HashApiInstanceXxx. (Just for example) HashApiInit ( IN EFI_GUID HashAlg, OUT HASH_HANDLE *HashHandle ) In this way, we can avoid hard-code hash API in the verification module. Although it's slow, I think the OBB verification or FV/PE image signature algorithm is still subject to change per security requirement change. During a transition period, we don't have to add a new code to handle a different algorithm in verification module. Instead we just need to link a different hash lib or set PCD to use a different algorithm. I think this makes things simpler and easier to change. The BaseCryptLib implemented in PPI/Protocol is a good idea to save image size. I think the Unified hash API (BaseHashLib) can make use of it to save extra hash algorithm library instances (HashApiInstanceSha1, Sha256,...). The only problem is that the structured PCD, like Mike mentioned before, doesn't support module override, which prevents different modules from using different algorithms and then cannot save module size utter mostly. Regards, Jian -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, November 26, 2019 5:53 AM To: rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean, A few examples that we want to target are - HDD Password verification - Capsule Verification (Platform recovery) - Bios Guard Platform Data Table - More importantly, Boot Guard Event Log driver that has a bunch of redundant code today calculating various hashes. I don't think this is open source though. Currently, the hash verification mechanism in the above features is hard-coded to SHA256 which we will need to change to SHA384 to support a more robust hashing mechanism. This is a good opportunity to replace the existing SHA256 API with an abstracted unified API. And if necessary, the abstracted unified API still supports driver specific hashing algorithm if needed (as opposed to the strongest hashing algorithm supported by the system). We are not targeting verification of OBB or for that matter any PE/COFF image with this mechanism, mainly because it has it's own wrapper and verification mechanism in DxeImageVerificationLib. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sean via Groups.Io Sent: Monday, November 25, 2019 1:24 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, I am interested to hear more about the actual use cases. I like the idea of abstracting the API so that a calling driver doesn't have to change when the hashing algorithm requirements change but am not convinced that this will actually be helpful (or at least worth the introduction of more complexity in security sensitive code). For the TPM and measured boot I agree. Random driver needs to extend a measurement then it can use the existing hashlib and it will be hashed and extended per platform policy. This is because the random driver doesn't need to understand the result. Only the TPM needs to and that is handled within the extend operation. For the other drivers that I have seen using hashing, they need to understand the result and make driver specific decisions based on the result. For example if I am verifying the OBB or some FV then I must make sure the known good and computed match. This driver must locate the known good and thus this process generally must know the format of the result. For this I think it is best to know what algorithm I am using and I don't see a strong use case to change algorithm to an abstracted algorithm. Do you have a set of example drivers/use cases that could use abstracted hash support? Thanks Sean -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N via Groups.Io Sent: Monday, November 25, 2019 12:08 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sean Brogan <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [EXTERNAL] Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike and Nate, With our implementation we are trying to address the following in EDKII: 1. A common Hashing API for UEFI drivers to consume instead of the current API that directly calls into the specific hashing algorithm, for instance, SHA1_Init, SHA256_Update, etc. Due to a stronger hashing algorithm requirement, certain drivers need to be upgraded to SHA384 from SHA256 and the unified API is proposed, so, while upgrading we remove the hard-coded dependency on each hashing algorithm API. 2. The size issue as a result of statically linking openssl/cryptolib API to the consuming driver. There are two ways we tried to address this issue, although, we didn't explicitly defined a PPI to address it. a. Introduced a fixed PCD per hashing algorithm to include/exclude a particular algorithm from BIOS. b. Use a register mechanism to link the instance of a particular hashing algorithm to Unified API library. This is configurable using PCD. Note that 2.a and 2.b implementation is very similar to HashLibBaseCryptoRouter used for TPM2 operations. From the size and static linking perspective, I believe we can have a discussion between 2.a and 2.b with Mike's implementation(https://nam06.safelinks.protection.outlo ok.com/?url=htt ps%3A %2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_P PI_Protocol_ Proposal_V5&data=02%7C01%7Csean.brogan%40microsoft. com%7Cd110f db2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd 011db47%7C1 %7C0%7C637103092953247213&sdata=2acK0dmJZ85aZ3LvOOq D3e3w44V EM5dW8TEhs9USyDM%3D&reserved=0) below. We still need a common hashing API so the hashing algorithm strength for a particular driver can be increased without having to modify the consumer driver code. Comments are welcome. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Thursday, November 21, 2019 4:45 PM To: rfc@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Nate and Mike! I am going through the code and comments and will respond shortly. In the meantime, here is the GitHub link to my PoC for the community to look at and comment: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fansukerk%2Fedk2&data=02%7C01%7Csean.brogan%40m icrosoft.c om%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141a f91ab2d7cd 011db47%7C1%7C0%7C637103092953247213&sdata=keRdfU%2 BxffkBsyv YHwvSOIA8ryexOigiKhR%2B%2F1q7DjE%3D&reserved=0. Best regards, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael D Kinney Sent: Thursday, November 21, 2019 4:37 PM To: rfc@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Nate and Amol, There is some work already started by Sean and Matt that implements a PEIM, DXE Driver, and SMM Driver to produce Protocol/PPI that wraps the BaseCryptLib services. This content broken out into its own package is available here: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmicrosoft%2Fmu_plus%2Ftree%2Fdev%2F201908%2FShared CryptoPkg& amp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fd b2a91f4e8c2 acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1% 7C0%7C637 103092953247213&sdata=88TRhUqsVrj7sx8P4hjbdLy2%2BDK nrBoi7HAI3G UhpPo%3D&reserved=0 I have ported and simplified this content into a proposed set of patches to the CryptoPkg. It uses a structured PCD to configure the services mapped into the Protocols/PPIs and avoids the issue Nate notes below with protocols and PPIs including all of the BaseCryptLib services. The structured PCD allows families of crypto services or individual services within a family to be enabled/disabled. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Pr oposal_V5&a mp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb 2a91f4e8c2a cb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7 C0%7C6371 03092953247213&sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5 dW8TEhs9 USyDM%3D&reserved=0 For example, the DSC file PCD statements to enable the SHA1 family and SHA256 family of hash services with the HashAll service disabled is: [PcdsFixedAtBuild] gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Services.Has hAll \| FALSE gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Services. HashAll \| FALSE Please take a look at this proposal and let me know if this can be used to address. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0 There is currently a limitation in the structured PCD feature that does not allow the structured PCD field values to be set in the scope of a module in a <PcdsFixedAtBuild> section. To work around this limitation, the CryptoPkg DSC file has a define called CRYPTO_SERVICES that can be set to ALL, NONE, MIN_PEI, or MIN_DXE_MIN_SMM. The default is ALL. Building with each of these values will build the modules with different sets of enabled services that matches the services enabled using multiple modules in the work from Sean and Matt. If this limitation is addressed in BaseTools, then CryptoPkg could remove the CRYPTO_SERVIES define and all !if statements. Thanks, Mike -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Nate DeSimone Sent: Wednesday, November 20, 2019 10:31 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Amol, With regard to verifying code hashes, it makes much more sense to me to have a GUID defined that represents the hash function to use versus a PCD. The reason for this is the method for signing Firmware Volumes typically involves using nested FVs where the GUID for the nested FV type indicates the algorithm needed to "extract" the nested FV. Extraction in this context is used to verify code hashes as well as decompress compressed code. The "extractor" is usually a statically linked library attached to either SignedSectionExtractPei and/or DxeIpl. For example, it is very possible for a platform to use more than one hashing algorithm, I can think of more than one closed source platform at Intel that actually does this. Now, one thing that I think your document implies is that we should have better APIs for crypto operations in general. If that was the intent then I totally agree with you. There are several reasons that I think this should be done. First, the API for OpenSSL has a tendency to change often enough to make integrating new versions into older codebases a headache, putting an abstraction layer on it would be helpful. Second, the method used to consume OpenSSL in edk2 causes some technical issues. Specifically, OpenSSL is statically linked and it defines a crazy number of global variables, enough so that on a debug build I have noticed roughly a 20KB increase in binary size even if you don't call a single function in OpenSSL. Wrapping the crypto operations in a PPI/Protocol so that only 1 driver needs to link to OpenSSL would probably help reduce binary size for most implementations. Your current document does not define a PPI/Protocol, that is something I would recommend. One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol have run into is that all the crypto functions need to be linked, even the ones that a platform does not use, which results in a net negative binary size impact. Accordingly, one would likely need to break the crypto functionality into multiple PPIs/Protocols so that only the functions a platform needs are actually included. I think some experiments need to be done to figure out the most optimal division of crypto APIs. Hope that helps, Nate -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 20, 2019 8:28 AM To: rfc@edk2.groups.io Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Importance: High Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0. You can also provide the feedback in the Bugzilla. Thanks, Amol
More All Messages By This Member
Michael D Kinney #224 Amol, All the APIs are available from BaseCryptLib. I agree that implementations of HashLib on top of other crypto libs would require a different implementation of the library instance and if all the families were not available, then more would fall through to the default case. In general, when you use a registration method passing in function pointers, the executable contains all the functions because the optimizer always sees a reference to the function. Even if one of the registered functions are never called. If you use the style I suggested, then optimizers can optimize away all the functions that are never called by the calling code leaving only the once actually called. Mike toggle quoted message Show quoted text -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, December 17, 2019 3:29 PM To: Kinney, Michael D <michael.d.kinney@...>; rfc@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike, In my proposal, only the API that is selected by PcdSystemHashPolicy will be registered and it will not matter whether other hashing algorithms are supported or not. Whereas, in your proposal, every platform will need to have API support for all the hashing algorithms supported in EDK2 (returning actual output or ASSERT()). Wouldn't this be a limitation? I have not done this analysis, but, I will analyze if there is significant penalty of iterating through 5 or 6 GUIDs. My design can accommodate mutually exclusive bitmap as well as enumeration, so, this should not be an issue. If you can comment on my question above, it will be helpful. Thanks, Amol -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Tuesday, December 17, 2019 3:46 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, Your style requires bigger source changes to exiting modules to switch from BaseCryptLib to HashLib. Your style also required a loop to search for matching API on every call. Mine will optimize away the case statement into a direct call to the select hash family API. I do not prefer the mutually exclusive bitmap. An enumeration guarantees no ambiguity. Mike -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, December 17, 2019 1:13 PM To: Kinney, Michael D <michael.d.kinney@...>; rfc@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Mike! Based on your suggestion below and keeping in line with our implementation, I would like to recommend the following changes. I will make those and upload for review: The PCD defined (this one at SecurityPkg level) will be as follows. Note that to keep it aligned with our implementation, we are keeping the bitmap "mutually exclusive" for each algorithm. [PcdsFixedAtBuild] ## This PCD indicates the HASH algorithm to verify unsigned PE/COFF image # Based on the value set, the required algorithm is chosen to verify # the unsigned image during Secure Boot.<BR> # The hashing algorithm selected must match the hashing algorithm used to # hash the image to be added to DB using tools such as KeyEnroll.<BR> # 0x00000001 - SHA1.<BR> # 0x00000002 - SHA256.<BR> # 0x00000004 - SHA384.<BR> # 0x00000008 - SHA512.<BR> # 0x00000010 - SM3_256.<BR> # 0x00000020 - MD4.<BR> # 0x00000040 - MD5.<BR> # @Prompt Set policy for hashing unsigned image for Secure Boot. # @ValidRange 0x80000001 \| 0x00000000 - 0x00000005 gEfiSecurityPkgTokenSpaceGuid.PcdSystemHashPolicy\|0x02\| UINT32\|0x00010024 the BaseHashLib API will provide the wrapper. As an example, for HashInit, the function definition would be EFI_STATUS EFIAPI HashApiInit ( OUT HASH_HANDLE HashHandle ) { . . . HashPolicy = PcdGet32 (PcdSystemHashPolicy); if ((mBaseHashInterfaceCount == 0) \|\| !(mCurrentHashMask & HashPolicy)) { return EFI_UNSUPPORTED; } HashCtx = AllocatePool (sizeof(HashCtx)); ASSERT (HashCtx != NULL); for (Index = 0; Index < mBaseHashInterfaceCount; Index++) { HashMask = GetApiHashMaskFromAlgo (&mHashOps[Index].HashGuid); if ((HashMask & HashPolicy) != 0) { mHashOps[Index].HashInit (HashCtx); break; } } HashHandle = (HASH_HANDLE)HashCtx; return EFI_SUCCESS; } And mHashOps[Index].HashInit will be registered to the PCD value of PcdSystemHashPolicy. For example, if SHA384 is selected as value of PcdSystemHashPolicy, The HashInit call will look like, EFI_STATUS EFIAPI Sha384_Init ( OUT HASH_HANDLE HashHandle ) { VOID Sha384Ctx; UINTN CtxSize; CtxSize = Sha384GetContextSize (); Sha384Ctx = AllocatePool (CtxSize); ASSERT (Sha384Ctx != NULL); Sha384Init (Sha384Ctx); HashHandle = (HASH_HANDLE)Sha384Ctx; return EFI_SUCCESS; } Do you see any issues/challenges with this implementation? Thanks, Amol -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Tuesday, December 17, 2019 12:47 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, I would recommend a different set of APIs in the HashLib class. Instead of doing a registration, define a PCD to select the hash type and have the generic HashLib function call the BaseCryptLib APIs based on the PCD value. If the PCD is type FixedAtBuild only the hash functions specified by the PCD are included in the executable. I would also recommend the HashLib and the PCD be added to the CryptoPkg instead of the SecurityPkg. The BaseCryptLib hash families today are: MD4, MD5, SHA1, SHA256, SHA384, SHA512 SM3 Proposed FixedAtBuild PCD to declare the hash family of APIs Provided by the HashLib. [PcdsFixedAtBuild] # Family of hash API provided by the HashLib. Can be set at # global platform scope or module specific scope. # # 0x00 - No hash family selected. All HashLib APIs ASSERT(). # 0x01 - HashLib provides MD4 family of APIs. # 0x02 - HashLib provides MD5 family of APIs. # 0x03 - HashLib provides SHA1 family of APIs. # 0x04 - HashLib provides SHA256 family of APIs. # 0x05 - HashLib provides SHA384 family of APIs. # 0x06 - HashLib provides SHA512 family of APIs. # gEfiCryptoPkgTokenSpaceGuid.PcdHashLibFamily\|0x00\|UINT8 \|0x00000002 The proposed HashLib APIs are shown below and are based on the BaseCryptLib hash APIs. This would allow existing modules that are currently using BaseCryptLib to be easily updated to use the HashLib APIs and can be easily configured with a PCD to select a specific Family of hash services. Can add #defines for the families to remove use of hard coded values. UINTN EFIAPI HashGetContextSize ( VOID ) { switch (PcdGet8 (PcdHashLibFamily)) { case 0x01: return Md4GetContextSize (); case 0x02: return Md5GetContextSize (); case 0x03: return Sha1GetContextSize (); case 0x04: return Sha256GetContextSize (); case 0x05: return Sha384GetContextSize (); case 0x06: return Sha512GetContextSize (); case 0x00: default: ASSERT (FALSE); return 0; } BOOLEAN EFIAPI HashInit ( OUT VOID HashContext ); BOOLEAN EFIAPI HashDuplicate ( IN CONST VOID HashContext, OUT VOID NewHashContext ); BOOLEAN EFIAPI HashUpdate ( IN OUT VOID HashContext, IN CONST VOID Data, IN UINTN DataSize ); BOOLEAN EFIAPI HashFinal ( IN OUT VOID HashContext, OUT UINT8 HashValue ); BOOLEAN EFIAPI HashAll ( IN CONST VOID Data, IN UINTN DataSize, OUT UINT8 HashValue ); Mike -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, December 17, 2019 10:23 AM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hello everyone, Checking if there are any review comments for https://github.com/ansukerk/edk2. Do I need to submit patches in any other format? Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 27, 2019 11:39 AM To: Kinney, Michael D <michael.d.kinney@...>; Wang, Jian J <jian.j.wang@...>; rfc@edk2.groups.io; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike, I think the point # 2 you are making is also the implementation we have proposed. Also, if we can have a two tier approach then we may be in a position to improve code flexibility: 1. Define FixedAtBuild PCD at SecurityPkg DSC file that can be used by each module intending to use system level hashing algorithm. 2. For modules that need a module-specific hashing algorithm, override the PCD at SecurityPkg DSC file with a FIxedAtBuild PCD at platform level DSC file. This can eliminate the need for each module to be forced to define the PCD at platform level. Thanks, Amol -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Tuesday, November 26, 2019 6:24 PM To: Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 As Sean points out, different components in a platform may have different hash requirements. If we want to go down the path where we have a few modules that produce Protocols/PPIs for the BaseCryptLib so all modules can share the same crypto services, then each module need to decide which specific hash algorithm it requires. Instead of defining another protocol/PPI for a unified hash service, we could consider a couple of other options: 1) Each module that uses hash services could use a FixedAtBuild PCD to decide which hash algorithm it should use. Based on this PCD value, different APIs in BaseCryptLib can be called. Since it is a FixedAtBuild PCD, the compiler/linker optimizations will only include the one hash algorithm selected. by using BaseCryptLib, this is compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs. 2) If we have many modules that need to make a hash algorithm selection, then define a new library class with the unified hash APIs, and the library instance uses a FixedAtBuild PCD to configure the hash algorithm used. This is also compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs and minimizes the sources in modules that need a design that supports flexible hash algorithm selection. If new hash algorithm is added, then only the lib instance and PCD need to be updated. Each module sets the FixedAtBuild PCD value in the scope of the module in the platform DSC file. If most or all modules in a platform need the same hash algorithm, then the FixedAtBuild PCD can be set at the platform scope in the platform DSC file. Best regards, Mike -----Original Message----- From: Wang, Jian J <jian.j.wang@...> Sent: Monday, November 25, 2019 9:09 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean and Amol, I believe the OBB and FV/PE image verification can still benefit from the unified API, if we add one more parameter to HashApiIinit() to force using a hash algorithm or add a PCD to choose one or link only required HashApiInstanceXxx. (Just for example) HashApiInit ( IN EFI_GUID HashAlg, OUT HASH_HANDLE *HashHandle ) In this way, we can avoid hard-code hash API in the verification module. Although it's slow, I think the OBB verification or FV/PE image signature algorithm is still subject to change per security requirement change. During a transition period, we don't have to add a new code to handle a different algorithm in verification module. Instead we just need to link a different hash lib or set PCD to use a different algorithm. I think this makes things simpler and easier to change. The BaseCryptLib implemented in PPI/Protocol is a good idea to save image size. I think the Unified hash API (BaseHashLib) can make use of it to save extra hash algorithm library instances (HashApiInstanceSha1, Sha256,...). The only problem is that the structured PCD, like Mike mentioned before, doesn't support module override, which prevents different modules from using different algorithms and then cannot save module size utter mostly. Regards, Jian -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, November 26, 2019 5:53 AM To: rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean, A few examples that we want to target are - HDD Password verification - Capsule Verification (Platform recovery) - Bios Guard Platform Data Table - More importantly, Boot Guard Event Log driver that has a bunch of redundant code today calculating various hashes. I don't think this is open source though. Currently, the hash verification mechanism in the above features is hard-coded to SHA256 which we will need to change to SHA384 to support a more robust hashing mechanism. This is a good opportunity to replace the existing SHA256 API with an abstracted unified API. And if necessary, the abstracted unified API still supports driver specific hashing algorithm if needed (as opposed to the strongest hashing algorithm supported by the system). We are not targeting verification of OBB or for that matter any PE/COFF image with this mechanism, mainly because it has it's own wrapper and verification mechanism in DxeImageVerificationLib. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sean via Groups.Io Sent: Monday, November 25, 2019 1:24 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, I am interested to hear more about the actual use cases. I like the idea of abstracting the API so that a calling driver doesn't have to change when the hashing algorithm requirements change but am not convinced that this will actually be helpful (or at least worth the introduction of more complexity in security sensitive code). For the TPM and measured boot I agree. Random driver needs to extend a measurement then it can use the existing hashlib and it will be hashed and extended per platform policy. This is because the random driver doesn't need to understand the result. Only the TPM needs to and that is handled within the extend operation. For the other drivers that I have seen using hashing, they need to understand the result and make driver specific decisions based on the result. For example if I am verifying the OBB or some FV then I must make sure the known good and computed match. This driver must locate the known good and thus this process generally must know the format of the result. For this I think it is best to know what algorithm I am using and I don't see a strong use case to change algorithm to an abstracted algorithm. Do you have a set of example drivers/use cases that could use abstracted hash support? Thanks Sean -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N via Groups.Io Sent: Monday, November 25, 2019 12:08 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sean Brogan <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [EXTERNAL] Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike and Nate, With our implementation we are trying to address the following in EDKII: 1. A common Hashing API for UEFI drivers to consume instead of the current API that directly calls into the specific hashing algorithm, for instance, SHA1_Init, SHA256_Update, etc. Due to a stronger hashing algorithm requirement, certain drivers need to be upgraded to SHA384 from SHA256 and the unified API is proposed, so, while upgrading we remove the hard-coded dependency on each hashing algorithm API. 2. The size issue as a result of statically linking openssl/cryptolib API to the consuming driver. There are two ways we tried to address this issue, although, we didn't explicitly defined a PPI to address it. a. Introduced a fixed PCD per hashing algorithm to include/exclude a particular algorithm from BIOS. b. Use a register mechanism to link the instance of a particular hashing algorithm to Unified API library. This is configurable using PCD. Note that 2.a and 2.b implementation is very similar to HashLibBaseCryptoRouter used for TPM2 operations. From the size and static linking perspective, I believe we can have a discussion between 2.a and 2.b with Mike's implementation(https://nam06.safelinks.protection.outlo ok.com/?url=htt ps%3A %2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_P PI_Protocol_ Proposal_V5&data=02%7C01%7Csean.brogan%40microsoft. com%7Cd110f db2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd 011db47%7C1 %7C0%7C637103092953247213&sdata=2acK0dmJZ85aZ3LvOOq D3e3w44V EM5dW8TEhs9USyDM%3D&reserved=0) below. We still need a common hashing API so the hashing algorithm strength for a particular driver can be increased without having to modify the consumer driver code. Comments are welcome. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Thursday, November 21, 2019 4:45 PM To: rfc@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Nate and Mike! I am going through the code and comments and will respond shortly. In the meantime, here is the GitHub link to my PoC for the community to look at and comment: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fansukerk%2Fedk2&data=02%7C01%7Csean.brogan%40m icrosoft.c om%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141a f91ab2d7cd 011db47%7C1%7C0%7C637103092953247213&sdata=keRdfU%2 BxffkBsyv YHwvSOIA8ryexOigiKhR%2B%2F1q7DjE%3D&reserved=0. Best regards, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael D Kinney Sent: Thursday, November 21, 2019 4:37 PM To: rfc@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Nate and Amol, There is some work already started by Sean and Matt that implements a PEIM, DXE Driver, and SMM Driver to produce Protocol/PPI that wraps the BaseCryptLib services. This content broken out into its own package is available here: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmicrosoft%2Fmu_plus%2Ftree%2Fdev%2F201908%2FShared CryptoPkg& amp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fd b2a91f4e8c2 acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1% 7C0%7C637 103092953247213&sdata=88TRhUqsVrj7sx8P4hjbdLy2%2BDK nrBoi7HAI3G UhpPo%3D&reserved=0 I have ported and simplified this content into a proposed set of patches to the CryptoPkg. It uses a structured PCD to configure the services mapped into the Protocols/PPIs and avoids the issue Nate notes below with protocols and PPIs including all of the BaseCryptLib services. The structured PCD allows families of crypto services or individual services within a family to be enabled/disabled. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Pr oposal_V5&a mp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb 2a91f4e8c2a cb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7 C0%7C6371 03092953247213&sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5 dW8TEhs9 USyDM%3D&reserved=0 For example, the DSC file PCD statements to enable the SHA1 family and SHA256 family of hash services with the HashAll service disabled is: [PcdsFixedAtBuild] gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Services.Has hAll \| FALSE gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Services. HashAll \| FALSE Please take a look at this proposal and let me know if this can be used to address. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0 There is currently a limitation in the structured PCD feature that does not allow the structured PCD field values to be set in the scope of a module in a <PcdsFixedAtBuild> section. To work around this limitation, the CryptoPkg DSC file has a define called CRYPTO_SERVICES that can be set to ALL, NONE, MIN_PEI, or MIN_DXE_MIN_SMM. The default is ALL. Building with each of these values will build the modules with different sets of enabled services that matches the services enabled using multiple modules in the work from Sean and Matt. If this limitation is addressed in BaseTools, then CryptoPkg could remove the CRYPTO_SERVIES define and all !if statements. Thanks, Mike -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Nate DeSimone Sent: Wednesday, November 20, 2019 10:31 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Amol, With regard to verifying code hashes, it makes much more sense to me to have a GUID defined that represents the hash function to use versus a PCD. The reason for this is the method for signing Firmware Volumes typically involves using nested FVs where the GUID for the nested FV type indicates the algorithm needed to "extract" the nested FV. Extraction in this context is used to verify code hashes as well as decompress compressed code. The "extractor" is usually a statically linked library attached to either SignedSectionExtractPei and/or DxeIpl. For example, it is very possible for a platform to use more than one hashing algorithm, I can think of more than one closed source platform at Intel that actually does this. Now, one thing that I think your document implies is that we should have better APIs for crypto operations in general. If that was the intent then I totally agree with you. There are several reasons that I think this should be done. First, the API for OpenSSL has a tendency to change often enough to make integrating new versions into older codebases a headache, putting an abstraction layer on it would be helpful. Second, the method used to consume OpenSSL in edk2 causes some technical issues. Specifically, OpenSSL is statically linked and it defines a crazy number of global variables, enough so that on a debug build I have noticed roughly a 20KB increase in binary size even if you don't call a single function in OpenSSL. Wrapping the crypto operations in a PPI/Protocol so that only 1 driver needs to link to OpenSSL would probably help reduce binary size for most implementations. Your current document does not define a PPI/Protocol, that is something I would recommend. One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol have run into is that all the crypto functions need to be linked, even the ones that a platform does not use, which results in a net negative binary size impact. Accordingly, one would likely need to break the crypto functionality into multiple PPIs/Protocols so that only the functions a platform needs are actually included. I think some experiments need to be done to figure out the most optimal division of crypto APIs. Hope that helps, Nate -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 20, 2019 8:28 AM To: rfc@edk2.groups.io Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Importance: High Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0. You can also provide the feedback in the Bugzilla. Thanks, Amol
More All Messages By This Member
Sukerkar, Amol N #225 Thanks, Mike! I was in the process of sending out the review, so, went ahead and blasted the review email. I will look at your suggestions in the meantime. ~ Amol toggle quoted message Show quoted text -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Wednesday, December 18, 2019 10:03 AM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, All the APIs are available from BaseCryptLib. I agree that implementations of HashLib on top of other crypto libs would require a different implementation of the library instance and if all the families were not available, then more would fall through to the default case. In general, when you use a registration method passing in function pointers, the executable contains all the functions because the optimizer always sees a reference to the function. Even if one of the registered functions are never called. If you use the style I suggested, then optimizers can optimize away all the functions that are never called by the calling code leaving only the once actually called. Mike -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, December 17, 2019 3:29 PM To: Kinney, Michael D <michael.d.kinney@...>; rfc@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike, In my proposal, only the API that is selected by PcdSystemHashPolicy will be registered and it will not matter whether other hashing algorithms are supported or not. Whereas, in your proposal, every platform will need to have API support for all the hashing algorithms supported in EDK2 (returning actual output or ASSERT()). Wouldn't this be a limitation? I have not done this analysis, but, I will analyze if there is significant penalty of iterating through 5 or 6 GUIDs. My design can accommodate mutually exclusive bitmap as well as enumeration, so, this should not be an issue. If you can comment on my question above, it will be helpful. Thanks, Amol -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Tuesday, December 17, 2019 3:46 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, Your style requires bigger source changes to exiting modules to switch from BaseCryptLib to HashLib. Your style also required a loop to search for matching API on every call. Mine will optimize away the case statement into a direct call to the select hash family API. I do not prefer the mutually exclusive bitmap. An enumeration guarantees no ambiguity. Mike -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, December 17, 2019 1:13 PM To: Kinney, Michael D <michael.d.kinney@...>; rfc@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Mike! Based on your suggestion below and keeping in line with our implementation, I would like to recommend the following changes. I will make those and upload for review: The PCD defined (this one at SecurityPkg level) will be as follows. Note that to keep it aligned with our implementation, we are keeping the bitmap "mutually exclusive" for each algorithm. [PcdsFixedAtBuild] ## This PCD indicates the HASH algorithm to verify unsigned PE/COFF image # Based on the value set, the required algorithm is chosen to verify # the unsigned image during Secure Boot.<BR> # The hashing algorithm selected must match the hashing algorithm used to # hash the image to be added to DB using tools such as KeyEnroll.<BR> # 0x00000001 - SHA1.<BR> # 0x00000002 - SHA256.<BR> # 0x00000004 - SHA384.<BR> # 0x00000008 - SHA512.<BR> # 0x00000010 - SM3_256.<BR> # 0x00000020 - MD4.<BR> # 0x00000040 - MD5.<BR> # @Prompt Set policy for hashing unsigned image for Secure Boot. # @ValidRange 0x80000001 \| 0x00000000 - 0x00000005 gEfiSecurityPkgTokenSpaceGuid.PcdSystemHashPolicy\|0x02\| UINT32\|0x00010024 the BaseHashLib API will provide the wrapper. As an example, for HashInit, the function definition would be EFI_STATUS EFIAPI HashApiInit ( OUT HASH_HANDLE HashHandle ) { . . . HashPolicy = PcdGet32 (PcdSystemHashPolicy); if ((mBaseHashInterfaceCount == 0) \|\| !(mCurrentHashMask & HashPolicy)) { return EFI_UNSUPPORTED; } HashCtx = AllocatePool (sizeof(HashCtx)); ASSERT (HashCtx != NULL); for (Index = 0; Index < mBaseHashInterfaceCount; Index++) { HashMask = GetApiHashMaskFromAlgo (&mHashOps[Index].HashGuid); if ((HashMask & HashPolicy) != 0) { mHashOps[Index].HashInit (HashCtx); break; } } HashHandle = (HASH_HANDLE)HashCtx; return EFI_SUCCESS; } And mHashOps[Index].HashInit will be registered to the PCD value of PcdSystemHashPolicy. For example, if SHA384 is selected as value of PcdSystemHashPolicy, The HashInit call will look like, EFI_STATUS EFIAPI Sha384_Init ( OUT HASH_HANDLE HashHandle ) { VOID Sha384Ctx; UINTN CtxSize; CtxSize = Sha384GetContextSize (); Sha384Ctx = AllocatePool (CtxSize); ASSERT (Sha384Ctx != NULL); Sha384Init (Sha384Ctx); HashHandle = (HASH_HANDLE)Sha384Ctx; return EFI_SUCCESS; } Do you see any issues/challenges with this implementation? Thanks, Amol -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Tuesday, December 17, 2019 12:47 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, I would recommend a different set of APIs in the HashLib class. Instead of doing a registration, define a PCD to select the hash type and have the generic HashLib function call the BaseCryptLib APIs based on the PCD value. If the PCD is type FixedAtBuild only the hash functions specified by the PCD are included in the executable. I would also recommend the HashLib and the PCD be added to the CryptoPkg instead of the SecurityPkg. The BaseCryptLib hash families today are: MD4, MD5, SHA1, SHA256, SHA384, SHA512 SM3 Proposed FixedAtBuild PCD to declare the hash family of APIs Provided by the HashLib. [PcdsFixedAtBuild] # Family of hash API provided by the HashLib. Can be set at # global platform scope or module specific scope. # # 0x00 - No hash family selected. All HashLib APIs ASSERT(). # 0x01 - HashLib provides MD4 family of APIs. # 0x02 - HashLib provides MD5 family of APIs. # 0x03 - HashLib provides SHA1 family of APIs. # 0x04 - HashLib provides SHA256 family of APIs. # 0x05 - HashLib provides SHA384 family of APIs. # 0x06 - HashLib provides SHA512 family of APIs. # gEfiCryptoPkgTokenSpaceGuid.PcdHashLibFamily\|0x00\|UINT8 \|0x00000002 The proposed HashLib APIs are shown below and are based on the BaseCryptLib hash APIs. This would allow existing modules that are currently using BaseCryptLib to be easily updated to use the HashLib APIs and can be easily configured with a PCD to select a specific Family of hash services. Can add #defines for the families to remove use of hard coded values. UINTN EFIAPI HashGetContextSize ( VOID ) { switch (PcdGet8 (PcdHashLibFamily)) { case 0x01: return Md4GetContextSize (); case 0x02: return Md5GetContextSize (); case 0x03: return Sha1GetContextSize (); case 0x04: return Sha256GetContextSize (); case 0x05: return Sha384GetContextSize (); case 0x06: return Sha512GetContextSize (); case 0x00: default: ASSERT (FALSE); return 0; } BOOLEAN EFIAPI HashInit ( OUT VOID HashContext ); BOOLEAN EFIAPI HashDuplicate ( IN CONST VOID HashContext, OUT VOID NewHashContext ); BOOLEAN EFIAPI HashUpdate ( IN OUT VOID HashContext, IN CONST VOID Data, IN UINTN DataSize ); BOOLEAN EFIAPI HashFinal ( IN OUT VOID HashContext, OUT UINT8 HashValue ); BOOLEAN EFIAPI HashAll ( IN CONST VOID Data, IN UINTN DataSize, OUT UINT8 HashValue ); Mike -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, December 17, 2019 10:23 AM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Wang, Jian J <jian.j.wang@...>; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hello everyone, Checking if there are any review comments for https://github.com/ansukerk/edk2. Do I need to submit patches in any other format? Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 27, 2019 11:39 AM To: Kinney, Michael D <michael.d.kinney@...>; Wang, Jian J <jian.j.wang@...>; rfc@edk2.groups.io; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Musti, Srinivas <srinivas.musti@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike, I think the point # 2 you are making is also the implementation we have proposed. Also, if we can have a two tier approach then we may be in a position to improve code flexibility: 1. Define FixedAtBuild PCD at SecurityPkg DSC file that can be used by each module intending to use system level hashing algorithm. 2. For modules that need a module-specific hashing algorithm, override the PCD at SecurityPkg DSC file with a FIxedAtBuild PCD at platform level DSC file. This can eliminate the need for each module to be forced to define the PCD at platform level. Thanks, Amol -----Original Message----- From: Kinney, Michael D <michael.d.kinney@...> Sent: Tuesday, November 26, 2019 6:24 PM To: Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 As Sean points out, different components in a platform may have different hash requirements. If we want to go down the path where we have a few modules that produce Protocols/PPIs for the BaseCryptLib so all modules can share the same crypto services, then each module need to decide which specific hash algorithm it requires. Instead of defining another protocol/PPI for a unified hash service, we could consider a couple of other options: 1) Each module that uses hash services could use a FixedAtBuild PCD to decide which hash algorithm it should use. Based on this PCD value, different APIs in BaseCryptLib can be called. Since it is a FixedAtBuild PCD, the compiler/linker optimizations will only include the one hash algorithm selected. by using BaseCryptLib, this is compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs. 2) If we have many modules that need to make a hash algorithm selection, then define a new library class with the unified hash APIs, and the library instance uses a FixedAtBuild PCD to configure the hash algorithm used. This is also compatible with the proposal to wrap BaseCryptLib in Protocols/PPIs and minimizes the sources in modules that need a design that supports flexible hash algorithm selection. If new hash algorithm is added, then only the lib instance and PCD need to be updated. Each module sets the FixedAtBuild PCD value in the scope of the module in the platform DSC file. If most or all modules in a platform need the same hash algorithm, then the FixedAtBuild PCD can be set at the platform scope in the platform DSC file. Best regards, Mike -----Original Message----- From: Wang, Jian J <jian.j.wang@...> Sent: Monday, November 25, 2019 9:09 PM To: Sukerkar, Amol N <amol.n.sukerkar@...>; rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean and Amol, I believe the OBB and FV/PE image verification can still benefit from the unified API, if we add one more parameter to HashApiIinit() to force using a hash algorithm or add a PCD to choose one or link only required HashApiInstanceXxx. (Just for example) HashApiInit ( IN EFI_GUID HashAlg, OUT HASH_HANDLE *HashHandle ) In this way, we can avoid hard-code hash API in the verification module. Although it's slow, I think the OBB verification or FV/PE image signature algorithm is still subject to change per security requirement change. During a transition period, we don't have to add a new code to handle a different algorithm in verification module. Instead we just need to link a different hash lib or set PCD to use a different algorithm. I think this makes things simpler and easier to change. The BaseCryptLib implemented in PPI/Protocol is a good idea to save image size. I think the Unified hash API (BaseHashLib) can make use of it to save extra hash algorithm library instances (HashApiInstanceSha1, Sha256,...). The only problem is that the structured PCD, like Mike mentioned before, doesn't support module override, which prevents different modules from using different algorithms and then cannot save module size utter mostly. Regards, Jian -----Original Message----- From: Sukerkar, Amol N <amol.n.sukerkar@...> Sent: Tuesday, November 26, 2019 5:53 AM To: rfc@edk2.groups.io; sean.brogan@...; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: RE: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Sean, A few examples that we want to target are - HDD Password verification - Capsule Verification (Platform recovery) - Bios Guard Platform Data Table - More importantly, Boot Guard Event Log driver that has a bunch of redundant code today calculating various hashes. I don't think this is open source though. Currently, the hash verification mechanism in the above features is hard-coded to SHA256 which we will need to change to SHA384 to support a more robust hashing mechanism. This is a good opportunity to replace the existing SHA256 API with an abstracted unified API. And if necessary, the abstracted unified API still supports driver specific hashing algorithm if needed (as opposed to the strongest hashing algorithm supported by the system). We are not targeting verification of OBB or for that matter any PE/COFF image with this mechanism, mainly because it has it's own wrapper and verification mechanism in DxeImageVerificationLib. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sean via Groups.Io Sent: Monday, November 25, 2019 1:24 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Amol, I am interested to hear more about the actual use cases. I like the idea of abstracting the API so that a calling driver doesn't have to change when the hashing algorithm requirements change but am not convinced that this will actually be helpful (or at least worth the introduction of more complexity in security sensitive code). For the TPM and measured boot I agree. Random driver needs to extend a measurement then it can use the existing hashlib and it will be hashed and extended per platform policy. This is because the random driver doesn't need to understand the result. Only the TPM needs to and that is handled within the extend operation. For the other drivers that I have seen using hashing, they need to understand the result and make driver specific decisions based on the result. For example if I am verifying the OBB or some FV then I must make sure the known good and computed match. This driver must locate the known good and thus this process generally must know the format of the result. For this I think it is best to know what algorithm I am using and I don't see a strong use case to change algorithm to an abstracted algorithm. Do you have a set of example drivers/use cases that could use abstracted hash support? Thanks Sean -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N via Groups.Io Sent: Monday, November 25, 2019 12:08 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sean Brogan <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [EXTERNAL] Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Mike and Nate, With our implementation we are trying to address the following in EDKII: 1. A common Hashing API for UEFI drivers to consume instead of the current API that directly calls into the specific hashing algorithm, for instance, SHA1_Init, SHA256_Update, etc. Due to a stronger hashing algorithm requirement, certain drivers need to be upgraded to SHA384 from SHA256 and the unified API is proposed, so, while upgrading we remove the hard-coded dependency on each hashing algorithm API. 2. The size issue as a result of statically linking openssl/cryptolib API to the consuming driver. There are two ways we tried to address this issue, although, we didn't explicitly defined a PPI to address it. a. Introduced a fixed PCD per hashing algorithm to include/exclude a particular algorithm from BIOS. b. Use a register mechanism to link the instance of a particular hashing algorithm to Unified API library. This is configurable using PCD. Note that 2.a and 2.b implementation is very similar to HashLibBaseCryptoRouter used for TPM2 operations. From the size and static linking perspective, I believe we can have a discussion between 2.a and 2.b with Mike's implementation(https://nam06.safelinks.protection.outlo ok.com/?url=htt ps%3A %2F%2Fgithub.com%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_P PI_Protocol_ Proposal_V5&data=02%7C01%7Csean.brogan%40microsoft. com%7Cd110f db2a91f4e8c2acb08d771e33390%7C72f988bf86f141af91ab2d7cd 011db47%7C1 %7C0%7C637103092953247213&sdata=2acK0dmJZ85aZ3LvOOq D3e3w44V EM5dW8TEhs9USyDM%3D&reserved=0) below. We still need a common hashing API so the hashing algorithm strength for a particular driver can be increased without having to modify the consumer driver code. Comments are welcome. Thanks, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Thursday, November 21, 2019 4:45 PM To: rfc@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Thanks, Nate and Mike! I am going through the code and comments and will respond shortly. In the meantime, here is the GitHub link to my PoC for the community to look at and comment: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fansukerk%2Fedk2&data=02%7C01%7Csean.brogan%40m icrosoft.c om%7Cd110fdb2a91f4e8c2acb08d771e33390%7C72f988bf86f141a f91ab2d7cd 011db47%7C1%7C0%7C637103092953247213&sdata=keRdfU%2 BxffkBsyv YHwvSOIA8ryexOigiKhR%2B%2F1q7DjE%3D&reserved=0. Best regards, Amol -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael D Kinney Sent: Thursday, November 21, 2019 4:37 PM To: rfc@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Kinney, Michael D <michael.d.kinney@...>; 'sean.brogan@...' <sean.brogan@...>; Matthew Carlson <macarl@...>; Gao, Liming <liming.gao@...>; Feng, Bob C <bob.c.feng@...>; Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...> Cc: Agrawal, Sachin <sachin.agrawal@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Nate and Amol, There is some work already started by Sean and Matt that implements a PEIM, DXE Driver, and SMM Driver to produce Protocol/PPI that wraps the BaseCryptLib services. This content broken out into its own package is available here: https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmicrosoft%2Fmu_plus%2Ftree%2Fdev%2F201908%2FShared CryptoPkg& amp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fd b2a91f4e8c2 acb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1% 7C0%7C637 103092953247213&sdata=88TRhUqsVrj7sx8P4hjbdLy2%2BDK nrBoi7HAI3G UhpPo%3D&reserved=0 I have ported and simplified this content into a proposed set of patches to the CryptoPkg. It uses a structured PCD to configure the services mapped into the Protocols/PPIs and avoids the issue Nate notes below with protocols and PPIs including all of the BaseCryptLib services. The structured PCD allows families of crypto services or individual services within a family to be enabled/disabled. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fgith ub.c om%2Fmdkinney%2Fedk2%2Ftree%2FCryptoPkg_PPI_Protocol_Pr oposal_V5&a mp;data=02%7C01%7Csean.brogan%40microsoft.com%7Cd110fdb 2a91f4e8c2a cb08d771e33390%7C72f988bf86f141af91ab2d7cd011db47%7C1%7 C0%7C6371 03092953247213&sdata=2acK0dmJZ85aZ3LvOOqD3e3w44VEM5 dW8TEhs9 USyDM%3D&reserved=0 For example, the DSC file PCD statements to enable the SHA1 family and SHA256 family of hash services with the HashAll service disabled is: [PcdsFixedAtBuild] gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha1.Services.Has hAll \| FALSE gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Family \| PCD_CRYPTO_SERVICE_ENABLE_FAMILY gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnabl e.Sha256.Services. HashAll \| FALSE Please take a look at this proposal and let me know if this can be used to address. https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0 There is currently a limitation in the structured PCD feature that does not allow the structured PCD field values to be set in the scope of a module in a <PcdsFixedAtBuild> section. To work around this limitation, the CryptoPkg DSC file has a define called CRYPTO_SERVICES that can be set to ALL, NONE, MIN_PEI, or MIN_DXE_MIN_SMM. The default is ALL. Building with each of these values will build the modules with different sets of enabled services that matches the services enabled using multiple modules in the work from Sean and Matt. If this limitation is addressed in BaseTools, then CryptoPkg could remove the CRYPTO_SERVIES define and all !if statements. Thanks, Mike -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Nate DeSimone Sent: Wednesday, November 20, 2019 10:31 PM To: rfc@edk2.groups.io; Sukerkar, Amol N <amol.n.sukerkar@...> Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...> Subject: Re: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Hi Amol, With regard to verifying code hashes, it makes much more sense to me to have a GUID defined that represents the hash function to use versus a PCD. The reason for this is the method for signing Firmware Volumes typically involves using nested FVs where the GUID for the nested FV type indicates the algorithm needed to "extract" the nested FV. Extraction in this context is used to verify code hashes as well as decompress compressed code. The "extractor" is usually a statically linked library attached to either SignedSectionExtractPei and/or DxeIpl. For example, it is very possible for a platform to use more than one hashing algorithm, I can think of more than one closed source platform at Intel that actually does this. Now, one thing that I think your document implies is that we should have better APIs for crypto operations in general. If that was the intent then I totally agree with you. There are several reasons that I think this should be done. First, the API for OpenSSL has a tendency to change often enough to make integrating new versions into older codebases a headache, putting an abstraction layer on it would be helpful. Second, the method used to consume OpenSSL in edk2 causes some technical issues. Specifically, OpenSSL is statically linked and it defines a crazy number of global variables, enough so that on a debug build I have noticed roughly a 20KB increase in binary size even if you don't call a single function in OpenSSL. Wrapping the crypto operations in a PPI/Protocol so that only 1 driver needs to link to OpenSSL would probably help reduce binary size for most implementations. Your current document does not define a PPI/Protocol, that is something I would recommend. One issue that prior attempts of wrapping OpenSSL in a PPI/Protocol have run into is that all the crypto functions need to be linked, even the ones that a platform does not use, which results in a net negative binary size impact. Accordingly, one would likely need to break the crypto functionality into multiple PPIs/Protocols so that only the functions a platform needs are actually included. I think some experiments need to be done to figure out the most optimal division of crypto APIs. Hope that helps, Nate -----Original Message----- From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Sukerkar, Amol N Sent: Wednesday, November 20, 2019 8:28 AM To: rfc@edk2.groups.io Cc: Agrawal, Sachin <sachin.agrawal@...>; Wang, Jian J <jian.j.wang@...>; Sukerkar, Amol N <amol.n.sukerkar@...>; Sukerkar, Amol N <amol.n.sukerkar@...> Subject: [edk2-rfc] Unified API for Hashing Algorithms in EDK2 Importance: High Hello, Currently the UEFI drivers using the SHA/SM3 hashing algorithms use hard-coded API to calculate the hash, such as, sha_256(...), etc. Since SHA384 and/or SM3 are being increasingly adopted, it becomes cumbersome to modify the driver with SHA384 or SM3 calls for each application. To better achieve this, we are proposing a unified API which can be used by UEFI drivers that provides the drivers with flexibility to use the hashing algorithm they desired or the strongest hashing algorithm the system supports (with openssl). Attached is the design proposal for the same and we request feedback from the community before we begin the process of making the changes to EDK2 repo. Alternatively, the design document is also attached to Bugzilla, https://nam06.safelinks.protection.outlook.com/?url=htt ps%3A%2F%2Fbugzilla. tianocore.org%2Fshow_bug.cgi%3Fid%3D2151&data=02%7C 01%7Csean.b rogan%40microsoft.com%7Cd110fdb2a91f4e8c2acb08d771e3339 0%7C72f988b f86f141af91ab2d7cd011db47%7C1%7C0%7C637103092953247213& amp;sdata =oTt8U7ZLy3tdb3bfslqeCcDxwR1FEkNv%2F4YVcCWl3Ps%3D&r eserved=0. You can also provide the feedback in the Bugzilla. Thanks, Amol
More All Messages By This Member

1 - 17 of 17

Previous Topic Next Topic

Home Hashtags About Features Pricing Updates Terms Help Twitter