From ed31053cdb86becd9fc2383401d71f6d0f60c2e5 Mon Sep 17 00:00:00 2001 From: Lev Kokotov Date: Thu, 30 Mar 2023 17:35:32 -0700 Subject: [PATCH] Fix spec --- pgcat.toml | 8 ++++++-- src/auth.rs | 28 ++++++++++++++++++++++------ src/messages.rs | 4 +++- src/server.rs | 9 ++++++++- tests/ruby/auth_query_spec.rb | 6 +++--- tests/ruby/helpers/pgcat_process.rb | 1 - 6 files changed, 42 insertions(+), 14 deletions(-) diff --git a/pgcat.toml b/pgcat.toml index 3ef929d..efcc03f 100644 --- a/pgcat.toml +++ b/pgcat.toml @@ -122,6 +122,10 @@ idle_timeout = 40000 # Connect timeout can be overwritten in the pool connect_timeout = 3000 +# auth_query = "SELECT * FROM public.user_lookup('$1')" +# auth_query_user = "postgres" +# auth_query_password = "postgres" + # User configs are structured as pool..users. # This secion holds the credentials for users that may connect to this cluster [pools.sharded_db.users.0] @@ -130,8 +134,8 @@ username = "sharding_user" # Postgresql password password = "sharding_user" -# Passwords the client can use to connect. Useful for password rotations. -secrets = [ "secret_one", "secret_two" ] +# # Passwords the client can use to connect. Useful for password rotations. +# secrets = [ "secret_one", "secret_two" ] # Maximum number of server connections that can be established for this user # The maximum number of connection from a single Pgcat process to any database in the cluster diff --git a/src/auth.rs b/src/auth.rs index 32094ec..c89ca4e 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -26,13 +26,19 @@ where { let config = get_config(); + debug!("Fetching auth hash"); + if config.is_auth_query_configured() { let address = pool.address(0, 0); if let Some(apt) = AuthPassthrough::from_pool_settings(&pool.settings) { let hash = apt.fetch_hash(address).await?; + debug!("Auth query succeeded"); + return Ok(hash); } + } else { + debug!("Auth query not configured on pool"); } error_response( @@ -303,6 +309,7 @@ impl Md5 { if our_hash != password_hash { wrong_password(write, &self.username).await?; + Err(Error::ClientError(format!( "Invalid password {{ username: {:?}, pool_name: {:?}, application_name: {:?} }}", self.username, self.pool_name, self.application_name @@ -346,12 +353,19 @@ impl Md5 { ))); } + debug!("Using auth_query"); + // Fetch hash from server let hash = (*pool.auth_hash.read()).clone(); let hash = match hash { - Some(hash) => hash.clone(), + Some(hash) => { + debug!("Using existing hash: {}", hash); + hash.clone() + } None => { + debug!("Pool has no hash set, fetching new one"); + let hash = refetch_auth_hash( &pool, write, @@ -370,6 +384,8 @@ impl Md5 { // Compare hashes if our_hash != password_hash { + debug!("Pool auth query hash did not match, refetching"); + // Server hash maybe changed let hash = refetch_auth_hash( &pool, @@ -382,6 +398,8 @@ impl Md5 { let our_hash = md5_hash_second_pass(&hash, &self.salt); if our_hash != password_hash { + debug!("Auth query failed, passwords don't match"); + wrong_password(write, &self.username).await?; Err(Error::ClientError(format!( @@ -402,12 +420,10 @@ impl Md5 { Ok(()) } } else { - wrong_password(write, &self.username).await?; + validate_pool(write, pool.clone(), &self.username, &self.pool_name) + .await?; - Err(Error::ClientError(format!( - "Invalid password {{ username: {:?}, pool_name: {:?}, application_name: {:?} }}", - self.username, self.pool_name, self.application_name - ))) + Ok(()) } } } diff --git a/src/messages.rs b/src/messages.rs index c9a2e4a..4ceddec 100644 --- a/src/messages.rs +++ b/src/messages.rs @@ -1,7 +1,7 @@ /// Helper functions to send one-off protocol messages /// and handle TcpStream (TCP socket). use bytes::{Buf, BufMut, BytesMut}; -use log::error; +use log::{debug, error}; use md5::{Digest, Md5}; use socket2::{SockRef, TcpKeepalive}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; @@ -234,6 +234,8 @@ pub async fn md5_password_with_hash(stream: &mut S, hash: &str, salt: &[u8]) where S: tokio::io::AsyncWrite + std::marker::Unpin, { + debug!("Sending hash {} to server", hash); + let password = md5_hash_second_pass(hash, salt); let mut message = BytesMut::with_capacity(password.len() as usize + 5); diff --git a/src/server.rs b/src/server.rs index 37f0e0c..75901ac 100644 --- a/src/server.rs +++ b/src/server.rs @@ -755,7 +755,9 @@ impl Server { Arc::new(RwLock::new(None)), ) .await?; - debug!("Connected!, sending query."); + + debug!("Connected!, sending query: {}", query); + server.send(&simple_query(query)).await?; let mut message = server.recv().await?; @@ -764,6 +766,8 @@ impl Server { } async fn parse_query_message(message: &mut BytesMut) -> Result, Error> { + debug!("Parsing query message"); + let mut pair = Vec::::new(); match message::backend::Message::parse(message) { Ok(Some(message::backend::Message::RowDescription(_description))) => {} @@ -833,6 +837,9 @@ async fn parse_query_message(message: &mut BytesMut) -> Result, Erro } }; } + + debug!("Got auth hash successfully"); + Ok(pair) } diff --git a/tests/ruby/auth_query_spec.rb b/tests/ruby/auth_query_spec.rb index 1ac6216..47c955e 100644 --- a/tests/ruby/auth_query_spec.rb +++ b/tests/ruby/auth_query_spec.rb @@ -67,7 +67,7 @@ describe "Auth Query" do end context 'and with cleartext passwords not set' do - let(:config_user) { { 'username' => 'sharding_user', 'password' => 'sharding_user' } } + let(:config_user) { { 'username' => 'sharding_user' } } it 'it uses obtained passwords' do connection_string = processes.pgcat.connection_string("sharded_db", pg_user['username'], pg_user['password']) @@ -76,7 +76,7 @@ describe "Auth Query" do end it 'allows passwords to be changed without closing existing connections' do - pgconn = PG.connect(processes.pgcat.connection_string("sharded_db", pg_user['username'])) + pgconn = PG.connect(processes.pgcat.connection_string("sharded_db", pg_user['username'], pg_user['password'])) expect(pgconn.exec("SELECT 1 + 2")).not_to be_nil Helpers::AuthQuery.exec_in_instances(query: "ALTER USER #{pg_user['username']} WITH ENCRYPTED PASSWORD 'secret2';") expect(pgconn.exec("SELECT 1 + 4")).not_to be_nil @@ -84,7 +84,7 @@ describe "Auth Query" do end it 'allows passwords to be changed and that new password is needed when reconnecting' do - pgconn = PG.connect(processes.pgcat.connection_string("sharded_db", pg_user['username'])) + pgconn = PG.connect(processes.pgcat.connection_string("sharded_db", pg_user['username'], pg_user['password'])) expect(pgconn.exec("SELECT 1 + 2")).not_to be_nil Helpers::AuthQuery.exec_in_instances(query: "ALTER USER #{pg_user['username']} WITH ENCRYPTED PASSWORD 'secret2';") newconn = PG.connect(processes.pgcat.connection_string("sharded_db", pg_user['username'], 'secret2')) diff --git a/tests/ruby/helpers/pgcat_process.rb b/tests/ruby/helpers/pgcat_process.rb index 5489bc9..bc04281 100644 --- a/tests/ruby/helpers/pgcat_process.rb +++ b/tests/ruby/helpers/pgcat_process.rb @@ -78,7 +78,6 @@ class PgcatProcess 10.times do Process.kill 0, @pid PG::connect(connection_string || example_connection_string).close - return self rescue Errno::ESRCH raise StandardError, "Process #{@pid} died. #{logs}"