mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-11 22:53:00 +00:00
fix(server): convert auth resolver panics to fail-closed errors
Completes the R52/R53 fail-closed posture from d4fb73d61. The jwt and
mtls strategy branches were still using panic!/expect/assert! when
their required material was missing or malformed, which would crash
the server binary instead of returning a clean startup error.
- decode_pem_env: return anyhow::Result<String> instead of panicking
on invalid base64 or invalid UTF-8.
- resolve_auth_mode_with_lookup: convert the missing-FABRO_JWT_PUBLIC_KEY,
invalid-PEM, and missing-[server.listen.tls]-for-mtls cases from
panics to anyhow::Err returns prefixed with "Fabro server refuses
to start".
- Update the resolve_auth_mode doc to drop the "Panics if..." caveat.
- Add three fail-closed tests covering each new error path.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
f79ca80591
commit
09b616cb12
1 changed files with 74 additions and 19 deletions
|
|
@ -69,14 +69,13 @@ pub enum AuthMode {
|
|||
pub struct PeerCertificates(pub Option<Vec<CertificateDer<'static>>>);
|
||||
|
||||
/// Decode a PEM env var that may be raw PEM or base64-encoded PEM.
|
||||
pub fn decode_pem_env(name: &str, value: &str) -> String {
|
||||
pub fn decode_pem_env(name: &str, value: &str) -> Result<String> {
|
||||
if value.starts_with("-----") {
|
||||
return value.to_string();
|
||||
return Ok(value.to_string());
|
||||
}
|
||||
let bytes = base64::Engine::decode(&BASE64_STANDARD, value)
|
||||
.unwrap_or_else(|e| panic!("{name} is not valid PEM or base64: {e}"));
|
||||
String::from_utf8(bytes)
|
||||
.unwrap_or_else(|e| panic!("{name} base64 decoded to invalid UTF-8: {e}"))
|
||||
.map_err(|e| anyhow!("{name} is not valid PEM or base64: {e}"))?;
|
||||
String::from_utf8(bytes).map_err(|e| anyhow!("{name} base64 decoded to invalid UTF-8: {e}"))
|
||||
}
|
||||
|
||||
/// Resolve the authentication mode from a [`SettingsFile`].
|
||||
|
|
@ -87,9 +86,9 @@ pub fn decode_pem_env(name: &str, value: &str) -> String {
|
|||
/// when `server.auth` resolves to at least one enabled strategy.
|
||||
///
|
||||
/// Fails closed when `server.auth` is absent or resolves to zero enabled
|
||||
/// strategies: startup refuses rather than silently accepting every
|
||||
/// request. Panics if a configured strategy is missing its required
|
||||
/// material (JWT public key, mTLS TLS config).
|
||||
/// strategies, or when a configured strategy is missing its required
|
||||
/// material (JWT public key, mTLS TLS config): startup refuses rather
|
||||
/// than silently accepting every request or panicking the binary.
|
||||
///
|
||||
/// Walks the v2 `server.auth.api.{jwt,mtls}` subtree and
|
||||
/// `server.auth.web.allowed_usernames`.
|
||||
|
|
@ -159,15 +158,20 @@ where
|
|||
}
|
||||
|
||||
if jwt_enabled {
|
||||
let raw = lookup("FABRO_JWT_PUBLIC_KEY").unwrap_or_else(|| {
|
||||
panic!(
|
||||
"FABRO_JWT_PUBLIC_KEY is not set. Provide an Ed25519 public key in PEM format \
|
||||
let raw = lookup("FABRO_JWT_PUBLIC_KEY").ok_or_else(|| {
|
||||
anyhow!(
|
||||
"Fabro server refuses to start: [server.auth.api.jwt] is enabled but \
|
||||
FABRO_JWT_PUBLIC_KEY is not set. Provide an Ed25519 public key in PEM format \
|
||||
(or base64-encoded PEM) for JWT authentication."
|
||||
)
|
||||
});
|
||||
let pem = decode_pem_env("FABRO_JWT_PUBLIC_KEY", &raw);
|
||||
let key = DecodingKey::from_ed_pem(pem.as_bytes())
|
||||
.expect("FABRO_JWT_PUBLIC_KEY contains an invalid Ed25519 PEM public key");
|
||||
})?;
|
||||
let pem = decode_pem_env("FABRO_JWT_PUBLIC_KEY", &raw)?;
|
||||
let key = DecodingKey::from_ed_pem(pem.as_bytes()).map_err(|e| {
|
||||
anyhow!(
|
||||
"Fabro server refuses to start: FABRO_JWT_PUBLIC_KEY contains an invalid \
|
||||
Ed25519 PEM public key: {e}"
|
||||
)
|
||||
})?;
|
||||
strategies.push(AuthStrategy::Jwt {
|
||||
key: Arc::new(key),
|
||||
validation: Arc::new(jwt_validation()),
|
||||
|
|
@ -176,10 +180,12 @@ where
|
|||
}
|
||||
|
||||
if mtls_enabled {
|
||||
assert!(
|
||||
tls_present,
|
||||
"mTLS authentication strategy requires [server.listen.tls] configuration with cert, key, and ca"
|
||||
);
|
||||
if !tls_present {
|
||||
return Err(anyhow!(
|
||||
"Fabro server refuses to start: [server.auth.api.mtls] is enabled but \
|
||||
[server.listen.tls] is missing required cert, key, or ca paths."
|
||||
));
|
||||
}
|
||||
strategies.push(AuthStrategy::Mtls);
|
||||
}
|
||||
|
||||
|
|
@ -550,6 +556,55 @@ ca = "/etc/fabro/tls/ca.pem"
|
|||
assert!(strategies.iter().any(|s| matches!(s, AuthStrategy::Mtls)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fail_closed_when_jwt_enabled_without_public_key_env() {
|
||||
let file = settings(
|
||||
r"
|
||||
_version = 1
|
||||
|
||||
[server.auth.api.jwt]
|
||||
enabled = true
|
||||
",
|
||||
);
|
||||
let err = resolve_auth_mode_with_lookup(&file, empty_lookup)
|
||||
.expect_err("missing FABRO_JWT_PUBLIC_KEY should refuse startup");
|
||||
assert!(err.to_string().contains("FABRO_JWT_PUBLIC_KEY"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fail_closed_when_jwt_public_key_is_invalid_pem() {
|
||||
let file = settings(
|
||||
r"
|
||||
_version = 1
|
||||
|
||||
[server.auth.api.jwt]
|
||||
enabled = true
|
||||
",
|
||||
);
|
||||
let err = resolve_auth_mode_with_lookup(&file, |name| {
|
||||
(name == "FABRO_JWT_PUBLIC_KEY").then(|| {
|
||||
"-----BEGIN PUBLIC KEY-----\ngarbage\n-----END PUBLIC KEY-----".to_string()
|
||||
})
|
||||
})
|
||||
.expect_err("invalid PEM should refuse startup");
|
||||
assert!(err.to_string().contains("invalid"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fail_closed_when_mtls_enabled_without_listen_tls() {
|
||||
let file = settings(
|
||||
r"
|
||||
_version = 1
|
||||
|
||||
[server.auth.api.mtls]
|
||||
enabled = true
|
||||
",
|
||||
);
|
||||
let err = resolve_auth_mode_with_lookup(&file, empty_lookup)
|
||||
.expect_err("mTLS without [server.listen.tls] should refuse startup");
|
||||
assert!(err.to_string().contains("server.listen.tls"));
|
||||
}
|
||||
|
||||
async fn protected_handler(_auth: AuthenticatedService) -> impl IntoResponse {
|
||||
"ok"
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue