mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-09-29 11:15:34 +00:00
s3: honor configured session bounds on AssumeRole and LDAP identity (#11478)
* sts: export CalculateSessionDuration Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: honor configured session bounds on AssumeRole and LDAP identity prepareSTSCredentials hardcoded a one-hour session when the caller omitted DurationSeconds, so sts.tokenDuration was ignored and sts.maxSessionLength only clamped explicit requests: asking for 3600s against a 20m ceiling was rejected while omitting the parameter was granted a full hour (#11473). The two affected handlers now use the same default-then-cap calculation as AssumeRoleWithWebIdentity. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * iam: keep MaxSessionDuration through role store copies copyRoleDefinition rebuilt RoleDefinition field by field and dropped MaxSessionDuration, so memory-backed role stores silently discarded the per-role session bound on every write and read (devin on #11478). Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * sts: apply per-role MaxSessionDuration to resolved session durations Review follow-up on #11478 (devin): the role bound only ever applied to explicit DurationSeconds values — an omitted duration resolved to the configured default and sailed past a shorter role max on every assume path. - capDurationByRole now resolves min(requested||tokenDuration, roleMax), so AssumeRoleWithWebIdentity and AssumeRoleWithCredentials cap defaults the same way they cap explicit values. - prepareSTSCredentials caps the calculated duration at the named role's MaxSessionDuration, covering the AssumeRole and LDAP handlers; self-assumption has no role definition to consult. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * iam: keep MaxSessionDuration through the cached role store genericCopyRoleDefinition drops MaxSessionDuration the same way copyRoleDefinition did, so the cached filer role store reads back a zero maximum and every downstream duration cap is skipped (greptile on #11478). Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * sts: only materialize defaults that pass session duration validation Review follow-up on #11478 (greptile): materializing an omitted DurationSeconds into an explicit value could exceed the service's own input bound (a configured tokenDuration above maxSessionLength) and turn a previously working request into a validation error. capDurationByRole now leaves nil anything the service can resolve better itself, clamps a tightened default at maxSessionLengthSeconds, and floors a role bound below 900s to the tightest issuable value. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
co-authored by
Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
parent
ab95d58b7c
commit
2864bc0fe8
@@ -123,9 +123,10 @@ func genericCopyRoleDefinition(role *RoleDefinition) *RoleDefinition {
|
||||
}
|
||||
|
||||
result := &RoleDefinition{
|
||||
RoleName: role.RoleName,
|
||||
RoleArn: role.RoleArn,
|
||||
Description: role.Description,
|
||||
RoleName: role.RoleName,
|
||||
RoleArn: role.RoleArn,
|
||||
Description: role.Description,
|
||||
MaxSessionDuration: role.MaxSessionDuration,
|
||||
}
|
||||
|
||||
// Deep copy trust policy if it exists
|
||||
|
||||
@@ -942,7 +942,7 @@ func (m *IAMManager) AssumeRoleWithWebIdentity(ctx context.Context, request *sts
|
||||
// Apply role-level MaxSessionDuration cap. The STS service still applies
|
||||
// the global MaxSessionLength and the source-token-expiry cap on top of
|
||||
// this; per-role takes precedence whenever it is the tightest bound.
|
||||
request.DurationSeconds = capDurationByRole(request.DurationSeconds, roleDef.MaxSessionDuration)
|
||||
request.DurationSeconds = capDurationByRole(request.DurationSeconds, roleDef.MaxSessionDuration, m.defaultTokenDurationSeconds(), m.maxSessionLengthSeconds())
|
||||
|
||||
// Use STS service to assume the role
|
||||
return m.stsService.AssumeRoleWithWebIdentity(ctx, request)
|
||||
@@ -1005,22 +1005,54 @@ func extractIssuerFromJWT(token string) (string, error) {
|
||||
return iss, nil
|
||||
}
|
||||
|
||||
// capDurationByRole returns the requested duration clamped to the role's
|
||||
// MaxSessionDuration. A nil requested duration is left nil so the STS
|
||||
// service's calculateSessionDuration applies the global default (typically
|
||||
// 1 hour) — substituting the role's max here would silently mint a 12h
|
||||
// session for any caller who omitted DurationSeconds, which AWS does not
|
||||
// do. The role-max upper bound still applies in the downstream cap chain
|
||||
// once the request has a concrete duration.
|
||||
func capDurationByRole(requested *int64, roleMax int64) *int64 {
|
||||
if roleMax <= 0 || requested == nil {
|
||||
return requested
|
||||
// capDurationByRole returns the session duration clamped to the role's
|
||||
// MaxSessionDuration. An omitted DurationSeconds resolves to the configured
|
||||
// default first, so the role bound caps defaults and explicit values alike.
|
||||
// A nil request is only materialized when something tightened the default
|
||||
// and the explicit value still passes the service's own input validation —
|
||||
// everything else is left nil so the service resolves the default and its
|
||||
// MaxSessionLength cap itself.
|
||||
func capDurationByRole(requested *int64, roleMax, defaultSec, serviceMaxSec int64) *int64 {
|
||||
if requested != nil {
|
||||
d := *requested
|
||||
if roleMax > 0 && d > roleMax {
|
||||
d = roleMax
|
||||
}
|
||||
return &d
|
||||
}
|
||||
if *requested > roleMax {
|
||||
v := roleMax
|
||||
return &v
|
||||
d := defaultSec
|
||||
if roleMax > 0 && d > roleMax {
|
||||
d = roleMax
|
||||
}
|
||||
return requested
|
||||
if d > serviceMaxSec {
|
||||
d = serviceMaxSec
|
||||
}
|
||||
if d < 900 && roleMax > 0 {
|
||||
d = 900
|
||||
}
|
||||
if d >= defaultSec || d < 900 {
|
||||
return nil
|
||||
}
|
||||
return &d
|
||||
}
|
||||
|
||||
func (m *IAMManager) defaultTokenDurationSeconds() int64 {
|
||||
if m.stsService == nil || m.stsService.Config == nil {
|
||||
return sts.DefaultTokenDuration
|
||||
}
|
||||
return int64(m.stsService.Config.TokenDuration.Duration / time.Second)
|
||||
}
|
||||
|
||||
// maxSessionLengthSeconds mirrors validateSessionDurationSeconds so a
|
||||
// materialized default stays inside the bound the service will enforce.
|
||||
func (m *IAMManager) maxSessionLengthSeconds() int64 {
|
||||
maxSec := int64(sts.DefaultMaxSessionLength)
|
||||
if m.stsService != nil && m.stsService.Config != nil && m.stsService.Config.MaxSessionLength.Duration > 0 {
|
||||
if configured := int64(m.stsService.Config.MaxSessionLength.Duration / time.Second); configured >= 900 {
|
||||
maxSec = configured
|
||||
}
|
||||
}
|
||||
return maxSec
|
||||
}
|
||||
|
||||
// AssumeRoleWithCredentials assumes a role using credentials (LDAP)
|
||||
@@ -1044,7 +1076,7 @@ func (m *IAMManager) AssumeRoleWithCredentials(ctx context.Context, request *sts
|
||||
}
|
||||
|
||||
// Apply role-level MaxSessionDuration cap.
|
||||
request.DurationSeconds = capDurationByRole(request.DurationSeconds, roleDef.MaxSessionDuration)
|
||||
request.DurationSeconds = capDurationByRole(request.DurationSeconds, roleDef.MaxSessionDuration, m.defaultTokenDurationSeconds(), m.maxSessionLengthSeconds())
|
||||
|
||||
// Use STS service to assume the role
|
||||
return m.stsService.AssumeRoleWithCredentials(ctx, request)
|
||||
|
||||
@@ -6,21 +6,27 @@ func intPtr(v int64) *int64 { return &v }
|
||||
|
||||
func TestCapDurationByRole(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
requested *int64
|
||||
roleMax int64
|
||||
want *int64
|
||||
name string
|
||||
requested *int64
|
||||
roleMax int64
|
||||
defaultSec int64
|
||||
serviceMaxSec int64
|
||||
want *int64
|
||||
}{
|
||||
{"no cap, no request", nil, 0, nil},
|
||||
{"no cap, with request", intPtr(7200), 0, intPtr(7200)},
|
||||
{"cap only, no request -> nil so STS default applies", nil, 3600, nil},
|
||||
{"request below cap -> request", intPtr(1800), 3600, intPtr(1800)},
|
||||
{"request equal cap -> request", intPtr(3600), 3600, intPtr(3600)},
|
||||
{"request above cap -> cap", intPtr(43200), 3600, intPtr(3600)},
|
||||
{"no cap, no request -> nil keeps service default", nil, 0, 3600, 43200, nil},
|
||||
{"no cap, with request", intPtr(7200), 0, 3600, 43200, intPtr(7200)},
|
||||
{"cap below default, no request -> cap", nil, 1800, 3600, 43200, intPtr(1800)},
|
||||
{"cap above default, no request -> nil", nil, 43200, 3600, 43200, nil},
|
||||
{"request below cap -> request", intPtr(1800), 3600, 900, 43200, intPtr(1800)},
|
||||
{"request equal cap -> request", intPtr(3600), 3600, 900, 43200, intPtr(3600)},
|
||||
{"request above cap -> cap", intPtr(43200), 3600, 900, 43200, intPtr(3600)},
|
||||
{"default above service max -> service cap materialized", nil, 0, 7200, 3600, intPtr(3600)},
|
||||
{"role bound above service max -> service cap still applies", nil, 40000, 43200, 3600, intPtr(3600)},
|
||||
{"role bound below service floor -> tightest issuable", nil, 500, 3600, 43200, intPtr(900)},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got := capDurationByRole(tc.requested, tc.roleMax)
|
||||
got := capDurationByRole(tc.requested, tc.roleMax, tc.defaultSec, tc.serviceMaxSec)
|
||||
switch {
|
||||
case got == nil && tc.want == nil:
|
||||
return
|
||||
|
||||
@@ -112,9 +112,10 @@ func copyRoleDefinition(original *RoleDefinition) *RoleDefinition {
|
||||
}
|
||||
|
||||
copied := &RoleDefinition{
|
||||
RoleName: original.RoleName,
|
||||
RoleArn: original.RoleArn,
|
||||
Description: original.Description,
|
||||
RoleName: original.RoleName,
|
||||
RoleArn: original.RoleArn,
|
||||
Description: original.Description,
|
||||
MaxSessionDuration: original.MaxSessionDuration,
|
||||
}
|
||||
|
||||
// Deep copy trust policy if it exists
|
||||
|
||||
@@ -604,7 +604,7 @@ func (s *STSService) AssumeRoleWithWebIdentity(ctx context.Context, request *Ass
|
||||
}
|
||||
|
||||
// 4. Calculate session duration
|
||||
sessionDuration := s.calculateSessionDuration(request.DurationSeconds)
|
||||
sessionDuration := s.CalculateSessionDuration(request.DurationSeconds)
|
||||
expiresAt := time.Now().Add(sessionDuration)
|
||||
|
||||
// 5. Generate session ID and credentials
|
||||
@@ -769,7 +769,7 @@ func (s *STSService) AssumeRoleWithCredentials(ctx context.Context, request *Ass
|
||||
func (s *STSService) issueSession(roleArn, roleSessionName, sessionPolicy string,
|
||||
durationSeconds *int64, providerName, subject string) (*AssumeRoleResponse, error) {
|
||||
|
||||
sessionDuration := s.calculateSessionDuration(durationSeconds)
|
||||
sessionDuration := s.CalculateSessionDuration(durationSeconds)
|
||||
expiresAt := time.Now().Add(sessionDuration)
|
||||
|
||||
sessionId, err := GenerateSessionId()
|
||||
@@ -1120,11 +1120,11 @@ func (s *STSService) validateRoleAssumptionForCredentials(ctx context.Context, r
|
||||
return nil
|
||||
}
|
||||
|
||||
// calculateSessionDuration returns the requested DurationSeconds, or the
|
||||
// CalculateSessionDuration returns the requested DurationSeconds, or the
|
||||
// configured TokenDuration default, capped at MaxSessionLength. The source
|
||||
// token's exp deliberately plays no part: per AWS semantics the session
|
||||
// outlives the (already verified) web identity token.
|
||||
func (s *STSService) calculateSessionDuration(durationSeconds *int64) time.Duration {
|
||||
func (s *STSService) CalculateSessionDuration(durationSeconds *int64) time.Duration {
|
||||
var duration time.Duration
|
||||
if durationSeconds != nil {
|
||||
duration = time.Duration(*durationSeconds) * time.Second
|
||||
|
||||
@@ -30,8 +30,8 @@ func TestCalculateSessionDuration(t *testing.T) {
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := svc.calculateSessionDuration(tc.durationSeconds); got != tc.want {
|
||||
t.Errorf("calculateSessionDuration(%v) = %v, want %v", tc.durationSeconds, got, tc.want)
|
||||
if got := svc.CalculateSessionDuration(tc.durationSeconds); got != tc.want {
|
||||
t.Errorf("CalculateSessionDuration(%v) = %v, want %v", tc.durationSeconds, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
+21
-3
@@ -834,12 +834,30 @@ func (h *STSHandlers) handleGetFederationToken(w http.ResponseWriter, r *http.Re
|
||||
func (h *STSHandlers) prepareSTSCredentials(ctx context.Context, roleArn, roleSessionName string,
|
||||
durationSeconds *int64, sessionPolicy string, modifyClaims func(*sts.STSSessionClaims)) (STSCredentials, *AssumedRoleUser, error) {
|
||||
|
||||
// Calculate duration
|
||||
duration := time.Hour // Default 1 hour
|
||||
if durationSeconds != nil {
|
||||
duration := time.Hour
|
||||
if h.stsService != nil && h.stsService.Config != nil {
|
||||
duration = h.stsService.CalculateSessionDuration(durationSeconds)
|
||||
} else if durationSeconds != nil {
|
||||
duration = time.Duration(*durationSeconds) * time.Second
|
||||
}
|
||||
|
||||
// A named role's MaxSessionDuration bounds the resolved duration the same
|
||||
// way capDurationByRole does on the SDK paths; self-assumption has no role
|
||||
// definition to consult.
|
||||
if h.iam != nil && h.iam.iamIntegration != nil {
|
||||
if roleName := utils.ExtractRoleNameFromArn(roleArn); roleName != "" {
|
||||
if provider, ok := h.iam.iamIntegration.(IAMManagerProvider); ok {
|
||||
if mgr := provider.GetIAMManager(); mgr != nil {
|
||||
if roleDef, roleErr := mgr.GetRole(ctx, roleName); roleErr == nil && roleDef.MaxSessionDuration > 0 {
|
||||
if roleMax := time.Duration(roleDef.MaxSessionDuration) * time.Second; duration > roleMax {
|
||||
duration = roleMax
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Generate session ID
|
||||
sessionId, err := sts.GenerateSessionId()
|
||||
if err != nil {
|
||||
|
||||
@@ -241,3 +241,64 @@ func newTestSTSIntegrationManager(t *testing.T) *integration.IAMManager {
|
||||
require.NoError(t, manager.Initialize(config, func() string { return "" }))
|
||||
return manager
|
||||
}
|
||||
|
||||
// AssumeRole derives its session length from the same default-then-cap rule as
|
||||
// the service layer (#11473): an omitted DurationSeconds yields TokenDuration,
|
||||
// and either path is capped at MaxSessionLength.
|
||||
func TestPrepareSTSCredentialsHonorsConfiguredDurations(t *testing.T) {
|
||||
stsService := sts.NewSTSService()
|
||||
require.NoError(t, stsService.Initialize(&sts.STSConfig{
|
||||
TokenDuration: sts.FlexibleDuration{Duration: 15 * time.Minute},
|
||||
MaxSessionLength: sts.FlexibleDuration{Duration: 20 * time.Minute},
|
||||
Issuer: "test-issuer",
|
||||
SigningKey: []byte("test-signing-key-at-least-32-bytes-long-for-security"),
|
||||
}))
|
||||
stsHandlers := NewSTSHandlers(stsService, nil)
|
||||
roleArn := fmt.Sprintf("arn:aws:iam::%s:role/test-role", defaultAccountID)
|
||||
|
||||
expiresIn := func(durationSeconds *int64) time.Duration {
|
||||
stsCreds, _, err := stsHandlers.prepareSTSCredentials(context.Background(), roleArn, "test-session", durationSeconds, "", nil)
|
||||
require.NoError(t, err)
|
||||
exp, err := time.Parse(time.RFC3339, stsCreds.Expiration)
|
||||
require.NoError(t, err)
|
||||
return time.Until(exp)
|
||||
}
|
||||
|
||||
oneHour := int64(3600)
|
||||
assert.InDelta(t, (15 * time.Minute).Seconds(), expiresIn(nil).Seconds(), 60)
|
||||
assert.InDelta(t, (20 * time.Minute).Seconds(), expiresIn(&oneHour).Seconds(), 60)
|
||||
}
|
||||
|
||||
// A named role's MaxSessionDuration bounds the session however DurationSeconds
|
||||
// was resolved, matching the SDK paths' capDurationByRole.
|
||||
func TestPrepareSTSCredentialsCapsAtRoleMaxDuration(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
manager := newTestSTSIntegrationManager(t)
|
||||
require.NoError(t, manager.CreateRole(ctx, "", "ShortLivedRole", &integration.RoleDefinition{
|
||||
RoleName: "ShortLivedRole",
|
||||
MaxSessionDuration: 3600,
|
||||
}))
|
||||
|
||||
stsService := sts.NewSTSService()
|
||||
require.NoError(t, stsService.Initialize(&sts.STSConfig{
|
||||
TokenDuration: sts.FlexibleDuration{Duration: 2 * time.Hour},
|
||||
MaxSessionLength: sts.FlexibleDuration{Duration: 12 * time.Hour},
|
||||
Issuer: "test-issuer",
|
||||
SigningKey: []byte("test-signing-key-at-least-32-bytes-long-for-security"),
|
||||
}))
|
||||
iam := &IdentityAccessManagement{iamIntegration: NewS3IAMIntegration(manager, "")}
|
||||
stsHandlers := NewSTSHandlers(stsService, iam)
|
||||
roleArn := fmt.Sprintf("arn:aws:iam::%s:role/ShortLivedRole", defaultAccountID)
|
||||
|
||||
expiresIn := func(durationSeconds *int64) time.Duration {
|
||||
stsCreds, _, err := stsHandlers.prepareSTSCredentials(ctx, roleArn, "test-session", durationSeconds, "", nil)
|
||||
require.NoError(t, err)
|
||||
exp, err := time.Parse(time.RFC3339, stsCreds.Expiration)
|
||||
require.NoError(t, err)
|
||||
return time.Until(exp)
|
||||
}
|
||||
|
||||
twoHours := int64(7200)
|
||||
assert.InDelta(t, float64(3600), expiresIn(nil).Seconds(), 60, "omitted duration resolves to the 2h default but the role caps it at 1h")
|
||||
assert.InDelta(t, float64(3600), expiresIn(&twoHours).Seconds(), 60, "explicit duration above the role max is capped")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user