From ffa55bbb07e795e9ccadfc456ba34b7a84a813b6 Mon Sep 17 00:00:00 2001 From: iremcaginyurtturk Date: Tue, 8 Sep 2026 17:02:15 +0300 Subject: [PATCH 1/6] fix(catalog/rest): sign SigV4 with credentials from catalog properties The REST catalog's SigV4 signer only used the AWS default credential chain (config.LoadDefaultConfig), ignoring the s3.* credential properties passed to the catalog. This forced callers targeting AWS SigV4 REST catalogs (S3 Tables, Glue) to export AWS_* environment variables even when they supplied credentials via catalog properties. Build a static credentials provider from s3.access-key-id / s3.secret-access-key / s3.session-token when present, and only fall back to the default chain otherwise. An explicitly provided aws.Config (WithAwsConfig) still wins. This mirrors the same gap tracked for the Python client in apache/iceberg-python#2070. Signed-off-by: iremcaginyurtturk --- catalog/rest/rest.go | 17 +++++++++++++++++ catalog/rest/rest_internal_test.go | 21 +++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/catalog/rest/rest.go b/catalog/rest/rest.go index 3ab7906a4..7bca174cd 100644 --- a/catalog/rest/rest.go +++ b/catalog/rest/rest.go @@ -49,6 +49,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4" "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/credentials" "golang.org/x/oauth2" "golang.org/x/oauth2/clientcredentials" "golang.org/x/sync/semaphore" @@ -1114,6 +1115,11 @@ func (r *Catalog) createSession(ctx context.Context, opts *options) (*http.Clien return nil, nil, err } + // Sign with the S3 credentials carried in the catalog properties when + // present, rather than only the AWS default credential chain. + if creds, ok := staticCredsFromProps(opts.additionalProps); ok { + cfg.Credentials = creds + } } if opts.sigv4Region != "" { cfg.Region = opts.sigv4Region @@ -1126,6 +1132,17 @@ func (r *Catalog) createSession(ctx context.Context, opts *options) (*http.Clien return cl, cleanup, nil } +// staticCredsFromProps returns a static credentials provider built from the S3 +// access-key properties, or ok=false when no key pair is present. +func staticCredsFromProps(props iceberg.Properties) (aws.CredentialsProvider, bool) { + accessKey, secretKey := props[iceio.S3AccessKeyID], props[iceio.S3SecretAccessKey] + if accessKey == "" || secretKey == "" { + return nil, false + } + + return credentials.NewStaticCredentialsProvider(accessKey, secretKey, props[iceio.S3SessionToken]), true +} + func (r *Catalog) fetchConfig(ctx context.Context, opts *options) (*options, error) { params := url.Values{} if opts.warehouseLocation != "" { diff --git a/catalog/rest/rest_internal_test.go b/catalog/rest/rest_internal_test.go index 880fa1e65..38b73e85a 100644 --- a/catalog/rest/rest_internal_test.go +++ b/catalog/rest/rest_internal_test.go @@ -42,6 +42,7 @@ import ( "github.com/apache/iceberg-go" "github.com/apache/iceberg-go/catalog" + iceio "github.com/apache/iceberg-go/io" "github.com/apache/iceberg-go/table" "github.com/aws/aws-sdk-go-v2/aws" v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4" @@ -52,6 +53,26 @@ import ( "golang.org/x/sync/errgroup" ) +func TestStaticCredsFromProps(t *testing.T) { + creds, ok := staticCredsFromProps(iceberg.Properties{ + iceio.S3AccessKeyID: "AK", + iceio.S3SecretAccessKey: "SK", + iceio.S3SessionToken: "ST", + }) + require.True(t, ok) + got, err := creds.Retrieve(context.Background()) + require.NoError(t, err) + require.Equal(t, "AK", got.AccessKeyID) + require.Equal(t, "SK", got.SecretAccessKey) + require.Equal(t, "ST", got.SessionToken) + + _, ok = staticCredsFromProps(iceberg.Properties{iceio.S3AccessKeyID: "AK"}) + require.False(t, ok, "a lone access key must not produce a provider") + + _, ok = staticCredsFromProps(iceberg.Properties{}) + require.False(t, ok, "no creds must not produce a provider") +} + func TestSplitIdentForPathRequiresNamespaceAndName(t *testing.T) { cat := &Catalog{} From cca7c834f73183fa51d6780c9686985edd7aae2a Mon Sep 17 00:00:00 2001 From: iremcaginyurtturk Date: Wed, 9 Sep 2026 09:28:33 +0300 Subject: [PATCH 2/6] test(catalog/rest): pin SigV4 props-cred signing; error on partial creds Address review feedback: - Add TestSigV4SignsWithPropsCredentials, which asserts the SigV4 Authorization header is signed with the credentials from catalog properties. This pins the wiring: removing the cfg.Credentials assignment makes the test fail. - staticCredsFromProps now returns an error when only one of the access-key / secret-access-key pair is set, instead of silently falling back to the ambient default identity. Neither set still falls back to the default chain. Signed-off-by: iremcaginyurtturk --- catalog/rest/rest.go | 26 +++++++++++----- catalog/rest/rest_internal_test.go | 49 ++++++++++++++++++++++++++---- 2 files changed, 61 insertions(+), 14 deletions(-) diff --git a/catalog/rest/rest.go b/catalog/rest/rest.go index 7bca174cd..02bda05e0 100644 --- a/catalog/rest/rest.go +++ b/catalog/rest/rest.go @@ -1107,8 +1107,13 @@ func (r *Catalog) createSession(ctx context.Context, opts *options) (*http.Clien if opts.enableSigv4 { cfg := opts.awsConfig if !opts.awsConfigSet { + creds, err := staticCredsFromProps(opts.additionalProps) + if err != nil { + cleanup() + + return nil, nil, err + } // If no config provided, load defaults from environment. - var err error cfg, err = config.LoadDefaultConfig(ctx) if err != nil { cleanup() @@ -1117,7 +1122,7 @@ func (r *Catalog) createSession(ctx context.Context, opts *options) (*http.Clien } // Sign with the S3 credentials carried in the catalog properties when // present, rather than only the AWS default credential chain. - if creds, ok := staticCredsFromProps(opts.additionalProps); ok { + if creds != nil { cfg.Credentials = creds } } @@ -1133,14 +1138,19 @@ func (r *Catalog) createSession(ctx context.Context, opts *options) (*http.Clien } // staticCredsFromProps returns a static credentials provider built from the S3 -// access-key properties, or ok=false when no key pair is present. -func staticCredsFromProps(props iceberg.Properties) (aws.CredentialsProvider, bool) { +// access-key properties. It returns (nil, nil) when neither key is set, so the +// caller falls back to the default credential chain, and an error when only one +// of the pair is set rather than silently signing as a different identity. +func staticCredsFromProps(props iceberg.Properties) (aws.CredentialsProvider, error) { accessKey, secretKey := props[iceio.S3AccessKeyID], props[iceio.S3SecretAccessKey] - if accessKey == "" || secretKey == "" { - return nil, false + switch { + case accessKey == "" && secretKey == "": + return nil, nil + case accessKey == "" || secretKey == "": + return nil, fmt.Errorf("rest: incomplete S3 credentials: both %s and %s are required for SigV4 signing", iceio.S3AccessKeyID, iceio.S3SecretAccessKey) + default: + return credentials.NewStaticCredentialsProvider(accessKey, secretKey, props[iceio.S3SessionToken]), nil } - - return credentials.NewStaticCredentialsProvider(accessKey, secretKey, props[iceio.S3SessionToken]), true } func (r *Catalog) fetchConfig(ctx context.Context, opts *options) (*options, error) { diff --git a/catalog/rest/rest_internal_test.go b/catalog/rest/rest_internal_test.go index 38b73e85a..48adea3f8 100644 --- a/catalog/rest/rest_internal_test.go +++ b/catalog/rest/rest_internal_test.go @@ -54,23 +54,60 @@ import ( ) func TestStaticCredsFromProps(t *testing.T) { - creds, ok := staticCredsFromProps(iceberg.Properties{ + creds, err := staticCredsFromProps(iceberg.Properties{ iceio.S3AccessKeyID: "AK", iceio.S3SecretAccessKey: "SK", iceio.S3SessionToken: "ST", }) - require.True(t, ok) + require.NoError(t, err) + require.NotNil(t, creds) got, err := creds.Retrieve(context.Background()) require.NoError(t, err) require.Equal(t, "AK", got.AccessKeyID) require.Equal(t, "SK", got.SecretAccessKey) require.Equal(t, "ST", got.SessionToken) - _, ok = staticCredsFromProps(iceberg.Properties{iceio.S3AccessKeyID: "AK"}) - require.False(t, ok, "a lone access key must not produce a provider") + creds, err = staticCredsFromProps(iceberg.Properties{}) + require.NoError(t, err, "no creds must fall back to the default chain") + require.Nil(t, creds) + + _, err = staticCredsFromProps(iceberg.Properties{iceio.S3AccessKeyID: "AK"}) + require.Error(t, err, "a lone access key must be an error, not the ambient identity") + + _, err = staticCredsFromProps(iceberg.Properties{iceio.S3SecretAccessKey: "SK"}) + require.Error(t, err, "a lone secret key must be an error, not the ambient identity") +} + +// TestSigV4SignsWithPropsCredentials pins the wiring: the SigV4 Authorization +// header must be signed with the credentials carried in the catalog properties. +func TestSigV4SignsWithPropsCredentials(t *testing.T) { + var authHeader string + mux := http.NewServeMux() + mux.HandleFunc("/v1/config", func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{"defaults": map[string]any{}, "overrides": map[string]any{}}) + }) + mux.HandleFunc("/test", func(w http.ResponseWriter, r *http.Request) { + authHeader = r.Header.Get("Authorization") + w.WriteHeader(http.StatusOK) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + cat, err := NewCatalog(context.Background(), "rest", srv.URL, + WithSigV4RegionSvc("us-east-1", "s3"), + WithAdditionalProps(iceberg.Properties{ + iceio.S3AccessKeyID: "AKIDEXAMPLEPROPS", + iceio.S3SecretAccessKey: "secretexample", + })) + require.NoError(t, err) + + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, srv.URL+"/test", nil) + require.NoError(t, err) + _, err = cat.cl.Do(req) + require.NoError(t, err) - _, ok = staticCredsFromProps(iceberg.Properties{}) - require.False(t, ok, "no creds must not produce a provider") + require.Contains(t, authHeader, "Credential=AKIDEXAMPLEPROPS/", + "SigV4 must sign with the credentials from catalog properties, not the default chain") } func TestSplitIdentForPathRequiresNamespaceAndName(t *testing.T) { From 48164f570c4e30b19ae6504dd43095d2213419d0 Mon Sep 17 00:00:00 2001 From: iremcaginyurtturk Date: Wed, 9 Sep 2026 09:34:33 +0300 Subject: [PATCH 3/6] docs(catalog/rest): document SigV4 signing credential precedence Address review feedback (minor): the SigV4 signing identity resolves as WithAwsConfig > s3.* catalog properties > AWS default credential chain. Document this on WithSigV4 and in the rest.sigv4-enabled configuration reference. Signed-off-by: iremcaginyurtturk --- catalog/rest/options.go | 4 ++++ website/src/configuration.md | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/catalog/rest/options.go b/catalog/rest/options.go index c52d48222..08a656386 100644 --- a/catalog/rest/options.go +++ b/catalog/rest/options.go @@ -88,6 +88,10 @@ func WithMetadataLocation(loc string) Option { } } +// WithSigV4 enables AWS SigV4 request signing for the REST catalog. The signing +// identity is resolved in order: an explicit WithAwsConfig, then the s3.* catalog +// credential properties (s3.access-key-id / s3.secret-access-key / s3.session-token), +// then the AWS default credential chain. func WithSigV4() Option { return func(o *options) { o.enableSigv4 = true diff --git a/website/src/configuration.md b/website/src/configuration.md index 7335c8a08..d72aa5e4b 100644 --- a/website/src/configuration.md +++ b/website/src/configuration.md @@ -56,7 +56,7 @@ catalog: | `catalog..aws-profile` | AWS named profile for the Glue catalog. When unset, the AWS SDK default credential chain is used. | | `catalog..sql-driver` | `database/sql` driver name for the SQL catalog. Maps to the `sql.driver` property. The default CLI binary only compiles in `sqliteshim`; other drivers require a custom build. | | `catalog..sql-dialect` | SQL dialect for the SQL catalog (`postgres`, `mysql`, `sqlite`, `mssql`, `oracle`). Maps to the `sql.dialect` property. The default CLI binary only ships `sqlite` via `sqliteshim`; other dialects need a custom build with their drivers. | -| `catalog..rest.sigv4-enabled` | Enable AWS SigV4 signing for REST. | +| `catalog..rest.sigv4-enabled` | Enable AWS SigV4 signing for REST. When enabled, requests are signed with the `s3.*` credential properties if set (`s3.access-key-id` / `s3.secret-access-key` / `s3.session-token`), otherwise with the AWS default credential chain. | | `catalog..rest.signing-name` | SigV4 service name. | | `catalog..rest.signing-region` | SigV4 region. | From daa7aa3ad8aac05b21f0c81cd360c70df13a4e7f Mon Sep 17 00:00:00 2001 From: iremcaginyurtturk Date: Mon, 14 Sep 2026 09:19:12 +0300 Subject: [PATCH 4/6] fix(catalog/rest): error on incomplete SigV4 creds via ValidateStaticCredentials Delegate staticCredsFromProps to internal/awsconfig.ValidateStaticCredentials so a token-only s3.session-token (no access/secret key) returns the ErrIncompleteStaticCredentials sentinel instead of silently dropping to the default credential chain. Add a token-only test case and assert the sentinel via errors.Is. Document the s3.* vs Java rest.* divergence in the WithSigV4 godoc and configuration.md. --- catalog/rest/options.go | 4 ++++ catalog/rest/rest.go | 22 ++++++++++++---------- catalog/rest/rest_internal_test.go | 8 ++++++-- website/src/configuration.md | 2 +- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/catalog/rest/options.go b/catalog/rest/options.go index 08a656386..32f9a3c34 100644 --- a/catalog/rest/options.go +++ b/catalog/rest/options.go @@ -92,6 +92,10 @@ func WithMetadataLocation(loc string) Option { // identity is resolved in order: an explicit WithAwsConfig, then the s3.* catalog // credential properties (s3.access-key-id / s3.secret-access-key / s3.session-token), // then the AWS default credential chain. +// +// Note: unlike the Java client, credentials are read from s3.* rather than +// rest.access-key-id / rest.secret-access-key. Setting only the rest.* keys +// falls through to the AWS default credential chain. func WithSigV4() Option { return func(o *options) { o.enableSigv4 = true diff --git a/catalog/rest/rest.go b/catalog/rest/rest.go index 02bda05e0..858bb5865 100644 --- a/catalog/rest/rest.go +++ b/catalog/rest/rest.go @@ -41,6 +41,7 @@ import ( "github.com/apache/iceberg-go" "github.com/apache/iceberg-go/catalog" + internalaws "github.com/apache/iceberg-go/internal/awsconfig" iceio "github.com/apache/iceberg-go/io" "github.com/apache/iceberg-go/metrics" "github.com/apache/iceberg-go/table" @@ -1138,19 +1139,20 @@ func (r *Catalog) createSession(ctx context.Context, opts *options) (*http.Clien } // staticCredsFromProps returns a static credentials provider built from the S3 -// access-key properties. It returns (nil, nil) when neither key is set, so the -// caller falls back to the default credential chain, and an error when only one -// of the pair is set rather than silently signing as a different identity. +// access-key properties. It returns (nil, nil) when no credential property is +// set, so the caller falls back to the default credential chain, and an +// ErrIncompleteStaticCredentials error when the properties form an incomplete +// pair rather than silently signing as a different identity. func staticCredsFromProps(props iceberg.Properties) (aws.CredentialsProvider, error) { - accessKey, secretKey := props[iceio.S3AccessKeyID], props[iceio.S3SecretAccessKey] - switch { - case accessKey == "" && secretKey == "": + accessKey, secretKey, token := props[iceio.S3AccessKeyID], props[iceio.S3SecretAccessKey], props[iceio.S3SessionToken] + if accessKey == "" && secretKey == "" && token == "" { return nil, nil - case accessKey == "" || secretKey == "": - return nil, fmt.Errorf("rest: incomplete S3 credentials: both %s and %s are required for SigV4 signing", iceio.S3AccessKeyID, iceio.S3SecretAccessKey) - default: - return credentials.NewStaticCredentialsProvider(accessKey, secretKey, props[iceio.S3SessionToken]), nil } + if err := internalaws.ValidateStaticCredentials(iceio.S3AccessKeyID, iceio.S3SecretAccessKey, iceio.S3SessionToken, accessKey, secretKey, token); err != nil { + return nil, err + } + + return credentials.NewStaticCredentialsProvider(accessKey, secretKey, token), nil } func (r *Catalog) fetchConfig(ctx context.Context, opts *options) (*options, error) { diff --git a/catalog/rest/rest_internal_test.go b/catalog/rest/rest_internal_test.go index 48adea3f8..8236c1618 100644 --- a/catalog/rest/rest_internal_test.go +++ b/catalog/rest/rest_internal_test.go @@ -42,6 +42,7 @@ import ( "github.com/apache/iceberg-go" "github.com/apache/iceberg-go/catalog" + internalaws "github.com/apache/iceberg-go/internal/awsconfig" iceio "github.com/apache/iceberg-go/io" "github.com/apache/iceberg-go/table" "github.com/aws/aws-sdk-go-v2/aws" @@ -72,10 +73,13 @@ func TestStaticCredsFromProps(t *testing.T) { require.Nil(t, creds) _, err = staticCredsFromProps(iceberg.Properties{iceio.S3AccessKeyID: "AK"}) - require.Error(t, err, "a lone access key must be an error, not the ambient identity") + require.ErrorIs(t, err, internalaws.ErrIncompleteStaticCredentials, "a lone access key must be an error, not the ambient identity") _, err = staticCredsFromProps(iceberg.Properties{iceio.S3SecretAccessKey: "SK"}) - require.Error(t, err, "a lone secret key must be an error, not the ambient identity") + require.ErrorIs(t, err, internalaws.ErrIncompleteStaticCredentials, "a lone secret key must be an error, not the ambient identity") + + _, err = staticCredsFromProps(iceberg.Properties{iceio.S3SessionToken: "ST"}) + require.ErrorIs(t, err, internalaws.ErrIncompleteStaticCredentials, "a lone session token must be an error, not the ambient identity") } // TestSigV4SignsWithPropsCredentials pins the wiring: the SigV4 Authorization diff --git a/website/src/configuration.md b/website/src/configuration.md index d72aa5e4b..24b891ebd 100644 --- a/website/src/configuration.md +++ b/website/src/configuration.md @@ -56,7 +56,7 @@ catalog: | `catalog..aws-profile` | AWS named profile for the Glue catalog. When unset, the AWS SDK default credential chain is used. | | `catalog..sql-driver` | `database/sql` driver name for the SQL catalog. Maps to the `sql.driver` property. The default CLI binary only compiles in `sqliteshim`; other drivers require a custom build. | | `catalog..sql-dialect` | SQL dialect for the SQL catalog (`postgres`, `mysql`, `sqlite`, `mssql`, `oracle`). Maps to the `sql.dialect` property. The default CLI binary only ships `sqlite` via `sqliteshim`; other dialects need a custom build with their drivers. | -| `catalog..rest.sigv4-enabled` | Enable AWS SigV4 signing for REST. When enabled, requests are signed with the `s3.*` credential properties if set (`s3.access-key-id` / `s3.secret-access-key` / `s3.session-token`), otherwise with the AWS default credential chain. | +| `catalog..rest.sigv4-enabled` | Enable AWS SigV4 signing for REST. When enabled, requests are signed with the `s3.*` credential properties if set (`s3.access-key-id` / `s3.secret-access-key` / `s3.session-token`), otherwise with the AWS default credential chain. Unlike the Java client, credentials are read from `s3.*` rather than `rest.access-key-id` / `rest.secret-access-key`; setting only the `rest.*` keys falls through to the default chain. | | `catalog..rest.signing-name` | SigV4 service name. | | `catalog..rest.signing-region` | SigV4 region. | From 1b370795321a2c5f22be453ac6f0f0eca7138a4a Mon Sep 17 00:00:00 2001 From: iremcaginyurtturk Date: Mon, 14 Sep 2026 09:21:27 +0300 Subject: [PATCH 5/6] feat(catalog/rest): accept Java rest.* SigV4 credential aliases Accept rest.access-key-id / rest.secret-access-key / rest.session-token as aliases for the s3.* signing-credential properties, resolved per field with the s3.* keys taking precedence. This lets operators migrating from the Java client configure SigV4 signing with the property names they already use instead of silently falling through to the AWS default credential chain. --- catalog/rest/options.go | 6 +++--- catalog/rest/rest.go | 30 ++++++++++++++++++++++++------ catalog/rest/rest_internal_test.go | 28 ++++++++++++++++++++++++++++ website/src/configuration.md | 2 +- 4 files changed, 56 insertions(+), 10 deletions(-) diff --git a/catalog/rest/options.go b/catalog/rest/options.go index 32f9a3c34..bf25b499d 100644 --- a/catalog/rest/options.go +++ b/catalog/rest/options.go @@ -93,9 +93,9 @@ func WithMetadataLocation(loc string) Option { // credential properties (s3.access-key-id / s3.secret-access-key / s3.session-token), // then the AWS default credential chain. // -// Note: unlike the Java client, credentials are read from s3.* rather than -// rest.access-key-id / rest.secret-access-key. Setting only the rest.* keys -// falls through to the AWS default credential chain. +// The Java-client property names (rest.access-key-id / rest.secret-access-key / +// rest.session-token) are accepted as aliases, resolved per field with the s3.* +// keys taking precedence when both are set. func WithSigV4() Option { return func(o *options) { o.enableSigv4 = true diff --git a/catalog/rest/rest.go b/catalog/rest/rest.go index 858bb5865..393bcac28 100644 --- a/catalog/rest/rest.go +++ b/catalog/rest/rest.go @@ -94,6 +94,12 @@ const ( keyRestSigV4Region = "rest.signing-region" keyRestSigV4Service = "rest.signing-name" keyAuthUrl = "rest.authorization-url" + // keyRestAccessKeyID and friends are the Java-client property names for the + // SigV4 signing credentials. They are accepted as aliases for the s3.* + // properties; the s3.* keys take precedence when both are set. + keyRestAccessKeyID = "rest.access-key-id" + keyRestSecretAccessKey = "rest.secret-access-key" + keyRestSessionToken = "rest.session-token" // keyOAuth2ServerURI is the portable, spec-aligned property for the OAuth2 // token endpoint used by Java, PyIceberg and iceberg-rust. It is the // preferred key; keyAuthUrl is retained as a compatibility alias. When both @@ -1138,13 +1144,25 @@ func (r *Catalog) createSession(ctx context.Context, opts *options) (*http.Clien return cl, cleanup, nil } -// staticCredsFromProps returns a static credentials provider built from the S3 -// access-key properties. It returns (nil, nil) when no credential property is -// set, so the caller falls back to the default credential chain, and an -// ErrIncompleteStaticCredentials error when the properties form an incomplete -// pair rather than silently signing as a different identity. +// staticCredsFromProps returns a static credentials provider built from the +// signing-credential properties. It reads the s3.* keys, falling back to the +// Java-compatible rest.* aliases per field. It returns (nil, nil) when no +// credential property is set, so the caller falls back to the default credential +// chain, and an ErrIncompleteStaticCredentials error when the properties form an +// incomplete pair rather than silently signing as a different identity. func staticCredsFromProps(props iceberg.Properties) (aws.CredentialsProvider, error) { - accessKey, secretKey, token := props[iceio.S3AccessKeyID], props[iceio.S3SecretAccessKey], props[iceio.S3SessionToken] + firstNonEmpty := func(keys ...string) string { + for _, k := range keys { + if v := props[k]; v != "" { + return v + } + } + + return "" + } + accessKey := firstNonEmpty(iceio.S3AccessKeyID, keyRestAccessKeyID) + secretKey := firstNonEmpty(iceio.S3SecretAccessKey, keyRestSecretAccessKey) + token := firstNonEmpty(iceio.S3SessionToken, keyRestSessionToken) if accessKey == "" && secretKey == "" && token == "" { return nil, nil } diff --git a/catalog/rest/rest_internal_test.go b/catalog/rest/rest_internal_test.go index 8236c1618..a6bf42392 100644 --- a/catalog/rest/rest_internal_test.go +++ b/catalog/rest/rest_internal_test.go @@ -80,6 +80,34 @@ func TestStaticCredsFromProps(t *testing.T) { _, err = staticCredsFromProps(iceberg.Properties{iceio.S3SessionToken: "ST"}) require.ErrorIs(t, err, internalaws.ErrIncompleteStaticCredentials, "a lone session token must be an error, not the ambient identity") + + creds, err = staticCredsFromProps(iceberg.Properties{ + keyRestAccessKeyID: "RAK", + keyRestSecretAccessKey: "RSK", + keyRestSessionToken: "RST", + }) + require.NoError(t, err) + require.NotNil(t, creds) + got, err = creds.Retrieve(context.Background()) + require.NoError(t, err) + require.Equal(t, "RAK", got.AccessKeyID) + require.Equal(t, "RSK", got.SecretAccessKey) + require.Equal(t, "RST", got.SessionToken) + + creds, err = staticCredsFromProps(iceberg.Properties{ + iceio.S3AccessKeyID: "AK", + iceio.S3SecretAccessKey: "SK", + keyRestAccessKeyID: "RAK", + keyRestSecretAccessKey: "RSK", + }) + require.NoError(t, err) + got, err = creds.Retrieve(context.Background()) + require.NoError(t, err) + require.Equal(t, "AK", got.AccessKeyID, "s3.* keys take precedence over rest.* aliases") + require.Equal(t, "SK", got.SecretAccessKey) + + _, err = staticCredsFromProps(iceberg.Properties{keyRestAccessKeyID: "RAK"}) + require.ErrorIs(t, err, internalaws.ErrIncompleteStaticCredentials, "a lone rest.* access key must be an error") } // TestSigV4SignsWithPropsCredentials pins the wiring: the SigV4 Authorization diff --git a/website/src/configuration.md b/website/src/configuration.md index 24b891ebd..9546cee55 100644 --- a/website/src/configuration.md +++ b/website/src/configuration.md @@ -56,7 +56,7 @@ catalog: | `catalog..aws-profile` | AWS named profile for the Glue catalog. When unset, the AWS SDK default credential chain is used. | | `catalog..sql-driver` | `database/sql` driver name for the SQL catalog. Maps to the `sql.driver` property. The default CLI binary only compiles in `sqliteshim`; other drivers require a custom build. | | `catalog..sql-dialect` | SQL dialect for the SQL catalog (`postgres`, `mysql`, `sqlite`, `mssql`, `oracle`). Maps to the `sql.dialect` property. The default CLI binary only ships `sqlite` via `sqliteshim`; other dialects need a custom build with their drivers. | -| `catalog..rest.sigv4-enabled` | Enable AWS SigV4 signing for REST. When enabled, requests are signed with the `s3.*` credential properties if set (`s3.access-key-id` / `s3.secret-access-key` / `s3.session-token`), otherwise with the AWS default credential chain. Unlike the Java client, credentials are read from `s3.*` rather than `rest.access-key-id` / `rest.secret-access-key`; setting only the `rest.*` keys falls through to the default chain. | +| `catalog..rest.sigv4-enabled` | Enable AWS SigV4 signing for REST. When enabled, requests are signed with the `s3.*` credential properties if set (`s3.access-key-id` / `s3.secret-access-key` / `s3.session-token`), otherwise with the AWS default credential chain. The Java-client names (`rest.access-key-id` / `rest.secret-access-key` / `rest.session-token`) are accepted as aliases, resolved per field with the `s3.*` keys taking precedence. | | `catalog..rest.signing-name` | SigV4 service name. | | `catalog..rest.signing-region` | SigV4 region. | From d401049bb807fb154a0e5c3be86697bf4074a7f3 Mon Sep 17 00:00:00 2001 From: iremcaginyurtturk Date: Tue, 22 Sep 2026 10:03:52 +0300 Subject: [PATCH 6/6] fix(catalog/rest): harden SigV4 credential boundaries Two credential-boundary fixes for the props-based SigV4 signing path: - Resolve the credential tuple atomically per namespace. Resolving each field independently let a partial s3.* pair be completed with a rest.* field (or inherit an unrelated rest.* token), signing as a hybrid identity that still passed validation. Now if any s3.* field is set we validate and use only the s3.* tuple, otherwise only the rest.* tuple; never backfill across namespaces. - Do not re-sign a cross-origin redirect. sessionTransport signed every hop, so a redirect to a different origin exposed the SigV4 Authorization header and session token to an unconfigured host. Signing is now skipped when the request origin (scheme/host/effective port) differs from the configured catalog origin. Tests cover the mixed-partial and s3-pair-plus-rest-token cases and assert a cross-origin redirect target receives neither the Authorization header nor the session token. Also closes a response body left open in an existing test. Signed-off-by: iremcaginyurtturk --- catalog/rest/rest.go | 68 ++++++++++++++++++++---------- catalog/rest/rest_internal_test.go | 67 ++++++++++++++++++++++++++++- 2 files changed, 112 insertions(+), 23 deletions(-) diff --git a/catalog/rest/rest.go b/catalog/rest/rest.go index 393bcac28..c97a43945 100644 --- a/catalog/rest/rest.go +++ b/catalog/rest/rest.go @@ -250,6 +250,31 @@ type sessionTransport struct { cfg aws.Config service string newHash func() hash.Hash + // signingOrigin is the configured catalog origin. Requests to a different + // origin (e.g. a redirect hop) are not signed, so the SigV4 Authorization + // header and session token never reach an unconfigured host. + signingOrigin *url.URL +} + +// sameOrigin reports whether two URLs share scheme, host, and effective port. +func sameOrigin(a, b *url.URL) bool { + return strings.EqualFold(a.Scheme, b.Scheme) && + strings.EqualFold(a.Hostname(), b.Hostname()) && + defaultedPort(a) == defaultedPort(b) +} + +func defaultedPort(u *url.URL) string { + if p := u.Port(); p != "" { + return p + } + switch strings.ToLower(u.Scheme) { + case "https": + return "443" + case "http": + return "80" + default: + return "" + } } // from https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/aws/signer/v4#Signer.SignHTTP @@ -299,7 +324,7 @@ func (s *sessionTransport) RoundTrip(r *http.Request) (*http.Response, error) { r.Header.Set(k, v) } - if s.signer != nil { + if s.signer != nil && (s.signingOrigin == nil || sameOrigin(s.signingOrigin, r.URL)) { var payloadHash string if r.Body == nil { payloadHash = emptyStringHash @@ -1139,38 +1164,37 @@ func (r *Catalog) createSession(ctx context.Context, opts *options) (*http.Clien session.cfg, session.service = cfg, opts.sigv4Service session.signer, session.newHash = v4.NewSigner(), sha256.New + session.signingOrigin = r.baseURI } return cl, cleanup, nil } // staticCredsFromProps returns a static credentials provider built from the -// signing-credential properties. It reads the s3.* keys, falling back to the -// Java-compatible rest.* aliases per field. It returns (nil, nil) when no -// credential property is set, so the caller falls back to the default credential -// chain, and an ErrIncompleteStaticCredentials error when the properties form an -// incomplete pair rather than silently signing as a different identity. +// signing-credential properties. It prefers the s3.* keys and falls back to the +// Java-compatible rest.* aliases, resolving the tuple atomically from a single +// namespace so a partial pair is never completed with fields from the other one. +// It returns (nil, nil) when neither namespace sets any credential property, so +// the caller falls back to the default credential chain, and an +// ErrIncompleteStaticCredentials error when the chosen namespace is incomplete. func staticCredsFromProps(props iceberg.Properties) (aws.CredentialsProvider, error) { - firstNonEmpty := func(keys ...string) string { - for _, k := range keys { - if v := props[k]; v != "" { - return v - } + namespaces := [][3]string{ + {iceio.S3AccessKeyID, iceio.S3SecretAccessKey, iceio.S3SessionToken}, + {keyRestAccessKeyID, keyRestSecretAccessKey, keyRestSessionToken}, + } + for _, ns := range namespaces { + accessKey, secretKey, token := props[ns[0]], props[ns[1]], props[ns[2]] + if accessKey == "" && secretKey == "" && token == "" { + continue + } + if err := internalaws.ValidateStaticCredentials(ns[0], ns[1], ns[2], accessKey, secretKey, token); err != nil { + return nil, err } - return "" - } - accessKey := firstNonEmpty(iceio.S3AccessKeyID, keyRestAccessKeyID) - secretKey := firstNonEmpty(iceio.S3SecretAccessKey, keyRestSecretAccessKey) - token := firstNonEmpty(iceio.S3SessionToken, keyRestSessionToken) - if accessKey == "" && secretKey == "" && token == "" { - return nil, nil - } - if err := internalaws.ValidateStaticCredentials(iceio.S3AccessKeyID, iceio.S3SecretAccessKey, iceio.S3SessionToken, accessKey, secretKey, token); err != nil { - return nil, err + return credentials.NewStaticCredentialsProvider(accessKey, secretKey, token), nil } - return credentials.NewStaticCredentialsProvider(accessKey, secretKey, token), nil + return nil, nil } func (r *Catalog) fetchConfig(ctx context.Context, opts *options) (*options, error) { diff --git a/catalog/rest/rest_internal_test.go b/catalog/rest/rest_internal_test.go index a6bf42392..1018c1965 100644 --- a/catalog/rest/rest_internal_test.go +++ b/catalog/rest/rest_internal_test.go @@ -108,6 +108,23 @@ func TestStaticCredsFromProps(t *testing.T) { _, err = staticCredsFromProps(iceberg.Properties{keyRestAccessKeyID: "RAK"}) require.ErrorIs(t, err, internalaws.ErrIncompleteStaticCredentials, "a lone rest.* access key must be an error") + + _, err = staticCredsFromProps(iceberg.Properties{ + iceio.S3AccessKeyID: "AK", + keyRestSecretAccessKey: "RSK", + }) + require.ErrorIs(t, err, internalaws.ErrIncompleteStaticCredentials, "a partial pair must not be completed with a field from the other namespace") + + creds, err = staticCredsFromProps(iceberg.Properties{ + iceio.S3AccessKeyID: "AK", + iceio.S3SecretAccessKey: "SK", + keyRestSessionToken: "RST", + }) + require.NoError(t, err) + got, err = creds.Retrieve(context.Background()) + require.NoError(t, err) + require.Equal(t, "AK", got.AccessKeyID) + require.Empty(t, got.SessionToken, "a complete s3.* pair must not inherit an unrelated rest.* session token") } // TestSigV4SignsWithPropsCredentials pins the wiring: the SigV4 Authorization @@ -135,13 +152,61 @@ func TestSigV4SignsWithPropsCredentials(t *testing.T) { req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, srv.URL+"/test", nil) require.NoError(t, err) - _, err = cat.cl.Do(req) + resp, err := cat.cl.Do(req) require.NoError(t, err) + require.NoError(t, resp.Body.Close()) require.Contains(t, authHeader, "Credential=AKIDEXAMPLEPROPS/", "SigV4 must sign with the credentials from catalog properties, not the default chain") } +// TestSigv4DoesNotSignCrossOriginRedirect pins that a redirect to a different +// origin is not re-signed, so the SigV4 Authorization header and session token +// never reach an unconfigured host. +func TestSigv4DoesNotSignCrossOriginRedirect(t *testing.T) { + var secondHit bool + var gotAuth, gotToken string + second := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + secondHit = true + gotAuth = r.Header.Get("Authorization") + gotToken = r.Header.Get("X-Amz-Security-Token") + w.WriteHeader(http.StatusOK) + })) + defer second.Close() + + var firstAuth string + mux := http.NewServeMux() + mux.HandleFunc("/v1/config", func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{"defaults": map[string]any{}, "overrides": map[string]any{}}) + }) + mux.HandleFunc("/redirect", func(w http.ResponseWriter, r *http.Request) { + firstAuth = r.Header.Get("Authorization") + http.Redirect(w, r, second.URL+"/landing", http.StatusTemporaryRedirect) + }) + first := httptest.NewServer(mux) + defer first.Close() + + cat, err := NewCatalog(context.Background(), "rest", first.URL, + WithSigV4RegionSvc("us-east-1", "s3"), + WithAdditionalProps(iceberg.Properties{ + iceio.S3AccessKeyID: "AKIDEXAMPLEPROPS", + iceio.S3SecretAccessKey: "secretexample", + iceio.S3SessionToken: "SESSIONTOKENEXAMPLE", + })) + require.NoError(t, err) + + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, first.URL+"/redirect", nil) + require.NoError(t, err) + resp, err := cat.cl.Do(req) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + + require.Contains(t, firstAuth, "Credential=AKIDEXAMPLEPROPS/", "the configured origin must still be signed") + require.True(t, secondHit, "the redirect target must be reached") + require.Empty(t, gotAuth, "the redirect target must not receive the SigV4 Authorization header") + require.Empty(t, gotToken, "the redirect target must not receive the session token") +} + func TestSplitIdentForPathRequiresNamespaceAndName(t *testing.T) { cat := &Catalog{}