mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-09-19 13:00:45 +02:00
s3api: unify auth error handling across s3tables, iceberg and lance (#11381)
* s3api: fail closed when S3 Tables signature verification fails * s3api: avoid nil Account dereference in S3 Tables auth log * iceberg: return auth error instead of falling back to DefaultAllow * lance: return auth error instead of falling back to DefaultAllow * s3api: stop trusting client-supplied s3-account-id The header is set by the server after successful authentication; scrub inbound values alongside the other internal headers, and apply the same admin guard to the header fallback branch of getAccountID that the identity branch already has. * test: cover table-catalog auth wrappers and principal resolution * test: configure anonymous identity where catalog clients do not sign * s3api: scrub s3-account-id after signature verification
This commit is contained in:
@@ -157,6 +157,12 @@ func newEnvironment() (*environment, error) {
|
||||
}
|
||||
|
||||
func (env *environment) start() error {
|
||||
// The catalog clients here do not sign, so anonymous access is configured explicitly.
|
||||
iamConfigPath, err := testutil.WriteIAMConfig(env.dataDir, accessKey, secretKey)
|
||||
if err != nil {
|
||||
return fmt.Errorf("write IAM config: %w", err)
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
env.weedCancel = cancel
|
||||
|
||||
@@ -171,6 +177,7 @@ func (env *environment) start() error {
|
||||
"-s3.port.grpc", fmt.Sprintf("%d", env.s3GrpcPort),
|
||||
"-s3.port.iceberg", fmt.Sprintf("%d", env.icebergPort),
|
||||
"-s3.port.lance", fmt.Sprintf("%d", env.lancePort),
|
||||
"-s3.config", iamConfigPath,
|
||||
"-ip.bind", "0.0.0.0",
|
||||
"-dir", env.dataDir,
|
||||
)
|
||||
|
||||
@@ -58,6 +58,9 @@ func WriteIAMConfig(dir, accessKey, secretKey string) (string, error) {
|
||||
"Tagging",
|
||||
"Write"
|
||||
]
|
||||
},
|
||||
{
|
||||
"name": "anonymous"
|
||||
}
|
||||
]
|
||||
}`, accessKey, secretKey)
|
||||
|
||||
@@ -1679,6 +1679,8 @@ func (iam *IdentityAccessManagement) authenticateRequestInternal(r *http.Request
|
||||
// for every auth path — prevents privilege escalation via header injection.
|
||||
r.Header.Del(s3_constants.SeaweedFSPrincipalHeader)
|
||||
r.Header.Del(s3_constants.SeaweedFSSessionTokenHeader)
|
||||
// Deferred so signature verification can still read a client-signed value.
|
||||
defer r.Header.Del(s3_constants.AmzAccountId)
|
||||
|
||||
reqAuthType := getRequestAuthType(r)
|
||||
|
||||
|
||||
@@ -250,25 +250,20 @@ func (s *Server) Auth(handler http.HandlerFunc) http.HandlerFunc {
|
||||
|
||||
identityName, identity, errCode := s.authenticator.AuthenticateRequest(r)
|
||||
if errCode != s3err.ErrNone {
|
||||
// If authentication failed but DefaultAllow is enabled, proceed without identity
|
||||
if s.authenticator.DefaultAllow() {
|
||||
glog.V(2).Infof("Iceberg: AuthenticateRequest failed (%v), but DefaultAllow is true, proceeding", errCode)
|
||||
} else {
|
||||
apiErr := s3err.GetAPIError(errCode)
|
||||
errorType := "RESTException"
|
||||
switch apiErr.HTTPStatusCode {
|
||||
case http.StatusForbidden:
|
||||
errorType = "ForbiddenException"
|
||||
case http.StatusUnauthorized:
|
||||
errorType = "NotAuthorizedException"
|
||||
case http.StatusBadRequest:
|
||||
errorType = "BadRequestException"
|
||||
case http.StatusInternalServerError:
|
||||
errorType = "InternalServerError"
|
||||
}
|
||||
writeError(w, apiErr.HTTPStatusCode, errorType, apiErr.Description)
|
||||
return
|
||||
apiErr := s3err.GetAPIError(errCode)
|
||||
errorType := "RESTException"
|
||||
switch apiErr.HTTPStatusCode {
|
||||
case http.StatusForbidden:
|
||||
errorType = "ForbiddenException"
|
||||
case http.StatusUnauthorized:
|
||||
errorType = "NotAuthorizedException"
|
||||
case http.StatusBadRequest:
|
||||
errorType = "BadRequestException"
|
||||
case http.StatusInternalServerError:
|
||||
errorType = "InternalServerError"
|
||||
}
|
||||
writeError(w, apiErr.HTTPStatusCode, errorType, apiErr.Description)
|
||||
return
|
||||
}
|
||||
|
||||
if identityName != "" || identity != nil {
|
||||
|
||||
+11
-14
@@ -155,21 +155,18 @@ func (s *Server) Auth(handler http.HandlerFunc) http.HandlerFunc {
|
||||
|
||||
identityName, identity, errCode := s.authenticator.AuthenticateRequest(r)
|
||||
if errCode != s3err.ErrNone {
|
||||
if !s.authenticator.DefaultAllow() {
|
||||
apiErr := s3err.GetAPIError(errCode)
|
||||
code := codeInternal
|
||||
switch apiErr.HTTPStatusCode {
|
||||
case http.StatusForbidden:
|
||||
code = codePermissionDenied
|
||||
case http.StatusUnauthorized:
|
||||
code = codeUnauthenticated
|
||||
case http.StatusBadRequest:
|
||||
code = codeInvalidInput
|
||||
}
|
||||
writeError(w, r, apiErr.HTTPStatusCode, code, apiErr.Description)
|
||||
return
|
||||
apiErr := s3err.GetAPIError(errCode)
|
||||
code := codeInternal
|
||||
switch apiErr.HTTPStatusCode {
|
||||
case http.StatusForbidden:
|
||||
code = codePermissionDenied
|
||||
case http.StatusUnauthorized:
|
||||
code = codeUnauthenticated
|
||||
case http.StatusBadRequest:
|
||||
code = codeInvalidInput
|
||||
}
|
||||
glog.V(2).Infof("lance: authentication failed (%v) but the gateway is open, proceeding", errCode)
|
||||
writeError(w, r, apiErr.HTTPStatusCode, code, apiErr.Description)
|
||||
return
|
||||
}
|
||||
|
||||
if identityName != "" || identity != nil {
|
||||
|
||||
@@ -162,6 +162,11 @@ func TestGetBucketOwnershipControlsDefaultsToBucketOwnerEnforced(t *testing.T) {
|
||||
})
|
||||
req := newBucketRequest(http.MethodGet, "b", "ownershipControls=", "")
|
||||
req.Header.Set(s3_constants.AmzAccountId, AccountAdmin.Id)
|
||||
req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), &Identity{
|
||||
Name: "admin",
|
||||
Account: &AccountAdmin,
|
||||
Actions: []Action{s3_constants.ACTION_ADMIN},
|
||||
}))
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
s3a.GetBucketOwnershipControls(rec, req)
|
||||
|
||||
@@ -701,20 +701,18 @@ func (s3a *S3ApiServer) authenticateS3Tables(f http.HandlerFunc) http.HandlerFun
|
||||
// Use AuthSignatureOnly to authenticate the request without authorizing specific actions
|
||||
identity, errCode := s3a.iam.AuthSignatureOnly(r)
|
||||
if errCode != s3err.ErrNone {
|
||||
// If IAM is enabled but DefaultAllow is true, we can proceed even if unauthenticated
|
||||
// authorization checks in handlers will then use DefaultAllow logic.
|
||||
if s3a.iam.iamIntegration != nil && s3a.iam.iamIntegration.DefaultAllow() {
|
||||
glog.V(2).Infof("S3Tables: AuthSignatureOnly failed (%v), but DefaultAllow is true, proceeding", errCode)
|
||||
} else {
|
||||
glog.Errorf("S3Tables: AuthSignatureOnly failed: %v", errCode)
|
||||
s3err.WriteErrorResponse(w, r, errCode)
|
||||
return
|
||||
}
|
||||
glog.Errorf("S3Tables: AuthSignatureOnly failed: %v", errCode)
|
||||
s3err.WriteErrorResponse(w, r, errCode)
|
||||
return
|
||||
}
|
||||
|
||||
// Store the authenticated identity in request context
|
||||
if identity != nil && identity.Name != "" {
|
||||
glog.V(2).Infof("S3Tables: authenticated identity Name=%s Account.Id=%s", identity.Name, identity.Account.Id)
|
||||
accountId := ""
|
||||
if identity.Account != nil {
|
||||
accountId = identity.Account.Id
|
||||
}
|
||||
glog.V(2).Infof("S3Tables: authenticated identity Name=%s Account.Id=%s", identity.Name, accountId)
|
||||
r = r.WithContext(recordIdentityInContext(r, identity))
|
||||
} else {
|
||||
glog.V(2).Infof("S3Tables: authenticated identity is nil or empty name")
|
||||
|
||||
@@ -0,0 +1,171 @@
|
||||
package s3api
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/aws/aws-sdk-go/aws/credentials"
|
||||
v4 "github.com/aws/aws-sdk-go/aws/signer/v4"
|
||||
"github.com/seaweedfs/seaweedfs/weed/s3api/iceberg"
|
||||
"github.com/seaweedfs/seaweedfs/weed/s3api/lance"
|
||||
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
|
||||
"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// setupDefaultAllowAuthServer builds the wiring NewS3ApiServer produces when
|
||||
// S3 credentials are configured but no -iam.config file was given: auth is
|
||||
// enforced while the IAM policy engine still defaults to allow.
|
||||
func setupDefaultAllowAuthServer(t *testing.T) *S3ApiServer {
|
||||
t.Helper()
|
||||
s3a := setupRoutingTestServer(t)
|
||||
manager, err := loadIAMManagerFromConfig("",
|
||||
func() string { return "localhost:8888" },
|
||||
func() string { return "test-signing-key" })
|
||||
require.NoError(t, err)
|
||||
require.True(t, manager.DefaultAllow())
|
||||
s3a.iam.iamIntegration = NewS3IAMIntegration(manager, "")
|
||||
return s3a
|
||||
}
|
||||
|
||||
func forgedS3TablesRequest(t *testing.T, method, target string) *http.Request {
|
||||
t.Helper()
|
||||
req, err := http.NewRequest(method, "http://localhost"+target, nil)
|
||||
require.NoError(t, err)
|
||||
req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=AAAA/20260917/us-east-1/s3tables/aws4_request, SignedHeaders=host, Signature=00")
|
||||
return req
|
||||
}
|
||||
|
||||
func TestS3TablesAuthRejectsFailedSignature(t *testing.T) {
|
||||
s3a := setupDefaultAllowAuthServer(t)
|
||||
|
||||
req := forgedS3TablesRequest(t, http.MethodGet, "/buckets")
|
||||
req.Header.Set(s3_constants.AmzAccountId, "admin")
|
||||
|
||||
reached := false
|
||||
rr := httptest.NewRecorder()
|
||||
s3a.authenticateS3Tables(func(w http.ResponseWriter, r *http.Request) {
|
||||
reached = true
|
||||
w.WriteHeader(http.StatusOK)
|
||||
})(rr, req)
|
||||
|
||||
assert.False(t, reached, "failed authentication must not reach the S3 Tables handler")
|
||||
assert.GreaterOrEqual(t, rr.Code, http.StatusBadRequest)
|
||||
assert.Empty(t, req.Header.Get(s3_constants.AmzAccountId), "client-supplied account header must not survive authentication")
|
||||
}
|
||||
|
||||
func TestS3TablesAuthControlDataPlane(t *testing.T) {
|
||||
s3a := setupDefaultAllowAuthServer(t)
|
||||
|
||||
req := forgedS3TablesRequest(t, http.MethodGet, "/buckets")
|
||||
reached := false
|
||||
rr := httptest.NewRecorder()
|
||||
s3a.iam.Auth(func(w http.ResponseWriter, r *http.Request) {
|
||||
reached = true
|
||||
}, s3_constants.ACTION_READ)(rr, req)
|
||||
|
||||
assert.False(t, reached, "control: data plane must reject the same forged request")
|
||||
assert.GreaterOrEqual(t, rr.Code, http.StatusBadRequest)
|
||||
}
|
||||
|
||||
func TestS3TablesAuthOpenWhenAuthDisabled(t *testing.T) {
|
||||
s3a := setupDefaultAllowAuthServer(t)
|
||||
s3a.iam.isAuthEnabled = false
|
||||
|
||||
req := forgedS3TablesRequest(t, http.MethodGet, "/buckets")
|
||||
reached := false
|
||||
rr := httptest.NewRecorder()
|
||||
s3a.authenticateS3Tables(func(w http.ResponseWriter, r *http.Request) {
|
||||
reached = true
|
||||
w.WriteHeader(http.StatusOK)
|
||||
})(rr, req)
|
||||
|
||||
assert.True(t, reached, "zero-config gateway keeps serving unauthenticated requests")
|
||||
assert.Equal(t, http.StatusOK, rr.Code)
|
||||
}
|
||||
|
||||
func TestS3TablesAuthSignedRequestPassess(t *testing.T) {
|
||||
s3a := setupDefaultAllowAuthServer(t)
|
||||
|
||||
req, err := http.NewRequest(http.MethodGet, "http://localhost/buckets", nil)
|
||||
require.NoError(t, err)
|
||||
creds := credentials.NewStaticCredentials(routingTestAccessKey, routingTestSecretKey, "")
|
||||
_, err = v4.NewSigner(creds).Sign(req, strings.NewReader(""), "s3tables", "us-east-1", time.Now())
|
||||
require.NoError(t, err)
|
||||
|
||||
reached := false
|
||||
rr := httptest.NewRecorder()
|
||||
s3a.authenticateS3Tables(func(w http.ResponseWriter, r *http.Request) {
|
||||
reached = true
|
||||
w.WriteHeader(http.StatusOK)
|
||||
})(rr, req)
|
||||
|
||||
assert.True(t, reached, "properly signed request must still pass; got %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
|
||||
func TestSignedAccountHeaderDoesNotReachHandler(t *testing.T) {
|
||||
s3a := setupDefaultAllowAuthServer(t)
|
||||
|
||||
req, err := http.NewRequest(http.MethodGet, "http://localhost/buckets", nil)
|
||||
require.NoError(t, err)
|
||||
req.Header.Set(s3_constants.AmzAccountId, "admin")
|
||||
signRoutingTestRequest(t, req, "", "s3tables")
|
||||
require.Contains(t, req.Header.Get("Authorization"), "s3-account-id", "the header must be covered by the signature")
|
||||
|
||||
reached := false
|
||||
rr := httptest.NewRecorder()
|
||||
s3a.authenticateS3Tables(func(w http.ResponseWriter, r *http.Request) {
|
||||
reached = true
|
||||
w.WriteHeader(http.StatusOK)
|
||||
})(rr, req)
|
||||
|
||||
assert.True(t, reached, "a signature covering s3-account-id must still verify; got %d %s", rr.Code, rr.Body.String())
|
||||
assert.Empty(t, req.Header.Get(s3_constants.AmzAccountId), "the signed-in header value must not survive authentication")
|
||||
}
|
||||
|
||||
func TestIcebergAuthRejectsFailedSignature(t *testing.T) {
|
||||
s3a := setupDefaultAllowAuthServer(t)
|
||||
server := iceberg.NewServer(nil, s3a)
|
||||
|
||||
req := forgedS3TablesRequest(t, http.MethodGet, "/v1/namespaces")
|
||||
reached := false
|
||||
rr := httptest.NewRecorder()
|
||||
server.Auth(func(w http.ResponseWriter, r *http.Request) {
|
||||
reached = true
|
||||
w.WriteHeader(http.StatusOK)
|
||||
})(rr, req)
|
||||
|
||||
assert.False(t, reached, "failed authentication must not reach the Iceberg handler")
|
||||
assert.NotEqual(t, http.StatusOK, rr.Code)
|
||||
}
|
||||
|
||||
func TestLanceAuthRejectsFailedSignature(t *testing.T) {
|
||||
s3a := setupDefaultAllowAuthServer(t)
|
||||
server := lance.NewServer(nil, s3a)
|
||||
|
||||
req := forgedS3TablesRequest(t, http.MethodGet, "/v1/namespace/list")
|
||||
reached := false
|
||||
rr := httptest.NewRecorder()
|
||||
server.Auth(func(w http.ResponseWriter, r *http.Request) {
|
||||
reached = true
|
||||
w.WriteHeader(http.StatusOK)
|
||||
})(rr, req)
|
||||
|
||||
assert.False(t, reached, "failed authentication must not reach the Lance handler")
|
||||
assert.NotEqual(t, http.StatusOK, rr.Code)
|
||||
}
|
||||
|
||||
func TestAuthSignatureOnlyScrubsAccountHeader(t *testing.T) {
|
||||
s3a := setupDefaultAllowAuthServer(t)
|
||||
|
||||
req := forgedS3TablesRequest(t, http.MethodGet, "/buckets")
|
||||
req.Header.Set(s3_constants.AmzAccountId, "admin")
|
||||
|
||||
_, errCode := s3a.iam.AuthSignatureOnly(req)
|
||||
assert.NotEqual(t, s3err.ErrNone, errCode)
|
||||
assert.Empty(t, req.Header.Get(s3_constants.AmzAccountId))
|
||||
}
|
||||
@@ -283,7 +283,9 @@ func (h *S3TablesHandler) getAccountID(r *http.Request) string {
|
||||
|
||||
if accountID := r.Header.Get(s3_constants.AmzAccountId); accountID != "" {
|
||||
if principal := normalizePrincipalID(accountID); principal != "" {
|
||||
return principal
|
||||
if principal != s3_constants.AccountAdminId || hasAdminAction(getIdentityActions(r)) {
|
||||
return principal
|
||||
}
|
||||
}
|
||||
}
|
||||
return h.accountID
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
package s3tables
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestGetAccountIDRejectsHeaderAdmin(t *testing.T) {
|
||||
h := NewS3TablesHandler()
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Header.Set(s3_constants.AmzAccountId, s3_constants.AccountAdminId)
|
||||
|
||||
assert.NotEqual(t, s3_constants.AccountAdminId, h.getAccountID(req),
|
||||
"a client-supplied account header must not resolve to the admin principal")
|
||||
}
|
||||
|
||||
func TestGetAccountIDHeaderBranchResolvesCaller(t *testing.T) {
|
||||
h := NewS3TablesHandler()
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Header.Set(s3_constants.AmzAccountId, "alice")
|
||||
|
||||
assert.Equal(t, "alice", h.getAccountID(req))
|
||||
}
|
||||
|
||||
func TestCheckPermissionDenyPolicyBindsNonAdmin(t *testing.T) {
|
||||
denyAll := `{"Statement":[{"Effect":"Deny","Principal":"*","Action":"s3tables:DeleteTableBucket"}]}`
|
||||
assert.False(t, CheckPermissionWithContext("s3tables:DeleteTableBucket", "mallory", "owner123", denyAll, "arn:aws:s3tables:us-east-1:000000000000:bucket/victim",
|
||||
&PolicyContext{DefaultAllow: true}))
|
||||
}
|
||||
Reference in New Issue
Block a user