Skip to content
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
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,20 @@ the HTTP layer conform to the RFCs it implements β€” caching, conditional and
range requests, interim responses, stream errors on HTTP/2 and HTTP/3 β€” and
makes startup, reload and shutdown fail closed and drop no request.

### πŸ›‘οΈ Underscore-named request fields can be allowlisted

A request field containing an underscore was dropped before routing on every
transport, because a CGI or FastCGI backend folds `x_probe` and `x-probe` onto
the same variable β€” an injection path onto the identity headers `forward_auth
copy_headers` is supposed to own. Caddy's `expected_underscore_headers` server
option is now implemented: names listed in `servers { … }` survive on every
listener, a `servers <address> { … }` block overrides that list for one
listener, and a trailing `*` matches a prefix. The hyphenated spelling of an
allowlisted name is dropped so the alias cannot reach the same variable by the
other route, and a repeated allowlisted field drops every occurrence rather
than the extra one. Without the option the default is unchanged: nothing with
an underscore (#295).

### πŸͺ΅ Newly declared access-log channels work on reload

Reload registers named access-log channels before building site routes, including
Expand Down
1 change: 1 addition & 0 deletions pingclair-config/src/adapter/caddyfile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ mod route_order;
mod scoped_middleware;
mod sites;
mod tls;
mod underscore_headers;

#[cfg(test)]
mod block_matcher_tests;
Expand Down
9 changes: 8 additions & 1 deletion pingclair-config/src/adapter/caddyfile/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,9 @@ pub(super) fn adapt_global(d: Directive) -> Result<GlobalBlock, AdapterError> {
"client_ip_headers" => {
global.client_ip_headers = parse_client_ip_headers(&sub.args)?;
}
"expected_underscore_headers" => {
global.expected_underscore_headers = super::underscore_headers::parse(&sub)?;
}
// πŸ“‘ `dns <provider> [args…]` names the provider used both for
// DNS-01 challenges and, upstream, for general resolution. We
// only have the first meaning, which is why `acme_dns` below
Expand Down Expand Up @@ -666,6 +669,10 @@ pub(super) fn expand_servers_block(
"client_ip_headers" => {
options.client_ip_headers = Some(parse_client_ip_headers(&child.args)?);
}
"expected_underscore_headers" => {
options.expected_underscore_headers =
Some(super::underscore_headers::parse(child)?);
}
// 🚫 Everything else has no meaning for one listener β€” `admin`,
// `email`, `pki`, `storage` are process-wide β€” or is a setting
// this build does not implement at all. Applying it to every
Expand All @@ -678,7 +685,7 @@ pub(super) fn expand_servers_block(
format!("global: servers {address} {{ {other} }}"),
format!(
"only `listener_wrappers`, `protocols`, `trusted_proxies`, \
`client_ip_headers` and `metrics` can be set for one listener; `{other}` is process-wide, \
`client_ip_headers`, `expected_underscore_headers` and `metrics` can be set for one listener; `{other}` is process-wide, \
so write it without an address (`servers {{ {other} … }}`) to apply \
it to every listener"
),
Expand Down
1 change: 1 addition & 0 deletions pingclair-config/src/adapter/caddyfile/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ pub(super) static GLOBAL_OPTIONS: &[Spec] = &[
implemented("dns"),
implemented("dns_refresh"),
implemented("email"),
implemented("expected_underscore_headers"),
implemented("grace_period"),
implemented("http_port"),
implemented("https_port"),
Expand Down
16 changes: 16 additions & 0 deletions pingclair-config/src/adapter/caddyfile/underscore_headers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// SPDX-License-Identifier: Apache-2.0
// Copyright 2026 Dorian Verlaine

//! πŸ›‘οΈ The server allowlist takes names, never a boolean or a nested block.

use super::*;

pub(super) fn parse(directive: &Directive) -> Result<Vec<String>, AdapterError> {
if directive.args.is_empty() || directive.block.is_some() {
return Err(AdapterError::InvalidArgument(
"expected_underscore_headers".into(),
"expected one or more header names or trailing-star prefixes, without a block".into(),
));
}
Ok(directive.args.clone())
}
4 changes: 4 additions & 0 deletions pingclair-config/src/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ fn compile_global(global: &GlobalBlock, config: &mut PingclairConfig) -> Compile

config.global.trusted_proxies = global.trusted_proxies.clone();
config.global.client_ip_headers = global.client_ip_headers.clone();
config.global.expected_underscore_headers = global.expected_underscore_headers.clone();
config.global.storage_path = global.storage_path.clone();

// 🚰 Left as `None` when unset, which is not "unconfigured" but the actual
Expand Down Expand Up @@ -409,11 +410,13 @@ fn compile_global(global: &GlobalBlock, config: &mut PingclairConfig) -> Compile
|| options.http3.is_some()
|| options.trusted_proxies.is_some()
|| options.client_ip_headers.is_some()
|| options.expected_underscore_headers.is_some()
})
.map(|(address, options)| {
(
address.clone(),
pingclair_core::config::ListenerOptions {
expected_underscore_headers: options.expected_underscore_headers.clone(),
proxy_protocol: options.proxy_protocol,
http3: options.http3,
trusted_proxies: options.trusted_proxies.clone(),
Expand Down Expand Up @@ -1019,6 +1022,7 @@ fn validate_cache_ceiling_agrees(config: &PingclairConfig) -> CompileResult<()>
}

pub fn validate_config(config: &PingclairConfig) -> CompileResult<()> {
crate::underscore_headers::validate(config)?;
for server in &config.servers {
pingclair_core::server::Router::validate_paths(&server.routes).map_err(|error| {
CompileError::InvalidRoute {
Expand Down
55 changes: 54 additions & 1 deletion pingclair-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ mod header_fields;
pub mod parser;
mod retired_placeholders;
mod shared_ports;
mod underscore_headers;
mod upstream_weights;

pub use parser::{
Expand Down Expand Up @@ -252,6 +253,7 @@ fn merge_globals(
blocked_ips,
trusted_proxies,
client_ip_headers,
expected_underscore_headers,
listener_options,
upstream_keepalive_pool_size,
http3,
Expand Down Expand Up @@ -357,6 +359,19 @@ fn merge_globals(
}
into.client_ip_headers = client_ip_headers;
}
if !expected_underscore_headers.is_empty() {
if !into.expected_underscore_headers.is_empty()
&& into.expected_underscore_headers != expected_underscore_headers
{
return Err(FullCompileError::Compile(pingclair_config_compile_error(
format!(
"`expected_underscore_headers` is set differently in more than one file (seen again in {})",
path.display()
),
)));
}
into.expected_underscore_headers = expected_underscore_headers;
}

// 🧭 One listener's options per address, accumulated like the channels in
// `merge_logging`. An address declared in two files is refused for the same
Expand Down Expand Up @@ -2576,7 +2591,8 @@ mod directory_merge_tests {
"00-globals.pingclair",
"{\n http_port 8080\n https_port 8443\n metrics\n\
\x20 trusted_proxies 10.0.0.0/8\n dns_refresh 90s\n\
\x20 servers {\n protocols h1 h2\n }\n}\n",
\x20 servers {\n protocols h1 h2\n\
\x20 expected_underscore_headers X_Probe\n }\n}\n",
);
let site = write(
dir.path(),
Expand All @@ -2601,6 +2617,43 @@ mod directory_merge_tests {
!merged.global.http3,
"`protocols h1 h2` was dropped, so HTTP/3 stayed on"
);
assert_eq!(
merged.global.expected_underscore_headers,
vec!["X_Probe".to_string()],
"expected_underscore_headers was dropped"
);
}

/// πŸ›‘οΈ Two files disagreeing about the underscore allowlist are refused,
/// the way `client_ip_headers` already is: a silent last-one-wins would
/// drop the first file's names without saying so.
#[test]
fn two_files_disagreeing_about_the_underscore_allowlist_are_refused() {
let dir = tempfile::tempdir().expect("tempdir");
let first = write(
dir.path(),
"00-allowlist.pingclair",
"{\n servers {\n expected_underscore_headers X_Probe\n }\n}\n",
);
let second = write(
dir.path(),
"10-allowlist.pingclair",
"{\n servers {\n expected_underscore_headers X_Other\n }\n}\n",
);
let site = write(
dir.path(),
"20-site.pingclair",
"http://:9000 {\n respond \"ok\"\n}\n",
);

let error = compile_multiple_files(&[first, second, site])
.expect_err("two allowlists must not merge silently");
assert!(
error
.to_string()
.contains("expected_underscore_headers` is set differently"),
"unexpected error: {error}"
);
}

/// πŸͺ΅ A log channel declared in one file must still exist after another
Expand Down
4 changes: 4 additions & 0 deletions pingclair-config/src/parser/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ pub struct GlobalBlock {
pub trusted_proxies: Vec<String>,
/// πŸ›‘οΈ Headers a trusted proxy may name the client in (`client_ip_headers`).
pub client_ip_headers: Vec<String>,
/// πŸ›‘οΈ Underscore-named request fields preserved on every listener by default.
pub expected_underscore_headers: Vec<String>,
/// πŸ”„ Upstream re-resolution interval in seconds; `Some(0)` disables it.
/// `None` means the directive was absent and the default applies.
pub dns_refresh_secs: Option<u64>,
Expand Down Expand Up @@ -136,6 +138,8 @@ pub struct GlobalBlock {
/// than applying it to all of them.
#[derive(Debug, Clone, Default)]
pub struct ListenerOptions {
/// πŸ›‘οΈ Underscore-named request fields preserved on this listener.
pub expected_underscore_headers: Option<Vec<String>>,
/// πŸ”Œ Whether this listener requires a PROXY protocol header before HTTP.
pub proxy_protocol: Option<bool>,
/// 🌐 Whether this listener offers HTTP/3.
Expand Down
108 changes: 108 additions & 0 deletions pingclair-config/src/underscore_headers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// SPDX-License-Identifier: Apache-2.0
// Copyright 2026 Dorian Verlaine

//! πŸ›‘οΈ Validate the allowlist on every configuration path, including Admin JSON.

use crate::compiler::{CompileError, CompileResult};
use pingclair_core::config::PingclairConfig;

pub(crate) fn validate(config: &PingclairConfig) -> CompileResult<()> {
let lists = std::iter::once(&config.global.expected_underscore_headers).chain(
config
.global
.listener_options
.values()
.filter_map(|options| options.expected_underscore_headers.as_ref()),
);
for entries in lists {
for entry in entries {
let name = entry.strip_suffix('*').unwrap_or(entry);
if !name.contains('_')
|| name.contains('*')
|| http::HeaderName::from_bytes(name.as_bytes()).is_err()
{
return Err(CompileError::InvalidServer {
message: format!(
"expected_underscore_headers entry `{entry}` must be an ASCII header name containing `_`, with at most one trailing `*`"
),
});
}
}
}
Ok(())
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn expected_underscore_headers_round_trip_with_listener_override() {
let config = crate::compile(
r#"
{
servers {
expected_underscore_headers X_Probe Webhook_*
}
servers :8081 {
expected_underscore_headers Other_Field
}
}
http://:8080, http://:8081 {
respond ok
}
"#,
)
.unwrap();
assert_eq!(
config.global.expected_underscore_headers,
["X_Probe", "Webhook_*"]
);
assert_eq!(
config.global.listener_options[":8081"].expected_underscore_headers,
Some(vec!["Other_Field".to_owned()])
);
let decoded: PingclairConfig =
serde_json::from_value(serde_json::to_value(&config).unwrap()).unwrap();
assert_eq!(
serde_json::to_value(&decoded).unwrap(),
serde_json::to_value(&config).unwrap()
);
crate::compiler::validate_config(&decoded).unwrap();
}

#[test]
fn expected_underscore_headers_reject_invalid_entries_on_all_paths() {
for entry in ["x-probe", "*", "x_*_bad", "x_Γ©", "x_ bad", "x_:bad"] {
let mut config = PingclairConfig::default();
config.global.expected_underscore_headers = vec![entry.to_owned()];
assert!(validate(&config).is_err(), "global: {entry}");
config.global.expected_underscore_headers.clear();
config.global.listener_options.insert(
":8080".into(),
pingclair_core::config::ListenerOptions {
expected_underscore_headers: Some(vec![entry.to_owned()]),
..Default::default()
},
);
assert!(
crate::compiler::validate_config(&config)
.unwrap_err()
.to_string()
.contains("expected_underscore_headers")
);
}
for directive in [
"expected_underscore_headers",
"expected_underscore_headers X_Probe { nested }",
"expected_underscore_headers true",
] {
assert!(
crate::compile(&format!(
"{{\n servers {{\n {directive}\n }}\n}}\nhttp://:8080 {{\n respond ok\n }}"
))
.is_err()
);
}
}
}
8 changes: 8 additions & 0 deletions pingclair-core/src/config/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ pub struct PingclairConfig {
/// in the first case and is overridden in the second.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)]
pub struct ListenerOptions {
/// πŸ›‘οΈ Underscore-named request fields this listener preserves.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub expected_underscore_headers: Option<Vec<String>>,
/// πŸ”Œ Whether this listener requires a PROXY protocol header before HTTP.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub proxy_protocol: Option<bool>,
Expand Down Expand Up @@ -221,6 +224,10 @@ pub struct GlobalConfig {
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub client_ip_headers: Vec<String>,

/// πŸ›‘οΈ Exact names or trailing-star prefixes allowed through the underscore guard.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub expected_underscore_headers: Vec<String>,

/// 🧭 Options an addressed `servers <address> { … }` block set, keyed by
/// the address the operator wrote.
///
Expand Down Expand Up @@ -485,6 +492,7 @@ impl Default for GlobalConfig {
blocked_ips: Vec::new(),
trusted_proxies: Vec::new(),
client_ip_headers: Vec::new(),
expected_underscore_headers: Vec::new(),
listener_options: BTreeMap::new(),
upstream_keepalive_pool_size: None,
http3: true,
Expand Down
Loading
Loading