mirror of
https://github.com/neondatabase/neon.git
synced 2026-07-03 12:10:36 +00:00
proxy: split password hack payload on first separator (#12899)
## Problem
`PasswordHackPayload::parse` iterated through `[";", "$"]` and used
whichever separator appeared in the bytes first by *iteration* order,
not by *position* in the input. The result: a client that picked `$` as
its separator and happened to put `;` somewhere in the password got
split at the `;` instead.
Concretely, `endpoint=foobar$pass;with;semis` parsed to:
- endpoint: `foobar$pass` (rejected later by `parse_endpoint_param`, or
routed to the wrong endpoint if the prefix happened to look valid)
- password: `with;semis`
Endpoint names are restricted to alphanumeric plus hyphen (see
`parse_endpoint_param`), so neither separator can ever appear inside an
endpoint. Whichever separator appears first in the bytes is the correct
one.
## Summary of changes
* `proxy/src/auth/password_hack.rs`: scan the input for the first
occurrence of either `;` or `$` (`bytes.iter().position(...)`), split
there, and use everything past it as the password.
* Add `parse_uses_first_separator` test covering both directions
(`$`-separated payload with `;` in the password, and the mirror case).
* Existing `parse_password_hack_payload_{project,endpoint,dollar}` tests
continue to pass.
Before:
$ cargo test -p proxy --lib
auth::password_hack::tests::parse_uses_first_separator
test auth::password_hack::tests::parse_uses_first_separator ... FAILED
thread '...' panicked at proxy/src/auth/password_hack.rs:126:9:
assertion `left == right` failed
left: EndpointId("foobar$pass")
right: "foobar"
After:
$ cargo test -p proxy --lib auth::password_hack
running 5 tests
test auth::password_hack::tests::parse_uses_first_separator ... ok
test auth::password_hack::tests::parse_password_hack_payload_dollar ...
ok
test auth::password_hack::tests::parse_password_hack_payload_project ...
ok
test auth::password_hack::tests::parse_password_hack_payload_endpoint
... ok
test auth::password_hack::tests::parse_endpoint_param_fn ... ok
test result: ok. 5 passed; 0 failed
This commit is contained in:
@@ -15,18 +15,18 @@ pub(crate) struct PasswordHackPayload {
|
||||
impl PasswordHackPayload {
|
||||
pub(crate) fn parse(bytes: &[u8]) -> Option<Self> {
|
||||
// The format is `project=<utf-8>;<password-bytes>` or `project=<utf-8>$<password-bytes>`.
|
||||
let separators = [";", "$"];
|
||||
for sep in separators {
|
||||
if let Some((endpoint, password)) = bytes.split_once_str(sep) {
|
||||
let endpoint = endpoint.to_str().ok()?;
|
||||
return Some(Self {
|
||||
endpoint: parse_endpoint_param(endpoint)?.into(),
|
||||
password: password.to_owned(),
|
||||
});
|
||||
}
|
||||
}
|
||||
// The endpoint name is restricted to alphanumeric/hyphen, so it never
|
||||
// contains either separator; split on whichever one appears first so
|
||||
// we don't truncate the password when it contains the other separator.
|
||||
let split = bytes.iter().position(|&b| b == b';' || b == b'$')?;
|
||||
let (endpoint, rest) = bytes.split_at(split);
|
||||
let password = &rest[1..];
|
||||
|
||||
None
|
||||
let endpoint = endpoint.to_str().ok()?;
|
||||
Some(Self {
|
||||
endpoint: parse_endpoint_param(endpoint)?.into(),
|
||||
password: password.to_owned(),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -118,4 +118,24 @@ mod tests {
|
||||
assert_eq!(payload.endpoint, "foobar");
|
||||
assert_eq!(payload.password, b"pass$word");
|
||||
}
|
||||
|
||||
// Whichever separator the client used must win, regardless of what
|
||||
// characters happen to appear later in the password. Previously the parser
|
||||
// always tried `;` before `$`, which truncated the endpoint when a client
|
||||
// used `$` as the separator but had `;` in the password (e.g. AWS DMS
|
||||
// forbids `: ;+%` only in the password text it builds itself, but other
|
||||
// tooling may legitimately pass these through unencoded).
|
||||
#[test]
|
||||
fn parse_uses_first_separator() {
|
||||
let bytes = b"endpoint=foobar$pass;with;semis";
|
||||
let payload = PasswordHackPayload::parse(bytes).expect("parsing failed");
|
||||
assert_eq!(payload.endpoint, "foobar");
|
||||
assert_eq!(payload.password, b"pass;with;semis");
|
||||
|
||||
// And the mirror case: `;` is the separator, password contains `$`.
|
||||
let bytes = b"endpoint=foobar;pass$with$dollars";
|
||||
let payload = PasswordHackPayload::parse(bytes).expect("parsing failed");
|
||||
assert_eq!(payload.endpoint, "foobar");
|
||||
assert_eq!(payload.password, b"pass$with$dollars");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user