From fe5757d12267e717dc7c6db65c51acfab70aa6e1 Mon Sep 17 00:00:00 2001 From: cloudhead Date: Tue, 23 Apr 2024 17:36:23 +0200 Subject: [PATCH] Tidy up environment variables * Try to use constants instead of strings * Move crypto seed override out of radicle-crypto * Use GIT_COMMITTER_DATE for commit overrides * Use RAD_LOCAL_TIME as general time override * Allow variables to be used in release mode --- radicle-cli/src/commands/auth.rs | 9 ++- radicle-cli/src/terminal/format.rs | 8 +-- radicle-cli/tests/commands.rs | 12 ++-- radicle-cob/src/backend/git.rs | 4 +- radicle-cob/src/backend/git/change.rs | 12 ++-- radicle-crypto/src/lib.rs | 37 ------------ radicle-crypto/src/ssh/keystore.rs | 26 +++++++-- radicle-httpd/src/test.rs | 6 +- radicle-node/src/test/environment.rs | 13 +++-- radicle-remote-helper/src/lib.rs | 2 +- radicle/src/cob/common.rs | 21 ------- radicle/src/cob/patch.rs | 5 +- radicle/src/cob/patch/cache.rs | 9 +-- radicle/src/cob/test.rs | 5 +- radicle/src/cob/thread.rs | 9 +-- radicle/src/lib.rs | 4 -- radicle/src/profile.rs | 81 ++++++++++++++++++++++++--- radicle/src/storage/refs.rs | 17 +++--- 18 files changed, 154 insertions(+), 126 deletions(-) diff --git a/radicle-cli/src/commands/auth.rs b/radicle-cli/src/commands/auth.rs index c1592304..94d06674 100644 --- a/radicle-cli/src/commands/auth.rs +++ b/radicle-cli/src/commands/auth.rs @@ -1,5 +1,4 @@ #![allow(clippy::or_fun_call)] -use std::env; use std::ffi::OsString; use std::ops::Not as _; use std::str::FromStr; @@ -9,7 +8,7 @@ use anyhow::anyhow; use radicle::crypto::ssh; use radicle::crypto::ssh::Passphrase; use radicle::node::Alias; -use radicle::profile::env::RAD_PASSPHRASE; +use radicle::profile::env; use radicle::{profile, Profile}; use crate::terminal as term; @@ -108,11 +107,11 @@ pub fn init(options: Options) -> anyhow::Result<()> { let passphrase = if options.stdin { term::passphrase_stdin() } else { - term::passphrase_confirm("Enter a passphrase:", RAD_PASSPHRASE) + term::passphrase_confirm("Enter a passphrase:", env::RAD_PASSPHRASE) }?; let passphrase = passphrase.trim().is_empty().not().then_some(passphrase); let spinner = term::spinner("Creating your Ed25519 keypair..."); - let profile = Profile::init(home, alias, passphrase.clone())?; + let profile = Profile::init(home, alias, passphrase.clone(), env::seed())?; let mut agent = true; spinner.finish(); @@ -199,7 +198,7 @@ pub fn authenticate(options: Options, profile: &Profile) -> anyhow::Result<()> { // Try RAD_PASSPHRASE fallback. if let Some(passphrase) = profile::env::passphrase() { ssh::keystore::MemorySigner::load(&profile.keystore, Some(passphrase)) - .map_err(|_| anyhow!("`{RAD_PASSPHRASE}` is invalid"))?; + .map_err(|_| anyhow!("`{}` is invalid", env::RAD_PASSPHRASE))?; return Ok(()); } diff --git a/radicle-cli/src/terminal/format.rs b/radicle-cli/src/terminal/format.rs index 4bec260c..74667719 100644 --- a/radicle-cli/src/terminal/format.rs +++ b/radicle-cli/src/terminal/format.rs @@ -5,12 +5,12 @@ use localtime::LocalTime; pub use radicle_term::format::*; pub use radicle_term::{style, Paint}; -use radicle::cob::{ObjectId, Timestamp}; +use radicle::cob::ObjectId; use radicle::identity::Visibility; use radicle::node::policy::Policy; use radicle::node::{Alias, AliasStore, NodeId}; use radicle::prelude::Did; -use radicle::profile::Profile; +use radicle::profile::{env, Profile}; use radicle::storage::RefUpdate; use radicle_term::element::Line; @@ -74,8 +74,8 @@ pub fn policy(p: &Policy) -> Paint { /// Format a timestamp. pub fn timestamp(time: impl Into) -> Paint { - let time: LocalTime = time.into(); - let now: LocalTime = Timestamp::now().into(); + let time = time.into(); + let now = env::local_time(); let duration = now - time; let fmt = timeago::Formatter::new(); diff --git a/radicle-cli/tests/commands.rs b/radicle-cli/tests/commands.rs index cbe5c830..81144659 100644 --- a/radicle-cli/tests/commands.rs +++ b/radicle-cli/tests/commands.rs @@ -1,6 +1,6 @@ use std::path::Path; use std::str::FromStr; -use std::{env, net, thread, time}; +use std::{net, thread, time}; use radicle::git; use radicle::node; @@ -11,7 +11,7 @@ use radicle::node::Handle as _; use radicle::node::{Address, Alias, DEFAULT_TIMEOUT}; use radicle::prelude::{NodeId, RepoId}; use radicle::profile; -use radicle::profile::Home; +use radicle::profile::{env, Home}; use radicle::storage::{ReadStorage, RefUpdate, RemoteRepository}; use radicle::test::fixtures; @@ -105,14 +105,14 @@ fn formula(root: &Path, test: impl AsRef) -> Result().unwrap(); + let (author, timestamp) = if let Ok(s) = std::env::var(crate::git::GIT_COMMITTER_DATE) { + let Ok(timestamp) = s.trim().parse::() else { + panic!( + "Invalid timestamp value {s:?} for `{}`", + crate::git::GIT_COMMITTER_DATE + ); + }; let author = Author { time: git_ext::author::Time::new(timestamp, 0), ..author diff --git a/radicle-crypto/src/lib.rs b/radicle-crypto/src/lib.rs index 550ce479..bc2a0b51 100644 --- a/radicle-crypto/src/lib.rs +++ b/radicle-crypto/src/lib.rs @@ -485,43 +485,6 @@ impl sqlite::BindableWithIndex for &Signature { } } -pub mod keypair { - use super::*; - - /// Generate a new keypair using OS randomness. - pub fn generate() -> KeyPair { - #[cfg(debug_assertions)] - if let Some(seed) = env::seed() { - // Generate a keypair based on the given environment variable. - // This is useful for debugging and testing, since the - // public key can be known in advance. - return KeyPair::from_seed(Seed::new(seed)); - } - KeyPair::generate() - } -} - -pub mod env { - use std::env; - - /// Return the seed stored in the `RAD_SEED` environment variable, if any. - pub fn seed() -> Option<[u8; 32]> { - if let Ok(seed) = env::var("RAD_SEED") { - let seed = (0..seed.len()) - .step_by(2) - .map(|i| u8::from_str_radix(&seed[i..i + 2], 16)) - .collect::, _>>() - .expect("env::seed: invalid hexadecimal value set in `RAD_SEED`"); - let seed: [u8; 32] = seed - .try_into() - .expect("env::seed: invalid seed length set in `RAD_SEED`"); - - return Some(seed); - } - None - } -} - #[cfg(test)] mod tests { use super::KeyPair; diff --git a/radicle-crypto/src/ssh/keystore.rs b/radicle-crypto/src/ssh/keystore.rs index 9fe4db66..d7dfdd86 100644 --- a/radicle-crypto/src/ssh/keystore.rs +++ b/radicle-crypto/src/ssh/keystore.rs @@ -8,7 +8,7 @@ use cyphernet::{EcSk, EcSkInvalid, Ecdh}; use thiserror::Error; use zeroize::Zeroizing; -use crate::{keypair, KeyPair, PublicKey, SecretKey, Signature, Signer, SignerError}; +use crate::{KeyPair, PublicKey, SecretKey, Signature, Signer, SignerError}; /// A secret key passphrase. pub type Passphrase = Zeroizing; @@ -58,10 +58,16 @@ impl Keystore { /// /// The `comment` is associated with the private key. /// The `passphrase` is used to encrypt the private key. + /// The `seed` is used to derive the private key and should almost always be generated. /// /// If `passphrase` is `None`, the key is not encrypted. - pub fn init(&self, comment: &str, passphrase: Option) -> Result { - self.store(keypair::generate(), comment, passphrase) + pub fn init( + &self, + comment: &str, + passphrase: Option, + seed: ec25519::Seed, + ) -> Result { + self.store(KeyPair::from_seed(seed), comment, passphrase) } /// Store a keypair on disk. Returns an error if the key already exists. @@ -285,7 +291,11 @@ mod tests { let store = Keystore::new(&tmp.path()); let public = store - .init("test", Some("hunter".to_owned().into())) + .init( + "test", + Some("hunter".to_owned().into()), + ec25519::Seed::default(), + ) .unwrap(); assert_eq!(public, store.public_key().unwrap().unwrap()); assert!(store.is_encrypted().unwrap()); @@ -306,7 +316,7 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); let store = Keystore::new(&tmp.path()); - let public = store.init("test", None).unwrap(); + let public = store.init("test", None, ec25519::Seed::default()).unwrap(); assert_eq!(public, store.public_key().unwrap().unwrap()); assert!(!store.is_encrypted().unwrap()); @@ -320,7 +330,11 @@ mod tests { let store = Keystore::new(&tmp.path()); let public = store - .init("test", Some("hunter".to_owned().into())) + .init( + "test", + Some("hunter".to_owned().into()), + ec25519::Seed::default(), + ) .unwrap(); let signer = MemorySigner::load(&store, Some("hunter".to_owned().into())).unwrap(); diff --git a/radicle-httpd/src/test.rs b/radicle-httpd/src/test.rs index 43d8fb27..47d0e032 100644 --- a/radicle-httpd/src/test.rs +++ b/radicle-httpd/src/test.rs @@ -1,8 +1,8 @@ use std::collections::BTreeSet; +use std::fs; use std::path::Path; use std::str::FromStr; use std::sync::Arc; -use std::{env, fs}; use axum::body::{Body, Bytes}; use axum::http::{Method, Request}; @@ -17,7 +17,7 @@ use radicle::crypto::ssh::Keystore; use radicle::crypto::{KeyPair, Seed, Signer}; use radicle::git::{raw as git2, RefString}; use radicle::identity::Visibility; -use radicle::profile::Home; +use radicle::profile::{env, Home}; use radicle::storage::ReadStorage; use radicle::Storage; use radicle::{node, profile}; @@ -144,7 +144,7 @@ fn seed_with_signer(dir: &Path, profile: radicle::Profile, signer: &G let workdir = dir.join("hello-world"); - env::set_var("RAD_COMMIT_TIME", TIMESTAMP.to_string()); + env::set_var(env::GIT_COMMITTER_DATE, TIMESTAMP.to_string()); fs::create_dir_all(&workdir).unwrap(); diff --git a/radicle-node/src/test/environment.rs b/radicle-node/src/test/environment.rs index 869d75da..7a41b76a 100644 --- a/radicle-node/src/test/environment.rs +++ b/radicle-node/src/test/environment.rs @@ -3,7 +3,7 @@ use std::mem::ManuallyDrop; use std::path::{Path, PathBuf}; use std::{ collections::{BTreeMap, BTreeSet}, - env, fs, io, iter, net, process, thread, time, + fs, io, iter, net, process, thread, time, time::Duration, }; @@ -24,7 +24,7 @@ use radicle::node::Database; use radicle::node::{Alias, POLICIES_DB_FILE}; use radicle::node::{ConnectOptions, Handle as _}; use radicle::profile; -use radicle::profile::{Home, Profile}; +use radicle::profile::{env, Home, Profile}; use radicle::rad; use radicle::storage::{ReadStorage as _, RemoteRepository as _, SignRepository as _}; use radicle::test::fixtures; @@ -413,11 +413,14 @@ impl NodeHandle { .env("GIT_COMMITTER_DATE", "1671125284") .env("GIT_COMMITTER_EMAIL", "radicle@localhost") .env("GIT_COMMITTER_NAME", "radicle") - .env("RAD_HOME", self.home.path().to_string_lossy().to_string()) - .env("RAD_PASSPHRASE", "radicle") + .env( + env::RAD_HOME, + self.home.path().to_string_lossy().to_string(), + ) + .env(env::RAD_PASSPHRASE, "radicle") + .env(env::RAD_LOCAL_TIME, "1671125284") .env("TZ", "UTC") .env("LANG", "C") - .env(radicle::cob::git::RAD_COMMIT_TIME, "1671125284") .envs(git::env::GIT_DEFAULT_CONFIG) .current_dir(cwd) .arg(cmd) diff --git a/radicle-remote-helper/src/lib.rs b/radicle-remote-helper/src/lib.rs index 6ad50d6e..bcee1452 100644 --- a/radicle-remote-helper/src/lib.rs +++ b/radicle-remote-helper/src/lib.rs @@ -120,7 +120,7 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> { .map(PathBuf::from) .map_err(|_| Error::NoGitDir)?; // Whether we should output debug logs. - let debug = env::var("RAD_DEBUG").is_ok(); + let debug = radicle::profile::env::debug(); let stdin = io::stdin(); let mut line = String::new(); diff --git a/radicle/src/cob/common.rs b/radicle/src/cob/common.rs index c9607182..0d69de15 100644 --- a/radicle/src/cob/common.rs +++ b/radicle/src/cob/common.rs @@ -19,27 +19,6 @@ use crate::storage::ReadRepository; pub struct Timestamp(LocalTime); impl Timestamp { - /// Construct a `Timestamp` corresponding to the current time. - /// - /// # Note - /// - /// If this is used in debug mode, `RAD_COMMIT_TIME` will be used - /// to construct the timestamp. - pub fn now() -> Self { - if cfg!(debug_assertions) { - if let Ok(s) = std::env::var("RAD_COMMIT_TIME") { - // SAFETY: Only used in test code. - #[allow(clippy::unwrap_used)] - let secs = s.trim().parse::().unwrap(); - Self::from_secs(secs) - } else { - Self(LocalTime::now()) - } - } else { - Self(LocalTime::now()) - } - } - pub fn from_secs(secs: u64) -> Self { Self(LocalTime::from_secs(secs)) } diff --git a/radicle/src/cob/patch.rs b/radicle/src/cob/patch.rs index 3e40d389..670379e7 100644 --- a/radicle/src/cob/patch.rs +++ b/radicle/src/cob/patch.rs @@ -2686,6 +2686,7 @@ mod test { use crate::crypto::test::signer::MockSigner; use crate::identity; use crate::patch::cache::Patches as _; + use crate::profile::env; use crate::test; use crate::test::arbitrary; use crate::test::arbitrary::gen; @@ -2996,7 +2997,7 @@ mod test { let base = arbitrary::oid(); let oid = arbitrary::oid(); let repo = gen::(1); - let time = Timestamp::now(); + let time = env::local_time(); let alice = MockSigner::default(); let bob = MockSigner::default(); let mut h0: cob::test::HistoryBuilder = cob::test::history( @@ -3012,7 +3013,7 @@ mod test { target: MergeTarget::Delegates, }, ], - time, + time.into(), &alice, ); let r1 = h0.commit( diff --git a/radicle/src/cob/patch/cache.rs b/radicle/src/cob/patch/cache.rs index 1c0ac4b4..28e3a512 100644 --- a/radicle/src/cob/patch/cache.rs +++ b/radicle/src/cob/patch/cache.rs @@ -689,11 +689,12 @@ mod tests { use crate::cob::cache::{Store, Update, Write}; use crate::cob::thread::{Comment, Thread}; - use crate::cob::{Author, Timestamp}; + use crate::cob::Author; use crate::patch::{ ByRevision, MergeTarget, Patch, PatchCounts, PatchId, Revision, RevisionId, State, Status, }; use crate::prelude::Did; + use crate::profile::env; use crate::test::arbitrary; use crate::test::storage::MockRepository; @@ -709,14 +710,14 @@ mod tests { let description = arbitrary::gen::(1); let base = arbitrary::oid(); let oid = arbitrary::oid(); - let timestamp = Timestamp::now(); + let timestamp = env::local_time(); let resolves = BTreeSet::new(); let mut revision = Revision::new( Author { id: author }, description, base, oid, - timestamp, + timestamp.into(), resolves, ); let comment = Comment::new( @@ -725,7 +726,7 @@ mod tests { None, None, vec![], - Timestamp::now(), + timestamp.into(), ); let thread = Thread::new(arbitrary::oid(), comment); revision.discussion = thread; diff --git a/radicle/src/cob/test.rs b/radicle/src/cob/test.rs index 2cb72c96..d008d538 100644 --- a/radicle/src/cob/test.rs +++ b/radicle/src/cob/test.rs @@ -18,6 +18,7 @@ use crate::git::ext::commit::headers::Headers; use crate::git::ext::commit::{trailers::OwnedTrailer, Commit}; use crate::git::Oid; use crate::prelude::Did; +use crate::profile::env; use crate::storage::ReadRepository; use crate::test::arbitrary; @@ -203,9 +204,9 @@ impl Actor { T::Action: Clone + Serialize, { let identity = arbitrary::oid(); - let timestamp = Timestamp::now(); + let timestamp = env::commit_time(); - self.op_with::(actions, Some(identity), timestamp) + self.op_with::(actions, Some(identity), timestamp.into()) } /// Get the actor's DID. diff --git a/radicle/src/cob/thread.rs b/radicle/src/cob/thread.rs index 1bd5c12a..eca10cdf 100644 --- a/radicle/src/cob/thread.rs +++ b/radicle/src/cob/thread.rs @@ -632,6 +632,7 @@ mod tests { use crate::cob::test; use crate::crypto::test::signer::MockSigner; use crate::crypto::Signer; + use crate::profile::env; use crate::test::arbitrary; use crate::test::arbitrary::gen; use crate::test::storage::MockRepository; @@ -743,14 +744,14 @@ mod tests { let bob = MockSigner::default(); let eve = MockSigner::default(); let repo = gen::(1); - let time = Timestamp::now(); + let time = env::local_time(); let mut a = test::history::( &[Action::Comment { body: "Thread root".to_owned(), reply_to: None, }], - time, + time.into(), &alice, ); a.comment("Alice comment", Some(*a.root().id()), &alice); @@ -810,14 +811,14 @@ mod tests { let repo = gen::(1); let alice = MockSigner::default(); let bob = MockSigner::default(); - let time = Timestamp::now(); + let time = env::local_time(); let mut a = test::history::( &[Action::Comment { body: "Thread root".to_owned(), reply_to: None, }], - time, + time.into(), &alice, ); let mut b = a.clone(); diff --git a/radicle/src/lib.rs b/radicle/src/lib.rs index 47d99783..993cfed7 100644 --- a/radicle/src/lib.rs +++ b/radicle/src/lib.rs @@ -46,7 +46,3 @@ pub mod prelude { BranchName, ReadRepository, ReadStorage, SignRepository, WriteRepository, WriteStorage, }; } - -pub mod env { - pub use crypto::env::*; -} diff --git a/radicle/src/profile.rs b/radicle/src/profile.rs index 8453080e..bc6f75d8 100644 --- a/radicle/src/profile.rs +++ b/radicle/src/profile.rs @@ -42,8 +42,38 @@ pub mod env { pub const RAD_PASSPHRASE: &str = "RAD_PASSPHRASE"; /// RNG seed. Must be convertible to a `u64`. pub const RAD_RNG_SEED: &str = "RAD_RNG_SEED"; + /// Private key seed. Used for generating deterministic keypairs. + pub const RAD_SEED: &str = "RAD_SEED"; /// Show radicle hints. pub const RAD_HINT: &str = "RAD_HINT"; + /// Environment variable to set to overwrite the commit date for both + /// the author and the committer. + /// + /// The format must be a unix timestamp. + pub const RAD_COMMIT_TIME: &str = "RAD_COMMIT_TIME"; + /// Override the device's local time. + /// The format must be a unix timestamp. + pub const RAD_LOCAL_TIME: &str = "RAD_LOCAL_TIME"; + // Turn debug mode on. + pub const RAD_DEBUG: &str = "RAD_DEBUG"; + // Used to set the Git committer timestamp. Can be overridden + // to generate deterministic COB IDs. + pub const GIT_COMMITTER_DATE: &str = "GIT_COMMITTER_DATE"; + + /// Commit timestamp to use. Can be overriden by [`RAD_COMMIT_TIME`]. + pub fn commit_time() -> localtime::LocalTime { + time(RAD_COMMIT_TIME).unwrap_or_else(local_time) + } + + /// Local time. Can be overriden by [`RAD_LOCAL_TIME`]. + pub fn local_time() -> localtime::LocalTime { + time(RAD_LOCAL_TIME).unwrap_or_else(localtime::LocalTime::now) + } + + /// Whether debug mode is on. + pub fn debug() -> bool { + var(RAD_DEBUG).is_ok() + } /// Whether or not to show hints. pub fn hints() -> bool { @@ -74,12 +104,44 @@ pub mod env { /// Get a random number generator from the environment. pub fn rng() -> fastrand::Rng { if let Ok(seed) = var(RAD_RNG_SEED) { - return fastrand::Rng::with_seed( - seed.parse() - .expect("env::rng: invalid seed specified in `RAD_RNG_SEED`"), - ); + let Ok(seed) = seed.parse() else { + panic!("env::rng: invalid seed specified in `{RAD_RNG_SEED}`"); + }; + fastrand::Rng::with_seed(seed) + } else { + fastrand::Rng::new() } - fastrand::Rng::new() + } + + /// Return the seed stored in the [`RAD_SEED`] environment variable, or generate a random one. + pub fn seed() -> crypto::Seed { + if let Ok(seed) = var(RAD_SEED) { + let Ok(seed) = (0..seed.len()) + .step_by(2) + .map(|i| u8::from_str_radix(&seed[i..i + 2], 16)) + .collect::, _>>() + else { + panic!("env::seed: invalid hexadecimal value set in `{RAD_SEED}`"); + }; + let Ok(seed): Result<[u8; 32], _> = seed.try_into() else { + panic!("env::seed: invalid seed length set in `{RAD_SEED}`"); + }; + crypto::Seed::new(seed) + } else { + crypto::Seed::generate() + } + } + + fn time(key: &str) -> Option { + if let Ok(s) = var(key) { + match s.trim().parse::() { + Ok(t) => return Some(localtime::LocalTime::from_secs(t)), + Err(e) => { + panic!("env::time: invalid value {s:?} for `{key}` environment variable: {e}"); + } + } + } + None } } @@ -206,9 +268,14 @@ pub struct Profile { } impl Profile { - pub fn init(home: Home, alias: Alias, passphrase: Option) -> Result { + pub fn init( + home: Home, + alias: Alias, + passphrase: Option, + seed: crypto::Seed, + ) -> Result { let keystore = Keystore::new(&home.keys()); - let public_key = keystore.init("radicle", passphrase)?; + let public_key = keystore.init("radicle", passphrase, seed)?; let config = Config::init(alias.clone(), home.config().as_path())?; let storage = Storage::open( home.storage(), diff --git a/radicle/src/storage/refs.rs b/radicle/src/storage/refs.rs index bef6c686..1a999625 100644 --- a/radicle/src/storage/refs.rs +++ b/radicle/src/storage/refs.rs @@ -14,6 +14,7 @@ use thiserror::Error; use crate::git; use crate::git::ext as git_ext; use crate::git::Oid; +use crate::profile::env; use crate::storage; use crate::storage::{ReadRepository, RemoteId, WriteRepository}; @@ -309,17 +310,17 @@ impl SignedRefs { }?; let sigref = sigref.with_namespace(remote.into()); - let author = raw.signature()?; - - #[cfg(debug_assertions)] - let author = if let Ok(s) = std::env::var("RAD_COMMIT_TIME") { - // SAFETY: Only used in test code. - #[allow(clippy::unwrap_used)] - let timestamp = s.trim().parse::().unwrap(); + let author = if let Ok(s) = env::var(env::GIT_COMMITTER_DATE) { + let Ok(timestamp) = s.trim().parse::() else { + panic!( + "Invalid timestamp value {s:?} for `{}`", + env::GIT_COMMITTER_DATE + ); + }; let time = git2::Time::new(timestamp, 0); git2::Signature::new("radicle", remote.to_string().as_str(), &time)? } else { - author + raw.signature()? }; let commit = raw.commit(