Skip to content

Commit

Permalink
Merge pull request #75 from ISibboI/72-switch-from-custom-secstr-to-s…
Browse files Browse the repository at this point in the history
…ecure-string

Change from secstr to secure-string.
  • Loading branch information
ISibboI authored Oct 25, 2023
2 parents e3a0c79 + b418a8c commit 477d93b
Show file tree
Hide file tree
Showing 11 changed files with 40 additions and 54 deletions.
16 changes: 4 additions & 12 deletions Cargo.lock

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

4 changes: 1 addition & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,4 @@ publish = false

[workspace.dependencies]
# sensitive data handling
secstr = { git = "https://codeberg.org/ISibboI/secstr.git", rev = "841f3d5c66b6057bcd19288d10206e1b1cbd76c1", features = [
"serde",
] }
secure-string = { version = "0.3.0", features = ["serde"] }
2 changes: 1 addition & 1 deletion backend/api_commands/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ serde = { version = "1.0.186", features = ["derive"] }
serde_json = "1.0.105"

# sensitive data handling
secstr.workspace = true
secure-string.workspace = true
9 changes: 3 additions & 6 deletions backend/api_commands/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
use secstr::SecVec;

pub type SecBytes = SecVec<u8>;

use secure_string::SecureBytes;
use serde::{Deserialize, Serialize};

#[derive(Serialize, Deserialize, Debug, Eq, PartialEq)]
pub struct CreateAccount {
pub username: String,
pub password: SecBytes,
pub password: SecureBytes,
}

#[derive(Serialize, Deserialize, Debug, Eq, PartialEq)]
pub struct Login {
pub username: String,
pub password: SecBytes,
pub password: SecureBytes,
}

#[cfg(test)]
Expand Down
3 changes: 3 additions & 0 deletions backend/integration-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,8 @@ serde_json = "1.0.105"
# web api
api_commands = { path = "../api_commands" }

# sensitive data handling
secure-string.workspace = true

# errors
anyhow = { version = "1.0.75", features = ["backtrace"] }
5 changes: 3 additions & 2 deletions backend/integration-tests/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use anyhow::{bail, Context};
use api_commands::{CreateAccount, Login, SecBytes};
use api_commands::{CreateAccount, Login};
use log::{debug, error, info};
use reqwest::StatusCode;
use secure_string::SecureBytes;
use simplelog::TermLogger;
use tokio::spawn;

Expand Down Expand Up @@ -151,7 +152,7 @@ async fn test_user_account_deletion() -> anyhow::Result<()> {

async fn test_login_logout() -> anyhow::Result<()> {
let client = HttpClient::new().await?;
let password = SecBytes::from("wald😀😀😀😀".to_owned());
let password = SecureBytes::from("wald😀😀😀😀".to_owned());

let response = client
.post(
Expand Down
2 changes: 1 addition & 1 deletion backend/rvoc-backend/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ argon2 = { version = "0.5.1", features = ["std"] }
rand = "0.8.5"

# sensitive data handling
secstr.workspace = true
secure-string.workspace = true

# web api
api_commands = { path = "../api_commands" }
Expand Down
11 changes: 5 additions & 6 deletions backend/rvoc-backend/src/cli/mod.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use std::sync::{atomic, Arc};

use api_commands::SecBytes;
use clap::Parser;
use diesel_async::RunQueryDsl;
use secstr::SecUtf8;
use secure_string::{SecureBytes, SecureString};
use tokio::io::{stdin, AsyncReadExt};
use tracing::{debug, info, instrument};

Expand Down Expand Up @@ -51,7 +50,7 @@ enum Cli {
/// The new password.
/// If not given, then it is read from stdin.
#[arg(short, long)]
password: Option<SecBytes>,
password: Option<SecureBytes>,
},
}

Expand Down Expand Up @@ -160,7 +159,7 @@ async fn expire_all_passwords(configuration: &Configuration) -> RVocResult<()> {
#[instrument(err, skip(configuration))]
async fn set_password(
username: String,
password: Option<SecBytes>,
password: Option<SecureBytes>,
configuration: &Configuration,
) -> RVocResult<()> {
let password = if let Some(password) = password {
Expand All @@ -172,11 +171,11 @@ async fn set_password(
source: Box::new(error),
}
})?;
SecBytes::from(password)
SecureBytes::from(password)
};

let password_hash = PasswordHash::new(password, configuration)?;
let password_hash = Option::<SecUtf8>::from(password_hash).expect(
let password_hash = Option::<SecureString>::from(password_hash).expect(
"creating a password hash from a password should never return an empty password hash",
);

Expand Down
13 changes: 5 additions & 8 deletions backend/rvoc-backend/src/configuration/mod.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
use std::{env::VarError, error::Error, net::SocketAddr, path::PathBuf, str::FromStr};

use crate::{
error::{RVocError, RVocResult, UserError},
SecBytes,
};
use crate::error::{RVocError, RVocResult, UserError};
use chrono::Duration;
use secstr::SecUtf8;
use secure_string::{SecureBytes, SecureString};

/// The configuration of the application.
#[derive(Debug, Clone)]
Expand All @@ -16,7 +13,7 @@ pub struct Configuration {
pub integration_test_mode: bool,

/// The url to access postgres.
pub postgres_url: SecUtf8,
pub postgres_url: SecureString,

/// The url to send opentelemetry to.
pub opentelemetry_url: Option<String>,
Expand Down Expand Up @@ -50,7 +47,7 @@ pub struct Configuration {
pub maximum_password_length: usize,

/// An additional salt that is shared between all passwords, but not stored in the database.
pub password_pepper: SecBytes,
pub password_pepper: SecureBytes,

/// The minimum memory parameter of the Argon2id password hash function.
/// See the [OWASP password storage cheat sheet](https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#argon2id)
Expand Down Expand Up @@ -252,7 +249,7 @@ impl Configuration {
}
}

pub fn verify_password_length(&self, password: &SecBytes) -> RVocResult<()> {
pub fn verify_password_length(&self, password: &SecureBytes) -> RVocResult<()> {
let unsecure_password = password.unsecure();
if unsecure_password.len() < self.minimum_password_length
|| unsecure_password.len() > self.maximum_password_length
Expand Down
3 changes: 0 additions & 3 deletions backend/rvoc-backend/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::error::RVocResult;
use crate::{configuration::Configuration, error::RVocError};
use cli::run_cli_command;
use secstr::SecVec;
use tracing::{info, instrument, Level};
use tracing_subscriber::filter::FilterFn;
use tracing_subscriber::Layer;
Expand All @@ -14,8 +13,6 @@ mod job_queue;
mod model;
mod web;

type SecBytes = SecVec<u8>;

#[instrument(err, skip(configuration))]
fn setup_tracing_subscriber(configuration: &Configuration) -> RVocResult<()> {
use opentelemetry::sdk::Resource;
Expand Down
26 changes: 14 additions & 12 deletions backend/rvoc-backend/src/model/user/password_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ use argon2::Argon2;
use argon2::PasswordHasher;
use password_hash::PasswordVerifier;
use password_hash::{rand_core::OsRng, SaltString};
use secstr::SecUtf8;
use secure_string::SecureBytes;
use secure_string::SecureString;

use crate::SecBytes;
use crate::{
configuration::Configuration,
error::{RVocError, RVocResult},
Expand All @@ -15,7 +15,7 @@ static HASH_ALGORITHM_VERSION: argon2::Version = argon2::Version::V0x13;

#[derive(Clone, Debug)]
pub struct PasswordHash {
argon_hash: Option<SecUtf8>,
argon_hash: Option<SecureString>,
}

#[must_use]
Expand All @@ -30,7 +30,7 @@ pub struct VerifyPasswordResult {

impl PasswordHash {
pub fn new(
plaintext_password: SecBytes,
plaintext_password: SecureBytes,
configuration: impl AsRef<Configuration>,
) -> RVocResult<Self> {
let configuration = configuration.as_ref();
Expand All @@ -55,7 +55,7 @@ impl PasswordHash {

pub fn verify(
&mut self,
plaintext_password: SecBytes,
plaintext_password: SecureBytes,
configuration: impl AsRef<Configuration>,
) -> RVocResult<VerifyPasswordResult> {
let Some(argon_hash) = &self.argon_hash else {
Expand Down Expand Up @@ -153,11 +153,11 @@ impl PasswordHash {

impl From<PasswordHash> for Option<String> {
fn from(value: PasswordHash) -> Self {
value.argon_hash.map(SecUtf8::into_unsecure)
value.argon_hash.map(SecureString::into_unsecure)
}
}

impl From<PasswordHash> for Option<SecUtf8> {
impl From<PasswordHash> for Option<SecureString> {
fn from(value: PasswordHash) -> Self {
value.argon_hash
}
Expand All @@ -171,8 +171,8 @@ impl From<Option<String>> for PasswordHash {
}
}

impl From<Option<SecUtf8>> for PasswordHash {
fn from(value: Option<SecUtf8>) -> Self {
impl From<Option<SecureString>> for PasswordHash {
fn from(value: Option<SecureString>) -> Self {
Self { argon_hash: value }
}
}
Expand All @@ -187,8 +187,10 @@ impl From<String> for PasswordHash {

#[cfg(test)]
mod tests {
use secure_string::SecureBytes;

use super::{PasswordHash, VerifyPasswordResult, HASH_ALGORITHM, HASH_ALGORITHM_VERSION};
use crate::{configuration::Configuration, SecBytes};
use crate::configuration::Configuration;

#[test]
fn test_password_check() {
Expand All @@ -201,7 +203,7 @@ mod tests {
configuration.build_argon2_parameters().unwrap()
);

let password = SecBytes::from("mypassword");
let password = SecureBytes::from("mypassword");
let mut password_hash = PasswordHash::new(password.clone(), &configuration).unwrap();

let verify_password_result = password_hash.verify(password.clone(), &configuration);
Expand Down Expand Up @@ -245,7 +247,7 @@ mod tests {
configuration.build_argon2_parameters().unwrap()
);

let password = SecBytes::from("mypassword");
let password = SecureBytes::from("mypassword");
let mut password_hash = PasswordHash::new(password.clone(), &configuration).unwrap();

let verify_password_result = password_hash.verify(password.clone(), &configuration);
Expand Down

0 comments on commit 477d93b

Please sign in to comment.