summaryrefslogtreecommitdiff
path: root/server
diff options
context:
space:
mode:
Diffstat (limited to 'server')
-rw-r--r--server/cmd/listend/main.go5
-rw-r--r--server/internal/mirror/mirror.go8
-rw-r--r--server/internal/mirror/mirror_test.go23
-rw-r--r--server/internal/safefetch/audit_test.go79
-rw-r--r--server/internal/safefetch/safefetch.go110
-rw-r--r--server/internal/safefetch/safefetch_test.go86
-rw-r--r--server/internal/transcript/harvest.go9
-rw-r--r--server/internal/transcript/harvest_test.go19
8 files changed, 324 insertions, 15 deletions
diff --git a/server/cmd/listend/main.go b/server/cmd/listend/main.go
index 3ca2727..0e787e4 100644
--- a/server/cmd/listend/main.go
+++ b/server/cmd/listend/main.go
@@ -31,6 +31,7 @@ import (
"github.com/terst/listen/server/internal/decode"
"github.com/terst/listen/server/internal/feed"
"github.com/terst/listen/server/internal/mirror"
+ "github.com/terst/listen/server/internal/safefetch"
"github.com/terst/listen/server/internal/signature"
"github.com/terst/listen/server/internal/skips"
"github.com/terst/listen/server/internal/store"
@@ -580,7 +581,9 @@ func fetchFeed(ctx context.Context, url string) ([]byte, error) {
return nil, err
}
req.Header.Set("User-Agent", "listen-server/1.0 (+https://terst.org)")
- resp, err := http.DefaultClient.Do(req)
+ // A feed URL is whatever the phone sent, so the same rule applies as to everything else
+ // this server fetches: it may only reach a public address.
+ resp, err := safefetch.Client(0).Do(req)
if err != nil {
return nil, err
}
diff --git a/server/internal/mirror/mirror.go b/server/internal/mirror/mirror.go
index 9e91041..47d29f2 100644
--- a/server/internal/mirror/mirror.go
+++ b/server/internal/mirror/mirror.go
@@ -14,6 +14,7 @@ import (
"strings"
"time"
+ "github.com/terst/listen/server/internal/safefetch"
"github.com/terst/listen/server/internal/store"
)
@@ -44,7 +45,9 @@ func (m *Mirror) client() *http.Client {
if m.Client != nil {
return m.Client
}
- return &http.Client{Timeout: DefaultTimeout}
+ // An enclosure URL comes from the publisher's feed. Same rule as every other outbound
+ // fetch here: it may only reach a public address.
+ return safefetch.Client(DefaultTimeout)
}
func (m *Mirror) userAgent() string {
@@ -67,6 +70,9 @@ func (m *Mirror) Fetch(ctx context.Context, e *store.Episode) error {
return fmt.Errorf("episode %s: %w", e.ID, err)
}
+ if err := safefetch.CheckURL(e.EnclosureURL); err != nil {
+ return err
+ }
req, err := http.NewRequestWithContext(ctx, http.MethodGet, e.EnclosureURL, nil)
if err != nil {
return fmt.Errorf("episode %s: %w", e.ID, err)
diff --git a/server/internal/mirror/mirror_test.go b/server/internal/mirror/mirror_test.go
index b510451..fd1e716 100644
--- a/server/internal/mirror/mirror_test.go
+++ b/server/internal/mirror/mirror_test.go
@@ -8,6 +8,7 @@ import (
"path/filepath"
"strings"
"testing"
+ "time"
"github.com/terst/listen/server/internal/store"
)
@@ -28,7 +29,7 @@ func TestFetchWritesTheAudioAndRecordsWhereItWent(t *testing.T) {
defer srv.Close()
dir := t.TempDir()
- m := &Mirror{AudioDir: dir, Probe: stubProbe{ms: 1325113}}
+ m := &Mirror{AudioDir: dir, Probe: stubProbe{ms: 1325113}, Client: loopbackClient()}
e := episode("abc", srv.URL+"/1.mp3")
if err := m.Fetch(context.Background(), e); err != nil {
@@ -57,7 +58,7 @@ func TestFetchNamesTheFileByEpisodeId(t *testing.T) {
}))
defer srv.Close()
- m := &Mirror{AudioDir: t.TempDir(), Probe: stubProbe{ms: 1}}
+ m := &Mirror{AudioDir: t.TempDir(), Probe: stubProbe{ms: 1}, Client: loopbackClient()}
e := episode("deadbeef", srv.URL+"/whatever.m4a")
if err := m.Fetch(context.Background(), e); err != nil {
t.Fatal(err)
@@ -79,7 +80,7 @@ func TestAFailedFetchLeavesNothingBehind(t *testing.T) {
defer srv.Close()
dir := t.TempDir()
- m := &Mirror{AudioDir: dir, Probe: stubProbe{ms: 1}}
+ m := &Mirror{AudioDir: dir, Probe: stubProbe{ms: 1}, Client: loopbackClient()}
e := episode("abc", srv.URL+"/1.mp3")
if err := m.Fetch(context.Background(), e); err == nil {
@@ -98,7 +99,7 @@ func TestAFailedFetchLeavesNothingBehind(t *testing.T) {
}
func TestFetchRefusesAnEpisodeWithNoEnclosure(t *testing.T) {
- m := &Mirror{AudioDir: t.TempDir(), Probe: stubProbe{ms: 1}}
+ m := &Mirror{AudioDir: t.TempDir(), Probe: stubProbe{ms: 1}, Client: loopbackClient()}
if err := m.Fetch(context.Background(), episode("abc", "")); err == nil {
t.Error("expected an error for an episode with nothing to download")
}
@@ -106,7 +107,7 @@ func TestFetchRefusesAnEpisodeWithNoEnclosure(t *testing.T) {
func TestPruneRemovesTheFileAndForgetsThePath(t *testing.T) {
dir := t.TempDir()
- m := &Mirror{AudioDir: dir, Probe: stubProbe{ms: 1}}
+ m := &Mirror{AudioDir: dir, Probe: stubProbe{ms: 1}, Client: loopbackClient()}
e := episode("abc", "https://cdn/1.mp3")
e.Path = "abc.mp3"
if err := os.WriteFile(filepath.Join(dir, e.Path), []byte("bytes"), 0o644); err != nil {
@@ -128,7 +129,7 @@ func TestPruneRemovesTheFileAndForgetsThePath(t *testing.T) {
// mean downloading it again.
func TestPruneKeepsWhatDetectionDecided(t *testing.T) {
dir := t.TempDir()
- m := &Mirror{AudioDir: dir, Probe: stubProbe{ms: 1}}
+ m := &Mirror{AudioDir: dir, Probe: stubProbe{ms: 1}, Client: loopbackClient()}
e := episode("abc", "https://cdn/1.mp3")
e.Path = "abc.mp3"
intro := int64(16652)
@@ -150,7 +151,7 @@ func TestPruneKeepsWhatDetectionDecided(t *testing.T) {
}
func TestPruningSomethingAlreadyGoneIsFine(t *testing.T) {
- m := &Mirror{AudioDir: t.TempDir(), Probe: stubProbe{ms: 1}}
+ m := &Mirror{AudioDir: t.TempDir(), Probe: stubProbe{ms: 1}, Client: loopbackClient()}
e := episode("abc", "https://cdn/1.mp3")
e.Path = "not-there.mp3"
if err := m.Prune(e); err != nil {
@@ -160,3 +161,11 @@ func TestPruningSomethingAlreadyGoneIsFine(t *testing.T) {
t.Error("Path not cleared")
}
}
+
+// loopbackClient is a plain http client for the test servers here.
+//
+// Production uses safefetch, which refuses a loopback address on purpose: an enclosure URL
+// comes from a publisher's feed and must not be able to name something only this host can
+// reach. A httptest server is on loopback by definition, so the tests supply the seam that
+// exists for exactly this.
+func loopbackClient() *http.Client { return &http.Client{Timeout: 30 * time.Second} }
diff --git a/server/internal/safefetch/audit_test.go b/server/internal/safefetch/audit_test.go
new file mode 100644
index 0000000..fa77cea
--- /dev/null
+++ b/server/internal/safefetch/audit_test.go
@@ -0,0 +1,79 @@
+package safefetch
+
+import (
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+)
+
+/*
+No outbound request in this server may bypass safefetch.
+
+The SSRF this package exists for was found in the transcript harvester, but the harvester was
+the third place that fetched a URL somebody else chose: the feed fetch and the enclosure
+download had the same shape and had had it for longer. Fixing the instance that was flagged and
+leaving the other two would have fixed nothing.
+
+This walks the server's own source and refuses a production file that builds its own HTTP client
+or uses the shared default one. The allowed way to get a client is safefetch.Client; a test may
+inject a plain one through the seam the type already has, because a httptest server is on
+loopback by definition.
+*/
+
+// banned are the ways to reach the network without going through this package.
+var banned = []string{
+ "http.DefaultClient",
+ "http.DefaultTransport.RoundTrip",
+ "&http.Client{",
+ "http.Get(",
+ "http.Post(",
+ "http.Head(",
+}
+
+// allowed names the files that may mention them, and why.
+var allowed = map[string]string{
+ // This package is where the safe client is built.
+ filepath.Join("internal", "safefetch", "safefetch.go"): "builds the client",
+}
+
+func TestNothingFetchesWithoutSafefetch(t *testing.T) {
+ root, err := filepath.Abs("../..")
+ if err != nil {
+ t.Fatalf("root: %v", err)
+ }
+
+ err = filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
+ if err != nil || info.IsDir() || !strings.HasSuffix(path, ".go") {
+ return err
+ }
+ // A test may inject a plain client through the seam: the servers it talks to are on
+ // loopback, which is the thing production is refusing.
+ if strings.HasSuffix(path, "_test.go") {
+ return nil
+ }
+ relative, err := filepath.Rel(root, path)
+ if err != nil {
+ return err
+ }
+ if _, ok := allowed[relative]; ok {
+ return nil
+ }
+ raw, err := os.ReadFile(path)
+ if err != nil {
+ return err
+ }
+ body := string(raw)
+ for _, pattern := range banned {
+ if strings.Contains(body, pattern) {
+ t.Errorf("%s uses %s -- every outbound request goes through safefetch.Client, "+
+ "because the URLs this server fetches are chosen by podcast publishers",
+ relative, pattern)
+ }
+ }
+ return nil
+ })
+ if err != nil {
+ t.Fatalf("walk: %v", err)
+ }
+}
diff --git a/server/internal/safefetch/safefetch.go b/server/internal/safefetch/safefetch.go
new file mode 100644
index 0000000..3233f8f
--- /dev/null
+++ b/server/internal/safefetch/safefetch.go
@@ -0,0 +1,110 @@
+// Package safefetch makes outbound requests to addresses this server did not choose.
+//
+// Everything listend fetches is named by somebody else. The feed URL comes from the phone, and
+// the enclosure and <podcast:transcript> URLs inside the feed come from the publisher. listend
+// listens on 127.0.0.1:38090 behind Apache on a host running other things, so a feed naming a
+// loopback, private or link-local address would have the server fetch something reachable only
+// from inside -- and in the transcript case store the result and hand it to the phone as an
+// episode's words.
+//
+// The check happens when the connection is dialled rather than when the URL is read. That is
+// the only point that also covers a redirect into an internal address and a hostname that
+// resolves to one: the address connected to is the address that was checked.
+package safefetch
+
+import (
+ "context"
+ "fmt"
+ "net"
+ "net/http"
+ "net/url"
+ "time"
+)
+
+// DefaultTimeout is what Client uses when given none.
+const DefaultTimeout = 60 * time.Second
+
+// CheckURL rejects anything that is not an ordinary web address.
+//
+// A publisher naming file:// or gopher:// has not given us a transcript, and Go's http client
+// would not fetch those anyway -- the value of saying so here is that the refusal is explicit
+// and testable rather than an obscure transport error.
+func CheckURL(raw string) error {
+ parsed, err := url.Parse(raw)
+ if err != nil {
+ return fmt.Errorf("safefetch: %q is not a URL: %w", raw, err)
+ }
+ if parsed.Scheme != "http" && parsed.Scheme != "https" {
+ return fmt.Errorf("safefetch: %q is not http or https", raw)
+ }
+ if parsed.Host == "" {
+ return fmt.Errorf("safefetch: %q names no host", raw)
+ }
+ return nil
+}
+
+// Client returns an http.Client that can only reach public addresses.
+func Client(timeout time.Duration) *http.Client {
+ if timeout <= 0 {
+ timeout = DefaultTimeout
+ }
+ dialer := &net.Dialer{Timeout: 20 * time.Second, KeepAlive: 30 * time.Second}
+ transport := http.DefaultTransport.(*http.Transport).Clone()
+ transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
+ host, port, err := net.SplitHostPort(addr)
+ if err != nil {
+ return nil, fmt.Errorf("safefetch: %q: %w", addr, err)
+ }
+ addresses, err := net.DefaultResolver.LookupIPAddr(ctx, host)
+ if err != nil {
+ return nil, fmt.Errorf("safefetch: %q: %w", host, err)
+ }
+ // Dial a checked address rather than the hostname, so the connection cannot land
+ // somewhere a second lookup would have resolved to.
+ var lastErr error
+ for _, candidate := range addresses {
+ if !permitted(candidate.IP) {
+ lastErr = fmt.Errorf("safefetch: %s is not a public address", candidate.IP)
+ continue
+ }
+ conn, err := dialer.DialContext(ctx, network, net.JoinHostPort(candidate.IP.String(), port))
+ if err == nil {
+ return conn, nil
+ }
+ lastErr = err
+ }
+ if lastErr == nil {
+ lastErr = fmt.Errorf("safefetch: %q resolved to nothing", host)
+ }
+ return nil, lastErr
+ }
+ return &http.Client{Timeout: timeout, Transport: transport}
+}
+
+// permitted reports whether an address is one the open internet could have given us.
+//
+// Everything a podcast is actually served from is a public address. The excluded ranges are the
+// ones that mean "somewhere only this host or this network can reach": loopback, the RFC 1918
+// private ranges, IPv6 unique-local, link-local -- which includes 169.254.169.254, the cloud
+// metadata address -- and the unspecified and multicast addresses.
+func permitted(ip net.IP) bool {
+ if ip == nil {
+ return false
+ }
+ // An IPv4 address written in IPv6-mapped form is the same address and reaches the same
+ // host, so it is judged as the IPv4 one.
+ if mapped := ip.To4(); mapped != nil {
+ ip = mapped
+ }
+ switch {
+ case ip.IsLoopback(),
+ ip.IsPrivate(),
+ ip.IsLinkLocalUnicast(),
+ ip.IsLinkLocalMulticast(),
+ ip.IsInterfaceLocalMulticast(),
+ ip.IsMulticast(),
+ ip.IsUnspecified():
+ return false
+ }
+ return true
+}
diff --git a/server/internal/safefetch/safefetch_test.go b/server/internal/safefetch/safefetch_test.go
new file mode 100644
index 0000000..dfa3470
--- /dev/null
+++ b/server/internal/safefetch/safefetch_test.go
@@ -0,0 +1,86 @@
+package safefetch
+
+import (
+ "context"
+ "net"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+)
+
+/*
+Outbound requests to addresses a podcast feed chose.
+
+Everything this server fetches is named by somebody else: the feed URL comes from the phone, and
+the enclosure and <podcast:transcript> URLs inside it come from the publisher. listend runs on
+127.0.0.1:38090 behind Apache, on a host with other services, so a feed naming a loopback or
+private address would have the server fetch something only it can reach -- and in the transcript
+case hand the result to the phone as an episode's words.
+
+Checked at dial time rather than on the URL string, because that is the only place that catches
+a redirect to an internal address and a hostname that resolves to one. The address actually
+connected to is the address that was checked.
+*/
+
+func TestLoopbackAndPrivateAddressesAreRefused(t *testing.T) {
+ blocked := []string{
+ "127.0.0.1", "127.1.2.3", "::1",
+ "10.0.0.5", "172.16.3.1", "192.168.1.1",
+ "169.254.169.254", // the cloud metadata address
+ "fe80::1", "fc00::1",
+ "0.0.0.0", "::",
+ }
+ for _, ip := range blocked {
+ if permitted(net.ParseIP(ip)) {
+ t.Errorf("%s was permitted", ip)
+ }
+ }
+}
+
+func TestOrdinaryPublicAddressesArePermitted(t *testing.T) {
+ for _, ip := range []string{"93.184.216.34", "1.1.1.1", "2606:4700:4700::1111"} {
+ if !permitted(net.ParseIP(ip)) {
+ t.Errorf("%s was refused", ip)
+ }
+ }
+}
+
+// An IPv4 address written as an IPv6-mapped one is the same address, and reaches the same host.
+func TestAMappedLoopbackAddressIsStillLoopback(t *testing.T) {
+ if permitted(net.ParseIP("::ffff:127.0.0.1")) {
+ t.Error("::ffff:127.0.0.1 was permitted")
+ }
+}
+
+// A real connection to a server on loopback, which is exactly the shape the finding describes.
+func TestAClientCannotReachALoopbackServer(t *testing.T) {
+ server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+ _, _ = w.Write([]byte("internal"))
+ }))
+ defer server.Close()
+
+ req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil)
+ if err != nil {
+ t.Fatalf("request: %v", err)
+ }
+ if _, err := Client(0).Do(req); err == nil {
+ t.Fatal("reached a loopback server")
+ } else if !strings.Contains(err.Error(), "not a public address") {
+ t.Errorf("refused for the wrong reason: %v", err)
+ }
+}
+
+// A publisher could name a file:// or gopher:// URL; neither is something to fetch.
+func TestOnlyHTTPAndHTTPSAreFetched(t *testing.T) {
+ for _, raw := range []string{"file:///etc/passwd", "gopher://x/", "ftp://x/f", "x", ""} {
+ if err := CheckURL(raw); err == nil {
+ t.Errorf("%q was accepted", raw)
+ }
+ }
+ for _, raw := range []string{"https://example.com/f.vtt", "http://example.com/f.vtt"} {
+ if err := CheckURL(raw); err != nil {
+ t.Errorf("%q was refused: %v", raw, err)
+ }
+ }
+}
diff --git a/server/internal/transcript/harvest.go b/server/internal/transcript/harvest.go
index ed149e3..6369ea3 100644
--- a/server/internal/transcript/harvest.go
+++ b/server/internal/transcript/harvest.go
@@ -6,6 +6,8 @@ import (
"io"
"net/http"
"time"
+
+ "github.com/terst/listen/server/internal/safefetch"
)
// maxTranscriptBytes caps a download. The largest in this corpus is Darknet Diaries' 1784-cue
@@ -44,7 +46,9 @@ func (h Harvester) client() *http.Client {
if h.Client != nil {
return h.Client
}
- return &http.Client{Timeout: 60 * time.Second}
+ // A transcript URL comes from a publisher's feed, so it names an address this server did
+ // not choose. safefetch refuses one that resolves to somewhere only this host can reach.
+ return safefetch.Client(60 * time.Second)
}
// Harvest fetches an offered transcript and stores it if it describes the mirrored file.
@@ -80,6 +84,9 @@ func (h Harvester) Harvest(ctx context.Context, offer Offer) (Result, error) {
}
func (h Harvester) download(ctx context.Context, url string) (string, error) {
+ if err := safefetch.CheckURL(url); err != nil {
+ return "", err
+ }
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil {
return "", fmt.Errorf("transcript: %w", err)
diff --git a/server/internal/transcript/harvest_test.go b/server/internal/transcript/harvest_test.go
index ec55681..23a192a 100644
--- a/server/internal/transcript/harvest_test.go
+++ b/server/internal/transcript/harvest_test.go
@@ -6,6 +6,7 @@ import (
"net/http/httptest"
"strings"
"testing"
+ "time"
)
/*
@@ -46,7 +47,7 @@ func serving(t *testing.T, status int, body, contentType string) *httptest.Serve
func TestHarvestStoresAPublishedTranscriptThatFitsTheFile(t *testing.T) {
server := serving(t, 200, harvestSRT, "application/srt")
store := Store{Dir: t.TempDir()}
- harvester := Harvester{Store: store}
+ harvester := Harvester{Store: store, Client: loopbackClient()}
got, err := harvester.Harvest(context.Background(), Offer{
EpisodeID: "ep1",
@@ -73,7 +74,7 @@ func TestHarvestStoresAPublishedTranscriptThatFitsTheFile(t *testing.T) {
func TestHarvestDeclinesATranscriptForAnAdInsertedCopy(t *testing.T) {
server := serving(t, 200, harvestSRT, "application/srt")
store := Store{Dir: t.TempDir()}
- harvester := Harvester{Store: store}
+ harvester := Harvester{Store: store, Client: loopbackClient()}
// Darknet Diaries' shape: the file is minutes longer than the feed declares.
got, err := harvester.Harvest(context.Background(), Offer{
@@ -101,7 +102,7 @@ func TestHarvestDeclinesATranscriptForAnAdInsertedCopy(t *testing.T) {
// is worth another attempt; a refusal is not.
func TestAFailedDownloadIsAnErrorAndNotAVerdict(t *testing.T) {
server := serving(t, 404, "nope", "text/plain")
- harvester := Harvester{Store: Store{Dir: t.TempDir()}}
+ harvester := Harvester{Store: Store{Dir: t.TempDir()}, Client: loopbackClient()}
got, err := harvester.Harvest(context.Background(), Offer{
EpisodeID: "ep1", URL: server.URL, Type: "application/srt",
@@ -119,7 +120,7 @@ func TestAFailedDownloadIsAnErrorAndNotAVerdict(t *testing.T) {
// that is a verdict: the feed says this is the transcript, and it is not one.
func TestSomethingThatIsNotCuesIsDeclinedNotRetried(t *testing.T) {
server := serving(t, 200, "<html><body>Transcript coming soon</body></html>", "text/html")
- harvester := Harvester{Store: Store{Dir: t.TempDir()}}
+ harvester := Harvester{Store: Store{Dir: t.TempDir()}, Client: loopbackClient()}
got, err := harvester.Harvest(context.Background(), Offer{
EpisodeID: "ep1", URL: server.URL, Type: "application/srt",
@@ -134,7 +135,7 @@ func TestSomethingThatIsNotCuesIsDeclinedNotRetried(t *testing.T) {
}
func TestAnEpisodeOfferingNoTranscriptIsNotFetched(t *testing.T) {
- harvester := Harvester{Store: Store{Dir: t.TempDir()}}
+ harvester := Harvester{Store: Store{Dir: t.TempDir()}, Client: loopbackClient()}
got, err := harvester.Harvest(context.Background(), Offer{EpisodeID: "ep1", ServedMs: 1_000})
if err != nil {
t.Fatalf("Harvest: %v", err)
@@ -143,3 +144,11 @@ func TestAnEpisodeOfferingNoTranscriptIsNotFetched(t *testing.T) {
t.Errorf("got %+v, want nothing done and nothing said", got)
}
}
+
+// loopbackClient is a plain http client for the test servers here.
+//
+// Production uses safefetch, which refuses a loopback address on purpose: a transcript URL
+// comes from a publisher's feed, and one naming 127.0.0.1 would have the server fetch
+// something only it can reach and hand the result to the phone as an episode's words. A
+// httptest server is on loopback by definition, so the tests supply the Client seam.
+func loopbackClient() *http.Client { return &http.Client{Timeout: 30 * time.Second} }
generated by cgit v1.2.3 (git 2.39.1) at 2026-11-14 21:43:55 +0000