From f69cd8d63d1a098bcceaa8313c6cc1c75dcee419 Mon Sep 17 00:00:00 2001 From: Dominik Menke Date: Sat, 5 Oct 2019 13:44:38 +0200 Subject: [PATCH] feat: ease operation behind proxy servers (#974) --- .golangci.toml | 9 + challenge/http01/domain_matcher.go | 184 +++++++++++++++++ challenge/http01/domain_matcher_test.go | 86 ++++++++ challenge/http01/http_challenge_server.go | 44 ++++- challenge/http01/http_challenge_test.go | 228 +++++++++++++++++++++- cmd/flags.go | 5 + cmd/setup_challenges.go | 12 +- docs/content/usage/cli/_index.md | 17 +- 8 files changed, 565 insertions(+), 20 deletions(-) create mode 100644 challenge/http01/domain_matcher.go create mode 100644 challenge/http01/domain_matcher_test.go diff --git a/.golangci.toml b/.golangci.toml index b50f78bc..5ff9033e 100644 --- a/.golangci.toml +++ b/.golangci.toml @@ -58,6 +58,15 @@ [[issues.exclude-rules]] path = "challenge/dns01/nameserver.go" text = "`(defaultNameservers|recursiveNameservers|dnsTimeout|fqdnToZone|muFqdnToZone)` is a global variable" + [[issues.exclude-rules]] + path = "challenge/http01/domain_matcher.go" + text = "string `Host` has \\d occurrences, make it a constant" + [[issues.exclude-rules]] + path = "challenge/http01/domain_matcher.go" + text = "cyclomatic complexity \\d+ of func `parseForwardedHeader` is high" + [[issues.exclude-rules]] + path = "challenge/http01/domain_matcher.go" + text = "Function 'parseForwardedHeader' has too many statements" [[issues.exclude-rules]] path = "challenge/tlsalpn01/tls_alpn_challenge.go" text = "`idPeAcmeIdentifierV1` is a global variable" diff --git a/challenge/http01/domain_matcher.go b/challenge/http01/domain_matcher.go new file mode 100644 index 00000000..a28d175a --- /dev/null +++ b/challenge/http01/domain_matcher.go @@ -0,0 +1,184 @@ +package http01 + +import ( + "fmt" + "net/http" + "strings" +) + +// A domainMatcher tries to match a domain (the one we're requesting a certificate for) +// in the HTTP request coming from the ACME validation servers. +// This step is part of DNS rebind attack prevention, +// where the webserver matches incoming requests to a list of domain the server acts authoritative for. +// +// The most simple check involves finding the domain in the HTTP Host header; +// this is what hostMatcher does. +// Use it, when the http01.ProviderServer is directly reachable from the internet, +// or when it operates behind a transparent proxy. +// +// In many (reverse) proxy setups, Apache and NGINX traditionally move the Host header to a new header named X-Forwarded-Host. +// Use arbitraryMatcher("X-Forwarded-Host") in this case, +// or the appropriate header name for other proxy servers. +// +// RFC7239 has standardized the different forwarding headers into a single header named Forwarded. +// The header value has a different format, so you should use forwardedMatcher +// when the http01.ProviderServer operates behind a RFC7239 compatible proxy. +// https://tools.ietf.org/html/rfc7239 +// +// Note: RFC7239 also reminds us, "that an HTTP list [...] may be split over multiple header fields" (section 7.1), +// meaning that +// X-Header: a +// X-Header: b +// is equal to +// X-Header: a, b +// +// All matcher implementations (explicitly not excluding arbitraryMatcher!) +// have in common that they only match against the first value in such lists. +type domainMatcher interface { + // matches checks whether the request is valid for the given domain. + matches(request *http.Request, domain string) bool + + // name returns the header name used in the check. + // This is primarily used to create meaningful error messages. + name() string +} + +// hostMatcher checks whether (*net/http).Request.Host starts with a domain name. +type hostMatcher struct{} + +func (m *hostMatcher) name() string { + return "Host" +} + +func (m *hostMatcher) matches(r *http.Request, domain string) bool { + return strings.HasPrefix(r.Host, domain) +} + +// hostMatcher checks whether the specified (*net/http.Request).Header value starts with a domain name. +type arbitraryMatcher string + +func (m arbitraryMatcher) name() string { + return string(m) +} + +func (m arbitraryMatcher) matches(r *http.Request, domain string) bool { + return strings.HasPrefix(r.Header.Get(m.name()), domain) +} + +// forwardedMatcher checks whether the Forwarded header contains a "host" element starting with a domain name. +// See https://tools.ietf.org/html/rfc7239 for details. +type forwardedMatcher struct{} + +func (m *forwardedMatcher) name() string { + return "Forwarded" +} + +func (m *forwardedMatcher) matches(r *http.Request, domain string) bool { + fwds, err := parseForwardedHeader(r.Header.Get(m.name())) + if err != nil { + return false + } + if len(fwds) == 0 { + + return false + } + + host := fwds[0]["host"] + return strings.HasPrefix(host, domain) +} + +// parsing requires some form of state machine +func parseForwardedHeader(s string) (elements []map[string]string, err error) { + cur := make(map[string]string) + key := "" + val := "" + inquote := false + + pos := 0 + l := len(s) + for i := 0; i < l; i++ { + r := rune(s[i]) + + if inquote { + if r == '"' { + cur[key] = s[pos:i] + key = "" + pos = i + inquote = false + } + continue + } + + switch { + case r == '"': // start of quoted-string + if key == "" { + return nil, fmt.Errorf("unexpected quoted string as pos %d", i) + } + inquote = true + pos = i + 1 + + case r == ';': // end of forwarded-pair + cur[key] = s[pos:i] + key = "" + i = skipWS(s, i) + pos = i + 1 + + case r == '=': // end of token + key = strings.ToLower(strings.TrimFunc(s[pos:i], isWS)) + i = skipWS(s, i) + pos = i + 1 + + case r == ',': // end of forwarded-element + if key != "" { + if val == "" { + val = s[pos:i] + } + cur[key] = val + } + elements = append(elements, cur) + cur = make(map[string]string) + key = "" + val = "" + + i = skipWS(s, i) + pos = i + 1 + case tchar(r) || isWS(r): // valid token character or whitespace + continue + default: + return nil, fmt.Errorf("invalid token character at pos %d: %c", i, r) + } + } + + if inquote { + return nil, fmt.Errorf("unterminated quoted-string at pos %d", len(s)) + } + + if key != "" { + if pos < len(s) { + val = s[pos:] + } + cur[key] = val + } + if len(cur) > 0 { + elements = append(elements, cur) + } + return elements, nil +} + +func tchar(r rune) bool { + return strings.ContainsRune("!#$%&'*+-.^_`|~", r) || + '0' <= r && r <= '9' || + 'a' <= r && r <= 'z' || + 'A' <= r && r <= 'Z' +} + +func skipWS(s string, i int) int { + for isWS(rune(s[i+1])) { + i++ + } + return i +} + +func isWS(r rune) bool { + return strings.ContainsRune(" \t\v\r\n", r) +} diff --git a/challenge/http01/domain_matcher_test.go b/challenge/http01/domain_matcher_test.go new file mode 100644 index 00000000..63d7ca7f --- /dev/null +++ b/challenge/http01/domain_matcher_test.go @@ -0,0 +1,86 @@ +package http01 + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseForwardedHeader(t *testing.T) { + testCases := []struct { + name string + input string + want []map[string]string + err string + }{ + { + name: "empty input", + input: "", + want: nil, + }, + { + name: "simple case", + input: `for=1.2.3.4;host=example.com; by=127.0.0.1`, + want: []map[string]string{ + {"for": "1.2.3.4", "host": "example.com", "by": "127.0.0.1"}, + }, + }, + { + name: "quoted-string", + input: `foo="bar"`, + want: []map[string]string{ + {"foo": "bar"}, + }, + }, + { + name: "multiple entries", + input: `a=1, b=2; c=3, d=4`, + want: []map[string]string{ + {"a": "1"}, + {"b": "2", "c": "3"}, + {"d": "4"}, + }, + }, + { + name: "whitespace", + input: " a = 1,\tb\n=\r\n2,c=\" untrimmed \"", + want: []map[string]string{ + {"a": "1"}, + {"b": "2"}, + {"c": " untrimmed "}, + }, + }, + { + name: "unterminated quote", + input: `x="y`, + err: "unterminated quoted-string", + }, + { + name: "unexpected quote", + input: `"x=y"`, + err: "unexpected quote", + }, + { + name: "invalid token", + input: `a=b, ipv6=[fe80::1], x=y`, + err: "invalid token character at pos 10: [", + }, + } + + for _, test := range testCases { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + actual, err := parseForwardedHeader(test.input) + if test.err == "" { + require.NoError(t, err) + assert.EqualValues(t, test.want, actual) + } else { + require.Error(t, err) + assert.Contains(t, err.Error(), test.err) + } + }) + } +} diff --git a/challenge/http01/http_challenge_server.go b/challenge/http01/http_challenge_server.go index b8e8cc9c..947fe839 100644 --- a/challenge/http01/http_challenge_server.go +++ b/challenge/http01/http_challenge_server.go @@ -4,6 +4,7 @@ import ( "fmt" "net" "net/http" + "net/textproto" "strings" "github.com/go-acme/lego/v3/log" @@ -15,6 +16,7 @@ import ( type ProviderServer struct { iface string port string + matcher domainMatcher done chan bool listener net.Listener } @@ -23,15 +25,15 @@ type ProviderServer struct { // Setting iface and / or port to an empty string will make the server fall back to // the "any" interface and port 80 respectively. func NewProviderServer(iface, port string) *ProviderServer { - return &ProviderServer{iface: iface, port: port} + if port == "" { + port = "80" + } + + return &ProviderServer{iface: iface, port: port, matcher: &hostMatcher{}} } // Present starts a web server and makes the token available at `ChallengePath(token)` for web requests. func (s *ProviderServer) Present(domain, token, keyAuth string) error { - if s.port == "" { - s.port = "80" - } - var err error s.listener, err = net.Listen("tcp", s.GetAddress()) if err != nil { @@ -57,14 +59,38 @@ func (s *ProviderServer) CleanUp(domain, token, keyAuth string) error { return nil } +// SetProxyHeader changes the validation of incoming requests. +// By default, s matches the "Host" header value to the domain name. +// +// When the server runs behind a proxy server, this is not the correct place to look at; +// Apache and NGINX have traditionally moved the original Host header into a new header named "X-Forwarded-Host". +// Other webservers might use different names; +// and RFC7239 has standadized a new header named "Forwarded" (with slightly different semantics). +// +// The exact behavior depends on the value of headerName: +// - "" (the empty string) and "Host" will restore the default and only check the Host header +// - "Forwarded" will look for a Forwarded header, and inspect it according to https://tools.ietf.org/html/rfc7239 +// - any other value will check the header value with the same name +func (s *ProviderServer) SetProxyHeader(headerName string) { + switch h := textproto.CanonicalMIMEHeaderKey(headerName); h { + case "", "Host": + s.matcher = &hostMatcher{} + case "Forwarded": + s.matcher = &forwardedMatcher{} + default: + s.matcher = arbitraryMatcher(h) + } +} + func (s *ProviderServer) serve(domain, token, keyAuth string) { path := ChallengePath(token) - // The handler validates the HOST header and request type. - // For validation it then writes the token the server returned with the challenge + // The incoming request must will be validated to prevent DNS rebind attacks. + // We only respond with the keyAuth, when we're receiving a GET requests with + // the "Host" header matching the domain (the latter is configurable though SetProxyHeader). mux := http.NewServeMux() mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { - if strings.HasPrefix(r.Host, domain) && r.Method == http.MethodGet { + if r.Method == http.MethodGet && s.matcher.matches(r, domain) { w.Header().Add("Content-Type", "text/plain") _, err := w.Write([]byte(keyAuth)) if err != nil { @@ -73,7 +99,7 @@ func (s *ProviderServer) serve(domain, token, keyAuth string) { } log.Infof("[%s] Served key authentication", domain) } else { - log.Warnf("Received request for domain %s with method %s but the domain did not match any challenge. Please ensure your are passing the HOST header properly.", r.Host, r.Method) + log.Warnf("Received request for domain %s with method %s but the domain did not match any challenge. Please ensure your are passing the %s header properly.", r.Host, r.Method, s.matcher.name()) _, err := w.Write([]byte("TEST")) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) diff --git a/challenge/http01/http_challenge_test.go b/challenge/http01/http_challenge_test.go index c8c2fe58..38e1bd7f 100644 --- a/challenge/http01/http_challenge_test.go +++ b/challenge/http01/http_challenge_test.go @@ -3,8 +3,10 @@ package http01 import ( "crypto/rand" "crypto/rsa" + "fmt" "io/ioutil" "net/http" + "net/textproto" "testing" "github.com/go-acme/lego/v3/acme" @@ -19,7 +21,7 @@ func TestChallenge(t *testing.T) { _, apiURL, tearDown := tester.SetupFakeAPI() defer tearDown() - providerServer := &ProviderServer{port: "23457"} + providerServer := NewProviderServer("", "23457") validate := func(_ *api.Core, _ string, chlng acme.Challenge) error { uri := "http://localhost" + providerServer.GetAddress() + ChallengePath(chlng.Token) @@ -80,7 +82,7 @@ func TestChallengeInvalidPort(t *testing.T) { validate := func(_ *api.Core, _ string, _ acme.Challenge) error { return nil } - solver := NewChallenge(core, validate, &ProviderServer{port: "123456"}) + solver := NewChallenge(core, validate, NewProviderServer("", "123456")) authz := acme.Authorization{ Identifier: acme.Identifier{ @@ -96,3 +98,225 @@ func TestChallengeInvalidPort(t *testing.T) { assert.Contains(t, err.Error(), "invalid port") assert.Contains(t, err.Error(), "123456") } + +type testProxyHeader struct { + name string + values []string +} + +func (h *testProxyHeader) update(r *http.Request) { + if h == nil || len(h.values) == 0 { + return + } + if h.name == "Host" { + r.Host = h.values[0] + } else if h.name != "" { + r.Header[h.name] = h.values + } +} + +func TestChallengeWithProxy(t *testing.T) { + h := func(name string, values ...string) *testProxyHeader { + name = textproto.CanonicalMIMEHeaderKey(name) + return &testProxyHeader{name, values} + } + + const ( + ok = "localhost:23457" + nook = "example.com" + ) + + var testCases = []struct { + name string + header *testProxyHeader + extra *testProxyHeader + isErr bool + }{ + // tests for hostMatcher + { + name: "no proxy", + }, + { + name: "empty string", + header: h(""), + }, + { + name: "empty Host", + header: h("host"), + }, + { + name: "matching Host", + header: h("host", ok), + }, + { + name: "Host mismatch", + header: h("host", nook), + isErr: true, + }, + { + name: "Host mismatch (ignoring forwarding header)", + header: h("host", nook), + extra: h("X-Forwarded-Host", ok), + isErr: true, + }, + // test for arbitraryMatcher + { + name: "matching X-Forwarded-Host", + header: h("X-Forwarded-Host", ok), + }, + { + name: "matching X-Forwarded-Host (multiple fields)", + header: h("X-Forwarded-Host", ok, nook), + }, + { + name: "matching X-Forwarded-Host (chain value)", + header: h("X-Forwarded-Host", ok+", "+nook), + }, + { + name: "X-Forwarded-Host mismatch", + header: h("X-Forwarded-Host", nook), + extra: h("host", ok), + isErr: true, + }, + { + name: "X-Forwarded-Host mismatch (multiple fields)", + header: h("X-Forwarded-Host", nook, ok), + isErr: true, + }, + { + name: "matching X-Something-Else", + header: h("X-Something-Else", ok), + }, + { + name: "matching X-Something-Else (multiple fields)", + header: h("X-Something-Else", ok, nook), + }, + { + name: "matching X-Something-Else (chain value)", + header: h("X-Something-Else", ok+", "+nook), + }, + { + name: "X-Something-Else mismatch", + header: h("X-Something-Else", nook), + isErr: true, + }, + { + name: "X-Something-Else mismatch (multiple fields)", + header: h("X-Something-Else", nook, ok), + isErr: true, + }, + { + name: "X-Something-Else mismatch (chain value)", + header: h("X-Something-Else", nook+", "+ok), + isErr: true, + }, + // tests for forwardedHeader + { + name: "matching Forwarded", + header: h("Forwarded", fmt.Sprintf("host=%q;foo=bar", ok)), + }, + { + name: "matching Forwarded (multiple fields)", + header: h("Forwarded", fmt.Sprintf("host=%q", ok), "host="+nook), + }, + { + name: "matching Forwarded (chain value)", + header: h("Forwarded", fmt.Sprintf("host=%q, host=%s", ok, nook)), + }, + { + name: "Forwarded mismatch", + header: h("Forwarded", "host="+nook), + isErr: true, + }, + { + name: "Forwarded mismatch (missing information)", + header: h("Forwarded", "for=127.0.0.1"), + isErr: true, + }, + { + name: "Forwarded mismatch (multiple fields)", + header: h("Forwarded", "host="+nook, fmt.Sprintf("host=%q", ok)), + isErr: true, + }, + { + name: "Forwarded mismatch (chain value)", + header: h("Forwarded", fmt.Sprintf("host=%s, host=%q", nook, ok)), + isErr: true, + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + testServeWithProxy(t, test.header, test.extra, test.isErr) + }) + } +} + +func testServeWithProxy(t *testing.T, header, extra *testProxyHeader, expectError bool) { + t.Helper() + + _, apiURL, tearDown := tester.SetupFakeAPI() + defer tearDown() + + providerServer := NewProviderServer("localhost", "23457") + if header != nil { + providerServer.SetProxyHeader(header.name) + } + + validate := func(_ *api.Core, _ string, chlng acme.Challenge) error { + uri := "http://" + providerServer.GetAddress() + ChallengePath(chlng.Token) + + req, err := http.NewRequest(http.MethodGet, uri, nil) + if err != nil { + return err + } + header.update(req) + extra.update(req) + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return err + } + defer resp.Body.Close() + + if want := "text/plain"; resp.Header.Get("Content-Type") != want { + return fmt.Errorf("Get(%q) Content-Type: got %q, want %q", uri, resp.Header.Get("Content-Type"), want) + } + + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return err + } + bodyStr := string(body) + + if bodyStr != chlng.KeyAuthorization { + return fmt.Errorf("Get(%q) Body: got %q, want %q", uri, bodyStr, chlng.KeyAuthorization) + } + + return nil + } + + privateKey, err := rsa.GenerateKey(rand.Reader, 512) + require.NoError(t, err, "Could not generate test key") + + core, err := api.New(http.DefaultClient, "lego-test", apiURL+"/dir", "", privateKey) + require.NoError(t, err) + + solver := NewChallenge(core, validate, providerServer) + + authz := acme.Authorization{ + Identifier: acme.Identifier{ + Value: "localhost:23457", + }, + Challenges: []acme.Challenge{ + {Type: challenge.HTTP01.String(), Token: "http1"}, + }, + } + + err = solver.Solve(authz) + if expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + } +} diff --git a/cmd/flags.go b/cmd/flags.go index dea7a40d..cc551f63 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -63,6 +63,11 @@ func CreateFlags(defaultPath string) []cli.Flag { Usage: "Set the port and interface to use for HTTP based challenges to listen on.Supported: interface:port or :port.", Value: ":80", }, + cli.StringFlag{ + Name: "http.proxy-header", + Usage: "Validate against this HTTP header when solving HTTP based challenges behind a reverse proxy.", + Value: "Host", + }, cli.StringFlag{ Name: "http.webroot", Usage: "Set the webroot folder to use for HTTP based challenges to write directly in a file in .well-known/acme-challenge.", diff --git a/cmd/setup_challenges.go b/cmd/setup_challenges.go index 27912efa..5f93b520 100644 --- a/cmd/setup_challenges.go +++ b/cmd/setup_challenges.go @@ -66,9 +66,17 @@ func setupHTTPProvider(ctx *cli.Context) challenge.Provider { log.Fatal(err) } - return http01.NewProviderServer(host, port) + srv := http01.NewProviderServer(host, port) + if header := ctx.GlobalString("http.proxy-header"); header != "" { + srv.SetProxyHeader(header) + } + return srv case ctx.GlobalBool("http"): - return http01.NewProviderServer("", "") + srv := http01.NewProviderServer("", "") + if header := ctx.GlobalString("http.proxy-header"); header != "" { + srv.SetProxyHeader(header) + } + return srv default: log.Fatal("Invalid HTTP challenge options.") return nil diff --git a/docs/content/usage/cli/_index.md b/docs/content/usage/cli/_index.md index 11174d6a..7e662722 100644 --- a/docs/content/usage/cli/_index.md +++ b/docs/content/usage/cli/_index.md @@ -19,12 +19,12 @@ USAGE: lego [global options] command [command options] [arguments...] COMMANDS: - run Register an account, then create and install a certificate - revoke Revoke a certificate - renew Renew a certificate - dnshelp Shows additional help for the '--dns' global option - list Display certificates and accounts information. - help, h Shows a list of commands or help for one command + run Register an account, then create and install a certificate + revoke Revoke a certificate + renew Renew a certificate + dnshelp Shows additional help for the '--dns' global option + list Display certificates and accounts information. + help, h Shows a list of commands or help for one command GLOBAL OPTIONS: --domains value, -d value Add a domain to the process. Can be specified multiple times. @@ -40,6 +40,7 @@ GLOBAL OPTIONS: --path value Directory to use for storing the data. (default: "./.lego") --http Use the HTTP challenge to solve challenges. Can be mixed with other types of challenges. --http.port value Set the port and interface to use for HTTP based challenges to listen on.Supported: interface:port or :port. (default: ":80") + --http.proxy-header value Validate against this HTTP header when solving HTTP based challenges behind a reverse proxy. (default: "Host") --http.webroot value Set the webroot folder to use for HTTP based challenges to write directly in a file in .well-known/acme-challenge. --http.memcached-host value Set the memcached host(s) to use for HTTP based challenges. Challenges will be written to all specified hosts. --tls Use the TLS challenge to solve challenges. Can be mixed with other types of challenges. @@ -87,8 +88,10 @@ lego to listen on that interface:port for any incoming challenges. If you are using this option, make sure you proxy all of the following traffic to these ports. -**HTTP Port:** All plaintext HTTP requests to port **80** which begin with a request path of `/.well-known/acme-challenge/` for the HTTP challenge. +**HTTP Port:** All plaintext HTTP requests to port **80** which begin with a request path of `/.well-known/acme-challenge/` for the HTTP challenge.[^header] **TLS Port:** All TLS handshakes on port **443** for the TLS-ALPN challenge. This traffic redirection is only needed as long as lego solves challenges. As soon as you have received your certificates you can deactivate the forwarding. + +[^header]: You must ensure that incoming validation requests containt the correct value for the HTTP `Host` header. If you operate lego behind a non-transparent reverse proxy (such as Apache or NGINX), you might need to alter the header field using `--http.proxy-header X-Forwarded-Host`.