Skip to content

authenticator: Zeroize ValidatedClaims and OidcClaims on drop#35704

Draft
jasonhernandez wants to merge 2 commits intomainfrom
jasonhernandez/sec-115-authenticator-zeroize
Draft

authenticator: Zeroize ValidatedClaims and OidcClaims on drop#35704
jasonhernandez wants to merge 2 commits intomainfrom
jasonhernandez/sec-115-authenticator-zeroize

Conversation

@jasonhernandez
Copy link
Copy Markdown
Contributor

Motivation

Defense-in-depth: zero user identity and JWT claim data from memory after use.

Description

  • Implement Zeroize + ZeroizeOnDrop for ValidatedClaims (user identity string)
  • Implement Zeroize for OidcClaims (issuer, audience, timestamps, unknown claims)
  • Document that OidcDecodingKey wraps opaque jsonwebtoken::DecodingKey and cannot be zeroized
  • Document that borrowed JWT token &str parameters are the caller's responsibility
  • Regression tests for trait bounds and field clearing

No public API changes.

Depends on

Part of SEC-115.

@github-actions
Copy link
Copy Markdown

Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone.

PR title guidelines

  • Use imperative mood: "Fix X" not "Fixed X" or "Fixes X"
  • Be specific: "Fix panic in catalog sync when controller restarts" not "Fix bug" or "Update catalog code"
  • Prefix with area if helpful: compute: , storage: , adapter: , sql:

Pre-merge checklist

  • The PR title is descriptive and will make sense in the git log.
  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).

@jasonhernandez jasonhernandez force-pushed the jasonhernandez/sec-115-authenticator-zeroize branch from e9bc8be to 1c8aafa Compare March 26, 2026 20:37
@jasonhernandez jasonhernandez force-pushed the jasonhernandez/sec-115-ore-zeroize-feature branch 4 times, most recently from 1308d53 to 9b3927b Compare March 31, 2026 17:21
Base automatically changed from jasonhernandez/sec-115-ore-zeroize-feature to main March 31, 2026 17:30
Implement Zeroize for OidcClaims (issuer, audience, unknown claims) and
Zeroize + ZeroizeOnDrop for ValidatedClaims (user identity string).

Document that OidcDecodingKey wraps an opaque foreign type and cannot be
zeroized, and that borrowed JWT token strings are the caller's
responsibility.

Part of SEC-115.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jasonhernandez jasonhernandez force-pushed the jasonhernandez/sec-115-authenticator-zeroize branch from 1c8aafa to eb68d55 Compare March 31, 2026 18:56
Adding `Drop` to `ValidatedClaims` prevents moving fields out directly.
Use `std::mem::take` to extract values while leaving empty defaults for
the Drop impl to zeroize.

Fixes: mz-pgwire and mz-environmentd build failures.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant