Skip to content

Conversation

@Vianpyro
Copy link
Member

@Vianpyro Vianpyro commented Apr 13, 2025

Brief Description of Changes

This pull request upgrades the JWT security model from HS256 (HMAC with SHA-256) to RS256 (RSA with SHA-256) and introduces support for rotating RSA keys using KIDs (Key IDs).

Detailed Changes

  • Switched JWT algorithm from HS256 to RS256 to leverage asymmetric cryptography.
  • Implemented support for JWT header kid to identify which public key to use during verification.
  • RSA key rotation is now supported via cron jobs that:
    • Generate new RSA key pairs periodically.
    • Update the current active signing key.
    • Archive older public keys for continued verification of existing tokens.
  • Updated environment/config handling to support multiple public keys mapped by KID.
  • Refactored token generation to include the correct kid in the JWT header.
  • Enhanced error handling when a kid is missing, invalid, or points to an unknown public key.

Objective of Changes/Issue Resolved (if applicable)

This change improves the security, maintainability, and scalability of our JWT authentication system. By using RS256:

  • The private signing key remains secure and private.
  • Public verification keys can be shared safely across services.

By adding KID-based rotation:

  • We reduce the risk of long-term key compromise.
  • Services can verify tokens issued with older keys during the rotation window.

Resolves the problem of shared-secret risks and improves future readiness for distributed and multi-tenant environments.

Screenshots (if applicable)

Not applicable.

Verification Steps

  1. Generate an initial RSA key pair with a unique KID.
  2. Export JWT_PRIVATE_KEY, JWT_PUBLIC_KEYS (map of KIDs to public keys), and JWT_ACTIVE_KID.
  3. Start the API — ensure it initializes correctly and reads the correct key config.
  4. Authenticate and verify that:
    • Tokens are signed with the current private key.
    • The JWT header includes the correct kid.
    • Token verification succeeds using the matching public key from the map.
  5. Manually rotate the key (as cron would) and verify:
    • New tokens are signed with the new private key and updated kid.
    • Previously issued tokens still verify correctly using archived public keys.

Notes for Reviewer

  • The cron logic for rotation is external to this PR, but the app is now compatible with externally managed key updates.
  • Review the structure and loading logic for the public key map (JWT_PUBLIC_KEYS) and how JWT_ACTIVE_KID is injected into the JWTs.
  • Let me know if you'd like helper scripts or documentation for key generation.

Vianpyro added 14 commits April 6, 2025 23:56
…, modify jwt_helper to use RSA keys, and add entrypoint script for key generation
…anagement in jwt_helper, and modify .gitignore for key files
…w jwtoken module, removing deprecated admin routes and cleanup scripts.
@Vianpyro Vianpyro added complexity: experimental Tasks exploring untested tools or approaches for infrastructure or deployment. priority: high Important tasks that require immediate attention. status: completed Fully implemented and verified. type: security Issues or improvements related to app security. labels Apr 13, 2025
@Vianpyro Vianpyro requested a review from Copilot April 13, 2025 15:43
@Vianpyro Vianpyro self-assigned this Apr 13, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 25 out of 28 changed files in this pull request and generated 1 comment.

Files not reviewed (3)
  • .devcontainer/Dockerfile: Language not supported
  • Dockerfile: Language not supported
  • entrypoint.sh: Language not supported
Comments suppressed due to low confidence (1)

jwtoken/tokens.py:55

  • [nitpick] Consider providing a more descriptive error message for token type mismatches to aid in debugging, such as including the expected type versus the received type.
raise jwt.InvalidTokenError("Invalid token type")

Vianpyro and others added 2 commits April 13, 2025 11:45
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Vianpyro Vianpyro added status: in progress Currently being worked on. and removed status: completed Fully implemented and verified. labels Apr 13, 2025
Vianney Veremme added 5 commits April 13, 2025 16:29
- Introduced key rotation logic to ensure keys directory and active_kid.txt file exist.
- Updated key paths in the rotate_keys function to use constants from the new config file.
- Created a new config file for JWT key management constants.
- Refactored keys_cleanup.py to use the new CREATED_AT_FILE constant.
…rotation before starting the app; update keys_rotation.py for improved key management and error handling.
… startup; add entrypoint.sh for improved process management.
@Vianpyro Vianpyro changed the title Security/use rsa for jwt Change JWT encryption from HMAC to RSA Apr 13, 2025
@Vianpyro Vianpyro changed the title Change JWT encryption from HMAC to RSA Change JWT encryption from HMAC to RSA with rotating keys Apr 13, 2025
@Vianpyro Vianpyro added status: completed Fully implemented and verified. and removed status: in progress Currently being worked on. labels Apr 13, 2025
@Vianpyro Vianpyro merged commit 5079f28 into main Apr 14, 2025
17 of 19 checks passed
@Vianpyro Vianpyro deleted the security/use_rsa_for_jwt branch April 14, 2025 11:06
Vianpyro added a commit that referenced this pull request Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

complexity: experimental Tasks exploring untested tools or approaches for infrastructure or deployment. priority: high Important tasks that require immediate attention. status: completed Fully implemented and verified. type: security Issues or improvements related to app security.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants