Skip to content

Commit

Permalink
Merge pull request kubernetes#667 from stlaz/no_old_tokens
Browse files Browse the repository at this point in the history 
Bug 1944631: openshift authenticator: don't allow old-style tokens
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot authored Apr 23, 2021
2 parents 4074fcf + 5eab02c commit a40ec9c
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 43 deletions.
10 changes: 6 additions & 4 deletions openshift-kube-apiserver/authentication/oauth/bootstrapauthenticator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,14 @@ func NewBootstrapAuthenticator(tokens oauthclient.OAuthAccessTokenInterface, get
func (a *bootstrapAuthenticator) AuthenticateToken(ctx context.Context, name string) (*kauthenticator.Response, bool, error) {
// hash token for new-style sha256~ prefixed token
// TODO: reject non-sha256 prefix tokens in 4.7+
if strings.HasPrefix(name, sha256Prefix) {
withoutPrefix := strings.TrimPrefix(name, sha256Prefix)
h := sha256.Sum256([]byte(withoutPrefix))
name = sha256Prefix + base64.RawURLEncoding.EncodeToString(h[0:])
if !strings.HasPrefix(name, sha256Prefix) {
return nil, false, errOldFormat
}

withoutPrefix := strings.TrimPrefix(name, sha256Prefix)
h := sha256.Sum256([]byte(withoutPrefix))
name = sha256Prefix + base64.RawURLEncoding.EncodeToString(h[0:])

token, err := a.tokens.Get(context.TODO(), name, metav1.GetOptions{})
if err != nil {
return nil, false, errLookup // mask the error so we do not leak token data in logs
Expand Down
28 changes: 23 additions & 5 deletions openshift-kube-apiserver/authentication/oauth/expirationvalidator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ package oauth

import (
"context"
"crypto/sha256"
"encoding/base64"
"fmt"
mathrand "math/rand"
"testing"
"time"

Expand All @@ -14,16 +18,18 @@ import (
)

func TestAuthenticateTokenExpired(t *testing.T) {
token1, token1Hash := generateOAuthTokenPair()
token2, token2Hash := generateOAuthTokenPair()
fakeOAuthClient := oauthfake.NewSimpleClientset(
// expired token that had a lifetime of 10 minutes
&oauthv1.OAuthAccessToken{
ObjectMeta: metav1.ObjectMeta{Name: "token1", CreationTimestamp: metav1.Time{Time: time.Now().Add(-1 * time.Hour)}},
ObjectMeta: metav1.ObjectMeta{Name: token1Hash, CreationTimestamp: metav1.Time{Time: time.Now().Add(-1 * time.Hour)}},
ExpiresIn: 600,
UserName: "foo",
},
// non-expired token that has a lifetime of 10 minutes, but has a non-nil deletion timestamp
&oauthv1.OAuthAccessToken{
ObjectMeta: metav1.ObjectMeta{Name: "token2", CreationTimestamp: metav1.Time{Time: time.Now()}, DeletionTimestamp: &metav1.Time{}},
ObjectMeta: metav1.ObjectMeta{Name: token2Hash, CreationTimestamp: metav1.Time{Time: time.Now()}, DeletionTimestamp: &metav1.Time{}},
ExpiresIn: 600,
UserName: "foo",
},
Expand All @@ -32,7 +38,7 @@ func TestAuthenticateTokenExpired(t *testing.T) {

tokenAuthenticator := NewTokenAuthenticator(fakeOAuthClient.OauthV1().OAuthAccessTokens(), fakeUserClient.UserV1().Users(), NoopGroupMapper{}, nil, NewExpirationValidator())

for _, tokenName := range []string{"token1", "token2"} {
for _, tokenName := range []string{token1, token2} {
userInfo, found, err := tokenAuthenticator.AuthenticateToken(context.TODO(), tokenName)
if found {
t.Error("Found token, but it should be missing!")
Expand All @@ -47,9 +53,10 @@ func TestAuthenticateTokenExpired(t *testing.T) {
}

func TestAuthenticateTokenValidated(t *testing.T) {
token, tokenHash := generateOAuthTokenPair()
fakeOAuthClient := oauthfake.NewSimpleClientset(
&oauthv1.OAuthAccessToken{
ObjectMeta: metav1.ObjectMeta{Name: "token", CreationTimestamp: metav1.Time{Time: time.Now()}},
ObjectMeta: metav1.ObjectMeta{Name: tokenHash, CreationTimestamp: metav1.Time{Time: time.Now()}},
ExpiresIn: 600, // 10 minutes
UserName: "foo",
UserUID: string("bar"),
Expand All @@ -59,7 +66,7 @@ func TestAuthenticateTokenValidated(t *testing.T) {

tokenAuthenticator := NewTokenAuthenticator(fakeOAuthClient.OauthV1().OAuthAccessTokens(), fakeUserClient.UserV1().Users(), NoopGroupMapper{}, nil, NewExpirationValidator(), NewUIDValidator())

userInfo, found, err := tokenAuthenticator.AuthenticateToken(context.TODO(), "token")
userInfo, found, err := tokenAuthenticator.AuthenticateToken(context.TODO(), token)
if !found {
t.Error("Did not find a token!")
}
Expand All @@ -70,3 +77,14 @@ func TestAuthenticateTokenValidated(t *testing.T) {
t.Error("Did not get a user!")
}
}

// generateOAuthTokenPair returns two tokens to use with OpenShift OAuth-based authentication.
// The first token is a private token meant to be used as a Bearer token to send
// queries to the API, the second token is a hashed token meant to be stored in
// the database.
func generateOAuthTokenPair() (privToken, pubToken string) {
const sha256Prefix = "sha256~"
randomToken := []byte(fmt.Sprintf("nottoorandom%d", mathrand.Int()))
hashed := sha256.Sum256([]byte(randomToken))
return sha256Prefix + string(randomToken), sha256Prefix + base64.RawURLEncoding.EncodeToString(hashed[:])
}
15 changes: 10 additions & 5 deletions openshift-kube-apiserver/authentication/oauth/tokenauthenticator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ import (
userclient "github.com/openshift/client-go/user/clientset/versioned/typed/user/v1"
)

var errLookup = errors.New("token lookup failed")
var (
errLookup = errors.New("token lookup failed")
errOldFormat = errors.New("old, insecure token format")
)

type tokenAuthenticator struct {
tokens oauthclient.OAuthAccessTokenInterface
Expand All @@ -42,12 +45,14 @@ const sha256Prefix = "sha256~"
func (a *tokenAuthenticator) AuthenticateToken(ctx context.Context, name string) (*kauthenticator.Response, bool, error) {
// hash token for new-style sha256~ prefixed token
// TODO: reject non-sha256 prefix tokens in 4.7+
if strings.HasPrefix(name, sha256Prefix) {
withoutPrefix := strings.TrimPrefix(name, sha256Prefix)
h := sha256.Sum256([]byte(withoutPrefix))
name = sha256Prefix + base64.RawURLEncoding.EncodeToString(h[0:])
if !strings.HasPrefix(name, sha256Prefix) {
return nil, false, errOldFormat
}

withoutPrefix := strings.TrimPrefix(name, sha256Prefix)
h := sha256.Sum256([]byte(withoutPrefix))
name = sha256Prefix + base64.RawURLEncoding.EncodeToString(h[0:])

token, err := a.tokens.Get(context.TODO(), name, metav1.GetOptions{})
if err != nil {
return nil, false, errLookup // mask the error so we do not leak token data in logs
Expand Down
64 changes: 35 additions & 29 deletions openshift-kube-apiserver/authentication/oauth/tokenauthenticator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ import (
)

func TestAuthenticateTokenInvalidUID(t *testing.T) {
token, tokenHash := generateOAuthTokenPair()
fakeOAuthClient := oauthfake.NewSimpleClientset(
&oauthv1.OAuthAccessToken{
ObjectMeta: metav1.ObjectMeta{Name: "token", CreationTimestamp: metav1.Time{Time: time.Now()}},
ObjectMeta: metav1.ObjectMeta{Name: tokenHash, CreationTimestamp: metav1.Time{Time: time.Now()}},
ExpiresIn: 600, // 10 minutes
UserName: "foo",
UserUID: string("bar1"),
Expand All @@ -35,7 +36,7 @@ func TestAuthenticateTokenInvalidUID(t *testing.T) {

tokenAuthenticator := NewTokenAuthenticator(fakeOAuthClient.OauthV1().OAuthAccessTokens(), fakeUserClient.UserV1().Users(), NoopGroupMapper{}, nil, NewUIDValidator())

userInfo, found, err := tokenAuthenticator.AuthenticateToken(context.TODO(), "token")
userInfo, found, err := tokenAuthenticator.AuthenticateToken(context.TODO(), token)
if found {
t.Error("Found token, but it should be missing!")
}
Expand Down Expand Up @@ -88,7 +89,7 @@ func TestAuthenticateTokenFormats(t *testing.T) {
{"prefixed token", "sha256~token", false, true, "tokenUser"},
{"unprefixed hash token", tokenSha256, true, false, ""},
{"prefixed hash token", "sha256~" + tokenSha256, true, false, ""},
{"unprefixed token2", "token2", false, true, "token2User"},
{"unprefixed token2", "token2", true, false, ""},
{"prefixed token2", "sha256~token2", true, false, ""},
{"unprefixed hash token2", token2Sha256, true, false, ""},
{"prefixed hash token2", "sha256~" + token2Sha256, true, false, ""},
Expand Down Expand Up @@ -122,7 +123,7 @@ func TestAuthenticateTokenNotFoundSuppressed(t *testing.T) {
fakeUserClient := userfake.NewSimpleClientset()
tokenAuthenticator := NewTokenAuthenticator(fakeOAuthClient.OauthV1().OAuthAccessTokens(), fakeUserClient.UserV1().Users(), NoopGroupMapper{}, nil)

userInfo, found, err := tokenAuthenticator.AuthenticateToken(context.TODO(), "token")
userInfo, found, err := tokenAuthenticator.AuthenticateToken(context.TODO(), "sha256~token")
if found {
t.Error("Found token, but it should be missing!")
}
Expand All @@ -142,7 +143,7 @@ func TestAuthenticateTokenOtherGetErrorSuppressed(t *testing.T) {
fakeUserClient := userfake.NewSimpleClientset()
tokenAuthenticator := NewTokenAuthenticator(fakeOAuthClient.OauthV1().OAuthAccessTokens(), fakeUserClient.UserV1().Users(), NoopGroupMapper{}, nil)

userInfo, found, err := tokenAuthenticator.AuthenticateToken(context.TODO(), "token")
userInfo, found, err := tokenAuthenticator.AuthenticateToken(context.TODO(), "sha256~token")
if found {
t.Error("Found token, but it should be missing!")
}
Expand Down Expand Up @@ -175,32 +176,37 @@ func TestAuthenticateTokenTimeout(t *testing.T) {
slowClient := oauthv1.OAuthClient{
ObjectMeta: metav1.ObjectMeta{Name: "slowClient"},
}
testTokenClear, testTokenHash := generateOAuthTokenPair()
testToken := oauthv1.OAuthAccessToken{
ObjectMeta: metav1.ObjectMeta{Name: "testToken", CreationTimestamp: metav1.Time{Time: testClock.Now()}},
ObjectMeta: metav1.ObjectMeta{Name: testTokenHash, CreationTimestamp: metav1.Time{Time: testClock.Now()}},
ClientName: "testClient",
ExpiresIn: 600, // 10 minutes
UserName: "foo",
UserUID: string("bar"),
InactivityTimeoutSeconds: clientTimeout,
}

quickTokenClear, quickTokenHash := generateOAuthTokenPair()
quickToken := oauthv1.OAuthAccessToken{
ObjectMeta: metav1.ObjectMeta{Name: "quickToken", CreationTimestamp: metav1.Time{Time: testClock.Now()}},
ObjectMeta: metav1.ObjectMeta{Name: quickTokenHash, CreationTimestamp: metav1.Time{Time: testClock.Now()}},
ClientName: "quickClient",
ExpiresIn: 600, // 10 minutes
UserName: "foo",
UserUID: string("bar"),
InactivityTimeoutSeconds: minTimeout,
}
slowTokenClear, slowTokenHash := generateOAuthTokenPair()
slowToken := oauthv1.OAuthAccessToken{
ObjectMeta: metav1.ObjectMeta{Name: "slowToken", CreationTimestamp: metav1.Time{Time: testClock.Now()}},
ObjectMeta: metav1.ObjectMeta{Name: slowTokenHash, CreationTimestamp: metav1.Time{Time: testClock.Now()}},
ClientName: "slowClient",
ExpiresIn: 600, // 10 minutes
UserName: "foo",
UserUID: string("bar"),
InactivityTimeoutSeconds: defaultTimeout,
}
emergTokenClear, emergTokenHash := generateOAuthTokenPair()
emergToken := oauthv1.OAuthAccessToken{
ObjectMeta: metav1.ObjectMeta{Name: "emergToken", CreationTimestamp: metav1.Time{Time: testClock.Now()}},
ObjectMeta: metav1.ObjectMeta{Name: emergTokenHash, CreationTimestamp: metav1.Time{Time: testClock.Now()}},
ClientName: "quickClient",
ExpiresIn: 600, // 10 minutes
UserName: "foo",
Expand Down Expand Up @@ -250,20 +256,20 @@ func TestAuthenticateTokenTimeout(t *testing.T) {
// TIME: 0 seconds have passed here

// first time should succeed for all
checkToken(t, "testToken", tokenAuthenticator, accessTokenGetter, testClock, true)
checkToken(t, testTokenClear, testTokenHash, tokenAuthenticator, accessTokenGetter, testClock, true)
wait(t, putTokenSync)

checkToken(t, "quickToken", tokenAuthenticator, accessTokenGetter, testClock, true)
checkToken(t, quickTokenClear, quickTokenHash, tokenAuthenticator, accessTokenGetter, testClock, true)
wait(t, putTokenSync)

wait(t, timeoutsSync) // from emergency flush because quickToken has a short enough timeout

checkToken(t, "slowToken", tokenAuthenticator, accessTokenGetter, testClock, true)
checkToken(t, slowTokenClear, slowTokenHash, tokenAuthenticator, accessTokenGetter, testClock, true)
wait(t, putTokenSync)

// this should cause an emergency flush, if not the next auth will fail,
// as the token will be timed out
checkToken(t, "emergToken", tokenAuthenticator, accessTokenGetter, testClock, true)
checkToken(t, emergTokenClear, emergTokenHash, tokenAuthenticator, accessTokenGetter, testClock, true)
wait(t, putTokenSync)

wait(t, timeoutsSync) // from emergency flush because emergToken has a super short timeout
Expand All @@ -277,7 +283,7 @@ func TestAuthenticateTokenTimeout(t *testing.T) {
// TIME: 6th second

// See if emergency flush happened
checkToken(t, "emergToken", tokenAuthenticator, accessTokenGetter, testClock, true)
checkToken(t, emergTokenClear, emergTokenHash, tokenAuthenticator, accessTokenGetter, testClock, true)
wait(t, putTokenSync)

wait(t, timeoutsSync) // from emergency flush because emergToken has a super short timeout
Expand All @@ -303,10 +309,10 @@ func TestAuthenticateTokenTimeout(t *testing.T) {
}

// this should fail, thus no call to wait(t, putTokenSync)
checkToken(t, "quickToken", tokenAuthenticator, accessTokenGetter, testClock, false)
checkToken(t, quickTokenClear, quickTokenHash, tokenAuthenticator, accessTokenGetter, testClock, false)

// while this should get updated
checkToken(t, "testToken", tokenAuthenticator, accessTokenGetter, testClock, true)
checkToken(t, testTokenClear, testTokenHash, tokenAuthenticator, accessTokenGetter, testClock, true)
wait(t, putTokenSync)

wait(t, timeoutsSync)
Expand All @@ -320,18 +326,18 @@ func TestAuthenticateTokenTimeout(t *testing.T) {
// TIME: 27th second

// this should get updated
checkToken(t, "slowToken", tokenAuthenticator, accessTokenGetter, testClock, true)
checkToken(t, slowTokenClear, slowTokenHash, tokenAuthenticator, accessTokenGetter, testClock, true)
wait(t, putTokenSync)

wait(t, timeoutsSync)

// while this should not fail
checkToken(t, "testToken", tokenAuthenticator, accessTokenGetter, testClock, true)
checkToken(t, testTokenClear, testTokenHash, tokenAuthenticator, accessTokenGetter, testClock, true)
wait(t, putTokenSync)

wait(t, timeoutsSync)
// and should be updated to last at least till the 31st second
token, err := accessTokenGetter.Get(context.TODO(), "testToken", metav1.GetOptions{})
token, err := accessTokenGetter.Get(context.TODO(), testTokenHash, metav1.GetOptions{})
if err != nil {
t.Error("Failed to get testToken")
} else {
Expand All @@ -357,13 +363,13 @@ func TestAuthenticateTokenTimeout(t *testing.T) {
wait(t, timeoutsSync)

// while this should not fail
checkToken(t, "testToken", tokenAuthenticator, accessTokenGetter, testClock, true)
checkToken(t, testTokenClear, testTokenHash, tokenAuthenticator, accessTokenGetter, testClock, true)
wait(t, putTokenSync)

wait(t, timeoutsSync)

// and should be updated to have a ZERO timeout
token, err = accessTokenGetter.Get(context.TODO(), "testToken", metav1.GetOptions{})
token, err = accessTokenGetter.Get(context.TODO(), testTokenHash, metav1.GetOptions{})
if err != nil {
t.Error("Failed to get testToken")
} else {
Expand All @@ -385,33 +391,33 @@ func (f fakeOAuthClientLister) List(selector labels.Selector) ([]*oauthv1.OAuthC
panic("not used")
}

func checkToken(t *testing.T, name string, authf authenticator.Token, tokens oauthclient.OAuthAccessTokenInterface, current clock.Clock, present bool) {
func checkToken(t *testing.T, tokenClear, tokenHash string, authf authenticator.Token, tokens oauthclient.OAuthAccessTokenInterface, current clock.Clock, present bool) {
t.Helper()
userInfo, found, err := authf.AuthenticateToken(context.TODO(), name)
userInfo, found, err := authf.AuthenticateToken(context.TODO(), tokenClear)
if present {
if !found {
t.Errorf("Did not find token %s!", name)
t.Errorf("Did not find token %s!", tokenClear)
}
if err != nil {
t.Errorf("Unexpected error checking for token %s: %v", name, err)
t.Errorf("Unexpected error checking for token %s: %v", tokenClear, err)
}
if userInfo == nil {
t.Errorf("Did not get a user for token %s!", name)
t.Errorf("Did not get a user for token %s!", tokenClear)
}
} else {
if found {
token, tokenErr := tokens.Get(context.TODO(), name, metav1.GetOptions{})
token, tokenErr := tokens.Get(context.TODO(), tokenHash, metav1.GetOptions{})
if tokenErr != nil {
t.Fatal(tokenErr)
}
t.Errorf("Found token (created=%s, timeout=%di, now=%s), but it should be gone!",
token.CreationTimestamp, token.InactivityTimeoutSeconds, current.Now())
}
if err != errTimedout {
t.Errorf("Unexpected error checking absence of token %s: %v", name, err)
t.Errorf("Unexpected error checking absence of token %s: %v", tokenHash, err)
}
if userInfo != nil {
t.Errorf("Unexpected user checking absence of token %s: %v", name, userInfo)
t.Errorf("Unexpected user checking absence of token %s: %v", tokenHash, userInfo)
}
}
}
Expand Down

0 comments on commit a40ec9c

Please sign in to comment.