From 0da168062c8ddd854ee799d103bb63eafd487952 Mon Sep 17 00:00:00 2001 From: lakshit verma Date: Mon, 24 Aug 2026 00:41:05 +0530 Subject: [PATCH] rfc6962,loglist: harden HTTP egress against SSRF via log lists Endpoints come from externally fetched log lists and were used as-is: any scheme was accepted and the default HTTP client followed up to 10 redirects, so a compromised list source could aim the tailer at internal hosts (e.g. cloud metadata) even though STH signatures would fail. Now NewClient requires https except for loopback (local test servers), both clients refuse redirects outright, and a redirect response surfaces as a normal HTTP error. Tests cover scheme rejection and redirect refusal. --- internal/loglist/loglist.go | 9 ++++++++- internal/rfc6962/client.go | 36 ++++++++++++++++++++++++++++++--- internal/rfc6962/client_test.go | 32 +++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 4 deletions(-) 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()) + } +}