Skip to content

Commit 7368e06

Browse files
committed
fix: check credential expiration timestamp when generating tokens
What this PR does / why we need it: There are two expirations which must be considered when using a signed EKS token: * 15 minutes after the point in time when the AWS STS request has been signed * The underlying AWS credentials can expire at which point the token won't be accepted The second case is particularly common when making frequent requests while using AssumeRole or AssumeRoleWithWebRequest as mentioned in #590 as the default session timeout is 1 hour. This PR adds an additional check fetching the AWS credential expiration and using that as the returned expiration if it is before the 15 minute token expiration.
1 parent b154c1d commit 7368e06

File tree

2 files changed

+61
-14
lines changed

2 files changed

+61
-14
lines changed

pkg/token/token.go

+8
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,11 @@ func (g generator) GetWithSTS(clusterID string, stsAPI stsiface.STSAPI) (Token,
316316
// override the Sign handler so we can control the now time for testing.
317317
request.Handlers.Sign.Swap("v4.SignRequestHandler", getNamedSigningHandler(g.nowFunc))
318318

319+
// Fetch the timestamp when the credentials we're going to use for signing will not be valid anymore
320+
// This operation is potentially racey, but the worst case is that we expire a token early
321+
// Not all credential providers support this, so we ignore any returned errors
322+
credentialsExpiration, _ := request.Config.Credentials.ExpiresAt()
323+
319324
// Sign the request. The expires parameter (sets the x-amz-expires header) is
320325
// currently ignored by STS, and the token expires 15 minutes after the x-amz-date
321326
// timestamp regardless. We set it to 60 seconds for backwards compatibility (the
@@ -329,6 +334,9 @@ func (g generator) GetWithSTS(clusterID string, stsAPI stsiface.STSAPI) (Token,
329334

330335
// Set token expiration to 1 minute before the presigned URL expires for some cushion
331336
tokenExpiration := g.nowFunc().Local().Add(presignedURLExpiration - 1*time.Minute)
337+
if !credentialsExpiration.IsZero() && credentialsExpiration.Before(tokenExpiration) {
338+
tokenExpiration = credentialsExpiration.Add(-1 * time.Minute)
339+
}
332340
// TODO: this may need to be a constant-time base64 encoding
333341
return Token{v1Prefix + base64.RawURLEncoding.EncodeToString([]byte(presignedURLString)), tokenExpiration}, nil
334342
}

pkg/token/token_test.go

+53-14
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,10 @@ func Test_getDefaultHostNameForRegion(t *testing.T) {
590590
func TestGetWithSTS(t *testing.T) {
591591
clusterID := "test-cluster"
592592

593+
// Example non-real credentials
594+
decodedAkid, _ := base64.StdEncoding.DecodeString("QVNJQVIyVEc0NFY2QVMzWlpFN0M=")
595+
decodedSk, _ := base64.StdEncoding.DecodeString("NEtENWNudEdjVm1MV1JkRjV3dk5SdXpOTDVReG1wNk9LVlk2RnovUQ==")
596+
593597
cases := []struct {
594598
name string
595599
creds *credentials.Credentials
@@ -598,23 +602,39 @@ func TestGetWithSTS(t *testing.T) {
598602
wantErr error
599603
}{
600604
{
601-
"Non-zero time",
602-
// Example non-real credentials
603-
func() *credentials.Credentials {
604-
decodedAkid, _ := base64.StdEncoding.DecodeString("QVNJQVIyVEc0NFY2QVMzWlpFN0M=")
605-
decodedSk, _ := base64.StdEncoding.DecodeString("NEtENWNudEdjVm1MV1JkRjV3dk5SdXpOTDVReG1wNk9LVlk2RnovUQ==")
606-
return credentials.NewStaticCredentials(
607-
string(decodedAkid),
608-
string(decodedSk),
609-
"",
610-
)
611-
}(),
612-
time.Unix(1682640000, 0),
613-
Token{
605+
name: "Non-zero time",
606+
creds: credentials.NewStaticCredentials(
607+
string(decodedAkid),
608+
string(decodedSk),
609+
"",
610+
),
611+
nowTime: time.Unix(1682640000, 0),
612+
want: Token{
614613
Token: "k8s-aws-v1.aHR0cHM6Ly9zdHMudXMtd2VzdC0yLmFtYXpvbmF3cy5jb20vP0FjdGlvbj1HZXRDYWxsZXJJZGVudGl0eSZWZXJzaW9uPTIwMTEtMDYtMTUmWC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BU0lBUjJURzQ0VjZBUzNaWkU3QyUyRjIwMjMwNDI4JTJGdXMtd2VzdC0yJTJGc3RzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyMzA0MjhUMDAwMDAwWiZYLUFtei1FeHBpcmVzPTYwJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCUzQngtazhzLWF3cy1pZCZYLUFtei1TaWduYXR1cmU9ZTIxMWRiYTc3YWJhOWRjNDRiMGI2YmUzOGI4ZWFhZDA5MjU5OWM1MTU3ZjYzMTQ0NDRjNWI5ZDg1NzQ3ZjVjZQ",
615614
Expiration: time.Unix(1682640000, 0).Local().Add(time.Minute * 14),
616615
},
617-
nil,
616+
wantErr: nil,
617+
},
618+
{
619+
name: "Signing creds expire before token",
620+
creds: func() *credentials.Credentials {
621+
622+
c := credentials.NewCredentials(&fakeCredentialProvider{
623+
value: credentials.Value{
624+
AccessKeyID: string(decodedAkid),
625+
SecretAccessKey: string(decodedSk),
626+
},
627+
expiresAt: time.Unix(1682640000, 0).Local().Add(time.Minute * 10),
628+
})
629+
_, _ = c.Get()
630+
return c
631+
}(),
632+
nowTime: time.Unix(1682640000, 0),
633+
want: Token{
634+
Token: "k8s-aws-v1.aHR0cHM6Ly9zdHMudXMtd2VzdC0yLmFtYXpvbmF3cy5jb20vP0FjdGlvbj1HZXRDYWxsZXJJZGVudGl0eSZWZXJzaW9uPTIwMTEtMDYtMTUmWC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BU0lBUjJURzQ0VjZBUzNaWkU3QyUyRjIwMjMwNDI4JTJGdXMtd2VzdC0yJTJGc3RzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyMzA0MjhUMDAwMDAwWiZYLUFtei1FeHBpcmVzPTYwJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCUzQngtazhzLWF3cy1pZCZYLUFtei1TaWduYXR1cmU9ZTIxMWRiYTc3YWJhOWRjNDRiMGI2YmUzOGI4ZWFhZDA5MjU5OWM1MTU3ZjYzMTQ0NDRjNWI5ZDg1NzQ3ZjVjZQ",
635+
Expiration: time.Unix(1682640000, 0).Local().Add(time.Minute * 9),
636+
},
637+
wantErr: nil,
618638
},
619639
}
620640

@@ -646,3 +666,22 @@ func TestGetWithSTS(t *testing.T) {
646666
})
647667
}
648668
}
669+
670+
type fakeCredentialProvider struct {
671+
value credentials.Value
672+
expiresAt time.Time
673+
}
674+
675+
func (f *fakeCredentialProvider) Retrieve() (credentials.Value, error) {
676+
return f.value, nil
677+
}
678+
679+
func (f *fakeCredentialProvider) IsExpired() bool {
680+
return false
681+
}
682+
683+
var _ credentials.Expirer = (*fakeCredentialProvider)(nil)
684+
685+
func (f *fakeCredentialProvider) ExpiresAt() time.Time {
686+
return f.expiresAt
687+
}

0 commit comments

Comments
 (0)