mirror of
https://github.com/openai/codex.git
synced 2026-11-10 21:00:47 +00:00
Add scoped request-body secret substitution to MITM hooks (#57174)
## What changed - Add `body_secret_substitution` actions to version 2 MITM configuration, replacing exact opaque placeholders with secrets from environment variables or files within the matching hook's scope. - Bound body size, expansion, replacement count, and search work. Reject overlapping matches, compressed payloads, trailers, and supplied payload-integrity headers. - Recompute `Content-Length` and substitute before SigV4 signing so signatures cover the transmitted body. ## Testing Add coverage for markers split across chunks, expansion limits, invalid payloads, overlapping bindings, secret redaction, and search limits. Add a TLS forwarding test that verifies hook scope and independently checks the signature of the substituted body. GitOrigin-RevId: fa95c9ae0c8fc8f695e03c0e87a0a4875d8418fd
This commit is contained in:
@@ -26,6 +26,7 @@ mod request_cancellation;
|
||||
mod request_disconnect;
|
||||
mod responses;
|
||||
mod runtime;
|
||||
mod secret_substitution;
|
||||
mod socket_path;
|
||||
mod socks5;
|
||||
mod state;
|
||||
@@ -126,6 +127,8 @@ pub use runtime::ConfigReloaderFuture;
|
||||
pub use runtime::ConfigState;
|
||||
pub use runtime::LocalBindingPolicy;
|
||||
pub use runtime::NetworkProxyState;
|
||||
pub use secret_substitution::BodySecretReplacement;
|
||||
pub use secret_substitution::BodySecretSubstitutionConfig;
|
||||
pub use state::NetworkProxyAuditMetadata;
|
||||
pub use state::NetworkProxyConstraintError;
|
||||
pub use state::NetworkProxyConstraints;
|
||||
|
||||
@@ -281,6 +281,7 @@ async fn handle_mitm_request(
|
||||
request_ctx: Arc<MitmRequestContext>,
|
||||
) -> Result<Response, std::convert::Infallible> {
|
||||
crate::aws_sigv4::preserve_signing_headers(&mut req);
|
||||
crate::secret_substitution::preserve_body_substitution_headers(&mut req);
|
||||
let response = match (request_ctx.policy.scheme == Scheme::HTTPS)
|
||||
.then(RemoveRequestHeaderLayer::hop_by_hop)
|
||||
.into_layer(service_fn(move |req| {
|
||||
@@ -369,6 +370,15 @@ async fn forward_request(req: Request, request_ctx: &MitmRequestContext) -> Resu
|
||||
};
|
||||
|
||||
let mut upstream_req = Request::from_parts(parts, body);
|
||||
if let Some(substitution) = hook_actions
|
||||
.as_ref()
|
||||
.and_then(|actions| actions.body_secret_substitution.as_ref())
|
||||
{
|
||||
let Ok(substituted) = substitution.apply(upstream_req).await else {
|
||||
return Ok(request_ctx.policy.deny_hook(&method, client).await);
|
||||
};
|
||||
upstream_req = substituted;
|
||||
}
|
||||
if let Some((signer, guest)) = signing {
|
||||
let Ok(signed) = signer.sign(upstream_req, &guest).await else {
|
||||
return Ok(request_ctx.policy.deny_hook(&method, client).await);
|
||||
|
||||
@@ -36,6 +36,15 @@ async fn tls_origin(
|
||||
dir: &Path,
|
||||
service: &str,
|
||||
alpn: &[u8],
|
||||
) -> (MitmRequestContext, TcpListener, TlsAcceptor) {
|
||||
tls_origin_with_hook(dir, service, alpn, |_, _| {}).await
|
||||
}
|
||||
|
||||
async fn tls_origin_with_hook(
|
||||
dir: &Path,
|
||||
service: &str,
|
||||
alpn: &[u8],
|
||||
configure: impl FnOnce(&mut MitmHookConfig, u16),
|
||||
) -> (MitmRequestContext, TcpListener, TlsAcceptor) {
|
||||
ensure_rustls_crypto_provider();
|
||||
let key = KeyPair::generate().expect("generate origin key");
|
||||
@@ -94,6 +103,7 @@ async fn tls_origin(
|
||||
}],
|
||||
..Default::default()
|
||||
};
|
||||
configure(&mut config.mitm_hooks[0], port);
|
||||
config.set_credential_broker_enabled(/*enabled*/ true);
|
||||
config.set_allowed_domains(vec!["localhost".to_string()]);
|
||||
let app_state = Arc::new(network_proxy_state_for_policy(config));
|
||||
@@ -479,3 +489,105 @@ async fn drained_signed_bodies_release_capacity_before_response_headers() {
|
||||
.expect("drained bodies must release buffer slots before response headers");
|
||||
assert_eq!(response.status(), StatusCode::OK);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn forwards_substituted_body_before_signing() {
|
||||
let marker = b"codex-secret-0123456789abcdef";
|
||||
let dir = tempdir().expect("TLS fixture directory");
|
||||
let secret = dir.path().join("body-secret");
|
||||
std::fs::write(&secret, "expanded-payload").unwrap();
|
||||
let configure = |hook: &mut MitmHookConfig, port| {
|
||||
hook.matcher.path_prefixes = vec!["/allowed/".into()];
|
||||
hook.actions.strip_request_headers.push("digest".into());
|
||||
hook.actions.body_secret_substitution = Some(crate::BodySecretSubstitutionConfig {
|
||||
replacements: vec![crate::BodySecretReplacement {
|
||||
placeholder: String::from_utf8(marker.to_vec()).unwrap(),
|
||||
secret: ProxySecretSource::File(secret.display().to_string()),
|
||||
}],
|
||||
port,
|
||||
max_body_bytes: 64,
|
||||
});
|
||||
};
|
||||
let (context, listener, acceptor) =
|
||||
tls_origin_with_hook(dir.path(), "s3", b"http/1.1", configure).await;
|
||||
let context = Arc::new(context);
|
||||
let signed_headers = "content-length;cookie;host;x-amz-date";
|
||||
let make_request = |path: &str| {
|
||||
let mut req = request(&context, "s3", "POST", path, marker);
|
||||
*req.version_mut() = rama_http::Version::HTTP_2;
|
||||
let headers = req.headers_mut();
|
||||
headers.remove("x-amz-content-sha256");
|
||||
headers.insert(
|
||||
"authorization",
|
||||
HeaderValue::from_str(&guest_authorization("s3", signed_headers)).unwrap(),
|
||||
);
|
||||
headers.insert("content-length", HeaderValue::from(marker.len()));
|
||||
headers.append("cookie", HeaderValue::from_static("a=1"));
|
||||
headers.append("cookie", HeaderValue::from_static("b=2"));
|
||||
req
|
||||
};
|
||||
let denied = handle_mitm_request(make_request("/denied"), context.clone())
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(denied.status(), StatusCode::FORBIDDEN);
|
||||
let mut invalid = make_request("/allowed/item");
|
||||
invalid
|
||||
.headers_mut()
|
||||
.insert("digest", HeaderValue::from_static("sha-256=guest"));
|
||||
let denied = handle_mitm_request(invalid, context.clone()).await.unwrap();
|
||||
assert_eq!(denied.status(), StatusCode::FORBIDDEN);
|
||||
let origin = tokio::spawn(async move {
|
||||
let (socket, _) = listener.accept().await.unwrap();
|
||||
let mut stream = acceptor.accept(socket).await.unwrap();
|
||||
let wire = read_request(&mut stream).await;
|
||||
stream.write_all(RESPONSE).await.unwrap();
|
||||
wire
|
||||
});
|
||||
let req = make_request("/allowed/item");
|
||||
let response = timeout(FORWARD_TIMEOUT, handle_mitm_request(req, context))
|
||||
.await
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
assert_eq!(response.status(), StatusCode::OK);
|
||||
let wire = origin.await.unwrap();
|
||||
assert_eq!(wire.body.as_slice(), b"expanded-payload");
|
||||
assert_eq!(wire.path, "/allowed/item");
|
||||
let headers = wire.headers;
|
||||
assert_eq!(headers["cookie"], "a=1; b=2");
|
||||
let expected_hash = format!("{:x}", sha2::Sha256::digest(&wire.body));
|
||||
assert_eq!(headers["x-amz-content-sha256"], expected_hash);
|
||||
// Rebuild the canonical request from the received HTTP/1.1 headers, without
|
||||
// the signer's request preparation or the SDK's header canonicalization.
|
||||
let canonical_headers = signed_headers
|
||||
.split(';')
|
||||
.map(|name| format!("{name}:{}\n", headers[name]))
|
||||
.collect::<String>();
|
||||
let canonical_request =
|
||||
format!("POST\n/allowed/item\n\n{canonical_headers}\n{signed_headers}\n{expected_hash}");
|
||||
let timestamp = &headers["x-amz-date"];
|
||||
let signing_time = chrono::NaiveDateTime::parse_from_str(timestamp, "%Y%m%dT%H%M%SZ")
|
||||
.unwrap()
|
||||
.and_utc();
|
||||
let scope = format!(
|
||||
"{}/us-east-1/s3/aws4_request",
|
||||
signing_time.format("%Y%m%d")
|
||||
);
|
||||
let string_to_sign = format!(
|
||||
"AWS4-HMAC-SHA256\n{timestamp}\n{scope}\n{:x}",
|
||||
sha2::Sha256::digest(canonical_request.as_bytes())
|
||||
);
|
||||
let signing_key = aws_sigv4::sign::v4::generate_signing_key(
|
||||
"test-secret-key",
|
||||
signing_time.into(),
|
||||
"us-east-1",
|
||||
"s3",
|
||||
);
|
||||
let signature =
|
||||
aws_sigv4::sign::v4::calculate_signature(signing_key, string_to_sign.as_bytes());
|
||||
assert_eq!(
|
||||
headers["authorization"],
|
||||
format!(
|
||||
"AWS4-HMAC-SHA256 Credential=AKIDEXAMPLE/{scope}, SignedHeaders={signed_headers}, Signature={signature}"
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
@@ -5,6 +5,8 @@ use crate::aws_sigv4::AwsSigV4Config;
|
||||
use crate::aws_sigv4::AwsSigV4Signer;
|
||||
use crate::config::NetworkProxyConfig;
|
||||
use crate::policy::normalize_host;
|
||||
use crate::secret_substitution::BodySecretSubstitution;
|
||||
use crate::secret_substitution::BodySecretSubstitutionConfig;
|
||||
use anyhow::Context as _;
|
||||
use anyhow::Result;
|
||||
use anyhow::anyhow;
|
||||
@@ -52,6 +54,7 @@ pub struct MitmHookMatchConfig {
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq, Eq)]
|
||||
#[serde(default)]
|
||||
pub struct MitmHookActionsConfig {
|
||||
pub body_secret_substitution: Option<BodySecretSubstitutionConfig>,
|
||||
pub aws_sigv4: Option<AwsSigV4Config>,
|
||||
pub strip_request_headers: Vec<String>,
|
||||
pub inject_request_headers: Vec<InjectedHeaderConfig>,
|
||||
@@ -104,6 +107,7 @@ pub struct HeaderConstraint {
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct MitmHookActions {
|
||||
pub(crate) body_secret_substitution: Option<Arc<BodySecretSubstitution>>,
|
||||
pub(crate) aws_sigv4: Option<Arc<AwsSigV4Signer>>,
|
||||
pub strip_request_headers: Vec<HeaderName>,
|
||||
pub inject_request_headers: Vec<ResolvedInjectedHeader>,
|
||||
@@ -245,6 +249,9 @@ pub(crate) fn validate_mitm_hook_config(config: &NetworkProxyConfig) -> Result<(
|
||||
if let Some(signing) = &hook.actions.aws_sigv4 {
|
||||
signing.validate()?;
|
||||
}
|
||||
if let Some(substitution) = &hook.actions.body_secret_substitution {
|
||||
substitution.validate()?;
|
||||
}
|
||||
validate_injected_headers(&hook.actions.inject_request_headers).with_context(|| {
|
||||
format!("invalid network.mitm_hooks[{hook_index}].actions.inject_request_headers")
|
||||
})?;
|
||||
@@ -373,6 +380,16 @@ where
|
||||
body: None,
|
||||
},
|
||||
actions: MitmHookActions {
|
||||
body_secret_substitution: hook
|
||||
.actions
|
||||
.body_secret_substitution
|
||||
.as_ref()
|
||||
.map(|substitution| {
|
||||
substitution
|
||||
.compile(&resolve_env_var, &read_secret_file)
|
||||
.map(Arc::new)
|
||||
})
|
||||
.transpose()?,
|
||||
aws_sigv4: hook
|
||||
.actions
|
||||
.aws_sigv4
|
||||
@@ -803,6 +820,7 @@ mod tests {
|
||||
..MitmHookMatchConfig::default()
|
||||
},
|
||||
actions: MitmHookActionsConfig {
|
||||
body_secret_substitution: None,
|
||||
aws_sigv4: None,
|
||||
strip_request_headers: vec!["authorization".to_string()],
|
||||
inject_request_headers: vec![InjectedHeaderConfig {
|
||||
|
||||
@@ -29,6 +29,7 @@ fn github_write_hook() -> crate::mitm_hook::MitmHookConfig {
|
||||
..crate::mitm_hook::MitmHookMatchConfig::default()
|
||||
},
|
||||
actions: crate::mitm_hook::MitmHookActionsConfig {
|
||||
body_secret_substitution: None,
|
||||
aws_sigv4: None,
|
||||
strip_request_headers: vec!["authorization".to_string()],
|
||||
inject_request_headers: vec![crate::mitm_hook::InjectedHeaderConfig {
|
||||
@@ -572,6 +573,7 @@ fn request_credentials_apply_hook_after_brokerage() {
|
||||
let mut secret = HeaderValue::from_static("Bearer secret-token");
|
||||
secret.set_sensitive(true);
|
||||
let actions = crate::mitm_hook::MitmHookActions {
|
||||
body_secret_substitution: None,
|
||||
aws_sigv4: None,
|
||||
strip_request_headers: vec![HeaderName::from_static("authorization")],
|
||||
inject_request_headers: vec![crate::mitm_hook::ResolvedInjectedHeader {
|
||||
|
||||
@@ -0,0 +1,264 @@
|
||||
//! Bounded byte-exact secret substitution after destination authorization, before signing.
|
||||
//! Placeholder bindings and resolved secret values are private to the trusted proxy.
|
||||
|
||||
use crate::aws_sigv4::ProxySecretSource;
|
||||
use anyhow::Result;
|
||||
use anyhow::anyhow;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use rama_core::extensions::ExtensionsMut;
|
||||
use rama_core::extensions::ExtensionsRef;
|
||||
use rama_http::Body;
|
||||
use rama_http::HeaderMap;
|
||||
use rama_http::HeaderValue;
|
||||
use rama_http::Request;
|
||||
use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use std::time::Duration;
|
||||
|
||||
// Bound the worst-case number of bytes compared by the placeholder window scans.
|
||||
const MAX_SUBSTITUTION_SEARCH_BYTES: usize = 256 * 1024 * 1024;
|
||||
|
||||
#[derive(Clone, Copy, Debug)]
|
||||
struct OriginalBodyHeadersSupported(bool);
|
||||
|
||||
pub(crate) fn preserve_body_substitution_headers(req: &mut Request) {
|
||||
if req
|
||||
.extensions()
|
||||
.get::<OriginalBodyHeadersSupported>()
|
||||
.is_none()
|
||||
{
|
||||
let supported = body_substitution_headers_supported(req.headers());
|
||||
req.extensions_mut()
|
||||
.insert(OriginalBodyHeadersSupported(supported));
|
||||
}
|
||||
}
|
||||
|
||||
fn body_substitution_headers_supported(headers: &HeaderMap) -> bool {
|
||||
let identity_encoding = headers.get_all("content-encoding").iter().all(|value| {
|
||||
value.to_str().is_ok_and(|value| {
|
||||
value
|
||||
.split(',')
|
||||
.all(|coding| coding.trim().eq_ignore_ascii_case("identity"))
|
||||
})
|
||||
});
|
||||
identity_encoding
|
||||
&& crate::body_budget::supported_transfer_framing(headers)
|
||||
&& !crate::body_budget::is_amazon_event_stream(headers)
|
||||
&& !headers.contains_key("trailer")
|
||||
&& !headers.contains_key("x-amz-trailer")
|
||||
&& !headers.contains_key("x-amz-decoded-content-length")
|
||||
&& !headers.keys().any(|name| {
|
||||
matches!(
|
||||
name.as_str(),
|
||||
"content-md5"
|
||||
| "digest"
|
||||
| "content-digest"
|
||||
| "repr-digest"
|
||||
| "x-amz-content-sha256"
|
||||
) || name.as_str().starts_with("x-amz-checksum-")
|
||||
})
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct BodySecretReplacement {
|
||||
pub placeholder: String,
|
||||
pub secret: ProxySecretSource,
|
||||
}
|
||||
|
||||
/// Exact opaque placeholders scoped by their enclosing host/method/path MITM hook.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct BodySecretSubstitutionConfig {
|
||||
pub replacements: Vec<BodySecretReplacement>,
|
||||
#[serde(default = "https_port")]
|
||||
pub port: u16,
|
||||
#[serde(default = "default_max_body_bytes")]
|
||||
pub max_body_bytes: usize,
|
||||
}
|
||||
|
||||
pub(crate) fn https_port() -> u16 {
|
||||
443
|
||||
}
|
||||
pub(crate) fn default_max_body_bytes() -> usize {
|
||||
8 * 1024 * 1024
|
||||
}
|
||||
|
||||
#[derive(Clone, PartialEq, Eq)]
|
||||
pub(crate) struct BodySecretSubstitution {
|
||||
config: BodySecretSubstitutionConfig,
|
||||
secrets: Vec<Vec<u8>>,
|
||||
}
|
||||
|
||||
impl std::fmt::Debug for BodySecretSubstitution {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.debug_struct("BodySecretSubstitution")
|
||||
.field("config", &self.config)
|
||||
.finish_non_exhaustive()
|
||||
}
|
||||
}
|
||||
|
||||
impl BodySecretSubstitutionConfig {
|
||||
pub(crate) fn validate(&self) -> Result<()> {
|
||||
if self.port == 0
|
||||
|| self.max_body_bytes > 16 * 1024 * 1024
|
||||
|| self.replacements.is_empty()
|
||||
|| self.replacements.len() > 32
|
||||
{
|
||||
return Err(anyhow!(
|
||||
"invalid body substitution port, bindings, or limit (maximum 16 MiB)"
|
||||
));
|
||||
}
|
||||
for (index, replacement) in self.replacements.iter().enumerate() {
|
||||
if !(16..=256).contains(&replacement.placeholder.len())
|
||||
|| !replacement.placeholder.is_ascii()
|
||||
{
|
||||
return Err(anyhow!(
|
||||
"body placeholders must be opaque ASCII markers of 16 to 256 bytes"
|
||||
));
|
||||
}
|
||||
replacement.secret.validate()?;
|
||||
if self.replacements[..index].iter().any(|previous| {
|
||||
previous.placeholder.contains(&replacement.placeholder)
|
||||
|| replacement.placeholder.contains(&previous.placeholder)
|
||||
}) {
|
||||
return Err(anyhow!("body substitution bindings overlap"));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(crate) fn compile(
|
||||
&self,
|
||||
env: &impl Fn(&str) -> Option<String>,
|
||||
file: &impl Fn(&AbsolutePathBuf) -> Result<String>,
|
||||
) -> Result<BodySecretSubstitution> {
|
||||
self.validate()?;
|
||||
let secrets = self
|
||||
.replacements
|
||||
.iter()
|
||||
.map(|replacement| {
|
||||
let secret = replacement.secret.resolve(env, file)?;
|
||||
if secret.len() > self.max_body_bytes
|
||||
|| self
|
||||
.replacements
|
||||
.iter()
|
||||
.any(|binding| secret.contains(&binding.placeholder))
|
||||
{
|
||||
return Err(anyhow!(
|
||||
"body substitution secret exceeds limit or contains a placeholder"
|
||||
));
|
||||
}
|
||||
Ok(secret.into_bytes())
|
||||
})
|
||||
.collect::<Result<Vec<_>>>()?;
|
||||
Ok(BodySecretSubstitution {
|
||||
config: self.clone(),
|
||||
secrets,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
impl BodySecretSubstitution {
|
||||
pub(crate) async fn apply(&self, mut req: Request) -> Result<Request> {
|
||||
if req.uri().scheme_str() != Some("https")
|
||||
|| req.uri().port_u16().unwrap_or(443) != self.config.port
|
||||
{
|
||||
return Err(anyhow!(
|
||||
"body substitution requires its configured HTTPS port"
|
||||
));
|
||||
}
|
||||
if req
|
||||
.extensions()
|
||||
.get::<OriginalBodyHeadersSupported>()
|
||||
.is_some_and(|original| !original.0)
|
||||
|| !body_substitution_headers_supported(req.headers())
|
||||
{
|
||||
return Err(anyhow!(
|
||||
"body substitution does not support encoded, streaming-signed, or integrity-protected payloads"
|
||||
));
|
||||
}
|
||||
let _permit = crate::body_budget::reserve(&mut req).await?;
|
||||
let (mut parts, body) = req.into_parts();
|
||||
let (body, trailers) = tokio::time::timeout(
|
||||
Duration::from_secs(30),
|
||||
crate::body_budget::collect_body(body, self.config.max_body_bytes),
|
||||
)
|
||||
.await
|
||||
.map_err(|_| anyhow!("body substitution read timed out"))?
|
||||
.map_err(|_| anyhow!("body substitution exceeds limit or cannot read payload"))?;
|
||||
if trailers.is_some() {
|
||||
return Err(anyhow!(
|
||||
"body substitution does not support request trailers"
|
||||
));
|
||||
}
|
||||
if !crate::body_budget::content_length_matches(&parts.headers, body.len()) {
|
||||
return Err(anyhow!(
|
||||
"body substitution content length does not match the request body"
|
||||
));
|
||||
}
|
||||
// Charge every window for its full length before searching, including failed matches.
|
||||
let search_bytes = self
|
||||
.config
|
||||
.replacements
|
||||
.iter()
|
||||
.try_fold(0usize, |work, binding| {
|
||||
let windows = body.len().saturating_sub(binding.placeholder.len() - 1);
|
||||
work.checked_add(windows.checked_mul(binding.placeholder.len())?)
|
||||
});
|
||||
if search_bytes.is_none_or(|work| work > MAX_SUBSTITUTION_SEARCH_BYTES) {
|
||||
return Err(anyhow!("body substitution search exceeds work limit"));
|
||||
}
|
||||
// Search the original bytes only. Replacements cannot recursively expand other bindings.
|
||||
let mut matches = Vec::new();
|
||||
for (index, binding) in self.config.replacements.iter().enumerate() {
|
||||
for (offset, window) in body.windows(binding.placeholder.len()).enumerate() {
|
||||
if window == binding.placeholder.as_bytes() {
|
||||
if matches.len() >= 65_536 {
|
||||
return Err(anyhow!("too many body substitution occurrences"));
|
||||
}
|
||||
matches.push((offset, offset + binding.placeholder.len(), index));
|
||||
}
|
||||
}
|
||||
}
|
||||
matches.sort_unstable();
|
||||
let mut cursor = 0;
|
||||
let mut size = body.len();
|
||||
for &(start, end, index) in &matches {
|
||||
if start < cursor {
|
||||
return Err(anyhow!("body substitution matches overlap"));
|
||||
}
|
||||
size = size
|
||||
.checked_sub(end - start)
|
||||
.and_then(|size| size.checked_add(self.secrets[index].len()))
|
||||
.ok_or_else(|| anyhow!("expanded body substitution exceeds limit"))?;
|
||||
cursor = end;
|
||||
}
|
||||
if size > self.config.max_body_bytes {
|
||||
return Err(anyhow!("expanded body substitution exceeds limit"));
|
||||
}
|
||||
let mut output = Vec::with_capacity(size);
|
||||
cursor = 0;
|
||||
for (start, end, index) in matches {
|
||||
output.extend_from_slice(&body[cursor..start]);
|
||||
output.extend_from_slice(&self.secrets[index]);
|
||||
cursor = end;
|
||||
}
|
||||
output.extend_from_slice(&body[cursor..]);
|
||||
parts.headers.remove("transfer-encoding");
|
||||
parts.headers.remove("trailer");
|
||||
parts.headers.insert(
|
||||
"content-length",
|
||||
HeaderValue::from_str(&output.len().to_string())?,
|
||||
);
|
||||
Ok(crate::body_budget::retain(Request::from_parts(
|
||||
parts,
|
||||
Body::from(output),
|
||||
)))
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "secret_substitution_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,261 @@
|
||||
//! Exact substitution preserves chunk boundaries, authorization scope, and signed wire bytes.
|
||||
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
use rama_core::bytes::Bytes;
|
||||
use rama_core::futures::stream;
|
||||
use rama_http::body::util::BodyExt;
|
||||
|
||||
const MARKER: &str = "codex-secret-0123456789abcdef";
|
||||
|
||||
fn config() -> BodySecretSubstitutionConfig {
|
||||
BodySecretSubstitutionConfig {
|
||||
replacements: vec![BodySecretReplacement {
|
||||
placeholder: MARKER.into(),
|
||||
secret: ProxySecretSource::EnvVar("SECRET".into()),
|
||||
}],
|
||||
port: 443,
|
||||
max_body_bytes: 1024,
|
||||
}
|
||||
}
|
||||
|
||||
fn compile(config: BodySecretSubstitutionConfig, secret: &str) -> BodySecretSubstitution {
|
||||
config
|
||||
.compile(&|_| Some(secret.into()), &|_| {
|
||||
panic!("unexpected file read")
|
||||
})
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn substitutes_split_markers_and_updates_framing() {
|
||||
let body = Body::from_stream(stream::iter([
|
||||
Ok::<_, std::io::Error>(Bytes::from_static(b"before:codex-secret-0123")),
|
||||
Ok(Bytes::from_static(b"456789abcdef:after")),
|
||||
]));
|
||||
let request = Request::builder()
|
||||
.method("PUT")
|
||||
.uri("https://bucket.s3.amazonaws.com/object")
|
||||
.header("transfer-encoding", "chunked")
|
||||
.header("content-encoding", "Identity, identity")
|
||||
.header("content-encoding", "IDENTITY")
|
||||
.body(body)
|
||||
.unwrap();
|
||||
let substituted = compile(config(), "actual-secret")
|
||||
.apply(request)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(substituted.headers()["content-length"], "26");
|
||||
assert!(!substituted.headers().contains_key("transfer-encoding"));
|
||||
let payload = b"before:actual-secret:after";
|
||||
assert_eq!(
|
||||
substituted
|
||||
.into_body()
|
||||
.collect()
|
||||
.await
|
||||
.unwrap()
|
||||
.to_bytes()
|
||||
.as_ref(),
|
||||
payload
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn scope_overflow_encoding_trailers_and_ambiguous_matches_fail_closed() {
|
||||
let mut small = config();
|
||||
small.max_body_bytes = 40;
|
||||
let transform = compile(small, "long-replacement-value");
|
||||
for (uri, body) in [
|
||||
("http://example.com/", Body::from(MARKER)),
|
||||
("https://example.com:444/", Body::from(MARKER)),
|
||||
("https://example.com/", Body::from(vec![b'x'; 41])),
|
||||
(
|
||||
"https://example.com/",
|
||||
Body::empty().with_trailer_headers(rama_http::HeaderMap::new()),
|
||||
),
|
||||
] {
|
||||
let req = Request::builder().uri(uri).body(body).unwrap();
|
||||
assert!(transform.apply(req).await.is_err());
|
||||
}
|
||||
// Cover expansion and contraction in both orders at the final byte limit.
|
||||
let expanding_marker = "0123456789abcdef";
|
||||
let shrinking_marker = "fedcba9876543210";
|
||||
let mut mixed = config();
|
||||
mixed.max_body_bytes = 32;
|
||||
mixed.replacements[0].placeholder = expanding_marker.into();
|
||||
mixed.replacements.push(BodySecretReplacement {
|
||||
placeholder: shrinking_marker.into(),
|
||||
secret: ProxySecretSource::EnvVar("SHRINKING".into()),
|
||||
});
|
||||
for expanding_secret in ["x".repeat(30), "x".repeat(32)] {
|
||||
let transform = mixed
|
||||
.compile(
|
||||
&|name| match name {
|
||||
"SECRET" => Some(expanding_secret.clone()),
|
||||
"SHRINKING" => Some("y".into()),
|
||||
_ => None,
|
||||
},
|
||||
&|_| panic!("unexpected file read"),
|
||||
)
|
||||
.unwrap();
|
||||
for (body, expected) in [
|
||||
(
|
||||
format!("{expanding_marker}{shrinking_marker}"),
|
||||
format!("{expanding_secret}y"),
|
||||
),
|
||||
(
|
||||
format!("{shrinking_marker}{expanding_marker}"),
|
||||
format!("y{expanding_secret}"),
|
||||
),
|
||||
] {
|
||||
let result = transform
|
||||
.apply(
|
||||
Request::builder()
|
||||
.uri("https://example.com/")
|
||||
.header("content-length", "32")
|
||||
.body(Body::from(body))
|
||||
.unwrap(),
|
||||
)
|
||||
.await;
|
||||
if expected.len() > mixed.max_body_bytes {
|
||||
assert_eq!(
|
||||
result.unwrap_err().to_string(),
|
||||
"expanded body substitution exceeds limit"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
let substituted = result.unwrap();
|
||||
assert_eq!(substituted.headers()["content-length"], "31");
|
||||
assert!(!substituted.headers().contains_key("transfer-encoding"));
|
||||
assert_eq!(
|
||||
substituted
|
||||
.into_body()
|
||||
.collect()
|
||||
.await
|
||||
.unwrap()
|
||||
.to_bytes()
|
||||
.as_ref(),
|
||||
expected.as_bytes()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
for coding in [None, Some("chunked")] {
|
||||
let mut req = Request::builder()
|
||||
.uri("https://example.com/")
|
||||
.header("content-length", "1");
|
||||
if let Some(coding) = coding {
|
||||
req = req.header("transfer-encoding", coding);
|
||||
}
|
||||
assert!(
|
||||
transform
|
||||
.apply(req.body(Body::from(MARKER)).unwrap())
|
||||
.await
|
||||
.is_err()
|
||||
);
|
||||
}
|
||||
|
||||
let mut overlaps = config();
|
||||
overlaps.replacements[0].placeholder = "0123456789abcdef0".into();
|
||||
let transform = compile(overlaps, "replacement");
|
||||
assert!(
|
||||
transform
|
||||
.apply(
|
||||
Request::builder()
|
||||
.uri("https://example.com/")
|
||||
.body(Body::from("0123456789abcdef0123456789abcdef0"))
|
||||
.unwrap()
|
||||
)
|
||||
.await
|
||||
.is_err()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_duplicate_bindings_recursive_values_and_redacts_secrets() {
|
||||
let mut duplicate = config();
|
||||
duplicate
|
||||
.replacements
|
||||
.push(duplicate.replacements[0].clone());
|
||||
assert!(duplicate.validate().is_err());
|
||||
assert!(
|
||||
config()
|
||||
.compile(&|_| Some(MARKER.into()), &|_| panic!("unexpected file"))
|
||||
.is_err()
|
||||
);
|
||||
let compiled = compile(config(), "do-not-log-this-secret");
|
||||
assert!(!format!("{compiled:?}").contains("do-not-log-this-secret"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn rejects_integrity_headers_and_preserves_original_validation() {
|
||||
let transform = compile(config(), "secret");
|
||||
for (name, value) in [
|
||||
(
|
||||
"content-type",
|
||||
"Application/Vnd.Amazon.Eventstream; version=1",
|
||||
),
|
||||
("content-encoding", "gzip"),
|
||||
("transfer-encoding", "gzip, chunked"),
|
||||
("trailer", "digest"),
|
||||
("x-amz-trailer", "x-amz-checksum-sha256"),
|
||||
("x-amz-decoded-content-length", "32"),
|
||||
("content-md5", "placeholder-digest"),
|
||||
("digest", "sha-256=placeholder-digest"),
|
||||
("content-digest", "sha-256=:placeholder-digest:"),
|
||||
("repr-digest", "sha-256=:placeholder-digest:"),
|
||||
("x-amz-content-sha256", "placeholder-digest"),
|
||||
("x-amz-checksum-sha256", "placeholder-digest"),
|
||||
("x-amz-checksum-crc32", "placeholder-digest"),
|
||||
] {
|
||||
for strip in [false, true] {
|
||||
let mut request = Request::builder()
|
||||
.uri("https://example.com/")
|
||||
.header(name, value)
|
||||
.body(Body::from(MARKER))
|
||||
.unwrap();
|
||||
preserve_body_substitution_headers(&mut request);
|
||||
if strip {
|
||||
request.headers_mut().remove(name);
|
||||
preserve_body_substitution_headers(&mut request);
|
||||
}
|
||||
assert!(
|
||||
transform.apply(request).await.is_err(),
|
||||
"{name}, strip={strip}"
|
||||
);
|
||||
}
|
||||
}
|
||||
let mut request = Request::builder()
|
||||
.uri("https://example.com/")
|
||||
.body(Body::from(MARKER))
|
||||
.unwrap();
|
||||
preserve_body_substitution_headers(&mut request);
|
||||
request
|
||||
.headers_mut()
|
||||
.insert("content-encoding", HeaderValue::from_static("gzip"));
|
||||
assert!(transform.apply(request).await.is_err());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn rejects_excessive_search_work_before_scanning_shared_prefixes() {
|
||||
let mut bounded = config();
|
||||
bounded.max_body_bytes = 1024 * 1024;
|
||||
bounded.replacements = (0..32)
|
||||
.map(|index| BodySecretReplacement {
|
||||
placeholder: format!("{}{:02}", "x".repeat(254), index),
|
||||
secret: ProxySecretSource::EnvVar("SECRET".into()),
|
||||
})
|
||||
.collect();
|
||||
let request = Request::builder()
|
||||
.uri("https://example.com/")
|
||||
.body(Body::from(vec![b'x'; bounded.max_body_bytes]))
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
compile(bounded, "secret")
|
||||
.apply(request)
|
||||
.await
|
||||
.unwrap_err()
|
||||
.to_string(),
|
||||
"body substitution search exceeds work limit"
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user