amadan
Require HTTPS before sibling requests can dial
3 files changed,
+40
−3
transport.go+3 −0transport_scheme_test.go+25 −0transport_test.go+12 −3
CI passed — run details
CI log
=== gate === ok amadan.net/oficina/suite 0.009s ok amadan.net/oficina/suite/conformance 0.005s
diff --git a/transport.go b/transport.go| index 14e0d5d..034110a 100644 |
| --- a/transport.go |
| +++ b/transport.go |
| @@ -46,6 +46,9 @@ type teamBoundTransport struct { |
| } |
| func (t *teamBoundTransport) RoundTrip(req *http.Request) (*http.Response, error) { |
| + if req.URL.Scheme != "https" { |
| + return nil, ErrContractBreach |
| + } |
| // Before the transport dials, so a refused host costs no connection. |
| urlHost := req.URL.Hostname() |
| if err := CheckSiblingHost(t.team, urlHost); err != nil { |
diff --git a/transport_scheme_test.go b/transport_scheme_test.go| new file mode 100644 |
| index 0000000..5f2cb95 |
| --- /dev/null |
| +++ b/transport_scheme_test.go |
| @@ -0,0 +1,25 @@ |
| +package suite |
| + |
| +import ( |
| + "errors" |
| + "net/http" |
| + "strings" |
| + "testing" |
| +) |
| + |
| +func TestSiblingClientRefusesPlaintextBeforeSendingCredentials(t *testing.T) { |
| + d := &recordingDialer{} |
| + c := NewSiblingClient("acme", d.dial) |
| + req, err := http.NewRequest("POST", "http://acme.docs.oficina.works/suite/s2", strings.NewReader(`{"item_id":"fixture"}`)) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + req.Header.Set("Authorization", "Bearer synthetic-fixture") |
| + _, err = c.Do(req) |
| + if !errors.Is(err, ErrContractBreach) { |
| + t.Errorf("plain HTTP returned %v; want contract refusal", err) |
| + } |
| + if len(d.dialed) != 0 { |
| + t.Errorf("plaintext credential request attempted network connection: %v", d.dialed) |
| + } |
| +} |
diff --git a/transport_test.go b/transport_test.go| index 78ee520..b8b0b12 100644 |
| --- a/transport_test.go |
| +++ b/transport_test.go |
| @@ -2,6 +2,8 @@ package suite |
| import ( |
| "context" |
| + "crypto/tls" |
| + "crypto/x509" |
| "net" |
| "net/http" |
| "net/http/httptest" |
| @@ -123,11 +125,11 @@ func TestSiblingClientRefusesAFixtureRedirect(t *testing.T) { |
| f := loadHostileHosts(t) |
| // A server that answers every request with a redirect to another team. |
| - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| http.Redirect(w, r, "http://"+f.Redirect.To+"/suite/s3/doc-1", http.StatusFound) |
| })) |
| defer srv.Close() |
| - srvAddr := strings.TrimPrefix(srv.URL, "http://") |
| + srvAddr := strings.TrimPrefix(srv.URL, "https://") |
| // Every name resolves to the test server, so the only thing stopping a |
| // second hop is the client's own refusal — not the absence of a host. |
| @@ -138,13 +140,20 @@ func TestSiblingClientRefusesAFixtureRedirect(t *testing.T) { |
| } |
| c := NewSiblingClient(f.Team, dial) |
| - resp, err := c.Get("http://" + f.Redirect.From + "/suite/s3/doc-1") |
| + roots := x509.NewCertPool() |
| + roots.AddCert(srv.Certificate()) |
| + c.Transport.(*teamBoundTransport).next.(*http.Transport).TLSClientConfig = &tls.Config{ |
| + RootCAs: roots, ServerName: srv.Certificate().DNSNames[0], |
| + } |
| + resp, err := c.Get("https://" + f.Redirect.From + "/suite/s3/doc-1") |
| if resp != nil { |
| resp.Body.Close() |
| } |
| if err == nil { |
| t.Error("got nil error following a redirect to another team, want a refusal") |
| + } else if !strings.Contains(err.Error(), "refusing to follow a redirect") { |
| + t.Errorf("request failed before reaching the redirect: %v", err) |
| } |
| if len(dialed) != 1 { |
| t.Fatalf("dialled %v, want exactly one hop — the allowed host, and no follow-up", dialed) |