Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace usage of RS256 JWT signing with simpler HS384 #1622

Merged
merged 1 commit into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -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:-/}

Expand Down
6 changes: 1 addition & 5 deletions chart/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,14 @@ 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 \
--from-literal=ENCRYPTION_KEY=xxxxxxx \
--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
Expand Down
7 changes: 1 addition & 6 deletions docs/dev/Troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
```
23 changes: 0 additions & 23 deletions scripts/gen-env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -412,7 +390,6 @@ prompt_user_gen_dotenv() {
fi

set_osm_credentials
generate_auth_keys
generate_dotenv

pretty_echo "Completed dotenv file generation"
Expand Down
19 changes: 11 additions & 8 deletions src/backend/app/auth/osm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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,
)


Expand All @@ -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:
Expand Down
20 changes: 12 additions & 8 deletions src/backend/app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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)


Expand Down
2 changes: 1 addition & 1 deletion src/frontend/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Loading