Skip to content

Commit

Permalink
[v15] Set default access request TTL to 1 week. (#39509)
Browse files Browse the repository at this point in the history 
* Set default access request TTL to 1 week.

The TTL for a request now defaults to 1 week. This will allow reviewers more
time to review an access request before it disappears.

Co-authored-by: Lisa Kim <[email protected]>

* Fix setting pending request TTL pass the access expiry time (#39548)

* Prevent setting pending request TTL pass the access expiry time

* Allow setting max duration less than sessionTTL when role max_duration is not set

* Add more test, clarify comment

* Remove unused params

* Add a test for respecting both pending and max duration request

* Add a fakeclock to fix access request time diff check differences

---------

Co-authored-by: Lisa Kim <[email protected]>
  • Loading branch information
@mdwn @kimlisa
mdwn and kimlisa authored Mar 28, 2024
1 parent 9ce6fb0 commit 0b1b579
Show file tree
Hide file tree
Showing 5 changed files with 262 additions and 144 deletions.
106 changes: 66 additions & 40 deletions lib/services/access_request.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,20 +37,25 @@ import (
apidefaults "github.com/gravitational/teleport/api/defaults"
"github.com/gravitational/teleport/api/types"
apiutils "github.com/gravitational/teleport/api/utils"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/tlsca"
"github.com/gravitational/teleport/lib/utils"
"github.com/gravitational/teleport/lib/utils/parse"
)

const maxAccessRequestReasonSize = 4096
const (
maxAccessRequestReasonSize = 4096

// A day is sometimes 23 hours, sometimes 25 hours, usually 24 hours.
const day = 24 * time.Hour
// A day is sometimes 23 hours, sometimes 25 hours, usually 24 hours.
day = 24 * time.Hour

// maxAccessDuration is the maximum duration that an access request can be
// granted for.
const MaxAccessDuration = 14 * day
// MaxAccessDuration is the maximum duration that an access request can be
// granted for.
MaxAccessDuration = 14 * day

// requestTTL is the the TTL for an access request, i.e. the amount of time that
// the access request can be reviewed. Defaults to 1 week.
requestTTL = 7 * day
)

// ValidateAccessRequest validates the AccessRequest and sets default values
func ValidateAccessRequest(ar types.AccessRequest) error {
Expand Down Expand Up @@ -1182,45 +1187,50 @@ func (m *RequestValidator) Validate(ctx context.Context, req types.AccessRequest
req.SetSuggestedReviewers(apiutils.Deduplicate(m.SuggestedReviewers))
}

now := m.clock.Now().UTC()

// Calculate the expiration time of the Access Request (how long it
// will await approval).
ttl, err := m.requestTTL(ctx, identity, req)
if err != nil {
return trace.Wrap(err)
}
req.SetExpiry(now.Add(ttl))

maxDuration, err := m.calculateMaxAccessDuration(req)
// Calculate the expiration time of the elevated certificate that will
// be issued if the Access Request is approved.
sessionTTL, err := m.sessionTTL(ctx, identity, req)
if err != nil {
return trace.Wrap(err)
}

// Calculate the expiration time of the elevated certificate that will
// be issued if the Access Request is approved.
sessionTTL, err := m.sessionTTL(ctx, identity, req)
maxDuration, err := m.calculateMaxAccessDuration(req, sessionTTL)
if err != nil {
return trace.Wrap(err)
}

// If the maxDuration flag is set, consider it instead of only using the session TTL.
var maxAccessDuration time.Duration
now := m.clock.Now().UTC()
if maxDuration > 0 {
req.SetSessionTLL(now.Add(min(sessionTTL, maxDuration)))
ttl = maxDuration
maxAccessDuration = maxDuration
} else {
req.SetSessionTLL(now.Add(sessionTTL))
ttl = sessionTTL
maxAccessDuration = sessionTTL
}

accessTTL := now.Add(ttl)
req.SetAccessExpiry(accessTTL)
// This is the final adjusted access expiry where both max duration
// and session TTL were taken into consideration.
accessExpiry := now.Add(maxAccessDuration)
// Adjusted max access duration is equal to the access expiry time.
req.SetMaxDuration(accessTTL)
req.SetMaxDuration(accessExpiry)

// Setting access expiry before calling `calculatePendingRequesetTTL`
// matters since the func relies on this adjusted expiry.
req.SetAccessExpiry(accessExpiry)

// Calculate the expiration time of the Access Request (how long it
// will await approval).
requestTTL, err := m.calculatePendingRequestTTL(req)
if err != nil {
return trace.Wrap(err)
}
req.SetExpiry(now.Add(requestTTL))

if req.GetAssumeStartTime() != nil {
assumeStartTime := *req.GetAssumeStartTime()
if err := types.ValidateAssumeStartTime(assumeStartTime, accessTTL, req.GetCreationTime()); err != nil {
if err := types.ValidateAssumeStartTime(assumeStartTime, accessExpiry, req.GetCreationTime()); err != nil {
return trace.Wrap(err)
}
}
Expand All @@ -1232,7 +1242,7 @@ func (m *RequestValidator) Validate(ctx context.Context, req types.AccessRequest
// calculateMaxAccessDuration calculates the maximum time for the access request.
// The max duration time is the minimum of the max_duration time set on the request
// and the max_duration time set on the request role.
func (m *RequestValidator) calculateMaxAccessDuration(req types.AccessRequest) (time.Duration, error) {
func (m *RequestValidator) calculateMaxAccessDuration(req types.AccessRequest, sessionTTL time.Duration) (time.Duration, error) {
// Check if the maxDuration time is set.
maxDurationTime := req.GetMaxDuration()
if maxDurationTime.IsZero() {
Expand Down Expand Up @@ -1272,36 +1282,52 @@ func (m *RequestValidator) calculateMaxAccessDuration(req types.AccessRequest) (
}
}

// minAdjDuration can end up being 0, if any role does not have
// field `max_duration` defined.
// In this case, return the smaller value between the sessionTTL
// and the requested max duration.
if minAdjDuration == 0 && maxDuration < sessionTTL {
return maxDuration, nil
}

return minAdjDuration, nil
}

// requestTTL calculates the TTL of the Access Request (how long it will await
// approval).
func (m *RequestValidator) requestTTL(ctx context.Context, identity tlsca.Identity, r types.AccessRequest) (time.Duration, error) {
// calculatePendingRequestTTL calculates the TTL of the Access Request (how long it will await
// approval). request TTL is capped to the smaller value between the const requsetTTL and the
// access request access expiry.
func (m *RequestValidator) calculatePendingRequestTTL(r types.AccessRequest) (time.Duration, error) {
accessExpiryTTL := r.GetAccessExpiry().Sub(m.clock.Now().UTC())

// If no expiration provided, use default.
expiry := r.Expiry()
if expiry.IsZero() {
expiry = m.clock.Now().UTC().Add(defaults.PendingAccessDuration)
// Guard against the default expiry being greater than access expiry.
if requestTTL < accessExpiryTTL {
expiry = m.clock.Now().UTC().Add(requestTTL)
} else {
expiry = m.clock.Now().UTC().Add(accessExpiryTTL)
}
}

if expiry.Before(m.clock.Now().UTC()) {
return 0, trace.BadParameter("invalid request TTL: Access Request can not be created in the past")
}

ttl, err := m.truncateTTL(ctx, identity, expiry, r.GetRoles())
if err != nil {
return 0, trace.BadParameter("invalid request TTL: %v", err)
}

// Before returning the TTL, validate that the value requested was smaller
// than the maximum value allowed. Used to return a sensible error to the
// user.
requestedTTL := expiry.Sub(m.clock.Now().UTC())
if !r.Expiry().IsZero() && requestedTTL > ttl {
return 0, trace.BadParameter("invalid request TTL: %v greater than maximum allowed (%v)", requestedTTL.Round(time.Minute), ttl.Round(time.Minute))
if !r.Expiry().IsZero() {
if requestedTTL > requestTTL {
return 0, trace.BadParameter("invalid request TTL: %v greater than maximum allowed (%v)", requestedTTL.Round(time.Minute), requestTTL.Round(time.Minute))
}
if requestedTTL > accessExpiryTTL {
return 0, trace.BadParameter("invalid request TTL: %v greater than maximum allowed (%v)", requestedTTL.Round(time.Minute), accessExpiryTTL.Round(time.Minute))
}
}

return ttl, nil
return requestedTTL, nil
}

// sessionTTL calculates the TTL of the elevated certificate that will be issued
Expand Down
Loading

0 comments on commit 0b1b579

Please sign in to comment.