mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-05-23 02:01:32 +00:00
fix(s3): strip client-supplied X-SeaweedFS-Principal/Session-Token in AuthSignatureOnly (#9120)
* fix(s3): strip client-supplied X-SeaweedFS-Principal/Session-Token in AuthSignatureOnly AuthSignatureOnly is the only auth gate in front of S3Tables routes (incl. CreateTableBucket) and UnifiedPostHandler, but unlike authenticateRequestInternal it did not clear the internal IAM trust headers before running signature verification. S3Tables authorizeIAMAction reads X-SeaweedFS-Principal directly from the request and prefers it over the authenticated identity's PrincipalArn, so a signed low-privilege caller could append that header after signing (unsigned header, SigV4 still verifies) and have IAM policy evaluated against a spoofed principal, bypassing authorization. Clear both X-SeaweedFS-Principal and X-SeaweedFS-Session-Token at the top of AuthSignatureOnly, mirroring the existing guard in authenticateRequestInternal. Add a regression test covering the header-injection path. * refactor(s3): route AuthSignatureOnly through authenticateRequestInternal Addresses review feedback: both entry points were independently maintaining the internal-IAM-header stripping and the auth-type dispatch switch. Collapse AuthSignatureOnly into a thin wrapper around authenticateRequestInternal so the security-critical header scrub and the signature-verify switch live in one place. Post-auth behavior unique to AuthSignatureOnly (AmzAccountId header) stays inline. No functional change beyond two harmless telemetry tweaks that now match authenticateRequestInternal: the per-branch glog verbosity shifts from V(3) to V(4), and the anonymous-found path now sets AmzAuthType. * refactor(s3): centralize X-SeaweedFS-Principal/Session-Token header names Introduce SeaweedFSPrincipalHeader and SeaweedFSSessionTokenHeader in weed/s3api/s3_constants so the trust-header literals are defined once and referenced consistently by the auth scrub, JWT auth path, bucket policy principal resolution, IAM authorization, and S3Tables IAM evaluation. Replace every remaining usage in weed/s3api and weed/s3api/s3tables. This removes the drift risk the reviewer called out: adding another call site with a typo can no longer silently bypass the scrub. Pure rename, no behavior change. No-op integration-test helper in test/s3/iam/s3_iam_framework.go left untouched (separate module, and the server now strips the client-supplied value regardless).
This commit is contained in:
@@ -1337,11 +1337,13 @@ func (iam *IdentityAccessManagement) authenticateRequestInternal(r *http.Request
|
||||
var found bool
|
||||
var amzAuthType string
|
||||
|
||||
// SECURITY: Prevent clients from spoofing internal IAM headers
|
||||
// SECURITY: Prevent clients from spoofing internal IAM headers.
|
||||
// These headers are only set by the server after successful JWT authentication
|
||||
// Clearing them here prevents privilege escalation via header injection
|
||||
r.Header.Del("X-SeaweedFS-Principal")
|
||||
r.Header.Del("X-SeaweedFS-Session-Token")
|
||||
// and are trusted by downstream authorization (including S3Tables IAM checks
|
||||
// reached via AuthSignatureOnly). Clearing them here — the shared entry point
|
||||
// for every auth path — prevents privilege escalation via header injection.
|
||||
r.Header.Del(s3_constants.SeaweedFSPrincipalHeader)
|
||||
r.Header.Del(s3_constants.SeaweedFSSessionTokenHeader)
|
||||
|
||||
reqAuthType := getRequestAuthType(r)
|
||||
|
||||
@@ -1478,53 +1480,13 @@ func (iam *IdentityAccessManagement) authRequestWithAuthType(r *http.Request, ac
|
||||
|
||||
// AuthSignatureOnly performs only signature verification without any authorization checks.
|
||||
// This is used for IAM API operations where authorization is handled separately based on
|
||||
// the specific IAM action (e.g., self-service vs admin operations).
|
||||
// the specific IAM action (e.g., self-service vs admin operations). It delegates to
|
||||
// authenticateRequestInternal so the internal-IAM-header stripping and the auth-type
|
||||
// dispatch stay in one place — any future fix applied there automatically covers
|
||||
// S3Tables, UnifiedPostHandler, and the other AuthSignatureOnly callers.
|
||||
// Returns the authenticated identity and any signature verification error.
|
||||
func (iam *IdentityAccessManagement) AuthSignatureOnly(r *http.Request) (*Identity, s3err.ErrorCode) {
|
||||
|
||||
var identity *Identity
|
||||
var s3Err s3err.ErrorCode
|
||||
var authType string
|
||||
switch getRequestAuthType(r) {
|
||||
case authTypeUnknown:
|
||||
glog.V(3).Infof("unknown auth type")
|
||||
r.Header.Set(s3_constants.AmzAuthType, "Unknown")
|
||||
return identity, s3err.ErrAccessDenied
|
||||
case authTypePresignedV2, authTypeSignedV2:
|
||||
glog.V(3).Infof("v2 auth type")
|
||||
identity, s3Err = iam.isReqAuthenticatedV2(r)
|
||||
authType = "SigV2"
|
||||
case authTypeStreamingSigned, authTypeSigned, authTypePresigned:
|
||||
glog.V(3).Infof("v4 auth type")
|
||||
identity, s3Err = iam.reqSignatureV4Verify(r)
|
||||
authType = "SigV4"
|
||||
|
||||
case authTypeStreamingUnsigned:
|
||||
glog.V(3).Infof("unsigned streaming upload")
|
||||
identity, s3Err = iam.reqSignatureV4Verify(r)
|
||||
authType = "SigV4"
|
||||
case authTypeJWT:
|
||||
glog.V(3).Infof("jwt auth type detected, iamIntegration != nil? %t", iam.iamIntegration != nil)
|
||||
r.Header.Set(s3_constants.AmzAuthType, "Jwt")
|
||||
if iam.iamIntegration != nil {
|
||||
identity, s3Err = iam.authenticateJWTWithIAM(r)
|
||||
authType = "Jwt"
|
||||
} else {
|
||||
glog.V(2).Infof("IAM integration is nil, returning ErrNotImplemented")
|
||||
return identity, s3err.ErrNotImplemented
|
||||
}
|
||||
case authTypeAnonymous:
|
||||
if ident, found := iam.LookupAnonymous(); found {
|
||||
return ident, s3err.ErrNone
|
||||
}
|
||||
return nil, s3err.ErrAccessDenied
|
||||
default:
|
||||
return identity, s3err.ErrNotImplemented
|
||||
}
|
||||
|
||||
if len(authType) > 0 {
|
||||
r.Header.Set(s3_constants.AmzAuthType, authType)
|
||||
}
|
||||
identity, s3Err, _ := iam.authenticateRequestInternal(r)
|
||||
if s3Err != s3err.ErrNone {
|
||||
return identity, s3Err
|
||||
}
|
||||
@@ -1631,7 +1593,7 @@ func (identity *Identity) isAdmin() bool {
|
||||
func buildPrincipalARN(identity *Identity, r *http.Request) string {
|
||||
// Check if principal ARN was already set by JWT authentication
|
||||
if r != nil {
|
||||
if principalARN := r.Header.Get("X-SeaweedFS-Principal"); principalARN != "" {
|
||||
if principalARN := r.Header.Get(s3_constants.SeaweedFSPrincipalHeader); principalARN != "" {
|
||||
glog.V(4).Infof("buildPrincipalARN: Using principal ARN from header: %s", principalARN)
|
||||
return principalARN
|
||||
}
|
||||
@@ -2000,8 +1962,8 @@ func (iam *IdentityAccessManagement) authenticateJWTWithIAM(r *http.Request) (*I
|
||||
}
|
||||
|
||||
// Store session info in request headers for later authorization
|
||||
r.Header.Set("X-SeaweedFS-Session-Token", iamIdentity.SessionToken)
|
||||
r.Header.Set("X-SeaweedFS-Principal", iamIdentity.Principal)
|
||||
r.Header.Set(s3_constants.SeaweedFSSessionTokenHeader, iamIdentity.SessionToken)
|
||||
r.Header.Set(s3_constants.SeaweedFSPrincipalHeader, iamIdentity.Principal)
|
||||
|
||||
return identity, s3err.ErrNone
|
||||
}
|
||||
@@ -2121,7 +2083,7 @@ func (iam *IdentityAccessManagement) VerifyActionPermission(r *http.Request, ide
|
||||
// JWT/STS identities (no Actions or having a session token) use IAM authorization.
|
||||
// IMPORTANT: We MUST prioritize IAM authorization for any request with a session token
|
||||
// to ensure that session policies are correctly enforced.
|
||||
hasSessionToken := r.Header.Get("X-SeaweedFS-Session-Token") != "" ||
|
||||
hasSessionToken := r.Header.Get(s3_constants.SeaweedFSSessionTokenHeader) != "" ||
|
||||
r.Header.Get("X-Amz-Security-Token") != "" ||
|
||||
r.URL.Query().Get("X-Amz-Security-Token") != ""
|
||||
iam.m.RLock()
|
||||
@@ -2167,9 +2129,9 @@ func (iam *IdentityAccessManagement) authorizeWithIAM(r *http.Request, identity
|
||||
ctx := r.Context()
|
||||
|
||||
// Get session info from request headers
|
||||
// First check for JWT-based authentication headers (X-SeaweedFS-Session-Token)
|
||||
sessionToken := r.Header.Get("X-SeaweedFS-Session-Token")
|
||||
principal := r.Header.Get("X-SeaweedFS-Principal")
|
||||
// First check for JWT-based authentication headers (SeaweedFSSessionTokenHeader)
|
||||
sessionToken := r.Header.Get(s3_constants.SeaweedFSSessionTokenHeader)
|
||||
principal := r.Header.Get(s3_constants.SeaweedFSPrincipalHeader)
|
||||
|
||||
// Fallback to AWS Signature V4 STS token if JWT token not present
|
||||
// This handles the case where STS AssumeRoleWithWebIdentity generates temporary credentials
|
||||
|
||||
@@ -129,6 +129,24 @@ func TestReproIssue7912(t *testing.T) {
|
||||
// No Authorization header
|
||||
_, errCode3 := iam.AuthSignatureOnly(r3)
|
||||
assert.Equal(t, s3err.ErrAccessDenied, errCode3, "AuthSignatureOnly should be denied with unsigned streaming if no auth header")
|
||||
|
||||
// Verify fix: client-supplied X-SeaweedFS-Principal / X-SeaweedFS-Session-Token
|
||||
// headers must be stripped, otherwise a signed low-privilege caller could
|
||||
// spoof the principal that downstream IAM authorization (e.g. S3Tables
|
||||
// CreateTableBucket) evaluates against.
|
||||
r4 := httptest.NewRequest(http.MethodGet, "http://localhost:8333/", nil)
|
||||
r4.Host = "localhost:8333"
|
||||
err = signRawHTTPRequest(context.Background(), r4, "readonly_access_key", "readonly_secret_key", "us-east-1")
|
||||
require.NoError(t, err)
|
||||
// Attacker injects these headers after signing; they are not part of
|
||||
// SignedHeaders, so SigV4 verification still passes.
|
||||
r4.Header.Set(s3_constants.SeaweedFSPrincipalHeader, "arn:aws:iam::123456789012:root")
|
||||
r4.Header.Set(s3_constants.SeaweedFSSessionTokenHeader, "spoofed-session-token")
|
||||
|
||||
_, errCode4 := iam.AuthSignatureOnly(r4)
|
||||
assert.Equal(t, s3err.ErrNone, errCode4)
|
||||
assert.Empty(t, r4.Header.Get(s3_constants.SeaweedFSPrincipalHeader), "client-supplied X-SeaweedFS-Principal must be stripped")
|
||||
assert.Empty(t, r4.Header.Get(s3_constants.SeaweedFSSessionTokenHeader), "client-supplied X-SeaweedFS-Session-Token must be stripped")
|
||||
})
|
||||
|
||||
t.Run("Wrong secret key", func(t *testing.T) {
|
||||
|
||||
@@ -28,8 +28,8 @@ func TestAuthorizeWithIAMSessionTokenExtraction(t *testing.T) {
|
||||
}
|
||||
|
||||
// Extract tokens the same way authorizeWithIAM does
|
||||
sessionToken := req.Header.Get("X-SeaweedFS-Session-Token")
|
||||
principal := req.Header.Get("X-SeaweedFS-Principal")
|
||||
sessionToken := req.Header.Get(s3_constants.SeaweedFSSessionTokenHeader)
|
||||
principal := req.Header.Get(s3_constants.SeaweedFSPrincipalHeader)
|
||||
|
||||
assert.Equal(t, "jwt-token-123", sessionToken, "Should extract JWT session token from header")
|
||||
assert.Equal(t, "arn:aws:iam::user/test", principal, "Should extract principal from header")
|
||||
@@ -44,8 +44,8 @@ func TestAuthorizeWithIAMSessionTokenExtraction(t *testing.T) {
|
||||
}
|
||||
|
||||
// Extract tokens the same way authorizeWithIAM does
|
||||
sessionToken := req.Header.Get("X-SeaweedFS-Session-Token")
|
||||
principal := req.Header.Get("X-SeaweedFS-Principal")
|
||||
sessionToken := req.Header.Get(s3_constants.SeaweedFSSessionTokenHeader)
|
||||
principal := req.Header.Get(s3_constants.SeaweedFSPrincipalHeader)
|
||||
|
||||
// If JWT token is empty, should fallback to X-Amz-Security-Token
|
||||
if sessionToken == "" {
|
||||
@@ -63,7 +63,7 @@ func TestAuthorizeWithIAMSessionTokenExtraction(t *testing.T) {
|
||||
}
|
||||
|
||||
// Extract tokens the same way authorizeWithIAM does
|
||||
sessionToken := req.Header.Get("X-SeaweedFS-Session-Token")
|
||||
sessionToken := req.Header.Get(s3_constants.SeaweedFSSessionTokenHeader)
|
||||
if sessionToken == "" {
|
||||
sessionToken = req.Header.Get("X-Amz-Security-Token")
|
||||
if sessionToken == "" {
|
||||
@@ -85,7 +85,7 @@ func TestAuthorizeWithIAMSessionTokenExtraction(t *testing.T) {
|
||||
}
|
||||
|
||||
// Extract tokens the same way authorizeWithIAM does
|
||||
sessionToken := req.Header.Get("X-SeaweedFS-Session-Token")
|
||||
sessionToken := req.Header.Get(s3_constants.SeaweedFSSessionTokenHeader)
|
||||
if sessionToken == "" {
|
||||
sessionToken = req.Header.Get("X-Amz-Security-Token")
|
||||
}
|
||||
|
||||
@@ -149,6 +149,17 @@ const (
|
||||
SeaweedFSSSEKMSBaseIVHeader = "X-SeaweedFS-SSE-KMS-Base-IV" // Header for passing base IV for multipart SSE-KMS
|
||||
SeaweedFSSSES3BaseIVHeader = "X-SeaweedFS-SSE-S3-Base-IV" // Header for passing base IV for multipart SSE-S3
|
||||
SeaweedFSSSES3KeyDataHeader = "X-SeaweedFS-SSE-S3-Key-Data" // Header for passing key data for multipart SSE-S3
|
||||
|
||||
// SeaweedFSPrincipalHeader and SeaweedFSSessionTokenHeader carry the
|
||||
// server-derived principal ARN and session token from the auth layer to
|
||||
// downstream authorization (IAM policy evaluation, S3Tables IAM checks).
|
||||
// These headers are trusted: callers of AuthSignatureOnly /
|
||||
// authenticateRequestInternal must scrub any client-supplied values at the
|
||||
// very start of authentication to prevent privilege escalation via header
|
||||
// injection. Prefer these constants over literals so that any future scrub
|
||||
// or consumer cannot drift by typo.
|
||||
SeaweedFSPrincipalHeader = "X-SeaweedFS-Principal"
|
||||
SeaweedFSSessionTokenHeader = "X-SeaweedFS-Session-Token"
|
||||
)
|
||||
|
||||
// Non-Standard S3 HTTP request constants
|
||||
|
||||
@@ -543,11 +543,11 @@ func testJWTAuthorizationWithRole(t *testing.T, iam *IdentityAccessManagement, i
|
||||
// Create test request
|
||||
req := httptest.NewRequest("GET", "/"+bucket+"/"+object, http.NoBody)
|
||||
req.Header.Set("Authorization", "Bearer "+token)
|
||||
req.Header.Set("X-SeaweedFS-Session-Token", token)
|
||||
req.Header.Set(s3_constants.SeaweedFSSessionTokenHeader, token)
|
||||
|
||||
// Use a proper principal ARN format that matches what STS would generate
|
||||
principalArn := "arn:aws:sts::assumed-role/" + roleName + "/test-session"
|
||||
req.Header.Set("X-SeaweedFS-Principal", principalArn)
|
||||
req.Header.Set(s3_constants.SeaweedFSPrincipalHeader, principalArn)
|
||||
|
||||
// Test authorization
|
||||
if iam.iamIntegration == nil {
|
||||
|
||||
@@ -79,7 +79,7 @@ func hasSessionToken(r *http.Request) bool {
|
||||
}
|
||||
|
||||
func extractSessionToken(r *http.Request) string {
|
||||
if token := r.Header.Get("X-SeaweedFS-Session-Token"); token != "" {
|
||||
if token := r.Header.Get(s3_constants.SeaweedFSSessionTokenHeader); token != "" {
|
||||
return token
|
||||
}
|
||||
if token := r.Header.Get("X-Amz-Security-Token"); token != "" {
|
||||
@@ -94,7 +94,7 @@ func (h *S3TablesHandler) authorizeIAMAction(r *http.Request, identityPolicyName
|
||||
glog.V(2).Infof("S3Tables: %v", err)
|
||||
return false, err
|
||||
}
|
||||
principal := r.Header.Get("X-SeaweedFS-Principal")
|
||||
principal := r.Header.Get(s3_constants.SeaweedFSPrincipalHeader)
|
||||
if principal == "" {
|
||||
principal = getIdentityPrincipalArn(r)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user