Address review feedback for GCS presigned downloads

- config: require an https GCS endpoint and HMAC credentials when signed URLs
  are enabled, and reject combining them with STS assume role (alongside the
  existing GCS IAM auth check).
- s3 store: build the presign client once and reuse it across Sign() calls.
- changes: note bootstrap package downloads are covered too.
- tests: assert the presigned URL shape and cover the STS assume-role rejection.

changes/gcs-software-installers-signed-url

@@ -1 +1 @@
-- Added the `s3_software_installers_signed_url` configuration option to serve software installer and in-house app downloads via GCS presigned URLs (the GCS counterpart to CloudFront URL signing), so clients download directly from object storage instead of streaming through the Fleet server.
+- Added the `s3_software_installers_signed_url` configuration option to serve software installer, in-house app, and bootstrap package downloads via GCS presigned URLs (the GCS counterpart to CloudFront URL signing), so clients download directly from object storage instead of streaming through the Fleet server.

ee/server/service/in_house_apps.go

@@ -218,7 +218,7 @@ func (svc *Service) GetInHouseAppManifest(ctx context.Context, titleID uint, tok
 		}
 	}
 
-	// Escape & characters in case of using CloudFront signed URL
+	// Escape & characters in case of using a signed URL (CloudFront or GCS presigned)
 	funcMap := map[string]any{
 		"xml": mobileconfig.XMLEscapeString,
 	}

ee/server/service/software_installers.go

@@ -1451,7 +1451,7 @@ func (svc *Service) GetSoftwareInstallDetails(ctx context.Context, installUUID s
 		return nil, err
 	}
 
-	// SoftwareInstallersCloudFrontSigner can only be set if license.IsPremium()
+	// Sign the download URL when CloudFront signing (premium-only) or GCS presigning is configured.
 	if svc.config.S3.SoftwareInstallersCloudFrontSigner != nil || svc.config.S3.SoftwareInstallersSignedURL {
 		// Sign the URL for the installer
 		installerURL, err := svc.getSoftwareInstallURL(ctx, details.InstallerID)

server/config/config.go

@@ -555,27 +555,35 @@ func (s S3Config) ValidateSoftwareInstallersSignedURL(initFatal func(err error,
 	if !s.SoftwareInstallersSignedURL {
 		return
 	}
-	// Validate against the parsed hostname rather than a substring match, so a
-	// URL that merely contains "storage.googleapis.com" elsewhere (e.g. a
-	// look-alike host or a path) is rejected.
-	endpoint := s.SoftwareInstallersEndpointURL
-	if !strings.Contains(endpoint, "://") {
-		// url.Parse needs a scheme to populate Hostname(); the endpoint may be
-		// configured without one (e.g. "storage.googleapis.com").
-		endpoint = "https://" + endpoint
-	}
-	u, err := url.Parse(endpoint)
+	// Presigned URLs point clients straight at the object store, so require an
+	// https scheme (no plaintext, and newS3Store's resolver needs one) and match
+	// the parsed hostname, not a substring, so a look-alike host can't satisfy it.
+	u, err := url.Parse(s.SoftwareInstallersEndpointURL)
 	if err != nil {
 		initFatal(fmt.Errorf("invalid s3_software_installers_endpoint_url: %w", err),
 			"S3 software installers signed URL")
 		return
 	}
+	if u.Scheme != "https" {
+		initFatal(errors.New("Couldn't configure. `s3_software_installers_signed_url` requires `s3_software_installers_endpoint_url` to be an https URL (e.g. https://storage.googleapis.com)."),
+			"S3 software installers signed URL")
+		return
+	}
 	host := strings.ToLower(u.Hostname())
 	if host != "storage.googleapis.com" && !strings.HasSuffix(host, ".storage.googleapis.com") {
 		initFatal(errors.New("Couldn't configure. `s3_software_installers_signed_url` requires `s3_software_installers_endpoint_url` to point at a GCS endpoint (storage.googleapis.com)."),
 			"S3 software installers signed URL")
 		return
 	}
+	// Presigning needs HMAC credentials. Without them it fails at request time and
+	// Fleet silently proxies every download, which is what this check prevents.
+	// IAM auth doesn't use HMAC creds and is rejected at store init, so skip it then.
+	if !s.SoftwareInstallersGCSIAMAuth &&
+		(s.SoftwareInstallersAccessKeyID == "" || s.SoftwareInstallersSecretAccessKey == "") {
+		initFatal(errors.New("Couldn't configure. `s3_software_installers_signed_url` requires `s3_software_installers_access_key_id` and `s3_software_installers_secret_access_key` for presigning."),
+			"S3 software installers signed URL")
+		return
+	}
 }
 
 func (s S3Config) BucketsAndPrefixesMatch() bool {

server/config/config_test.go

@@ -778,25 +778,35 @@ func TestValidateCloudfrontURL(t *testing.T) {
 func TestValidateSoftwareInstallersSignedURL(t *testing.T) {
 	t.Parallel()
 	cases := []struct {
-		name      string
-		enabled   bool
-		endpoint  string
-		wantFatal bool
+		name       string
+		enabled    bool
+		endpoint   string
+		accessKey  string
+		secret     string
+		gcsIAMAuth bool
+		wantFatal  bool
 	}{
-		{"disabled skips validation", false, "https://s3.amazonaws.com", false},
-		{"gcs host", true, "https://storage.googleapis.com", false},
-		{"gcs host without scheme", true, "storage.googleapis.com", false},
-		{"gcs bucket virtual host", true, "https://my-bucket.storage.googleapis.com", false},
-		{"look-alike host rejected", true, "https://storage.googleapis.com.evil.com", true},
-		{"substring in path rejected", true, "https://evil.com/storage.googleapis.com", true},
-		{"non-gcs host rejected", true, "https://s3.amazonaws.com", true},
+		{"disabled skips validation", false, "https://s3.amazonaws.com", "", "", false, false},
+		{"gcs host with hmac creds", true, "https://storage.googleapis.com", "GOOG-key", "secret", false, false},
+		{"gcs bucket virtual host", true, "https://my-bucket.storage.googleapis.com", "GOOG-key", "secret", false, false},
+		{"scheme-less endpoint rejected", true, "storage.googleapis.com", "GOOG-key", "secret", false, true},
+		{"http scheme rejected", true, "http://storage.googleapis.com", "GOOG-key", "secret", false, true},
+		{"look-alike host rejected", true, "https://storage.googleapis.com.evil.com", "GOOG-key", "secret", false, true},
+		{"substring in path rejected", true, "https://evil.com/storage.googleapis.com", "GOOG-key", "secret", false, true},
+		{"non-gcs host rejected", true, "https://s3.amazonaws.com", "GOOG-key", "secret", false, true},
+		{"missing access key rejected", true, "https://storage.googleapis.com", "", "secret", false, true},
+		{"missing secret rejected", true, "https://storage.googleapis.com", "GOOG-key", "", false, true},
+		{"iam auth skips hmac cred check", true, "https://storage.googleapis.com", "", "", true, false},
 	}
 
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
 			s3 := S3Config{
-				SoftwareInstallersSignedURL:   c.enabled,
-				SoftwareInstallersEndpointURL: c.endpoint,
+				SoftwareInstallersSignedURL:       c.enabled,
+				SoftwareInstallersEndpointURL:     c.endpoint,
+				SoftwareInstallersAccessKeyID:     c.accessKey,
+				SoftwareInstallersSecretAccessKey: c.secret,
+				SoftwareInstallersGCSIAMAuth:      c.gcsIAMAuth,
 			}
 			var gotFatal bool
 			initFatal := func(err error, msg string) {

server/datastore/s3/common_file_store.go

@@ -37,6 +37,10 @@ type commonFileStore struct {
 	fileLabel  string // how to call the file in error messages
 }
 
+// isGCS reports whether the endpoint targets Google Cloud Storage. The loose
+// substring match is deliberate: the GCS workarounds it gates must also apply to
+// the local mock servers in the tests. Presigning is separate and validates the
+// hostname strictly in ValidateSoftwareInstallersSignedURL.
 func isGCS(endpointURL string) bool {
 	return strings.Contains(endpointURL, "storage.googleapis.com")
 }
@@ -194,7 +198,7 @@ func (s *commonFileStore) Sign(ctx context.Context, fileID string, expiresIn tim
 	if s.cloudFrontConfig != nil {
 		urlToAccess, err := url.JoinPath(s.cloudFrontConfig.BaseURL, s.keyForFile(fileID))
 		if err != nil {
-			return "", ctxerr.Wrapf(ctx, err, "building URL for %s  with ID %s in S3 store", s.fileLabel, fileID)
+			return "", ctxerr.Wrapf(ctx, err, "building URL for %s with ID %s in S3 store", s.fileLabel, fileID)
 		}
 		signer := sign.NewURLSigner(s.cloudFrontConfig.SigningPublicKeyID, s.cloudFrontConfig.Signer)
 		signedURL, err := signer.Sign(urlToAccess, time.Now().Add(expiresIn))
@@ -204,22 +208,12 @@ func (s *commonFileStore) Sign(ctx context.Context, fileID string, expiresIn tim
 		return signedURL, nil
 	}
 
-	// GCS (or other S3-compatible store): hand out a presigned GET URL generated
-	// with this store's own client/credentials, so clients download directly
-	// from the bucket instead of proxying the bytes through Fleet.
+	// GCS: hand out a presigned GET URL generated with this store's own
+	// client/credentials, so clients download directly from the bucket instead
+	// of proxying the bytes through Fleet.
 	if s.signedURL {
 		key := s.keyForFile(fileID)
-		// Drop the inherited APIOptions for presigning: the GCS request
-		// workarounds (ignoreSigningHeaders/disableTrailingChecksum) insert
-		// middleware relative to the "Signing" step, which doesn't exist in the
-		// presign stack, and they only matter for actual upload/download
-		// requests, not for computing a presigned GET URL.
-		presignClient := s3.NewPresignClient(s.s3Client, func(po *s3.PresignOptions) {
-			po.ClientOptions = append(po.ClientOptions, func(o *s3.Options) {
-				o.APIOptions = nil
-			})
-		})
-		req, err := presignClient.PresignGetObject(ctx, &s3.GetObjectInput{
+		req, err := s.presignClient.PresignGetObject(ctx, &s3.GetObjectInput{
 			Bucket: &s.bucket,
 			Key:    &key,
 		}, s3.WithPresignExpires(expiresIn))

server/datastore/s3/s3.go

@@ -44,6 +44,8 @@ type s3store struct {
 	// CloudFront-style signer). Gated by config and validated to require a GCS
 	// endpoint.
 	signedURL bool
+	// presignClient is built once when signedURL is enabled and reused by Sign().
+	presignClient *s3.PresignClient
 }
 
 type installerNotFoundError struct{}
@@ -72,6 +74,12 @@ func newS3Store(cfg config.S3ConfigInternal) (*s3store, error) {
 		return nil, errors.New("software installers signed URL cannot be combined with gcs iam auth; configure HMAC credentials (access key/secret) for presigning")
 	}
 
+	// An STS assume-role provider likewise replaces the HMAC credentials with
+	// temporary AWS credentials GCS can't verify, so reject that combination too.
+	if cfg.SignedURL && cfg.StsAssumeRoleArn != "" {
+		return nil, errors.New("software installers signed URL cannot be combined with sts assume role; configure HMAC credentials (access key/secret) for presigning")
+	}
+
 	if cfg.GCSIAMAuth {
 		switch {
 		case cfg.EndpointURL == "":
@@ -184,6 +192,19 @@ func newS3Store(cfg config.S3ConfigInternal) (*s3store, error) {
 		}
 	})
 
+	// Build the presign client once and reuse it in Sign(). Clear the inherited
+	// APIOptions: the GCS workarounds (ignoreSigningHeaders, disableTrailingChecksum)
+	// insert middleware at the "Signing" step, which the presign stack lacks, and
+	// they only matter for real upload/download requests.
+	var presignClient *s3.PresignClient
+	if cfg.SignedURL {
+		presignClient = s3.NewPresignClient(s3Client, func(po *s3.PresignOptions) {
+			po.ClientOptions = append(po.ClientOptions, func(o *s3.Options) {
+				o.APIOptions = nil
+			})
+		})
+	}
+
 	return &s3store{
 		s3Client:         s3Client,
 		bucket:           cfg.Bucket,
@@ -191,6 +212,7 @@ func newS3Store(cfg config.S3ConfigInternal) (*s3store, error) {
 		cloudFrontConfig: cfg.CloudFrontConfig,
 		gcs:              gcsEndpoint,
 		signedURL:        cfg.SignedURL,
+		presignClient:    presignClient,
 	}, nil
 }
 

server/datastore/s3/signed_url_test.go

@@ -2,8 +2,7 @@ package s3
 
 import (
 	"context"
-	"errors"
-	"strings"
+	"net/url"
 	"testing"
 	"time"
 
@@ -36,11 +35,21 @@ func TestSignGCSPresignedURL(t *testing.T) {
 
 		signed, err := store.Sign(context.Background(), "abc123", 15*time.Minute)
 		require.NoError(t, err)
-		require.Contains(t, signed, "storage.googleapis.com")
-		require.Contains(t, signed, "test-bucket")
+
+		u, err := url.Parse(signed)
+		require.NoError(t, err)
+		require.Equal(t, "https", u.Scheme)
+		require.Equal(t, "storage.googleapis.com", u.Host)
+		// Path-style addressing puts the bucket and key in the path.
+		require.Contains(t, u.Path, "test-bucket")
+		require.Contains(t, u.Path, "abc123")
+
+		q := u.Query()
 		require.True(t,
-			strings.Contains(signed, "X-Amz-Signature") || strings.Contains(signed, "X-Goog-Signature"),
+			q.Get("X-Amz-Signature") != "" || q.Get("X-Goog-Signature") != "",
 			"expected a presigned signature query param, got %s", signed)
+		require.NotEmpty(t, q.Get("X-Amz-Algorithm"))
+		require.Equal(t, "900", q.Get("X-Amz-Expires")) // 15 minutes
 	})
 
 	t.Run("signed url disabled and no cloudfront returns ErrNotConfigured", func(t *testing.T) {
@@ -48,7 +57,7 @@ func TestSignGCSPresignedURL(t *testing.T) {
 		require.NoError(t, err)
 
 		_, err = store.Sign(context.Background(), "abc123", 15*time.Minute)
-		require.True(t, errors.Is(err, fleet.ErrNotConfigured), "expected ErrNotConfigured, got %v", err)
+		require.ErrorIs(t, err, fleet.ErrNotConfigured)
 	})
 
 	t.Run("signed url with gcs iam auth is rejected", func(t *testing.T) {
@@ -59,6 +68,17 @@ func TestSignGCSPresignedURL(t *testing.T) {
 		cfg.SoftwareInstallersGCSIAMAuth = true
 
 		_, err := NewSoftwareInstallerStore(cfg)
-		require.Error(t, err)
+		require.ErrorContains(t, err, "gcs iam auth")
+	})
+
+	t.Run("signed url with sts assume role is rejected", func(t *testing.T) {
+		// STS assume-role swaps the HMAC credentials presigning needs for
+		// temporary AWS credentials GCS can't verify, so store init must fail.
+		cfg := baseCfg()
+		cfg.SoftwareInstallersSignedURL = true
+		cfg.SoftwareInstallersStsAssumeRoleArn = "arn:aws:iam::123456789012:role/test"
+
+		_, err := NewSoftwareInstallerStore(cfg)
+		require.ErrorContains(t, err, "sts assume role")
 	})
 }