summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLeander Schwarz <lschwarz@mozilla.com>2023-01-16 17:56:44 +0000
committerLeander Schwarz <lschwarz@mozilla.com>2023-01-16 17:56:44 +0000
commit8fbccd58013ff02d09c796ac1fbdbdf2382747e6 (patch)
tree4f0b8a11176be4452285d0946835943d89985914
parent02fdc1311c15077f38613b231388b1840daa5367 (diff)
downloadnss-hg-8fbccd58013ff02d09c796ac1fbdbdf2382747e6.tar.gz
Bug 1714245 - On HRR skip PSK incompatible with negotiated ciphersuites hash algorithm. r=djackson
Differential Revision: https://phabricator.services.mozilla.com/D156660
-rw-r--r--gtests/nss_bogo_shim/config.json1
-rw-r--r--lib/ssl/tls13con.c5
-rw-r--r--lib/ssl/tls13exthandle.c13
3 files changed, 17 insertions, 2 deletions
diff --git a/gtests/nss_bogo_shim/config.json b/gtests/nss_bogo_shim/config.json
index 073ad84e5..bf32596cb 100644
--- a/gtests/nss_bogo_shim/config.json
+++ b/gtests/nss_bogo_shim/config.json
@@ -47,7 +47,6 @@
"*ECH*RandomHRR*":"NSS sends real ECH in CH2 after receiving HRR rejcting ECH formally, Bogo expects instant ech_required alert. Bug 1779357",
"*ECH*UnsolicitedInnerServerNameAck":"NSS always sends SNI in CHInner, Bogo tests if the client detects an unsolicited SNI in SH if CHInner did not include it. Bug 1781224",
"CorruptTicket-TLS-TLS12":"NSS sends an alert on reception of a corrupted session ticket instead of falling back to full handshake. Bug 1783812",
- "HelloRetryRequest-NonResumableCipher-TLS13":"TODO",
"FalseStart-ALPN*":"TODO",
"####################":"####################",
diff --git a/lib/ssl/tls13con.c b/lib/ssl/tls13con.c
index 23fe279a8..4cfb6c360 100644
--- a/lib/ssl/tls13con.c
+++ b/lib/ssl/tls13con.c
@@ -3035,7 +3035,10 @@ tls13_HandleServerHelloPart2(sslSocket *ss, const PRUint8 *savedMsg, PRUint32 sa
/* We may have offered a PSK. If the server didn't negotiate
* it, clear this state to re-extract the Early Secret. */
if (ss->ssl3.hs.currentSecret) {
- PORT_Assert(ssl3_ExtensionAdvertised(ss, ssl_tls13_pre_shared_key_xtn));
+ /* We might have dropped incompatible PSKs on HRR
+ * (see RFC8466, Section 4.1.4). */
+ PORT_Assert(ss->ssl3.hs.helloRetry ||
+ ssl3_ExtensionAdvertised(ss, ssl_tls13_pre_shared_key_xtn));
PK11_FreeSymKey(ss->ssl3.hs.currentSecret);
ss->ssl3.hs.currentSecret = NULL;
}
diff --git a/lib/ssl/tls13exthandle.c b/lib/ssl/tls13exthandle.c
index 3999025c8..91f96b933 100644
--- a/lib/ssl/tls13exthandle.c
+++ b/lib/ssl/tls13exthandle.c
@@ -423,6 +423,19 @@ tls13_ClientSendPreSharedKeyXtn(const sslSocket *ss, TLSExtensionData *xtnData,
return SECSuccess;
}
+ /* ...or if PSKs are incompatible with negotiated ciphersuites
+ * (different hash algorithms) on HRR.
+ *
+ * In addition, in its updated ClientHello, the client SHOULD NOT offer any
+ * pre-shared keys associated with a hash other than that of the selected
+ * cipher suite. This allows the client to avoid having to compute partial
+ * hash transcripts for multiple hashes in the second ClientHello
+ * [RFC8446, Section 4.1.4]. */
+ if (ss->ssl3.hs.helloRetry &&
+ (psk->hash != ss->ssl3.hs.suite_def->prf_hash)) {
+ return SECSuccess;
+ }
+
/* Save where this extension starts so that if we have to add padding, it
* can be inserted before this extension. */
PORT_Assert(buf->len >= 4);