From bbfe82cbe6257320aab5116c6061f8310cd2dac2 Mon Sep 17 00:00:00 2001 From: Sam <78538841+spwoodcock@users.noreply.github.com> Date: Wed, 3 Jul 2024 17:40:52 +0200 Subject: [PATCH] refactor: replace RS256 jwt signing with simpler HS384 (#1622) --- .env.example | 2 -- chart/README.md | 6 +----- docs/dev/Troubleshooting.md | 7 +------ scripts/gen-env.sh | 23 ----------------------- src/backend/app/auth/osm.py | 19 +++++++++++-------- src/backend/app/config.py | 20 ++++++++++++-------- src/frontend/src/App.tsx | 2 +- 7 files changed, 26 insertions(+), 53 deletions(-) diff --git a/.env.example b/.env.example index 4bc5c33e91..5e086c1c22 100644 --- a/.env.example +++ b/.env.example @@ -11,8 +11,6 @@ ENCRYPTION_KEY=${ENCRYPTION_KEY:-"pIxxYIXe4oAVHI36lTveyc97FKK2O_l2VHeiuqU-K_4="} FMTM_DOMAIN=${FMTM_DOMAIN:-"fmtm.localhost"} FMTM_DEV_PORT=${FMTM_DEV_PORT:-7050} CERT_EMAIL=${CERT_EMAIL} -AUTH_PUBLIC_KEY=${AUTH_PUBLIC_KEY} -AUTH_PRIVATE_KEY=${AUTH_PRIVATE_KEY} # Use API_PREFIX if running behind a proxy subpath (e.g. /api) API_PREFIX=${API_PREFIX:-/} diff --git a/chart/README.md b/chart/README.md index a4827300f7..1615a50f8d 100644 --- a/chart/README.md +++ b/chart/README.md @@ -50,8 +50,6 @@ kubectl - key: OSM_CLIENT_ID - key: OSM_CLIENT_SECRET - key: OSM_SECRET_KEY - - key: AUTH_PUBLIC_KEY - - key: AUTH_PRIVATE_KEY ```bash kubectl create secret generic api-fmtm-vars --namespace fmtm \ @@ -59,9 +57,7 @@ kubectl --from-literal=FMTM_DOMAIN=some.domain.com \ --from-literal=OSM_CLIENT_ID=xxxxxxx \ --from-literal=OSM_CLIENT_SECRET=xxxxxxx \ - --from-literal=OSM_SECRET_KEY=xxxxxxx \ - --from-file=AUTH_PUBLIC_KEY=/path/to/pub/key.pem \ - --from-file=AUTH_PRIVATE_KEY=/path/to/priv/key.pem + --from-literal=OSM_SECRET_KEY=xxxxxxx ``` ## Deployment diff --git a/docs/dev/Troubleshooting.md b/docs/dev/Troubleshooting.md index 0c9810ee8f..f20078be63 100644 --- a/docs/dev/Troubleshooting.md +++ b/docs/dev/Troubleshooting.md @@ -34,10 +34,6 @@ OSM_SCOPE field required (type=value_error.missing) OSM_LOGIN_REDIRECT_URI field required (type=value_error.missing) -AUTH_PUBLIC_KEY - field required (type=value_error.missing) -AUTH_PRIVATE_KEY - field required (type=value_error.missing) ``` Then you need to set the env variables on your system. @@ -48,7 +44,6 @@ an alternative can be to feed them into the pdm command: ```bash FMTM_DOMAIN="" \ OSM_CLIENT_ID="" OSM_CLIENT_SECRET="" OSM_SECRET_KEY="" \ -S3_ACCESS_KEY="" S3_SECRET_KEY="" \ -AUTH_PUBLIC_KEY="" AUTH_PRIVATE_KEY="" \ +S3_ACCESS_KEY="" S3_SECRET_KEY="" ENCRYPTION_KEY="" \ pdm run uvicorn app.main:api --host 0.0.0.0 --port 8000 ``` diff --git a/scripts/gen-env.sh b/scripts/gen-env.sh index 1ee52c7468..1fbc35b874 100644 --- a/scripts/gen-env.sh +++ b/scripts/gen-env.sh @@ -342,28 +342,6 @@ check_change_port() { fi } -generate_auth_keys() { - pretty_echo "Generating Auth Keys" - - if ! AUTH_PRIVATE_KEY=$(openssl genrsa 4096 2>/dev/null); then - echo "Error generating private key. Aborting." - return 1 - fi - - if ! AUTH_PUBLIC_KEY=$(echo "$AUTH_PRIVATE_KEY" | openssl rsa -pubout 2>/dev/null); then - echo "Error generating public key. Aborting." - return 1 - fi - - # Quotes are required around key variables, else dotenv does not load - export AUTH_PRIVATE_KEY="\"$AUTH_PRIVATE_KEY\"" - export AUTH_PUBLIC_KEY="\"$AUTH_PUBLIC_KEY\"" - - echo - echo "Auth keys generated." - echo -} - generate_dotenv() { pretty_echo "Generating Dotenv File" @@ -412,7 +390,6 @@ prompt_user_gen_dotenv() { fi set_osm_credentials - generate_auth_keys generate_dotenv pretty_echo "Completed dotenv file generation" diff --git a/src/backend/app/auth/osm.py b/src/backend/app/auth/osm.py index e3a954c30b..f36e86132f 100644 --- a/src/backend/app/auth/osm.py +++ b/src/backend/app/auth/osm.py @@ -101,11 +101,14 @@ def create_tokens(payload: dict) -> tuple[str, str]: access_token_payload["exp"] = ( int(time.time()) + 86400 ) # set access token expiry to 1 day - private_key = settings.AUTH_PRIVATE_KEY access_token = jwt.encode( - access_token_payload, str(private_key), algorithm=settings.ALGORITHM + access_token_payload, + settings.ENCRYPTION_KEY, + algorithm=settings.JWT_ENCRYPTION_ALGORITHM, + ) + refresh_token = jwt.encode( + payload, settings.ENCRYPTION_KEY, algorithm=settings.JWT_ENCRYPTION_ALGORITHM ) - refresh_token = jwt.encode(payload, str(private_key), algorithm=settings.ALGORITHM) return access_token, refresh_token @@ -116,9 +119,10 @@ def refresh_access_token(payload: dict) -> str: int(time.time()) + 60 ) # Access token valid for 15 minutes - private_key = settings.AUTH_PRIVATE_KEY return jwt.encode( - access_token_payload, str(private_key), algorithm=settings.ALGORITHM + access_token_payload, + settings.ENCRYPTION_KEY, + algorithm=settings.JWT_ENCRYPTION_ALGORITHM, ) @@ -134,12 +138,11 @@ def verify_token(token: str): Raises: HTTPException: If the token has expired or credentials could not be validated. """ - public_key = settings.AUTH_PUBLIC_KEY try: return jwt.decode( token, - str(public_key), - algorithms=[settings.ALGORITHM], + settings.ENCRYPTION_KEY, + algorithms=[settings.JWT_ENCRYPTION_ALGORITHM], audience=settings.FMTM_DOMAIN, ) except jwt.ExpiredSignatureError as e: diff --git a/src/backend/app/config.py b/src/backend/app/config.py index 11ce2fe76b..c0aab93c0b 100644 --- a/src/backend/app/config.py +++ b/src/backend/app/config.py @@ -150,6 +150,9 @@ class Settings(BaseSettings): DEBUG: bool = False LOG_LEVEL: str = "INFO" ENCRYPTION_KEY: str + # NOTE HS384 is used for simplicity of implementation and compatibility with + # existing Fernet based database value encryption + JWT_ENCRYPTION_ALGORITHM: str = "HS384" FMTM_DOMAIN: str FMTM_DEV_PORT: Optional[str] = "7050" @@ -279,10 +282,6 @@ def set_raw_data_api_auth_none(cls, v: Optional[str]) -> Optional[str]: return None return v - ALGORITHM: str = "RS256" - AUTH_PRIVATE_KEY: str - AUTH_PUBLIC_KEY: str - MONITORING: Optional[MonitoringTypes] = None @computed_field @@ -302,16 +301,21 @@ def get_settings(): _settings = Settings() if _settings.DEBUG: - print( - "Loaded settings: " - f"{_settings.model_dump(exclude=['AUTH_PRIVATE_KEY', 'AUTH_PUBLIC_KEY'])}" - ) + print("Loaded settings: " f"{_settings.model_dump()}") return _settings @lru_cache def get_cipher_suite(): """Cache cypher suite.""" + # Fernet is used by cryptography as a simple and effective default + # it enforces a 32 char secret. + # + # In the future we could migrate this to HS384 encryption, which we also + # use for our JWT signing. Ideally this needs 48 characters, but for now + # we are stuck at 32 char to maintain support with Fernet (reuse the same key). + # + # However this would require a migration for all existing instances of FMTM. return Fernet(settings.ENCRYPTION_KEY) diff --git a/src/frontend/src/App.tsx b/src/frontend/src/App.tsx index 083e51608c..650ff2b8da 100755 --- a/src/frontend/src/App.tsx +++ b/src/frontend/src/App.tsx @@ -43,7 +43,7 @@ const CheckLoginState = () => { useEffect(() => { // Check current login state (omit callback url) if (!window.location.pathname.includes('osmauth')) { - // No need for introspect check if user details are not set + // No need for token refresh check if user details are not set if (!authDetails) return; checkIfUserLoginValid(); }