diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 1e93bb632..48a5ccbc0 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -169,20 +169,17 @@ func NewIdentityAccessManagementWithStore(option *S3ApiServerOption, explicitSto if err := iam.loadS3ApiConfigurationFromFile(option.Config); err != nil { glog.Fatalf("fail to load config file %s: %v", option.Config, err) } - iam.useStaticConfig = true // Track identity names from static config to protect them from dynamic updates + // Must be done under lock to avoid race conditions iam.m.Lock() + iam.useStaticConfig = true iam.staticIdentityNames = make(map[string]bool) for _, identity := range iam.identities { iam.staticIdentityNames[identity.Name] = true } - iam.m.Unlock() - - // Check if any identities were actually loaded from the config file - iam.m.RLock() configLoaded = len(iam.identities) > 0 - iam.m.RUnlock() + iam.m.Unlock() } else { glog.V(3).Infof("no static config file specified... loading config from credential manager") if err := iam.loadS3ApiConfigurationFromFiler(option); err != nil { @@ -544,17 +541,17 @@ func (iam *IdentityAccessManagement) mergeS3ApiConfiguration(config *iam_pb.S3Ap } // Update or add the identity - if existingIdx := -1; existingIdx >= 0 { - // Find and replace existing dynamic identity - for i, existing := range identities { - if existing.Name == ident.Name { - existingIdx = i - break - } - } - if existingIdx >= 0 { - identities[existingIdx] = t + existingIdx := -1 + for i, existing := range identities { + if existing.Name == ident.Name { + existingIdx = i + break } + } + + if existingIdx >= 0 { + // Replace existing dynamic identity + identities[existingIdx] = t } else { // Add new dynamic identity identities = append(identities, t) @@ -1226,6 +1223,12 @@ func determineIAMAuthPath(sessionToken, principal, principalArn string) iamAuthP // VerifyActionPermission checks if the identity is allowed to perform the action on the resource. // It handles both traditional identities (via Actions) and IAM/STS identities (via Policy). func (iam *IdentityAccessManagement) VerifyActionPermission(r *http.Request, identity *Identity, action Action, bucket, object string) s3err.ErrorCode { + // Fail closed if identity is nil + if identity == nil { + glog.V(3).Infof("VerifyActionPermission called with nil identity for action %s on %s/%s", action, bucket, object) + return s3err.ErrAccessDenied + } + // Traditional identities (with Actions from -s3.config) use legacy auth, // JWT/STS identities (no Actions) use IAM authorization if len(identity.Actions) > 0 { diff --git a/weed/s3api/auth_credentials_subscribe.go b/weed/s3api/auth_credentials_subscribe.go index e2d54e307..06d771cc4 100644 --- a/weed/s3api/auth_credentials_subscribe.go +++ b/weed/s3api/auth_credentials_subscribe.go @@ -59,6 +59,10 @@ func (s3a *S3ApiServer) onIamConfigChange(dir string, oldEntry *filer_pb.Entry, if dir != filer.IamConfigDirectory { return nil } + if s3a.iam != nil && s3a.iam.IsStaticConfig() { + glog.V(1).Infof("Skipping IAM config update for static configuration") + return nil + } // Handle deletion: reset to empty config if newEntry == nil && oldEntry != nil && oldEntry.Name == filer.IamIdentityFile { diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go index c8127c638..7ebdefd1c 100644 --- a/weed/s3api/auth_signature_v4.go +++ b/weed/s3api/auth_signature_v4.go @@ -337,8 +337,16 @@ func (iam *IdentityAccessManagement) validateSTSSessionToken(r *http.Request, se // Verify that the access key in the request matches the one in the session token if sessionInfo.Credentials.AccessKeyId != accessKey { + // Mask access keys to avoid exposing credentials in logs + truncateKey := func(k string) string { + const mask = "***" + if len(k) > 4 { + return k[:4] + mask + } + return mask + } glog.V(2).Infof("Access key mismatch: request has %s, session token has %s", - accessKey, sessionInfo.Credentials.AccessKeyId) + truncateKey(accessKey), truncateKey(sessionInfo.Credentials.AccessKeyId)) return nil, nil, s3err.ErrInvalidAccessKeyID } diff --git a/weed/s3api/auth_signature_v4_sts_test.go b/weed/s3api/auth_signature_v4_sts_test.go index 1d9fd5d41..077d67770 100644 --- a/weed/s3api/auth_signature_v4_sts_test.go +++ b/weed/s3api/auth_signature_v4_sts_test.go @@ -16,9 +16,11 @@ import ( // MockIAMIntegration is a mock implementation of IAM integration for testing type MockIAMIntegration struct { authorizeFunc func(ctx context.Context, identity *IAMIdentity, action Action, bucket, object string, r *http.Request) s3err.ErrorCode + authCalled bool } func (m *MockIAMIntegration) AuthorizeAction(ctx context.Context, identity *IAMIdentity, action Action, bucket, object string, r *http.Request) s3err.ErrorCode { + m.authCalled = true if m.authorizeFunc != nil { return m.authorizeFunc(ctx, identity, action, bucket, object, r) } @@ -169,7 +171,7 @@ func TestVerifyV4SignatureWithSTSIdentity(t *testing.T) { if len(tt.identity.Actions) == 0 && tt.shouldCheckPermissions { if tt.iamIntegration != nil { // When IAM integration exists, it should have been called - // The result depends on what the mock returns + assert.True(t, tt.iamIntegration.authCalled, "IAM integration should have been called for STS identity") } else { assert.Equal(t, s3err.ErrAccessDenied, errCode, "STS identity should be denied without IAM integration") }