Bug 1753204 - Retry without 0-RTT after SSL_ERROR_HANDSHAKE_UNEXPECTE…

…D_ALERT r=necko-reviewers,valentin Differential Revision: https://phabricator.services.mozilla.com/D139302
dothq · Feb 29, 2024 · 29695cd · 29695cd
1 parent 86760e7
commit 29695cd
Show file tree

Hide file tree

Showing 6 changed files with 16 additions and 14 deletions.
diff --git a/netwerk/protocol/http/Http2Session.cpp b/netwerk/protocol/http/Http2Session.cpp
@@ -223,7 +223,7 @@ void Http2Session::ShutdownStream(Http2StreamBase* aStream, nsresult aReason) {
     CloseStream(aStream, NS_ERROR_NET_INADEQUATE_SECURITY);
   } else if (!mCleanShutdown && (mGoAwayReason != NO_HTTP_ERROR)) {
     CloseStream(aStream, NS_ERROR_NET_HTTP2_SENT_GOAWAY);
-  } else if (!mCleanShutdown && SecurityErrorThatMayNeedRestart(aReason)) {
+  } else if (!mCleanShutdown && PossibleZeroRTTRetryError(aReason)) {
     CloseStream(aStream, aReason);
   } else {
     CloseStream(aStream, NS_ERROR_ABORT);

diff --git a/netwerk/protocol/http/nsHttp.cpp b/netwerk/protocol/http/nsHttp.cpp
@@ -1017,13 +1017,15 @@ SupportedAlpnRank IsAlpnSupported(const nsACString& aAlpn) {
   return SupportedAlpnRank::NOT_SUPPORTED;
 }
 
-// On some security error when 0RTT is used we want to restart transactions
-// without 0RTT. Some firewalls do not behave well with 0RTT and cause this
-// errors.
-bool SecurityErrorThatMayNeedRestart(nsresult aReason) {
+// NSS Errors which *may* have been triggered by the use of 0-RTT in the
+// presence of badly behaving middleboxes. We may re-attempt the connection
+// without early data.
+bool PossibleZeroRTTRetryError(nsresult aReason) {
   return (aReason ==
           psm::GetXPCOMFromNSSError(SSL_ERROR_PROTOCOL_VERSION_ALERT)) ||
-         (aReason == psm::GetXPCOMFromNSSError(SSL_ERROR_BAD_MAC_ALERT));
+         (aReason == psm::GetXPCOMFromNSSError(SSL_ERROR_BAD_MAC_ALERT)) ||
+         (aReason ==
+          psm::GetXPCOMFromNSSError(SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT));
 }
 
 nsresult MakeOriginURL(const nsACString& origin, nsCOMPtr<nsIURI>& url) {

diff --git a/netwerk/protocol/http/nsHttp.h b/netwerk/protocol/http/nsHttp.h
@@ -499,8 +499,6 @@ static inline bool AllowedErrorForHTTPSRRFallback(nsresult aError) {
          aError == NS_ERROR_UNKNOWN_HOST || aError == NS_ERROR_NET_TIMEOUT;
 }
 
-bool SecurityErrorThatMayNeedRestart(nsresult aReason);
-
 [[nodiscard]] nsresult MakeOriginURL(const nsACString& origin,
                                      nsCOMPtr<nsIURI>& url);
 
@@ -521,6 +519,8 @@ uint64_t WebTransportErrorToHttp3Error(uint8_t aErrorCode);
 
 uint8_t Http3ErrorToWebTransportError(uint64_t aErrorCode);
 
+bool PossibleZeroRTTRetryError(nsresult aReason);
+
 }  // namespace net
 }  // namespace mozilla
 

diff --git a/netwerk/protocol/http/nsHttpConnection.cpp b/netwerk/protocol/http/nsHttpConnection.cpp
@@ -742,7 +742,7 @@ void nsHttpConnection::Close(nsresult reason, bool aIsShutdown) {
       gHttpHandler->ClearHostMapping(mConnInfo);
     }
     if (mTlsHandshaker->EarlyDataWasAvailable() &&
-        SecurityErrorThatMayNeedRestart(reason)) {
+        PossibleZeroRTTRetryError(reason)) {
       gHttpHandler->Exclude0RttTcp(mConnInfo);
     }
 

diff --git a/netwerk/protocol/http/nsHttpTransaction.cpp b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -1341,7 +1341,7 @@ bool nsHttpTransaction::ShouldRestartOn0RttError(nsresult reason) {
        "mEarlyDataWasAvailable=%d error=%" PRIx32 "]\n",
        this, mEarlyDataWasAvailable, static_cast<uint32_t>(reason)));
   return StaticPrefs::network_http_early_data_disable_on_error() &&
-         mEarlyDataWasAvailable && SecurityErrorThatMayNeedRestart(reason);
+         mEarlyDataWasAvailable && PossibleZeroRTTRetryError(reason);
 }
 
 static void MaybeRemoveSSLToken(nsITransportSecurityInfo* aSecurityInfo) {
@@ -1508,7 +1508,7 @@ void nsHttpTransaction::Close(nsresult reason) {
 
     if (reason ==
             psm::GetXPCOMFromNSSError(SSL_ERROR_DOWNGRADE_WITH_EARLY_DATA) ||
-        reason == psm::GetXPCOMFromNSSError(SSL_ERROR_PROTOCOL_VERSION_ALERT) ||
+        PossibleZeroRTTRetryError(reason) ||
         (!mReceivedData && ((mRequestHead && mRequestHead->IsSafeMethod()) ||
                             !reallySentData || connReused)) ||
         shouldRestartTransactionForHTTPSRR) {
@@ -1549,9 +1549,8 @@ void nsHttpTransaction::Close(nsresult reason) {
       } else if (reason == psm::GetXPCOMFromNSSError(
                                SSL_ERROR_DOWNGRADE_WITH_EARLY_DATA)) {
         SetRestartReason(TRANSACTION_RESTART_DOWNGRADE_WITH_EARLY_DATA);
-      } else if (reason ==
-                 psm::GetXPCOMFromNSSError(SSL_ERROR_PROTOCOL_VERSION_ALERT)) {
-        SetRestartReason(TRANSACTION_RESTART_PROTOCOL_VERSION_ALERT);
+      } else if (PossibleZeroRTTRetryError(reason)) {
+        SetRestartReason(TRANSACTION_RESTART_POSSIBLE_0RTT_ERROR);
       }
       // if restarting fails, then we must proceed to close the pipe,
       // which will notify the channel that the transaction failed.

diff --git a/netwerk/protocol/http/nsHttpTransaction.h b/netwerk/protocol/http/nsHttpTransaction.h
@@ -299,6 +299,7 @@ class nsHttpTransaction final : public nsAHttpTransaction,
     TRANSACTION_RESTART_HTTP3_FAST_FALLBACK,
     TRANSACTION_RESTART_OTHERS,
     TRANSACTION_RESTART_PROTOCOL_VERSION_ALERT,
+    TRANSACTION_RESTART_POSSIBLE_0RTT_ERROR
   };
   void SetRestartReason(TRANSACTION_RESTART_REASON aReason);