Switch JWT signing from HS256 to RS256 (4096-bit RSA)
- Replace symmetric SECRET_KEY with JWT_PRIVATE_KEY / JWT_PUBLIC_KEY (PEM) - Add iat claim to every token - Add expand_newlines validator in config for single-line .env PEM values - Add scripts/generate_jwt_keys.py key-generation helper - Update security-auditor agent JWT checklist with RS256 enforcement rules - Mark RS256 as done in TODO.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -21,7 +21,7 @@ You are a senior application security engineer embedded in this project. Unlike
|
|||||||
- **Existing security controls** (do not remove or weaken):
|
- **Existing security controls** (do not remove or weaken):
|
||||||
- `backend/app/core/sanitize.py` — `sanitize_str`, `normalize_email`, `validate_phone`, `validate_date_of_birth` applied to all user inputs before DB
|
- `backend/app/core/sanitize.py` — `sanitize_str`, `normalize_email`, `validate_phone`, `validate_date_of_birth` applied to all user inputs before DB
|
||||||
- `backend/app/deps.py` — `get_current_admin` returns 404 (not 403) for non-admins
|
- `backend/app/deps.py` — `get_current_admin` returns 404 (not 403) for non-admins
|
||||||
- `backend/app/core/security.py` — bcrypt direct (no passlib), JWT via python-jose
|
- `backend/app/core/security.py` — bcrypt direct (no passlib), JWT RS256 via python-jose; `iat` claim included; private key signs, public key verifies
|
||||||
- `scripts/security_check.py` — pre-commit hook: secrets, dangerous patterns, weak crypto, SQL injection patterns, sanitization patterns, bandit
|
- `scripts/security_check.py` — pre-commit hook: secrets, dangerous patterns, weak crypto, SQL injection patterns, sanitization patterns, bandit
|
||||||
- All SQLAlchemy queries use ORM bound parameters — no raw `text()` with string formatting
|
- All SQLAlchemy queries use ORM bound parameters — no raw `text()` with string formatting
|
||||||
|
|
||||||
@@ -61,16 +61,20 @@ When reviewing any authentication code, verify all of the following:
|
|||||||
| Check | What to look for | Severity |
|
| Check | What to look for | Severity |
|
||||||
|---|---|---|
|
|---|---|---|
|
||||||
| Algorithm confusion | `algorithms=["none"]` or `algorithm="none"` in `jwt.decode()` | Critical |
|
| Algorithm confusion | `algorithms=["none"]` or `algorithm="none"` in `jwt.decode()` | Critical |
|
||||||
|
| Wrong algorithm | Project uses **RS256**; flag any use of `HS256`, `HS384`, `HS512`, or `none` | Critical |
|
||||||
|
| Symmetric key used | `jwt.encode/decode` must use `JWT_PRIVATE_KEY` / `JWT_PUBLIC_KEY` (PEM); flag any use of a plain `SECRET_KEY` string for JWT | Critical |
|
||||||
| Expiry enforcement | `verify_exp=False` or `options={"verify_exp": False}` | Critical |
|
| Expiry enforcement | `verify_exp=False` or `options={"verify_exp": False}` | Critical |
|
||||||
| Token lifetime | `ACCESS_TOKEN_EXPIRE_MINUTES` — must be ≤ 480 (8 h); flag `timedelta(days=...)` in token creation | High |
|
| Token lifetime | `ACCESS_TOKEN_EXPIRE_MINUTES` — must be ≤ 480 (8 h); flag `timedelta(days=...)` in token creation | High |
|
||||||
| Secret key strength | `SECRET_KEY` must come from env var, ≥ 32 random chars; flag hardcoded strings | High |
|
| Key loaded from env | `JWT_PRIVATE_KEY` and `JWT_PUBLIC_KEY` must come from env vars — flag hardcoded PEM strings | High |
|
||||||
| Algorithm pinned | `jwt.decode()` must pass `algorithms=["HS256"]` (or project algorithm) explicitly — never a variable | High |
|
| Algorithm pinned | `jwt.decode()` must pass `algorithms=["RS256"]` explicitly — never a variable or list containing other algorithms | High |
|
||||||
| Missing claims | Token payload should include `sub`, `exp`, `iat`; flag if `iat` is absent | Medium |
|
| Missing claims | Token payload must include `sub`, `exp`, `iat`; flag if any are absent | Medium |
|
||||||
| Token storage | Frontend stores JWT in `localStorage` — note the XSS exposure tradeoff; recommend `httpOnly` cookie migration when hardening | Medium |
|
| Token storage | Frontend stores JWT in `localStorage` — note the XSS exposure tradeoff; recommend `httpOnly` cookie migration when hardening | Medium |
|
||||||
| No refresh tokens | Project policy: no permanent sessions, no refresh tokens. Flag any `refresh_token` implementation | Medium |
|
| No refresh tokens | Project policy: no permanent sessions, no refresh tokens. Flag any `refresh_token` implementation | Medium |
|
||||||
| No "remember me" | No `remember_me` or extended-expiry paths in auth flow | Medium |
|
| No "remember me" | No `remember_me` or extended-expiry paths in auth flow | Medium |
|
||||||
|
|
||||||
Current project policy: **8-hour JWT, no refresh tokens, no permanent login.**
|
Current project policy: **RS256 (4096-bit RSA), 8-hour JWT, no refresh tokens, no permanent login.**
|
||||||
|
|
||||||
|
Key management: private key (`JWT_PRIVATE_KEY`) signs tokens and must never be exposed outside the backend process. Public key (`JWT_PUBLIC_KEY`) verifies tokens and can be shared. Both are generated by `scripts/generate_jwt_keys.py`.
|
||||||
|
|
||||||
## Hard rules
|
## Hard rules
|
||||||
|
|
||||||
|
|||||||
+5
-1
@@ -1,3 +1,7 @@
|
|||||||
DATABASE_URL=postgresql+asyncpg://postgres:password@localhost:5432/destroying_sap
|
DATABASE_URL=postgresql+asyncpg://postgres:password@localhost:5432/destroying_sap
|
||||||
SECRET_KEY=change-me-in-production
|
|
||||||
CORS_ORIGINS=["http://localhost:5173"]
|
CORS_ORIGINS=["http://localhost:5173"]
|
||||||
|
|
||||||
|
# RS256 JWT keys — generate with: python scripts/generate_jwt_keys.py
|
||||||
|
# Paste the output of that script here (single-line PEM with \n escaped)
|
||||||
|
JWT_PRIVATE_KEY=""
|
||||||
|
JWT_PUBLIC_KEY=""
|
||||||
|
|||||||
@@ -11,6 +11,7 @@
|
|||||||
## Auth / session security
|
## Auth / session security
|
||||||
|
|
||||||
- [x] **8-hour JWT expiry** — `ACCESS_TOKEN_EXPIRE_MINUTES = 60 * 8`; no permanent login
|
- [x] **8-hour JWT expiry** — `ACCESS_TOKEN_EXPIRE_MINUTES = 60 * 8`; no permanent login
|
||||||
|
- [x] **RS256 JWT signing** — 4096-bit RSA asymmetric keys; `iat` claim included; generate keys with `scripts/generate_jwt_keys.py`
|
||||||
- [ ] **No refresh tokens** — refresh token flow not implemented; if added later, must use `httpOnly` cookies and rotation
|
- [ ] **No refresh tokens** — refresh token flow not implemented; if added later, must use `httpOnly` cookies and rotation
|
||||||
- [ ] **`httpOnly` cookie migration** — currently storing JWT in `localStorage` (XSS-exposed); migrate to `httpOnly` cookie when hardening for production
|
- [ ] **`httpOnly` cookie migration** — currently storing JWT in `localStorage` (XSS-exposed); migrate to `httpOnly` cookie when hardening for production
|
||||||
|
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
from pydantic import field_validator
|
||||||
from pydantic_settings import BaseSettings
|
from pydantic_settings import BaseSettings
|
||||||
|
|
||||||
|
|
||||||
@@ -6,12 +7,20 @@ class Settings(BaseSettings):
|
|||||||
|
|
||||||
DATABASE_URL: str = "postgresql+asyncpg://postgres:password@localhost:5432/destroying_sap"
|
DATABASE_URL: str = "postgresql+asyncpg://postgres:password@localhost:5432/destroying_sap"
|
||||||
|
|
||||||
SECRET_KEY: str = "change-me-in-production"
|
# RS256 asymmetric signing — generate keys with scripts/generate_jwt_keys.py
|
||||||
ALGORITHM: str = "HS256"
|
ALGORITHM: str = "RS256"
|
||||||
|
JWT_PRIVATE_KEY: str = "" # PEM, required; set via env var
|
||||||
|
JWT_PUBLIC_KEY: str = "" # PEM, required; set via env var
|
||||||
ACCESS_TOKEN_EXPIRE_MINUTES: int = 60 * 8 # 8 hours — no permanent sessions
|
ACCESS_TOKEN_EXPIRE_MINUTES: int = 60 * 8 # 8 hours — no permanent sessions
|
||||||
|
|
||||||
CORS_ORIGINS: list[str] = ["http://localhost:5173"]
|
CORS_ORIGINS: list[str] = ["http://localhost:5173"]
|
||||||
|
|
||||||
|
@field_validator("JWT_PRIVATE_KEY", "JWT_PUBLIC_KEY", mode="before")
|
||||||
|
@classmethod
|
||||||
|
def expand_newlines(cls, v: str) -> str:
|
||||||
|
"""Allow PEM keys stored on a single line with literal \\n in .env."""
|
||||||
|
return v.replace("\\n", "\n") if isinstance(v, str) else v
|
||||||
|
|
||||||
class Config:
|
class Config:
|
||||||
env_file = ".env"
|
env_file = ".env"
|
||||||
|
|
||||||
|
|||||||
@@ -15,14 +15,19 @@ def verify_password(plain: str, hashed: str) -> bool:
|
|||||||
|
|
||||||
|
|
||||||
def create_access_token(subject: str) -> str:
|
def create_access_token(subject: str) -> str:
|
||||||
expire = datetime.now(timezone.utc) + timedelta(minutes=settings.ACCESS_TOKEN_EXPIRE_MINUTES)
|
now = datetime.now(timezone.utc)
|
||||||
|
expire = now + timedelta(minutes=settings.ACCESS_TOKEN_EXPIRE_MINUTES)
|
||||||
return jwt.encode(
|
return jwt.encode(
|
||||||
{"sub": subject, "exp": expire},
|
{"sub": subject, "exp": expire, "iat": now},
|
||||||
settings.SECRET_KEY,
|
settings.JWT_PRIVATE_KEY,
|
||||||
algorithm=settings.ALGORITHM,
|
algorithm=settings.ALGORITHM,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def decode_access_token(token: str) -> str:
|
def decode_access_token(token: str) -> str:
|
||||||
payload = jwt.decode(token, settings.SECRET_KEY, algorithms=[settings.ALGORITHM])
|
payload = jwt.decode(
|
||||||
|
token,
|
||||||
|
settings.JWT_PUBLIC_KEY,
|
||||||
|
algorithms=[settings.ALGORITHM],
|
||||||
|
)
|
||||||
return payload["sub"]
|
return payload["sub"]
|
||||||
|
|||||||
@@ -0,0 +1,20 @@
|
|||||||
|
# 2026-04-13 — Switch JWT signing to RS256 (4096-bit RSA)
|
||||||
|
|
||||||
|
**Timestamp:** 2026-04-13T05:00:00
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
Replaced symmetric HS256 JWT signing with asymmetric RS256 using a 4096-bit RSA key pair. The private key signs tokens; the public key verifies them. Added `iat` (issued-at) claim to every token and a key-generation helper script.
|
||||||
|
|
||||||
|
## Motivation
|
||||||
|
|
||||||
|
HS256 uses the same secret for signing and verification — if the key leaks, an attacker can forge arbitrary tokens. RS256 with a 4096-bit key eliminates this: the private key never leaves the backend process, and the public key can be distributed safely.
|
||||||
|
|
||||||
|
## Files Added / Modified
|
||||||
|
|
||||||
|
- `scripts/generate_jwt_keys.py` — generates a 4096-bit RSA key pair; outputs single-line PEM values ready to paste into `backend/.env`
|
||||||
|
- `backend/app/core/config.py` — replaced `SECRET_KEY` / `ALGORITHM=HS256` with `JWT_PRIVATE_KEY`, `JWT_PUBLIC_KEY`, `ALGORITHM=RS256`; added `expand_newlines` validator to handle `\n`-escaped PEM in `.env`
|
||||||
|
- `backend/app/core/security.py` — `create_access_token` now signs with `JWT_PRIVATE_KEY` and includes `iat` claim; `decode_access_token` verifies with `JWT_PUBLIC_KEY` and pins `algorithms=["RS256"]`
|
||||||
|
- `.env.example` — removed `SECRET_KEY`, added `JWT_PRIVATE_KEY` and `JWT_PUBLIC_KEY` placeholders with generation instructions
|
||||||
|
- `.claude/agents/security-auditor.md` — updated JWT checklist: added checks for wrong algorithm (non-RS256), symmetric key usage, and key-from-env requirement; updated policy note
|
||||||
|
- `TODO.md` — added RS256 item to Auth/session security section (checked off)
|
||||||
@@ -0,0 +1,45 @@
|
|||||||
|
#!/usr/bin/env python3
|
||||||
|
"""
|
||||||
|
Generate a 4096-bit RSA key pair for JWT RS256 signing.
|
||||||
|
|
||||||
|
Usage (from repo root):
|
||||||
|
python scripts/generate_jwt_keys.py
|
||||||
|
|
||||||
|
Output: prints the two env var assignments to paste into backend/.env
|
||||||
|
"""
|
||||||
|
|
||||||
|
from cryptography.hazmat.primitives import serialization
|
||||||
|
from cryptography.hazmat.primitives.asymmetric import rsa
|
||||||
|
from cryptography.hazmat.backends import default_backend
|
||||||
|
|
||||||
|
|
||||||
|
def main() -> None:
|
||||||
|
private_key = rsa.generate_private_key(
|
||||||
|
public_exponent=65537,
|
||||||
|
key_size=4096,
|
||||||
|
backend=default_backend(),
|
||||||
|
)
|
||||||
|
|
||||||
|
private_pem = private_key.private_bytes(
|
||||||
|
encoding=serialization.Encoding.PEM,
|
||||||
|
format=serialization.PrivateFormat.TraditionalOpenSSL,
|
||||||
|
encryption_algorithm=serialization.NoEncryption(),
|
||||||
|
).decode()
|
||||||
|
|
||||||
|
public_pem = private_key.public_key().public_bytes(
|
||||||
|
encoding=serialization.Encoding.PEM,
|
||||||
|
format=serialization.PublicFormat.SubjectPublicKeyInfo,
|
||||||
|
).decode()
|
||||||
|
|
||||||
|
# Collapse newlines to \n so the value fits on one .env line
|
||||||
|
private_oneline = private_pem.replace("\n", "\\n")
|
||||||
|
public_oneline = public_pem.replace("\n", "\\n")
|
||||||
|
|
||||||
|
print("# Paste these lines into backend/.env\n")
|
||||||
|
print(f'JWT_PRIVATE_KEY="{private_oneline}"')
|
||||||
|
print(f'JWT_PUBLIC_KEY="{public_oneline}"')
|
||||||
|
print("\n# Keep JWT_PRIVATE_KEY secret — never commit it.")
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
main()
|
||||||
Reference in New Issue
Block a user