mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-09-13 10:00:41 +02:00
fix(s3api/sts): respect MaxSessionLength config in DurationSeconds validation (#11267)
* Refactor parseDurationSeconds into a STSHandlers method Convert the parseDurationSeconds wrapper from a package-level function into a method on STSHandlers so it can reach the configured STS service. No behavior change; the three AssumeRole* handlers now invoke it via their receiver. * Respect MaxSessionLength config in STS DurationSeconds validation parseDurationSeconds validated DurationSeconds against a hardcoded 43200s (12h) ceiling, so raising maxSessionLength in iam.json above 12h had no effect on AssumeRole, AssumeRoleWithWebIdentity, or AssumeRoleWithLDAPIdentity — requests were rejected at the handler before reaching the service layer. Derive the upper bound from the configured STS MaxSessionLength, falling back to maxDurationSeconds (43200s) when unset. The service layer (calculateSessionDuration) already caps the issued duration at MaxSessionLength, so this only relaxes the input-validation gate. * Add tests for STS DurationSeconds MaxSessionLength bound Cover the configured MaxSessionLength upper bound, rejection above it, fallback to the 43200s default when STS config is unset, the 900s minimum, and the empty-parameter nil path. * Refactor validateSessionDurationSeconds into a STSService method Convert validateSessionDurationSeconds from a package-level function into a method on STSService so it can reach the configured STS config. No behavior change; the three assume-role entry points in the service (AssumeRoleForPrincipal, validateAssumeRoleWithWebIdentityRequest, validateAssumeRoleWithCredentialsRequest) now invoke it via their receiver. * Respect MaxSessionLength config in STS service DurationSeconds validation The STS service validateSessionDurationSeconds rejected DurationSeconds above a hardcoded 43200s (12h) ceiling, so even after the handler accepted a longer duration it was rejected again in the service layer for AssumeRoleForPrincipal, AssumeRoleWithWebIdentity, and AssumeRoleWithCredentials. Derive the upper bound from the configured MaxSessionLength, falling back to DefaultMaxSessionLength (43200s) when unset. The issued duration is still capped at MaxSessionLength by calculateSessionDuration. * Add tests for STS service DurationSeconds MaxSessionLength bound Cover the configured MaxSessionLength upper bound, rejection above it, fallback to the 43200s default when STS config is unset, the 900s minimum, and the nil DurationSeconds path. * Preserve capping when MaxSessionLength is below the API minimum Deriving the DurationSeconds upper bound directly from MaxSessionLength created an empty valid range when MaxSessionLength is configured below the 900s API minimum, rejecting every explicit DurationSeconds that the old code silently capped via calculateSessionDuration. Only apply the configured MaxSessionLength as the upper bound when it is at least minDurationSeconds; otherwise keep the default bound and let calculateSessionDuration enforce the shorter configured limit. * Add tests for sub-minimum MaxSessionLength capping behavior Verify that a MaxSessionLength below the 900s API minimum keeps the default upper bound so explicit DurationSeconds within the default range are still accepted (and later capped by calculateSessionDuration).
This commit is contained in:
@@ -807,12 +807,19 @@ func (s *STSService) issueSession(roleArn, roleSessionName, sessionPolicy string
|
||||
// validateSessionDurationSeconds bounds a requested session lifetime the way
|
||||
// AWS STS does. Every assume-role entry point runs it, so a duration that came
|
||||
// from configuration is checked the same as one from a request.
|
||||
func validateSessionDurationSeconds(durationSeconds *int64) error {
|
||||
func (s *STSService) validateSessionDurationSeconds(durationSeconds *int64) error {
|
||||
if durationSeconds == nil {
|
||||
return nil
|
||||
}
|
||||
if *durationSeconds < 900 || *durationSeconds > 43200 { // 15min to 12 hours
|
||||
return fmt.Errorf("DurationSeconds must be between 900 and 43200 seconds")
|
||||
maxSec := int64(DefaultMaxSessionLength)
|
||||
if s.Config != nil && s.Config.MaxSessionLength.Duration > 0 {
|
||||
configuredMax := int64(s.Config.MaxSessionLength.Duration / time.Second)
|
||||
if configuredMax >= 900 {
|
||||
maxSec = configuredMax
|
||||
}
|
||||
}
|
||||
if *durationSeconds < 900 || *durationSeconds > maxSec {
|
||||
return fmt.Errorf("DurationSeconds must be between 900 and %d seconds", maxSec)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@@ -857,7 +864,7 @@ func (s *STSService) AssumeRoleForPrincipal(ctx context.Context, request *Assume
|
||||
if request.Principal == "" {
|
||||
return nil, fmt.Errorf("principal cannot be empty")
|
||||
}
|
||||
if err := validateSessionDurationSeconds(request.DurationSeconds); err != nil {
|
||||
if err := s.validateSessionDurationSeconds(request.DurationSeconds); err != nil {
|
||||
return nil, fmt.Errorf("invalid request: %w", err)
|
||||
}
|
||||
|
||||
@@ -935,7 +942,7 @@ func (s *STSService) validateAssumeRoleWithWebIdentityRequest(request *AssumeRol
|
||||
return fmt.Errorf("RoleSessionName is required")
|
||||
}
|
||||
|
||||
return validateSessionDurationSeconds(request.DurationSeconds)
|
||||
return s.validateSessionDurationSeconds(request.DurationSeconds)
|
||||
}
|
||||
|
||||
// validateWebIdentityToken validates the web identity token with strict issuer-to-provider mapping
|
||||
@@ -1152,7 +1159,7 @@ func (s *STSService) validateAssumeRoleWithCredentialsRequest(request *AssumeRol
|
||||
return fmt.Errorf("ProviderName is required")
|
||||
}
|
||||
|
||||
return validateSessionDurationSeconds(request.DurationSeconds)
|
||||
return s.validateSessionDurationSeconds(request.DurationSeconds)
|
||||
}
|
||||
|
||||
// ExpireSessionForTesting manually expires a session for testing purposes
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
package sts
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func newSTSServiceWithMaxSession(t *testing.T, maxSession time.Duration) *STSService {
|
||||
t.Helper()
|
||||
s := NewSTSService()
|
||||
assert.NoError(t, s.Initialize(&STSConfig{
|
||||
Issuer: "test-issuer",
|
||||
SigningKey: []byte("test-signing-key-at-least-32-bytes-long"),
|
||||
TokenDuration: FlexibleDuration{Duration: time.Hour},
|
||||
MaxSessionLength: FlexibleDuration{Duration: maxSession},
|
||||
}))
|
||||
return s
|
||||
}
|
||||
|
||||
func secondsPtr(v int64) *int64 { return &v }
|
||||
|
||||
func TestValidateSessionDurationSeconds_RespectsConfiguredMaxSessionLength(t *testing.T) {
|
||||
s := newSTSServiceWithMaxSession(t, 168*time.Hour)
|
||||
assert.NoError(t, s.validateSessionDurationSeconds(secondsPtr(604800)))
|
||||
}
|
||||
|
||||
func TestValidateSessionDurationSeconds_RejectsAboveConfiguredMaxSessionLength(t *testing.T) {
|
||||
s := newSTSServiceWithMaxSession(t, 24*time.Hour)
|
||||
err := s.validateSessionDurationSeconds(secondsPtr(90000))
|
||||
assert.Error(t, err)
|
||||
}
|
||||
|
||||
func TestValidateSessionDurationSeconds_FallsBackToDefaultWhenUnset(t *testing.T) {
|
||||
s := &STSService{}
|
||||
assert.NoError(t, s.validateSessionDurationSeconds(secondsPtr(43200)))
|
||||
err := s.validateSessionDurationSeconds(secondsPtr(43201))
|
||||
assert.Error(t, err)
|
||||
}
|
||||
|
||||
func TestValidateSessionDurationSeconds_EnforcesMinimum(t *testing.T) {
|
||||
s := newSTSServiceWithMaxSession(t, 168*time.Hour)
|
||||
assert.Error(t, s.validateSessionDurationSeconds(secondsPtr(899)))
|
||||
}
|
||||
|
||||
func TestValidateSessionDurationSeconds_NilReturnsNil(t *testing.T) {
|
||||
s := newSTSServiceWithMaxSession(t, 168*time.Hour)
|
||||
assert.NoError(t, s.validateSessionDurationSeconds(nil))
|
||||
}
|
||||
|
||||
func TestValidateSessionDurationSeconds_SubMinimumMaxSessionLengthKeepsCapping(t *testing.T) {
|
||||
s := newSTSServiceWithMaxSession(t, 5*time.Minute)
|
||||
assert.NoError(t, s.validateSessionDurationSeconds(secondsPtr(900)))
|
||||
assert.NoError(t, s.validateSessionDurationSeconds(secondsPtr(43200)))
|
||||
assert.Error(t, s.validateSessionDurationSeconds(secondsPtr(43201)))
|
||||
}
|
||||
+13
-6
@@ -131,9 +131,16 @@ func parseDurationSecondsWithBounds(r *http.Request, minSec, maxSec int64) (*int
|
||||
return &ds, "", nil
|
||||
}
|
||||
|
||||
// parseDurationSeconds parses DurationSeconds for AssumeRole (15 min to 12 hours)
|
||||
func parseDurationSeconds(r *http.Request) (*int64, STSErrorCode, error) {
|
||||
return parseDurationSecondsWithBounds(r, minDurationSeconds, maxDurationSeconds)
|
||||
// parseDurationSeconds parses DurationSeconds for AssumeRole (15 min to MaxSessionLength)
|
||||
func (h *STSHandlers) parseDurationSeconds(r *http.Request) (*int64, STSErrorCode, error) {
|
||||
maxSec := maxDurationSeconds
|
||||
if h.stsService != nil && h.stsService.Config != nil && h.stsService.Config.MaxSessionLength.Duration > 0 {
|
||||
configuredMax := int64(h.stsService.Config.MaxSessionLength.Duration / time.Second)
|
||||
if configuredMax >= minDurationSeconds {
|
||||
maxSec = configuredMax
|
||||
}
|
||||
}
|
||||
return parseDurationSecondsWithBounds(r, minDurationSeconds, maxSec)
|
||||
}
|
||||
|
||||
// Removed generateSecureCredentials - now using STS service's JWT token generation
|
||||
@@ -253,7 +260,7 @@ func (h *STSHandlers) handleAssumeRoleWithWebIdentity(w http.ResponseWriter, r *
|
||||
}
|
||||
|
||||
// Parse and validate DurationSeconds using helper
|
||||
durationSeconds, errCode, err := parseDurationSeconds(r)
|
||||
durationSeconds, errCode, err := h.parseDurationSeconds(r)
|
||||
if err != nil {
|
||||
h.writeSTSErrorResponse(w, r, errCode, err)
|
||||
return
|
||||
@@ -350,7 +357,7 @@ func (h *STSHandlers) handleAssumeRole(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
|
||||
// Parse and validate DurationSeconds using helper
|
||||
durationSeconds, errCode, err := parseDurationSeconds(r)
|
||||
durationSeconds, errCode, err := h.parseDurationSeconds(r)
|
||||
if err != nil {
|
||||
h.writeSTSErrorResponse(w, r, errCode, err)
|
||||
return
|
||||
@@ -506,7 +513,7 @@ func (h *STSHandlers) handleAssumeRoleWithLDAPIdentity(w http.ResponseWriter, r
|
||||
}
|
||||
|
||||
// Parse and validate DurationSeconds using helper
|
||||
durationSeconds, errCode, err := parseDurationSeconds(r)
|
||||
durationSeconds, errCode, err := h.parseDurationSeconds(r)
|
||||
if err != nil {
|
||||
h.writeSTSErrorResponse(w, r, errCode, err)
|
||||
return
|
||||
|
||||
@@ -0,0 +1,105 @@
|
||||
package s3api
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/weed/iam/sts"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func newDurationSecondsRequest(t *testing.T, seconds string) *http.Request {
|
||||
t.Helper()
|
||||
form := url.Values{}
|
||||
if seconds != "" {
|
||||
form.Set("DurationSeconds", seconds)
|
||||
}
|
||||
req, err := http.NewRequest("POST", "/", strings.NewReader(form.Encode()))
|
||||
require.NoError(t, err)
|
||||
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
require.NoError(t, req.ParseForm())
|
||||
return req
|
||||
}
|
||||
|
||||
func newSTSHandlersWithMaxSession(t *testing.T, maxSession time.Duration) *STSHandlers {
|
||||
t.Helper()
|
||||
s := sts.NewSTSService()
|
||||
require.NoError(t, s.Initialize(&sts.STSConfig{
|
||||
Issuer: "test-issuer",
|
||||
SigningKey: []byte("test-signing-key-at-least-32-bytes-long-for-security"),
|
||||
TokenDuration: sts.FlexibleDuration{Duration: time.Hour},
|
||||
MaxSessionLength: sts.FlexibleDuration{Duration: maxSession},
|
||||
}))
|
||||
return NewSTSHandlers(s, nil)
|
||||
}
|
||||
|
||||
func TestParseDurationSeconds_RespectsConfiguredMaxSessionLength(t *testing.T) {
|
||||
h := newSTSHandlersWithMaxSession(t, 168*time.Hour)
|
||||
|
||||
ds, errCode, err := h.parseDurationSeconds(newDurationSecondsRequest(t, "604800"))
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, STSErrorCode(""), errCode)
|
||||
if assert.NotNil(t, ds) {
|
||||
assert.Equal(t, int64(604800), *ds)
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseDurationSeconds_RejectsAboveConfiguredMaxSessionLength(t *testing.T) {
|
||||
h := newSTSHandlersWithMaxSession(t, 24*time.Hour)
|
||||
|
||||
ds, errCode, err := h.parseDurationSeconds(newDurationSecondsRequest(t, "90000"))
|
||||
assert.Error(t, err)
|
||||
assert.Equal(t, STSErrInvalidParameterValue, errCode)
|
||||
assert.Nil(t, ds)
|
||||
}
|
||||
|
||||
func TestParseDurationSeconds_FallsBackToDefaultWhenUnset(t *testing.T) {
|
||||
h := &STSHandlers{stsService: nil}
|
||||
|
||||
ds, errCode, err := h.parseDurationSeconds(newDurationSecondsRequest(t, "43200"))
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, STSErrorCode(""), errCode)
|
||||
if assert.NotNil(t, ds) {
|
||||
assert.Equal(t, int64(43200), *ds)
|
||||
}
|
||||
|
||||
_, errCode, err = h.parseDurationSeconds(newDurationSecondsRequest(t, "43201"))
|
||||
assert.Error(t, err)
|
||||
assert.Equal(t, STSErrInvalidParameterValue, errCode)
|
||||
}
|
||||
|
||||
func TestParseDurationSeconds_EnforcesMinimum(t *testing.T) {
|
||||
h := newSTSHandlersWithMaxSession(t, 168*time.Hour)
|
||||
|
||||
_, errCode, err := h.parseDurationSeconds(newDurationSecondsRequest(t, "899"))
|
||||
assert.Error(t, err)
|
||||
assert.Equal(t, STSErrInvalidParameterValue, errCode)
|
||||
}
|
||||
|
||||
func TestParseDurationSeconds_EmptyReturnsNil(t *testing.T) {
|
||||
h := newSTSHandlersWithMaxSession(t, 168*time.Hour)
|
||||
|
||||
ds, errCode, err := h.parseDurationSeconds(newDurationSecondsRequest(t, ""))
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, STSErrorCode(""), errCode)
|
||||
assert.Nil(t, ds)
|
||||
}
|
||||
|
||||
func TestParseDurationSeconds_SubMinimumMaxSessionLengthKeepsCapping(t *testing.T) {
|
||||
h := newSTSHandlersWithMaxSession(t, 5*time.Minute)
|
||||
|
||||
ds, errCode, err := h.parseDurationSeconds(newDurationSecondsRequest(t, "900"))
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, STSErrorCode(""), errCode)
|
||||
if assert.NotNil(t, ds) {
|
||||
assert.Equal(t, int64(900), *ds)
|
||||
}
|
||||
|
||||
_, errCode, err = h.parseDurationSeconds(newDurationSecondsRequest(t, "43201"))
|
||||
assert.Error(t, err)
|
||||
assert.Equal(t, STSErrInvalidParameterValue, errCode)
|
||||
}
|
||||
Reference in New Issue
Block a user