Add Token Hashing

Reduces the attack surface by obfuscating tokens using SHA512 and
PBKDF2, thus they are only ever persisted client side, we continue to
maintain a plaintext in-memory cache for performace reasons.

Fixes #253

Author: spjmurray

charts/identity/crds/identity.unikorn-cloud.org_users.yaml

@@ -56,8 +56,8 @@ spec:
                   properties:
                     accessToken:
                       description: |-
-                        AccessToken s the access token currently issued for the
-                        session.
+                        AccessToken is the access token currently issued for the
+                        session.  This field is opaque, and may be hashed for security.
                       type: string
                     authorizationCodeID:
                       description: |-
@@ -74,7 +74,8 @@ spec:
                     refreshToken:
                       description: |-
                         RefreshToken is the single-use refresh token currently
-                        issued for the session.
+                        issued for the session. This field is opaque, and may be hashed
+                        for security.
                       type: string
                   required:
                   - accessToken

pkg/apis/unikorn/v1alpha1/helpers.go

@@ -17,8 +17,16 @@ limitations under the License.
 package v1alpha1
 
 import (
+	"bytes"
+	"crypto/pbkdf2"
+	"crypto/rand"
+	"crypto/sha512"
+	"encoding/base64"
 	"errors"
+	"fmt"
 	"slices"
+	"strconv"
+	"strings"
 
 	unikornv1core "github.com/unikorn-cloud/core/pkg/apis/unikorn/v1alpha1"
 
@@ -28,6 +36,10 @@ import (
 
 var (
 	ErrReference = errors.New("resource reference error")
+
+	ErrReadLength = errors.New("read length not as expected")
+
+	ErrValidation = errors.New("validation error")
 )
 
 // Paused implements the ReconcilePauser interface.
@@ -66,3 +78,102 @@ func (u *User) Session(clientID string) (*UserSession, error) {
 
 	return &u.Spec.Sessions[index], nil
 }
+
+const (
+	// saltLength is the number of bytes of cryptographically random data
+	// for salted hashing.  128 bits is recommended by NIST.
+	// See https://en.wikipedia.org/wiki/PBKDF2 for more details.
+	saltLength = 16
+
+	// hashIterations defines the number of iterations for a salted hash
+	// function.  The bigger the number, the harder to exhaustively search
+	// but also the longer it takes to compute.
+	// See https://en.wikipedia.org/wiki/PBKDF2 for more details.
+	// However... we are just hashing a JWE encoded token, not a password,
+	// thus it's already a millin times harder to guess than a password!
+	hashIterations = 1
+
+	// algorithmSha512 indicates the sha512 hashing algorithm.
+	algorithmSha512 = "sha512"
+)
+
+// Set encodes the raw value as a PBKDF2 key.
+func (t *Token) Set(value string) error {
+	salt := make([]byte, saltLength)
+
+	if n, err := rand.Read(salt); err != nil {
+		return err
+	} else if n != saltLength {
+		return fmt.Errorf("%w: unable to create salt for token hashing", ErrReadLength)
+	}
+
+	key, err := pbkdf2.Key(sha512.New, value, salt, hashIterations, sha512.Size)
+	if err != nil {
+		return err
+	}
+
+	encoding := algorithmSha512 + "$" + base64.RawURLEncoding.EncodeToString(salt) + "$" + strconv.Itoa(hashIterations) + "$" + base64.RawURLEncoding.EncodeToString(key)
+
+	*t = Token(encoding)
+
+	return nil
+}
+
+// Validate checks the provided value matches the hashed value in persistent
+// storage.
+func (t *Token) Validate(value string) error {
+	encoding := string(*t)
+
+	// TODO: this aids in the transition and can be removed soon after.
+	// Like 3 months after (by default) given the lifetime of service
+	// account tokens.
+	if !strings.Contains(encoding, "$") {
+		if value != encoding {
+			return fmt.Errorf("%w: plantext token values do not match", ErrValidation)
+		}
+
+		return nil
+	}
+
+	parts := strings.Split(encoding, "$")
+
+	if len(parts) != 4 {
+		return fmt.Errorf("%w: token encoding malformed", ErrValidation)
+	}
+
+	algorithm := parts[0]
+	if algorithm != algorithmSha512 {
+		return fmt.Errorf("%w: unsupported hash algorithm %s", ErrValidation, algorithm)
+	}
+
+	salt, err := base64.RawURLEncoding.DecodeString(parts[1])
+	if err != nil {
+		return err
+	}
+
+	iterations, err := strconv.Atoi(parts[2])
+	if err != nil {
+		return err
+	}
+
+	hash, err := base64.RawURLEncoding.DecodeString(parts[3])
+	if err != nil {
+		return err
+	}
+
+	key, err := pbkdf2.Key(sha512.New, value, salt, iterations, sha512.Size)
+	if err != nil {
+		return err
+	}
+
+	if !bytes.Equal(hash, key) {
+		return fmt.Errorf("%w: token mismatch", ErrValidation)
+	}
+
+	return nil
+}
+
+// Clear resets a token.
+func (t *Token) Clear() {
+	*t = Token("")
+}

pkg/apis/unikorn/v1alpha1/types.go

@@ -310,18 +310,22 @@ type UserSignup struct {
 	ClientID string `json:"clientID"`
 }
 
+// Token abstracts token storage in a CRD, with possible obfuscation.
+type Token string
+
 type UserSession struct {
 	// ClientID is the client the session is bound to.
 	ClientID string `json:"clientID"`
 	// AuthorizationCodeID is the authorization code ID used to generate
 	// the tokens.
 	AuthorizationCodeID string `json:"authorizationCodeID"`
-	// AccessToken s the access token currently issued for the
-	// session.
-	AccessToken string `json:"accessToken"`
+	// AccessToken is the access token currently issued for the
+	// session.  This field is opaque, and may be hashed for security.
+	AccessToken Token `json:"accessToken"`
 	// RefreshToken is the single-use refresh token currently
-	// issued for the session.
-	RefreshToken string `json:"refreshToken"`
+	// issued for the session. This field is opaque, and may be hashed
+	// for security.
+	RefreshToken Token `json:"refreshToken"`
 	// LastAuthentication records when the user last authenticated.
 	LastAuthentication *metav1.Time `json:"lastAuthentication,omitempty"`
 }
@@ -382,7 +386,7 @@ type ServiceAccountSpec struct {
 	Tags unikornv1core.TagList `json:"tags,omitempty"`
 	// AccessToken is the encrypted access token that is valid for this
 	// service acocunt.
-	AccessToken string `json:"accessToken"`
+	AccessToken Token `json:"accessToken"`
 	// Expiry is a hint as to when the issued token will exipre.
 	// The access token itself is the source of truth, provided the private key is
 	// still around, so this is a fallback, as well as a cache to improve API read

pkg/handler/serviceaccounts/client.go

@@ -96,7 +96,7 @@ func convert(in *unikornv1.ServiceAccount, groups *unikornv1.GroupList) *openapi
 
 // convertCreate converts from Kubernetes into OpenAPi for create/update requests that
 // have extra information e.g. the access token.
-func convertCreate(in *unikornv1.ServiceAccount, groups *unikornv1.GroupList) *openapi.ServiceAccountCreate {
+func convertCreate(in *unikornv1.ServiceAccount, groups *unikornv1.GroupList, accessToken string) *openapi.ServiceAccountCreate {
 	temp := convert(in, groups)
 
 	out := &openapi.ServiceAccountCreate{
@@ -105,7 +105,7 @@ func convertCreate(in *unikornv1.ServiceAccount, groups *unikornv1.GroupList) *o
 		Status:   temp.Status,
 	}
 
-	out.Status.AccessToken = &in.Spec.AccessToken
+	out.Status.AccessToken = &accessToken
 
 	return out
 }
@@ -163,14 +163,6 @@ func (c *Client) generate(ctx context.Context, organization *organizations.Meta,
 		},
 	}
 
-	tokens, err := c.generateAccessToken(ctx, organization, out.Name)
-	if err != nil {
-		return nil, errors.OAuth2ServerError("unable to issue access token").WithError(err)
-	}
-
-	out.Spec.Expiry = &metav1.Time{Time: tokens.Expiry}
-	out.Spec.AccessToken = tokens.AccessToken
-
 	return out, nil
 }
 
@@ -246,6 +238,17 @@ func (c *Client) Create(ctx context.Context, organizationID string, request *ope
 		return nil, errors.OAuth2ServerError("failed to generate service account").WithError(err)
 	}
 
+	tokens, err := c.generateAccessToken(ctx, organization, resource.Name)
+	if err != nil {
+		return nil, errors.OAuth2ServerError("unable to issue access token").WithError(err)
+	}
+
+	resource.Spec.Expiry = &metav1.Time{Time: tokens.Expiry}
+
+	if err := resource.Spec.AccessToken.Set(tokens.AccessToken); err != nil {
+		return nil, errors.OAuth2ServerError("failed to set access token").WithError(err)
+	}
+
 	if err := c.client.Create(ctx, resource); err != nil {
 		return nil, errors.OAuth2ServerError("failed to create service account").WithError(err)
 	}
@@ -259,7 +262,7 @@ func (c *Client) Create(ctx context.Context, organizationID string, request *ope
 		return nil, err
 	}
 
-	return convertCreate(resource, groups), nil
+	return convertCreate(resource, groups, tokens.AccessToken), nil
 }
 
 // Get retrieves information about a service account.
@@ -370,7 +373,10 @@ func (c *Client) Rotate(ctx context.Context, organizationID, serviceAccountID st
 
 	updated := current.DeepCopy()
 	updated.Spec.Expiry = &metav1.Time{Time: tokens.Expiry}
-	updated.Spec.AccessToken = tokens.AccessToken
+
+	if err := updated.Spec.AccessToken.Set(tokens.AccessToken); err != nil {
+		return nil, errors.OAuth2ServerError("failed to set access token").WithError(err)
+	}
 
 	if err := c.client.Patch(ctx, updated, client.MergeFrom(current)); err != nil {
 		return nil, errors.OAuth2ServerError("failed to patch group").WithError(err)
@@ -383,7 +389,7 @@ func (c *Client) Rotate(ctx context.Context, organizationID, serviceAccountID st
 		return nil, err
 	}
 
-	return convertCreate(updated, groups), nil
+	return convertCreate(updated, groups, tokens.AccessToken), nil
 }
 
 // Delete removes the service account and revokes the access token.

pkg/oauth2/oauth2.go

@@ -1639,15 +1639,15 @@ func (a *Authenticator) validateRefreshToken(ctx context.Context, r *http.Reques
 		return errors.OAuth2InvalidGrant("no active session for user found")
 	}
 
-	if user.Spec.Sessions[index].RefreshToken != refreshToken {
-		return errors.OAuth2InvalidGrant("refresh token reuse")
+	if err := user.Spec.Sessions[index].RefreshToken.Validate(refreshToken); err != nil {
+		return errors.OAuth2InvalidGrant("refresh token invalid or possible reuse").WithError(err)
 	}
 
 	// Things can still go wrong between here and issuing the new token, so invalidate
 	// the session now rather than relying on the reissue doing it for us.
 	a.InvalidateToken(ctx, user.Spec.Sessions[index].AccessToken)
 
-	user.Spec.Sessions[index].RefreshToken = ""
+	user.Spec.Sessions[index].RefreshToken.Clear()
 
 	if err := a.client.Update(ctx, user); err != nil {
 		return errors.OAuth2ServerError("failed to revoke user session").WithError(err)

pkg/oauth2/tokens.go

@@ -164,10 +164,14 @@ func (a *Authenticator) updateSession(ctx context.Context, user *unikornv1.User,
 		a.InvalidateToken(ctx, session.AccessToken)
 	}
 
-	session.AccessToken = tokens.AccessToken
+	if err := session.AccessToken.Set(tokens.AccessToken); err != nil {
+		return time.Time{}, err
+	}
 
 	if tokens.RefreshToken != nil {
-		session.RefreshToken = *tokens.RefreshToken
+		if err := session.RefreshToken.Set(*tokens.RefreshToken); err != nil {
+			return time.Time{}, err
+		}
 	}
 
 	if info.Interactive {
@@ -352,8 +356,8 @@ func (a *Authenticator) verifyServiceAccount(ctx context.Context, info *VerifyIn
 		return err
 	}
 
-	if info.Token != serviceAccount.Spec.AccessToken {
-		return fmt.Errorf("%w: service account token invalid", ErrTokenVerification)
+	if err := serviceAccount.Spec.AccessToken.Validate(info.Token); err != nil {
+		return fmt.Errorf("%w: service account token invalid", err)
 	}
 
 	return nil
@@ -379,15 +383,29 @@ func (a *Authenticator) verifyUserSession(ctx context.Context, info *VerifyInfo,
 		return fmt.Errorf("%w: no active session for token", ErrTokenVerification)
 	}
 
-	if user.Spec.Sessions[index].AccessToken != info.Token {
-		return fmt.Errorf("%w: token invalid for active session", ErrTokenVerification)
+	if err := user.Spec.Sessions[index].AccessToken.Validate(info.Token); err != nil {
+		return fmt.Errorf("%w: token invalid for active session", err)
 	}
 
 	return nil
 }
 
 // InvalidateToken immediately invalidates the token so it's unusable again.
 // TODO: this only considers caching in the identity service, it's still usable.
-func (a *Authenticator) InvalidateToken(ctx context.Context, token string) {
-	a.tokenCache.Remove(token)
+func (a *Authenticator) InvalidateToken(ctx context.Context, token unikornv1.Token) {
+	// TODO: This needs a bit of a rethink, we cache the raw access token in memory
+	// to avoid the decryption penalty, however the token stored in persistent storage
+	// is hashed, thus we need to do a potentially costly exhaustive search.
+	for _, key := range a.tokenCache.Keys() {
+		t, ok := key.(string)
+		if !ok {
+			continue
+		}
+
+		if err := token.Validate(t); err != nil {
+			continue
+		}
+
+		a.tokenCache.Remove(key)
+	}
 }