Compare commits

...

2 Commits

Author SHA1 Message Date
maxts0gt d74ae22ac3 chore: clearer description 2025-07-27 12:52:35 +09:00
max-ts0gt af3d0e0bfb feat: add opt-in CORS security for wildcard origins
- Preserve backward compatibility: wildcard origins allow credentials by default
- Add MINIO_API_CORS_ALLOW_CREDENTIALS_WITH_WILDCARD=off for enhanced security
- Addresses CORS specification compliance while preventing service disruption
- Existing deployments continue working unchanged
2025-07-27 12:41:44 +09:00
5 changed files with 144 additions and 92 deletions

View File

@ -683,6 +683,7 @@ func corsHandler(handler http.Handler) http.Handler {
// Configure CORS dynamically based on current settings // Configure CORS dynamically based on current settings
// This ensures we handle configuration changes and wildcard security properly // This ensures we handle configuration changes and wildcard security properly
hasWildcard := configHasWildcard() hasWildcard := configHasWildcard()
allowCredentialsWithWildcard := globalAPIConfig.getCorsAllowCredentialsWithWildcard()
opts := cors.Options{ opts := cors.Options{
AllowOriginFunc: func(origin string) bool { AllowOriginFunc: func(origin string) bool {
@ -702,11 +703,12 @@ func corsHandler(handler http.Handler) http.Handler {
http.MethodOptions, http.MethodOptions,
http.MethodPatch, http.MethodPatch,
}, },
AllowedHeaders: commonS3Headers, AllowedHeaders: commonS3Headers,
ExposedHeaders: commonS3Headers, ExposedHeaders: commonS3Headers,
// CORS spec compliance: disable credentials when wildcard origins are configured // CORS spec compliance: disable credentials when wildcard origins are configured
// Unless explicitly overridden by administrator (for backward compatibility)
// This prevents the security vulnerability where any website can make credentialed requests // This prevents the security vulnerability where any website can make credentialed requests
AllowCredentials: !hasWildcard, AllowCredentials: !hasWildcard || allowCredentialsWithWildcard,
} }
// Use rs/cors directly without custom wrapper to avoid interface issues // Use rs/cors directly without custom wrapper to avoid interface issues

View File

@ -27,59 +27,83 @@ import (
func TestCORSCredentialsWithWildcard(t *testing.T) { func TestCORSCredentialsWithWildcard(t *testing.T) {
// Save original config and restore after test // Save original config and restore after test
originalOrigins := globalAPIConfig.getCorsAllowOrigins() originalOrigins := globalAPIConfig.getCorsAllowOrigins()
originalAllowCredentialsWithWildcard := globalAPIConfig.getCorsAllowCredentialsWithWildcard()
defer func() { defer func() {
globalAPIConfig.mu.Lock() globalAPIConfig.mu.Lock()
globalAPIConfig.corsAllowOrigins = originalOrigins globalAPIConfig.corsAllowOrigins = originalOrigins
globalAPIConfig.corsAllowCredentialsWithWildcard = originalAllowCredentialsWithWildcard
globalAPIConfig.mu.Unlock() globalAPIConfig.mu.Unlock()
}() }()
// Setup wildcard CORS config // Test 1: Default secure behavior (wildcard without credentials)
globalAPIConfig.mu.Lock() t.Run("Secure Default", func(t *testing.T) {
globalAPIConfig.corsAllowOrigins = []string{"*"} // Setup wildcard CORS config with default secure behavior
globalAPIConfig.mu.Unlock() globalAPIConfig.mu.Lock()
globalAPIConfig.corsAllowOrigins = []string{"*"}
globalAPIConfig.corsAllowCredentialsWithWildcard = false // default secure behavior
globalAPIConfig.mu.Unlock()
// Create a simple handler // Create a simple handler
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK) w.WriteHeader(http.StatusOK)
})
// Wrap with CORS handler
corsWrappedHandler := corsHandler(handler)
// Test preflight request
req := httptest.NewRequest("OPTIONS", "/", nil)
req.Header.Set("Origin", "https://example.com")
req.Header.Set("Access-Control-Request-Method", "GET")
rr := httptest.NewRecorder()
corsWrappedHandler.ServeHTTP(rr, req)
// Verify specific origin is echoed back (rs/cors library behavior)
if got := rr.Header().Get("Access-Control-Allow-Origin"); got != "https://example.com" {
t.Errorf("Expected Access-Control-Allow-Origin: https://example.com, got: %s", got)
}
// Verify credentials header is NOT present (security fix)
if got := rr.Header().Get("Access-Control-Allow-Credentials"); got != "" {
t.Errorf("Expected no Access-Control-Allow-Credentials header with wildcard origin, got: %s", got)
}
}) })
// Wrap with CORS handler // Test 2: Backward compatibility opt-out (wildcard with credentials - insecure)
corsWrappedHandler := corsHandler(handler) t.Run("Backward Compatibility Opt-out", func(t *testing.T) {
// Setup wildcard CORS config with explicit opt-out for backward compatibility
globalAPIConfig.mu.Lock()
globalAPIConfig.corsAllowOrigins = []string{"*"}
globalAPIConfig.corsAllowCredentialsWithWildcard = true // explicitly allow insecure behavior
globalAPIConfig.mu.Unlock()
// Test preflight request // Create a simple handler
req := httptest.NewRequest("OPTIONS", "/", nil) handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
req.Header.Set("Origin", "https://example.com") w.WriteHeader(http.StatusOK)
req.Header.Set("Access-Control-Request-Method", "GET") })
rr := httptest.NewRecorder() // Wrap with CORS handler
corsWrappedHandler.ServeHTTP(rr, req) corsWrappedHandler := corsHandler(handler)
// Verify specific origin is echoed back (rs/cors library behavior) // Test preflight request
if got := rr.Header().Get("Access-Control-Allow-Origin"); got != "https://example.com" { req := httptest.NewRequest("OPTIONS", "/", nil)
t.Errorf("Expected Access-Control-Allow-Origin: https://example.com, got: %s", got) req.Header.Set("Origin", "https://example.com")
} req.Header.Set("Access-Control-Request-Method", "GET")
// Verify credentials header is NOT present (security fix) rr := httptest.NewRecorder()
if got := rr.Header().Get("Access-Control-Allow-Credentials"); got != "" { corsWrappedHandler.ServeHTTP(rr, req)
t.Errorf("Expected no Access-Control-Allow-Credentials header with wildcard origin, got: %s", got)
}
// Test actual GET request // Verify specific origin is echoed back (rs/cors library behavior)
req = httptest.NewRequest("GET", "/", nil) if got := rr.Header().Get("Access-Control-Allow-Origin"); got != "https://example.com" {
req.Header.Set("Origin", "https://example.com") t.Errorf("Expected Access-Control-Allow-Origin: https://example.com, got: %s", got)
}
rr = httptest.NewRecorder() // Verify credentials header IS present (backward compatibility)
corsWrappedHandler.ServeHTTP(rr, req) if got := rr.Header().Get("Access-Control-Allow-Credentials"); got != "true" {
t.Errorf("Expected Access-Control-Allow-Credentials: true with opt-out enabled, got: %s", got)
// Verify specific origin is echoed back (rs/cors library behavior) }
if got := rr.Header().Get("Access-Control-Allow-Origin"); got != "https://example.com" { })
t.Errorf("Expected Access-Control-Allow-Origin: https://example.com, got: %s", got)
}
// Verify credentials header is NOT present
if got := rr.Header().Get("Access-Control-Allow-Credentials"); got != "" {
t.Errorf("Expected no Access-Control-Allow-Credentials header with wildcard origin, got: %s", got)
}
} }
// Test that CORS credentials are allowed for specific origins // Test that CORS credentials are allowed for specific origins
@ -168,7 +192,7 @@ func TestCORSUnauthorizedOrigin(t *testing.T) {
// Test preflight request from unauthorized origin // Test preflight request from unauthorized origin
req := httptest.NewRequest("OPTIONS", "/", nil) req := httptest.NewRequest("OPTIONS", "/", nil)
req.Header.Set("Origin", "https://example.org") // This origin is NOT in the allowed list req.Header.Set("Origin", "https://example.org") // This origin is NOT in the allowed list
req.Header.Set("Access-Control-Request-Method", "GET") req.Header.Set("Access-Control-Request-Method", "GET")
rr := httptest.NewRecorder() rr := httptest.NewRecorder()
@ -188,15 +212,18 @@ func TestCORSUnauthorizedOrigin(t *testing.T) {
func TestCORSMixedConfiguration(t *testing.T) { func TestCORSMixedConfiguration(t *testing.T) {
// Save original config and restore after test // Save original config and restore after test
originalOrigins := globalAPIConfig.getCorsAllowOrigins() originalOrigins := globalAPIConfig.getCorsAllowOrigins()
originalAllowCredentialsWithWildcard := globalAPIConfig.getCorsAllowCredentialsWithWildcard()
defer func() { defer func() {
globalAPIConfig.mu.Lock() globalAPIConfig.mu.Lock()
globalAPIConfig.corsAllowOrigins = originalOrigins globalAPIConfig.corsAllowOrigins = originalOrigins
globalAPIConfig.corsAllowCredentialsWithWildcard = originalAllowCredentialsWithWildcard
globalAPIConfig.mu.Unlock() globalAPIConfig.mu.Unlock()
}() }()
// Setup mixed CORS config (wildcard + specific origins) // Setup mixed CORS config (wildcard + specific origins) with default secure behavior
globalAPIConfig.mu.Lock() globalAPIConfig.mu.Lock()
globalAPIConfig.corsAllowOrigins = []string{"*", "https://example.com"} globalAPIConfig.corsAllowOrigins = []string{"*", "https://example.com"}
globalAPIConfig.corsAllowCredentialsWithWildcard = false // default secure behavior
globalAPIConfig.mu.Unlock() globalAPIConfig.mu.Unlock()
// Create a simple handler // Create a simple handler
@ -220,7 +247,7 @@ func TestCORSMixedConfiguration(t *testing.T) {
t.Errorf("Expected Access-Control-Allow-Origin: https://example.com, got: %s", got) t.Errorf("Expected Access-Control-Allow-Origin: https://example.com, got: %s", got)
} }
// Verify credentials header is NOT present due to wildcard in config // Verify credentials header is NOT present due to wildcard in config (secure default)
if got := rr.Header().Get("Access-Control-Allow-Credentials"); got != "" { if got := rr.Header().Get("Access-Control-Allow-Credentials"); got != "" {
t.Errorf("Expected no Access-Control-Allow-Credentials header with wildcard in config, got: %s", got) t.Errorf("Expected no Access-Control-Allow-Credentials header with wildcard in config, got: %s", got)
} }

View File

@ -49,14 +49,15 @@ type apiConfig struct {
replicationMaxLWorkers int replicationMaxLWorkers int
transitionWorkers int transitionWorkers int
staleUploadsExpiry time.Duration staleUploadsExpiry time.Duration
staleUploadsCleanupInterval time.Duration staleUploadsCleanupInterval time.Duration
deleteCleanupInterval time.Duration deleteCleanupInterval time.Duration
enableODirect bool enableODirect bool
gzipObjects bool gzipObjects bool
rootAccess bool rootAccess bool
syncEvents bool syncEvents bool
objectMaxVersions int64 objectMaxVersions int64
corsAllowCredentialsWithWildcard bool
} }
const ( const (
@ -187,6 +188,7 @@ func (t *apiConfig) init(cfg api.Config, setDriveCounts []int, legacy bool) {
t.rootAccess = cfg.RootAccess t.rootAccess = cfg.RootAccess
t.syncEvents = cfg.SyncEvents t.syncEvents = cfg.SyncEvents
t.objectMaxVersions = cfg.ObjectMaxVersions t.objectMaxVersions = cfg.ObjectMaxVersions
t.corsAllowCredentialsWithWildcard = cfg.CorsAllowCredentialsWithWildcard
if t.staleUploadsCleanupInterval != cfg.StaleUploadsCleanupInterval { if t.staleUploadsCleanupInterval != cfg.StaleUploadsCleanupInterval {
t.staleUploadsCleanupInterval = cfg.StaleUploadsCleanupInterval t.staleUploadsCleanupInterval = cfg.StaleUploadsCleanupInterval
@ -244,6 +246,12 @@ func (t *apiConfig) getCorsAllowOrigins() []string {
return corsAllowOrigins return corsAllowOrigins
} }
func (t *apiConfig) getCorsAllowCredentialsWithWildcard() bool {
t.mu.RLock()
defer t.mu.RUnlock()
return t.corsAllowCredentialsWithWildcard
}
func (t *apiConfig) getStaleUploadsCleanupInterval() time.Duration { func (t *apiConfig) getStaleUploadsCleanupInterval() time.Duration {
t.mu.RLock() t.mu.RLock()
defer t.mu.RUnlock() defer t.mu.RUnlock()

View File

@ -32,14 +32,15 @@ import (
// API sub-system constants // API sub-system constants
const ( const (
apiRequestsMax = "requests_max" apiRequestsMax = "requests_max"
apiClusterDeadline = "cluster_deadline" apiClusterDeadline = "cluster_deadline"
apiCorsAllowOrigin = "cors_allow_origin" apiCorsAllowOrigin = "cors_allow_origin"
apiRemoteTransportDeadline = "remote_transport_deadline" apiCorsAllowCredentialsWithWildcard = "cors_allow_credentials_with_wildcard"
apiListQuorum = "list_quorum" apiRemoteTransportDeadline = "remote_transport_deadline"
apiReplicationPriority = "replication_priority" apiListQuorum = "list_quorum"
apiReplicationMaxWorkers = "replication_max_workers" apiReplicationPriority = "replication_priority"
apiReplicationMaxLWorkers = "replication_max_lrg_workers" apiReplicationMaxWorkers = "replication_max_workers"
apiReplicationMaxLWorkers = "replication_max_lrg_workers"
apiTransitionWorkers = "transition_workers" apiTransitionWorkers = "transition_workers"
apiStaleUploadsCleanupInterval = "stale_uploads_cleanup_interval" apiStaleUploadsCleanupInterval = "stale_uploads_cleanup_interval"
@ -52,17 +53,18 @@ const (
apiSyncEvents = "sync_events" apiSyncEvents = "sync_events"
apiObjectMaxVersions = "object_max_versions" apiObjectMaxVersions = "object_max_versions"
EnvAPIRequestsMax = "MINIO_API_REQUESTS_MAX" EnvAPIRequestsMax = "MINIO_API_REQUESTS_MAX"
EnvAPIRequestsDeadline = "MINIO_API_REQUESTS_DEADLINE" EnvAPIRequestsDeadline = "MINIO_API_REQUESTS_DEADLINE"
EnvAPIClusterDeadline = "MINIO_API_CLUSTER_DEADLINE" EnvAPIClusterDeadline = "MINIO_API_CLUSTER_DEADLINE"
EnvAPICorsAllowOrigin = "MINIO_API_CORS_ALLOW_ORIGIN" EnvAPICorsAllowOrigin = "MINIO_API_CORS_ALLOW_ORIGIN"
EnvAPIRemoteTransportDeadline = "MINIO_API_REMOTE_TRANSPORT_DEADLINE" EnvAPICorsAllowCredentialsWithWildcard = "MINIO_API_CORS_ALLOW_CREDENTIALS_WITH_WILDCARD"
EnvAPITransitionWorkers = "MINIO_API_TRANSITION_WORKERS" EnvAPIRemoteTransportDeadline = "MINIO_API_REMOTE_TRANSPORT_DEADLINE"
EnvAPIListQuorum = "MINIO_API_LIST_QUORUM" EnvAPITransitionWorkers = "MINIO_API_TRANSITION_WORKERS"
EnvAPISecureCiphers = "MINIO_API_SECURE_CIPHERS" // default config.EnableOn EnvAPIListQuorum = "MINIO_API_LIST_QUORUM"
EnvAPIReplicationPriority = "MINIO_API_REPLICATION_PRIORITY" EnvAPISecureCiphers = "MINIO_API_SECURE_CIPHERS" // default config.EnableOn
EnvAPIReplicationMaxWorkers = "MINIO_API_REPLICATION_MAX_WORKERS" EnvAPIReplicationPriority = "MINIO_API_REPLICATION_PRIORITY"
EnvAPIReplicationMaxLWorkers = "MINIO_API_REPLICATION_MAX_LRG_WORKERS" EnvAPIReplicationMaxWorkers = "MINIO_API_REPLICATION_MAX_WORKERS"
EnvAPIReplicationMaxLWorkers = "MINIO_API_REPLICATION_MAX_LRG_WORKERS"
EnvAPIStaleUploadsCleanupInterval = "MINIO_API_STALE_UPLOADS_CLEANUP_INTERVAL" EnvAPIStaleUploadsCleanupInterval = "MINIO_API_STALE_UPLOADS_CLEANUP_INTERVAL"
EnvAPIStaleUploadsExpiry = "MINIO_API_STALE_UPLOADS_EXPIRY" EnvAPIStaleUploadsExpiry = "MINIO_API_STALE_UPLOADS_EXPIRY"
@ -100,6 +102,10 @@ var (
Key: apiCorsAllowOrigin, Key: apiCorsAllowOrigin,
Value: "*", Value: "*",
}, },
config.KV{
Key: apiCorsAllowCredentialsWithWildcard,
Value: config.EnableOn,
},
config.KV{ config.KV{
Key: apiRemoteTransportDeadline, Key: apiRemoteTransportDeadline,
Value: "2h", Value: "2h",
@ -166,23 +172,24 @@ var (
// Config storage class configuration // Config storage class configuration
type Config struct { type Config struct {
RequestsMax int `json:"requests_max"` RequestsMax int `json:"requests_max"`
ClusterDeadline time.Duration `json:"cluster_deadline"` ClusterDeadline time.Duration `json:"cluster_deadline"`
CorsAllowOrigin []string `json:"cors_allow_origin"` CorsAllowOrigin []string `json:"cors_allow_origin"`
RemoteTransportDeadline time.Duration `json:"remote_transport_deadline"` CorsAllowCredentialsWithWildcard bool `json:"cors_allow_credentials_with_wildcard"`
ListQuorum string `json:"list_quorum"` RemoteTransportDeadline time.Duration `json:"remote_transport_deadline"`
ReplicationPriority string `json:"replication_priority"` ListQuorum string `json:"list_quorum"`
ReplicationMaxWorkers int `json:"replication_max_workers"` ReplicationPriority string `json:"replication_priority"`
ReplicationMaxLWorkers int `json:"replication_max_lrg_workers"` ReplicationMaxWorkers int `json:"replication_max_workers"`
TransitionWorkers int `json:"transition_workers"` ReplicationMaxLWorkers int `json:"replication_max_lrg_workers"`
StaleUploadsCleanupInterval time.Duration `json:"stale_uploads_cleanup_interval"` TransitionWorkers int `json:"transition_workers"`
StaleUploadsExpiry time.Duration `json:"stale_uploads_expiry"` StaleUploadsCleanupInterval time.Duration `json:"stale_uploads_cleanup_interval"`
DeleteCleanupInterval time.Duration `json:"delete_cleanup_interval"` StaleUploadsExpiry time.Duration `json:"stale_uploads_expiry"`
EnableODirect bool `json:"enable_odirect"` DeleteCleanupInterval time.Duration `json:"delete_cleanup_interval"`
GzipObjects bool `json:"gzip_objects"` EnableODirect bool `json:"enable_odirect"`
RootAccess bool `json:"root_access"` GzipObjects bool `json:"gzip_objects"`
SyncEvents bool `json:"sync_events"` RootAccess bool `json:"root_access"`
ObjectMaxVersions int64 `json:"object_max_versions"` SyncEvents bool `json:"sync_events"`
ObjectMaxVersions int64 `json:"object_max_versions"`
} }
// UnmarshalJSON - Validate SS and RRS parity when unmarshalling JSON. // UnmarshalJSON - Validate SS and RRS parity when unmarshalling JSON.
@ -211,11 +218,13 @@ func LookupConfig(kvs config.KVS) (cfg Config, err error) {
enableODirect := env.Get(EnvAPIODirect, kvs.Get(apiODirect)) == config.EnableOn enableODirect := env.Get(EnvAPIODirect, kvs.Get(apiODirect)) == config.EnableOn
gzipObjects := env.Get(EnvAPIGzipObjects, kvs.Get(apiGzipObjects)) == config.EnableOn gzipObjects := env.Get(EnvAPIGzipObjects, kvs.Get(apiGzipObjects)) == config.EnableOn
rootAccess := env.Get(EnvAPIRootAccess, kvs.Get(apiRootAccess)) == config.EnableOn rootAccess := env.Get(EnvAPIRootAccess, kvs.Get(apiRootAccess)) == config.EnableOn
corsAllowCredentialsWithWildcard := env.Get(EnvAPICorsAllowCredentialsWithWildcard, kvs.Get(apiCorsAllowCredentialsWithWildcard)) == config.EnableOn
cfg = Config{ cfg = Config{
EnableODirect: enableODirect || !disableODirect, EnableODirect: enableODirect || !disableODirect,
GzipObjects: gzipObjects, GzipObjects: gzipObjects,
RootAccess: rootAccess, RootAccess: rootAccess,
CorsAllowCredentialsWithWildcard: corsAllowCredentialsWithWildcard,
} }
var corsAllowOrigin []string var corsAllowOrigin []string

View File

@ -44,6 +44,12 @@ var (
Optional: true, Optional: true,
Type: "csv", Type: "csv",
}, },
config.HelpKV{
Key: apiCorsAllowCredentialsWithWildcard,
Description: `allow credentials with wildcard CORS origins (default: 'on' for backward compatibility, 'off' for enhanced security compliance)` + defaultHelpPostfix(apiCorsAllowCredentialsWithWildcard),
Optional: true,
Type: "on|off",
},
config.HelpKV{ config.HelpKV{
Key: apiRemoteTransportDeadline, Key: apiRemoteTransportDeadline,
Description: `set the deadline for API requests on remote transports while proxying between federated instances e.g. "2h"` + defaultHelpPostfix(apiRemoteTransportDeadline), Description: `set the deadline for API requests on remote transports while proxying between federated instances e.g. "2h"` + defaultHelpPostfix(apiRemoteTransportDeadline),