Merge pull request #2986 from MegaManSec/noticeFailure

Better handle server failures and stale responses

Author: jedisct1

dnscrypt-proxy/query_processing.go

@@ -75,6 +75,7 @@ func processDNSCryptQuery(
 			if err != nil {
 				pluginsState.returnCode = PluginsReturnCodeParseError
 				pluginsState.ApplyLoggingPlugins(&proxy.pluginsGlobals)
+				serverInfo.noticeFailure(proxy)
 				return nil, err
 			}
 			response, err = proxy.exchangeWithTCPServer(serverInfo, sharedKey, encryptedQuery, clientNonce)
@@ -87,7 +88,10 @@ func processDNSCryptQuery(
 	if err != nil {
 		if stale, ok := pluginsState.sessionData["stale"]; ok {
 			dlog.Debug("Serving stale response")
-			response, err = (stale.(*dns.Msg)).Pack()
+			serverInfo.noticeFailure(proxy)
+			if staleResponse, err := (stale.(*dns.Msg)).Pack(); err == nil {
+				return staleResponse, nil
+			}
 		}
 	}
 
@@ -119,29 +123,30 @@ func processDoHQuery(
 	serverResponse, _, tls, _, err := proxy.xTransport.DoHQuery(serverInfo.useGet, serverInfo.URL, query, proxy.timeout)
 	SetTransactionID(query, tid)
 
-	// Check for stale response if there was an error
-	if err != nil || tls == nil || !tls.HandshakeComplete {
-		if stale, ok := pluginsState.sessionData["stale"]; ok {
-			dlog.Debug("Serving stale response")
-			return (stale.(*dns.Msg)).Pack()
+	// A response was received, and the TLS handshake was complete.
+	if err == nil && tls != nil && tls.HandshakeComplete {
+		// Restore the original transaction ID
+		response := serverResponse
+		if len(response) >= MinDNSPacketSize {
+			SetTransactionID(response, tid)
 		}
+		return response, nil
 	}
 
-	// Handle error cases
-	if err != nil {
-		pluginsState.returnCode = PluginsReturnCodeNetworkError
-		pluginsState.ApplyLoggingPlugins(&proxy.pluginsGlobals)
-		serverInfo.noticeFailure(proxy)
-		return nil, err
-	}
+	serverInfo.noticeFailure(proxy)
 
-	// Restore the original transaction ID
-	response := serverResponse
-	if len(response) >= MinDNSPacketSize {
-		SetTransactionID(response, tid)
+	// Attempt to serve a stale response as a fallback.
+	if stale, ok := pluginsState.sessionData["stale"]; ok {
+		dlog.Debug("Serving stale response")
+		if staleResponse, packErr := (stale.(*dns.Msg)).Pack(); packErr == nil {
+			return staleResponse, nil
+		}
 	}
 
-	return response, nil
+	// If no stale response was served, return the original error.
+	pluginsState.returnCode = PluginsReturnCodeNetworkError
+	pluginsState.ApplyLoggingPlugins(&proxy.pluginsGlobals)
+	return nil, err
 }
 
 // processODoHQuery - Processes a query using the ODoH protocol
@@ -156,6 +161,8 @@ func processODoHQuery(
 		return nil, nil
 	}
 
+	serverInfo.noticeBegin(proxy)
+
 	target := serverInfo.odohTargetConfigs[rand.Intn(len(serverInfo.odohTargetConfigs))]
 	odohQuery, err := target.encryptQuery(query)
 	if err != nil {
@@ -175,6 +182,7 @@ func processODoHQuery(
 		response, err := odohQuery.decryptResponse(responseBody)
 		if err != nil {
 			dlog.Warnf("Failed to decrypt response from [%v]", serverInfo.Name)
+			serverInfo.noticeFailure(proxy)
 			return nil, err
 		}