Skip to content

Commit

Permalink
fix accidental pipelining of http doh requests
Browse files Browse the repository at this point in the history
  • Loading branch information
cottand committed Nov 6, 2023
1 parent dd4937b commit 1023a2e
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 11 deletions.
19 changes: 11 additions & 8 deletions doh.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
stdlog "log"
"net"
"net/http"
"strconv"
"time"
)

Expand Down Expand Up @@ -109,9 +108,10 @@ func (s *ServerHTTPS) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}

var writer = DohResponseWriter{remoteAddr: r.RemoteAddr, host: r.Host, delegate: w}
var writer = DohResponseWriter{remoteAddr: r.RemoteAddr, host: r.Host, delegate: w, completed: make(chan empty)}
s.handler.ServeDNS(&writer, msg)
if writer.err != nil {
_, ok := <-writer.completed
if writer.err != nil || ok != true {
return
}

Expand Down Expand Up @@ -170,7 +170,7 @@ func getRequestToMsg(req *http.Request) (*dns.Msg, error) {
}

func base64ToMsg(b64 string) (*dns.Msg, error) {
buf, err := b64Enc.DecodeString(b64)
buf, err := base64.RawURLEncoding.DecodeString(b64)
if err != nil {
return nil, err
}
Expand All @@ -181,16 +181,16 @@ func base64ToMsg(b64 string) (*dns.Msg, error) {
return m, err
}

var b64Enc = base64.RawURLEncoding

var _ dns.ResponseWriter = &DohResponseWriter{}
type empty struct{}

// DohResponseWriter implements dns.ResponseWriter
type DohResponseWriter struct {
msg *dns.Msg
remoteAddr string
delegate http.ResponseWriter
host string
err error
completed chan empty
}

// See section 4.2.1 of RFC 8484.
Expand All @@ -215,13 +215,16 @@ func (w *DohResponseWriter) RemoteAddr() net.Addr {
}

func (w *DohResponseWriter) WriteMsg(msg *dns.Msg) error {
defer func() {
w.completed <- empty{}
close(w.completed)
}()
w.msg = msg
buf, err := msg.Pack()
if err != nil {
w.handleErr(err)
return err
}
w.delegate.Header().Set("Content-Length", strconv.Itoa(len(buf)))
w.delegate.Header().Set("Content-Type", mimeTypeDOH)
_, err = w.Write(buf)
if err != nil {
Expand Down
5 changes: 2 additions & 3 deletions grimd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,13 +172,12 @@ func TestDohIntegration(t *testing.T) {
})
}

// TestDohAsProxy checks that DoH works for non-custom records
func TestDohAsProxy(t *testing.T) {
t.SkipNow() // see https://github.com/coredns/coredns/issues/4547
dohBind := "localhost:8181"
integrationTest(func(c *Config) {
c.DnsOverHttpServer.Bind = dohBind
c.DnsOverHttpServer.Enabled = true
//c.CustomDNSRecords = []string{"example.com IN A 10.10.0.1 "}
}, func(_ *dns.Client, _ string) {
resp, err := http.Get("http://" + dohBind + "/dns-query?dns=AAABAAABAAAAAAAAA3d3dwdleGFtcGxlA2NvbQAAAQAB")

Expand All @@ -194,7 +193,7 @@ func TestDohAsProxy(t *testing.T) {
msg := dns.Msg{}
err = msg.Unpack(respPacket)
if err != nil {
t.Fatalf("unexpected error during lookup %v", err)
t.Fatalf("unexpected error during lookup %v (response len=%vB)", err, len(respPacket))
}

if len(msg.Answer) < 1 {
Expand Down

0 comments on commit 1023a2e

Please sign in to comment.