Re: [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Add RSA PSS verify support

["Agrawal, Sachin" <sachin.agrawal@intel.com>]

Hi Jiewen,

Thanks for sharing these references.

We are currently using Salt Length of digest length.
I will add the test for new API in the unit test framework in the next version of the patch.

In reference to adding support for RsaPssSign() API : This maybe due to my ignorance, but I am unaware of usages where BIOS is involved in doing asymmetric signing during run time. I do see that CryptoPkg also contains TLS interface and that would involve asymmetric signing, but that will directly use the OpenSSL's TLS interface for signing. And, therefore I was skeptical about adding RsaPssSign interface.

Thanks
Sachin

-----Original Message-----
From: Yao, Jiewen <jiewen.yao@intel.com> 
Sent: Tuesday, April 20, 2021 6:29 PM
To: Agrawal, Sachin <sachin.agrawal@intel.com>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
Subject: RE: [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Add RSA PSS verify support

HI Sachin
Sorry, I forget to add link for the reference.

1) TPM2 Library Specification, part 2 structure (https://trustedcomputinggroup.org/wp-content/uploads/TCG_TPM2_r1p64_Part2_Structures_15may2021.pdf) describes the PSS salt length.

For the TPM_ALG_RSAPSS signing scheme, ...
.... The salt size is
always the largest salt value that will fit into the available space.


2) NIST FIPS 186-5 draft (https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-5-draft.pdf) and NIST FIPS 186-4 (https://doi.org/10.6028/NIST.FIPS.186-4) says:

For RSASSA-PSS,
the length (in bytes) of the salt (sLen) shall satisfy 0 ≤ sLen ≤ hLen

3) TCG FIPS 140-2 Guidance for TPM2 (https://trustedcomputinggroup.org/resource/tcg-fips-140-2-guidance-for-tpm-2-0/) mentions:

Language in [1] Part 1 Appendix B.7 RSASSA_PSS indicates:
    "For both restricted and unrestricted signing keys, the random salt length will be the largest
    size allowed by the key size and message digest size.
NOTE If the TPM implementation is required to be compliant with FIPS 186-4, then the random
    salt length will be the largest size allowed by that specification."

4) TLS1.3 - RFC8446 (https://datatracker.ietf.org/doc/rfc8446/) has below.

   RSASSA-PSS PSS algorithms: 
      The length of the Salt MUST be equal to the length of the digest
      algorithm.


My view is that, TLS 1.3 and TPM FIPS mode require salt length == hash length, explicitly.

May I know that in your production, which salt length you choose in signing?
If you also choose salt length == hash length, then I would recommend make the default behavior to be HASH_LEN instead of AUTO.

Also, may I recommend we add RsaPssSign API as well?

Please also add the new API to the crypto test unit test.


I notice that crypto implementation (such as openssl, mbedtls) has API to let caller indicate what is the expected salt length. The caller may want AUTO or MAX in their special environment. I am OK to add another API later (such as RsaPssVerifyEx) to satisfy that need, if there is real use case.




> -----Original Message-----
> From: Agrawal, Sachin <sachin.agrawal@intel.com>
> Sent: Tuesday, April 20, 2021 11:20 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX 
> <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> Subject: RE: [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Add RSA PSS 
> verify support
> 
> Hi Jiewen,
> 
> I reviewed RFC 8017 and I could not find any specific 
> 'recommendations' on salt length to be used during signing with PSS encoding scheme.
> However, in Section D.5.2.2.1(Notes 2) of IEEE 1363a-2004, it is 
> recommended to use salt length atleast equal to the hash digest length.
> 
> We can modify the current API to take a additional parameter as salt 
> length and ONLY pursue verification operation if Salt length is 
> atleast equal to digest length.
> This will act as a hardening mechanism for Edk2 as it will accept 
> signatures only with 'appropriate' salt lengths.
> 
> Let me know if this is fine and I will push a corresponding patch.
> 
> Thx
> Sachin
> 
> 
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Tuesday, April 20, 2021 2:12 AM
> To: Agrawal, Sachin <sachin.agrawal@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX 
> <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> Subject: RE: [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Add RSA PSS 
> verify support
> 
> Right. That has PROs and CONs.
> 
> On one hand, that allows maximum compatibility, salt could be 
> HASH_SIZE or MAX, or even 0 ?
> 
> On the other hand, what if the consumer only wants to accept a 
> specific length? E.g. TPM in FIPS mode and TLS requires SaltLength==HashLength.
> 
> Thank you
> Yao Jiewen
> 
> 
> > -----Original Message-----
> > From: Agrawal, Sachin <sachin.agrawal@intel.com>
> > Sent: Tuesday, April 20, 2021 3:19 PM
> > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX 
> > <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> > Subject: RE: [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Add RSA PSS 
> > verify support
> >
> > Hi Jiewen,
> >
> > From Section 9.1 in RFC 8017:
> > " Note that the verification operation follows reverse steps to recover
> >    salt and then forward steps to recompute and compare H."
> >
> > Therefore, salt length can be inferred from the PSS block structure 
> > during verification operation.
> >
> > I opted for 'RSA_PSS_SALTLEN_AUTO' as it will allow Edk2 to verify 
> > PSS signatures of any salt lengths.
> >
> > Thanks
> > Sachin
> >
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Monday, April 19, 2021 7:30 PM
> > To: Agrawal, Sachin <sachin.agrawal@intel.com>; devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX 
> > <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> > Subject: RE: [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Add RSA PSS 
> > verify support
> >
> > Hi Sachin
> > May I know why you hardcode PSS salt length to be
> RSA_PSS_SALTLEN_AUTO ?
> >
> > Thank you
> > Yao Jiewen
> >
> >
> > > -----Original Message-----
> > > From: Agrawal, Sachin <sachin.agrawal@intel.com>
> > > Sent: Tuesday, April 20, 2021 10:02 AM
> > > To: devel@edk2.groups.io
> > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J 
> > > <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; 
> > > Jiang, Guomin <guomin.jiang@intel.com>; Agrawal, Sachin 
> > > <sachin.agrawal@intel.com>
> > > Subject: [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Add RSA PSS 
> > > verify support
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3314
> > >
> > > This patch uses Openssl's EVP API's to perform RSASSA-PSS 
> > > verification of a binary blob.
> > >
> > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> > > Cc: Guomin Jiang <guomin.jiang@intel.com>
> > >
> > > Signed-off-by: Sachin Agrawal <sachin.agrawal@intel.com>
> > > ---
> > >  CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c     | 139
> > > ++++++++++++++++++++
> > >  CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssNull.c |  43 ++++++
> > >  CryptoPkg/Include/Library/BaseCryptLib.h            |  27 ++++
> > >  CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf     |   1 +
> > >  CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf      |   1 +
> > >  CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf  |   1 +
> > >  CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf      |   1 +
> > >  7 files changed, 213 insertions(+)
> > >
> > > diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c
> > > b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c
> > > new file mode 100644
> > > index 000000000000..acf5eb689cd8
> > > --- /dev/null
> > > +++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c
> > > @@ -0,0 +1,139 @@
> > > +/** @file
> > > +  RSA Asymmetric Cipher Wrapper Implementation over OpenSSL.
> > > +
> > > +  This file implements following APIs which provide basic 
> > > + capabilities for
> RSA:
> > > +  1) RsaPssVerify
> > > +
> > > +Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> > > +SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +
> > > +**/
> > > +
> > > +#include "InternalCryptLib.h"
> > > +
> > > +#include <openssl/bn.h>
> > > +#include <openssl/rsa.h>
> > > +#include <openssl/objects.h>
> > > +#include <openssl/evp.h>
> > > +
> > > +
> > > +/**
> > > +  Retrieve a pointer to EVP message digest object.
> > > +
> > > +  @param[in]  DigestLen   Length of the message digest.
> > > +
> > > +**/
> > > +static
> > > +EVP_MD*
> > > +GetEvpMD (
> > > +  IN UINT16 DigestLen
> > > +  )
> > > +{
> > > +  switch (DigestLen){
> > > +    case SHA256_DIGEST_SIZE:
> > > +      return EVP_sha256();
> > > +      break;
> > > +    case SHA384_DIGEST_SIZE:
> > > +      return EVP_sha384();
> > > +      break;
> > > +    case SHA512_DIGEST_SIZE:
> > > +      return EVP_sha512();
> > > +      break;
> > > +    default:
> > > +      return NULL;
> > > +  }
> > > +}
> > > +
> > > +
> > > +/**
> > > +  Verifies the RSA signature with RSASSA-PSS signature scheme 
> > > +defined in RFC
> > > 8017.
> > > +  Implementation determines salt length automatically from the 
> > > + signature
> > > encoding.
> > > +  Mask generation function is the same as the message digest algorithm.
> > > +
> > > +  @param[in]  RsaContext      Pointer to RSA context for signature
> verification.
> > > +  @param[in]  Message         Pointer to octet message to be verified.
> > > +  @param[in]  MsgSize         Size of the message in bytes.
> > > +  @param[in]  Signature       Pointer to RSASSA-PSS signature to be verified.
> > > +  @param[in]  SigSize         Size of signature in bytes.
> > > +  @param[in]  DigestLen       Length of digest for RSA operation.
> > > +
> > > +  @retval  TRUE   Valid signature encoded in RSASSA-PSS.
> > > +  @retval  FALSE  Invalid signature or invalid RSA context.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +RsaPssVerify (
> > > +  IN  VOID         *RsaContext,
> > > +  IN  CONST UINT8  *Message,
> > > +  IN  UINTN        MsgSize,
> > > +  IN  CONST UINT8  *Signature,
> > > +  IN  UINTN        SigSize,
> > > +  IN  UINT16       DigestLen
> > > +  )
> > > +{
> > > +  BOOLEAN Result;
> > > +  EVP_PKEY *pEvpRsaKey = NULL;
> > > +  EVP_MD_CTX *pEvpVerifyCtx = NULL;
> > > +  EVP_PKEY_CTX *pKeyCtx = NULL;
> > > +  CONST EVP_MD  *HashAlg = NULL;
> > > +
> > > +  if (RsaContext == NULL) {
> > > +    return FALSE;
> > > +  }
> > > +  if (Message == NULL || MsgSize == 0 || MsgSize > INT_MAX) {
> > > +    return FALSE;
> > > +  }
> > > +  if (Signature == NULL || SigSize == 0 || SigSize > INT_MAX) {
> > > +    return FALSE;
> > > +  }
> > > +
> > > +  HashAlg = GetEvpMD(DigestLen);
> > > +
> > > +  if (HashAlg == NULL) {
> > > +    return FALSE;
> > > +  }
> > > +
> > > +  pEvpRsaKey = EVP_PKEY_new();
> > > +  if (pEvpRsaKey == NULL) {
> > > +    goto _Exit;
> > > +  }
> > > +
> > > +  EVP_PKEY_set1_RSA(pEvpRsaKey, RsaContext);
> > > +
> > > +  pEvpVerifyCtx = EVP_MD_CTX_create();  if (pEvpVerifyCtx == NULL) {
> > > +    goto _Exit;
> > > +  }
> > > +
> > > +  Result = EVP_DigestVerifyInit(pEvpVerifyCtx, &pKeyCtx, HashAlg, 
> > > + NULL,
> > > pEvpRsaKey) > 0;
> > > +  if (pKeyCtx == NULL) {
> > > +    goto _Exit;
> > > +  }
> > > +
> > > +  if (Result) {
> > > +    Result = EVP_PKEY_CTX_set_rsa_padding(pKeyCtx,
> > > RSA_PKCS1_PSS_PADDING) > 0;
> > > +  }
> > > +  if (Result) {
> > > +    Result = EVP_PKEY_CTX_set_rsa_pss_saltlen(pKeyCtx,
> > > RSA_PSS_SALTLEN_AUTO) > 0;
> > > +  }
> > > +  if (Result) {
> > > +    Result = EVP_PKEY_CTX_set_rsa_mgf1_md(pKeyCtx, HashAlg) > 0;  
> > > + } if (Result) {
> > > +    Result = EVP_DigestVerifyUpdate(pEvpVerifyCtx, Message,
> > > (UINT32)MsgSize) > 0;
> > > +  }
> > > +  if (Result) {
> > > +    Result = EVP_DigestVerifyFinal(pEvpVerifyCtx, Signature,
> > > + (UINT32)SigSize) > 0;  }
> > > +
> > > +_Exit :
> > > +  if (pEvpRsaKey) {
> > > +    EVP_PKEY_free(pEvpRsaKey);
> > > +  }
> > > +  if (pEvpVerifyCtx) {
> > > +    EVP_MD_CTX_destroy(pEvpVerifyCtx);
> > > +  }
> > > +
> > > +  return Result;
> > > +}
> > > diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssNull.c
> > > b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssNull.c
> > > new file mode 100644
> > > index 000000000000..8d84b4c1426c
> > > --- /dev/null
> > > +++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssNull.c
> > > @@ -0,0 +1,43 @@
> > > +/** @file
> > > +  RSA-PSS Asymmetric Cipher Wrapper Implementation over OpenSSL.
> > > +
> > > +  This file does not provide real capabilities for following APIs 
> > > + in RSA
> handling:
> > > +  1) RsaPssVerify
> > > +
> > > +Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> > > +SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +
> > > +**/
> > > +
> > > +#include "InternalCryptLib.h"
> > > +
> > > +/**
> > > +  Verifies the RSA signature with RSASSA-PSS signature scheme 
> > > +defined in RFC
> > > 8017.
> > > +  Implementation determines salt length automatically from the 
> > > + signature
> > > encoding.
> > > +  Mask generation function is the same as the message digest algorithm.
> > > +
> > > +  @param[in]  RsaContext      Pointer to RSA context for signature
> verification.
> > > +  @param[in]  Message         Pointer to octet message to be verified.
> > > +  @param[in]  MsgSize         Size of the message in bytes.
> > > +  @param[in]  Signature       Pointer to RSASSA-PSS signature to be verified.
> > > +  @param[in]  SigSize         Size of signature in bytes.
> > > +  @param[in]  DigestLen       Length of digest for RSA operation.
> > > +
> > > +  @retval  TRUE   Valid signature encoded in RSASSA-PSS.
> > > +  @retval  FALSE  Invalid signature or invalid RSA context.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +RsaPssVerify (
> > > +  IN  VOID         *RsaContext,
> > > +  IN  CONST UINT8  *Message,
> > > +  IN  UINTN        MsgSize,
> > > +  IN  CONST UINT8  *Signature,
> > > +  IN  UINTN        SigSize,
> > > +  IN  UINT16       DigestLen
> > > +  )
> > > +{
> > > +  ASSERT (FALSE);
> > > +  return FALSE;
> > > +}
> > > diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h
> > > b/CryptoPkg/Include/Library/BaseCryptLib.h
> > > index 496121e6a4ed..36d560b8d691 100644
> > > --- a/CryptoPkg/Include/Library/BaseCryptLib.h
> > > +++ b/CryptoPkg/Include/Library/BaseCryptLib.h
> > > @@ -1363,6 +1363,33 @@ RsaPkcs1Verify (
> > >    IN  UINTN        SigSize
> > >    );
> > >
> > > +/**
> > > +  Verifies the RSA signature with RSASSA-PSS signature scheme 
> > > +defined in RFC
> > > 8017.
> > > +  Implementation determines salt length automatically from the 
> > > + signature
> > > encoding.
> > > +  Mask generation function is the same as the message digest algorithm.
> > > +
> > > +  @param[in]  RsaContext      Pointer to RSA context for signature
> verification.
> > > +  @param[in]  Message         Pointer to octet message to be verified.
> > > +  @param[in]  MsgSize         Size of the message in bytes.
> > > +  @param[in]  Signature       Pointer to RSASSA-PSS signature to be verified.
> > > +  @param[in]  SigSize         Size of signature in bytes.
> > > +  @param[in]  DigestLen       Length of digest for RSA operation.
> > > +
> > > +  @retval  TRUE   Valid signature encoded in RSASSA-PSS.
> > > +  @retval  FALSE  Invalid signature or invalid RSA context.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +RsaPssVerify (
> > > +  IN  VOID         *RsaContext,
> > > +  IN  CONST UINT8  *Message,
> > > +  IN  UINTN        MsgSize,
> > > +  IN  CONST UINT8  *Signature,
> > > +  IN  UINTN        SigSize,
> > > +  IN  UINT16       DigestLen
> > > +  );
> > > +
> > >  /**
> > >    Retrieve the RSA Private Key from the password-protected PEM key data.
> > >