Skip to content

Commit

Permalink
Fixes #23159: Use the secrecy crates for wrapping passwords
Browse files Browse the repository at this point in the history
  • Loading branch information
amousset committed Jul 30, 2023
1 parent 97a2e11 commit b888524
Show file tree
Hide file tree
Showing 9 changed files with 117 additions and 65 deletions.
17 changes: 17 additions & 0 deletions relay/sources/relayd/Cargo.lock

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

1 change: 1 addition & 0 deletions relay/sources/relayd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ prometheus = { version = "0.13", default-features = false, features = ["process"
regex = "1"
# Use openssl for TLS to be consistent
reqwest = { version = "0.11.1", default-features = false, features = ["stream", "blocking", "native-tls-vendored"] }
secrecy = { version = "0.8", features = ["serde"] }
serde = { version = "1", features = ["derive"] }
serde_json = "1"
sha2 = "0.10"
Expand Down
32 changes: 1 addition & 31 deletions relay/sources/relayd/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,41 +5,11 @@ pub mod cli;
pub mod logging;
pub mod main;

use std::{fmt, path::Path};
use std::path::Path;

use anyhow::Error;
use logging::LogConfig;
use main::Configuration;
use serde::Deserialize;

/// Allows hiding a value in logs
#[derive(Deserialize, PartialEq, Eq, Clone, Default)]
#[serde(transparent)]
pub struct Secret {
value: String,
}

impl<'a> Secret {
pub fn new(value: String) -> Self {
Self { value }
}

pub fn value(&'a self) -> &'a str {
&self.value
}
}

impl fmt::Display for Secret {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "******")
}
}

impl fmt::Debug for Secret {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "\"******\"")
}
}

type Warnings = Vec<Error>;

Expand Down
57 changes: 39 additions & 18 deletions relay/sources/relayd/src/configuration/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ use std::{
};

use anyhow::{anyhow, Context, Error};
use secrecy::SecretString;
use serde::{
de::{Deserializer, Error as SerdeError, Unexpected, Visitor},
Deserialize,
};
use tracing::{debug, warn};

use crate::{configuration::Secret, data::node::NodeId};
use crate::data::node::NodeId;

pub type BaseDirectory = PathBuf;
pub type WatchedDirectory = PathBuf;
Expand Down Expand Up @@ -554,17 +555,25 @@ pub struct OutputConfig {
pub upstream: UpstreamConfig,
}

#[derive(Deserialize, Debug, PartialEq, Eq, Clone)]
#[derive(Deserialize, Debug, Clone)]
pub struct DatabaseConfig {
/// URL without the password
#[serde(default = "DatabaseConfig::default_url")]
pub url: String,
/// When the section is there, password is mandatory
pub password: Secret,
pub password: SecretString,
#[serde(default = "DatabaseConfig::default_max_pool_size")]
pub max_pool_size: u32,
}

impl PartialEq for DatabaseConfig {
fn eq(&self, other: &Self) -> bool {
self.url == other.url && self.max_pool_size == other.max_pool_size
}
}

impl Eq for DatabaseConfig {}

impl DatabaseConfig {
fn default_url() -> String {
"postgres://rudder@127.0.0.1/rudder".to_string()
Expand All @@ -579,13 +588,13 @@ impl Default for DatabaseConfig {
fn default() -> Self {
Self {
url: Self::default_url(),
password: Default::default(),
password: SecretString::new("".to_string()),
max_pool_size: Self::default_max_pool_size(),
}
}
}

#[derive(Deserialize, Debug, PartialEq, Eq, Clone)]
#[derive(Deserialize, Debug, Clone)]
pub struct UpstreamConfig {
/// DEPRECATED: use host and global.https_port
#[serde(default)]
Expand All @@ -596,10 +605,10 @@ pub struct UpstreamConfig {
#[serde(default = "UpstreamConfig::default_user")]
pub user: String,
/// When the section is there, password is mandatory
pub password: Secret,
pub password: SecretString,
/// Default password, to be used for new inventories
#[serde(default = "UpstreamConfig::default_default_password")]
pub default_password: Secret,
pub default_password: SecretString,
#[serde(default = "UpstreamConfig::default_verify_certificates")]
/// Allows to completely disable certificate validation.
///
Expand All @@ -610,12 +619,24 @@ pub struct UpstreamConfig {
pub verify_certificates: bool,
/// Allows specifying the root certificate path
/// Used for our Rudder PKI
/// Not used if verification model is not `Rudder`.
/// Not used if the verification model is not `Rudder`.
#[serde(default = "UpstreamConfig::default_server_certificate_file")]
pub server_certificate_file: PathBuf,
// TODO timeout?
}

impl PartialEq for UpstreamConfig {
fn eq(&self, other: &Self) -> bool {
self.url == other.url
&& self.host == other.host
&& self.user == other.user
&& self.verify_certificates == other.verify_certificates
&& self.server_certificate_file == other.server_certificate_file
}
}

impl Eq for UpstreamConfig {}

impl UpstreamConfig {
fn default_user() -> String {
"rudder".to_string()
Expand All @@ -625,8 +646,8 @@ impl UpstreamConfig {
true
}

fn default_default_password() -> Secret {
Secret::new("rudder".into())
fn default_default_password() -> SecretString {
SecretString::new("rudder".into())
}

fn default_server_certificate_file() -> PathBuf {
Expand All @@ -640,8 +661,8 @@ impl Default for UpstreamConfig {
url: Default::default(),
host: Default::default(),
user: Self::default_user(),
password: Default::default(),
default_password: Default::default(),
password: SecretString::new("".to_string()),
default_password: SecretString::new("".to_string()),
verify_certificates: Self::default_verify_certificates(),
server_certificate_file: Self::default_server_certificate_file(),
}
Expand Down Expand Up @@ -732,14 +753,14 @@ mod tests {
url: None,
host: "".to_string(),
user: "rudder".to_string(),
password: Secret::new("".to_string()),
default_password: Secret::new("".to_string()),
password: SecretString::new("".to_string()),
default_password: SecretString::new("".to_string()),
verify_certificates: true,
server_certificate_file: PathBuf::from("/var/rudder/lib/ssl/policy_server.pem"),
},
database: DatabaseConfig {
url: "postgres://rudder@127.0.0.1/rudder".to_string(),
password: Secret::new("".to_string()),
password: SecretString::new("".to_string()),
max_pool_size: 10,
},
},
Expand Down Expand Up @@ -831,16 +852,16 @@ mod tests {
url: None,
host: "rudder.example.com".to_string(),
user: "rudder".to_string(),
password: Secret::new("password".to_string()),
default_password: Secret::new("rudder".to_string()),
password: SecretString::new("password".to_string()),
default_password: SecretString::new("rudder".to_string()),
verify_certificates: true,
server_certificate_file: PathBuf::from(
"tests/files/keys/e745a140-40bc-4b86-b6dc-084488fc906b.cert",
),
},
database: DatabaseConfig {
url: "postgres://rudderreports@postgres/rudder".to_string(),
password: Secret::new("PASSWORD".to_string()),
password: SecretString::new("PASSWORD".to_string()),
max_pool_size: 5,
},
},
Expand Down
3 changes: 2 additions & 1 deletion relay/sources/relayd/src/output/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use diesel::{
prelude::*,
r2d2::{ConnectionManager, Pool},
};
use secrecy::ExposeSecret;
use tracing::{debug, error, instrument, trace};

use crate::{
Expand Down Expand Up @@ -65,7 +66,7 @@ pub fn pg_pool(configuration: &DatabaseConfig) -> Result<PgPool, Error> {
} else {
"?"
},
urlencoding::encode(configuration.password.value())
urlencoding::encode(configuration.password.expose_secret())
));
Ok(Pool::builder()
.max_size(configuration.max_pool_size)
Expand Down
7 changes: 4 additions & 3 deletions relay/sources/relayd/src/output/upstream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
use std::{path::PathBuf, sync::Arc};

use reqwest::Body;
use secrecy::{ExposeSecret, SecretString};
use tracing::{debug, instrument};

use crate::{configuration::Secret, processing::inventory::InventoryType, Error, JobConfig};
use crate::{processing::inventory::InventoryType, Error, JobConfig};

#[instrument(name = "upstream", level = "debug", skip(job_config))]
pub async fn send_report(job_config: Arc<JobConfig>, path: PathBuf) -> Result<(), Error> {
Expand Down Expand Up @@ -44,7 +45,7 @@ async fn forward_file(
job_config: Arc<JobConfig>,
endpoint: &str,
path: PathBuf,
password: Secret,
password: SecretString,
) -> Result<(), Error> {
let content = tokio::fs::read(path.clone()).await?;

Expand All @@ -59,7 +60,7 @@ async fn forward_file(
))
.basic_auth(
&job_config.cfg.output.upstream.user,
Some(&password.value()),
Some(&password.expose_secret()),
)
.body(Body::from(content))
.send()
Expand Down
12 changes: 12 additions & 0 deletions relay/sources/relayd/supply-chain/audits.toml
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,12 @@ user-id = 2915 # Amanieu d'Antras (Amanieu)
start = "2020-02-16"
end = "2024-06-27"

[[trusted.secrecy]]
criteria = "safe-to-deploy"
user-id = 19631 # Tony Arcieri (iqlusion) (tony-iqlusion)
start = "2019-05-23"
end = "2024-07-30"

[[trusted.serde]]
criteria = "safe-to-deploy"
user-id = 3618 # David Tolnay (dtolnay)
Expand Down Expand Up @@ -566,3 +572,9 @@ criteria = "safe-to-deploy"
user-id = 6743 # Ed Page (epage)
start = "2023-02-22"
end = "2024-06-27"

[[trusted.zeroize]]
criteria = "safe-to-deploy"
user-id = 267 # Tony Arcieri (tarcieri)
start = "2021-11-05"
end = "2024-07-30"
12 changes: 0 additions & 12 deletions relay/sources/relayd/supply-chain/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ url = "https://raw.githubusercontent.com/divviup/libprio-rs/main/supply-chain/au
[imports.mozilla]
url = "https://raw.githubusercontent.com/mozilla/supply-chain/main/audits.toml"

[[exemptions.adler]]
version = "1.0.2"
criteria = "safe-to-deploy"

[[exemptions.android-tzdata]]
version = "0.1.1"
criteria = "safe-to-deploy"
Expand Down Expand Up @@ -183,10 +179,6 @@ criteria = "safe-to-deploy"
version = "0.2.1"
criteria = "safe-to-deploy"

[[exemptions.miniz_oxide]]
version = "0.7.1"
criteria = "safe-to-deploy"

[[exemptions.nom]]
version = "7.1.3"
criteria = "safe-to-deploy"
Expand Down Expand Up @@ -215,10 +207,6 @@ criteria = "safe-to-deploy"
version = "1.1.0"
criteria = "safe-to-deploy"

[[exemptions.pin-project-lite]]
version = "0.2.9"
criteria = "safe-to-deploy"

[[exemptions.pkg-config]]
version = "0.3.27"
criteria = "safe-to-deploy"
Expand Down
Loading

0 comments on commit b888524

Please sign in to comment.