diff --git a/TODO.md b/TODO.md index da1345b..9ae228e 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,12 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-16 a source host that is an IPv6 address in brackets with no port, + such as `[2001:db8::a]`, is fetched like one with a port, through `/v1/image/` + and through an encrypted URL (closes #295). The fetcher's check of the source + URL kept the brackets when no port followed them, could not look the host up, + and the request answered 500. It now checks the address without brackets, so a + private one is refused with 403, as with a port. - 2026-10-16 a `state_dir` whose name holds `%`, `?` or `#` keeps its database at `state.sqlite3` in that directory (closes #290): the `db_url` derived from it put the name into a `file:` URL as it was, so SQLite read `%41` as `A`, a diff --git a/internal/httpfetcher/httpfetcher.go b/internal/httpfetcher/httpfetcher.go index 1428040..159ee68 100644 --- a/internal/httpfetcher/httpfetcher.go +++ b/internal/httpfetcher/httpfetcher.go @@ -473,16 +473,16 @@ func validateURL(ctx context.Context, rawURL string, allowHTTP bool) error { return ErrUnsupportedScheme } - // Parse to extract host - host := extractHost(rawURL) - if host == "" { - return ErrInvalidHost + parsedURL, err := neturl.Parse(rawURL) + if err != nil { + return fmt.Errorf("%w: %w", ErrInvalidHost, err) } - // Remove port if present - h, _, err := net.SplitHostPort(host) - if err == nil { - host = h + // Hostname drops any port, and the brackets of an IPv6 address whether + // or not a port follows them + host := parsedURL.Hostname() + if host == "" { + return ErrInvalidHost } // Block obvious localhost patterns diff --git a/internal/server/source_url_internal_test.go b/internal/server/source_url_internal_test.go index d006c44..0a5b4a1 100644 --- a/internal/server/source_url_internal_test.go +++ b/internal/server/source_url_internal_test.go @@ -6,12 +6,16 @@ import ( "net/url" "regexp" "slices" + "strconv" "strings" "sync" "testing" + "time" + "sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/session" + "sneak.berlin/go/pixa/internal/signature" ) // percentSignPath is a source path holding a "%", which a URL writes %25. @@ -433,3 +437,109 @@ func TestGeneratorRefusesSourceURLItCannotFetchAsWritten(t *testing.T) { }) } } + +// ipv6SourcePath is the source path of the image URLs signedImageURL and +// encryptedImageURL make. +const ipv6SourcePath = "/ipv6.png" + +// TestImageRoutesFetchIPv6SourceHostWithOrWithoutPort requests ipv6SourcePath +// on an IPv6 source host, written with and without a port, through a +// /v1/image/ URL with a signature and through an encrypted URL. The public +// address 2001:db8::a, a documentation address (RFC 3849) the fetcher accepts +// as public, answers 200, and the upstream receives the request for +// ipv6SourcePath. The private address fd00::1 answers 403, and the upstream +// receives no request: the test's fetcher connects every address to the test +// upstream, so only the fetcher's check of the source URL can refuse it. +func TestImageRoutesFetchIPv6SourceHostWithOrWithoutPort(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + sourceHost string + wantStatus int + // wantRequests are the request targets the upstream must receive + wantRequests []string + }{ + {"public address without a port", "[2001:db8::a]", + http.StatusOK, []string{ipv6SourcePath}}, + {"public address with a port", "[2001:db8::a]:8443", + http.StatusOK, []string{ipv6SourcePath}}, + {"private address without a port", "[fd00::1]", + http.StatusForbidden, nil}, + {"private address with a port", "[fd00::1]:8443", + http.StatusForbidden, nil}, + } + + for _, tt := range tests { + imageURLs := map[string]string{ + "image URL with a signature": signedImageURL(t, tt.sourceHost), + "encrypted URL": encryptedImageURL(t, tt.sourceHost), + } + + for route, imageURL := range imageURLs { + t.Run(tt.name+" "+route, func(t *testing.T) { + t.Parallel() + + upstream := startRecordingUpstream(t) + s, _, _ := startImageProxy(t, upstream.Server) + + rec := httptest.NewRecorder() + s.ServeHTTP(rec, httptest.NewRequestWithContext( + t.Context(), http.MethodGet, imageURL, nil)) + t.Logf("GET %s: %d", imageURL, rec.Code) + + if rec.Code != tt.wantStatus { + t.Errorf("status = %d, want %d; body %s", + rec.Code, tt.wantStatus, rec.Body.String()) + } + + got := upstream.requestTargets() + t.Logf("upstream received %q", got) + + if !slices.Equal(got, tt.wantRequests) { + t.Errorf("upstream received %q, want %q", got, tt.wantRequests) + } + }) + } + } +} + +// signedImageURL returns the /v1/image/ URL of ipv6SourcePath on sourceHost at +// its original size as PNG, with a signature made with the test signing key, +// which a host not on the allowlist needs. The URL has no q or fit, so it is +// signed with the quality 85 and the fit cover. +func signedImageURL(t *testing.T, sourceHost string) string { + t.Helper() + + path, sig, exp := signature.New(testSigningKey).GenerateSignedURL( + &signature.Request{ + SourceHost: sourceHost, + SourcePath: ipv6SourcePath, + Format: string(imgcache.FormatPNG), + Quality: 85, + FitMode: string(imgcache.FitCover), + }, time.Hour) + + return path + "?sig=" + sig + "&exp=" + strconv.FormatInt(exp, 10) +} + +// encryptedImageURL returns the encrypted URL of ipv6SourcePath on sourceHost +// at its original size as PNG, made with the test signing key. +func encryptedImageURL(t *testing.T, sourceHost string) string { + t.Helper() + + generator, err := encurl.NewGenerator(testSigningKey) + if err != nil { + t.Fatalf("encurl.NewGenerator: %v", err) + } + + token, err := generator.Generate(&encurl.Payload{ + SourceHost: sourceHost, SourcePath: ipv6SourcePath, + Format: imgcache.FormatPNG, + }) + if err != nil { + t.Fatalf("encurl Generate: %v", err) + } + + return "/v1/e/" + token + "/img.png" +}