Skip to content

Commit

Permalink
proxy: async aware password validation (#7176)
Browse files Browse the repository at this point in the history
## Problem

spawn_blocking in #7171 was a hack

## Summary of changes

neondatabase/rust-postgres#29
  • Loading branch information
conradludgate authored Mar 21, 2024
1 parent 94138c1 commit 5ec6862
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 25 deletions.
11 changes: 6 additions & 5 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 6 additions & 5 deletions proxy/src/proxy/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,10 @@ impl TestAuth for NoAuth {}
struct Scram(scram::ServerSecret);

impl Scram {
fn new(password: &str) -> anyhow::Result<Self> {
let secret =
scram::ServerSecret::build(password).context("failed to generate scram secret")?;
async fn new(password: &str) -> anyhow::Result<Self> {
let secret = scram::ServerSecret::build(password)
.await
.context("failed to generate scram secret")?;
Ok(Scram(secret))
}

Expand Down Expand Up @@ -284,7 +285,7 @@ async fn scram_auth_good(#[case] password: &str) -> anyhow::Result<()> {
let proxy = tokio::spawn(dummy_proxy(
client,
Some(server_config),
Scram::new(password)?,
Scram::new(password).await?,
));

let (_client, _conn) = tokio_postgres::Config::new()
Expand All @@ -308,7 +309,7 @@ async fn scram_auth_disable_channel_binding() -> anyhow::Result<()> {
let proxy = tokio::spawn(dummy_proxy(
client,
Some(server_config),
Scram::new("password")?,
Scram::new("password").await?,
));

let (_client, _conn) = tokio_postgres::Config::new()
Expand Down
4 changes: 2 additions & 2 deletions proxy/src/proxy/tests/mitm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ async fn scram_auth_disable_channel_binding() -> anyhow::Result<()> {
let proxy = tokio::spawn(dummy_proxy(
client,
Some(server_config),
Scram::new("password")?,
Scram::new("password").await?,
));

let _client_err = tokio_postgres::Config::new()
Expand Down Expand Up @@ -231,7 +231,7 @@ async fn connect_failure(
let proxy = tokio::spawn(dummy_proxy(
client,
Some(server_config),
Scram::new("password")?,
Scram::new("password").await?,
));

let _client_err = tokio_postgres::Config::new()
Expand Down
2 changes: 1 addition & 1 deletion proxy/src/scram.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ mod tests {
}

async fn run_round_trip_test(server_password: &str, client_password: &str) {
let scram_secret = ServerSecret::build(server_password).unwrap();
let scram_secret = ServerSecret::build(server_password).await.unwrap();
let sasl_client =
ScramSha256::new(client_password.as_bytes(), ChannelBinding::unsupported());

Expand Down
9 changes: 1 addition & 8 deletions proxy/src/scram/exchange.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,7 @@ pub async fn exchange(
.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))?;
let sent = match init.transition(secret, &tls_server_end_point, client_first)? {
Continue(sent, server_first) => {
// `client.update` might perform `pbkdf2(pw)`, best to spawn it in a blocking thread.
// TODO(conrad): take this code from tokio-postgres and make an async-aware pbkdf2 impl
client = tokio::task::spawn_blocking(move || {
client.update(server_first.as_bytes())?;
Ok::<ScramSha256, std::io::Error>(client)
})
.await
.expect("should not panic while performing password hash")?;
client.update(server_first.as_bytes()).await?;
sent
}
Success(x, _) => match x {},
Expand Down
6 changes: 2 additions & 4 deletions proxy/src/scram/secret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,8 @@ impl ServerSecret {
/// Build a new server secret from the prerequisites.
/// XXX: We only use this function in tests.
#[cfg(test)]
pub fn build(password: &str) -> Option<Self> {
Self::parse(&postgres_protocol::password::scram_sha_256(
password.as_bytes(),
))
pub async fn build(password: &str) -> Option<Self> {
Self::parse(&postgres_protocol::password::scram_sha_256(password.as_bytes()).await)
}
}

Expand Down

1 comment on commit 5ec6862

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2786 tests run: 2641 passed, 1 failed, 144 skipped (full report)


Failures on Postgres 14

  • test_bulk_insert[neon-github-actions-selfhosted]: release
# Run all failed tests locally:
scripts/pytest -vv -n $(nproc) -k "test_bulk_insert[neon-release-pg14-github-actions-selfhosted]"

Code coverage* (full report)

  • functions: 28.5% (7208 of 25309 functions)
  • lines: 47.1% (44241 of 93846 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
5ec6862 at 2024-03-21T12:06:40.156Z :recycle:

Please sign in to comment.