diff --git a/weed/s3api/auth_config_default_deny_test.go b/weed/s3api/auth_config_default_deny_test.go new file mode 100644 index 000000000..bb1277ce0 --- /dev/null +++ b/weed/s3api/auth_config_default_deny_test.go @@ -0,0 +1,68 @@ +package s3api + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/stretchr/testify/assert" +) + +// A config file the proto parser reads as empty - an unpopulated secret mount, +// a singular "identity" - used to leave the gateway serving every anonymous +// request, including ListBuckets and bucket creation. +func TestConfigWithoutIdentitiesDeniesAnonymous(t *testing.T) { + for name, config := range map[string]string{ + "no identities key": `{}`, + "empty": `{"identities":[]}`, + "unrecognised key": `{"identity":[{"name":"admin","credentials":[{"accessKey":"adminkey","secretKey":"adminsecret"}],"actions":["Admin"]}]}`, + } { + t.Run(name, func(t *testing.T) { + resetMemoryStore() + clearEnvironmentVariableCredentials(t) + + path := writeTempIamConfig(t, config) + iam := NewIdentityAccessManagementWithStore(&S3ApiServerOption{Config: path}, nil, "memory") + + assert.True(t, iam.isEnabled(), "naming a config file asks for authentication, even if it yields no identity") + + handlerCalled := false + handler := iam.Auth(func(w http.ResponseWriter, r *http.Request) { + handlerCalled = true + }, s3_constants.ACTION_LIST) + + recorder := httptest.NewRecorder() + handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/", nil)) + + assert.False(t, handlerCalled, "ListBuckets must not run for an anonymous caller") + assert.Equal(t, http.StatusForbidden, recorder.Code) + }) + } +} + +// `weed mini` and `docker run seaweedfs` name no config file and stay open. +func TestNoConfigKeepsAnonymousAllowed(t *testing.T) { + resetMemoryStore() + clearEnvironmentVariableCredentials(t) + + iam := NewIdentityAccessManagementWithStore(&S3ApiServerOption{}, nil, "memory") + + assert.False(t, iam.isEnabled(), "auth must stay off when no config file and no identities are configured") +} + +// AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY register an admin identity of +// their own, which would decide isAuthEnabled before the config file does. +func clearEnvironmentVariableCredentials(t *testing.T) { + t.Setenv("AWS_ACCESS_KEY_ID", "") + t.Setenv("AWS_SECRET_ACCESS_KEY", "") +} + +// The proto parser drops what it does not recognise, so a typo has to be named +// at startup or the resulting lockout has no visible cause. +func TestUnknownS3ConfigKeys(t *testing.T) { + assert.Equal(t, []string{"identity"}, unknownS3ConfigKeys([]byte(`{"identity":[],"accounts":[]}`))) + assert.Empty(t, unknownS3ConfigKeys([]byte(`{"identities":[],"service_accounts":[],"serviceAccounts":[],"policies":[],"groups":[]}`))) + assert.Empty(t, unknownS3ConfigKeys([]byte(`{"kms":{},"sts":{},"policy":{},"providers":[],"roles":[]}`)), "sections owned by other subsystems are not typos") + assert.Empty(t, unknownS3ConfigKeys([]byte(`not json`))) +} diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index add4e69f5..bfea12eba 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -28,6 +28,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" "github.com/seaweedfs/seaweedfs/weed/util/wildcard" "github.com/seaweedfs/seaweedfs/weed/wdclient" + "google.golang.org/protobuf/reflect/protoreflect" // Import KMS providers to register them _ "github.com/seaweedfs/seaweedfs/weed/kms/aws" @@ -400,24 +401,23 @@ func NewIdentityAccessManagementWithStore(option *S3ApiServerOption, filerClient // For "weed mini" without any S3 config, default to allowing all access (isAuthEnabled = false) // If any credentials are configured (via file, filer, or env vars), enable authentication iam.m.Lock() - iam.isAuthEnabled = len(iam.identities) > 0 + identityCount := len(iam.identities) + // Pointing the gateway at a config file is the operator asking for + // authentication. A file that yields no identity - an empty secret mount, a + // key the proto parser does not recognise - must deny everyone rather than serve + // the cluster to anonymous callers. + iam.isAuthEnabled = identityCount > 0 || startConfigFile != "" iam.m.Unlock() - if iam.isAuthEnabled { - hasAnyIdentity.Store(true) - } - if iam.isAuthEnabled { - // Credentials were configured - enable authentication - glog.V(1).Infof("S3 authentication enabled (%d identities configured)", len(iam.identities)) - } else { - // No credentials configured - if startConfigFile != "" { - // Config file was specified but contained no identities - this is unusual, log a warning - glog.Warningf("S3 config file %s specified but no identities loaded - authentication disabled", startConfigFile) - } else { - // No config file and no identities - this is the normal allow-all case - glog.V(1).Infof("S3 authentication disabled - no credentials configured (allowing all access)") - } + switch { + case identityCount > 0: + hasAnyIdentity.Store(true) + glog.V(1).Infof("S3 authentication enabled (%d identities configured)", identityCount) + case startConfigFile != "": + glog.Warningf("S3 config file %s loaded no identities - every request is denied until one is configured", startConfigFile) + default: + // No config file and no identities - this is the normal allow-all case + glog.V(1).Infof("S3 authentication disabled - no credentials configured (allowing all access)") } return iam @@ -629,6 +629,10 @@ func (iam *IdentityAccessManagement) loadS3ApiConfigurationFromFile(fileName str return fmt.Errorf("fail to read %s : %v", fileName, readErr) } + if unknown := unknownS3ConfigKeys(content); len(unknown) > 0 { + glog.Warningf("S3 config %s: ignoring unknown top-level keys %v", fileName, unknown) + } + // Initialize KMS if configuration contains KMS settings if err := iam.initializeKMSFromConfig(content); err != nil { glog.Warningf("KMS initialization failed: %v", err) @@ -650,6 +654,31 @@ func (iam *IdentityAccessManagement) loadS3ApiConfigurationFromFile(fileName str return nil } +// nonIdentityS3ConfigKeys are the top-level sections of a config file that +// belong to another subsystem rather than to S3ApiConfiguration: the KMS block, +// and the advanced IAM blocks of a -s3.iam.config file. +var nonIdentityS3ConfigKeys = []string{"kms", "sts", "policy", "providers", "roles"} + +// unknownS3ConfigKeys returns the top-level keys the proto parser discards. A +// singular "identity" otherwise loads as an empty config, which denies every +// request with nothing pointing at the mistake. +func unknownS3ConfigKeys(content []byte) []string { + var root map[string]json.RawMessage + if err := json.Unmarshal(content, &root); err != nil { + return nil + } + fields := (&iam_pb.S3ApiConfiguration{}).ProtoReflect().Descriptor().Fields() + var unknown []string + for key := range root { + if slices.Contains(nonIdentityS3ConfigKeys, key) || fields.ByName(protoreflect.Name(key)) != nil || fields.ByJSONName(key) != nil { + continue + } + unknown = append(unknown, key) + } + slices.Sort(unknown) + return unknown +} + func (iam *IdentityAccessManagement) LoadS3ApiConfigurationFromBytes(content []byte) error { _, err := iam.loadS3ApiConfigurationFromBytes(content, false) return err @@ -1297,6 +1326,7 @@ func (iam *IdentityAccessManagement) UpsertIdentity(ident *iam_pb.Identity) erro // // Driven solely by isAuthEnabled, which is set when: // - any locally managed identities/credentials are loaded (file/filer/env), or +// - the operator names a config file, whether or not it yields an identity, or // - the operator passes -s3.iam.config, which triggers EnableAuthEnforcement // at startup time even before any identities sync in. //