From 2fc65c393924ea5074e7b1bdad98b0fe61e21374 Mon Sep 17 00:00:00 2001 From: Patrick O'Doherty Date: Fri, 8 Dec 2023 09:27:34 -0800 Subject: [PATCH 01/10] golink: listen on HTTPS and redirect HTTP traffic Updates tailscale/golink#9 On tailnets with TLS enabled serve HTTP traffic with a separate redirectHandler which sends requests to our HTTPS listener destination. Add `-L` to documented examples of using `curl` to follow these redirects if present. Signed-off-by: Patrick O'Doherty --- golink.go | 83 ++++++++++++++++++++++++++++++++++++++++++++++---- tmpl/help.html | 4 +-- 2 files changed, 79 insertions(+), 8 deletions(-) diff --git a/golink.go b/golink.go index 560b232..4da8066 100644 --- a/golink.go +++ b/golink.go @@ -9,6 +9,7 @@ import ( "bytes" "context" "crypto/rand" + "crypto/tls" "embed" "encoding/base64" "encoding/json" @@ -169,18 +170,78 @@ func Run() error { if err := srv.Start(); err != nil { return err } - localClient, _ = srv.LocalClient() - l80, err := srv.Listen("tcp", ":80") + // create tsNet server and wait for it to be ready & connected. + localClient, _ = srv.LocalClient() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + _, err = srv.Up(ctx) if err != nil { return err } - log.Printf("Serving http://%s/ ...", *hostname) - if err := http.Serve(l80, serveHandler()); err != nil { - return err + enableTLS := len(srv.CertDomains()) > 0 + if enableTLS { + // warm the certificate cache for all cert domains to prevent users waiting + // on ACME challenges in-line on their first request. + for _, d := range srv.CertDomains() { + log.Printf("Provisioning TLS certificate for %s ...", d) + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + + _, _, err := localClient.CertPair(ctx, d) + if err != nil { + return err + } + } + + redirectFqdn := srv.CertDomains()[0] + // HTTP listener that redirects to our HTTPS listener. + log.Println("Listening on :80") + httpListener, err := srv.Listen("tcp", ":80") + if err != nil { + return err + } + go func() error { + log.Printf("Serving http://%s/ ...", *hostname) + if err := http.Serve(httpListener, redirectHandler(redirectFqdn)); err != nil { + return err + } + return nil + }() + + log.Println("Listening on :443") + httpsListener, err := srv.Listen("tcp", ":443") + if err != nil { + return err + } + s := http.Server{ + Addr: ":443", + Handler: serveHandler(), + TLSConfig: &tls.Config{ + GetCertificate: localClient.GetCertificate, + }, + } + + log.Printf("Serving https://%s/\n", redirectFqdn) + if err := s.ServeTLS(httpsListener, "", ""); err != nil { + return err + } + return nil + } else { + // no TLS, just serve on :80 + log.Println("Listening on :80") + httpListener, err := srv.Listen("tcp", ":80") + if err != nil { + return err + } + log.Printf("Serving http://%s/ ...", *hostname) + if err := http.Serve(httpListener, serveHandler()); err != nil { + return err + } + return nil } - return nil + } var ( @@ -286,6 +347,16 @@ func deleteLinkStats(link *Link) { db.DeleteStats(link.Short) } +// redirectHandler returns the http.Handler for serving all plaintext HTTP +// requests. It redirects all requests to the HTTPs version of the same URL. +func redirectHandler(hostname string) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + newUrl := fmt.Sprintf("https://%s%s", hostname, path) + http.Redirect(w, r, newUrl, http.StatusMovedPermanently) + }) +} + // serverHandler returns the main http.Handler for serving all requests. func serveHandler() http.Handler { mux := http.NewServeMux() diff --git a/tmpl/help.html b/tmpl/help.html index 0871803..b64eb36 100644 --- a/tmpl/help.html +++ b/tmpl/help.html @@ -114,7 +114,7 @@

Application Programming Interface (API)

Visit go/.export to export all saved links and their metadata in JSON Lines format. This is useful to create data snapshots that can be restored later. -
{{`$ curl go/.export
+
{{`$ curl -L go/.export
 {"Short":"go","Long":"http://go","Created":"2022-05-31T13:04:44.741457796-07:00","LastEdit":"2022-05-31T13:04:44.741457796-07:00","Owner":"amelie@example.com","Clicks":1}
 {"Short":"slack","Long":"https://company.slack.com/{{if .Path}}channels/{{PathEscape .Path}}{{end}}","Created":"2022-06-17T18:05:43.562948451Z","LastEdit":"2022-06-17T18:06:35.811398Z","Owner":"amelie@example.com","Clicks":4}`}}
 
@@ -122,7 +122,7 @@

Application Programming Interface (API)

Create a new link by sending a POST request with a short and long value: -

{{`$ curl -d short=cs -d long=https://cs.github.com/ go
+
{{`$ curl -L -d short=cs -d long=https://cs.github.com/ go
 {"Short":"cs","Long":"https://cs.github.com/","Created":"2022-06-03T22:15:29.993978392Z","LastEdit":"2022-06-03T22:15:29.993978392Z","Owner":"amelie@example.com"}`}}
 
From 23f9f9642e179a724301c49a5eabfe9c824d114a Mon Sep 17 00:00:00 2001 From: Patrick O'Doherty Date: Fri, 8 Dec 2023 15:48:00 -0800 Subject: [PATCH 02/10] golink: set Strict-Transport-Security header on HTTPS responses Signed-off-by: Patrick O'Doherty --- golink.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/golink.go b/golink.go index 4da8066..4e4f314 100644 --- a/golink.go +++ b/golink.go @@ -217,7 +217,7 @@ func Run() error { } s := http.Server{ Addr: ":443", - Handler: serveHandler(), + Handler: HSTS(serveHandler()), TLSConfig: &tls.Config{ GetCertificate: localClient.GetCertificate, }, @@ -357,6 +357,15 @@ func redirectHandler(hostname string) http.Handler { }) } +// HSTS wraps the provided handler and sets Strict-Transport-Security header on +// all responses. +func HSTS(h http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Strict-Transport-Security", "max-age=31536000") + h.ServeHTTP(w, r) + }) +} + // serverHandler returns the main http.Handler for serving all requests. func serveHandler() http.Handler { mux := http.NewServeMux() From 5722cf28955abf374de4bd5946dc39c50152c0d4 Mon Sep 17 00:00:00 2001 From: Patrick O'Doherty Date: Mon, 11 Dec 2023 09:19:12 -0800 Subject: [PATCH 03/10] golink: address PR feedback * use `srv.ListenTLS` API instead of DIY'ing it. * DRY up http & https listener code. * use type safe URL generation for redirect handler * use status API to determine HTTPS capabilities directly. * handle http only case gracefully. Signed-off-by: Patrick O'Doherty --- golink.go | 96 ++++++++++++++++++++++++------------------------------- 1 file changed, 41 insertions(+), 55 deletions(-) diff --git a/golink.go b/golink.go index 4e4f314..792fc98 100644 --- a/golink.go +++ b/golink.go @@ -9,7 +9,6 @@ import ( "bytes" "context" "crypto/rand" - "crypto/tls" "embed" "encoding/base64" "encoding/json" @@ -159,6 +158,7 @@ func Run() error { return errors.New("--hostname, if specified, cannot be empty") } + // create tsNet server and wait for it to be ready & connected. srv := &tsnet.Server{ ControlURL: *controlURL, Hostname: *hostname, @@ -171,77 +171,65 @@ func Run() error { return err } - // create tsNet server and wait for it to be ready & connected. localClient, _ = srv.LocalClient() +out: + for { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + status, err := srv.Up(ctx) + if err == nil && status != nil { + break out + } + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - _, err = srv.Up(ctx) + status, err := localClient.Status(ctx) if err != nil { return err } + enableTLS := status.Self.HasCap(tailcfg.CapabilityHTTPS) + dnsName := status.Self.DNSName - enableTLS := len(srv.CertDomains()) > 0 + var httpHandler http.Handler + var httpsHandler http.Handler if enableTLS { - // warm the certificate cache for all cert domains to prevent users waiting - // on ACME challenges in-line on their first request. - for _, d := range srv.CertDomains() { - log.Printf("Provisioning TLS certificate for %s ...", d) - ctx, cancel := context.WithTimeout(context.Background(), time.Minute) - defer cancel() - - _, _, err := localClient.CertPair(ctx, d) - if err != nil { - return err - } - } + redirectFqdn := strings.TrimSuffix(dnsName, ".") + httpHandler = redirectHandler(redirectFqdn) + httpsHandler = HSTS(serveHandler()) + } else { + httpHandler = serveHandler() + httpsHandler = nil + } - redirectFqdn := srv.CertDomains()[0] - // HTTP listener that redirects to our HTTPS listener. - log.Println("Listening on :80") - httpListener, err := srv.Listen("tcp", ":80") + if httpsHandler != nil { + log.Println("Listening on :443") + httpsListener, err := srv.ListenTLS("tcp", ":443") if err != nil { return err } go func() error { - log.Printf("Serving http://%s/ ...", *hostname) - if err := http.Serve(httpListener, redirectHandler(redirectFqdn)); err != nil { + log.Printf("Serving https://%s/ ...", strings.TrimSuffix(dnsName, ".")) + if err := http.Serve(httpsListener, httpsHandler); err != nil { return err } return nil }() + } - log.Println("Listening on :443") - httpsListener, err := srv.Listen("tcp", ":443") - if err != nil { - return err - } - s := http.Server{ - Addr: ":443", - Handler: HSTS(serveHandler()), - TLSConfig: &tls.Config{ - GetCertificate: localClient.GetCertificate, - }, - } - - log.Printf("Serving https://%s/\n", redirectFqdn) - if err := s.ServeTLS(httpsListener, "", ""); err != nil { - return err - } - return nil - } else { - // no TLS, just serve on :80 - log.Println("Listening on :80") - httpListener, err := srv.Listen("tcp", ":80") - if err != nil { - return err - } - log.Printf("Serving http://%s/ ...", *hostname) - if err := http.Serve(httpListener, serveHandler()); err != nil { - return err - } - return nil + // HTTP handler that either serves primary handler or redirects to HTTPS + // depending on availability of TLS. + log.Println("Listening on :80") + httpListener, err := srv.Listen("tcp", ":80") + if err != nil { + return err + } + log.Printf("Serving http://%s/ ...", *hostname) + if err := http.Serve(httpListener, httpHandler); err != nil { + return err } + return nil } var ( @@ -351,9 +339,7 @@ func deleteLinkStats(link *Link) { // requests. It redirects all requests to the HTTPs version of the same URL. func redirectHandler(hostname string) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - path := r.URL.Path - newUrl := fmt.Sprintf("https://%s%s", hostname, path) - http.Redirect(w, r, newUrl, http.StatusMovedPermanently) + http.Redirect(w, r, (&url.URL{Scheme: "https", Host: hostname, Path: r.URL.Path}).String(), http.StatusMovedPermanently) }) } From 5e7093544bb8ca02d5c74e4b899ae3d891cd1294 Mon Sep 17 00:00:00 2001 From: Patrick O'Doherty Date: Mon, 11 Dec 2023 16:37:44 -0800 Subject: [PATCH 04/10] golink: PR feedback round two * create discrete ctx variables for localClient & tsnet server interactions. * DRY up the http(s) handler code even further. * call log.Fatal for https serving errors that were previously swallowed. Signed-off-by: Patrick O'Doherty --- golink.go | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/golink.go b/golink.go index 792fc98..df4d075 100644 --- a/golink.go +++ b/golink.go @@ -174,53 +174,43 @@ func Run() error { localClient, _ = srv.LocalClient() out: for { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + upCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - status, err := srv.Up(ctx) + status, err := srv.Up(upCtx) if err == nil && status != nil { break out } } - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + statusCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - status, err := localClient.Status(ctx) + status, err := localClient.Status(statusCtx) if err != nil { return err } enableTLS := status.Self.HasCap(tailcfg.CapabilityHTTPS) - dnsName := status.Self.DNSName + fqdn := strings.TrimSuffix(status.Self.DNSName, ".") - var httpHandler http.Handler - var httpsHandler http.Handler + httpHandler := serveHandler() if enableTLS { - redirectFqdn := strings.TrimSuffix(dnsName, ".") - httpHandler = redirectHandler(redirectFqdn) - httpsHandler = HSTS(serveHandler()) - } else { - httpHandler = serveHandler() - httpsHandler = nil - } + httpsHandler := HSTS(httpHandler) + httpHandler = redirectHandler(fqdn) - if httpsHandler != nil { - log.Println("Listening on :443") httpsListener, err := srv.ListenTLS("tcp", ":443") if err != nil { return err } - go func() error { - log.Printf("Serving https://%s/ ...", strings.TrimSuffix(dnsName, ".")) + log.Println("Listening on :443") + go func() { + log.Printf("Serving https://%s/ ...", fqdn) if err := http.Serve(httpsListener, httpsHandler); err != nil { - return err + log.Fatal(err) } - return nil }() } - // HTTP handler that either serves primary handler or redirects to HTTPS - // depending on availability of TLS. - log.Println("Listening on :80") httpListener, err := srv.Listen("tcp", ":80") + log.Println("Listening on :80") if err != nil { return err } From 7917da1dfae72e183ab2e78d5ce10df9c834e26d Mon Sep 17 00:00:00 2001 From: Patrick O'Doherty Date: Thu, 14 Dec 2023 13:16:24 -0800 Subject: [PATCH 05/10] golink: use http.StatusFound for redirects If users belong to multiple tailnets with golinks deployed (as is common) then permanent redirects for one will conflict with the others. To prevent this we will use `http.StatusFound` to prompt browsers to continue to visit the `go/` URL in the future. Signed-off-by: Patrick O'Doherty --- golink.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/golink.go b/golink.go index df4d075..d576141 100644 --- a/golink.go +++ b/golink.go @@ -329,7 +329,7 @@ func deleteLinkStats(link *Link) { // requests. It redirects all requests to the HTTPs version of the same URL. func redirectHandler(hostname string) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - http.Redirect(w, r, (&url.URL{Scheme: "https", Host: hostname, Path: r.URL.Path}).String(), http.StatusMovedPermanently) + http.Redirect(w, r, (&url.URL{Scheme: "https", Host: hostname, Path: r.URL.Path}).String(), http.StatusFound) }) } From 8ac87f5b776a844cda14c1a24c39828427db274e Mon Sep 17 00:00:00 2001 From: Patrick O'Doherty Date: Fri, 15 Dec 2023 23:45:41 +0000 Subject: [PATCH 06/10] golink: do not set HSTS headers when serving non-FQDN origins Inspect the `Host` header to ensure that we do not return HSTS headers for short domains which can lead to some clients pinning short domains to endpoints with invalid certificates. Signed-off-by: Patrick O'Doherty --- golink.go | 11 +++++++++-- golink_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/golink.go b/golink.go index d576141..7e8e0b1 100644 --- a/golink.go +++ b/golink.go @@ -334,10 +334,17 @@ func redirectHandler(hostname string) http.Handler { } // HSTS wraps the provided handler and sets Strict-Transport-Security header on -// all responses. +// responses. It inspects the Host header to ensure we do not specify HSTS +// response on non fully qualified domain name origins. func HSTS(h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Strict-Transport-Security", "max-age=31536000") + host, found := r.Header["Host"] + if found { + host := host[0] + if strings.Contains(host, ".") { + w.Header().Set("Strict-Transport-Security", "max-age=31536000") + } + } h.ServeHTTP(w, r) }) } diff --git a/golink_test.go b/golink_test.go index 9506add..2cd29f8 100644 --- a/golink_test.go +++ b/golink_test.go @@ -524,3 +524,41 @@ func TestResolveLink(t *testing.T) { }) } } + +func TestNoHSTSShortDomain(t *testing.T) { + var err error + db, err = NewSQLiteDB(":memory:") + if err != nil { + t.Fatal(err) + } + db.Save(&Link{Short: "foobar", Long: "http://foobar/"}) + + tests := []struct { + host string + expectHsts bool + }{ + { + host: "go", + expectHsts: false, + }, + { + host: "go.prawn-universe.ts.net", + expectHsts: true, + }, + } + for _, tt := range tests { + name := "HSTS: " + tt.host + t.Run(name, func(t *testing.T) { + r := httptest.NewRequest("GET", "/foobar", nil) + r.Header.Add("Host", tt.host) + + w := httptest.NewRecorder() + HSTS(serveHandler()).ServeHTTP(w, r) + + _, found := w.Header()["Strict-Transport-Security"] + if found != tt.expectHsts { + t.Errorf("HSTS expectation: domain %s want: %t got: %t", tt.host, tt.expectHsts, found) + } + }) + } +} From 39b1540ad1ca3e1e2b7efb9e04a69bdbe081de67 Mon Sep 17 00:00:00 2001 From: Patrick O'Doherty Date: Fri, 15 Dec 2023 17:22:12 -0800 Subject: [PATCH 07/10] golink: add README section about HTTPS behavior Append section about HTTPS behavior to the README. Include a note about the use of `-L` with all `curl` scripts in such deployments to prevent silent early termination with empty responses. Signed-off-by: Patrick O'Doherty --- README.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/README.md b/README.md index 1a10f1d..8bdc77a 100644 --- a/README.md +++ b/README.md @@ -146,3 +146,16 @@ If you're using Firefox, you might want to configure two options to make it easy with a value of _true_ * if you use HTTPS-Only Mode, [add an exception](https://support.mozilla.org/en-US/kb/https-only-prefs#w_add-exceptions-for-http-websites-when-youre-in-https-only-mode) + +## HTTPS + +When golink joins your tailnet it will check to see if HTTPS is enabled and +begin serving HTTPS traffic if the answer is yes. If HTTPs is enabled golink +will redirect all requests received by the HTTP endpoint first to their internal +HTTPS equivalent before redirecting to the external link destination. + +**NB:** If you use `curl` to interact with the API of a golink instance with HTTPS +enabled over its HTTP interface you _must_ specify the `-L` flag to follow these +redirects or else your request will terminate early with an empty response. We +recommend the use of the `-L` flag in all deployments regardless of current +HTTPS status to avoid accidental outages should it be enabled in the future. From 7e903c17b1b0d78e61c1922752e31a088aa90ed8 Mon Sep 17 00:00:00 2001 From: Patrick O'Doherty Date: Fri, 15 Dec 2023 17:51:23 -0800 Subject: [PATCH 08/10] golink: use dnsname.FQDN to validate host header Signed-off-by: Patrick O'Doherty --- golink.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/golink.go b/golink.go index 7e8e0b1..0411eb9 100644 --- a/golink.go +++ b/golink.go @@ -36,6 +36,7 @@ import ( "tailscale.com/ipn" "tailscale.com/tailcfg" "tailscale.com/tsnet" + "tailscale.com/util/dnsname" ) const defaultHostname = "go" @@ -341,8 +342,12 @@ func HSTS(h http.Handler) http.Handler { host, found := r.Header["Host"] if found { host := host[0] - if strings.Contains(host, ".") { - w.Header().Set("Strict-Transport-Security", "max-age=31536000") + fqdn, err := dnsname.ToFQDN(host) + if err == nil { + segCount := fqdn.NumLabels() + if segCount > 1 { + w.Header().Set("Strict-Transport-Security", "max-age=31536000") + } } } h.ServeHTTP(w, r) From 9cabf69f0669f97900570b6dc8545335ade74db0 Mon Sep 17 00:00:00 2001 From: Patrick O'Doherty Date: Mon, 18 Dec 2023 10:32:05 -0800 Subject: [PATCH 09/10] golink: fixup README copy on HTTPS behavior Signed-off-by: Patrick O'Doherty --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8bdc77a..e358e8f 100644 --- a/README.md +++ b/README.md @@ -150,7 +150,7 @@ If you're using Firefox, you might want to configure two options to make it easy ## HTTPS When golink joins your tailnet it will check to see if HTTPS is enabled and -begin serving HTTPS traffic if the answer is yes. If HTTPs is enabled golink +begin serving HTTPS traffic it detects that it is. When HTTPs is enabled golink will redirect all requests received by the HTTP endpoint first to their internal HTTPS equivalent before redirecting to the external link destination. From a1ea1daa91e8599ce62e82775359a2b66055396c Mon Sep 17 00:00:00 2001 From: Patrick O'Doherty Date: Mon, 18 Dec 2023 10:40:28 -0800 Subject: [PATCH 10/10] golink: consistency for HTTPS acronym usage Signed-off-by: Patrick O'Doherty --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e358e8f..775b890 100644 --- a/README.md +++ b/README.md @@ -150,7 +150,7 @@ If you're using Firefox, you might want to configure two options to make it easy ## HTTPS When golink joins your tailnet it will check to see if HTTPS is enabled and -begin serving HTTPS traffic it detects that it is. When HTTPs is enabled golink +begin serving HTTPS traffic it detects that it is. When HTTPS is enabled golink will redirect all requests received by the HTTP endpoint first to their internal HTTPS equivalent before redirecting to the external link destination.