fix(security): lock down signing-key file permissions
Private JWK files were written under the default umask (observed 0664 — group and world readable). Create the key directory 0700, chmod private key files (private_jwks.json, token_jwks.json) to 0600 after they are written, and refuse to start if a pre-existing private key is group/world accessible. Tests now use an isolated per-test key directory. Refs: porchlight-91i Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
cba63280fb
commit
c7550cbf09
4 changed files with 67 additions and 6 deletions
|
|
@ -1,5 +1,6 @@
|
||||||
"""idpyoidc Server initialization."""
|
"""idpyoidc Server initialization."""
|
||||||
|
|
||||||
|
import os
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from idpyoidc.server import Server
|
from idpyoidc.server import Server
|
||||||
|
|
@ -144,8 +145,32 @@ def _build_server_config(settings: Settings) -> dict:
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
_PRIVATE_KEY_FILES = ("private_jwks.json", "token_jwks.json")
|
||||||
|
|
||||||
|
|
||||||
def create_oidc_server(settings: Settings) -> Server:
|
def create_oidc_server(settings: Settings) -> Server:
|
||||||
"""Create and configure an idpyoidc Server instance."""
|
"""Create and configure an idpyoidc Server instance.
|
||||||
|
|
||||||
|
Private signing keys are written to ``signing_key_path``; lock the directory
|
||||||
|
to 0700 and the private key files to 0600, and refuse to start if a
|
||||||
|
pre-existing private key is group/world accessible (a key disclosure).
|
||||||
|
"""
|
||||||
|
key_path = Path(settings.signing_key_path)
|
||||||
|
key_path.mkdir(parents=True, exist_ok=True)
|
||||||
|
os.chmod(key_path, 0o700)
|
||||||
|
|
||||||
|
# Fail on pre-existing keys with loose permissions (left by a prior run).
|
||||||
|
for name in _PRIVATE_KEY_FILES:
|
||||||
|
f = key_path / name
|
||||||
|
if f.exists() and (f.stat().st_mode & 0o077):
|
||||||
|
raise RuntimeError(f"Insecure permission on {f}: private key is group/world accessible")
|
||||||
|
|
||||||
config = _build_server_config(settings)
|
config = _build_server_config(settings)
|
||||||
server = Server(conf=config)
|
server = Server(conf=config)
|
||||||
|
|
||||||
|
# Lock down any keys idpyoidc just wrote (umask may have left them 0644).
|
||||||
|
for name in _PRIVATE_KEY_FILES:
|
||||||
|
f = key_path / name
|
||||||
|
if f.exists():
|
||||||
|
os.chmod(f, 0o600)
|
||||||
return server
|
return server
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,6 @@
|
||||||
import re
|
import re
|
||||||
from collections.abc import AsyncIterator
|
from collections.abc import AsyncIterator
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from httpx import ASGITransport, AsyncClient
|
from httpx import ASGITransport, AsyncClient
|
||||||
|
|
@ -10,8 +11,13 @@ from porchlight.rate_limit import limiter
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def settings() -> Settings:
|
def settings(tmp_path: Path) -> Settings:
|
||||||
return Settings(issuer="http://localhost:8000", sqlite_path=":memory:", session_https_only=False)
|
return Settings(
|
||||||
|
issuer="http://localhost:8000",
|
||||||
|
sqlite_path=":memory:",
|
||||||
|
session_https_only=False,
|
||||||
|
signing_key_path=str(tmp_path / "keys"),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
|
|
|
||||||
|
|
@ -19,7 +19,7 @@ scope = ["openid", "profile"]
|
||||||
toml_file = tmp_path / "test.toml"
|
toml_file = tmp_path / "test.toml"
|
||||||
toml_file.write_text(toml_content)
|
toml_file.write_text(toml_content)
|
||||||
|
|
||||||
settings = Settings(_toml_file=str(toml_file), session_secret="x" * 32)
|
settings = Settings(_toml_file=str(toml_file), session_secret="x" * 32, signing_key_path=str(tmp_path / "keys"))
|
||||||
app = create_app(settings)
|
app = create_app(settings)
|
||||||
|
|
||||||
async with (
|
async with (
|
||||||
|
|
@ -39,9 +39,11 @@ scope = ["openid", "profile"]
|
||||||
assert cdb_entry["allowed_scopes"] == ["openid", "profile"]
|
assert cdb_entry["allowed_scopes"] == ["openid", "profile"]
|
||||||
|
|
||||||
|
|
||||||
async def test_manage_app_always_registered() -> None:
|
async def test_manage_app_always_registered(tmp_path: Path) -> None:
|
||||||
"""The internal manage-app client is always registered, even without config file clients."""
|
"""The internal manage-app client is always registered, even without config file clients."""
|
||||||
settings = Settings(issuer="https://test.example.com", session_secret="x" * 32)
|
settings = Settings(
|
||||||
|
issuer="https://test.example.com", session_secret="x" * 32, signing_key_path=str(tmp_path / "keys")
|
||||||
|
)
|
||||||
app = create_app(settings)
|
app = create_app(settings)
|
||||||
|
|
||||||
async with (
|
async with (
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,9 @@
|
||||||
|
import os
|
||||||
import shutil
|
import shutil
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
from porchlight.config import Settings
|
from porchlight.config import Settings
|
||||||
from porchlight.oidc.claims import PorchlightUserInfo
|
from porchlight.oidc.claims import PorchlightUserInfo
|
||||||
from porchlight.oidc.provider import create_oidc_server
|
from porchlight.oidc.provider import create_oidc_server
|
||||||
|
|
@ -53,3 +56,28 @@ def test_create_server_userinfo_is_porchlight() -> None:
|
||||||
assert isinstance(server.context.userinfo, PorchlightUserInfo)
|
assert isinstance(server.context.userinfo, PorchlightUserInfo)
|
||||||
finally:
|
finally:
|
||||||
shutil.rmtree(key_path, ignore_errors=True)
|
shutil.rmtree(key_path, ignore_errors=True)
|
||||||
|
|
||||||
|
|
||||||
|
def test_signing_key_files_have_strict_permissions(tmp_path: Path) -> None:
|
||||||
|
key_path = tmp_path / "keys"
|
||||||
|
settings = Settings(issuer="http://localhost:8000", sqlite_path=":memory:", signing_key_path=str(key_path))
|
||||||
|
create_oidc_server(settings)
|
||||||
|
|
||||||
|
assert (key_path.stat().st_mode & 0o077) == 0, "key directory must not be group/world accessible"
|
||||||
|
for name in ("private_jwks.json", "token_jwks.json"):
|
||||||
|
f = key_path / name
|
||||||
|
assert f.exists()
|
||||||
|
assert (f.stat().st_mode & 0o077) == 0, f"{name} must be 0600"
|
||||||
|
|
||||||
|
|
||||||
|
def test_startup_fails_on_world_readable_private_key(tmp_path: Path) -> None:
|
||||||
|
key_path = tmp_path / "keys"
|
||||||
|
key_path.mkdir()
|
||||||
|
# Simulate a pre-existing private key left group/world readable.
|
||||||
|
leaked = key_path / "private_jwks.json"
|
||||||
|
leaked.write_text("{}")
|
||||||
|
os.chmod(leaked, 0o644)
|
||||||
|
|
||||||
|
settings = Settings(issuer="http://localhost:8000", sqlite_path=":memory:", signing_key_path=str(key_path))
|
||||||
|
with pytest.raises(RuntimeError, match="permission"):
|
||||||
|
create_oidc_server(settings)
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue