diff options
| author | Dennis Jackson <djackson@mozilla.com> | 2023-05-05 09:16:14 +0000 |
|---|---|---|
| committer | Dennis Jackson <djackson@mozilla.com> | 2023-05-05 09:16:14 +0000 |
| commit | 4a4054b5586eb7f1f3bc01afe9aa93a69a582dbe (patch) | |
| tree | c1fba5917e53045aa8a00ea69e9769a5ed262fba | |
| parent | a7d6369a22adcb31d3c76379e4ea0cdfad0bd8cf (diff) | |
| download | nss-hg-4a4054b5586eb7f1f3bc01afe9aa93a69a582dbe.tar.gz | |
Bug 1829391 - Tidy up DTLS ACK Error Handling Path. r=mt
Differential Revision: https://phabricator.services.mozilla.com/D176156
| -rw-r--r-- | gtests/ssl_gtest/ssl_agent_unittest.cc | 4 | ||||
| -rw-r--r-- | lib/ssl/dtls13con.c | 15 |
2 files changed, 13 insertions, 6 deletions
diff --git a/gtests/ssl_gtest/ssl_agent_unittest.cc b/gtests/ssl_gtest/ssl_agent_unittest.cc index 8c0957d2f..283bfec16 100644 --- a/gtests/ssl_gtest/ssl_agent_unittest.cc +++ b/gtests/ssl_gtest/ssl_agent_unittest.cc @@ -154,7 +154,6 @@ TEST_F(TlsAgentDgramTestClient, AckWithBogusLengthField) { sizeof(ackBuf), &record, 0); agent_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_3, SSL_LIBRARY_VERSION_TLS_1_3); - ExpectAlert(kTlsAlertDecodeError); ProcessMessage(record, TlsAgent::STATE_ERROR, SSL_ERROR_RX_MALFORMED_DTLS_ACK); } @@ -171,7 +170,8 @@ TEST_F(TlsAgentDgramTestClient, AckWithNonEvenLength) { // Because we haven't negotiated the version, // ssl3_DecodeError() sends an older (pre-TLS error). ExpectAlert(kTlsAlertIllegalParameter); - ProcessMessage(record, TlsAgent::STATE_ERROR, SSL_ERROR_BAD_SERVER); + ProcessMessage(record, TlsAgent::STATE_ERROR, + SSL_ERROR_RX_MALFORMED_DTLS_ACK); } TEST_F(TlsAgentStreamTestClient, Set0RttOptionThenWrite) { diff --git a/lib/ssl/dtls13con.c b/lib/ssl/dtls13con.c index 5307419b6..a2cf4a8c6 100644 --- a/lib/ssl/dtls13con.c +++ b/lib/ssl/dtls13con.c @@ -460,11 +460,10 @@ dtls13_HandleAck(sslSocket *ss, sslBuffer *databuf) SSL_TRC(10, ("%d: SSL3[%d]: Handling ACK", SSL_GETPID(), ss->fd)); rv = ssl3_ConsumeHandshakeNumber(ss, &length, 2, &b, &l); if (rv != SECSuccess) { - return SECFailure; + goto loser; } if (length != l) { - tls13_FatalError(ss, SSL_ERROR_RX_MALFORMED_DTLS_ACK, decode_error); - return SECFailure; + goto loser; } while (l > 0) { @@ -473,7 +472,7 @@ dtls13_HandleAck(sslSocket *ss, sslBuffer *databuf) rv = ssl3_ConsumeHandshakeNumber64(ss, &seq, 8, &b, &l); if (rv != SECSuccess) { - return SECFailure; + goto loser; } for (cursor = PR_LIST_HEAD(&ss->ssl3.hs.dtlsSentHandshake); @@ -521,6 +520,14 @@ dtls13_HandleAck(sslSocket *ss, sslBuffer *databuf) } } return SECSuccess; + +loser: + /* Due to bug 1829391 we may incorrectly send an alert rather than + * ignore an invalid record here. */ + SSL_TRC(11, ("%d: SSL3[%d]: Error processing DTLS1.3 ACK.", + SSL_GETPID(), ss->fd)); + PORT_SetError(SSL_ERROR_RX_MALFORMED_DTLS_ACK); + return SECFailure; } /* Clean up the read timer for the handshake cipher suites on the |