[RFC] TLS salt length auto detection, switch from DIGEST to AUTO

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[RFC] TLS salt length auto detection, switch from DIGEST to AUTO

Andersen, John S
Hi All,

The TPM 2.0 PKCS11 project has been attempting to get the TPM working with
EAP-TLS WiFi.

We've run into an issue where the TPM spec specifies that for RSA PSS signing
keys, the random salt length will be the largest size allowed by the key size
and message digest size.

Server side, in SSL state machine the salt length gets set to
RSA_PSS_SALTLEN_DIGEST (aka -1) which means the salt length must equal the
hash length. Since the TPM used the largest size allowed by the key size and
message digest size, rather than digest size, the handshake fails.

The TSS and TPM TCG working groups will be working to modify this behavior, so
that the salt length equals the hash length. However, rolling out the update to
the spec and then firmware updates to TPMs will take a very long time. As such
we're wondering if OpenSSL would default to verifying with
RSA_PSS_SALTLEN_AUTO for RSA PSS keys instead of RSA_PSS_SALTLEN_DIGEST
as an intermediary measure.

This was my original stab at it which made it work, which of course isn't upstreamable.

diff --git a/crypto/rsa/rsa_pss.c b/crypto/rsa/rsa_pss.c
index f7c575d00a..26c9dcd078 100644
--- a/crypto/rsa/rsa_pss.c
+++ b/crypto/rsa/rsa_pss.c
@@ -50,6 +50,10 @@ int RSA_verify_PKCS1_PSS_mgf1(RSA *rsa, const unsigned char *mHash,
     hLen = EVP_MD_size(Hash);
     if (hLen < 0)
         goto err;
+
+    dprintf(2, "openssl: sLen: %d\n", sLen);
+    sLen = -2;
+
     /*-
      * Negative sLen has special meanings:
      *      -1      sLen == hLen



The following isn't hacky, but it doesn't work and I'm not yet sure why (still in the
process of debugging but wanted to float the idea on the list).



diff --git b/ssl/statem/statem_srvr.c a/ssl/statem/statem_srvr.c
index 8cf9c40d15..d6793e01a4 100644
--- b/ssl/statem/statem_srvr.c
+++ a/ssl/statem/statem_srvr.c
@@ -2783,7 +2783,7 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt)
         }
         if (lu->sig == EVP_PKEY_RSA_PSS) {
             if (EVP_PKEY_CTX_set_rsa_padding(pctx, RSA_PKCS1_PSS_PADDING) <= 0
-                || EVP_PKEY_CTX_set_rsa_pss_saltlen(pctx, RSA_PSS_SALTLEN_DIGEST) <= 0) {
+                || EVP_PKEY_CTX_set_rsa_pss_saltlen(pctx, RSA_PSS_SALTLEN_AUTO) <= 0) {
                 SSLfatal(s, SSL_AD_INTERNAL_ERROR,
                          SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE,
                         ERR_R_EVP_LIB);


Reference: https://github.com/tpm2-software/tpm2-pkcs11/pull/403#issuecomment-590395767


Thank you,
John
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] TLS salt length auto detection, switch from DIGEST to AUTO

William Roberts
On Thu, Feb 27, 2020 at 1:01 PM Andersen, John S
<[hidden email]> wrote:

>
> Hi All,
>
> The TPM 2.0 PKCS11 project has been attempting to get the TPM working with
> EAP-TLS WiFi.
>
> We've run into an issue where the TPM spec specifies that for RSA PSS signing
> keys, the random salt length will be the largest size allowed by the key size
> and message digest size.
>
> Server side, in SSL state machine the salt length gets set to
> RSA_PSS_SALTLEN_DIGEST (aka -1) which means the salt length must equal the
> hash length. Since the TPM used the largest size allowed by the key size and
> message digest size, rather than digest size, the handshake fails.
>
> The TSS and TPM TCG working groups will be working to modify this behavior, so
> that the salt length equals the hash length. However, rolling out the update to
> the spec and then firmware updates to TPMs will take a very long time. As such
> we're wondering if OpenSSL would default to verifying with
> RSA_PSS_SALTLEN_AUTO for RSA PSS keys instead of RSA_PSS_SALTLEN_DIGEST
> as an intermediary measure.
>
> This was my original stab at it which made it work, which of course isn't upstreamable.
>
> diff --git a/crypto/rsa/rsa_pss.c b/crypto/rsa/rsa_pss.c
> index f7c575d00a..26c9dcd078 100644
> --- a/crypto/rsa/rsa_pss.c
> +++ b/crypto/rsa/rsa_pss.c
> @@ -50,6 +50,10 @@ int RSA_verify_PKCS1_PSS_mgf1(RSA *rsa, const unsigned char *mHash,
>      hLen = EVP_MD_size(Hash);
>      if (hLen < 0)
>          goto err;
> +
> +    dprintf(2, "openssl: sLen: %d\n", sLen);
> +    sLen = -2;
> +
>      /*-
>       * Negative sLen has special meanings:
>       *      -1      sLen == hLen
>
>
>
> The following isn't hacky, but it doesn't work and I'm not yet sure why (still in the
> process of debugging but wanted to float the idea on the list).
>
>
>
> diff --git b/ssl/statem/statem_srvr.c a/ssl/statem/statem_srvr.c
> index 8cf9c40d15..d6793e01a4 100644
> --- b/ssl/statem/statem_srvr.c
> +++ a/ssl/statem/statem_srvr.c
> @@ -2783,7 +2783,7 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt)
>          }
>          if (lu->sig == EVP_PKEY_RSA_PSS) {
>              if (EVP_PKEY_CTX_set_rsa_padding(pctx, RSA_PKCS1_PSS_PADDING) <= 0
> -                || EVP_PKEY_CTX_set_rsa_pss_saltlen(pctx, RSA_PSS_SALTLEN_DIGEST) <= 0) {
> +                || EVP_PKEY_CTX_set_rsa_pss_saltlen(pctx, RSA_PSS_SALTLEN_AUTO) <= 0) {
>                  SSLfatal(s, SSL_AD_INTERNAL_ERROR,
>                           SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE,
>                          ERR_R_EVP_LIB);
>
>
> Reference: https://github.com/tpm2-software/tpm2-pkcs11/pull/403#issuecomment-590395767
>

I was hoping to hear some thoughts from OSSL maintainers. I guess
perhaps the question wasn't clear. The
question is, can we relax the TLS requirement that slen == hlen and
set the flag to autodetect and upstream this?

Thanks,
Bill