Refactor cryptokeys and jwtutils interfaces and logic

sreya · sreya · commit e654a6552ca3 · 2024-10-03T22:30:07.000Z
- Enhance comments for key interfaces to clarify usage
  and considerations for time validity and clock skew.
- Refactor JWE/JWS logic to simplify serialization and
  deserialization processes, ensuring more efficient and
  concise handling of JWTs. Implement compact serialization
  and remove unnecessary base64 encoding.
diff --git a/coderd/cryptokeys/keycache.go b/coderd/cryptokeys/keycache.go
@@ -15,13 +15,27 @@ var (
 )
 
 type EncryptionKeycache interface {
+	// EncryptingKey returns the latest valid key for encrypting payloads. A valid
+	// key is one that is both past its start time and before its deletion time.
 	EncryptingKey(ctx context.Context) (id string, key interface{}, err error)
+	// DecryptingKey returns the key with the provided id which maps to its sequence
+	// number. The key is valid for decryption as long as it is not deleted or past
+	// its deletion date. We must allow for keys prior to their start time to
+	// account for clock skew between peers (one key may be past its start time on
+	// one machine while another is not).
 	DecryptingKey(ctx context.Context, id string) (key interface{}, err error)
 	io.Closer
 }
 
 type SigningKeycache interface {
+	// SigningKey returns the latest valid key for signing. A valid key is one
+	// that is both past its start time and before its deletion time.
 	SigningKey(ctx context.Context) (id string, key interface{}, err error)
+	// VerifyingKey returns the key with the provided id which should map to its
+	// sequence number. The key is valid for verifying as long as it is not deleted
+	// or past its deletion date. We must allow for keys prior to their start time
+	// to account for clock skew between peers (one key may be past its start time
+	// on one machine while another is not).
 	VerifyingKey(ctx context.Context, id string) (key interface{}, err error)
 	io.Closer
 }
diff --git a/coderd/jwtutils/jwe.go b/coderd/jwtutils/jwe.go
@@ -2,7 +2,6 @@ package jwtutils
 
 import (
 	"context"
-	"encoding/base64"
 	"encoding/json"
 	"time"
 
@@ -58,15 +57,17 @@ func Encrypt(ctx context.Context, e EncryptKeyProvider, claims Claims) (string,
 		return "", xerrors.Errorf("encrypt: %w", err)
 	}
 
-	serialized := []byte(encrypted.FullSerialize())
-	return base64.RawURLEncoding.EncodeToString(serialized), nil
+	compact, err := encrypted.CompactSerialize()
+	if err != nil {
+		return "", xerrors.Errorf("compact serialize: %w", err)
+	}
+
+	return compact, nil
 }
 
 // DecryptOptions are options for decrypting a JWE.
 type DecryptOptions struct {
-	RegisteredClaims jwt.Expected
-
-	// The following should only be used for JWEs.
+	RegisteredClaims           jwt.Expected
 	KeyAlgorithm               jose.KeyAlgorithm
 	ContentEncryptionAlgorithm jose.ContentEncryption
 }
@@ -85,12 +86,7 @@ func Decrypt(ctx context.Context, d DecryptKeyProvider, token string, claims Cla
 		opt(&options)
 	}
 
-	encrypted, err := base64.RawURLEncoding.DecodeString(token)
-	if err != nil {
-		return xerrors.Errorf("decode: %w", err)
-	}
-
-	object, err := jose.ParseEncrypted(string(encrypted),
+	object, err := jose.ParseEncrypted(token,
 		[]jose.KeyAlgorithm{options.KeyAlgorithm},
 		[]jose.ContentEncryption{options.ContentEncryptionAlgorithm},
 	)
diff --git a/coderd/jwtutils/jws.go b/coderd/jwtutils/jws.go
@@ -71,9 +71,7 @@ func Sign(ctx context.Context, s SigningKeyProvider, claims Claims) (string, err
 
 // VerifyOptions are options for verifying a JWT.
 type VerifyOptions struct {
-	RegisteredClaims jwt.Expected
-
-	// The following are only used for JWSs.
+	RegisteredClaims   jwt.Expected
 	SignatureAlgorithm jose.SignatureAlgorithm
 }
 

Original file line number	Diff line number	Diff line change
`@@ -71,9 +71,7 @@ func Sign(ctx context.Context, s SigningKeyProvider, claims Claims) (string, err`
`71`	`71`
`72`	`72`	`// VerifyOptions are options for verifying a JWT.`
`73`	`73`	`type VerifyOptions struct {`
`74`		`- RegisteredClaims jwt.Expected`
`75`		`-`
`76`		`- // The following are only used for JWSs.`
	`74`	`+ RegisteredClaims jwt.Expected`
`77`	`75`	`SignatureAlgorithm jose.SignatureAlgorithm`
`78`	`76`	`}`
`79`	`77`