s3: deny anonymous access when the identity config loads no identities (#10954)

* s3: deny anonymous requests when the identity config loads no identities

Naming a config file is the operator asking for authentication. A file that
yields no identity - an unpopulated secret mount, or a mistyped top-level key
the proto parser silently drops - left the gateway open to every anonymous
caller: ListBuckets returned 200, and anonymous PUT could create buckets and
write objects.

* s3: name the unknown top-level keys in an identity config

The proto parser discards what it does not recognise, so a mistyped
"identites" loads as an empty config. Naming the dropped keys at startup turns
the resulting lockout into a one-line diagnosis.

* s3: isolate the auth-enforcement tests from AWS environment credentials

* s3: use a singular "identity" as the unrecognised-key example

Codespell rejects the misspelling the example used.

* s3: cover the empty identity config alongside the unrecognised key

* s3: cover a config file whose body is an empty object

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`)))
+}

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.
 //