From 9315c3e6b6ff161da77090e0271975792a6bb098 Mon Sep 17 00:00:00 2001 From: Joshua Rogers Date: Wed, 24 Sep 2025 20:11:19 +0200 Subject: [PATCH 1/4] =?UTF-8?q?processDNSCryptQuery:=20call=20noticeFailur?= =?UTF-8?q?e=20on=20UDP=E2=86=92TCP=20re-encrypt=20error?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After noticeBegin, the UDP->TCP retry path returned early on Encrypt error without notifying failure. Invoke serverInfo.noticeFailure() before returning to keep begin/failure pairing and avoid skewed observability metrics. --- dnscrypt-proxy/query_processing.go | 1 + 1 file changed, 1 insertion(+) diff --git a/dnscrypt-proxy/query_processing.go b/dnscrypt-proxy/query_processing.go index 8ebf27673b..38a343523f 100644 --- a/dnscrypt-proxy/query_processing.go +++ b/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) From d9f8895a60ab6d0445d10f43d89de9fa631b13a9 Mon Sep 17 00:00:00 2001 From: Joshua Rogers Date: Wed, 24 Sep 2025 22:23:31 +0200 Subject: [PATCH 2/4] Further clean up serverInfo.noticeBegin and serverInfo.noticeFailure --- dnscrypt-proxy/query_processing.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/dnscrypt-proxy/query_processing.go b/dnscrypt-proxy/query_processing.go index 38a343523f..f5eaebe067 100644 --- a/dnscrypt-proxy/query_processing.go +++ b/dnscrypt-proxy/query_processing.go @@ -88,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, packErr := (stale.(*dns.Msg)).Pack(); packErr == nil { + return staleResponse, nil + } } } @@ -122,9 +125,12 @@ func processDoHQuery( // Check for stale response if there was an error if err != nil || tls == nil || !tls.HandshakeComplete { + serverInfo.noticeFailure(proxy) if stale, ok := pluginsState.sessionData["stale"]; ok { - dlog.Debug("Serving stale response") - return (stale.(*dns.Msg)).Pack() + dlog.Debug("Serving stale response after error") + if staleResponse, err := (stale.(*dns.Msg)).Pack(); err == nil { + return staleResponse, nil + } } } @@ -132,7 +138,6 @@ func processDoHQuery( if err != nil { pluginsState.returnCode = PluginsReturnCodeNetworkError pluginsState.ApplyLoggingPlugins(&proxy.pluginsGlobals) - serverInfo.noticeFailure(proxy) return nil, err } @@ -157,10 +162,13 @@ func processODoHQuery( return nil, nil } + serverInfo.noticeBegin(proxy) + target := serverInfo.odohTargetConfigs[rand.Intn(len(serverInfo.odohTargetConfigs))] odohQuery, err := target.encryptQuery(query) if err != nil { dlog.Errorf("Failed to encrypt query for [%v]", serverInfo.Name) + serverInfo.noticeFailure(proxy) return nil, err } @@ -176,6 +184,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 } From 8edb1eac73610ed9052d6c2f579aa9b6b1435e7e Mon Sep 17 00:00:00 2001 From: Joshua Rogers Date: Wed, 24 Sep 2025 22:28:44 +0200 Subject: [PATCH 3/4] Clean up proxy.xTransport.DoHQuery and handle correctly handle when tls == nil || !tls.HandshakeComplete --- dnscrypt-proxy/query_processing.go | 41 +++++++++++++++--------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/dnscrypt-proxy/query_processing.go b/dnscrypt-proxy/query_processing.go index f5eaebe067..7b8913646b 100644 --- a/dnscrypt-proxy/query_processing.go +++ b/dnscrypt-proxy/query_processing.go @@ -87,9 +87,9 @@ func processDNSCryptQuery( // Check for stale response if there was an error if err != nil { if stale, ok := pluginsState.sessionData["stale"]; ok { - dlog.Debug("Serving stale response") + dlog.Debug("Serving stale response after error") serverInfo.noticeFailure(proxy) - if staleResponse, packErr := (stale.(*dns.Msg)).Pack(); packErr == nil { + if staleResponse, err := (stale.(*dns.Msg)).Pack(); err == nil { return staleResponse, nil } } @@ -123,31 +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 { - serverInfo.noticeFailure(proxy) - if stale, ok := pluginsState.sessionData["stale"]; ok { - dlog.Debug("Serving stale response after error") - if staleResponse, err := (stale.(*dns.Msg)).Pack(); err == nil { - return staleResponse, nil - } + // 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) - 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 after error") + 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 From cc629856dffbef9e1a3685be3cd44b4cb1dc3452 Mon Sep 17 00:00:00 2001 From: Joshua Rogers Date: Wed, 24 Sep 2025 23:04:05 +0200 Subject: [PATCH 4/4] fixup error msg + do not penalize on client error --- dnscrypt-proxy/query_processing.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dnscrypt-proxy/query_processing.go b/dnscrypt-proxy/query_processing.go index 7b8913646b..56e3b53327 100644 --- a/dnscrypt-proxy/query_processing.go +++ b/dnscrypt-proxy/query_processing.go @@ -87,7 +87,7 @@ func processDNSCryptQuery( // Check for stale response if there was an error if err != nil { if stale, ok := pluginsState.sessionData["stale"]; ok { - dlog.Debug("Serving stale response after error") + dlog.Debug("Serving stale response") serverInfo.noticeFailure(proxy) if staleResponse, err := (stale.(*dns.Msg)).Pack(); err == nil { return staleResponse, nil @@ -137,7 +137,7 @@ func processDoHQuery( // Attempt to serve a stale response as a fallback. if stale, ok := pluginsState.sessionData["stale"]; ok { - dlog.Debug("Serving stale response after error") + dlog.Debug("Serving stale response") if staleResponse, packErr := (stale.(*dns.Msg)).Pack(); packErr == nil { return staleResponse, nil } @@ -167,7 +167,6 @@ func processODoHQuery( odohQuery, err := target.encryptQuery(query) if err != nil { dlog.Errorf("Failed to encrypt query for [%v]", serverInfo.Name) - serverInfo.noticeFailure(proxy) return nil, err }