diff --git a/config.example.yaml b/config.example.yaml index 002d5069..98e9b908 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -133,12 +133,15 @@ storage: # How long presigned URLs remain valid (e.g. "5m", "1h"). Default: "15m". direct_serve_ttl: "15m" - # Public base URL to substitute into presigned URLs. Set this when the - # proxy reaches storage at an internal address (127.0.0.1, a Docker - # service name) but clients must use a public hostname. Only scheme and - # host are used; the signed path and query are preserved. For S3/MinIO - # the reverse proxy at this address must forward requests with the - # internal Host header or the SigV4 signature will not validate. + # Public base URL for presigned URLs. Set this when the proxy reaches + # storage at an internal address (127.0.0.1, a Docker service name, or + # VPC DNS) but clients must use a public hostname. Only scheme and host + # are used. For S3/MinIO the URL is signed for this host, so the server + # there must receive that Host header: a reverse proxy or CDN that sends + # the internal or bucket host to the origin fails signature validation. + # With virtual-hosted addressing (no use_path_style) the URL carries no + # bucket name, so this must be the bucket's own hostname. For GCS and + # Azure the host is replaced after signing. # direct_serve_base_url: "https://minio.example.com" # Database configuration diff --git a/go.mod b/go.mod index dce58411..f1cb4f7a 100644 --- a/go.mod +++ b/go.mod @@ -5,8 +5,11 @@ go 1.27.3 require ( github.com/BurntSushi/toml v1.6.0 github.com/Masterminds/semver/v3 v3.5.0 + github.com/aws/aws-sdk-go-v2 v1.47.5 github.com/aws/aws-sdk-go-v2/config v1.33.12 github.com/aws/aws-sdk-go-v2/service/ecr v1.67.1 + github.com/aws/aws-sdk-go-v2/service/s3 v1.111.0 + github.com/aws/smithy-go v1.28.7 github.com/git-pkgs/archives v0.10.0 github.com/git-pkgs/artifacts v0.2.1 github.com/git-pkgs/cooldown v0.3.1 @@ -51,7 +54,6 @@ require ( github.com/PuerkitoBio/purell v1.1.1 // indirect github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 // indirect github.com/apapsch/go-jsonmerge/v2 v2.0.0 // indirect - github.com/aws/aws-sdk-go-v2 v1.47.5 // indirect github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.20 // indirect github.com/aws/aws-sdk-go-v2/credentials v1.20.12 // indirect github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.20.5 // indirect @@ -63,12 +65,10 @@ require ( github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.11.2 // indirect github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.14.8 // indirect github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.20.2 // indirect - github.com/aws/aws-sdk-go-v2/service/s3 v1.111.0 // indirect github.com/aws/aws-sdk-go-v2/service/signin v1.12.0 // indirect github.com/aws/aws-sdk-go-v2/service/sso v1.39.0 // indirect github.com/aws/aws-sdk-go-v2/service/ssooidc v1.44.0 // indirect github.com/aws/aws-sdk-go-v2/service/sts v1.51.6 // indirect - github.com/aws/smithy-go v1.28.7 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/cenk/backoff v2.2.1+incompatible // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect diff --git a/internal/config/config.go b/internal/config/config.go index 6f8171fa..ddfbb938 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -519,10 +519,10 @@ type StorageConfig struct { // Uses Go duration syntax (e.g. "5m", "1h"). Default: "15m". DirectServeTTL string `json:"direct_serve_ttl" yaml:"direct_serve_ttl"` - // DirectServeBaseURL overrides the scheme and host of presigned URLs - // before returning them to clients. Useful when the proxy reaches - // storage at an internal address (e.g. 127.0.0.1 or a Docker hostname) - // but clients must use a public one. + // DirectServeBaseURL sets the scheme and host of presigned URLs. Useful + // when the proxy reaches storage at an internal address (e.g. 127.0.0.1 + // or a Docker hostname) but clients must use a public one. S3 URLs are + // signed for this host; GCS and Azure URLs have it replaced after signing. DirectServeBaseURL string `json:"direct_serve_base_url" yaml:"direct_serve_base_url"` // CacheArtifacts stores fetched artifacts so later downloads are served diff --git a/internal/handler/handler.go b/internal/handler/handler.go index cefc0bdb..69c25af5 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -242,11 +242,7 @@ type Proxy struct { NPMFullMetadata bool DirectServe bool DirectServeTTL time.Duration - // DirectServeBaseURL, if set, replaces the scheme and host of presigned - // URLs so clients receive a public address even when the proxy reaches - // storage at an internal one. - DirectServeBaseURL string - HTTPClient *http.Client + HTTPClient *http.Client // RateLimits is the gate under the upstream clients. Artifact fetches // check it before calling Fetcher, so held hosts never reach its breaker. @@ -576,7 +572,7 @@ func (p *Proxy) readStored(ctx context.Context, pkgPURL, versionPURL, filename s if p.DirectServe { signed, err := p.Storage.SignedURL(ctx, artifact.StoragePath, p.DirectServeTTL) if err == nil { - result.RedirectURL = rewriteSignedURLHost(signed, p.DirectServeBaseURL) + result.RedirectURL = signed p.recordCacheHit(artifact.Ecosystem, versionPURL, filename) return result, nil } @@ -617,26 +613,6 @@ func (p *Proxy) readStored(ctx context.Context, pkgPURL, versionPURL, filename s return result, nil } -// rewriteSignedURLHost replaces the scheme and host of a signed URL with those -// from baseURL, preserving the path and query (which carry the signature). -// Returns signed unchanged if baseURL is empty or either URL fails to parse. -func rewriteSignedURLHost(signed, baseURL string) string { - if baseURL == "" { - return signed - } - s, err := url.Parse(signed) - if err != nil { - return signed - } - b, err := url.Parse(baseURL) - if err != nil || b.Scheme == "" || b.Host == "" { - return signed - } - s.Scheme = b.Scheme - s.Host = b.Host - return s.String() -} - func (p *Proxy) recordCacheHit(ecosystem, versionPURL, filename string) { _ = p.DB.RecordArtifactHit(versionPURL, filename) metrics.RecordArtifactCacheHit(ecosystem) diff --git a/internal/handler/handler_test.go b/internal/handler/handler_test.go index ed56b631..8c5e7981 100644 --- a/internal/handler/handler_test.go +++ b/internal/handler/handler_test.go @@ -639,74 +639,6 @@ func TestGetOrFetchArtifact_DirectServe_Redirect(t *testing.T) { } } -func TestGetOrFetchArtifact_DirectServe_BaseURLRewrite(t *testing.T) { - proxy, db, store, _ := setupTestProxy(t) - seedPackage(t, db, store, "npm", "lodash", "4.17.21", "lodash-4.17.21.tgz", "cached content") - - proxy.DirectServe = true - proxy.DirectServeBaseURL = "https://cdn.example.com" - store.signedURL = "http://127.0.0.1:9000/bucket/npm/lodash?X-Amz-Signature=abc&X-Amz-Expires=900" - - result, err := proxy.GetOrFetchArtifact(context.Background(), "npm", "lodash", "4.17.21", "lodash-4.17.21.tgz") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - want := "https://cdn.example.com/bucket/npm/lodash?X-Amz-Signature=abc&X-Amz-Expires=900" - if result.RedirectURL != want { - t.Errorf("RedirectURL = %q, want %q", result.RedirectURL, want) - } -} - -func TestRewriteSignedURLHost(t *testing.T) { - tests := []struct { - name string - signed string - baseURL string - want string - }{ - { - "empty base url is no-op", - "http://127.0.0.1:9000/bucket/key?sig=abc", - "", - "http://127.0.0.1:9000/bucket/key?sig=abc", - }, - { - "replaces scheme and host", - "http://127.0.0.1:9000/bucket/key?sig=abc", - "https://cdn.example.com", - "https://cdn.example.com/bucket/key?sig=abc", - }, - { - "preserves path and query", - "http://minio:9000/bucket/npm/lodash/4.17.21/lodash.tgz?X-Amz-Signature=abc&X-Amz-Date=20260101", - "https://files.example.com", - "https://files.example.com/bucket/npm/lodash/4.17.21/lodash.tgz?X-Amz-Signature=abc&X-Amz-Date=20260101", - }, - { - "ignores base url path", - "http://127.0.0.1:9000/bucket/key?sig=abc", - "https://cdn.example.com/ignored", - "https://cdn.example.com/bucket/key?sig=abc", - }, - { - "invalid base url is no-op", - "http://127.0.0.1:9000/bucket/key?sig=abc", - "://bad", - "http://127.0.0.1:9000/bucket/key?sig=abc", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := rewriteSignedURLHost(tt.signed, tt.baseURL) - if got != tt.want { - t.Errorf("rewriteSignedURLHost(%q, %q) = %q, want %q", tt.signed, tt.baseURL, got, tt.want) - } - }) - } -} - func TestGetOrFetchArtifact_DirectServe_FallbackOnUnsupported(t *testing.T) { proxy, db, store, _ := setupTestProxy(t) seedPackage(t, db, store, "npm", "lodash", "4.17.21", "lodash-4.17.21.tgz", "cached content") diff --git a/internal/handler/profile.go b/internal/handler/profile.go index ceff0a30..8e6ad611 100644 --- a/internal/handler/profile.go +++ b/internal/handler/profile.go @@ -14,7 +14,7 @@ func (p *Proxy) WithCooldown(policy CooldownPolicy, shortest time.Duration) *Pro GradleReadOnly: p.GradleReadOnly, HostedEnabled: p.HostedEnabled, GradleMaxUploadSize: p.GradleMaxUploadSize, NPMFullMetadata: p.NPMFullMetadata, DirectServe: p.DirectServe, DirectServeTTL: p.DirectServeTTL, - DirectServeBaseURL: p.DirectServeBaseURL, HTTPClient: p.HTTPClient, + HTTPClient: p.HTTPClient, RateLimits: p.RateLimits, HostedNPMScopes: p.HostedNPMScopes, HostedComposerVendors: p.HostedComposerVendors, StreamArtifacts: p.StreamArtifacts, Scanners: p.Scanners, ScanSigningKey: p.ScanSigningKey, ScanFetchBaseURL: p.ScanFetchBaseURL, diff --git a/internal/handler/profile_test.go b/internal/handler/profile_test.go index 41bdaa74..a85d6ca3 100644 --- a/internal/handler/profile_test.go +++ b/internal/handler/profile_test.go @@ -34,7 +34,6 @@ func TestWithCooldownCopiesProxyFields(t *testing.T) { p.NPMFullMetadata = true p.DirectServe = true p.DirectServeTTL = time.Minute - p.DirectServeBaseURL = "https://storage.example.com" p.RateLimits = registryclient.NewRateLimitGate(http.DefaultTransport) p.HostedNPMScopes = []string{"@example"} p.HostedComposerVendors = []string{"example"} diff --git a/internal/server/server.go b/internal/server/server.go index c836ee34..59717d44 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -189,7 +189,11 @@ func New(cfg *config.Config, logger *slog.Logger, buildInfo BuildInfo) (*Server, // Fall back to file:// with Path storageURL = "file://" + cfg.Storage.Path //nolint:staticcheck // backwards compat } - store, err := storage.OpenBucket(context.Background(), storageURL) + var storageOpts []storage.Option + if cfg.Storage.DirectServe && cfg.Storage.DirectServeBaseURL != "" { + storageOpts = append(storageOpts, storage.WithSignEndpoint(cfg.Storage.DirectServeBaseURL)) + } + store, err := storage.OpenBucket(context.Background(), storageURL, storageOpts...) if err != nil { _ = db.Close() return nil, fmt.Errorf("initializing storage: %w", err) @@ -312,7 +316,6 @@ func (s *Server) serve(listener net.Listener) error { proxy.GradleMaxUploadSize = s.cfg.ParseGradleBuildCacheMaxUploadSize() proxy.DirectServe = s.cfg.Storage.DirectServe proxy.DirectServeTTL = s.cfg.ParseDirectServeTTL() - proxy.DirectServeBaseURL = s.cfg.Storage.DirectServeBaseURL proxy.StreamArtifacts = !s.cfg.Storage.CacheArtifacts if err := ConfigureDenylist(s.cfg, proxy); err != nil { return err diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 02d781d1..d631b9a8 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -1740,6 +1740,63 @@ func TestNewServer_StorageConnectivityCheck(t *testing.T) { _ = srv.db.Close() } +func TestNewServer_DirectServeBaseURL(t *testing.T) { + tempDir := t.TempDir() + awsConfigFile := filepath.Join(tempDir, "aws-config") + awsCredsFile := filepath.Join(tempDir, "aws-credentials") + if err := os.WriteFile(awsConfigFile, nil, 0o600); err != nil { + t.Fatalf("writing empty AWS config file: %v", err) + } + if err := os.WriteFile(awsCredsFile, nil, 0o600); err != nil { + t.Fatalf("writing empty AWS credentials file: %v", err) + } + t.Setenv("AWS_PROFILE", "") + t.Setenv("AWS_DEFAULT_PROFILE", "") + t.Setenv("AWS_CONFIG_FILE", awsConfigFile) + t.Setenv("AWS_SHARED_CREDENTIALS_FILE", awsCredsFile) + t.Setenv("AWS_ACCESS_KEY_ID", "test") + t.Setenv("AWS_SECRET_ACCESS_KEY", "test") + + s3Srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + defer s3Srv.Close() + + cfg := &config.Config{ + Listen: ":0", + BaseURL: "http://localhost:8080", + Storage: config.StorageConfig{ + URL: "s3://bucket?region=us-1&endpoint=" + s3Srv.URL + "&use_path_style=true", + DirectServe: true, + DirectServeBaseURL: "https://cdn.example.com", + }, + Database: config.DatabaseConfig{Path: filepath.Join(tempDir, "test.db")}, + } + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + srv, err := New(cfg, logger, BuildInfo{}) + if err != nil { + t.Fatalf("New() failed: %v", err) + } + defer func() { + _ = srv.storage.Close() + _ = srv.db.Close() + }() + + signed, err := srv.storage.SignedURL(context.Background(), "npm/lodash/4.17.21/lodash-4.17.21.tgz", time.Minute) + if err != nil { + t.Fatalf("SignedURL() failed: %v", err) + } + + u, err := url.Parse(signed) + if err != nil { + t.Fatalf("parsing signed URL %q: %v", signed, err) + } + if u.Scheme != "https" || u.Host != "cdn.example.com" { + t.Errorf("signed URL = %q, want scheme https and host cdn.example.com", signed) + } +} + func TestNewServer_InvalidAccessLogFailsBeforeDatabaseInit(t *testing.T) { tempDir := t.TempDir() dbPath := filepath.Join(tempDir, "test.db") diff --git a/internal/storage/blob.go b/internal/storage/blob.go index 50f8c40a..651da4ca 100644 --- a/internal/storage/blob.go +++ b/internal/storage/blob.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "net/url" "os" "path/filepath" "runtime" @@ -14,6 +15,9 @@ import ( "sync" "time" + "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/aws/smithy-go/middleware" + smithyhttp "github.com/aws/smithy-go/transport/http" "gocloud.dev/blob" _ "gocloud.dev/blob/azureblob" _ "gocloud.dev/blob/fileblob" @@ -54,11 +58,33 @@ type Blob struct { bucket *blob.Bucket url string + // postSignRewriteEndpoint, when set, replaces the scheme and host of + // presigned URLs after signing for backends that do not include the host + // in their signature (e.g. Azure). S3 rewrites the host before signing via + // configureS3SignEndpoint so the SigV4 Host signature matches. + postSignRewriteEndpoint string + // fileRoot is the directory backing a file:// bucket, empty for cloud // backends. Used only to clear sidecars an earlier version wrote. fileRoot string } +// Option configures OpenBucket. +type Option func(*options) + +type options struct { + signEndpoint string +} + +// WithSignEndpoint configures presigned URLs to use the scheme and host of +// signEndpoint (e.g. "https://storage.example.com") instead of the storage +// endpoint. For S3, the URL is signed directly against signEndpoint so the +// SigV4 Host signature matches; for other backends, the signed URL's scheme +// and host are rewritten. +func WithSignEndpoint(signEndpoint string) Option { + return func(o *options) { o.signEndpoint = signEndpoint } +} + // OpenBucket opens a blob bucket from a URL. // // Supported URL schemes: @@ -72,9 +98,19 @@ type Blob struct { // For local filesystem, the directory is created if it doesn't exist. // //nolint:ireturn // The URL scheme selects the storage implementation. -func OpenBucket(ctx context.Context, urlStr string) (Storage, error) { +func OpenBucket(ctx context.Context, urlStr string, opts ...Option) (Storage, error) { + var o options + for _, opt := range opts { + opt(&o) + } + if strings.HasPrefix(urlStr, "gs://") { - return OpenGCS(ctx, urlStr) + g, err := OpenGCS(ctx, urlStr) + if err != nil { + return nil, err + } + g.postSignRewriteEndpoint = o.signEndpoint + return g, nil } var fileRoot string @@ -136,7 +172,53 @@ func OpenBucket(ctx context.Context, urlStr string) (Storage, error) { return nil, fmt.Errorf("opening bucket: %w", err) } - return &Blob{bucket: bucket, url: urlStr, fileRoot: fileRoot}, nil + b := &Blob{bucket: bucket, url: urlStr, fileRoot: fileRoot} + + if o.signEndpoint != "" { + var s3Client *s3.Client + if bucket.As(&s3Client) { + if err := configureS3SignEndpoint(s3Client, o.signEndpoint); err != nil { + _ = bucket.Close() + return nil, err + } + } else { + b.postSignRewriteEndpoint = o.signEndpoint + } + } + + return b, nil +} + +// configureS3SignEndpoint registers a middleware on s3Client that rewrites +// presigned request URLs to the scheme and host of signEndpoint before SigV4 +// signing, while leaving regular S3 operations and object paths unchanged. +func configureS3SignEndpoint(s3Client *s3.Client, signEndpoint string) error { + sign, err := url.Parse(signEndpoint) + if err != nil || sign.Scheme == "" || sign.Host == "" { + return fmt.Errorf("invalid sign endpoint %q: must be an absolute URL", signEndpoint) + } + + *s3Client = *s3.New(s3Client.Options(), func(o *s3.Options) { + o.APIOptions = append(o.APIOptions, func(stack *middleware.Stack) error { + if _, ok := stack.Finalize.Get("PresignHTTPRequest"); !ok { + return nil + } + return stack.Finalize.Insert(middleware.FinalizeMiddlewareFunc( + "signEndpointHost", + func(ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler) ( + middleware.FinalizeOutput, middleware.Metadata, error, + ) { + if req, ok := in.Request.(*smithyhttp.Request); ok { + req.URL.Scheme = sign.Scheme + req.URL.Host = sign.Host + req.Host = sign.Host + } + return next.HandleFinalize(ctx, in) + }, + ), "PresignHTTPRequest", middleware.Before) + }) + }) + return nil } // legacySidecarPath gives the ".attrs" path an earlier version wrote for key, @@ -315,7 +397,7 @@ func (b *Blob) Delete(ctx context.Context, path string) error { } func (b *Blob) SignedURL(ctx context.Context, path string, expiry time.Duration) (string, error) { - url, err := b.bucket.SignedURL(ctx, path, &blob.SignedURLOptions{ + signed, err := b.bucket.SignedURL(ctx, path, &blob.SignedURLOptions{ Method: http.MethodGet, Expiry: expiry, }) @@ -325,7 +407,30 @@ func (b *Blob) SignedURL(ctx context.Context, path string, expiry time.Duration) } return "", fmt.Errorf("signing URL: %w", err) } - return url, nil + if b.postSignRewriteEndpoint != "" { + signed = rewriteSignedURLHost(signed, b.postSignRewriteEndpoint) + } + return signed, nil +} + +// rewriteSignedURLHost replaces the scheme and host of a signed URL with those +// from baseURL, preserving the path and query (which carry the signature). +// Returns signed unchanged if baseURL is empty or either URL fails to parse. +func rewriteSignedURLHost(signed, baseURL string) string { + if baseURL == "" { + return signed + } + s, err := url.Parse(signed) + if err != nil { + return signed + } + b, err := url.Parse(baseURL) + if err != nil || b.Scheme == "" || b.Host == "" { + return signed + } + s.Scheme = b.Scheme + s.Host = b.Host + return s.String() } func (b *Blob) Size(ctx context.Context, path string) (int64, error) { diff --git a/internal/storage/blob_test.go b/internal/storage/blob_test.go index 1bb9425e..debb16d5 100644 --- a/internal/storage/blob_test.go +++ b/internal/storage/blob_test.go @@ -9,6 +9,8 @@ import ( "fmt" "io" "io/fs" + "net/http" + "net/url" "os" "path/filepath" "runtime" @@ -18,6 +20,8 @@ import ( "testing" "time" + "github.com/aws/aws-sdk-go-v2/aws" + awssigv4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4" "gocloud.dev/blob" ) @@ -262,6 +266,203 @@ func TestBlobSignedURLUnsupported(t *testing.T) { } } +func TestBlobSignedURLSignEndpoint(t *testing.T) { + awsDir := t.TempDir() + awsConfigFile := filepath.Join(awsDir, "config") + awsCredsFile := filepath.Join(awsDir, "credentials") + if err := os.WriteFile(awsConfigFile, nil, 0o600); err != nil { + t.Fatalf("writing empty AWS config file: %v", err) + } + if err := os.WriteFile(awsCredsFile, nil, 0o600); err != nil { + t.Fatalf("writing empty AWS credentials file: %v", err) + } + t.Setenv("AWS_PROFILE", "") + t.Setenv("AWS_DEFAULT_PROFILE", "") + t.Setenv("AWS_CONFIG_FILE", awsConfigFile) + t.Setenv("AWS_SHARED_CREDENTIALS_FILE", awsCredsFile) + t.Setenv("AWS_ACCESS_KEY_ID", "test") + t.Setenv("AWS_SECRET_ACCESS_KEY", "test") + + const ( + pathStyleURL = "s3://bucket?region=us-1&endpoint=http://127.0.0.1:9000&use_path_style=true" + vhostURL = "s3://bucket?region=us-1&endpoint=http://127.0.0.1:9000" + signEndpoint = "https://cdn.example.com" + artifactPath = "npm/lodash/4.17.21/lodash-4.17.21.tgz" + ) + + tests := []struct { + name string + storageURL string + opts []Option + host string + path string + }{ + { + "storage endpoint without a sign endpoint", + pathStyleURL, + nil, + "127.0.0.1:9000", + "/bucket/" + artifactPath, + }, + { + "configured sign endpoint, reached over another host", + pathStyleURL, + []Option{WithSignEndpoint(signEndpoint)}, + "cdn.example.com", + "/bucket/" + artifactPath, + }, + { + "configured sign endpoint with virtual-hosted storage URL", + vhostURL, + []Option{WithSignEndpoint(signEndpoint)}, + "cdn.example.com", + "/" + artifactPath, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + + b, err := OpenBucket(ctx, tt.storageURL, tt.opts...) + if err != nil { + t.Fatalf("OpenBucket failed: %v", err) + } + defer func() { _ = b.Close() }() + + signed, err := b.SignedURL(ctx, artifactPath, time.Minute) + if err != nil { + t.Fatalf("SignedURL failed: %v", err) + } + + u, err := url.Parse(signed) + if err != nil { + t.Fatalf("parsing signed URL %q: %v", signed, err) + } + if u.Host != tt.host { + t.Errorf("signed URL %q host = %q, want %q", signed, u.Host, tt.host) + } + // The bucket and key stay where the storage URL puts them, so the + // request the client makes still names the object. + if u.Path != tt.path { + t.Errorf("signed URL path = %q, want %q", u.Path, tt.path) + } + assertSigV4Signature(t, u, "us-1", aws.Credentials{AccessKeyID: "test", SecretAccessKey: "test"}) + }) + } +} + +// assertSigV4Signature recomputes the SigV4 presigned signature for u and +// verifies it matches u's X-Amz-Signature query parameter. +func assertSigV4Signature(t *testing.T, u *url.URL, region string, creds aws.Credentials) { + t.Helper() + + q := u.Query() + signTime, err := time.Parse("20060102T150405Z", q.Get("X-Amz-Date")) + if err != nil { + t.Fatalf("parsing X-Amz-Date from %q: %v", u, err) + } + q.Del("X-Amz-Signature") + unsigned := &url.URL{Scheme: u.Scheme, Host: u.Host, Path: u.Path, RawQuery: q.Encode()} + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, unsigned.String(), nil) + if err != nil { + t.Fatalf("creating request: %v", err) + } + wantSigned, _, err := awssigv4.NewSigner(func(o *awssigv4.SignerOptions) { + o.DisableURIPathEscaping = true + }).PresignHTTP(context.Background(), creds, req, "UNSIGNED-PAYLOAD", "s3", region, signTime) + if err != nil { + t.Fatalf("PresignHTTP failed: %v", err) + } + wantURL, err := url.Parse(wantSigned) + if err != nil { + t.Fatalf("parsing expected signed URL %q: %v", wantSigned, err) + } + if gotSig, wantSig := u.Query().Get("X-Amz-Signature"), wantURL.Query().Get("X-Amz-Signature"); gotSig != wantSig { + t.Errorf("X-Amz-Signature = %q, want %q (for host %q)", gotSig, wantSig, u.Host) + } +} + +func TestAzureSignedURLSignEndpoint(t *testing.T) { + t.Setenv("AZURE_STORAGE_SAS_TOKEN", "") + t.Setenv("AZURE_STORAGE_ACCOUNT", "devstoreaccount1") + t.Setenv("AZURE_STORAGE_KEY", "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==") + t.Setenv("AZURE_STORAGE_PROTOCOL", "http") + t.Setenv("AZURE_STORAGE_IS_LOCAL_EMULATOR", "true") + + ctx := context.Background() + b, err := OpenBucket(ctx, "azblob://bucket?domain=127.0.0.1:10000", WithSignEndpoint("https://cdn.example.com")) + if err != nil { + t.Fatalf("OpenBucket failed: %v", err) + } + defer func() { _ = b.Close() }() + + signed, err := b.SignedURL(ctx, "npm/lodash/4.17.21/lodash-4.17.21.tgz", time.Minute) + if err != nil { + t.Fatalf("SignedURL failed: %v", err) + } + + u, err := url.Parse(signed) + if err != nil { + t.Fatalf("parsing signed URL %q: %v", signed, err) + } + if u.Scheme != "https" || u.Host != "cdn.example.com" { + t.Errorf("signed URL = %q, want https://cdn.example.com", signed) + } + if u.Query().Get("sig") == "" { + t.Errorf("signed URL %q carries no SAS signature", signed) + } +} + +func TestRewriteSignedURLHost(t *testing.T) { + tests := []struct { + name string + signed string + baseURL string + want string + }{ + { + "empty base url is no-op", + "http://127.0.0.1:9000/bucket/key?sig=abc", + "", + "http://127.0.0.1:9000/bucket/key?sig=abc", + }, + { + "replaces scheme and host", + "http://127.0.0.1:9000/bucket/key?sig=abc", + "https://cdn.example.com", + "https://cdn.example.com/bucket/key?sig=abc", + }, + { + "preserves path and query", + "http://minio:9000/bucket/npm/lodash/4.17.21/lodash.tgz?X-Amz-Signature=abc&X-Amz-Date=20260101", + "https://files.example.com", + "https://files.example.com/bucket/npm/lodash/4.17.21/lodash.tgz?X-Amz-Signature=abc&X-Amz-Date=20260101", + }, + { + "ignores base url path", + "http://127.0.0.1:9000/bucket/key?sig=abc", + "https://cdn.example.com/ignored", + "https://cdn.example.com/bucket/key?sig=abc", + }, + { + "invalid base url is no-op", + "http://127.0.0.1:9000/bucket/key?sig=abc", + "://bad", + "http://127.0.0.1:9000/bucket/key?sig=abc", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := rewriteSignedURLHost(tt.signed, tt.baseURL) + if got != tt.want { + t.Errorf("rewriteSignedURLHost(%q, %q) = %q, want %q", tt.signed, tt.baseURL, got, tt.want) + } + }) + } +} + func TestBlobOverwrite(t *testing.T) { b := createTestBlob(t) ctx := context.Background() diff --git a/internal/storage/gcs.go b/internal/storage/gcs.go index a096c18c..675a1015 100644 --- a/internal/storage/gcs.go +++ b/internal/storage/gcs.go @@ -15,6 +15,11 @@ import ( type GCS struct { bucket *gcstorage.Bucket url string + + // postSignRewriteEndpoint replaces the scheme and host of presigned URLs + // after signing (GCS signs against storage.googleapis.com and does not + // include the rewritten host in its V2 signature). + postSignRewriteEndpoint string } // OpenGCS opens a Google Cloud Storage bucket from a gs:// URL. @@ -59,12 +64,17 @@ func (g *GCS) Size(ctx context.Context, path string) (int64, error) { return size, err } +// SignedURL returns a presigned URL granting time-limited GET access to path, +// rewriting the scheme and host to postSignRewriteEndpoint when configured. func (g *GCS) SignedURL(ctx context.Context, path string, expiry time.Duration) (string, error) { u, err := g.bucket.SignedURL(ctx, path, expiry) if errors.Is(err, gcstorage.ErrSignedURLUnsupported) { return "", ErrSignedURLUnsupported } - return u, err + if err != nil { + return "", err + } + return rewriteSignedURLHost(u, g.postSignRewriteEndpoint), nil } func (g *GCS) UsedSpace(ctx context.Context) (int64, error) { diff --git a/internal/storage/gcs_test.go b/internal/storage/gcs_test.go index 1bd8a407..461fb866 100644 --- a/internal/storage/gcs_test.go +++ b/internal/storage/gcs_test.go @@ -2,14 +2,20 @@ package storage import ( "context" + "crypto/rand" + "crypto/rsa" "crypto/sha256" + "crypto/x509" "encoding/hex" "encoding/json" + "encoding/pem" "errors" "io" "net/http" "net/http/httptest" "net/url" + "os" + "path/filepath" "sort" "strconv" "strings" @@ -89,6 +95,56 @@ func TestOpenBucketGCSRoundTripWithEmulator(t *testing.T) { } } +func TestGCSSignedURLSignEndpoint(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("generating RSA key: %v", err) + } + pemBytes := pem.EncodeToMemory(&pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: x509.MarshalPKCS1PrivateKey(key), + }) + credsJSON, err := json.Marshal(map[string]string{ + "type": "service_account", + "client_email": "test@example.iam.gserviceaccount.com", + "private_key": string(pemBytes), + }) + if err != nil { + t.Fatalf("marshaling creds: %v", err) + } + credsFile := filepath.Join(t.TempDir(), "creds.json") + if err := os.WriteFile(credsFile, credsJSON, 0o600); err != nil { + t.Fatalf("writing creds: %v", err) + } + t.Setenv("GOOGLE_APPLICATION_CREDENTIALS", credsFile) + t.Setenv("STORAGE_EMULATOR_HOST", "") + + ctx := context.Background() + store, err := OpenBucket(ctx, "gs://test-bucket", WithSignEndpoint("https://cdn.example.com")) + if err != nil { + t.Fatalf("OpenBucket failed: %v", err) + } + defer func() { _ = store.Close() }() + + signed, err := store.SignedURL(ctx, "npm/pkg/file.tgz", time.Minute) + if err != nil { + t.Fatalf("SignedURL failed: %v", err) + } + u, err := url.Parse(signed) + if err != nil { + t.Fatalf("parsing signed URL %q: %v", signed, err) + } + if u.Scheme != "https" || u.Host != "cdn.example.com" { + t.Errorf("signed URL = %q, want https://cdn.example.com", signed) + } + if u.Path != "/test-bucket/npm/pkg/file.tgz" { + t.Errorf("signed URL path = %q, want /test-bucket/npm/pkg/file.tgz", u.Path) + } + if u.Query().Get("Signature") == "" { + t.Errorf("signed URL %q carries no signature", signed) + } +} + type fakeGCSServer struct { t *testing.T objects map[string]string