Bug 1654507 - Part2: Plumbing for echRetry, r=necko-reviewers,dragana

Differential Revision: https://phabricator.services.mozilla.com/D119403

netwerk/protocol/http/Http3Session.cpp

@@ -219,10 +219,13 @@ void Http3Session::DoSetEchConfig(const nsACString& aEchConfig) {
 void Http3Session::Shutdown() {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
+  bool isEchRetry = mError == mozilla::psm::GetXPCOMFromNSSError(
+                                  SSL_ERROR_ECH_RETRY_WITH_ECH);
   if ((mBeforeConnectedError ||
        (mError == NS_ERROR_NET_HTTP3_PROTOCOL_ERROR)) &&
       (mError !=
-       mozilla::psm::GetXPCOMFromNSSError(SSL_ERROR_BAD_CERT_DOMAIN))) {
+       mozilla::psm::GetXPCOMFromNSSError(SSL_ERROR_BAD_CERT_DOMAIN)) &&
+      !isEchRetry) {
     gHttpHandler->ExcludeHttp3(mConnInfo);
   }
 
@@ -232,7 +235,13 @@ void Http3Session::Shutdown() {
       // The transaction restart code path will remove AltSvc mapping and the
       // direct path will be used.
       MOZ_ASSERT(NS_FAILED(mError));
-      stream->Close(NS_ERROR_NET_RESET);
+      if (isEchRetry) {
+        // We have to propagate this error to nsHttpTransaction, so the
+        // transaction will be restarted with a new echConfig.
+        stream->Close(mError);
+      } else {
+        stream->Close(NS_ERROR_NET_RESET);
+      }
     } else if (!stream->HasStreamId()) {
       if (NS_SUCCEEDED(mError)) {
         // Connection has not been started yet. We can restart it.
@@ -467,7 +476,7 @@ nsresult Http3Session::ProcessEvents() {
         if (!mAuthenticationStarted) {
           mAuthenticationStarted = true;
           LOG(("Http3Session::ProcessEvents - AuthenticationNeeded called"));
-          CallCertVerification();
+          CallCertVerification(Nothing());
         }
         break;
       case Http3Event::Tag::ZeroRttRejected:
@@ -556,6 +565,11 @@ nsresult Http3Session::ProcessEvents() {
           if (isStatelessResetOrNoError(event.connection_closing.error)) {
             mError = NS_ERROR_NET_RESET;
           }
+          if (event.connection_closing.error.tag == CloseError::Tag::EchRetry) {
+            mSocketControl->SetRetryEchConfig(Substring(
+                reinterpret_cast<const char*>(data.Elements()), data.Length()));
+            mError = psm::GetXPCOMFromNSSError(SSL_ERROR_ECH_RETRY_WITH_ECH);
+          }
         }
         return mError;
         break;
@@ -566,11 +580,28 @@ nsresult Http3Session::ProcessEvents() {
           CloseConnectionTelemetry(event.connection_closed.error, false);
         }
         mIsClosedByNeqo = true;
+        if (event.connection_closed.error.tag == CloseError::Tag::EchRetry) {
+          mSocketControl->SetRetryEchConfig(Substring(
+              reinterpret_cast<const char*>(data.Elements()), data.Length()));
+          mError = psm::GetXPCOMFromNSSError(SSL_ERROR_ECH_RETRY_WITH_ECH);
+        }
         LOG(("Http3Session::ProcessEvents - ConnectionClosed error=%" PRIx32,
              static_cast<uint32_t>(mError)));
         // We need to return here and let HttpConnectionUDP close the session.
         return mError;
         break;
+      case Http3Event::Tag::EchFallbackAuthenticationNeeded: {
+        nsCString echPublicName(reinterpret_cast<const char*>(data.Elements()),
+                                data.Length());
+        LOG(
+            ("Http3Session::ProcessEvents - EchFallbackAuthenticationNeeded "
+             "echPublicName=%s",
+             echPublicName.get()));
+        if (!mAuthenticationStarted) {
+          mAuthenticationStarted = true;
+          CallCertVerification(Some(echPublicName));
+        }
+      } break;
       default:
         break;
     }
@@ -1562,7 +1593,7 @@ bool Http3Session::RealJoinConnection(const nsACString& hostname, int32_t port,
   return joinedReturn;
 }
 
-void Http3Session::CallCertVerification() {
+void Http3Session::CallCertVerification(Maybe<nsCString> aEchPublicName) {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
   LOG(("Http3Session::CallCertVerification [this=%p]", this));
 
@@ -1588,9 +1619,17 @@ void Http3Session::CallCertVerification() {
   // the return value is always NS_OK, just ignore it.
   Unused << mSocketControl->GetProviderFlags(&providerFlags);
 
+  nsCString echConfig;
+  nsresult nsrv = mSocketControl->GetEchConfig(echConfig);
+  bool verifyToEchPublicName = NS_SUCCEEDED(nsrv) && !echConfig.IsEmpty() &&
+                               aEchPublicName && !aEchPublicName->IsEmpty();
+  const nsACString& hostname =
+      verifyToEchPublicName ? *aEchPublicName : mSocketControl->GetHostName();
+
   SECStatus rv = AuthCertificateHookWithInfo(
-      mSocketControl, static_cast<const void*>(this), std::move(certInfo.certs),
-      stapledOCSPResponse, sctsFromTLSExtension, providerFlags);
+      mSocketControl, hostname, static_cast<const void*>(this),
+      std::move(certInfo.certs), stapledOCSPResponse, sctsFromTLSExtension,
+      providerFlags);
   if ((rv != SECSuccess) && (rv != SECWouldBlock)) {
     LOG(("Http3Session::CallCertVerification [this=%p] AuthCertificate failed",
          this));

netwerk/protocol/http/Http3Session.h

@@ -127,7 +127,7 @@ class Http3Session final : public nsAHttpTransaction, public nsAHttpConnection {
   void RemoveStreamFromQueues(Http3Stream*);
   void ProcessPending();
 
-  void CallCertVerification();
+  void CallCertVerification(Maybe<nsCString> aEchPublicName);
   void SetSecInfo();
 
   void StreamReadyToWrite(Http3Stream* aStream);

netwerk/test/unit/test_httpssvc_retry_with_ech.js

@@ -276,3 +276,24 @@ async function H3ECHTest(echConfig) {
 add_task(async function testH3ConnectWithECH() {
   await H3ECHTest(h3EchConfig);
 });
+
+add_task(async function testH3ConnectWithECHRetry() {
+  dns.clearCache(true);
+  Services.obs.notifyObservers(null, "net:cancel-all-connections");
+  await new Promise(resolve => setTimeout(resolve, 1000));
+
+  function base64ToArray(base64) {
+    var binary_string = atob(base64);
+    var len = binary_string.length;
+    var bytes = new Uint8Array(len);
+    for (var i = 0; i < len; i++) {
+      bytes[i] = binary_string.charCodeAt(i);
+    }
+    return bytes;
+  }
+
+  let decodedConfig = base64ToArray(h3EchConfig);
+  decodedConfig[6] ^= 0x94;
+  let encoded = btoa(String.fromCharCode.apply(null, decodedConfig));
+  await H3ECHTest(encoded);
+});

security/manager/ssl/SSLServerCertVerification.cpp

@@ -1273,8 +1273,8 @@ SECStatus AuthCertificateHook(void* arg, PRFileDesc* fd, PRBool checkSig,
 // checks and calls SSLServerCertVerificationJob::Dispatch.
 // This function is used for Quic.
 SECStatus AuthCertificateHookWithInfo(
-    TransportSecurityInfo* infoObject, const void* aPtrForLogging,
-    nsTArray<nsTArray<uint8_t>>&& peerCertChain,
+    TransportSecurityInfo* infoObject, const nsACString& aHostName,
+    const void* aPtrForLogging, nsTArray<nsTArray<uint8_t>>&& peerCertChain,
     Maybe<nsTArray<nsTArray<uint8_t>>>& stapledOCSPResponses,
     Maybe<nsTArray<uint8_t>>& sctsFromTLSExtension, uint32_t providerFlags) {
   if (peerCertChain.IsEmpty()) {
@@ -1311,10 +1311,10 @@ SECStatus AuthCertificateHookWithInfo(
   // for Delegated Credentials.
   Maybe<DelegatedCredentialInfo> dcInfo;
 
-  return AuthCertificateHookInternal(
-      infoObject, aPtrForLogging, cert, infoObject->GetHostName(),
-      std::move(peerCertChain), stapledOCSPResponse, sctsFromTLSExtension,
-      dcInfo, providerFlags, certVerifierFlags);
+  return AuthCertificateHookInternal(infoObject, aPtrForLogging, cert,
+                                     aHostName, std::move(peerCertChain),
+                                     stapledOCSPResponse, sctsFromTLSExtension,
+                                     dcInfo, providerFlags, certVerifierFlags);
 }
 
 NS_IMPL_ISUPPORTS_INHERITED0(SSLServerCertVerificationResult, Runnable)

security/manager/ssl/SSLServerCertVerification.h

@@ -34,8 +34,8 @@ SECStatus AuthCertificateHook(void* arg, PRFileDesc* fd, PRBool checkSig,
 // asynchronous and the info object will be notified when the verification has
 // completed via SetCertVerificationResult.
 SECStatus AuthCertificateHookWithInfo(
-    TransportSecurityInfo* infoObject, const void* aPtrForLogging,
-    nsTArray<nsTArray<uint8_t>>&& peerCertChain,
+    TransportSecurityInfo* infoObject, const nsACString& aHostName,
+    const void* aPtrForLogging, nsTArray<nsTArray<uint8_t>>&& peerCertChain,
     Maybe<nsTArray<nsTArray<uint8_t>>>& stapledOCSPResponses,
     Maybe<nsTArray<uint8_t>>& sctsFromTLSExtension, uint32_t providerFlags);