vault: harden auth review findings

- Cap JWKS response body to 1 MB to prevent resource exhaustion
- Make all AuthResult fields unexported with accessor methods
- Add iat claim validation to JWT parser
- Annotate unreachable return in allowlist retry loop
- Fix misleading test name for digest verification delegation

Author: prashantkumar1982

core/capabilities/vault/allow_list_based_auth.go

@@ -71,11 +71,11 @@ func (r *allowListBasedAuth) AuthorizeRequest(ctx context.Context, req jsonrpc.R
 
 	digestKey := string(allowlistedRequest.RequestDigest[:])
 	r.lggr.Debugw("AllowListBasedAuth authorization succeeded", "method", req.Method, "requestID", req.ID, "authorizedRequestStr", digestKey, "owner", allowlistedRequest.Owner.Hex(), "expiryTimestamp", allowlistedRequest.ExpiryTimestamp)
-	return &AuthResult{
-		workflowOwner: allowlistedRequest.Owner.Hex(),
-		Digest:        digestKey,
-		ExpiresAt:     int64(allowlistedRequest.ExpiryTimestamp),
-	}, nil
+	return NewAllowListBasedAuthResult(
+		allowlistedRequest.Owner.Hex(),
+		digestKey,
+		int64(allowlistedRequest.ExpiryTimestamp),
+	), nil
 }
 
 func (r *allowListBasedAuth) findAllowlistedItemWithRetry(ctx context.Context, req jsonrpc.Request[json.RawMessage], requestDigest string, requestDigestBytes32 [32]byte) (*workflow_registry_wrapper_v2.WorkflowRegistryOwnerAllowlistedRequest, []string, error) {
@@ -109,7 +109,7 @@ func (r *allowListBasedAuth) findAllowlistedItemWithRetry(ctx context.Context, r
 		}
 	}
 
-	return nil, nil, nil
+	return nil, nil, nil // unreachable: loop always returns
 }
 
 func (r *allowListBasedAuth) fetchAllowlistedItem(allowListedRequests []workflow_registry_wrapper_v2.WorkflowRegistryOwnerAllowlistedRequest, digest [32]byte) *workflow_registry_wrapper_v2.WorkflowRegistryOwnerAllowlistedRequest {

core/capabilities/vault/allow_list_based_auth_test.go

@@ -179,8 +179,8 @@ func testAuthForRequests(t *testing.T, allowlistedRequest, notAllowlistedRequest
 	authResult, err := auth.AuthorizeRequest(t.Context(), allowlistedRequest)
 	require.NoError(t, err)
 	require.Equal(t, owner.Hex(), authResult.AuthorizedOwner())
-	require.Equal(t, expiry, authResult.ExpiresAt)
-	require.NotEmpty(t, authResult.Digest)
+	require.Equal(t, expiry, authResult.GetExpiresAt())
+	require.NotEmpty(t, authResult.GetDigest())
 
 	// Same request is still authorized here; replay protection lives in the generic Authorizer.
 	authResult, err = auth.AuthorizeRequest(t.Context(), allowlistedRequest)
@@ -245,5 +245,5 @@ func TestAllowListBasedAuth_RetriesUntilRequestIsAllowlisted(t *testing.T) {
 	authResult, err := auth.AuthorizeRequest(t.Context(), req)
 	require.NoError(t, err)
 	require.Equal(t, owner.Hex(), authResult.AuthorizedOwner())
-	require.Equal(t, expiry, authResult.ExpiresAt)
+	require.Equal(t, expiry, authResult.GetExpiresAt())
 }

core/capabilities/vault/authorizer.go

@@ -14,24 +14,24 @@ import (
 type AuthResult struct {
 	orgID         string
 	workflowOwner string
-	Digest        string
-	ExpiresAt     int64
+	digest        string
+	expiresAt     int64
 }
 
 func NewAllowListBasedAuthResult(workflowOwner, digest string, expiresAt int64) *AuthResult {
 	return &AuthResult{
 		workflowOwner: workflowOwner,
-		Digest:        digest,
-		ExpiresAt:     expiresAt,
+		digest:        digest,
+		expiresAt:     expiresAt,
 	}
 }
 
 func NewJWTBasedAuthResult(orgID, workflowOwner, digest string, expiresAt int64) *AuthResult {
 	return &AuthResult{
 		orgID:         orgID,
 		workflowOwner: workflowOwner,
-		Digest:        digest,
-		ExpiresAt:     expiresAt,
+		digest:        digest,
+		expiresAt:     expiresAt,
 	}
 }
 
@@ -46,6 +46,23 @@ func (a *AuthResult) AuthorizedOwner() string {
 	return a.workflowOwner
 }
 
+// GetDigest returns the request digest used for replay protection.
+func (a *AuthResult) GetDigest() string {
+	if a == nil {
+		return ""
+	}
+	return a.digest
+}
+
+// GetExpiresAt returns the unix timestamp (UTC) after which this
+// authorization is no longer valid.
+func (a *AuthResult) GetExpiresAt() int64 {
+	if a == nil {
+		return 0
+	}
+	return a.expiresAt
+}
+
 // GetUntrustedWorkflowOwner returns the workflow owner only for JWTBasedAuth results.
 func (a *AuthResult) GetUntrustedWorkflowOwner() (string, error) {
 	if a == nil {
@@ -93,11 +110,11 @@ func (a *authorizer) AuthorizeRequest(ctx context.Context, req jsonrpc.Request[j
 		a.lggr.Errorw("auth mechanism returned nil auth result", "method", req.Method, "requestID", req.ID, "hasAuth", req.Auth != "")
 		return nil, err
 	}
-	if err := a.replayGuard.CheckAndRecord(authResult.Digest, authResult.ExpiresAt); err != nil {
-		a.lggr.Debugw("replay guard rejected request", "method", req.Method, "requestID", req.ID, "owner", authResult.AuthorizedOwner(), "digest", authResult.Digest, "expiresAt", authResult.ExpiresAt, "hasAuth", req.Auth != "", "error", err)
+	if err := a.replayGuard.CheckAndRecord(authResult.GetDigest(), authResult.GetExpiresAt()); err != nil {
+		a.lggr.Debugw("replay guard rejected request", "method", req.Method, "requestID", req.ID, "owner", authResult.AuthorizedOwner(), "digest", authResult.GetDigest(), "expiresAt", authResult.GetExpiresAt(), "hasAuth", req.Auth != "", "error", err)
 		return nil, err
 	}
-	a.lggr.Debugw("request authorized", "method", req.Method, "requestID", req.ID, "owner", authResult.AuthorizedOwner(), "digest", authResult.Digest, "expiresAt", authResult.ExpiresAt, "hasAuth", req.Auth != "")
+	a.lggr.Debugw("request authorized", "method", req.Method, "requestID", req.ID, "owner", authResult.AuthorizedOwner(), "digest", authResult.GetDigest(), "expiresAt", authResult.GetExpiresAt(), "hasAuth", req.Auth != "")
 	return authResult, nil
 }
 

core/capabilities/vault/authorizer_test.go

@@ -86,7 +86,7 @@ func TestAuthorizer_UsesJWTWhenGateEnabled(t *testing.T) {
 	require.Equal(t, "0xworkflow", untrustedWorkflowOwner)
 }
 
-func TestAuthorizer_RejectsJWTDigestMismatch(t *testing.T) {
+func TestAuthorizer_DelegatesDigestVerificationToJWTAuth(t *testing.T) {
 	params, err := json.Marshal(vaultcommon.CreateSecretsRequest{})
 	require.NoError(t, err)
 

core/capabilities/vault/jwt_based_auth.go

@@ -200,6 +200,7 @@ func (v *jwtBasedAuth) validateToken(ctx context.Context, tokenString string) (*
 		jwt.WithIssuer(v.issuerURL),
 		jwt.WithAudience(v.audience),
 		jwt.WithExpirationRequired(),
+		jwt.WithIssuedAt(),
 	)
 	if err != nil {
 		return nil, fmt.Errorf("%w: %w", ErrInvalidToken, err)
@@ -346,7 +347,8 @@ func (v *jwtBasedAuth) refreshJWKS(ctx context.Context) error {
 		return fmt.Errorf("%w: HTTP %d", ErrJWKSFetchFailed, resp.StatusCode)
 	}
 
-	body, err := io.ReadAll(resp.Body)
+	const maxJWKSBodySize = 1 << 20 // 1 MB
+	body, err := io.ReadAll(io.LimitReader(resp.Body, maxJWKSBodySize))
 	if err != nil {
 		return fmt.Errorf("%w: %w", ErrJWKSFetchFailed, err)
 	}