fancy-dns/internal/server/funcs_test.go

package server

import (
	"bytes"
	"crypto/tls"
	"encoding/base64"
	"errors"
	"io"
	"net/http"
	"net/http/httptest"
	"strings"
	"testing"
	"time"

	"github.com/miekg/dns"
)

// countingReader counts the bytes readCapped actually pulled out of a body, so
// the "refused on the header" branch can be told apart from the "refused while
// reading" branch. Both return an error; only one of them let the client send
// the body.
type countingReader struct {
	r io.Reader
	n int
}

func (c *countingReader) Read(p []byte) (int, error) {
	n, err := c.r.Read(p)
	c.n += n
	return n, err
}

// errReader emits a prefix and then fails, standing in for a client that
// disconnects mid-body. readCapped has to surface that as an error: a truncated
// DNS message handed to dns.Msg.Unpack would be unpacked as garbage.
type errReader struct {
	prefix string
	err    error
	done   bool
}

func (f *errReader) Read(p []byte) (int, error) {
	if f.done {
		return 0, f.err
	}
	f.done = true
	return copy(p, f.prefix), nil
}

// TestReadCapped covers the DoH POST body reader. It is a security control, not
// a convenience: the handler is unauthenticated, and the bound is what stops a
// client from making the server allocate an arbitrary amount of memory per
// request. Two branches have to both work, because the whole cap is only as
// strong as the weaker one:
//
//   - a declared Content-Length over the cap is refused WITHOUT reading the
//     body at all (the header is a fast path, and the counting reader proves
//     nothing was read);
//   - a missing or understated Content-Length is caught by the reader itself,
//     which takes max+1 bytes and only then says "too large".
func TestReadCapped(t *testing.T) {
	const max = 16
	full := strings.Repeat("a", max)
	cases := []struct {
		name string
		// body is what the client sends.
		body string
		// failAfter, when non-zero, makes the body stop with an error after
		// that many bytes.
		failAfter int
		// length is the Content-Length the request advertises. -1 is
		// net/http's value for a chunked request: no declared length at all.
		length int64
		want   string
		// wantReads is the number of body bytes readCapped should have
		// consumed. -1 means the case is not about how much was read.
		wantReads int
		wantErr   bool
	}{
		{
			name: "an empty body is legal", body: "", length: 0,
			want: "", wantReads: 0,
		},
		{
			name: "a body exactly at the cap is accepted whole", body: full, length: max,
			want: full, wantReads: max,
		},
		{
			name: "one byte over the cap with an honest Content-Length is refused before the body is read",
			body: full + "a", length: max + 1,
			want: "", wantReads: 0, wantErr: true,
		},
		{
			// A client may claim any length it likes, so an over-declared
			// length must not be the only thing that decides. It is still
			// the fast path, so it must not cost a read.
			name: "a wildly over-declared Content-Length is refused before the body is read",
			body: full + "a", length: 64 << 20,
			want: "", wantReads: 0, wantErr: true,
		},
		{
			// ContentLength == -1 is what net/http reports for a chunked
			// request, so this is the branch a length-less client hits.
			name: "one byte over the cap with no declared length is caught while reading",
			body: full + "a", length: -1,
			want: "", wantReads: max + 1, wantErr: true,
		},
		{
			name: "a body under the cap with no declared length is accepted whole",
			body: "chunked-body", length: -1,
			want: "chunked-body", wantReads: len("chunked-body"),
		},
		{
			// The cap is not the only thing that can end a read early, and a
			// half-read body must not be handed on as if it were complete.
			name: "a body that fails mid-read surfaces the read error",
			body: "abcde", failAfter: 5, length: -1,
			want: "", wantReads: 5, wantErr: true,
		},
	}
	for _, tc := range cases {
		t.Run(tc.name, func(t *testing.T) {
			var src io.Reader = strings.NewReader(tc.body)
			if tc.failAfter > 0 {
				src = &errReader{prefix: tc.body, err: errors.New("connection reset by peer")}
			}
			cr := &countingReader{r: src}
			r := httptest.NewRequest(http.MethodPost, "/dns-query", nil)
			r.Body = io.NopCloser(cr)
			r.ContentLength = tc.length

			got, err := readCapped(r, max)
			if tc.wantErr {
				if err == nil {
					t.Fatalf("readCapped accepted %q, want an error", got)
				}
				if got != nil {
					t.Errorf("readCapped returned %q alongside its error, want nil", got)
				}
			} else {
				if err != nil {
					t.Fatalf("readCapped(%q) = %v, want it accepted", tc.want, err)
				}
				if string(got) != tc.want {
					t.Errorf("readCapped = %q, want %q", got, tc.want)
				}
			}
			if tc.wantReads >= 0 && cr.n != tc.wantReads {
				t.Errorf("readCapped read %d body bytes, want %d", cr.n, tc.wantReads)
			}
		})
	}
}

// TestDecodeBase64URL covers the RFC 8484 §4.1 GET parameter decoder. The two
// encodings it accepts are base64url with and without padding: the RFC asks for
// the unpadded form, every real client sends the padded one, and refusing the
// padded form would break all of them. What it must NOT accept is the standard
// alphabet, because '+' and '/' have to be percent-encoded in a query string
// and a decoder that took them would be a decoder for a different encoding.
func TestDecodeBase64URL(t *testing.T) {
	msg := []byte{0xde, 0xad, 0xbe, 0xef, 0x00, 0x01, 0x02, 0x03}
	// Standard base64 of these bytes contains both '+' and '/'.
	std := base64.StdEncoding.EncodeToString(msg)
	if !strings.ContainsAny(std, "+/") {
		t.Fatalf("fixture %q contains no + or /; the standard-alphabet case would prove nothing", std)
	}
	cases := []struct {
		name    string
		in      string
		want    []byte
		wantErr bool
	}{
		{name: "unpadded base64url", in: base64.RawURLEncoding.EncodeToString(msg), want: msg},
		{name: "padded base64url", in: base64.URLEncoding.EncodeToString(msg), want: msg},
		{name: "standard base64 with + and / is rejected", in: std, wantErr: true},
		{name: "a space inside the parameter is rejected", in: "QQ BB", wantErr: true},
		{name: "a tab inside the parameter is rejected", in: "QQ\tBB", wantErr: true},
		{
			// encoding/base64 skips CR and LF in every encoding, padded or
			// not. A newline in ?dns= therefore decodes rather than 400s.
			// It is harmless — the bytes are the same message — and it is
			// pinned here so nobody "fixes" it by assuming the decoder
			// rejects all whitespace. The size cap is applied to the raw
			// parameter before decoding, so this cannot be used to smuggle
			// an oversized message past the cap.
			name: "a newline inside the parameter is ignored by the decoder",
			in:   "QQ\n==", want: []byte{0x41},
		},
		{name: "an empty parameter decodes to nothing", in: "", want: []byte{}},
		{name: "a truncated quantum is rejected", in: "QQQQQ", wantErr: true},
		{name: "a single character is rejected", in: "A", wantErr: true},
		{
			// Two characters is a legal UNPADDED encoding of one byte, so it
			// decodes on the RawURLEncoding path before the padded one is
			// tried. It is not an error case; the case above ("QQQQQ") is,
			// because five characters is not a whole number of quanta.
			name: "two characters are a valid unpadded encoding of one byte",
			in:   "QQ", want: []byte{0x41},
		},
		{
			// decodeBase64URL is a pure decoder: it imposes no size limit,
			// because the limit that matters (the longest possible encoding
			// of a 65535-byte DNS message) is a property of the caller's
			// parameter, and the DoH handler applies it before calling here.
			// The bound itself is pinned in TestDoHGETQueryStringOverTheCapIsRefused.
			name: "a long but legal parameter decodes; the cap is the handler's job",
			in:   base64.RawURLEncoding.EncodeToString(bytes.Repeat([]byte{0x41}, 1<<20)),
			want: bytes.Repeat([]byte{0x41}, 1<<20),
		},
	}
	for _, tc := range cases {
		t.Run(tc.name, func(t *testing.T) {
			got, err := decodeBase64URL(tc.in)
			if tc.wantErr {
				if err == nil {
					t.Fatalf("decodeBase64URL(%q) = %x, want an error", tc.in, got)
				}
				return
			}
			if err != nil {
				t.Fatalf("decodeBase64URL(%q) = %v, want it accepted", tc.in, err)
			}
			if !bytes.Equal(got, tc.want) {
				t.Errorf("decodeBase64URL(%q) = %x, want %x", tc.in, got, tc.want)
			}
		})
	}
	// The cases above are the decoder's edge cases; this is the case that
	// actually happens. A real query must survive the round trip byte for
	// byte, in both encodings, or a client using the padded form (which is
	// what curl and most browsers send) gets a 400 for a well-formed query.
	wire, err := question("roundtrip.test.", dns.TypeA).Pack()
	if err != nil {
		t.Fatalf("pack: %v", err)
	}
	for _, enc := range []struct {
		name string
		in   string
	}{
		{"unpadded", base64.RawURLEncoding.EncodeToString(wire)},
		{"padded", base64.URLEncoding.EncodeToString(wire)},
	} {
		t.Run("a real query round trips, "+enc.name, func(t *testing.T) {
			got, err := decodeBase64URL(enc.in)
			if err != nil {
				t.Fatalf("decodeBase64URL: %v", err)
			}
			if !bytes.Equal(got, wire) {
				t.Fatalf("round trip changed the message: %d bytes in, %d out", len(wire), len(got))
			}
			if m := new(dns.Msg); m.Unpack(got) != nil {
				t.Fatalf("the decoded bytes are not a DNS message")
			}
		})
	}
}

// TestParseTLSVersion pins the fallback, which is a security setting: an
// operator who typos min_version, or leaves it empty, must get TLS 1.2 — never
// TLS 1.0 and never "whatever the Go default happens to be this release".
func TestParseTLSVersion(t *testing.T) {
	cases := []struct {
		in   string
		want uint16
		why  string
	}{
		{in: "1.2", want: tls.VersionTLS12, why: "the documented value"},
		{in: "1.3", want: tls.VersionTLS13, why: "the documented value"},
		{in: "1.0", want: tls.VersionTLS10, why: "still reachable, though config documents only 1.2/1.3"},
		{in: "1.1", want: tls.VersionTLS12, why: "unknown values fail UP to 1.2, never down to 1.1"},
		{in: "", want: tls.VersionTLS12, why: "an unset min_version means the secure default"},
		{in: "junk", want: tls.VersionTLS12, why: "a typo must not weaken the floor"},
		{in: "13", want: tls.VersionTLS12, why: "only the exact string 1.3 is honoured"},
		{in: "1.3 ", want: tls.VersionTLS12, why: "no trimming: an odd value falls back, it does not panic"},
	}
	for _, tc := range cases {
		t.Run("min_version="+tc.in, func(t *testing.T) {
			if got := parseTLSVersion(tc.in); got != tc.want {
				t.Errorf("parseTLSVersion(%q) = %d, want %d (%s)", tc.in, got, tc.want, tc.why)
			}
		})
	}
}

// TestNormaliseBase covers the DoH base path normaliser. What it does is
// deliberately narrow — add the leading slash, trim the trailing ones — because
// it runs on operator config at Start and must never fail a startup. What it
// must NOT grow is a second parser: the path-traversal cases are here to pin
// that the string is passed through verbatim, because the thing that actually
// keeps a traversal out of the handler is net/http's ServeMux, which cleans
// the path before it matches. That is pinned by
// TestDoHBasePathTraversalNeverReachesTheHandler below; a sanitiser here would
// be a second, weaker implementation of the same rule.
func TestNormaliseBase(t *testing.T) {
	cases := []struct{ in, want, why string }{
		{in: "", want: "/dns-query", why: "the default, so an unset base path still serves DoH"},
		{in: "query", want: "/query", why: "a path written without its leading slash is what operators type"},
		{in: "/query", want: "/query", why: "already normalised"},
		{in: "/query/", want: "/query", why: "a trailing slash would register a different mux pattern"},
		{in: "/query///", want: "/query", why: "all of them, and never the empty string"},
		{in: "/", want: "/", why: "serve at the root is a legal (if blunt) choice"},
		{in: "///", want: "/", why: "trimming stops at a single slash"},
		{in: "///query///", want: "///query", why: "only a MISSING leading slash is added; runs of them are the mux's business"},
		{in: "/a//b", want: "/a//b", why: "only the ends are touched; an inner // is left for the mux"},
		{in: "/query?x=1", want: "/query?x=1", why: "no query splitting: this is a path, not a URL"},
		{in: "/../etc", want: "/../etc", why: "passed through; ServeMux cleans the path (see the traversal test)"},
		{in: "/dns-query/..", want: "/dns-query/..", why: "passed through, same reason"},
	}
	for _, tc := range cases {
		t.Run(tc.in, func(t *testing.T) {
			if got := normaliseBase(tc.in); got != tc.want {
				t.Errorf("normaliseBase(%q) = %q, want %q (%s)", tc.in, got, tc.want, tc.why)
			}
		})
	}
}

// TestDoHBasePathTraversalNeverReachesTheHandler is the security half of the
// normaliseBase comment above. The DoH mux is built from a configured path, so
// the question is what happens when a request arrives with dot segments in it:
// it must never be answered by the DNS handler, and it must never reach the
// engine. net/http's ServeMux cleans the path and issues a 307 to the cleaned
// form, which is why there is no sanitiser in normaliseBase. Redirects are not
// followed here: following one would answer this test with the redirect
// TARGET's behaviour rather than the traversal's.
func TestDoHBasePathTraversalNeverReachesTheHandler(t *testing.T) {
	h := newHarness(t, nil)
	cl := noRedirectClient(t, h)
	cases := []struct {
		path string
		want int
	}{
		// Cleaned to /healthz, which is a registered path: a 307 to it, and
		// the DNS handler never sees the request. ServeMux is what keeps the
		// traversal out; normaliseBase deliberately does not try.
		{path: "/dns-query/../healthz", want: http.StatusTemporaryRedirect},
		{path: "/dns-query/./x", want: http.StatusTemporaryRedirect},
		// Encoded dot segments are not cleaned by the mux, so they match no
		// pattern at all.
		{path: "/dns-query/%2e%2e/healthz", want: http.StatusNotFound},
		{path: "/dns-query/..%2fhealthz", want: http.StatusNotFound},
		{path: "/..%2f..%2fetc/passwd", want: http.StatusNotFound},
	}
	for _, tc := range cases {
		t.Run(tc.path, func(t *testing.T) {
			res, err := cl.Get(h.dohURL(tc.path))
			if err != nil {
				t.Fatalf("GET %s: %v", tc.path, err)
			}
			_ = res.Body.Close()
			if res.StatusCode != tc.want {
				t.Errorf("GET %s = %d, want %d", tc.path, res.StatusCode, tc.want)
			}
		})
	}
	// Nothing above reached the engine, and the plain path still works, so
	// the 404s are not the DNS handler being broken. Exactly one request
	// reaches the engine: the control query.
	h.fake.reset()
	if a := answerAddr(h.askDoHGET(question("control.test.", dns.TypeA))); a != "203.0.113.9" {
		t.Errorf("control query answered %q, want 203.0.113.9", a)
	}
	if n := len(h.fake.seen()); n != 1 {
		t.Errorf("%d requests reached the engine, want exactly the control query: %+v", n, h.fake.seen())
	}
}

// noRedirectClient is h.dohClient without redirect following, so a test can
// assert on the status the server produced rather than on whatever the
// Location header points at.
func noRedirectClient(t *testing.T, h *harness) *http.Client {
	t.Helper()
	tr := &http.Transport{TLSClientConfig: h.clientTLS()}
	t.Cleanup(tr.CloseIdleConnections)
	return &http.Client{
		Transport: tr,
		Timeout:   5 * time.Second,
		CheckRedirect: func(*http.Request, []*http.Request) error {
			return http.ErrUseLastResponse
		},
	}
}