From: David Woodhouse <dwmw2@infradead.org>
Date: Wed, 28 Jul 2021 11:22:07 +0000 (+0100)
Subject: Do not truncate RSA-PSS salt length for small keys
X-Git-Tag: v8.20~76
X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=6c2266deb189a55a00be8a8f8448d879c3faff6a;p=users%2Fdwmw2%2Fopenconnect.git

Do not truncate RSA-PSS salt length for small keys

RFC8446 forbids this, and it looks like it was a bug that it ever worked
against GnuTLS.

 • https://gitlab.com/gnutls/gnutls/-/issues/1258
 • https://github.com/openssl/openssl/issues/16167

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
---

diff --git a/gnutls_tpm2.c b/gnutls_tpm2.c
index 30bc674d..fcf7af57 100644
--- a/gnutls_tpm2.c
+++ b/gnutls_tpm2.c
@@ -110,8 +110,13 @@ static int rsa_key_info(gnutls_privkey_t key, unsigned int flags, void *_certinf
 	if (flags & GNUTLS_PRIVKEY_INFO_PK_ALGO)
 		return GNUTLS_PK_RSA;
 
+	if (flags & GNUTLS_PRIVKEY_INFO_SIGN_ALGO)
+		return GNUTLS_SIGN_RSA_RAW;
+
+	int bits = tpm2_rsa_key_bits(vpninfo, certinfo);
+
 	if (flags & GNUTLS_PRIVKEY_INFO_PK_ALGO_BITS)
-		return tpm2_rsa_key_bits(vpninfo, certinfo);
+		return bits;
 
 	if (flags & GNUTLS_PRIVKEY_INFO_HAVE_SIGN_ALGO) {
 		gnutls_sign_algorithm_t algo = GNUTLS_FLAGS_TO_SIGN_ALGO(flags);
@@ -123,14 +128,24 @@ static int rsa_key_info(gnutls_privkey_t key, unsigned int flags, void *_certinf
 		case GNUTLS_SIGN_RSA_SHA512:
 			return 1;
 
+		/* Only support RSA-PSS for a given hash if the key is large
+		 * enough, since RFC8446 mandates that the salt length MUST
+		 * equal the digest output length. So we need 2N + 2 bytes. */
 		case GNUTLS_SIGN_RSA_PSS_SHA256:
 		case GNUTLS_SIGN_RSA_PSS_RSAE_SHA256:
+			if (bits >= (SHA256_SIZE * 16) + 16)
+				return 1;
+			/* Fall through */
 		case GNUTLS_SIGN_RSA_PSS_SHA384:
 		case GNUTLS_SIGN_RSA_PSS_RSAE_SHA384:
+			if (bits >= (SHA384_SIZE * 16) + 16)
+				return 1;
+			/* Fall through */
 		case GNUTLS_SIGN_RSA_PSS_SHA512:
 		case GNUTLS_SIGN_RSA_PSS_RSAE_SHA512:
-			return 1;
-
+			if (bits >= (SHA512_SIZE * 16) + 16)
+				return 1;
+			/* Fall through */
 		default:
 			vpn_progress(vpninfo, PRG_DEBUG,
 				     _("Not supporting EC sign algo %s\n"),
@@ -139,9 +154,6 @@ static int rsa_key_info(gnutls_privkey_t key, unsigned int flags, void *_certinf
 		}
 	}
 
-	if (flags & GNUTLS_PRIVKEY_INFO_SIGN_ALGO)
-		return GNUTLS_SIGN_RSA_RAW;
-
 	return -1;
 }
 #endif
@@ -461,16 +473,29 @@ static int oc_pss_mgf1_pad(struct openconnect_info *vpninfo, gnutls_digest_algor
 	 * Better still, could GnuTLS just do this all for us and we only
 	 * do a raw signature — really raw, unlike GNUTLS_SIGN_RSA_RAW
 	 * which AIUI is actually padded. */
-	if (mHash->size > emLen - 2) {
+
+	/* Actually, RFC8446 §4.2.3 mandates that the salt length MUST be
+	 * equal to the length of the output of the digest algorithm. So
+	 * truncating is it *wrong*.
+	 *
+	 *  • https://gitlab.com/gnutls/gnutls/-/issues/1258
+	 *  • https://github.com/openssl/openssl/issues/16167
+	 */
+	int sLen = mHash->size;
+	if (sLen + mHash->size > emLen - 2) {
 		vpn_progress(vpninfo, PRG_ERR,
 			     _("PSS encoding failed; hash size %d too large for RSA key %d\n"),
 			     mHash->size, emLen);
 		return GNUTLS_E_PK_SIGN_FAILED;
 	}
 
-	int sLen = mHash->size;
-	if (sLen + mHash->size > emLen - 2)
-		sLen = emLen - 2 - mHash->size;
+	/*
+	 * We don't truncate salt since RFC8446 forbids it for TLSv1.3 and
+	 * that's all we are using it for.
+	 *
+	 * if (sLen + mHash->size > emLen - 2)
+	 *	sLen = emLen - 2 - mHash->size;
+	 */
 
 	char salt[SHA512_SIZE];
 	if (sLen) {