diff --git a/weed/iamapi/iamapi_authz_test.go b/weed/iamapi/iamapi_authz_test.go index 5684a163b7..c4d4db0a34 100644 --- a/weed/iamapi/iamapi_authz_test.go +++ b/weed/iamapi/iamapi_authz_test.go @@ -2,10 +2,16 @@ package iamapi import ( "context" + "crypto/hmac" + "crypto/sha1" "crypto/sha256" + "encoding/base64" "fmt" + "io" "net/http" "net/http/httptest" + "net/url" + "strconv" "strings" "testing" "time" @@ -141,3 +147,69 @@ func TestIamManagementAllowedForAdmin(t *testing.T) { "", "Action=CreateUser&UserName=new-user&Version=2010-05-08") assert.Equal(t, http.StatusOK, resp.StatusCode) } + +// The implicit UserName must come from the identity authentication verified, +// never from re-parsing the Authorization header: a request can carry presigned +// credentials for one user alongside a junk SigV4 header naming another. +func TestImplicitUserNameUsesVerifiedIdentity(t *testing.T) { + server := newAuthzTestServer(t) + expires := strconv.FormatInt(time.Now().Add(10*time.Minute).Unix(), 10) + sts := "POST\n\napplication/x-www-form-urlencoded\n" + expires + "\n/" + mac := hmac.New(sha1.New, []byte("wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY")) + mac.Write([]byte(sts)) + sig := base64.StdEncoding.EncodeToString(mac.Sum(nil)) + q := url.Values{"AWSAccessKeyId": {"AKIAIOSFODNN7EXAMPLE"}, "Expires": {expires}, "Signature": {sig}} + + post := func(t *testing.T, body string) (int, string) { + t.Helper() + req, err := http.NewRequest(http.MethodPost, server.URL+"/?"+q.Encode(), strings.NewReader(body)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=AKIATESTFAKEADMIN0001/20261021/us-east-1/iam/aws4_request, SignedHeaders=host, Signature=00") + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + b, err := io.ReadAll(resp.Body) + require.NoError(t, err) + t.Logf("%d %s", resp.StatusCode, b) + return resp.StatusCode, string(b) + } + + for _, action := range []string{"ListAccessKeys", "CreateAccessKey", "GetUser"} { + t.Run(action, func(t *testing.T) { + code, body := post(t, "Action="+action+"&Version=2010-05-08") + assert.Equal(t, http.StatusOK, code) + assert.NotContains(t, body, "iam_admin") + assert.Contains(t, body, "power_user") + }) + } + + // ListUserPolicies is not self-service: a caller without IAM rights is + // denied whichever name the implicit resolution lands on. + t.Run("ListUserPolicies", func(t *testing.T) { + code, body := post(t, "Action=ListUserPolicies&Version=2010-05-08") + assert.Equal(t, http.StatusForbidden, code) + assert.NotContains(t, body, "iam_admin") + }) + + // A write aimed at the header-named admin's key resolves to the verified + // caller instead: the key is not power_user's, so it is not found. + t.Run("UpdateAccessKey", func(t *testing.T) { + code, body := post(t, "Action=UpdateAccessKey&AccessKeyId=AKIATESTFAKEADMIN0001&Status=Inactive&Version=2010-05-08") + assert.Equal(t, http.StatusNotFound, code) + assert.Contains(t, body, "NoSuchEntity") + }) + + t.Run("DeleteAccessKey", func(t *testing.T) { + code, _ := post(t, "Action=DeleteAccessKey&AccessKeyId=AKIATESTFAKEADMIN0001&Version=2010-05-08") + assert.Equal(t, http.StatusOK, code) + }) + + // The admin key the junk header named is untouched: still present and active. + resp := postSignedIamAction(t, server.URL, "AKIATESTFAKEADMIN0001", "testAdminSecretFake0000000000000000000000", + "", "Action=ListAccessKeys&Version=2010-05-08") + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + assert.Contains(t, string(body), "AKIATESTFAKEADMIN0001") + assert.Contains(t, string(body), "Active") +} diff --git a/weed/iamapi/iamapi_management_handlers.go b/weed/iamapi/iamapi_management_handlers.go index 76fe3154e9..85381e2696 100644 --- a/weed/iamapi/iamapi_management_handlers.go +++ b/weed/iamapi/iamapi_management_handlers.go @@ -22,6 +22,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" "github.com/seaweedfs/seaweedfs/weed/s3api" "github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" "github.com/seaweedfs/seaweedfs/weed/util/request_id" ) @@ -1265,53 +1266,19 @@ func (iama *IamApiServer) DeleteAccessKey(s3cfg *iam_pb.S3ApiConfiguration, valu return resp } -// handleImplicitUsername adds username who signs the request to values if 'username' is not specified. -// According to AWS documentation: "If you do not specify a user name, IAM determines the user name -// implicitly based on the Amazon Web Services access key ID signing the request." -// This function extracts the AccessKeyId from the SigV4 credential and looks up the corresponding -// identity name in the credential store. +// handleImplicitUsername adds the name of the authenticated caller to values +// when 'UserName' is not specified, per AWS: "If you do not specify a user name, +// IAM determines the user name implicitly based on the Amazon Web Services +// access key ID signing the request." The name comes from the identity the +// authentication middleware verified, which the request context carries; the +// Authorization header itself is attacker-controlled and is never re-parsed. func (iama *IamApiServer) handleImplicitUsername(r *http.Request, values url.Values) { - if len(r.Header["Authorization"]) == 0 || values.Get("UserName") != "" { + if values.Get("UserName") != "" { return } - // Log presence of auth header without exposing sensitive signature material - glog.V(4).Infof("Authorization header present, extracting access key") - // Parse AWS SigV4 Authorization header format: - // "AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/20130524/us-east-1/iam/aws4_request, ..." - s := strings.Split(r.Header["Authorization"][0], "Credential=") - if len(s) < 2 { - return + if name := s3_constants.GetIdentityNameFromContext(r); name != "" { + values.Set("UserName", name) } - s = strings.Split(s[1], ",") - if len(s) < 1 { - return - } - s = strings.Split(s[0], "/") - if len(s) < 1 { - return - } - // s[0] is the AccessKeyId - accessKeyId := s[0] - if accessKeyId == "" { - return - } - // Nil-guard: ensure iam is initialized before lookup - if iama.iam == nil { - glog.V(4).Infof("IAM not initialized, cannot look up access key") - return - } - // Look up the identity by access key to get the username - identity, _, found := iama.iam.LookupByAccessKey(accessKeyId) - if !found { - // Mask access key in logs - show only first 4 chars - maskedKey := accessKeyId - if len(accessKeyId) > 4 { - maskedKey = accessKeyId[:4] + "***" - } - glog.V(4).Infof("Access key %s not found in credential store", maskedKey) - return - } - values.Set("UserName", identity.Name) } // targetedCredentialManager returns the credential manager through which a change to the named @@ -1470,6 +1437,7 @@ func (iama *IamApiServer) DoActions(w http.ResponseWriter, r *http.Request) { } } case "GetUser": + iama.handleImplicitUsername(r, values) userName := values.Get("UserName") var err *IamError response, err = iama.GetUser(s3cfg, userName) diff --git a/weed/iamapi/iamapi_test.go b/weed/iamapi/iamapi_test.go index 893e907008..6000aff6a3 100644 --- a/weed/iamapi/iamapi_test.go +++ b/weed/iamapi/iamapi_test.go @@ -21,6 +21,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" "github.com/seaweedfs/seaweedfs/weed/s3api" "github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/util/request_id" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -524,32 +525,13 @@ func executeRequest(req *http.Request, v interface{}) (*httptest.ResponseRecorde } func TestHandleImplicitUsername(t *testing.T) { - // Create a mock IamApiServer with credential store - // The handleImplicitUsername function now looks up the username from the - // credential store based on AccessKeyId, not from the region field in the auth header. - // Note: Using obviously fake access keys to avoid CI secret scanner false positives + // The implicit name is the authenticated identity recorded in the request + // context by the auth middleware; request headers are never re-parsed. + iama := &IamApiServer{} - // Create IAM directly as struct literal (same pattern as other tests) - iam := &s3api.IdentityAccessManagement{} - - // Load test credentials - map access key to identity name - testConfig := &iam_pb.S3ApiConfiguration{ - Identities: []*iam_pb.Identity{ - { - Name: "testuser1", - Credentials: []*iam_pb.Credential{ - {AccessKey: "AKIATESTFAKEKEY000001", SecretKey: "testsecretfake"}, - }, - }, - }, - } - err := iam.LoadS3ApiConfigurationFromBytes(mustMarshalJSON(t, testConfig)) - if err != nil { - t.Fatalf("Failed to load test config: %v", err) - } - - iama := &IamApiServer{ - iam: iam, + withIdentity := func(name string) *http.Request { + r := &http.Request{Header: http.Header{}} + return r.WithContext(s3_constants.SetIdentityNameInContext(r.Context(), name)) } var tests = []struct { @@ -557,14 +539,14 @@ func TestHandleImplicitUsername(t *testing.T) { values url.Values userName string }{ - // No authorization header - should not set username + // No authenticated identity - should not set username {&http.Request{}, url.Values{}, ""}, - // Valid auth header with known access key - should look up and find "testuser1" - {&http.Request{Header: http.Header{"Authorization": []string{"AWS4-HMAC-SHA256 Credential=AKIATESTFAKEKEY000001/20220420/us-east-1/iam/aws4_request, SignedHeaders=content-type;host;x-amz-date, Signature=fakesignature0123456789abcdef"}}}, url.Values{}, "testuser1"}, - // Malformed auth header (no Credential=) - should not set username - {&http.Request{Header: http.Header{"Authorization": []string{"AWS4-HMAC-SHA256 =AKIATESTFAKEKEY000001/20220420/test1/iam/aws4_request, SignedHeaders=content-type;host;x-amz-date, Signature=fakesignature0123456789abcdef"}}}, url.Values{}, ""}, - // Unknown access key - should not set username - {&http.Request{Header: http.Header{"Authorization": []string{"AWS4-HMAC-SHA256 Credential=AKIATESTUNKNOWN000000/20220420/us-east-1/iam/aws4_request, SignedHeaders=content-type;host;x-amz-date, Signature=fakesignature0123456789abcdef"}}}, url.Values{}, ""}, + // Authenticated identity in context - sets its name + {withIdentity("testuser1"), url.Values{}, "testuser1"}, + // A client-controlled Authorization header naming another user is ignored + {&http.Request{Header: http.Header{"Authorization": []string{"AWS4-HMAC-SHA256 Credential=AKIATESTFAKEKEY000001/20220420/us-east-1/iam/aws4_request, SignedHeaders=content-type;host;x-amz-date, Signature=fakesignature0123456789abcdef"}}}, url.Values{}, ""}, + // An explicit UserName is left alone + {withIdentity("testuser1"), url.Values{"UserName": {"other"}}, "other"}, } for i, test := range tests { diff --git a/weed/s3api/s3api_embedded_iam.go b/weed/s3api/s3api_embedded_iam.go index 46cfddc3a6..bce22df8aa 100644 --- a/weed/s3api/s3api_embedded_iam.go +++ b/weed/s3api/s3api_embedded_iam.go @@ -2594,53 +2594,19 @@ func (e *EmbeddedIamApi) ListGroupPolicies(s3cfg *iam_pb.S3ApiConfiguration, val return resp, nil } -// handleImplicitUsername adds username who signs the request to values if 'username' is not specified. -// According to AWS documentation: "If you do not specify a user name, IAM determines the user name -// implicitly based on the Amazon Web Services access key ID signing the request." -// This function extracts the AccessKeyId from the SigV4 credential and looks up the corresponding -// identity name in the credential store. +// handleImplicitUsername adds the name of the authenticated caller to values +// when 'UserName' is not specified, per AWS: "If you do not specify a user name, +// IAM determines the user name implicitly based on the Amazon Web Services +// access key ID signing the request." The name comes from the identity the +// authentication middleware verified, which the request context carries; the +// Authorization header itself is attacker-controlled and is never re-parsed. func (e *EmbeddedIamApi) handleImplicitUsername(r *http.Request, values url.Values) { - if len(r.Header["Authorization"]) == 0 || values.Get("UserName") != "" { + if values.Get("UserName") != "" { return } - // Log presence of auth header without exposing sensitive signature material - glog.V(4).Infof("Authorization header present, extracting access key") - // Parse AWS SigV4 Authorization header format: - // "AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/20130524/us-east-1/iam/aws4_request, ..." - s := strings.Split(r.Header["Authorization"][0], "Credential=") - if len(s) < 2 { - return + if name := s3_constants.GetIdentityNameFromContext(r); name != "" { + values.Set("UserName", name) } - s = strings.Split(s[1], ",") - if len(s) < 1 { - return - } - s = strings.Split(s[0], "/") - if len(s) < 1 { - return - } - // s[0] is the AccessKeyId - accessKeyId := s[0] - if accessKeyId == "" { - return - } - // Nil-guard: ensure iam is initialized before lookup - if e.iam == nil { - glog.V(4).Infof("IAM not initialized, cannot look up access key") - return - } - // Look up the identity by access key to get the username - identity, _, found := e.iam.LookupByAccessKey(accessKeyId) - if !found { - // Mask access key in logs - show only first 4 chars - maskedKey := accessKeyId - if len(accessKeyId) > 4 { - maskedKey = accessKeyId[:4] + "***" - } - glog.V(4).Infof("Access key %s not found in credential store", maskedKey) - return - } - values.Set("UserName", identity.Name) } // iamSelfServiceActions are actions that users can perform on their own resources without admin rights. diff --git a/weed/s3api/s3api_embedded_iam_test.go b/weed/s3api/s3api_embedded_iam_test.go index 55ff8ed9af..2e7f9206af 100644 --- a/weed/s3api/s3api_embedded_iam_test.go +++ b/weed/s3api/s3api_embedded_iam_test.go @@ -372,13 +372,13 @@ func TestEmbeddedIamGetUserImplicitUsername(t *testing.T) { t.Fatalf("failed to load iam config: %v", err) } - // GetUser request with NO UserName, but signed by accessKey. + // GetUser request with NO UserName, carrying the identity authentication + // verified for accessKey, the way the middleware records it. form := url.Values{} form.Set("Action", "GetUser") req, _ := http.NewRequest(http.MethodPost, "/", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential="+accessKey+ - "/20220420/us-east-1/iam/aws4_request, SignedHeaders=content-type;host;x-amz-date, Signature=fakesig") + req = req.WithContext(SetIdentityNameInContext(req.Context(), "TestUser")) out := iamGetUserResponse{} rr, err := executeEmbeddedIamRequest(api, req, &out) @@ -1578,29 +1578,15 @@ func TestEmbeddedIamDeleteAccessKey(t *testing.T) { assert.Len(t, api.mockConfig.Identities[0].Credentials, 0) } -// TestEmbeddedIamHandleImplicitUsername tests implicit username extraction from authorization header +// TestEmbeddedIamHandleImplicitUsername checks that the implicit user name is +// taken from the authenticated identity in the request context, never from +// re-parsing the Authorization header. func TestEmbeddedIamHandleImplicitUsername(t *testing.T) { - // Create IAM with test credentials - the handleImplicitUsername function now looks - // up the username from the credential store based on AccessKeyId - // Note: Using obviously fake access keys to avoid secret scanner false positives - iam := &IdentityAccessManagement{} - testConfig := &iam_pb.S3ApiConfiguration{ - Identities: []*iam_pb.Identity{ - { - Name: "testuser1", - Credentials: []*iam_pb.Credential{ - {AccessKey: UserAccessKeyPrefix + "TESTFAKEKEY000001", SecretKey: "testsecretfake"}, - }, - }, - }, - } - err := iam.LoadS3ApiConfigurationFromBytes(mustMarshalJSON(testConfig)) - if err != nil { - t.Fatalf("Failed to load test config: %v", err) - } + embeddedApi := &EmbeddedIamApi{} - embeddedApi := &EmbeddedIamApi{ - iam: iam, + withIdentity := func(name string) *http.Request { + r := &http.Request{Header: http.Header{}} + return r.WithContext(SetIdentityNameInContext(r.Context(), name)) } var tests = []struct { @@ -1608,14 +1594,14 @@ func TestEmbeddedIamHandleImplicitUsername(t *testing.T) { values url.Values userName string }{ - // No authorization header - should not set username + // No authenticated identity - should not set username {&http.Request{}, url.Values{}, ""}, - // Valid auth header with known access key - should look up and find "testuser1" - {&http.Request{Header: http.Header{"Authorization": []string{"AWS4-HMAC-SHA256 Credential=" + UserAccessKeyPrefix + "TESTFAKEKEY000001/20220420/us-east-1/iam/aws4_request, SignedHeaders=content-type;host;x-amz-date, Signature=fakesignature0123456789abcdef"}}}, url.Values{}, "testuser1"}, - // Malformed auth header (no Credential=) - should not set username - {&http.Request{Header: http.Header{"Authorization": []string{"AWS4-HMAC-SHA256 =" + UserAccessKeyPrefix + "TESTFAKEKEY000001/20220420/test1/iam/aws4_request, SignedHeaders=content-type;host;x-amz-date, Signature=fakesignature0123456789abcdef"}}}, url.Values{}, ""}, - // Unknown access key - should not set username - {&http.Request{Header: http.Header{"Authorization": []string{"AWS4-HMAC-SHA256 Credential=" + UserAccessKeyPrefix + "TESTUNKNOWN000000/20220420/us-east-1/iam/aws4_request, SignedHeaders=content-type;host;x-amz-date, Signature=fakesignature0123456789abcdef"}}}, url.Values{}, ""}, + // Authenticated identity in context - sets its name + {withIdentity("testuser1"), url.Values{}, "testuser1"}, + // A client-controlled Authorization header naming another user is ignored + {&http.Request{Header: http.Header{"Authorization": []string{"AWS4-HMAC-SHA256 Credential=" + UserAccessKeyPrefix + "TESTFAKEKEY000001/20220420/us-east-1/iam/aws4_request, SignedHeaders=content-type;host;x-amz-date, Signature=fakesignature0123456789abcdef"}}}, url.Values{}, ""}, + // An explicit UserName is left alone + {withIdentity("testuser1"), url.Values{"UserName": {"other"}}, "other"}, } for i, test := range tests {