Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix!: standardize gRPC authentication and mitigate DoS #5936

Merged
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
107 changes: 45 additions & 62 deletions applications/minotari_app_grpc/src/authentication/basic_auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,9 @@ use tari_utilities::{ByteArray, SafePassword};
use tonic::metadata::{errors::InvalidMetadataValue, Ascii, MetadataValue};
use zeroize::{Zeroize, Zeroizing};

use crate::authentication::salted_password::create_salted_hashed_password;

const MAX_USERNAME_LEN: usize = 256;

/// Implements [RFC 2617](https://www.ietf.org/rfc/rfc2617.txt#:~:text=The%20%22basic%22%20authentication%20scheme%20is,other%20realms%20on%20that%20server.)
/// Represents the username and password contained within a Authenticate header.
/// Implements [RFC 2617](https://www.ietf.org/rfc/rfc2617.txt) by allowing authentication of provided credentials
#[derive(Debug)]
pub struct BasicAuthCredentials {
/// The username bytes length
Expand Down Expand Up @@ -82,42 +79,29 @@ impl BasicAuthCredentials {
})
}

/// Creates a `Credentials` instance from a base64 `String`
/// which must encode user credentials as `username:password`
pub fn decode(auth_header_value: &str) -> Result<Self, BasicAuthError> {
let decoded = base64::decode(auth_header_value)?;
let as_utf8 = Zeroizing::new(String::from_utf8(decoded)?);

if let Some((user_name, password)) = as_utf8.split_once(':') {
let hashed_password = create_salted_hashed_password(password.as_bytes())?;
let credentials = Self::new(user_name.into(), hashed_password.to_string().into())?;
return Ok(credentials);
}

Err(BasicAuthError::InvalidAuthorizationHeader)
}

/// Creates a `Credentials` instance from an HTTP Authorization header
/// which schema is a valid `Basic` HTTP Authorization Schema.
pub fn from_header(auth_header: &str) -> Result<BasicAuthCredentials, BasicAuthError> {
// check if its a valid basic auth header
/// Parses the contents of an HTTP Authorization (Basic) header into a username and password
/// These can be used later to validate against `BasicAuthCredentials`
/// The input must be of the form `Basic base64(username:password)`
pub fn parse_header(auth_header: &str) -> Result<(String, SafePassword), BasicAuthError> {
// Check that the authentication type is `Basic`
let (auth_type, encoded_credentials) = auth_header
.split_once(' ')
.ok_or(BasicAuthError::InvalidAuthorizationHeader)?;

if encoded_credentials.contains(' ') {
// Invalid authorization token received
return Err(BasicAuthError::InvalidAuthorizationHeader);
}

// Check the provided authorization header
// to be a "Basic" authorization header
if auth_type.to_lowercase() != "basic" {
return Err(BasicAuthError::InvalidScheme(auth_type.to_string()));
}

let credentials = BasicAuthCredentials::decode(encoded_credentials)?;
Ok(credentials)
// Decode the credentials using base64
let decoded = base64::decode(encoded_credentials)?;
let as_utf8 = Zeroizing::new(String::from_utf8(decoded)?);

// Parse the username and password, which must be separated by a colon
if let Some((user_name, password)) = as_utf8.split_once(':') {
return Ok((user_name.into(), password.into()));
}

Err(BasicAuthError::InvalidAuthorizationHeader)
}

// This function provides a constant time comparison of the given username with the registered username.
Expand Down Expand Up @@ -150,7 +134,7 @@ impl BasicAuthCredentials {
/// do the same amount of work irrespective if the username or password is correct or not. This is to prevent timing
/// attacks. Also, no distinction is made between a non-existent username or an incorrect password in the error
/// that is returned.
pub fn constant_time_validate(&self, username: &str, password: &[u8]) -> Result<(), BasicAuthError> {
pub fn constant_time_validate(&self, username: &str, password: &SafePassword) -> Result<(), BasicAuthError> {
let valid_username = self.constant_time_verify_username(username);

// These bytes can leak if the password is not utf-8, but since argon encoding is utf-8 the given
Expand All @@ -159,7 +143,9 @@ impl BasicAuthCredentials {
let str_password = Zeroizing::new(String::from_utf8(bytes)?);
let header_password = PasswordHash::parse(&str_password, Encoding::B64)?;
let valid_password = Choice::from(u8::from(
Argon2::default().verify_password(password, &header_password).is_ok(),
Argon2::default()
.verify_password(password.reveal(), &header_password)
.is_ok(),
));

// The use of `Choice` logic here is by design to hide the boolean logic from compiler optimizations.
Expand Down Expand Up @@ -218,26 +204,20 @@ mod tests {

#[test]
fn it_decodes_from_well_formed_header() {
let credentials = BasicAuthCredentials::from_header("Basic YWRtaW46c2VjcmV0").unwrap();
assert_eq!(credentials.user_name_bytes_length, "admin".as_bytes().len());
let bytes = "admin".as_bytes();
let mut user_name_bytes = [0u8; MAX_USERNAME_LEN];
user_name_bytes[0..bytes.len()].clone_from_slice(bytes);
user_name_bytes[bytes.len()..MAX_USERNAME_LEN]
.clone_from_slice(&credentials.random_bytes[bytes.len()..MAX_USERNAME_LEN]);
assert_eq!(credentials.user_name_bytes, user_name_bytes);
assert!(credentials.constant_time_validate("admin", b"secret").is_ok());
let (username, password) = BasicAuthCredentials::parse_header("Basic YWRtaW46c2VjcmV0").unwrap();
assert_eq!(username, "admin".to_string());
assert_eq!(password.reveal(), b"secret");
}

#[test]
fn it_rejects_header_without_basic_scheme() {
let err = BasicAuthCredentials::from_header(" YWRtaW46c2VjcmV0").unwrap_err();
let err = BasicAuthCredentials::parse_header(" YWRtaW46c2VjcmV0").unwrap_err();
if let BasicAuthError::InvalidScheme(s) = err {
assert_eq!(s, "");
} else {
panic!("Unexpected error: {:?}", err);
};
let err = BasicAuthCredentials::from_header("Cookie YWRtaW46c2VjcmV0").unwrap_err();
let err = BasicAuthCredentials::parse_header("Cookie YWRtaW46c2VjcmV0").unwrap_err();
if let BasicAuthError::InvalidScheme(s) = err {
assert_eq!(s, "Cookie");
} else {
Expand All @@ -264,10 +244,14 @@ mod tests {
// Typical username
let credentials =
BasicAuthCredentials::new("admin".to_string(), hashed_password.to_string().into()).unwrap();
credentials.constant_time_validate("admin", b"secret").unwrap();
credentials
.constant_time_validate("admin", &SafePassword::from("secret".to_string()))
.unwrap();
// Empty username is also fine
let credentials = BasicAuthCredentials::new("".to_string(), hashed_password.to_string().into()).unwrap();
credentials.constant_time_validate("", b"secret").unwrap();
credentials
.constant_time_validate("", &SafePassword::from("secret".to_string()))
.unwrap();
}

#[test]
Expand All @@ -286,16 +270,20 @@ mod tests {
BasicAuthCredentials::new("admin".to_string(), hashed_password.to_string().into()).unwrap();

// Wrong password
let err = credentials.constant_time_validate("admin", b"bruteforce").unwrap_err();
let err = credentials
.constant_time_validate("admin", &SafePassword::from("bruteforce".to_string()))
.unwrap_err();
assert!(matches!(err, BasicAuthError::InvalidUsernameOrPassword));

// Wrong username
let err = credentials.constant_time_validate("wrong_user", b"secret").unwrap_err();
let err = credentials
.constant_time_validate("wrong_user", &SafePassword::from("secret".to_string()))
.unwrap_err();
assert!(matches!(err, BasicAuthError::InvalidUsernameOrPassword));

// Wrong username and password
let err = credentials
.constant_time_validate("wrong_user", b"bruteforce")
.constant_time_validate("wrong_user", &SafePassword::from("bruteforce".to_string()))
.unwrap_err();
assert!(matches!(err, BasicAuthError::InvalidUsernameOrPassword));
}
Expand Down Expand Up @@ -561,21 +549,22 @@ mod tests {

let start = Instant::now();
for short in &short_usernames {
let res = credentials.constant_time_validate(short, b"bruteforce");
let res = credentials.constant_time_validate(short, &SafePassword::from("bruteforce".to_string()));
assert!(res.is_err());
}
let time_taken_1 = start.elapsed().as_millis();

let start = Instant::now();
for long in &long_usernames {
let res = credentials.constant_time_validate(long, b"bruteforce");
let res = credentials.constant_time_validate(long, &SafePassword::from("bruteforce".to_string()));
assert!(res.is_err());
}
let time_taken_2 = start.elapsed().as_millis();

let start = Instant::now();
for _ in 0..COUNTS {
let res = credentials.constant_time_validate(username_actual, b"secret");
let res =
credentials.constant_time_validate(username_actual, &SafePassword::from("secret".to_string()));
assert!(res.is_ok());
}
let time_taken_3 = start.elapsed().as_millis();
Expand Down Expand Up @@ -624,15 +613,9 @@ mod tests {
#[test]
fn it_generates_a_valid_header() {
let header = BasicAuthCredentials::generate_header("admin", b"secret").unwrap();
let cred = BasicAuthCredentials::from_header(header.to_str().unwrap()).unwrap();
assert_eq!(cred.user_name_bytes_length, "admin".as_bytes().len());
let bytes = "admin".as_bytes();
let mut user_name_bytes = [0u8; MAX_USERNAME_LEN];
user_name_bytes[0..bytes.len()].clone_from_slice(bytes);
user_name_bytes[bytes.len()..MAX_USERNAME_LEN]
.clone_from_slice(&cred.random_bytes[bytes.len()..MAX_USERNAME_LEN]);
assert_eq!(cred.user_name_bytes, user_name_bytes);
assert!(cred.constant_time_validate("admin", b"secret").is_ok());
let (username, password) = BasicAuthCredentials::parse_header(header.to_str().unwrap()).unwrap();
assert_eq!(username, "admin".to_string());
assert_eq!(password.reveal(), b"secret");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,35 +22,53 @@

use log::*;
use tari_common_types::grpc_authentication::GrpcAuthentication;
use tari_utilities::SafePassword;
use tonic::{codegen::http::header::AUTHORIZATION, service::Interceptor, Request, Status};

use crate::authentication::BasicAuthCredentials;
use crate::authentication::{salted_password::create_salted_hashed_password, BasicAuthCredentials};

const LOG_TARGET: &str = "applications::minotari_app_grpc::authentication";

#[derive(Debug)]
pub struct ServerAuthenticationInterceptor {
auth: GrpcAuthentication,
auth: GrpcAuthentication, // this contains a hashed PHC password in the case of basic authentication
}

impl ServerAuthenticationInterceptor {
pub fn new(auth: GrpcAuthentication) -> Self {
Self { auth }
pub fn new(auth: GrpcAuthentication) -> Option<Self> {
// The server password needs to be hashed for later client verification
let processed_auth = match auth {
GrpcAuthentication::None => auth,
GrpcAuthentication::Basic { username, password } => GrpcAuthentication::Basic {
username,
password: create_salted_hashed_password(password.reveal()).ok()?.as_str().into(),
},
};

Some(Self { auth: processed_auth })
}

fn handle_basic_auth(
&self,
req: Request<()>,
valid_username: &str,
valid_password: &[u8],
valid_phc_password: &SafePassword,
) -> Result<Request<()>, Status> {
match req.metadata().get(AUTHORIZATION.as_str()) {
Some(t) => {
let val = t.to_str().map_err(unauthenticated)?;
let credentials = BasicAuthCredentials::from_header(val).map_err(unauthenticated)?;
credentials
.constant_time_validate(valid_username, valid_password)
// Parse the provided header
let header = t.to_str().map_err(unauthenticated)?;
let (header_username, header_password) =
BasicAuthCredentials::parse_header(header).map_err(unauthenticated)?;

// Validate the header credentials against the valid credentials
let valid_credentials =
BasicAuthCredentials::new(valid_username.to_owned(), valid_phc_password.clone())
.map_err(unauthenticated)?;
valid_credentials
.constant_time_validate(&header_username, &header_password)
.map_err(unauthenticated)?;

Ok(req)
},
_ => Err(unauthenticated("Missing authorization header")),
Expand All @@ -64,8 +82,8 @@ impl Interceptor for ServerAuthenticationInterceptor {
GrpcAuthentication::None => Ok(request),
GrpcAuthentication::Basic {
username,
password: hashed_password,
} => self.handle_basic_auth(request, username, hashed_password.reveal()),
password: phc_password,
} => self.handle_basic_auth(request, username, phc_password),
}
}
}
Expand Down
31 changes: 0 additions & 31 deletions applications/minotari_console_wallet/src/automation/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ use chrono::{DateTime, Utc};
use digest::Digest;
use futures::FutureExt;
use log::*;
use minotari_app_grpc::authentication::salted_password::create_salted_hashed_password;
use minotari_wallet::{
connectivity_service::WalletConnectivityInterface,
output_manager_service::{handle::OutputManagerHandle, UtxoSelectionCriteria},
Expand Down Expand Up @@ -962,36 +961,6 @@ pub async fn command_runner(
eprintln!("RevalidateWalletDb error! {}", e);
}
},
HashGrpcPassword(args) => {
match config
.grpc_authentication
.username_password()
.ok_or_else(|| CommandError::General("GRPC basic auth is not configured".to_string()))
{
Ok((username, password)) => {
match create_salted_hashed_password(password.reveal())
.map_err(|e| CommandError::General(e.to_string()))
{
Ok(hashed_password) => {
if args.short {
println!("{}", *hashed_password);
} else {
println!("Your hashed password is:");
println!("{}", *hashed_password);
println!();
println!(
"Use HTTP basic auth with username '{}' and the hashed password to make GRPC \
requests",
username
);
}
},
Err(e) => eprintln!("HashGrpcPassword error! {}", e),
}
},
Err(e) => eprintln!("HashGrpcPassword error! {}", e),
}
},
RegisterValidatorNode(args) => {
let tx_id = register_validator_node(
args.amount,
Expand Down
7 changes: 0 additions & 7 deletions applications/minotari_console_wallet/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ pub enum CliCommands {
FinaliseShaAtomicSwap(FinaliseShaAtomicSwapArgs),
ClaimShaAtomicSwapRefund(ClaimShaAtomicSwapRefundArgs),
RevalidateWalletDb,
HashGrpcPassword(HashPasswordArgs),
RegisterValidatorNode(RegisterValidatorNodeArgs),
}

Expand Down Expand Up @@ -281,12 +280,6 @@ pub struct ClaimShaAtomicSwapRefundArgs {
pub message: String,
}

#[derive(Debug, Args, Clone)]
pub struct HashPasswordArgs {
/// If true, only output the hashed password and the salted password. Otherwise a usage explanation is output.
pub short: bool,
}

#[derive(Debug, Args, Clone)]
pub struct RegisterValidatorNodeArgs {
pub amount: MicroMinotari,
Expand Down
4 changes: 2 additions & 2 deletions applications/minotari_console_wallet/src/wallet_modes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,8 @@ async fn run_grpc(

info!(target: LOG_TARGET, "Starting GRPC on {}", grpc_listener_addr);
let address = multiaddr_to_socketaddr(&grpc_listener_addr).map_err(|e| e.to_string())?;
let auth = ServerAuthenticationInterceptor::new(auth_config);
let auth = ServerAuthenticationInterceptor::new(auth_config)
.ok_or("Unable to prepare server gRPC authentication".to_string())?;
let service = minotari_app_grpc::tari_rpc::wallet_server::WalletServer::with_interceptor(grpc, auth);

Server::builder()
Expand Down Expand Up @@ -481,7 +482,6 @@ mod test {
CliCommands::FinaliseShaAtomicSwap(_) => {},
CliCommands::ClaimShaAtomicSwapRefund(_) => {},
CliCommands::RevalidateWalletDb => {},
CliCommands::HashGrpcPassword(_) => {},
CliCommands::RegisterValidatorNode(_) => {},
}
}
Expand Down
Loading
Loading