Skip to content

Commit f28f0b3

Browse files
committed
Nits
1 parent fef535c commit f28f0b3

4 files changed

Lines changed: 57 additions & 57 deletions

File tree

dnscrypt-proxy/dnsutils.go

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,34 @@ func validateResponseQuestion(query, response *dns.Msg) error {
2323
rQuestion := response.Question[0]
2424
qHeader := qQuestion.Header()
2525
rHeader := rQuestion.Header()
26-
if dns.RRToType(qQuestion) != dns.RRToType(rQuestion) || qHeader.Class != rHeader.Class || !dns.EqualName(qHeader.Name, rHeader.Name) {
26+
qType := dns.RRToType(qQuestion)
27+
rType := dns.RRToType(rQuestion)
28+
if qType != rType || qHeader.Class != rHeader.Class || !dns.EqualName(qHeader.Name, rHeader.Name) {
2729
return fmt.Errorf("Response question does not match query: %s/%d/%d != %s/%d/%d",
2830
rHeader.Name,
29-
dns.RRToType(rQuestion),
31+
rType,
3032
rHeader.Class,
3133
qHeader.Name,
32-
dns.RRToType(qQuestion),
34+
qType,
3335
qHeader.Class,
3436
)
3537
}
3638
return nil
3739
}
3840

41+
func validateResponseForQuery(query, response *dns.Msg) error {
42+
if query == nil || response == nil {
43+
return errors.New("Missing query or response")
44+
}
45+
if !response.Response {
46+
return errors.New("response bit is not set")
47+
}
48+
if response.ID != query.ID {
49+
return fmt.Errorf("response ID mismatch: %d != %d", response.ID, query.ID)
50+
}
51+
return validateResponseQuestion(query, response)
52+
}
53+
3954
func EmptyResponseFromMessage(srcMsg *dns.Msg) *dns.Msg {
4055
dstMsg := &dns.Msg{}
4156
dstMsg.ID = srcMsg.ID
@@ -522,13 +537,7 @@ func _dnsExchange(
522537
if err := msg.Unpack(); err != nil {
523538
return DNSExchangeResponse{err: err}
524539
}
525-
if !msg.Response {
526-
return DNSExchangeResponse{err: errors.New("response bit is not set")}
527-
}
528-
if msg.ID != query.ID {
529-
return DNSExchangeResponse{err: fmt.Errorf("response ID mismatch: %d != %d", msg.ID, query.ID)}
530-
}
531-
if err := validateResponseQuestion(query, &msg); err != nil {
540+
if err := validateResponseForQuery(query, &msg); err != nil {
532541
return DNSExchangeResponse{err: err}
533542
}
534543
return DNSExchangeResponse{response: &msg, rtt: rtt, err: nil}

dnscrypt-proxy/plugins.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ func (pluginsState *PluginsState) ApplyResponsePlugins(
342342
if err := msg.Unpack(); err != nil {
343343
return packet, err
344344
}
345-
if err := validateResponseQuestion(pluginsState.questionMsg, &msg); err != nil {
345+
if err := validateResponseForQuery(pluginsState.questionMsg, &msg); err != nil {
346346
return packet, err
347347
}
348348
switch Rcode(packet) {
@@ -381,6 +381,9 @@ func (pluginsState *PluginsState) ApplyResponsePlugins(
381381
}
382382
pluginsGlobals.RUnlock()
383383
}
384+
if err := validateResponseForQuery(pluginsState.questionMsg, &msg); err != nil {
385+
return packet, err
386+
}
384387
if err := msg.Pack(); err != nil {
385388
return packet, err
386389
}

dnscrypt-proxy/query_processing.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ func validateQuery(query []byte) bool {
2323

2424
// handleSynthesizedResponse - Handles a synthesized DNS response from plugins
2525
func handleSynthesizedResponse(pluginsState *PluginsState, synth *dns.Msg) ([]byte, error) {
26+
if err := validateResponseForQuery(pluginsState.questionMsg, synth); err != nil {
27+
pluginsState.returnCode = PluginsReturnCodeParseError
28+
return nil, err
29+
}
2630
if err := synth.Pack(); err != nil {
2731
pluginsState.returnCode = PluginsReturnCodeParseError
2832
return nil, err
@@ -296,12 +300,11 @@ func processPlugins(
296300
}
297301

298302
if pluginsState.synthResponse != nil {
299-
if err = pluginsState.synthResponse.Pack(); err != nil {
300-
pluginsState.returnCode = PluginsReturnCodeParseError
303+
response, err = handleSynthesizedResponse(pluginsState, pluginsState.synthResponse)
304+
if err != nil {
301305
pluginsState.ApplyLoggingPlugins(&proxy.pluginsGlobals)
302306
return response, err
303307
}
304-
response = pluginsState.synthResponse.Data
305308
}
306309

307310
// Check rcode and handle failures

dnscrypt-proxy/serversInfo.go

Lines changed: 28 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -874,30 +874,24 @@ func fetchDNSCryptServerInfo(proxy *Proxy, name string, stamp stamps.ServerStamp
874874
&name,
875875
false,
876876
)
877-
if err == nil && len(msg.Question) > 0 {
878-
question := msg.Question[0]
879-
if dns.RRToType(question) == dns.RRToType(query.Question[0]) && strings.EqualFold(question.Header().Name, query.Question[0].Header().Name) {
880-
dlog.Debugf("[%s] also serves plaintext DNS", name)
881-
if msg.ID != 0xcafe {
882-
dlog.Infof("[%s] handling of DNS message identifiers is broken", name)
883-
}
884-
for _, rr := range msg.Answer {
885-
rrType := dns.RRToType(rr)
886-
if rrType == dns.TypeA || rrType == dns.TypeAAAA {
887-
dlog.Warnf("[%s] may be a lying resolver -- skipping", name)
888-
return ServerInfo{}, fmt.Errorf("[%s] unexpected record: [%s]", name, rr.String())
889-
}
877+
if err == nil {
878+
dlog.Debugf("[%s] also serves plaintext DNS", name)
879+
for _, rr := range msg.Answer {
880+
rrType := dns.RRToType(rr)
881+
if rrType == dns.TypeA || rrType == dns.TypeAAAA {
882+
dlog.Warnf("[%s] may be a lying resolver -- skipping", name)
883+
return ServerInfo{}, fmt.Errorf("[%s] unexpected record: [%s]", name, rr.String())
890884
}
891-
for _, rr := range msg.Extra {
892-
if dns.RRToType(rr) == dns.TypeTXT {
893-
dlog.Warnf("[%s] may be a dummy resolver -- skipping", name)
894-
txts := rr.(*dns.TXT).Txt
895-
cause := ""
896-
if len(txts) > 0 {
897-
cause = txts[0]
898-
}
899-
return ServerInfo{}, fmt.Errorf("[%s] unexpected record: [%s]", name, cause)
885+
}
886+
for _, rr := range msg.Extra {
887+
if dns.RRToType(rr) == dns.TypeTXT {
888+
dlog.Warnf("[%s] may be a dummy resolver -- skipping", name)
889+
txts := rr.(*dns.TXT).Txt
890+
cause := ""
891+
if len(txts) > 0 {
892+
cause = txts[0]
900893
}
894+
return ServerInfo{}, fmt.Errorf("[%s] unexpected record: [%s]", name, cause)
901895
}
902896
}
903897
}
@@ -919,7 +913,7 @@ func fetchDNSCryptServerInfo(proxy *Proxy, name string, stamp stamps.ServerStamp
919913
}, nil
920914
}
921915

922-
func dohTestPacket(msgID uint16) []byte {
916+
func dohTestPacket(msgID uint16) *dns.Msg {
923917
msg := dns.NewMsg(".", dns.TypeNS)
924918
msg.ID = msgID
925919
msg.RecursionDesired = true
@@ -932,10 +926,10 @@ func dohTestPacket(msgID uint16) []byte {
932926
if err := msg.Pack(); err != nil {
933927
dlog.Fatal(err)
934928
}
935-
return msg.Data
929+
return msg
936930
}
937931

938-
func dohNXTestPacket(msgID uint16) []byte {
932+
func dohNXTestPacket(msgID uint16) *dns.Msg {
939933
qName := make([]byte, 16)
940934
charset := "abcdefghijklmnopqrstuvwxyz"
941935
for i := range qName {
@@ -953,7 +947,7 @@ func dohNXTestPacket(msgID uint16) []byte {
953947
if err := msg.Pack(); err != nil {
954948
dlog.Fatal(err)
955949
}
956-
return msg.Data
950+
return msg
957951
}
958952

959953
func plainNXTestPacket(msgID uint16) *dns.Msg {
@@ -985,7 +979,7 @@ func fetchDoHServerInfo(proxy *Proxy, name string, stamp stamps.ServerStamp, isN
985979
Host: stamp.ProviderName,
986980
Path: stamp.Path,
987981
}
988-
body := dohTestPacket(0xcafe)
982+
body := dohTestPacket(0xcafe).Data
989983
useGet := false
990984
if _, _, _, _, err := proxy.xTransport.DoHQuery(useGet, url, body, proxy.timeout); err != nil {
991985
useGet = true
@@ -994,25 +988,21 @@ func fetchDoHServerInfo(proxy *Proxy, name string, stamp stamps.ServerStamp, isN
994988
}
995989
dlog.Debugf("Server [%s] doesn't appear to support POST; falling back to GET requests", name)
996990
}
997-
body = dohNXTestPacket(0xcafe)
998-
serverResponse, _, tls, rtt, err := proxy.xTransport.DoHQuery(useGet, url, body, proxy.timeout)
991+
queryMsg := dohNXTestPacket(0xcafe)
992+
serverResponse, _, tls, rtt, err := proxy.xTransport.DoHQuery(useGet, url, queryMsg.Data, proxy.timeout)
999993
if err != nil {
1000994
dlog.Infof("[%s] [%s]: %v", name, url, err)
1001995
return ServerInfo{}, err
1002996
}
1003997
if tls == nil || !tls.HandshakeComplete {
1004998
return ServerInfo{}, errors.New("TLS handshake failed")
1005999
}
1006-
queryMsg := dns.Msg{Data: body}
1007-
if err := queryMsg.Unpack(); err != nil {
1008-
return ServerInfo{}, err
1009-
}
10101000
msg := dns.Msg{Data: serverResponse}
10111001
if err := msg.Unpack(); err != nil {
10121002
dlog.Warnf("[%s]: %v", name, err)
10131003
return ServerInfo{}, err
10141004
}
1015-
if err := validateResponseQuestion(&queryMsg, &msg); err != nil {
1005+
if err := validateResponseForQuery(queryMsg, &msg); err != nil {
10161006
return ServerInfo{}, err
10171007
}
10181008
if msg.Rcode != dns.RcodeNameError {
@@ -1133,8 +1123,7 @@ func _fetchODoHTargetInfo(proxy *Proxy, name string, stamp stamps.ServerStamp, i
11331123
for _, odohTargetConfig := range odohTargetConfigs {
11341124
url := relay.ODoH.URL
11351125

1136-
query := dohTestPacket(0xcafe)
1137-
odohQuery, err := odohTargetConfig.encryptQuery(query)
1126+
odohQuery, err := odohTargetConfig.encryptQuery(dohTestPacket(0xcafe).Data)
11381127
if err != nil {
11391128
continue
11401129
}
@@ -1148,8 +1137,8 @@ func _fetchODoHTargetInfo(proxy *Proxy, name string, stamp stamps.ServerStamp, i
11481137
dlog.Debugf("Server [%s] doesn't appear to support POST; falling back to GET requests", name)
11491138
}
11501139

1151-
query = dohNXTestPacket(0xcafe)
1152-
odohQuery, err = odohTargetConfig.encryptQuery(query)
1140+
queryMsg := dohNXTestPacket(0xcafe)
1141+
odohQuery, err = odohTargetConfig.encryptQuery(queryMsg.Data)
11531142
if err != nil {
11541143
continue
11551144
}
@@ -1173,16 +1162,12 @@ func _fetchODoHTargetInfo(proxy *Proxy, name string, stamp stamps.ServerStamp, i
11731162
}
11741163
workingConfigs = append(workingConfigs, odohTargetConfig)
11751164

1176-
queryMsg := dns.Msg{Data: query}
1177-
if err := queryMsg.Unpack(); err != nil {
1178-
return ServerInfo{}, err
1179-
}
11801165
msg := dns.Msg{Data: serverResponse}
11811166
if err := msg.Unpack(); err != nil {
11821167
dlog.Warnf("[%s]: %v", name, err)
11831168
return ServerInfo{}, err
11841169
}
1185-
if err := validateResponseQuestion(&queryMsg, &msg); err != nil {
1170+
if err := validateResponseForQuery(queryMsg, &msg); err != nil {
11861171
return ServerInfo{}, err
11871172
}
11881173
if msg.Rcode != dns.RcodeNameError {

0 commit comments

Comments
 (0)