Skip to content

Commit

Permalink
Merge pull request #174 hide sensitive strings behind a SecretString …
Browse files Browse the repository at this point in the history
…from uslon/master
  • Loading branch information
rekby authored Mar 6, 2024
2 parents 77f0d86 + a74a284 commit ed92218
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 22 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/rust-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
strategy:
fail-fast: false
matrix:
rust_version: [ "1.56.0", "1.75.0" ]
rust_version: [ "1.60.0", "1.75.0" ]

services:
ydb:
Expand Down Expand Up @@ -63,6 +63,6 @@ jobs:
run: cargo test --verbose --workspace -- --include-ignored

- name: Linter
if: matrix.rust_version != '1.56.0'
if: matrix.rust_version != '1.60.0'
run: |
cargo clippy --workspace --all-targets -- -D warnings
3 changes: 2 additions & 1 deletion ydb/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ edition = "2021"
license = "Apache-2.0"
description = "Crate contains generated low-level grpc code from YDB API protobuf, used as base for ydb crate"
repository = "https://github.com/ydb-platform/ydb-rs-sdk/tree/master/ydb"
rust-version = "1.56"
rust-version = "1.60"

[features]
force-exhaustive-all=[] # The feature disable all non_exhaustive attributes in ydb public interface.
Expand All @@ -27,6 +27,7 @@ prost-types = "0.11.2"
pbjson-types = "0.5.1"
rand = "0.8"
reqwest = {version="0.11", features = ["blocking", "json","rustls"], default-features = false}
secrecy = "0.4.1"
serde={version="1.0", features=["derive"]}
serde_json="1.0"
strum = { version = "0.21", features = ["derive"] }
Expand Down
15 changes: 9 additions & 6 deletions ydb/src/auth_test.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use secrecy::ExposeSecret;
use tracing::trace;
use tracing_test::traced_test;

Expand All @@ -15,10 +16,11 @@ fn auth_success_test() -> YdbResult<()> {
let database = uri.path().to_string();
let up_auth = StaticCredentialsAuth::new("root".to_string(), "1234".to_string(), uri, database);

let token_str = up_auth.create_token()?.token;
let token_sec = up_auth.create_token()?.token;
let raw_token = token_sec.expose_secret();

trace!("got token: `{}'", token_str);
if token_str.is_empty() {
trace!("got token: `{}'", raw_token);
if raw_token.is_empty() {
panic!("got the empty token on the presumably successful auth request");
}

Expand All @@ -34,14 +36,15 @@ async fn auth_async_success_test() -> YdbResult<()> {
let database = uri.path().to_string();
let up_auth = StaticCredentialsAuth::new("root".to_string(), "1234".to_string(), uri, database);

let token_str = std::thread::spawn(move || up_auth.create_token())
let token_sec = std::thread::spawn(move || up_auth.create_token())
.join()
.unwrap()
.unwrap()
.token;
let raw_token = token_sec.expose_secret();

trace!("got token: `{}'", token_str);
if token_str.is_empty() {
trace!("got token: `{}'", raw_token);
if raw_token.is_empty() {
panic!("got the empty token on the presumably successful auth request");
}

Expand Down
3 changes: 2 additions & 1 deletion ydb/src/client_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::credentials::CredentialsRef;
use crate::errors::YdbResult;
use crate::pub_traits::TokenInfo;
use crate::waiter::Waiter;
use secrecy::SecretString;
use std::sync::{Arc, Mutex, RwLock};
use std::time::Instant;
use tokio::sync::watch;
Expand Down Expand Up @@ -40,7 +41,7 @@ impl TokenCache {
Ok(token_cache)
}

pub(crate) fn token(&self) -> String {
pub(crate) fn token(&self) -> SecretString {
let now = Instant::now();

let read = self.0.read().unwrap();
Expand Down
13 changes: 7 additions & 6 deletions ydb/src/credentials.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::grpc_wrapper::runtime_interceptors::MultiInterceptor;
use crate::load_balancer::{SharedLoadBalancer, StaticLoadBalancer};
use crate::pub_traits::{Credentials, TokenInfo};
use http::Uri;
use secrecy::{ExposeSecret, SecretString};
use serde::Deserialize;
use std::fmt::Debug;
use std::ops::Add;
Expand Down Expand Up @@ -125,7 +126,7 @@ impl Credentials for CommandLineYcToken {
fn debug_string(&self) -> String {
let token_describe: String = match self.create_token() {
Ok(token_info) => {
let token = token_info.token;
let token = token_info.token.expose_secret();
let desc: String = if token.len() > 20 {
format!(
"{}..{}",
Expand Down Expand Up @@ -238,7 +239,7 @@ impl Credentials for GCEMetadata {

pub struct StaticCredentialsAuth {
username: String,
password: String,
password: SecretString,
database: String,
endpoint: Uri,
}
Expand All @@ -261,17 +262,17 @@ impl StaticCredentialsAuth {
let raw_request = RawLoginRequest {
operation_params: TimeoutSettings::default().operation_params(),
user: self.username.clone(),
password: self.password.clone(),
password: self.password.expose_secret().clone(),
};

let raw_result = auth_client.login(raw_request).await?;
Ok(raw_result.token)
let raw_response = auth_client.login(raw_request).await?;
Ok(raw_response.token)
}

pub fn new(username: String, password: String, endpoint: Uri, database: String) -> Self {
Self {
username,
password,
password: SecretString::new(password),
database,
endpoint,
}
Expand Down
8 changes: 5 additions & 3 deletions ydb/src/grpc_wrapper/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::grpc_wrapper::runtime_interceptors::{
GrpcInterceptor, InterceptorError, InterceptorRequest, InterceptorResult, RequestMetadata,
};
use http::HeaderValue;
use secrecy::ExposeSecret;

pub(crate) struct AuthGrpcInterceptor {
db_name: HeaderValue,
Expand Down Expand Up @@ -34,9 +35,10 @@ impl GrpcInterceptor for AuthGrpcInterceptor {
_metadata: &mut RequestMetadata,
mut req: InterceptorRequest,
) -> InterceptorResult<InterceptorRequest> {
let token = self.token_cache.token();
let token = HeaderValue::from_str(token.as_str()).map_err(|err| {
InterceptorError::custom(format!("received bad token (len={}): {}", token.len(), err))
let token_secret = self.token_cache.token();
let token_string = token_secret.expose_secret();
let token = HeaderValue::from_str(token_string.as_str()).map_err(|err| {
InterceptorError::custom(format!("received bad token (len={}): {}", token_string.len(), err))
})?;

req.headers_mut()
Expand Down
6 changes: 3 additions & 3 deletions ydb/src/pub_traits.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

use crate::errors::YdbResult;
use secrecy::SecretString;
use std::fmt::{Debug, Formatter};
use std::ops::Add;
use std::time::{Duration, Instant};
Expand All @@ -8,14 +8,14 @@ pub(crate) const DEFAULT_TOKEN_RENEW_INTERVAL: Duration = Duration::from_secs(36

#[derive(Debug, Clone)]
pub struct TokenInfo {
pub(crate) token: String,
pub(crate) token: SecretString,
pub(crate) next_renew: Instant,
}

impl TokenInfo {
pub(crate) fn token(token: String) -> Self {
Self {
token,
token: SecretString::new(token),
next_renew: Instant::now().add(DEFAULT_TOKEN_RENEW_INTERVAL),
}
}
Expand Down

0 comments on commit ed92218

Please sign in to comment.