mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-05-21 01:01:29 +00:00
fix(s3): stop S3 Tables routes from swallowing buckets named "buckets" or "get-table" (#9566)
* fix(s3): stop S3 Tables routes from swallowing buckets named "buckets" or "get-table"
The S3 Tables REST endpoints share top-level paths with the regular S3
API (/buckets for ListTableBuckets/CreateTableBucket, /get-table for
GetTable). They are registered first on the same router as the bucket
subrouter, so a path-style request such as GET /buckets?list-type=2 on
a bucket actually named "buckets" matched ListTableBuckets and returned
JSON. AWS SDK V2 (and Hadoop s3a / Spark) then failed XML parsing with
"Unexpected character '{' (code 123) in prolog".
Disambiguate by requiring the AWS V4 credential scope to name the
s3tables service on the colliding routes. Regular S3 SDKs sign with
service=s3, S3 Tables SDKs sign with service=s3tables, and the scope is
present in both the Authorization header and the X-Amz-Credential query
parameter for presigned URLs, so the matcher works for both flavors.
ARN-bearing S3 Tables routes (/buckets/<arn>, /namespaces/<arn>, etc.)
already cannot collide because colons are not valid in bucket names, so
they are left untouched.
* fix(s3): accept AWS JSON RPC content type as S3 Tables intent signal
The Iceberg catalog integration tests send unsigned PUT /buckets with
Content-Type: application/x-amz-json-1.1 to create table buckets. With
only the credential-scope check, those requests fell through to the
regular S3 CreateBucket handler and the suite went red on this branch.
Extend the matcher so a request is recognized as S3 Tables when either:
- its AWS V4 credential scope names SERVICE=s3tables; or
- it carries the canonical AWS JSON RPC 1.1 content type and is
unsigned (a request explicitly signed for SERVICE=s3 still wins).
The regular S3 SDKs do not send application/x-amz-json-1.1, so the
signal is safe for the colliding paths (/buckets, /get-table).
Also add an AWS SDK V2 for Go integration test under
test/s3/sdk_v2_routing/ that drives the SDK's own XML deserializer
against a bucket literally named "buckets" and "get-table" — the SDK
errors before the test asserts if the server returns the wrong body
shape. Wired up via .github/workflows/s3-sdk-v2-routing-tests.yml,
mirroring the etag/acl workflow.
* s3api: extend service matcher to all S3 Tables routes; simplify scope check
- Apply serviceMatcher to every S3 Tables route, not just the bare-path
ones. ARN-bearing paths could otherwise be hit by an S3 object key
that starts with arn:aws:s3tables:..., inside a bucket named
"buckets", "namespaces", "tables", or "tag". One matcher everywhere
closes both collision classes.
- Replace strings.Split + index lookup with strings.Contains for the
credential-scope check. The scope shape is fixed at
AK/DATE/REGION/SERVICE/aws4_request, slashes only delimit components,
and access keys are alphanumeric — so /s3tables/ matches iff SERVICE
is exactly s3tables. Existing unit cases (including the
access-key-substring case) still pass.
- Read the GetObject body in the SDK v2 routing test with io.ReadAll;
the single Read could return short and make the equality check flaky.
* s3api: drop content-type fallback; sign s3 tables harness traffic instead
The content-type fallback in isS3TablesSignedRequest let an anonymous
regular-S3 request whose body type is application/x-amz-json-1.1 hit
an S3 Tables route when the path-style object key happened to be
shaped like an S3 Tables ARN (e.g. PutObject on bucket "buckets"
with key arn:aws:s3tables:.../bucket/foo/policy). Narrow the matcher
back to the AWS V4 credential scope so only requests signed for
SERVICE=s3tables match the S3 Tables routes.
Update the Iceberg catalog test harness — the only caller still
sending unsigned PUT /buckets — to sign with SERVICE=s3tables. The
mini instance runs in default-allow mode, so the signature itself is
not verified; only the credential scope matters for the route match.
Drop the stale unit cases for the JSON-RPC content-type signal and
the routing test that exercised unsigned harness traffic.
This commit is contained in:
110
.github/workflows/s3-sdk-v2-routing-tests.yml
vendored
Normal file
110
.github/workflows/s3-sdk-v2-routing-tests.yml
vendored
Normal file
@@ -0,0 +1,110 @@
|
||||
name: "S3 SDK V2 Route Disambiguation Tests"
|
||||
|
||||
on:
|
||||
push:
|
||||
branches: [ master ]
|
||||
paths:
|
||||
- 'weed/s3api/**'
|
||||
- 'test/s3/sdk_v2_routing/**'
|
||||
- '.github/workflows/s3-sdk-v2-routing-tests.yml'
|
||||
pull_request:
|
||||
branches: [ master ]
|
||||
paths:
|
||||
- 'weed/s3api/**'
|
||||
- 'test/s3/sdk_v2_routing/**'
|
||||
- '.github/workflows/s3-sdk-v2-routing-tests.yml'
|
||||
|
||||
concurrency:
|
||||
group: ${{ github.head_ref || github.ref }}/s3-sdk-v2-routing-tests
|
||||
cancel-in-progress: true
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
s3-sdk-v2-routing-tests:
|
||||
name: S3 SDK V2 Routing Tests
|
||||
runs-on: ubuntu-22.04
|
||||
timeout-minutes: 10
|
||||
steps:
|
||||
- name: Check out code
|
||||
uses: actions/checkout@v6
|
||||
|
||||
- name: Set up Go
|
||||
uses: actions/setup-go@v6
|
||||
with:
|
||||
go-version-file: 'go.mod'
|
||||
|
||||
- name: Install SeaweedFS
|
||||
run: |
|
||||
cd weed && go install -buildvcs=false
|
||||
|
||||
- name: Start weed mini (S3 on :8333)
|
||||
# Pins the regression for issue #9559: AWS SDK V2 / Hadoop s3a
|
||||
# listing a bucket literally named "buckets" must get an XML
|
||||
# ListObjectsV2 response, not the JSON ListTableBuckets body
|
||||
# served by the S3 Tables REST endpoint on the same path.
|
||||
run: |
|
||||
mkdir -p /tmp/seaweedfs-sdk-v2-routing
|
||||
cat > /tmp/seaweedfs-sdk-v2-routing-s3.json <<'JSON'
|
||||
{
|
||||
"identities": [
|
||||
{
|
||||
"name": "admin",
|
||||
"credentials": [
|
||||
{"accessKey": "some_access_key1", "secretKey": "some_secret_key1"}
|
||||
],
|
||||
"actions": ["Admin", "Read", "Write"]
|
||||
}
|
||||
]
|
||||
}
|
||||
JSON
|
||||
AWS_ACCESS_KEY_ID=some_access_key1 \
|
||||
AWS_SECRET_ACCESS_KEY=some_secret_key1 \
|
||||
weed mini \
|
||||
-dir=/tmp/seaweedfs-sdk-v2-routing \
|
||||
-s3.port=8333 \
|
||||
-s3.config=/tmp/seaweedfs-sdk-v2-routing-s3.json \
|
||||
-ip=127.0.0.1 \
|
||||
> /tmp/weed-mini.log 2>&1 &
|
||||
echo $! > /tmp/weed-mini.pid
|
||||
|
||||
for i in $(seq 1 30); do
|
||||
if curl -s -o /dev/null -w "%{http_code}" http://127.0.0.1:8333/ | grep -qE "^(200|403)$"; then
|
||||
echo "weed mini is ready"
|
||||
exit 0
|
||||
fi
|
||||
sleep 1
|
||||
done
|
||||
echo "weed mini failed to start within 30s"
|
||||
tail -50 /tmp/weed-mini.log
|
||||
exit 1
|
||||
|
||||
- name: Run SDK V2 routing tests
|
||||
env:
|
||||
S3_ENDPOINT: http://127.0.0.1:8333
|
||||
AWS_ACCESS_KEY_ID: some_access_key1
|
||||
AWS_SECRET_ACCESS_KEY: some_secret_key1
|
||||
AWS_REGION: us-east-1
|
||||
run: go test -v -timeout=5m ./test/s3/sdk_v2_routing/...
|
||||
|
||||
- name: Stop weed mini
|
||||
if: always()
|
||||
run: |
|
||||
if [ -f /tmp/weed-mini.pid ]; then
|
||||
kill "$(cat /tmp/weed-mini.pid)" 2>/dev/null || true
|
||||
fi
|
||||
|
||||
- name: Show server log on failure
|
||||
if: failure()
|
||||
run: |
|
||||
echo "=== weed mini log (last 200 lines) ==="
|
||||
tail -n 200 /tmp/weed-mini.log 2>/dev/null || echo "no log available"
|
||||
|
||||
- name: Archive log
|
||||
if: failure()
|
||||
uses: actions/upload-artifact@v7
|
||||
with:
|
||||
name: s3-sdk-v2-routing-server-log
|
||||
path: /tmp/weed-mini.log
|
||||
retention-days: 3
|
||||
41
test/s3/sdk_v2_routing/Makefile
Normal file
41
test/s3/sdk_v2_routing/Makefile
Normal file
@@ -0,0 +1,41 @@
|
||||
# AWS SDK V2 Route Disambiguation Integration Tests
|
||||
#
|
||||
# Pins the regression for the route collision between the regular S3 API
|
||||
# and the S3 Tables REST API on shared top-level paths (/buckets,
|
||||
# /get-table). The tests use the real AWS SDK V2 for Go so the SDK's own
|
||||
# XML deserializer is the assertion — a JSON body produces an SDK error
|
||||
# before any test code runs.
|
||||
#
|
||||
# Prerequisites:
|
||||
# - SeaweedFS running with S3 API enabled on port 8333
|
||||
# - Go 1.21+
|
||||
#
|
||||
# Usage:
|
||||
# make test - Run the SDK V2 routing tests
|
||||
# make test-verbose - Run with verbose output
|
||||
# make clean - Clean test cache
|
||||
|
||||
.PHONY: all test test-verbose clean help
|
||||
|
||||
S3_ENDPOINT ?= http://127.0.0.1:8333
|
||||
|
||||
all: test
|
||||
|
||||
test:
|
||||
@echo "Running SDK V2 routing tests against $(S3_ENDPOINT)..."
|
||||
S3_ENDPOINT=$(S3_ENDPOINT) go test -v -timeout 5m ./...
|
||||
|
||||
test-verbose:
|
||||
S3_ENDPOINT=$(S3_ENDPOINT) go test -v -timeout 5m -count=1 ./...
|
||||
|
||||
clean:
|
||||
go clean -testcache
|
||||
|
||||
help:
|
||||
@echo "AWS SDK V2 Route Disambiguation Tests"
|
||||
@echo "Targets:"
|
||||
@echo " test Run the routing tests"
|
||||
@echo " test-verbose Run with verbose output"
|
||||
@echo " clean Clean test cache"
|
||||
@echo "Environment Variables:"
|
||||
@echo " S3_ENDPOINT S3 endpoint URL (default: http://127.0.0.1:8333)"
|
||||
210
test/s3/sdk_v2_routing/s3_sdk_v2_routing_test.go
Normal file
210
test/s3/sdk_v2_routing/s3_sdk_v2_routing_test.go
Normal file
@@ -0,0 +1,210 @@
|
||||
// Package sdkv2routing_test exercises route disambiguation between the
|
||||
// regular S3 API and the S3 Tables REST API on top-level paths the two
|
||||
// share (/buckets, /get-table). The bug it pins:
|
||||
//
|
||||
// When a user has an S3 bucket named "buckets" (or "get-table"), a
|
||||
// path-style ListObjectsV2 request sent by AWS SDK V2 / Hadoop s3a /
|
||||
// Spark would be routed to the S3 Tables ListTableBuckets handler and
|
||||
// receive a JSON body. AWS SDK V2 then fails XML parsing with
|
||||
// "Unexpected character '{' (code 123) in prolog".
|
||||
//
|
||||
// These tests use the real AWS SDK V2 for Go, so the SDK's own
|
||||
// deserializer is the assertion: if the server returns the wrong
|
||||
// content type, the SDK errors out before any test assertion runs.
|
||||
package sdkv2routing_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"io"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/aws/aws-sdk-go-v2/aws"
|
||||
"github.com/aws/aws-sdk-go-v2/config"
|
||||
"github.com/aws/aws-sdk-go-v2/credentials"
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
const (
|
||||
defaultEndpoint = "http://127.0.0.1:8333"
|
||||
defaultAccessKey = "some_access_key1"
|
||||
defaultSecretKey = "some_secret_key1"
|
||||
defaultRegion = "us-east-1"
|
||||
)
|
||||
|
||||
func getS3Client(t *testing.T) *s3.Client {
|
||||
t.Helper()
|
||||
|
||||
endpoint := os.Getenv("S3_ENDPOINT")
|
||||
if endpoint == "" {
|
||||
endpoint = defaultEndpoint
|
||||
}
|
||||
accessKey := os.Getenv("AWS_ACCESS_KEY_ID")
|
||||
if accessKey == "" {
|
||||
accessKey = defaultAccessKey
|
||||
}
|
||||
secretKey := os.Getenv("AWS_SECRET_ACCESS_KEY")
|
||||
if secretKey == "" {
|
||||
secretKey = defaultSecretKey
|
||||
}
|
||||
region := os.Getenv("AWS_REGION")
|
||||
if region == "" {
|
||||
region = defaultRegion
|
||||
}
|
||||
|
||||
cfg, err := config.LoadDefaultConfig(context.TODO(),
|
||||
config.WithRegion(region),
|
||||
config.WithCredentialsProvider(credentials.NewStaticCredentialsProvider(accessKey, secretKey, "")),
|
||||
config.WithEndpointResolverWithOptions(aws.EndpointResolverWithOptionsFunc(
|
||||
func(service, region string, options ...interface{}) (aws.Endpoint, error) {
|
||||
return aws.Endpoint{
|
||||
URL: endpoint,
|
||||
SigningRegion: defaultRegion,
|
||||
HostnameImmutable: true,
|
||||
}, nil
|
||||
})),
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
return s3.NewFromConfig(cfg, func(o *s3.Options) {
|
||||
o.UsePathStyle = true
|
||||
})
|
||||
}
|
||||
|
||||
// ensureBucket creates bucket if it doesn't already exist. It tolerates
|
||||
// BucketAlreadyOwnedByYou / BucketAlreadyExists so the tests are
|
||||
// idempotent across local re-runs.
|
||||
func ensureBucket(t *testing.T, ctx context.Context, client *s3.Client, bucket string) {
|
||||
t.Helper()
|
||||
_, err := client.CreateBucket(ctx, &s3.CreateBucketInput{Bucket: aws.String(bucket)})
|
||||
if err == nil {
|
||||
return
|
||||
}
|
||||
msg := err.Error()
|
||||
if strings.Contains(msg, "BucketAlreadyOwnedByYou") || strings.Contains(msg, "BucketAlreadyExists") {
|
||||
return
|
||||
}
|
||||
t.Fatalf("CreateBucket(%q) failed: %v", bucket, err)
|
||||
}
|
||||
|
||||
// deleteBucket best-effort cleans up a bucket and any objects in it.
|
||||
// Test does not fail if cleanup fails — the next run is idempotent.
|
||||
func deleteBucket(ctx context.Context, client *s3.Client, bucket string) {
|
||||
paginator := s3.NewListObjectsV2Paginator(client, &s3.ListObjectsV2Input{Bucket: aws.String(bucket)})
|
||||
for paginator.HasMorePages() {
|
||||
page, err := paginator.NextPage(ctx)
|
||||
if err != nil {
|
||||
break
|
||||
}
|
||||
for _, obj := range page.Contents {
|
||||
client.DeleteObject(ctx, &s3.DeleteObjectInput{Bucket: aws.String(bucket), Key: obj.Key})
|
||||
}
|
||||
}
|
||||
client.DeleteBucket(ctx, &s3.DeleteBucketInput{Bucket: aws.String(bucket)})
|
||||
}
|
||||
|
||||
// TestListObjectsV2_OnBucketNamedBuckets is the direct reproducer for
|
||||
// issue #9559: Spark / Hadoop s3a does a ListObjectsV2 against bucket
|
||||
// "buckets" via AWS SDK V2, which fails with
|
||||
// "Could not parse XML response. ... Unexpected character '{' (code 123)
|
||||
// in prolog" when SeaweedFS routes the request to the JSON-returning
|
||||
// ListTableBuckets handler. The SDK's response deserializer is the
|
||||
// real assertion here — a JSON body produces an SDK error before we
|
||||
// reach require.NoError.
|
||||
func TestListObjectsV2_OnBucketNamedBuckets(t *testing.T) {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
|
||||
defer cancel()
|
||||
client := getS3Client(t)
|
||||
|
||||
const bucket = "buckets"
|
||||
ensureBucket(t, ctx, client, bucket)
|
||||
t.Cleanup(func() { deleteBucket(ctx, client, bucket) })
|
||||
|
||||
out, err := client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{
|
||||
Bucket: aws.String(bucket),
|
||||
Prefix: aws.String("logs/"),
|
||||
})
|
||||
require.NoError(t, err, "AWS SDK V2 must parse the response as XML")
|
||||
require.NotNil(t, out)
|
||||
}
|
||||
|
||||
// TestPutGetObject_OnBucketNamedBuckets exercises the full read/write
|
||||
// round-trip on the colliding bucket name. PutObject and GetObject go
|
||||
// through different routes than ListObjectsV2, and verifying them
|
||||
// guards against future regressions that re-route only some verbs.
|
||||
func TestPutGetObject_OnBucketNamedBuckets(t *testing.T) {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
|
||||
defer cancel()
|
||||
client := getS3Client(t)
|
||||
|
||||
const bucket = "buckets"
|
||||
const key = "logs/hello.txt"
|
||||
const body = "hello from issue 9559"
|
||||
|
||||
ensureBucket(t, ctx, client, bucket)
|
||||
t.Cleanup(func() { deleteBucket(ctx, client, bucket) })
|
||||
|
||||
_, err := client.PutObject(ctx, &s3.PutObjectInput{
|
||||
Bucket: aws.String(bucket),
|
||||
Key: aws.String(key),
|
||||
Body: strings.NewReader(body),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
out, err := client.GetObject(ctx, &s3.GetObjectInput{Bucket: aws.String(bucket), Key: aws.String(key)})
|
||||
require.NoError(t, err)
|
||||
defer out.Body.Close()
|
||||
|
||||
got, err := io.ReadAll(out.Body)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, body, string(got))
|
||||
}
|
||||
|
||||
// TestListObjectsV2_OnBucketNamedGetTable covers the second colliding
|
||||
// path. The S3 Tables GET /get-table endpoint shares its path with a
|
||||
// bucket literally named "get-table", which is a legal S3 bucket name.
|
||||
func TestListObjectsV2_OnBucketNamedGetTable(t *testing.T) {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
|
||||
defer cancel()
|
||||
client := getS3Client(t)
|
||||
|
||||
const bucket = "get-table"
|
||||
ensureBucket(t, ctx, client, bucket)
|
||||
t.Cleanup(func() { deleteBucket(ctx, client, bucket) })
|
||||
|
||||
out, err := client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{Bucket: aws.String(bucket)})
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, out)
|
||||
}
|
||||
|
||||
// TestCreateAndListBuckets_ServiceLevel verifies the SDK's service-level
|
||||
// ListBuckets still parses as XML when a bucket named "buckets" exists.
|
||||
// ListBuckets goes through GET / (root) which is unaffected by the
|
||||
// /buckets route collision — this is a guard against the matcher
|
||||
// accidentally widening to top-level paths.
|
||||
func TestCreateAndListBuckets_ServiceLevel(t *testing.T) {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
|
||||
defer cancel()
|
||||
client := getS3Client(t)
|
||||
|
||||
const bucket = "buckets"
|
||||
ensureBucket(t, ctx, client, bucket)
|
||||
t.Cleanup(func() { deleteBucket(ctx, client, bucket) })
|
||||
|
||||
out, err := client.ListBuckets(ctx, &s3.ListBucketsInput{})
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, out)
|
||||
|
||||
found := false
|
||||
for _, b := range out.Buckets {
|
||||
if aws.ToString(b.Name) == bucket {
|
||||
found = true
|
||||
break
|
||||
}
|
||||
}
|
||||
require.True(t, found, "bucket %q must appear in service-level ListBuckets", bucket)
|
||||
}
|
||||
|
||||
@@ -17,6 +17,9 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/aws/aws-sdk-go/aws/credentials"
|
||||
v4 "github.com/aws/aws-sdk-go/aws/signer/v4"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/test/testutil"
|
||||
)
|
||||
|
||||
@@ -451,19 +454,22 @@ func icebergPath(prefix, path string) string {
|
||||
return withPrefix
|
||||
}
|
||||
|
||||
// createTableBucket creates a table bucket via the S3Tables REST API
|
||||
// createTableBucket creates a table bucket via the S3Tables REST API.
|
||||
// The request is AWS V4 signed for SERVICE=s3tables so the S3 Tables
|
||||
// route matcher accepts it; signing with regular SERVICE=s3 would let
|
||||
// the request fall through to the S3 CreateBucket handler.
|
||||
func createTableBucket(t *testing.T, env *TestEnvironment, bucketName string) {
|
||||
t.Helper()
|
||||
|
||||
// Use S3Tables REST API to create the bucket
|
||||
endpoint := fmt.Sprintf("http://localhost:%d/buckets", env.s3Port)
|
||||
|
||||
reqBody := fmt.Sprintf(`{"name":"%s"}`, bucketName)
|
||||
|
||||
req, err := http.NewRequest(http.MethodPut, endpoint, strings.NewReader(reqBody))
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create request: %v", err)
|
||||
}
|
||||
req.Header.Set("Content-Type", "application/x-amz-json-1.1")
|
||||
signS3TablesRequest(t, req, reqBody)
|
||||
|
||||
resp, err := http.DefaultClient.Do(req)
|
||||
if err != nil {
|
||||
@@ -480,6 +486,20 @@ func createTableBucket(t *testing.T, env *TestEnvironment, bucketName string) {
|
||||
t.Logf("Created table bucket %s", bucketName)
|
||||
}
|
||||
|
||||
// signS3TablesRequest signs req with AWS V4 for SERVICE=s3tables. The
|
||||
// underlying weed mini instance runs in default-allow mode so the
|
||||
// signature itself is not verified; only the credential scope matters,
|
||||
// because the S3 Tables route matcher requires SERVICE=s3tables to
|
||||
// distinguish S3 Tables traffic from regular S3 calls on the same paths.
|
||||
func signS3TablesRequest(t *testing.T, req *http.Request, body string) {
|
||||
t.Helper()
|
||||
creds := credentials.NewStaticCredentials("test-ak", "test-sk", "")
|
||||
signer := v4.NewSigner(creds)
|
||||
if _, err := signer.Sign(req, strings.NewReader(body), "s3tables", "us-east-1", time.Now()); err != nil {
|
||||
t.Fatalf("Failed to sign S3 Tables request: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// randomSuffix returns a short random hex suffix for unique resource naming.
|
||||
func randomSuffix() string {
|
||||
return fmt.Sprintf("%x", time.Now().UnixNano()&0xffffffff)
|
||||
|
||||
@@ -84,54 +84,64 @@ func (s3a *S3ApiServer) registerS3TablesRoutes(router *mux.Router) {
|
||||
targetMatcher := func(r *http.Request, rm *mux.RouteMatch) bool {
|
||||
return strings.HasPrefix(r.Header.Get("X-Amz-Target"), "S3Tables.")
|
||||
}
|
||||
// serviceMatcher gates every S3 Tables route so it only matches when the
|
||||
// request is genuinely targeting the s3tables service. The bare paths
|
||||
// (/buckets, /get-table) collide with regular S3 buckets of the same
|
||||
// name; the ARN-bearing paths (/buckets/<arn>, /namespaces/<arn>, ...)
|
||||
// could collide with object keys that look like S3 Tables ARNs inside a
|
||||
// bucket named "buckets", "namespaces", "tables", or "tag". A single
|
||||
// matcher applied to every route closes both classes of collision.
|
||||
serviceMatcher := func(r *http.Request, rm *mux.RouteMatch) bool {
|
||||
return isS3TablesSignedRequest(r)
|
||||
}
|
||||
router.Methods(http.MethodPost).Path("/").MatcherFunc(targetMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.S3TablesHandler), "S3Tables-Target"))
|
||||
router.Methods(http.MethodPut).Path("/buckets").
|
||||
router.Methods(http.MethodPut).Path("/buckets").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("CreateTableBucket", buildCreateTableBucketRequest)), "S3Tables-CreateTableBucket"))
|
||||
router.Methods(http.MethodGet).Path("/buckets").
|
||||
router.Methods(http.MethodGet).Path("/buckets").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("ListTableBuckets", buildListTableBucketsRequest)), "S3Tables-ListTableBuckets"))
|
||||
router.Methods(http.MethodGet).Path("/buckets/{tableBucketARN:" + tableBucketARNRegex + "}").
|
||||
router.Methods(http.MethodGet).Path("/buckets/{tableBucketARN:" + tableBucketARNRegex + "}").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("GetTableBucket", buildTableBucketArnRequest)), "S3Tables-GetTableBucket"))
|
||||
router.Methods(http.MethodDelete).Path("/buckets/{tableBucketARN:" + tableBucketARNRegex + "}").
|
||||
router.Methods(http.MethodDelete).Path("/buckets/{tableBucketARN:" + tableBucketARNRegex + "}").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("DeleteTableBucket", buildDeleteTableBucketRequest)), "S3Tables-DeleteTableBucket"))
|
||||
router.Methods(http.MethodPut).Path("/buckets/{tableBucketARN:" + tableBucketARNRegex + "}/policy").
|
||||
router.Methods(http.MethodPut).Path("/buckets/{tableBucketARN:" + tableBucketARNRegex + "}/policy").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("PutTableBucketPolicy", buildPutTableBucketPolicyRequest)), "S3Tables-PutTableBucketPolicy"))
|
||||
router.Methods(http.MethodGet).Path("/buckets/{tableBucketARN:" + tableBucketARNRegex + "}/policy").
|
||||
router.Methods(http.MethodGet).Path("/buckets/{tableBucketARN:" + tableBucketARNRegex + "}/policy").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("GetTableBucketPolicy", buildGetTableBucketPolicyRequest)), "S3Tables-GetTableBucketPolicy"))
|
||||
router.Methods(http.MethodDelete).Path("/buckets/{tableBucketARN:" + tableBucketARNRegex + "}/policy").
|
||||
router.Methods(http.MethodDelete).Path("/buckets/{tableBucketARN:" + tableBucketARNRegex + "}/policy").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("DeleteTableBucketPolicy", buildDeleteTableBucketPolicyRequest)), "S3Tables-DeleteTableBucketPolicy"))
|
||||
|
||||
router.Methods(http.MethodPut).Path("/namespaces/{tableBucketARN:" + tableBucketARNRegex + "}").
|
||||
router.Methods(http.MethodPut).Path("/namespaces/{tableBucketARN:" + tableBucketARNRegex + "}").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("CreateNamespace", buildCreateNamespaceRequest)), "S3Tables-CreateNamespace"))
|
||||
router.Methods(http.MethodGet).Path("/namespaces/{tableBucketARN:" + tableBucketARNRegex + "}").
|
||||
router.Methods(http.MethodGet).Path("/namespaces/{tableBucketARN:" + tableBucketARNRegex + "}").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("ListNamespaces", buildListNamespacesRequest)), "S3Tables-ListNamespaces"))
|
||||
router.Methods(http.MethodGet).Path("/namespaces/{tableBucketARN:" + tableBucketARNRegex + "}/{namespace}").
|
||||
router.Methods(http.MethodGet).Path("/namespaces/{tableBucketARN:" + tableBucketARNRegex + "}/{namespace}").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("GetNamespace", buildGetNamespaceRequest)), "S3Tables-GetNamespace"))
|
||||
router.Methods(http.MethodDelete).Path("/namespaces/{tableBucketARN:" + tableBucketARNRegex + "}/{namespace}").
|
||||
router.Methods(http.MethodDelete).Path("/namespaces/{tableBucketARN:" + tableBucketARNRegex + "}/{namespace}").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("DeleteNamespace", buildDeleteNamespaceRequest)), "S3Tables-DeleteNamespace"))
|
||||
|
||||
router.Methods(http.MethodPut).Path("/tables/{tableBucketARN:" + tableBucketARNRegex + "}/{namespace}").
|
||||
router.Methods(http.MethodPut).Path("/tables/{tableBucketARN:" + tableBucketARNRegex + "}/{namespace}").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("CreateTable", buildCreateTableRequest)), "S3Tables-CreateTable"))
|
||||
router.Methods(http.MethodGet).Path("/tables/{tableBucketARN:" + tableBucketARNRegex + "}").
|
||||
router.Methods(http.MethodGet).Path("/tables/{tableBucketARN:" + tableBucketARNRegex + "}").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("ListTables", buildListTablesRequest)), "S3Tables-ListTables"))
|
||||
router.Methods(http.MethodDelete).Path("/tables/{tableBucketARN:" + tableBucketARNRegex + "}/{namespace}/{name}").
|
||||
router.Methods(http.MethodDelete).Path("/tables/{tableBucketARN:" + tableBucketARNRegex + "}/{namespace}/{name}").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("DeleteTable", buildDeleteTableRequest)), "S3Tables-DeleteTable"))
|
||||
|
||||
router.Methods(http.MethodPut).Path("/tables/{tableBucketARN:" + tableBucketARNRegex + "}/{namespace}/{name}/policy").
|
||||
router.Methods(http.MethodPut).Path("/tables/{tableBucketARN:" + tableBucketARNRegex + "}/{namespace}/{name}/policy").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("PutTablePolicy", buildPutTablePolicyRequest)), "S3Tables-PutTablePolicy"))
|
||||
router.Methods(http.MethodGet).Path("/tables/{tableBucketARN:" + tableBucketARNRegex + "}/{namespace}/{name}/policy").
|
||||
router.Methods(http.MethodGet).Path("/tables/{tableBucketARN:" + tableBucketARNRegex + "}/{namespace}/{name}/policy").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("GetTablePolicy", buildGetTablePolicyRequest)), "S3Tables-GetTablePolicy"))
|
||||
router.Methods(http.MethodDelete).Path("/tables/{tableBucketARN:" + tableBucketARNRegex + "}/{namespace}/{name}/policy").
|
||||
router.Methods(http.MethodDelete).Path("/tables/{tableBucketARN:" + tableBucketARNRegex + "}/{namespace}/{name}/policy").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("DeleteTablePolicy", buildDeleteTablePolicyRequest)), "S3Tables-DeleteTablePolicy"))
|
||||
|
||||
router.Methods(http.MethodPost).Path("/tag/{resourceArn:arn:aws:s3tables:.*}").
|
||||
router.Methods(http.MethodPost).Path("/tag/{resourceArn:arn:aws:s3tables:.*}").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("TagResource", buildTagResourceRequest)), "S3Tables-TagResource"))
|
||||
router.Methods(http.MethodGet).Path("/tag/{resourceArn:arn:aws:s3tables:.*}").
|
||||
router.Methods(http.MethodGet).Path("/tag/{resourceArn:arn:aws:s3tables:.*}").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("ListTagsForResource", buildListTagsForResourceRequest)), "S3Tables-ListTagsForResource"))
|
||||
router.Methods(http.MethodDelete).Path("/tag/{resourceArn:arn:aws:s3tables:.*}").
|
||||
router.Methods(http.MethodDelete).Path("/tag/{resourceArn:arn:aws:s3tables:.*}").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("UntagResource", buildUntagResourceRequest)), "S3Tables-UntagResource"))
|
||||
|
||||
router.Methods(http.MethodGet).Path("/get-table").
|
||||
router.Methods(http.MethodGet).Path("/get-table").MatcherFunc(serviceMatcher).
|
||||
HandlerFunc(track(s3a.authenticateS3Tables(s3TablesApi.handleRestOperation("GetTable", buildGetTableRequest)), "S3Tables-GetTable"))
|
||||
|
||||
glog.V(1).Infof("S3 Tables API enabled")
|
||||
@@ -623,6 +633,49 @@ func buildUntagResourceRequest(r *http.Request) (interface{}, error) {
|
||||
}, nil
|
||||
}
|
||||
|
||||
// isS3TablesSignedRequest reports whether the request is targeting the
|
||||
// S3 Tables service. The signal is the AWS V4 credential scope, which
|
||||
// names SERVICE=s3tables for S3 Tables SDKs and SERVICE=s3 for regular
|
||||
// S3 SDKs. The credential scope appears in the Authorization header
|
||||
// (Credential=AK/DATE/REGION/SERVICE/aws4_request) for signed requests
|
||||
// and in the X-Amz-Credential query parameter for presigned requests.
|
||||
//
|
||||
// The credential scope is the only acceptable signal: a content-type-
|
||||
// based fallback would let an anonymous regular-S3 request (e.g. a
|
||||
// PutObject with body type application/x-amz-json-1.1) sneak through
|
||||
// to an S3 Tables route whenever the object key is shaped like an
|
||||
// S3 Tables ARN. Clients that genuinely target S3 Tables — including
|
||||
// internal test harnesses running against a default-allow server —
|
||||
// must sign with SERVICE=s3tables.
|
||||
func isS3TablesSignedRequest(r *http.Request) bool {
|
||||
scope := extractCredentialScope(r)
|
||||
// Credential scope is AK/DATE/REGION/SERVICE/aws4_request. Slashes
|
||||
// do not appear inside any other component (access keys are
|
||||
// alphanumeric), so /s3tables/ matches iff SERVICE is exactly
|
||||
// s3tables.
|
||||
return scope != "" && strings.Contains(scope, "/s3tables/")
|
||||
}
|
||||
|
||||
// extractCredentialScope returns the raw credential value from either the
|
||||
// Authorization header or the X-Amz-Credential query parameter, without the
|
||||
// "Credential=" prefix. Returns the empty string when neither is present.
|
||||
func extractCredentialScope(r *http.Request) string {
|
||||
if auth := r.Header.Get("Authorization"); auth != "" {
|
||||
idx := strings.Index(auth, "Credential=")
|
||||
if idx >= 0 {
|
||||
tail := auth[idx+len("Credential="):]
|
||||
if comma := strings.IndexByte(tail, ','); comma >= 0 {
|
||||
tail = tail[:comma]
|
||||
}
|
||||
return strings.TrimSpace(tail)
|
||||
}
|
||||
}
|
||||
if cred := r.URL.Query().Get("X-Amz-Credential"); cred != "" {
|
||||
return cred
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// authenticateS3Tables wraps the handler with IAM authentication using AuthSignatureOnly
|
||||
// This authenticates the request but delegates authorization to the S3 Tables handler
|
||||
// which performs granular permission checks based on the specific operation.
|
||||
|
||||
158
weed/s3api/s3api_tables_routing_test.go
Normal file
158
weed/s3api/s3api_tables_routing_test.go
Normal file
@@ -0,0 +1,158 @@
|
||||
package s3api
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/gorilla/mux"
|
||||
)
|
||||
|
||||
// TestIsS3TablesSignedRequest covers the credential-scope parser used to
|
||||
// disambiguate S3 Tables REST requests from regular S3 requests on paths
|
||||
// the two APIs share (e.g. /buckets, /get-table).
|
||||
func TestIsS3TablesSignedRequest(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
auth string
|
||||
cred string
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
name: "regular S3 auth header",
|
||||
auth: "AWS4-HMAC-SHA256 Credential=AKIA/20260101/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-date, Signature=deadbeef",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "S3 Tables auth header",
|
||||
auth: "AWS4-HMAC-SHA256 Credential=AKIA/20260101/us-east-1/s3tables/aws4_request, SignedHeaders=host;x-amz-date, Signature=deadbeef",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "S3 Tables presigned query",
|
||||
cred: "AKIA/20260101/us-east-1/s3tables/aws4_request",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "regular S3 presigned query",
|
||||
cred: "AKIA/20260101/us-east-1/s3/aws4_request",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "unsigned request",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "malformed credential",
|
||||
auth: "AWS4-HMAC-SHA256 Credential=AKIA, Signature=zzz",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "service substring in access key must not match",
|
||||
auth: "AWS4-HMAC-SHA256 Credential=s3tablesAKIA/20260101/us-east-1/s3/aws4_request, Signature=zzz",
|
||||
want: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
req := httptest.NewRequest(http.MethodGet, "/buckets", nil)
|
||||
if tc.auth != "" {
|
||||
req.Header.Set("Authorization", tc.auth)
|
||||
}
|
||||
if tc.cred != "" {
|
||||
q := req.URL.Query()
|
||||
q.Set("X-Amz-Credential", tc.cred)
|
||||
req.URL.RawQuery = q.Encode()
|
||||
}
|
||||
if got := isS3TablesSignedRequest(req); got != tc.want {
|
||||
t.Fatalf("isS3TablesSignedRequest=%v, want %v", got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestRouting_ListObjectsV2OnBucketNamedBuckets covers the route collision
|
||||
// between regular S3 listObjectsV2 on a bucket named "buckets" and the
|
||||
// S3 Tables ListTableBuckets endpoint, both of which use the path /buckets.
|
||||
// The credential scope on a regular S3 request is /s3/aws4_request, so the
|
||||
// S3 Tables route must reject it and let the bucket subrouter take it,
|
||||
// otherwise AWS SDK V2 / s3a clients receive a JSON body in place of the
|
||||
// expected XML and fail to parse the response.
|
||||
func TestRouting_ListObjectsV2OnBucketNamedBuckets(t *testing.T) {
|
||||
router := mux.NewRouter()
|
||||
s3a := setupRoutingTestServer(t)
|
||||
s3a.registerRouter(router)
|
||||
|
||||
req, _ := http.NewRequest(http.MethodGet, "/buckets?list-type=2&prefix=logs%2F", nil)
|
||||
signRoutingTestRequest(t, req, "", "s3")
|
||||
|
||||
var match mux.RouteMatch
|
||||
if !router.Match(req, &match) {
|
||||
t.Fatalf("expected GET /buckets?list-type=2 (signed for s3) to match a route; got no match: %v", match.MatchErr)
|
||||
}
|
||||
if tmpl, _ := match.Route.GetPathTemplate(); tmpl == "/buckets" {
|
||||
t.Fatalf("GET /buckets?list-type=2 signed for service=s3 matched the S3 Tables ListTableBuckets route (Path=%q); it must fall through to the bucket subrouter", tmpl)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRouting_S3TablesListTableBucketsStillReachable verifies the matcher
|
||||
// does not break legitimate S3 Tables traffic: a request signed for the
|
||||
// s3tables service on GET /buckets must still reach ListTableBuckets.
|
||||
func TestRouting_S3TablesListTableBucketsStillReachable(t *testing.T) {
|
||||
router := mux.NewRouter()
|
||||
s3a := setupRoutingTestServer(t)
|
||||
s3a.registerRouter(router)
|
||||
|
||||
req, _ := http.NewRequest(http.MethodGet, "/buckets", nil)
|
||||
signRoutingTestRequest(t, req, "", "s3tables")
|
||||
|
||||
var match mux.RouteMatch
|
||||
if !router.Match(req, &match) {
|
||||
t.Fatalf("expected GET /buckets (signed for s3tables) to match a route; got no match: %v", match.MatchErr)
|
||||
}
|
||||
tmpl, _ := match.Route.GetPathTemplate()
|
||||
if tmpl != "/buckets" {
|
||||
t.Fatalf("GET /buckets signed for service=s3tables matched %q; want the S3 Tables route /buckets", tmpl)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRouting_CreateBucketNamedBuckets covers the PUT collision: a PUT
|
||||
// /buckets signed for s3 is CreateBucket on the regular S3 API, not
|
||||
// CreateTableBucket.
|
||||
func TestRouting_CreateBucketNamedBuckets(t *testing.T) {
|
||||
router := mux.NewRouter()
|
||||
s3a := setupRoutingTestServer(t)
|
||||
s3a.registerRouter(router)
|
||||
|
||||
req, _ := http.NewRequest(http.MethodPut, "/buckets", nil)
|
||||
signRoutingTestRequest(t, req, "", "s3")
|
||||
|
||||
var match mux.RouteMatch
|
||||
if !router.Match(req, &match) {
|
||||
t.Fatalf("expected PUT /buckets (signed for s3) to match a route; got no match: %v", match.MatchErr)
|
||||
}
|
||||
if tmpl, _ := match.Route.GetPathTemplate(); tmpl == "/buckets" {
|
||||
t.Fatalf("PUT /buckets signed for service=s3 matched the S3 Tables CreateTableBucket route (Path=%q); it must fall through to the bucket subrouter", tmpl)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRouting_GetObjectOnBucketNamedGetTable covers the GET /get-table
|
||||
// collision: a regular S3 request on a bucket named "get-table" must
|
||||
// reach the bucket subrouter, not the S3 Tables GetTable handler.
|
||||
func TestRouting_GetObjectOnBucketNamedGetTable(t *testing.T) {
|
||||
router := mux.NewRouter()
|
||||
s3a := setupRoutingTestServer(t)
|
||||
s3a.registerRouter(router)
|
||||
|
||||
req, _ := http.NewRequest(http.MethodGet, "/get-table?list-type=2", nil)
|
||||
signRoutingTestRequest(t, req, "", "s3")
|
||||
|
||||
var match mux.RouteMatch
|
||||
if !router.Match(req, &match) {
|
||||
t.Fatalf("expected GET /get-table?list-type=2 (signed for s3) to match a route; got no match: %v", match.MatchErr)
|
||||
}
|
||||
if tmpl, _ := match.Route.GetPathTemplate(); tmpl == "/get-table" {
|
||||
t.Fatalf("GET /get-table signed for service=s3 matched the S3 Tables GetTable route (Path=%q); it must fall through to the bucket subrouter", tmpl)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user