diff --git a/internal/loglist/loglist.go b/internal/loglist/loglist.go index 6f030ed..d2a6c95 100644 --- a/internal/loglist/loglist.go +++ b/internal/loglist/loglist.go @@ -86,7 +86,14 @@ func (l *Log) Endpoint() string { func FetchAll(ctx context.Context, client *http.Client) ([]Log, error) { if client == nil { - client = &http.Client{Timeout: 30 * time.Second} + client = &http.Client{ + Timeout: 30 * time.Second, + // The list sources are fixed https URLs; never follow a + // redirect so a hijacked source cannot redirect us elsewhere. + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + } } urls := []string{ChromeList, ChromeAllList, AppleList} seen := make(map[string]*Log) diff --git a/internal/rfc6962/client.go b/internal/rfc6962/client.go index ae4ee09..88a7ece 100644 --- a/internal/rfc6962/client.go +++ b/internal/rfc6962/client.go @@ -12,7 +12,9 @@ import ( "encoding/json" "fmt" "io" + "net" "net/http" + "net/url" "strings" "time" ) @@ -36,12 +38,32 @@ type Client struct { } func NewClient(baseURL, keyB64 string) (*Client, error) { - // Log list URLs are joined with "/ct/v1/..." paths below; a trailing - // slash would produce "//ct/v1/..." and every request would 404. - c := &Client{BaseURL: strings.TrimRight(baseURL, "/"), HTTP: &http.Client{Timeout: 30 * time.Second}} if keyB64 == "" { return nil, fmt.Errorf("missing log key") } + // Log list URLs are joined with "/ct/v1/..." paths below; a trailing + // slash would produce "//ct/v1/..." and every request would 404. + baseURL = strings.TrimRight(baseURL, "/") + u, err := url.Parse(baseURL) + if err != nil { + return nil, fmt.Errorf("bad log url: %w", err) + } + // Endpoints come from externally fetched log lists, so treat them as + // untrusted input: plain http is only tolerated on loopback (local + // test servers), and redirects are never followed — a hijacked list + // source must not be able to aim the tailer at internal hosts. + if u.Scheme != "https" && !isLoopbackHost(u.Hostname()) { + return nil, fmt.Errorf("log url must be https, got %q", u.Scheme) + } + c := &Client{ + BaseURL: baseURL, + HTTP: &http.Client{ + Timeout: 30 * time.Second, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + }, + } der, err := base64.StdEncoding.DecodeString(keyB64) if err != nil { return nil, fmt.Errorf("bad log key: %w", err) @@ -56,6 +78,14 @@ func NewClient(baseURL, keyB64 string) (*Client, error) { return c, nil } +func isLoopbackHost(host string) bool { + if host == "localhost" { + return true + } + ip := net.ParseIP(host) + return ip != nil && ip.IsLoopback() +} + func (c *Client) LogID() string { return base64.StdEncoding.EncodeToString(c.logID) } diff --git a/internal/rfc6962/client_test.go b/internal/rfc6962/client_test.go index 615b131..c9d2e4b 100644 --- a/internal/rfc6962/client_test.go +++ b/internal/rfc6962/client_test.go @@ -48,3 +48,35 @@ func TestNewClientTrimsTrailingSlash(t *testing.T) { t.Fatal("no request reached the server") } } + +func TestNewClientRejectsPlainHTTP(t *testing.T) { + if _, err := NewClient("http://ct.example.com/", testKeyB64(t)); err == nil { + t.Fatal("NewClient should reject non-loopback plain http") + } +} + +func TestClientDoesNotFollowRedirects(t *testing.T) { + var targetHits atomic.Int32 + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + targetHits.Add(1) + w.Write([]byte(`{}`)) + })) + defer target.Close() + + source := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, target.URL+"/ct/v1/get-sth", http.StatusFound) + })) + defer source.Close() + + c, err := NewClient(source.URL, testKeyB64(t)) // loopback http is allowed + if err != nil { + t.Fatal(err) + } + _, err = c.STH(context.Background()) + if err == nil { + t.Fatal("STH via redirecting server should fail") + } + if targetHits.Load() != 0 { + t.Fatalf("redirect was followed to target (%d hits)", targetHits.Load()) + } +}