diff --git a/client/src/cli.rs b/client/src/cli.rs index 06a95d8..76aad00 100644 --- a/client/src/cli.rs +++ b/client/src/cli.rs @@ -96,7 +96,7 @@ impl CommandContext for Cli { fn session_stopped(&mut self, session_id: &SessionId) -> Result<(), CommandError> { let mut session_guard = self.session.lock().unwrap(); if let Some(session) = session_guard.as_ref() - && session.session_id() == session_id + && session.session_id() == *session_id { let _ = session_guard.take(); } diff --git a/client/src/commands.rs b/client/src/commands.rs index c823874..aa26def 100644 --- a/client/src/commands.rs +++ b/client/src/commands.rs @@ -759,7 +759,7 @@ async fn session( .await? .into_inner(); if let Some(session_id) = session_id - && *session.session_id() != session_id + && session.session_id() != session_id { return Err(CommandError::MissingSession); } @@ -781,7 +781,7 @@ async fn session( let session = if let Some(session_id) = session_id { Session::new(session_id) } else { - Session::new(SessionId::new()) + Session::new(SessionId::random()) }; with_login(ctx, client, async || { client @@ -809,7 +809,7 @@ async fn session( with_login(ctx, client, async || { client .session_allow_attach() - .session_id(session_id.clone()) + .session_id(session_id) .key_id(key_id.clone()) .access(access) .send() @@ -827,7 +827,7 @@ async fn session( with_login(ctx, client, async || { client .session_deny_attach() - .session_id(session_id.clone()) + .session_id(session_id) .key_id(key_id.clone()) .send() .await @@ -843,11 +843,7 @@ async fn session( return Err(CommandError::MissingSession); }; with_login(ctx, client, async || { - client - .session_stop() - .session_id(session_id.clone()) - .send() - .await + client.session_stop().session_id(*session_id).send().await }) .await?; ctx.session_stopped(session_id)?; @@ -920,7 +916,7 @@ async fn job( { Ok(resp) => resp.into_inner(), Err(CommandError::NotFound) => { - let session = Session::new(SessionId::new()); + let session = Session::new(SessionId::random()); with_login(ctx, client, async || { client .session_start() @@ -1136,7 +1132,7 @@ async fn job_start( match with_login_via(ctx, client, Some(&target), async || { client .job_output() - .job_id(&job_id) + .job_id(job_id) .target(target.to_string()) .stream(stream) .send() @@ -1242,7 +1238,7 @@ async fn job_output( // Fetch job status for output length and hash. let status = job_status_try_from_json_map( with_login_via(ctx, client, Some(target), async || { - client.job_status().job_id(&job_id).send().await + client.job_status().job_id(job_id).send().await }) .await? .into_inner(), @@ -1348,7 +1344,7 @@ async fn job_output( async || { client .job_output() - .job_id(&job_id) + .job_id(job_id) .target(target.to_string()) .stream(stream) .send() diff --git a/client/src/identity.rs b/client/src/identity.rs index 3763126..25d290f 100644 --- a/client/src/identity.rs +++ b/client/src/identity.rs @@ -171,7 +171,7 @@ impl IdentityError { #[cfg(test)] mod test { - use sush_common::codephrases::{PHRASE_WORDS_ID, WORD_SEPARATOR, generate_id}; + use sush_common::codephrases::Codephrase; use tempfile::TempDir; use tokio::process::Command; @@ -214,12 +214,9 @@ mod test { let mut agent = SshAgentConnection::connect(&sock).await.unwrap(); for key in agent.list_identities().await.unwrap() { - let key_id = key.key_id().unwrap(); - assert_eq!(key_id.split(WORD_SEPARATOR).count(), PHRASE_WORDS_ID); - - let nonce = generate_id(); - let signature = agent.sign_with(&key, nonce.as_bytes()).await.unwrap(); - key.verify(nonce.as_bytes(), &signature).unwrap(); + let nonce = Codephrase::random(); + let signature = agent.sign_with(&key, &nonce.to_be_bytes()).await.unwrap(); + key.verify(&nonce.to_be_bytes(), &signature).unwrap(); } agent_process diff --git a/common/src/authn.rs b/common/src/authn.rs index 3ec3cdb..3ccd184 100644 --- a/common/src/authn.rs +++ b/common/src/authn.rs @@ -30,10 +30,9 @@ use std::str::FromStr; use std::time::Duration; use blake3::Hasher; -use borsh::io::{Error, ErrorKind, Read, Write}; use borsh::{BorshDeserialize, BorshSerialize}; use chrono::{DateTime, Utc}; -use crypto_bigint::{ArrayEncoding as _, U256}; +use crypto_bigint::U256; use ed25519_dalek::{Signature as Ed25519Signature, Signer as _, SigningKey, VerifyingKey}; use http::header::{HeaderValue, InvalidHeaderValue}; use rand_core::OsRng; @@ -41,10 +40,11 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use thiserror::Error; -use crate::codephrases::{ - InvalidCodephrase, WORD_SEPARATOR, codephrase, decode_phrase, generate_id, +use crate::codephrase_newtype; +use crate::codephrases::InvalidCodephrase; +use crate::keys::{ + EccR, EccS, EncodedSignature, KeyError, KeyId, Signed, SshPublicKey, ToBeSigned, Verified, }; -use crate::keys::{EncodedSignature, KeyError, KeyId, Signed, SshPublicKey, ToBeSigned, Verified}; /// The name of our custom HTTP authentication scheme. pub const AUTHN_SCHEME: &str = "Sush"; @@ -63,43 +63,22 @@ pub const IDENTITY_MAX_TTL: Duration = Duration::from_secs(8 * 3600); /// The time-to-live of a nonce. pub const NONCE_TTL: Duration = Duration::from_secs(60); -/// A unique random string. Authentication credentials have two -/// of these: one generated by the server, and one by the client. -/// This structure is agnostic to the syntax of the string. -#[derive( - BorshDeserialize, - BorshSerialize, - Clone, - Debug, - Deserialize, - Eq, - Hash, - JsonSchema, - PartialEq, - Serialize, -)] -pub struct Nonce(String); - -impl Nonce { - pub fn generate() -> Self { - Self(generate_id()) - } - - pub fn as_bytes(&self) -> &[u8] { - self.0.as_bytes() - } -} - -impl From for Nonce { - fn from(nonce: String) -> Self { - Self(nonce) - } -} - -impl fmt::Display for Nonce { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.0) - } +codephrase_newtype! { + /// A unique random string. Authentication credentials have two + /// of these: one generated by the server, and one by the client. + /// This structure is agnostic to the syntax of the string. + #[derive( + BorshDeserialize, + BorshSerialize, + Clone, + Deserialize, + Eq, + Hash, + JsonSchema, + PartialEq, + Serialize, + )] + pub struct Nonce = Truncated; } /// Authentication challenge. @@ -135,7 +114,7 @@ impl FromStr for Challenge { /// Parse a server challenge, e.g., `Sush nonce=foo-bar-baz`. fn from_str(s: &str) -> Result { let mut params = AuthnParams::from_str(s)?; - let nonce = params.get("nonce")?; + let nonce = params.parse("nonce")?; params.finished()?; Ok(Self::new(Nonce(nonce))) } @@ -155,20 +134,19 @@ impl RequestKey { } pub fn verifier(&self) -> RequestVerifier { - RequestVerifier(self.0.verifying_key()) + RequestVerifier::from_be_bytes(*self.0.verifying_key().as_bytes()) } /// Sign `request`, yielding the credentials that authorize it. pub fn bind(&self, key_id: KeyId, nonce: Nonce, request: &BoundRequest) -> BoundCredentials { let signature = self.0.sign(&request.to_be_signed(&key_id, &nonce)); - let phrase = |bytes: &[u8]| codephrase(U256::from_be_slice(bytes)).join(WORD_SEPARATOR); BoundCredentials { key_id, nonce, seq: request.seq, signature: EncodedSignature { - r: phrase(&signature.r_bytes()[..]), - s: phrase(&signature.s_bytes()[..]), + r: EccR::from_be_bytes(*signature.r_bytes()), + s: EccS::from_be_bytes(*signature.s_bytes()), flags: 0, counter: 0, }, @@ -176,15 +154,14 @@ impl RequestKey { } } -/// The server half of an ephemeral request-signing key. -#[derive(Clone, Debug, Eq, PartialEq)] -pub struct RequestVerifier(VerifyingKey); +codephrase_newtype! { + /// The server half of an ephemeral request-signing key. + #[derive(Clone, Eq, PartialEq, BorshSerialize, BorshDeserialize)] + pub struct RequestVerifier = Full; -impl RequestVerifier { - fn as_bytes(&self) -> &[u8] { - self.0.as_bytes() - } +} +impl RequestVerifier { /// Verify that `credentials` sign `request`. pub fn verify( &self, @@ -194,58 +171,22 @@ impl RequestVerifier { if request.seq != credentials.seq { return Err(AuthnError::InvalidSignature); } - let half = |phrase: &str| -> Result<[u8; 32], AuthnError> { - Ok(decode_phrase(phrase)?.to_be_byte_array().into()) - }; let EncodedSignature { r, s, .. } = &credentials.signature; - let signature = Ed25519Signature::from_components(half(r)?, half(s)?); - self.0 - .verify_strict( - &request.to_be_signed(&credentials.key_id, &credentials.nonce), - &signature, - ) - .map_err(|_| AuthnError::InvalidSignature) - } -} + let signature = Ed25519Signature::from_components(r.to_be_bytes(), s.to_be_bytes()); -impl BorshSerialize for RequestVerifier { - fn serialize(&self, writer: &mut W) -> borsh::io::Result<()> { - writer.write_all(self.as_bytes()) - } -} - -impl BorshDeserialize for RequestVerifier { - fn deserialize_reader(reader: &mut R) -> borsh::io::Result { - let mut bytes = [0; 32]; - reader.read_exact(&mut bytes)?; - VerifyingKey::from_bytes(&bytes) - .map(Self) - .map_err(|err| Error::new(ErrorKind::InvalidData, err)) - } -} - -impl fmt::Display for RequestVerifier { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "{}", - codephrase(U256::from_be_slice(self.as_bytes())).join(WORD_SEPARATOR) - ) - } -} - -impl FromStr for RequestVerifier { - type Err = AuthnError; - - fn from_str(s: &str) -> Result { - let bytes = decode_phrase(s)?.to_be_byte_array(); - let key = VerifyingKey::from_bytes(&bytes.into()).map_err(|_| AuthnError::InvalidKey)?; + let key = + VerifyingKey::from_bytes(&self.to_be_bytes()).map_err(|_| AuthnError::InvalidKey)?; // A small-order key verifies anything, degrading the session // back to a bearer credential. if key.is_weak() { return Err(AuthnError::InvalidKey); } - Ok(Self(key)) + + key.verify_strict( + &request.to_be_signed(&credentials.key_id, &credentials.nonce), + &signature, + ) + .map_err(|_| AuthnError::InvalidSignature) } } @@ -290,8 +231,8 @@ impl BoundRequest { seq, } = self; hash(Self::TYPE_NAME); - hash(key_id.as_bytes()); - hash(nonce.as_bytes()); + hash(&key_id.to_be_bytes()); + hash(&nonce.to_be_bytes()); hash(method.as_bytes()); hash(target.as_bytes()); hash(&seq.to_be_bytes()); @@ -333,12 +274,13 @@ impl FromStr for BoundCredentials { fn from_str(s: &str) -> Result { let mut params = AuthnParams::from_str(s)?; let creds = Self { - key_id: params.get("key-id")?, - nonce: params.get("nonce")?, + key_id: params.parse("key-id")?, + nonce: params.parse("nonce")?, seq: params.parse("seq")?, signature: EncodedSignature { - r: params.get("r")?, - s: params.get("s")?, + r: params.parse("r")?, + + s: params.parse("s")?, flags: 0, counter: 0, }, @@ -432,7 +374,7 @@ impl ChallengeResponse { pub fn new(challenge: Challenge, epk: RequestVerifier) -> Self { Self { nonce: challenge.nonce, - cnonce: Nonce::generate(), + cnonce: Nonce::random(), epk, } } @@ -453,9 +395,9 @@ impl ToBeSigned for ChallengeResponse { let Self { nonce, cnonce, epk } = self; hash(Self::TYPE_NAME); - hash(nonce.as_bytes()); - hash(cnonce.as_bytes()); - hash(epk.as_bytes()); + hash(&nonce.to_be_bytes()); + hash(&cnonce.to_be_bytes()); + hash(&epk.to_be_bytes()); hasher.finalize().as_bytes().to_vec() } } @@ -543,13 +485,13 @@ impl FromStr for Credentials { fn from_str(s: &str) -> Result { let mut params = AuthnParams::from_str(s)?; let creds = Self { - nonce: params.get("nonce")?, - cnonce: params.get("cnonce")?, - key_id: params.get("key-id")?, + nonce: params.parse("nonce")?, + cnonce: params.parse("cnonce")?, + key_id: params.parse("key-id")?, epk: params.parse("epk")?, signature: EncodedSignature { - r: params.get("r")?, - s: params.get("s")?, + r: params.parse("r")?, + s: params.parse("s")?, flags: params.parse("flags")?, counter: params.parse("counter")?, }, @@ -697,7 +639,7 @@ mod test { let mut signer = EphemeralKey::new_root(KeyType::Ed25519, name, validity).unwrap(); for _ in 0..10 { let key_id = signer.key_id().clone(); - let nonce = Nonce::generate(); + let nonce = Nonce::random(); let challenge = Challenge::new(nonce.clone()); let www_authenticate = challenge.to_string(); assert_eq!(www_authenticate, format!("Sush nonce={nonce}")); @@ -752,7 +694,11 @@ mod test { let key = RequestKey::new(); let verifier = key.verifier(); let request = BoundRequest::new("get", "/jobs?limit=1", 7); - let creds = key.bind(KeyId::from("baz".to_string()), Nonce::generate(), &request); + let creds = key.bind( + KeyId::from_str("abandon").unwrap(), + Nonce::random(), + &request, + ); assert_eq!( creds.to_string().parse::().unwrap(), creds @@ -781,25 +727,24 @@ mod test { // The identity is part of the signed material: credentials // re-labeled with another key ID or nonce do not verify. let relabeled = BoundCredentials { - key_id: KeyId::from("qux".to_string()), + key_id: KeyId::from_str("ability").unwrap(), ..creds.clone() }; assert!(verifier.verify(&request, &relabeled).is_err()); let relabeled = BoundCredentials { - nonce: Nonce::generate(), - ..creds + nonce: Nonce::random(), + ..creds.clone() }; assert!(verifier.verify(&request, &relabeled).is_err()); // A small-order ephemeral key is refused at parse. - let weak = codephrase(U256::from_be_slice(&{ + let weak = RequestVerifier::from_be_bytes({ let mut identity = [0u8; 32]; identity[0] = 1; identity - })) - .join(WORD_SEPARATOR); + }); assert!(matches!( - weak.parse::().unwrap_err(), + weak.verify(&request, &creds).unwrap_err(), AuthnError::InvalidKey )); } @@ -851,27 +796,27 @@ mod test { AuthnError::MissingParam(p) if p == "nonce" )); assert!(matches!( - Credentials::from_str("Sush nonce=foo").unwrap_err(), + Credentials::from_str("Sush nonce=abandon").unwrap_err(), AuthnError::MissingParam(p) if p == "cnonce" )); let epk = RequestKey::new().verifier(); assert!(matches!( Credentials::from_str( - "Sush nonce=foo,cnonce=bar,key-id=baz,epk=plugh,r=r,s=s,flags=0,counter=0" + "Sush nonce=abandon,cnonce=ability,key-id=able,epk=plugh,r=burger,s=burst,flags=0,counter=0" ) .unwrap_err(), AuthnError::InvalidParam )); assert!(matches!( Credentials::from_str(&format!( - "Sush nonce=foo,cnonce=bar,key-id=baz,epk={epk},r=r,s=s,flags=0,counter=foo" + "Sush nonce=abandon,cnonce=ability,key-id=able,epk={epk},r=burger,s=burst,flags=0,counter=foo" )) .unwrap_err(), AuthnError::InvalidParam )); assert!(matches!( Credentials::from_str(&format!( - "Sush nonce=foo,cnonce=bar,key-id=baz,epk={epk},r=r,s=s,flags=0,counter=0,foo=bar" + "Sush nonce=abandon,cnonce=ability,key-id=able,epk={epk},r=burger,s=burst,flags=0,counter=0,foo=bar" )) .unwrap_err(), AuthnError::TooManyParams diff --git a/common/src/borsh.rs b/common/src/borsh.rs index e1966a9..9925ad0 100644 --- a/common/src/borsh.rs +++ b/common/src/borsh.rs @@ -12,7 +12,6 @@ use sled_hardware_types::BaseboardId; use x509_cert::Certificate; use x509_cert::der::{Decode as _, Encode as _}; -use crate::jobs::JobId; use crate::targets::Target; /// Borsh-encode a [`blake3::Hash`] as its 32 raw bytes. @@ -87,29 +86,3 @@ pub fn borsh_de_cert(reader: &mut R) -> Result { ) }) } - -pub fn borsh_de_job_id(reader: &mut R) -> Result { - Ok(String::deserialize_reader(reader)? - .parse::() - .map_err(|_| Error::new(ErrorKind::InvalidData, "failed to deserialize job ID"))? - .to_string()) -} - -#[cfg(test)] -mod test { - use crate::jobs::SessionId; - - use super::*; - - #[test] - fn borsh_de_job_id() { - let hostile = borsh::to_vec(&"../../etc/passwd".to_string()).unwrap(); - assert!(borsh::from_slice::(&hostile).is_err()); - - let sneaky = borsh::to_vec(&"ALPHA-BRAVO".to_string()).unwrap(); - assert!(borsh::from_slice::(&sneaky).is_err()); - - let good = borsh::to_vec(&SessionId::new().first_job_id().to_string()).unwrap(); - assert!(borsh::from_slice::(&good).is_ok()); - } -} diff --git a/common/src/codephrases.rs b/common/src/codephrases.rs index 1fac573..47aa45b 100644 --- a/common/src/codephrases.rs +++ b/common/src/codephrases.rs @@ -9,8 +9,20 @@ //! that must be readily transmissible over low bandwidth channels (e.g., //! email, voice, printed or handwritten notes, etc.). -use crypto_bigint::{CheckedAdd as _, CheckedMul as _, Limb, Random as _, Reciprocal, U256}; +use std::borrow::Cow; +use std::fmt; +use std::io::{self, Read, Write}; +use std::str::FromStr; + +use borsh::{BorshDeserialize, BorshSerialize}; +use crypto_bigint::{ + ArrayEncoding as _, CheckedAdd as _, CheckedMul as _, Encoding as _, Limb, Random as _, + Reciprocal, U256, +}; use rand_core::OsRng; +use schemars::JsonSchema; +use serde::de::Error as _; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; use thiserror::Error; use crate::wordlist::{WORDLIST, WORDLIST_LEN}; @@ -19,7 +31,12 @@ use crate::wordlist::{WORDLIST, WORDLIST_LEN}; /// to 2048, which gives us indexes into the BIP-39 word list. /// Since 204823 < 2256 < 204824, /// 24 words suffice to represent 256 bits with no redundancy. -pub const PHRASE_WORDS_256: usize = 24; +const PHRASE_WORDS_256: usize = 24; + +/// The BIP-39 word list contains no punctuation of any kind, so we are +/// free to use ASCII hyphen (`-`) as the default word separator. Decoding +/// will also accept arbitrary ASCII whitespace as word separators. +const WORD_SEPARATOR: &str = "-"; /// With 2048 words, an 8 word code phrase has ~88 bits of entropy, /// making it suitable for use as a unique, hard-to-guess identifier, @@ -28,104 +45,245 @@ pub const PHRASE_WORDS_256: usize = 24; /// Note that the security of the Support Shell protocol (RFD 620) does /// *not* rely on such identifiers being unguessable; it relies only on /// the strength of the signatures produced over these phrases. -pub const PHRASE_WORDS_ID: usize = 8; +const TRUNCATED_BITS: u32 = 88; -/// The BIP-39 word list contains no punctuation of any kind, so we are -/// free to use ASCII hyphen (`-`) as the default word separator. Decoding -/// will also accept arbitrary ASCII whitespace as word separators. -pub const WORD_SEPARATOR: &str = "-"; +/// Random code phrase like `abstract misery favorite ordinary moon talk`. +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct Codephrase(U256); -/// Decoding a phrase failed. -#[derive(Debug, Error)] -#[error("invalid or non-canonical code phrase")] -pub struct InvalidCodephrase; +impl Codephrase { + /// Generate a new random codephrase. + pub fn random() -> Self { + Self(U256::random(&mut OsRng)) + } -/// Look up a word in the word list. -#[inline] -fn word(index: usize) -> &'static str { - WORDLIST[index] + /// Turn 256 bits of entropy into a reasonably unique code phrase. + /// We use the low-order words of the full codephrase, and pad to + /// the canonical length; these phrases are intended for machine + /// consumption, and are short enough for easy transmission. + pub fn truncate(self) -> Self { + let mask = U256::from_u128(2u128.pow(TRUNCATED_BITS) - 1); + Self(self.0.bitand(&mask)) + } + + /// Construct a codephrase from its big-endian byte representation. + pub fn from_be_bytes(bytes: [u8; 32]) -> Self { + Self(U256::from_be_bytes(bytes)) + } + + /// Get the underlying big-endian byte representation of the codephrase. + pub fn to_be_bytes(&self) -> [u8; 32] { + self.0.to_be_byte_array().into() + } } -/// Find the index of a word in the word list. -#[inline] -fn index(word: &str) -> Result { - WORDLIST.binary_search(&word).map_err(|_| InvalidCodephrase) +impl fmt::Display for Codephrase { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // Turn 256 bits of entropy into an un-padded big-endian code phrase. + let b = Reciprocal::new(Limb(WORDLIST_LEN as u64)).expect("should have some words"); + let mut n = self.0; + let mut r; + let mut words = Vec::with_capacity(PHRASE_WORDS_256); + while n > U256::ZERO { + (n, r) = n.ct_div_rem_limb_with_reciprocal(&b); + words.push(word(r.0 as usize)); // accumulate little-endian + } + words.reverse(); // emit big-endian + + f.write_str(&words.join(WORD_SEPARATOR)) + } } -/// Turn 256 bits of entropy into an un-padded big-endian code phrase. -pub fn codephrase(value: U256) -> Vec<&'static str> { - let b = Reciprocal::new(Limb(WORDLIST_LEN as u64)).expect("should have some words"); - let mut n = value; - let mut r; - let mut words = Vec::with_capacity(PHRASE_WORDS_256); - while n > U256::ZERO { - (n, r) = n.ct_div_rem_limb_with_reciprocal(&b); - words.push(word(r.0 as usize)); // accumulate little-endian +impl fmt::Debug for Codephrase { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "Codephrase({self})") } - words.reverse(); // emit big-endian - words } -/// Turn 256 bits of entropy into a reasonably unique code phrase. -/// We use the low-order words of the full codephrase, and pad to -/// the canonical length; these phrases are intended for machine -/// consumption, and are short enough for easy transmission. -pub fn id_phrase(value: U256) -> Vec<&'static str> { - let mut phrase = codephrase(value); - let n = phrase.len().saturating_sub(PHRASE_WORDS_ID); - let mut phrase = phrase.split_off(n); - while phrase.len() < PHRASE_WORDS_ID { - phrase.insert(0, word(0)); // pad w/leading zeros +impl FromStr for Codephrase { + type Err = InvalidCodephrase; + + fn from_str(value: &str) -> Result { + // Decode a big endian code phrase into 256 bits of entropy. + // + // Decoding is non-injective, or "liberal" in the sense that it will + // accept non-canonical codephrases, e.g., ones with spaces instead of + // separators, words in mixed case, or without leading zeros. This is + // for the comfort of humans that may have to transmit such phrases. + // It rejects phrases that no entropy encodes to: unknown words, more + // than [`PHRASE_WORDS_256`] words, or a value of 256 bits or more. + // The empty phrase decodes to zero. Callers that must distinguish + // absent input from zero should check before decoding. + + let b = U256::from_u64(WORDLIST_LEN as u64); + let mut n = U256::ZERO; + let mut words = 0; + for word in value + .to_ascii_lowercase() + .replace(WORD_SEPARATOR, " ") + .split_ascii_whitespace() + { + words += 1; + if words > PHRASE_WORDS_256 { + return Err(InvalidCodephrase); + } + let r = U256::from_u64(index(word)? as u64); + n = Option::from(n.checked_mul(&b)).ok_or(InvalidCodephrase)?; + n = Option::from(n.checked_add(&r)).ok_or(InvalidCodephrase)?; + } + Ok(Self(n)) } - phrase } -/// Generate a code phrase for use as an identifier. -pub fn generate_id() -> String { - id_phrase(U256::random(&mut OsRng)).join(WORD_SEPARATOR) +impl Serialize for Codephrase { + fn serialize(&self, serializer: S) -> Result { + serializer.serialize_str(&self.to_string()) + } } -/// Decode a big endian code phrase into 256 bits of entropy. -/// -/// Decoding is non-injective, or "liberal" in the sense that it will -/// accept non-canonical codephrases, e.g., ones with spaces instead of -/// separators, words in mixed case, or without leading zeros. This is -/// for the comfort of humans that may have to transmit such phrases. -/// It rejects phrases that no entropy encodes to: unknown words, more -/// than [`PHRASE_WORDS_256`] words, or a value of 256 bits or more. -/// The empty phrase decodes to zero. Callers that must distinguish -/// absent input from zero should check before decoding. -pub fn decode_phrase(phrase: &str) -> Result { - let b = U256::from_u64(WORDLIST_LEN as u64); - let mut n = U256::ZERO; - let mut words = 0; - for word in phrase - .to_ascii_lowercase() - .replace(WORD_SEPARATOR, " ") - .split_ascii_whitespace() - { - words += 1; - if words > PHRASE_WORDS_256 { - return Err(InvalidCodephrase); +impl<'de> Deserialize<'de> for Codephrase { + fn deserialize>(deserializer: D) -> Result { + let string = ::deserialize(deserializer)?; + Self::from_str(&string).map_err(D::Error::custom) + } +} + +impl BorshSerialize for Codephrase { + fn serialize(&self, writer: &mut W) -> io::Result<()> { + <[u8; 32] as BorshSerialize>::serialize(&self.to_be_bytes(), writer) + } +} + +impl BorshDeserialize for Codephrase { + fn deserialize_reader(reader: &mut R) -> io::Result { + let bytes = <[u8; 32] as BorshDeserialize>::deserialize_reader(reader)?; + Ok(Self(U256::from_be_byte_array(bytes.into()))) + } +} + +// Treat a codephrase as a string in JSON schemas. +impl JsonSchema for Codephrase { + fn schema_name() -> String { + String::schema_name() + } + + fn json_schema(generator: &mut schemars::r#gen::SchemaGenerator) -> schemars::schema::Schema { + String::json_schema(generator) + } + + fn is_referenceable() -> bool { + String::is_referenceable() + } + + fn schema_id() -> Cow<'static, str> { + String::schema_id() + } +} + +#[macro_export] +macro_rules! codephrase_newtype { + ($(#[$meta:meta])* $vis:vis struct $name:ident = $len:ident;) => { + $(#[$meta])* + $vis struct $name($crate::codephrases::Codephrase); + + impl $name { + #[allow(unused)] + $vis fn random() -> Self { + let mut codephrase = $crate::codephrases::Codephrase::random(); + match $crate::codephrases::CodephraseLength::$len { + $crate::codephrases::CodephraseLength::Full => {} + $crate::codephrases::CodephraseLength::Truncated => { + codephrase = codephrase.truncate(); + } + } + Self(codephrase) + } + + #[allow(unused)] + $vis fn from_hash(hash: blake3::Hash) -> Self { + let mut codephrase = $crate::codephrases::Codephrase::from_be_bytes(*hash.as_bytes()); + match $crate::codephrases::CodephraseLength::$len { + $crate::codephrases::CodephraseLength::Full => {} + $crate::codephrases::CodephraseLength::Truncated => { + codephrase = codephrase.truncate(); + } + } + Self(codephrase) + } + + #[allow(unused)] + $vis fn from_be_bytes(bytes: [u8; 32]) -> Self { + let mut codephrase = $crate::codephrases::Codephrase::from_be_bytes(bytes); + match $crate::codephrases::CodephraseLength::$len { + $crate::codephrases::CodephraseLength::Full => {} + $crate::codephrases::CodephraseLength::Truncated => { + codephrase = codephrase.truncate(); + } + } + Self(codephrase) + } + + #[allow(unused)] + $vis fn to_be_bytes(&self) -> [u8; 32] { + self.0.to_be_bytes() + } + } + + impl std::fmt::Debug for $name { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}({})", stringify!($name), self.0) + } + } + + impl std::fmt::Display for $name { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + <$crate::codephrases::Codephrase as std::fmt::Display>::fmt(&self.0, f) + } + } + + impl std::str::FromStr for $name { + type Err = $crate::codephrases::InvalidCodephrase; + + fn from_str(s: &str) -> Result<$name, Self::Err> { + Ok($name(s.parse()?)) + } } - let r = U256::from_u64(index(word)? as u64); - n = Option::from(n.checked_mul(&b)).ok_or(InvalidCodephrase)?; - n = Option::from(n.checked_add(&r)).ok_or(InvalidCodephrase)?; } - Ok(n) +} + +#[derive(Clone, Copy)] +pub enum CodephraseLength { + Full, + Truncated, +} + +/// Decoding a phrase failed. +#[derive(Debug, Error)] +#[error("invalid or non-canonical code phrase")] +pub struct InvalidCodephrase; + +/// Look up a word in the word list. +#[inline] +fn word(index: usize) -> &'static str { + WORDLIST[index] +} + +/// Find the index of a word in the word list. +#[inline] +fn index(word: &str) -> Result { + WORDLIST.binary_search(&word).map_err(|_| InvalidCodephrase) } #[cfg(test)] mod test { use super::*; - use sha2::{Digest as _, Sha256}; use std::collections::HashSet; fn round_trip(entropy: U256) -> String { - let codephrase = codephrase(entropy).join(WORD_SEPARATOR); - let decoded = decode_phrase(&codephrase).unwrap(); - assert_eq!(entropy, decoded); - codephrase + let codephrase = Codephrase::from_be_bytes(entropy.to_be_bytes()); + let decoded: Codephrase = codephrase.to_string().parse().unwrap(); + assert_eq!(codephrase, decoded); + codephrase.to_string() } #[test] @@ -158,44 +316,49 @@ mod test { #[test] fn non_phrases() { // Unknown words, wherever they appear. - assert!(decode_phrase("plugh").is_err()); - assert!(decode_phrase("abandon-plugh").is_err()); + assert!(Codephrase::from_str("plugh").is_err()); + assert!(Codephrase::from_str("abandon-plugh").is_err()); // More words than any entropy encodes to. let long = [word(0); PHRASE_WORDS_256 + 1].join(WORD_SEPARATOR); - assert!(decode_phrase(&long).is_err()); + assert!(Codephrase::from_str(&long).is_err()); // 24-word values of 256 bits or more: the smallest, exactly // 2^256, and the largest. let mut smallest = vec![word(8)]; smallest.extend([word(0); PHRASE_WORDS_256 - 1]); - assert!(decode_phrase(&smallest.join(WORD_SEPARATOR)).is_err()); + assert!(Codephrase::from_str(&smallest.join(WORD_SEPARATOR)).is_err()); let largest = [word(WORDLIST_LEN - 1); PHRASE_WORDS_256].join(WORD_SEPARATOR); - assert!(decode_phrase(&largest).is_err()); + assert!(Codephrase::from_str(&largest).is_err()); } #[test] fn constant_codephrases() { assert_eq!( - round_trip(U256::from_be_slice(&Sha256::digest("test phrase one"))), - "abstract-summer-orange-gown-urge-model-\ - exact-gorilla-outside-common-this-pepper-\ - pear-dust-minimum-black-double-recipe-\ - castle-crystal-clog-logic-delay-hamster" + round_trip(U256::from_be_slice( + blake3::hash(b"test phrase one").as_slice() + )), + "able-wet-frame-clown-gauge-gather-curious-\ + stereo-moment-moral-mirror-net-laptop-square-\ + toe-skill-upper-credit-cancel-flag-what-powder-\ + guide-hold" ); assert_eq!( - round_trip(U256::from_be_slice(&Sha256::digest("another test phrase"))), - "abstract-misery-favorite-ordinary-moon-talk-\ - write-coffee-digital-slogan-spray-angry-\ - once-jazz-random-income-garage-regret-accident-\ - file-release-deny-reward-drastic" + round_trip(U256::from_be_slice( + blake3::hash(b"another test phrase").as_slice() + )), + "about-item-skill-author-expose-assume-language-\ + mix-tornado-undo-dolphin-obtain-good-quarter-\ + poem-under-system-hybrid-foil-person-together-\ + output-exhaust-today" ); assert_eq!( - round_trip(U256::from_be_slice(&Sha256::digest("one more for luck!"))), - "able-dismiss-cost-scheme-amazing-slogan-\ - service-current-protect-feed-length-text-\ - cruise-wisdom-beauty-angle-regret-truck-\ - prosper-album-decline-wheel-pause-legend" + round_trip(U256::from_be_slice( + blake3::hash(b"one more for luck!").as_slice() + )), + "able-hazard-bread-decade-elegant-omit-ensure-\ + sudden-beef-voice-remove-nut-wish-bind-birth-b\ + leak-brush-joke-seven-amused-sunny-kite-flee-tape" ); } @@ -236,22 +399,17 @@ mod test { fn id_phrases() { let mut seen = HashSet::new(); for _ in 0..10_000 { - let id = generate_id(); - let n = id.split(WORD_SEPARATOR).count(); - assert_eq!(n, PHRASE_WORDS_ID); - assert!(seen.insert(id.clone()), "duplicate ID {id}"); - - let entropy = decode_phrase(&id).unwrap(); - round_trip(entropy); - assert_eq!(id, id_phrase(entropy).join(WORD_SEPARATOR)); + let id = Codephrase::random().truncate(); + assert!(seen.insert(id), "duplicate ID {id}"); + + let roundtrip = Codephrase::from_str(&id.to_string()).unwrap(); + assert_eq!(id, roundtrip); } } #[test] - fn id_leading_zero() { - let v = U256::ONE.shl_vartime(66); // 7 digits, since 2048⁶ = 2⁶⁶ - let id = id_phrase(v); - assert_eq!(id.len(), PHRASE_WORDS_ID); - assert_eq!(decode_phrase(&id.join(WORD_SEPARATOR)).unwrap(), v); + fn leading_zeros_after_truncation() { + let truncated = Codephrase::random().truncate(); + assert_eq!(U256::from_u8(0), truncated.0.shr(TRUNCATED_BITS as _)); } } diff --git a/common/src/jobs.rs b/common/src/jobs.rs index 78fbf42..78ff42c 100644 --- a/common/src/jobs.rs +++ b/common/src/jobs.rs @@ -14,7 +14,6 @@ use blake3::{Hash, Hasher, hash}; use borsh::{BorshDeserialize, BorshSerialize}; use bytesize::GB; use chrono::{DateTime, TimeDelta, Utc}; -use crypto_bigint::U256; use rlimit::Resource; use schemars::schema::{Schema, SchemaObject}; use schemars::{JsonSchema, SchemaGenerator}; @@ -24,79 +23,30 @@ use sled_hardware_types::{BaseboardId, BaseboardIdParseError}; use thiserror::Error; use crate::borsh::{ - borsh_de_datetime, borsh_de_hash, borsh_de_job_id, borsh_de_target, borsh_ser_datetime, - borsh_ser_hash, borsh_ser_target, -}; -use crate::codephrases::{ - InvalidCodephrase, WORD_SEPARATOR, decode_phrase, generate_id, id_phrase, + borsh_de_datetime, borsh_de_hash, borsh_de_target, borsh_ser_datetime, borsh_ser_hash, + borsh_ser_target, }; use crate::interactive::InteractiveJobError; use crate::keys::{KeyId, Signed, ToBeSigned, Verified}; use crate::targets::Target; -/// A globally unique identifier for a job within a session. -#[derive( - BorshDeserialize, - BorshSerialize, - Clone, - Debug, - Deserialize, - Eq, - Hash, - JsonSchema, - Ord, - PartialEq, - PartialOrd, - Serialize, -)] -#[serde(try_from = "String")] -pub struct JobId(#[borsh(deserialize_with = "borsh_de_job_id")] String); - -impl Deref for JobId { - type Target = str; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl fmt::Display for JobId { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.0) - } -} - -impl From<&Self> for JobId { - fn from(other: &Self) -> Self { - other.to_owned() - } -} - -impl From for JobId { - fn from(value: U256) -> Self { - Self(id_phrase(value).join(WORD_SEPARATOR)) - } -} - -impl FromStr for JobId { - type Err = InvalidCodephrase; - - fn from_str(s: &str) -> Result { - let p = id_phrase(decode_phrase(s)?).join(WORD_SEPARATOR); - if p == *s { - Ok(Self(p)) - } else { - Err(InvalidCodephrase) - } - } -} - -impl TryFrom for JobId { - type Error = InvalidCodephrase; - - fn try_from(s: String) -> Result { - JobId::from_str(&s) - } +codephrase_newtype! { + /// A globally unique identifier for a job within a session. + #[derive( + BorshDeserialize, + BorshSerialize, + Copy, + Clone, + Deserialize, + Eq, + Hash, + JsonSchema, + Ord, + PartialEq, + PartialOrd, + Serialize, + )] + pub struct JobId = Truncated; } impl slog::Value for JobId { @@ -106,73 +56,46 @@ impl slog::Value for JobId { key: slog::Key, serializer: &mut dyn slog::Serializer, ) -> slog::Result { - serializer.emit_str(key, self) - } -} - -/// A globally unique identifier for a session. -#[derive( - BorshDeserialize, - BorshSerialize, - Clone, - Debug, - Deserialize, - Eq, - Hash, - JsonSchema, - Ord, - PartialEq, - PartialOrd, - Serialize, -)] -pub struct SessionId(String); - -#[allow(clippy::new_without_default)] -impl SessionId { - pub fn new() -> Self { - Self(generate_id()) - } - - pub fn first_job_id(&self) -> JobId { - U256::from_be_slice(hash(self.0.as_bytes()).as_bytes()).into() - } - - pub fn next_job_id(&self, last_job: &LastJob) -> JobId { - U256::from_be_slice( - match last_job { - LastJob::None => hash(&[b"None", self.0.as_bytes()].concat()), - LastJob::Some(job) => hash(&[b"Some", job.to_be_signed().as_slice()].concat()), - LastJob::Burned(job_id) => hash(&[b"Burned", job_id.as_bytes()].concat()), - } - .as_bytes(), - ) - .into() + serializer.emit_str(key, &self.0.to_string()) } } -impl Deref for SessionId { - type Target = str; - - fn deref(&self) -> &Self::Target { - &self.0 +impl From<&JobId> for JobId { + fn from(value: &JobId) -> Self { + *value } } -impl fmt::Display for SessionId { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.0) - } +codephrase_newtype! { + /// A globally unique identifier for a session. + #[derive( + BorshDeserialize, + BorshSerialize, + Copy, + Clone, + Deserialize, + Eq, + Hash, + JsonSchema, + Ord, + PartialEq, + PartialOrd, + Serialize, + )] + pub struct SessionId = Truncated; } -impl From<&Self> for SessionId { - fn from(other: &Self) -> Self { - other.to_owned() +impl SessionId { + pub fn first_job_id(&self) -> JobId { + JobId::from_hash(hash(&self.0.to_be_bytes())) } -} -impl> From for SessionId { - fn from(s: S) -> Self { - Self(s.as_ref().to_string()) + pub fn next_job_id(&self, last_job: &LastJob) -> JobId { + JobId::from_hash(match last_job { + LastJob::None => hash(&[b"None", self.0.to_be_bytes().as_slice()].concat()), + LastJob::Some(job) => hash(&[b"Some", job.to_be_signed().as_slice()].concat()), + LastJob::Burned(job_id) => hash(&[b"Burned", job_id.to_be_bytes().as_slice()].concat()), + }) } } @@ -210,8 +133,8 @@ impl Session { } } - pub fn session_id(&self) -> &SessionId { - &self.session_id + pub fn session_id(&self) -> SessionId { + self.session_id } pub fn started_by(&self) -> Option<&KeyId> { @@ -321,7 +244,7 @@ impl ToBeSigned for JobStartRequest { target, } = self; hash_with_len(Self::TYPE_NAME); - hash_with_len(job_id.as_bytes()); + hash_with_len(&job_id.to_be_bytes()); hash_with_len(command.as_bytes()); hash_with_len(if *interactive { &[1] } else { &[0] }); hash_with_len(target.to_string().as_bytes()); diff --git a/common/src/keys.rs b/common/src/keys.rs index d26b2ae..7dcdc84 100644 --- a/common/src/keys.rs +++ b/common/src/keys.rs @@ -18,7 +18,7 @@ use borsh::io::{Error as BorshError, ErrorKind as BorshErrorKind, Read, Write}; use borsh::{BorshDeserialize, BorshSerialize}; use bytes::{Buf as _, BufMut as _, BytesMut}; use chrono::{DateTime, Utc}; -use crypto_bigint::{ArrayEncoding as _, Random as _, U128, U256}; +use crypto_bigint::{ArrayEncoding as _, Random as _, U128}; use ed25519_dalek::{ Signature as Ed25519Signature, Signer as _, SigningKey as Ed25519SigningKey, VerifyingKey as Ed25519VerifyingKey, @@ -29,7 +29,6 @@ use rand_core::OsRng; use schemars::schema::Schema; use schemars::{JsonSchema, SchemaGenerator}; use serde::{Deserialize, Serialize}; -use sha2::{Digest as _, Sha256}; use signature::Verifier; use ssh_key::{ Algorithm as SshAlgorithm, EcdsaCurve, Error as SshKeyError, Mpint, Signature as SshSignature, @@ -46,44 +45,25 @@ use x509_cert::spki::{AlgorithmIdentifierOwned, SubjectPublicKeyInfo}; use x509_cert::time::Validity; use x509_cert::{Certificate, TbsCertificate, Version}; -use crate::codephrases::{InvalidCodephrase, WORD_SEPARATOR, codephrase, decode_phrase, id_phrase}; - -/// SHA-256 of a certificate subject or an identity public key, -/// encoded as a pseudorandom code phrase for storage & transport. -#[derive( - BorshDeserialize, - BorshSerialize, - Clone, - Debug, - Deserialize, - Eq, - Hash, - JsonSchema, - Ord, - PartialEq, - PartialOrd, - Serialize, -)] -pub struct KeyId(String); - -impl Deref for KeyId { - type Target = str; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl fmt::Display for KeyId { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.0) - } -} - -impl From for KeyId { - fn from(s: String) -> Self { - Self(s) - } +use crate::codephrases::InvalidCodephrase; + +codephrase_newtype! { + /// SHA-256 of a certificate subject or an identity public key, + /// encoded as a pseudorandom code phrase for storage & transport. + #[derive( + BorshDeserialize, + BorshSerialize, + Clone, + Deserialize, + Eq, + Hash, + JsonSchema, + Ord, + PartialEq, + PartialOrd, + Serialize, + )] + pub struct KeyId = Truncated; } impl From<&Self> for KeyId { @@ -97,9 +77,8 @@ impl TryFrom<&Certificate> for KeyId { type Error = KeyError; fn try_from(cert: &Certificate) -> Result { - let hash = Sha256::digest(cert.tbs_certificate.subject_public_key_info.to_der()?); - let phrase = id_phrase(U256::from_be_slice(hash.as_slice())); - Ok(KeyId(phrase.join(WORD_SEPARATOR))) + let hash = blake3::hash(&cert.tbs_certificate.subject_public_key_info.to_der()?); + Ok(KeyId::from_hash(hash)) } } @@ -107,9 +86,8 @@ impl TryFrom<&ssh_key::PublicKey> for KeyId { type Error = KeyError; fn try_from(public_key: &ssh_key::PublicKey) -> Result { - let hash = Sha256::digest(public_key.to_bytes()?); - let phrase = id_phrase(U256::from_be_slice(hash.as_slice())); - Ok(KeyId(phrase.join(WORD_SEPARATOR))) + let hash = blake3::hash(&public_key.to_bytes()?); + Ok(KeyId::from_hash(hash)) } } @@ -187,6 +165,48 @@ impl JsonSchema for SshPublicKey { } } +codephrase_newtype! { + /// The first half of a 256 bit elliptic-curve signature: the scalar + /// `r` of an ECDSA pair, or the encoded point `R` of an Ed25519 + /// signature. It is carried as an opaque 256 bit value to which only + /// the signature algorithm assigns meaning. + #[derive( + BorshDeserialize, + BorshSerialize, + Clone, + Deserialize, + Eq, + Hash, + JsonSchema, + Ord, + PartialEq, + PartialOrd, + Serialize, + )] + pub struct EccR = Full; +} + +codephrase_newtype! { + /// The second half of a 256 bit elliptic-curve signature: the scalar + /// `s` of an ECDSA pair, or the little-endian scalar `S` of an + /// Ed25519 signature. It is carried as an opaque 256 bit value like + /// [`EccR`]. + #[derive( + BorshDeserialize, + BorshSerialize, + Clone, + Deserialize, + Eq, + Hash, + JsonSchema, + Ord, + PartialEq, + PartialOrd, + Serialize, + )] + pub struct EccS = Full; +} + /// Code phrase encoded signature. /// /// The decoded phrases `r` and `s` together comprise a signature @@ -210,8 +230,8 @@ impl JsonSchema for SshPublicKey { Serialize, )] pub struct EncodedSignature { - pub r: String, - pub s: String, + pub r: EccR, + pub s: EccS, #[serde(default, skip_serializing_if = "is_zero_flags")] pub flags: u8, @@ -241,8 +261,8 @@ impl EncodedSignature { flags: _, counter: _, } = self; - let r = decode_phrase(r)?.to_be_byte_array(); - let s = decode_phrase(s)?.to_be_byte_array(); + let r = r.to_be_bytes(); + let s = s.to_be_bytes(); match signature_algorithm { AlgorithmIdentifierOwned { oid: ECDSA_WITH_SHA_256, @@ -253,10 +273,7 @@ impl EncodedSignature { AlgorithmIdentifierOwned { oid: ID_ED_25519, parameters: None, - } => Ok(Signature::Ed25519(Ed25519Signature::from_components( - r.into(), - s.into(), - ))), + } => Ok(Signature::Ed25519(Ed25519Signature::from_components(r, s))), _ => Err(KeyError::InvalidPublicKeyAlgorithm), } } @@ -270,16 +287,13 @@ impl EncodedSignature { flags, counter, } = self; - let r = decode_phrase(r)?.to_be_byte_array(); - let s = decode_phrase(s)?.to_be_byte_array(); + let r = r.to_be_bytes(); + let s = s.to_be_bytes(); match algorithm { Ecdsa { curve: NistP256 } => Ok(Signature::EcdsaSha256( ecdsa::Signature::from_scalars(r, s)?, )), - Ed25519 => Ok(Signature::Ed25519(Ed25519Signature::from_components( - r.into(), - s.into(), - ))), + Ed25519 => Ok(Signature::Ed25519(Ed25519Signature::from_components(r, s))), SkEcdsaSha2NistP256 => { let mut bytes = BytesMut::new(); put_mpint(&mut bytes, r)?; @@ -328,17 +342,16 @@ impl Signature { } pub fn encode(&self) -> Result { - let codephrase = |x: U256| codephrase(x).join(WORD_SEPARATOR); match self { Self::EcdsaSha256(signature) => Ok(EncodedSignature { - r: codephrase(U256::from_be_byte_array(signature.r().to_bytes())), - s: codephrase(U256::from_be_byte_array(signature.s().to_bytes())), + r: EccR::from_be_bytes(signature.r().to_bytes().into()), + s: EccS::from_be_bytes(signature.s().to_bytes().into()), flags: 0, counter: 0, }), Self::Ed25519(signature) => Ok(EncodedSignature { - r: codephrase(U256::from_be_slice(signature.r_bytes())), - s: codephrase(U256::from_be_slice(signature.s_bytes())), + r: EccR::from_be_bytes(*signature.r_bytes()), + s: EccS::from_be_bytes(*signature.s_bytes()), flags: 0, counter: 0, }), @@ -349,8 +362,8 @@ impl Signature { let s = get_mpint(&mut signature)?; let e = || KeyError::InvalidSignatureEncoding; Ok(EncodedSignature { - r: codephrase(u256_be(r.as_positive_bytes().ok_or_else(e)?)?), - s: codephrase(u256_be(s.as_positive_bytes().ok_or_else(e)?)?), + r: EccR::from_be_bytes(u256_be(r.as_positive_bytes().ok_or_else(e)?)?), + s: EccS::from_be_bytes(u256_be(s.as_positive_bytes().ok_or_else(e)?)?), flags, counter, }) @@ -359,8 +372,8 @@ impl Signature { let (signature, flags, counter) = sk_split(signature.as_bytes())?; let signature = Ed25519Signature::from_slice(signature)?; Ok(EncodedSignature { - r: codephrase(U256::from_be_slice(signature.r_bytes())), - s: codephrase(U256::from_be_slice(signature.s_bytes())), + r: EccR::from_be_bytes(*signature.r_bytes()), + s: EccS::from_be_bytes(*signature.s_bytes()), flags, counter, }) @@ -421,13 +434,13 @@ impl Signature { /// A `U256` from up to 32 big-endian bytes. An mpint's minimal /// encoding may carry fewer, which `U256::from_be_slice` refuses. -fn u256_be(bytes: &[u8]) -> Result { +fn u256_be(bytes: &[u8]) -> Result<[u8; 32], KeyError> { let Some(pad) = 32usize.checked_sub(bytes.len()) else { return Err(KeyError::InvalidSignatureEncoding); }; let mut buf = [0; 32]; buf[pad..].copy_from_slice(bytes); - Ok(U256::from_be_slice(&buf)) + Ok(buf) } /// Append one SSH mpint, encoded from positive big-endian bytes. @@ -1068,10 +1081,10 @@ mod test { .unwrap(); let mut signatures = HashSet::new(); for _ in 0..100 { - let nonce = Nonce::generate(); - let signed = key.sign(nonce.as_bytes()).await.unwrap(); + let nonce = Nonce::random(); + let signed = key.sign(nonce.to_be_bytes()).await.unwrap(); assert_eq!(signed.key_id(), key.key_id()); - assert_eq!(*signed.payload(), nonce.as_bytes()); + assert_eq!(*signed.payload(), nonce.to_be_bytes()); let signature = signed.signature(); assert!(signatures.insert(signature.clone()), "duplicate signature"); let signature_string = serde_json::to_string(&signature).unwrap(); @@ -1084,7 +1097,7 @@ mod test { ); let verified = signed.verify_with_cert(key.cert()).unwrap(); assert_eq!(verified.verified_by(), key.key_id()); - assert_eq!(verified.into_payload(), nonce.as_bytes()); + assert_eq!(verified.into_payload(), nonce.to_be_bytes()); } } } diff --git a/common/src/lib.rs b/common/src/lib.rs index a88bc26..da9e5b3 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -6,6 +6,7 @@ pub mod authn; pub mod borsh; +#[macro_use] pub mod codephrases; pub mod interactive; pub mod jobs; diff --git a/server/src/executor.rs b/server/src/executor.rs index 603d119..ab96f27 100644 --- a/server/src/executor.rs +++ b/server/src/executor.rs @@ -130,9 +130,9 @@ impl Executor { }; let stop = self.shutdown.child_token(); - self.stop.insert(job_id.clone(), stop.clone()); + self.stop.insert(job_id, stop.clone()); spawn(job_spawn( - self.log.new(o!("job_id" => job_id.clone())), + self.log.new(o!("job_id" => job_id)), events, self.output_dir.clone(), verified_request, @@ -372,7 +372,7 @@ async fn job_spawn( // Announce the birth of our new job! let _ = events - .send(Event::Job(JobEvent::Start(job_id.clone(), Utc::now()))) + .send(Event::Job(JobEvent::Start(job_id, Utc::now()))) .await; // Wait for the job to die and send an event when it does. @@ -381,7 +381,7 @@ async fn job_spawn( Ok((result, state)) => { let _ = events .send(Event::Job(JobEvent::Stop( - job_id.clone(), + job_id, Utc::now(), result, state, @@ -391,7 +391,7 @@ async fn job_spawn( Err(error) => { let _ = events .send(Event::Job(JobEvent::Error( - job_id.clone(), + job_id, Utc::now(), ProcessError::Join(error.to_string()), ))) diff --git a/server/src/history.rs b/server/src/history.rs index eead89f..8a4e08b 100644 --- a/server/src/history.rs +++ b/server/src/history.rs @@ -104,7 +104,7 @@ impl JobHistory { if let Some(queued) = queued { ineligible.extend(queued.keys().cloned()); } - ineligible.extend(running.iter().map(|((id, _), _)| id.clone())); + ineligible.extend(running.iter().map(|((id, _), _)| *id)); // Cache causal jobs. let causal = self diff --git a/server/src/manager.rs b/server/src/manager.rs index 44563d5..3fc0ad4 100644 --- a/server/src/manager.rs +++ b/server/src/manager.rs @@ -256,7 +256,7 @@ impl JobManager { macro_rules! unauthorized { ($error:expr) => {{ warn!(self.log, "authentication failed"; "error" => %$error); - let nonce = Nonce::generate(); + let nonce = Nonce::random(); self.nonces.lock().await.put(nonce.clone(), Instant::now()); return Err(JobError::unauthorized(nonce)); }}; @@ -433,7 +433,7 @@ impl JobManager { move |state| { state .session() - .is_some_and(|s| *s.session_id() == session_id) + .is_some_and(|s| s.session_id() == session_id) } } @@ -493,11 +493,10 @@ impl JobManager { session_id: SessionId, wait: bool, ) -> Result<(), JobError> { - self.session_request(authn, SessionRequest::Start(session_id.clone())) + self.session_request(authn, SessionRequest::Start(session_id)) .await?; if wait { - self.wait_for(self.wait_for_session(session_id.clone())) - .await?; + self.wait_for(self.wait_for_session(session_id)).await?; } Ok(()) } diff --git a/server/src/messages.rs b/server/src/messages.rs index 37a3283..9ed896a 100644 --- a/server/src/messages.rs +++ b/server/src/messages.rs @@ -44,6 +44,7 @@ pub mod v0 { use super::*; #[derive(BorshDeserialize, BorshSerialize, Clone, Debug, Eq, PartialEq)] + #[allow(clippy::large_enum_variant)] pub enum Message { Request(Request), Event( @@ -158,6 +159,7 @@ pub mod v0 { } #[derive(BorshDeserialize, BorshSerialize, Clone, Debug, Eq, PartialEq)] + #[allow(clippy::large_enum_variant)] pub enum JobRequest { Start(SignedJob, JobStartParams), Stop(JobId), @@ -240,6 +242,8 @@ mod wire_format { use super::v0::*; use super::*; + use std::str::FromStr as _; + use sush_common::authn::Nonce; /// Serialize a message and compare against the snapshot in /// `tests/output/`, or rewrite it under `EXPECTORATE=overwrite`. @@ -264,11 +268,15 @@ mod wire_format { assert_eq!(decoded, message, "wire format should round-trip"); } + fn sid(name: &str) -> SessionId { + name.parse().unwrap() + } + #[test] fn session_start_request() { let msg: VersionedMessage = Message::Request(Request::session( - KeyId::from("zoo-zero".to_string()), - SessionRequest::Start(SessionId::from("abandon-ability")), + KeyId::from_str("zoo-zero").unwrap(), + SessionRequest::Start(sid("abandon-ability")), )) .into(); assert_wire_format("session-start-request", msg); @@ -277,8 +285,8 @@ mod wire_format { #[test] fn session_stop_request() { let msg: VersionedMessage = Message::Request(Request::session( - KeyId::from("zoo-zero".to_string()), - SessionRequest::Stop(SessionId::from("abandon-ability")), + KeyId::from_str("zoo-zero").unwrap(), + SessionRequest::Stop(sid("abandon-ability")), )) .into(); assert_wire_format("session-stop-request", msg); @@ -287,10 +295,10 @@ mod wire_format { #[test] fn session_allow_attach_request() { let msg: VersionedMessage = Message::Request(Request::session( - KeyId::from("zoo-zero".to_string()), + KeyId::from_str("zoo-zero").unwrap(), SessionRequest::AllowAttach( - SessionId::from("abandon-ability"), - KeyId::from("able-about".to_string()), + sid("abandon-ability"), + KeyId::from_str("able-about").unwrap(), Access::ReadWrite, ), )) @@ -301,10 +309,10 @@ mod wire_format { #[test] fn session_deny_attach_request() { let msg: VersionedMessage = Message::Request(Request::session( - KeyId::from("zoo-zero".to_string()), + KeyId::from_str("zoo-zero").unwrap(), SessionRequest::DenyAttach( - SessionId::from("abandon-ability"), - KeyId::from("able-about".to_string()), + sid("abandon-ability"), + KeyId::from_str("able-about").unwrap(), ), )) .into(); @@ -326,16 +334,16 @@ mod wire_format { ); let signed = Signed::new( request, - KeyId::from("zoo-zero".to_string()), + KeyId::from_str("zoo-zero").unwrap(), EncodedSignature { - r: "abandon".to_string(), - s: "zoo".to_string(), + r: "abandon".parse().unwrap(), + s: "zoo".parse().unwrap(), flags: 0, counter: 0, }, ); let msg: VersionedMessage = Message::Request(Request::job( - KeyId::from("zoo-zero".to_string()), + KeyId::from_str("zoo-zero").unwrap(), JobRequest::Start(signed, JobStartParams::default()), )) .into(); @@ -350,9 +358,11 @@ mod wire_format { // Craft deterministic evidence: nonces, then the ed25519 // basepoint as the verifier. let mut evidence = Vec::new(); - for nonce in ["abandon", "ability"] { - evidence.extend((nonce.len() as u32).to_le_bytes()); - evidence.extend(nonce.as_bytes()); + for nonce in [ + Nonce::from_str("abandon").unwrap(), + Nonce::from_str("ability").unwrap(), + ] { + evidence.extend(&nonce.to_be_bytes()); } evidence.push(0x58); evidence.extend([0x66; 31]); @@ -366,16 +376,16 @@ mod wire_format { let signed = Signed::new( response, - KeyId::from("zoo-zero".to_string()), + KeyId::from_str("zoo-zero").unwrap(), EncodedSignature { - r: "abandon".to_string(), - s: "zoo".to_string(), + r: "abandon".parse().unwrap(), + s: "zoo".parse().unwrap(), flags: 0, counter: 0, }, ); let msg: VersionedMessage = Message::Request(Request::identity( - KeyId::from("zoo-zero".to_string()), + KeyId::from_str("zoo-zero").unwrap(), IdentityRequest::Login(public_key, signed), )) .into(); diff --git a/server/src/output.rs b/server/src/output.rs index 58283f2..6fbf1e5 100644 --- a/server/src/output.rs +++ b/server/src/output.rs @@ -195,7 +195,7 @@ impl JobOutputDir { Ok(len) => Ok(len), Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(0), Err(err) => Err(ExecutionError::io( - job_id.clone(), + *job_id, format!("getting length of {}", path.display()), err, )), diff --git a/server/src/server.rs b/server/src/server.rs index 621464a..76bbb56 100644 --- a/server/src/server.rs +++ b/server/src/server.rs @@ -183,7 +183,7 @@ impl SushApi for ApiServer { .await?; let WaitParam { wait } = query.into_inner(); let SessionIdParam { session_id } = params.into_inner(); - mgr.session_start(&authn, session_id.clone(), wait).await?; + mgr.session_start(&authn, session_id, wait).await?; Ok(HttpResponseUpdatedNoContent()) } diff --git a/server/src/state.rs b/server/src/state.rs index b3fec2e..27da3ed 100644 --- a/server/src/state.rs +++ b/server/src/state.rs @@ -173,7 +173,7 @@ struct SessionGuard<'a> { } impl<'a> SessionGuard<'a> { - pub fn session_id(&self) -> &SessionId { + pub fn session_id(&self) -> SessionId { self.inner.session_id() } @@ -182,7 +182,7 @@ impl<'a> SessionGuard<'a> { } pub fn skip_job(&mut self, job_id: &JobId) { - self.inner.skip_job(job_id.clone()) + self.inner.skip_job(*job_id) } pub fn next_queued_job(&mut self) -> Option<(SignedJob, JobStartParams)> { @@ -201,7 +201,7 @@ impl<'a> SessionGuard<'a> { params: JobStartParams, actor: &KeyId, ) { - let job_id = job.job_id().clone(); + let job_id = *job.job_id(); let targeted = job.payload().target().includes(own_baseboard, cubbies); if history.contains(&job_id) { // Note but otherwise ignore the duplicate job. @@ -215,13 +215,13 @@ impl<'a> SessionGuard<'a> { // Insert the job into our queue. Every job joins the // queue to keep the causal chain whole, but only jobs // targeting this sled record a local status. - self.queued_jobs.insert(job_id.clone(), (job, params)); + self.queued_jobs.insert(job_id, (job, params)); if targeted { history.set_job_status( &job_id, own_baseboard, JobStatus::Queued { - job_id: job_id.clone(), + job_id, time_queued: Utc::now(), actor: actor.clone(), }, @@ -248,7 +248,7 @@ impl<'a> SessionGuard<'a> { None, |old_status| match old_status { None | Some(JobStatus::Queued { .. }) => Some(JobStatus::Cancelled { - job_id: job_id.clone(), + job_id: *job_id, time_cancelled: Utc::now(), actor: actor.clone(), }), @@ -285,7 +285,7 @@ impl<'a> SessionGuard<'a> { .unwrap_or(true) { executor.job_start(certs, request.clone(), params, tx_attachment); - attachments.insert(job_id.clone(), rx_attachment); + attachments.insert(job_id, rx_attachment); } self.job_started(request); } @@ -434,6 +434,7 @@ impl State { self.revoked_keys.peek(key_id).is_some() } + #[allow(clippy::result_large_err)] fn update( &mut self, log: &Logger, @@ -474,7 +475,7 @@ impl State { // Re-announcement of active session; absorb and ignore. Active { frontier, session, .. - } if session.session_id() == session_id => { + } if session.session_id() == *session_id => { *frontier |= incoming_version.clone(); info!(log, "duplicate session start"; "session_id" => %session_id); } @@ -494,10 +495,7 @@ impl State { self.session = Active { frontier: self.session.frontier() | incoming_version.clone(), started: incoming_version.clone(), - session: Box::new(Session::started( - session_id.clone(), - actor.clone(), - )), + session: Box::new(Session::started(*session_id, actor.clone())), queued_jobs: QueuedJobs::new(), attach_grants: BTreeMap::new(), } @@ -515,9 +513,9 @@ impl State { .. } if incoming_version.partial_cmp(frontier).is_none() => { let error = Error::ConcurrentSessions { - own_session: session.session_id().clone(), + own_session: session.session_id(), own_version: started.clone(), - incoming_session: session_id.clone(), + incoming_session: *session_id, incoming_version: incoming_version.clone(), }; self.session = Inactive { @@ -553,7 +551,7 @@ impl State { if let Active { frontier, session, .. } = &self.session - && session.session_id() == session_id + && session.session_id() == *session_id { info!( log, "session stopped"; @@ -570,7 +568,7 @@ impl State { attach_grants, .. } = &mut self.session - && session.session_id() == session_id + && session.session_id() == *session_id { if session.started_by() == Some(actor) { attach_grants.insert(key_id.clone(), *access); @@ -592,7 +590,7 @@ impl State { attach_grants, .. } = &mut self.session - && session.session_id() == session_id + && session.session_id() == *session_id { if session.started_by() == Some(actor) { attach_grants.remove(key_id); @@ -610,7 +608,7 @@ impl State { } (actor, SessionRequest::Skip(session_id, job_id)) => { if let Some(mut session) = self.session.active_session() - && session.session_id() == session_id + && session.session_id() == *session_id { info!( log, "job skipped"; @@ -747,13 +745,12 @@ impl State { Event::Job(job_event) => match job_event { JobEvent::Start(job_id, when) => { info!(log, "job started"; "job_id" => %job_id, "when" => %when); - self.running - .insert((job_id.clone(), baseboard_id.clone()), *when); + self.running.insert((*job_id, baseboard_id.clone()), *when); self.history.set_job_status( job_id, baseboard_id, JobStatus::Started { - job_id: job_id.clone(), + job_id: *job_id, time_started: *when, }, Some(incoming_version.rank()), @@ -766,7 +763,7 @@ impl State { if baseboard_id == &self.own_baseboard { self.attachments.remove(job_id); } - self.running.remove(&(job_id.clone(), baseboard_id.clone())); + self.running.remove(&(*job_id, baseboard_id.clone())); self.history.transition_job_status( job_id, baseboard_id, @@ -774,7 +771,7 @@ impl State { |old_status| match old_status { Some(JobStatus::Started { time_started, .. }) => { Some(JobStatus::Stopped { - job_id: job_id.clone(), + job_id: *job_id, time_started: *time_started, time_stopped: *when, result: result.clone(), @@ -795,12 +792,12 @@ impl State { if baseboard_id == &self.own_baseboard { self.attachments.remove(job_id); } - self.running.remove(&(job_id.clone(), baseboard_id.clone())); + self.running.remove(&(*job_id, baseboard_id.clone())); self.history.set_job_status( job_id, baseboard_id, JobStatus::Error { - job_id: job_id.clone(), + job_id: *job_id, time_error: *when, error: error.clone(), }, diff --git a/server/tests/common/mod.rs b/server/tests/common/mod.rs index 44dabd0..1a62c12 100644 --- a/server/tests/common/mod.rs +++ b/server/tests/common/mod.rs @@ -26,7 +26,7 @@ use sprockets_tls_test_utils::{ private_key_path, root_prefix, sprockets_auth_prefix, }; use sush_common::authn::{Challenge, ChallengeResponse, Identity, Nonce, RequestKey}; -use sush_common::codephrases::generate_id; +use sush_common::codephrases::Codephrase; use sush_common::jobs::{JobId, JobStartRequest, SignedJob}; use sush_common::keys::{EphemeralKey, KeyType, Signer as _}; use sush_common::targets::Target; @@ -126,7 +126,7 @@ pub async fn eventually(what: &str, secs: u64, mut condition: impl AsyncFnMut() pub fn ephemeral_root() -> EphemeralKey { let mut buf = [0; 8]; OsRng.fill_bytes(&mut buf); - let id = generate_id(); + let id = Codephrase::random().truncate(); EphemeralKey::new_root( KeyType::P256, format!("CN=Ephemeral Test Key {id},O=Oxide Computer Company,C=US") @@ -139,7 +139,7 @@ pub fn ephemeral_root() -> EphemeralKey { /// An authenticated identity for `key`, as `iam` would produce. pub async fn fake_identity(key: &mut EphemeralKey) -> Identity { - let challenge = Challenge::new(Nonce::generate()); + let challenge = Challenge::new(Nonce::random()); let response = ChallengeResponse::new(challenge, RequestKey::new().verifier()); let signed = key.sign(response).await.unwrap(); let verified = signed diff --git a/server/tests/distributed.rs b/server/tests/distributed.rs index bf119ee..9ee6584 100644 --- a/server/tests/distributed.rs +++ b/server/tests/distributed.rs @@ -117,16 +117,16 @@ async fn jobs_gossip_between_sleds() { // A session started on sled A becomes B's active session too. let authn_a = fake_identity(&mut root).await; let authn_b = fake_identity(&mut root).await; - let session_id = SessionId::new(); - let session = Session::new(session_id.clone()); + let session_id = SessionId::random(); + let session = Session::new(session_id); a.mgr - .session_start(&authn_a, session_id.clone(), true) + .session_start(&authn_a, session_id, true) .await .unwrap(); eventually("session gossips to B", 60, async || { b.mgr .session(&authn_b) - .is_some_and(|s| *s.session_id() == session_id) + .is_some_and(|s| s.session_id() == session_id) }) .await; @@ -161,15 +161,15 @@ async fn jobs_gossip_between_sleds() { .await; // A session started on B supersedes A's everywhere. - let successor = SessionId::new(); + let successor = SessionId::random(); b.mgr - .session_start(&authn_b, successor.clone(), true) + .session_start(&authn_b, successor, true) .await .unwrap(); eventually("supersession gossips to A", 60, async || { a.mgr .session(&authn_a) - .is_some_and(|s| *s.session_id() == successor) + .is_some_and(|s| s.session_id() == successor) }) .await; diff --git a/server/tests/output/identity-login-request.bin b/server/tests/output/identity-login-request.bin index cde4443..90e9242 100644 Binary files a/server/tests/output/identity-login-request.bin and b/server/tests/output/identity-login-request.bin differ diff --git a/server/tests/output/job-start-request.bin b/server/tests/output/job-start-request.bin index a648505..3fa78f1 100644 Binary files a/server/tests/output/job-start-request.bin and b/server/tests/output/job-start-request.bin differ diff --git a/server/tests/output/session-allow-attach-request.bin b/server/tests/output/session-allow-attach-request.bin index ffb4e3d..9ce5e5a 100644 Binary files a/server/tests/output/session-allow-attach-request.bin and b/server/tests/output/session-allow-attach-request.bin differ diff --git a/server/tests/output/session-deny-attach-request.bin b/server/tests/output/session-deny-attach-request.bin index 9f90f5c..0eacba1 100644 Binary files a/server/tests/output/session-deny-attach-request.bin and b/server/tests/output/session-deny-attach-request.bin differ diff --git a/server/tests/output/session-start-request.bin b/server/tests/output/session-start-request.bin index af0a909..cd0df69 100644 Binary files a/server/tests/output/session-start-request.bin and b/server/tests/output/session-start-request.bin differ diff --git a/server/tests/output/session-stop-request.bin b/server/tests/output/session-stop-request.bin index 635a8a0..469491a 100644 Binary files a/server/tests/output/session-stop-request.bin and b/server/tests/output/session-stop-request.bin differ diff --git a/tests/src/integration_tests.rs b/tests/src/integration_tests.rs index 67356e0..f11df1c 100644 --- a/tests/src/integration_tests.rs +++ b/tests/src/integration_tests.rs @@ -94,7 +94,7 @@ async fn client_server() { assert_eq!(iam, identity, "who am I?"); // Start a session and run a job. - let session = Session::new(SessionId::new()); + let session = Session::new(SessionId::random()); client .session_start() .session_id(session.session_id()) @@ -112,7 +112,7 @@ async fn client_server() { } = JobLimits::default(); client .job_start() - .job_id(&job_id) + .job_id(job_id) .max_cpu(max_cpu) .max_mem(max_mem) .max_fsize(max_fsize) @@ -125,7 +125,7 @@ async fn client_server() { // Check the job output. let mut output = client .job_output() - .job_id(&job_id) + .job_id(job_id) .stream(JobOutputStream::Stdout) .target("*") .send() @@ -191,7 +191,7 @@ async fn client_proxy_server() { // Attach to an interactive job through the proxy, routed by the // target path segment, and echo bytes over the bridged upgrade. - let session = Session::new(SessionId::new()); + let session = Session::new(SessionId::random()); client .session_start() .session_id(session.session_id()) @@ -209,7 +209,7 @@ async fn client_proxy_server() { } = JobLimits::default(); client .job_start() - .job_id(&job_id) + .job_id(job_id) .max_cpu(max_cpu) .max_mem(max_mem) .max_fsize(max_fsize) @@ -222,7 +222,7 @@ async fn client_proxy_server() { .expect("can't start job"); let socket = client .job_attach() - .job_id(&job_id) + .job_id(job_id) .target(test_baseboard_id().to_string()) .send() .await @@ -244,7 +244,7 @@ async fn client_proxy_server() { // A target the proxy cannot route is refused at the proxy. let Err(unrouted) = client .job_output() - .job_id(&job_id) + .job_id(job_id) .stream(JobOutputStream::Stdout) .target("913-0000019:BRM99999999") .send() @@ -534,7 +534,7 @@ async fn interactive_job() { assert_eq!(iam, identity, "who am I?"); // Start a session and run an interactive job. - let session = Session::new(SessionId::new()); + let session = Session::new(SessionId::random()); client .session_start() .session_id(session.session_id()) @@ -552,7 +552,7 @@ async fn interactive_job() { } = JobLimits::default(); client .job_start() - .job_id(&job_id) + .job_id(job_id) .max_cpu(max_cpu) .max_mem(max_mem) .max_fsize(max_fsize) @@ -567,7 +567,7 @@ async fn interactive_job() { // Attach to the job. let socket1 = client .job_attach() - .job_id(&job_id) + .job_id(job_id) .target(test_baseboard_id().to_string()) .send() .await @@ -596,7 +596,7 @@ async fn interactive_job() { // and playback. let socket2 = client .job_attach() - .job_id(&job_id) + .job_id(job_id) .target(test_baseboard_id().to_string()) .send() .await @@ -637,7 +637,7 @@ async fn interactive_job() { let guest_attach = async || { guest_client .job_attach() - .job_id(&job_id) + .job_id(job_id) .target(test_baseboard_id().to_string()) .send() .await @@ -757,7 +757,7 @@ async fn interactive_job() { // Stop the job. client .job_stop() - .job_id(&job_id) + .job_id(job_id) .wait(JobWait::Stop) .send() .await @@ -767,7 +767,7 @@ async fn interactive_job() { // from the read-only guest. let mut output = client .job_output() - .job_id(&job_id) + .job_id(job_id) .stream(JobOutputStream::Stdout) .target(test_baseboard_id().to_string()) .send() diff --git a/tests/src/manager_tests.rs b/tests/src/manager_tests.rs index eb7e53e..8fb18fa 100644 --- a/tests/src/manager_tests.rs +++ b/tests/src/manager_tests.rs @@ -104,11 +104,9 @@ async fn jobs() { let (mgr, mut root, _dir, _shutdown) = manager_and_test_root(log).await; let baseboard_id = mgr.own_baseboard(); let authn = fake_identity(&mut root).await; - let session_id = SessionId::new(); - let mut session = Session::new(session_id.clone()); - mgr.session_start(&authn, session_id.clone(), true) - .await - .unwrap(); + let session_id = SessionId::random(); + let mut session = Session::new(session_id); + mgr.session_start(&authn, session_id, true).await.unwrap(); let job_id = session.next_job_id(); let job = root.sign_job_request(&job_id, "true", false).await; @@ -230,11 +228,9 @@ async fn job_stop() { let (mgr, mut root, _dir, _shutdown) = manager_and_test_root(log).await; let baseboard_id = mgr.own_baseboard(); let authn = fake_identity(&mut root).await; - let session_id = SessionId::new(); - let mut session = Session::new(session_id.clone()); - mgr.session_start(&authn, session_id.clone(), true) - .await - .unwrap(); + let session_id = SessionId::random(); + let mut session = Session::new(session_id); + mgr.session_start(&authn, session_id, true).await.unwrap(); // Stopping a nonexistent job should mark it cancelled, and so succeed // immediately. @@ -255,8 +251,8 @@ async fn job_stop() { )); // Skip the cancelled job. - session.skip_job(job_id.clone()); - mgr.session_skip_job(&authn, session_id.clone(), job_id.clone()) + session.skip_job(job_id); + mgr.session_skip_job(&authn, session_id, job_id) .await .expect("should be able to skip cancelled job"); @@ -314,11 +310,9 @@ async fn cancel_queued_job() { let log = test_logger(function_name!()); let (mgr, mut root, _dir, _shutdown) = manager_and_test_root(log).await; let authn = fake_identity(&mut root).await; - let session_id = SessionId::new(); - let mut session = Session::new(session_id.clone()); - mgr.session_start(&authn, session_id.clone(), true) - .await - .unwrap(); + let session_id = SessionId::random(); + let mut session = Session::new(session_id); + mgr.session_start(&authn, session_id, true).await.unwrap(); // Queue job A, which won't finish soon. let command_a = "sleep 10"; @@ -394,11 +388,9 @@ async fn job_output_perms() { let dir_perms = metadata(&dir).await.unwrap().permissions(); let baseboard_id = mgr.own_baseboard(); let authn = fake_identity(&mut root).await; - let session_id = SessionId::new(); - let session = Session::new(session_id.clone()); - mgr.session_start(&authn, session_id.clone(), true) - .await - .unwrap(); + let session_id = SessionId::random(); + let session = Session::new(session_id); + mgr.session_start(&authn, session_id, true).await.unwrap(); // Run a job with some output on both streams. let command = "echo -n foo && echo -n bar >&2"; @@ -456,11 +448,9 @@ async fn cubby_targets() { .await .unwrap(); let authn = fake_identity(&mut root).await; - let session_id = SessionId::new(); - let mut session = Session::new(session_id.clone()); - mgr.session_start(&authn, session_id.clone(), true) - .await - .unwrap(); + let session_id = SessionId::random(); + let mut session = Session::new(session_id); + mgr.session_start(&authn, session_id, true).await.unwrap(); let target: Target = "14".parse().unwrap(); // With the map empty, a cubby-targeted job records no status here. @@ -559,11 +549,9 @@ async fn root_certs_from_files() { // A job signed by the configured root runs. let authn = fake_identity(&mut root).await; - let session_id = SessionId::new(); - let session = Session::new(session_id.clone()); - mgr.session_start(&authn, session_id.clone(), true) - .await - .unwrap(); + let session_id = SessionId::random(); + let session = Session::new(session_id); + mgr.session_start(&authn, session_id, true).await.unwrap(); let job_id = session.next_job_id(); let job = root.sign_job_request(&job_id, "true", false).await; mgr.job_start( @@ -634,11 +622,9 @@ async fn job_output_dir_moves() { .unwrap(); let baseboard_id = mgr.own_baseboard(); let authn = fake_identity(&mut root).await; - let session_id = SessionId::new(); - let mut session = Session::new(session_id.clone()); - mgr.session_start(&authn, session_id.clone(), true) - .await - .unwrap(); + let session_id = SessionId::random(); + let mut session = Session::new(session_id); + mgr.session_start(&authn, session_id, true).await.unwrap(); // Record a job's output under the first base. let first = session.next_job_id(); @@ -728,8 +714,8 @@ async fn universe_swap() { let authn = fake_identity(&mut root).await; async fn run_job(mgr: &JobManager, root: &mut EphemeralKey, authn: &Identity) -> JobId { - let session_id = SessionId::new(); - let session = Session::new(session_id.clone()); + let session_id = SessionId::random(); + let session = Session::new(session_id); mgr.session_start(authn, session_id, true).await.unwrap(); let job_id = session.next_job_id(); let job = root.sign_job_request(&job_id, "true", false).await; @@ -774,11 +760,9 @@ async fn shutdown() { let log = test_logger(function_name!()); let (mut mgr, mut root, _dir, shutdown) = manager_and_test_root(log).await; let authn = fake_identity(&mut root).await; - let session_id = SessionId::new(); - let session = Session::new(session_id.clone()); - mgr.session_start(&authn, session_id.clone(), true) - .await - .unwrap(); + let session_id = SessionId::random(); + let session = Session::new(session_id); + mgr.session_start(&authn, session_id, true).await.unwrap(); let command = "sleep 30"; let job_id = session.next_job_id(); @@ -887,8 +871,8 @@ async fn cert_chain() { ); // Start a job signed with the child. - let session_id = SessionId::new(); - let mut session = Session::new(session_id.clone()); + let session_id = SessionId::random(); + let mut session = Session::new(session_id); mgr.session_start(&authn, session_id, true).await.unwrap(); let job_id = session.next_job_id(); let job = child.sign_job_request(&job_id, "true", false).await; @@ -915,11 +899,9 @@ async fn attribution() { let log = test_logger(function_name!()); let (mgr, mut root, _dir, _shutdown) = manager_and_test_root(log).await; let authn = fake_identity(&mut root).await; - let session_id = SessionId::new(); - let mut session = Session::new(session_id.clone()); - mgr.session_start(&authn, session_id.clone(), true) - .await - .unwrap(); + let session_id = SessionId::random(); + let mut session = Session::new(session_id); + mgr.session_start(&authn, session_id, true).await.unwrap(); assert_eq!( mgr.session(&authn).unwrap().started_by(), Some(&authn.key_id) @@ -1028,8 +1010,8 @@ async fn revocation_tombstones() { )) .into() }; - for i in 0..200 { - peer.send(revoke(KeyId::from(format!("bogus-{i}")))); + for _ in 0..200 { + peer.send(revoke(KeyId::random())); } peer.send(revoke(doomed.key_id().clone())); @@ -1137,13 +1119,13 @@ async fn job_targets() { let log = test_logger(function_name!()); let (mgr, mut root, _dir, _shutdown) = manager_and_test_root(log).await; let authn = fake_identity(&mut root).await; - let session_id = SessionId::new(); - let mut session = Session::new(session_id.clone()); + let session_id = SessionId::random(); + let mut session = Session::new(session_id); mgr.session_start(&authn, session_id, true).await.unwrap(); let mut start = async |command: &str, target: &str| { let job_id = session.next_job_id(); - let request = JobStartRequest::new(job_id.clone(), command, false, target.parse().unwrap()); + let request = JobStartRequest::new(job_id, command, false, target.parse().unwrap()); let job = root .sign(request) .await @@ -1214,7 +1196,7 @@ async fn gossiped_identities() { // A liar claims root's key with evidence signed by their own. let bogus_key = RequestKey::new(); - let bogus = ChallengeResponse::new(Challenge::new(Nonce::generate()), bogus_key.verifier()); + let bogus = ChallengeResponse::new(Challenge::new(Nonce::random()), bogus_key.verifier()); let signed_by_liar = liar.sign(bogus).await.unwrap(); let bogus_verified = signed_by_liar .clone() @@ -1233,8 +1215,7 @@ async fn gossiped_identities() { // Real evidence authorizes here without ever logging in here. let request_key = RequestKey::new(); - let response = - ChallengeResponse::new(Challenge::new(Nonce::generate()), request_key.verifier()); + let response = ChallengeResponse::new(Challenge::new(Nonce::random()), request_key.verifier()); let signed = root.sign(response).await.unwrap(); let verified = signed.clone().verify_with_ssh_public_key(&root_pk).unwrap(); let mut credentials = Credentials::new(verified); @@ -1284,11 +1265,9 @@ async fn attach_grants() { EphemeralKey::new_root(KeyType::P256, ephemeral_test_subject(), validity).unwrap(); let guest = fake_identity(&mut guest_key).await; - let session_id = SessionId::new(); - let session = Session::new(session_id.clone()); - mgr.session_start(&owner, session_id.clone(), true) - .await - .unwrap(); + let session_id = SessionId::random(); + let session = Session::new(session_id); + mgr.session_start(&owner, session_id, true).await.unwrap(); let job_id = session.next_job_id(); let job = root.sign_job_request(&job_id, "sleep 10", false).await; mgr.job_start( @@ -1315,13 +1294,8 @@ async fn attach_grants() { Err(JobError::AttachDenied) )); assert!(matches!( - mgr.session_allow_attach( - &guest, - session_id.clone(), - owner.key_id.clone(), - Access::ReadOnly - ) - .await, + mgr.session_allow_attach(&guest, session_id, owner.key_id.clone(), Access::ReadOnly) + .await, Err(JobError::NotSessionStarter) )); @@ -1344,25 +1318,15 @@ async fn attach_grants() { .await .expect("grant never took effect") }; - mgr.session_allow_attach( - &owner, - session_id.clone(), - guest.key_id.clone(), - Access::ReadOnly, - ) - .await - .unwrap(); + mgr.session_allow_attach(&owner, session_id, guest.key_id.clone(), Access::ReadOnly) + .await + .unwrap(); granted(guest.clone(), Some(Access::ReadOnly)).await; - mgr.session_allow_attach( - &owner, - session_id.clone(), - guest.key_id.clone(), - Access::ReadWrite, - ) - .await - .unwrap(); + mgr.session_allow_attach(&owner, session_id, guest.key_id.clone(), Access::ReadWrite) + .await + .unwrap(); granted(guest.clone(), Some(Access::ReadWrite)).await; - mgr.session_deny_attach(&owner, session_id.clone(), guest.key_id.clone()) + mgr.session_deny_attach(&owner, session_id, guest.key_id.clone()) .await .unwrap(); granted(guest.clone(), None).await; @@ -1376,11 +1340,7 @@ async fn attach_grants() { peer.send( Message::Request(Request::session( guest.key_id.clone(), - SessionRequest::AllowAttach( - session_id.clone(), - guest.key_id.clone(), - Access::ReadWrite, - ), + SessionRequest::AllowAttach(session_id, guest.key_id.clone(), Access::ReadWrite), )) .into(), ); @@ -1651,11 +1611,9 @@ async fn too_much_cpu() { let (mgr, mut root, _dir, _shutdown) = manager_and_test_root(log).await; let baseboard_id = mgr.own_baseboard(); let authn = fake_identity(&mut root).await; - let session_id = SessionId::new(); - let session = Session::new(session_id.clone()); - mgr.session_start(&authn, session_id.clone(), true) - .await - .unwrap(); + let session_id = SessionId::random(); + let session = Session::new(session_id); + mgr.session_start(&authn, session_id, true).await.unwrap(); let job_id = session.next_job_id(); let command = "openssl speed sha1"; let job = root.sign_job_request(&job_id, command, false).await; @@ -1732,11 +1690,9 @@ async fn too_much_output() { let log = test_logger(function_name!()); let (mgr, mut root, _dir, _shutdown) = manager_and_test_root(log).await; let authn = fake_identity(&mut root).await; - let session_id = SessionId::new(); - let mut session = Session::new(session_id.clone()); - mgr.session_start(&authn, session_id.clone(), true) - .await - .unwrap(); + let session_id = SessionId::random(); + let mut session = Session::new(session_id); + mgr.session_start(&authn, session_id, true).await.unwrap(); let job_id = session.next_job_id(); let command = "yes"; @@ -1824,11 +1780,9 @@ async fn output_ranges() { let (mgr, mut root, _dir, _shutdown) = manager_and_test_root(log).await; let baseboard_id = mgr.own_baseboard(); let authn = fake_identity(&mut root).await; - let session_id = SessionId::new(); - let session = Session::new(session_id.clone()); - mgr.session_start(&authn, session_id.clone(), true) - .await - .unwrap(); + let session_id = SessionId::random(); + let session = Session::new(session_id); + mgr.session_start(&authn, session_id, true).await.unwrap(); let job_id = session.next_job_id(); // Read some random bytes. diff --git a/tests/src/test_utils.rs b/tests/src/test_utils.rs index 448f305..7f7a87e 100644 --- a/tests/src/test_utils.rs +++ b/tests/src/test_utils.rs @@ -25,7 +25,7 @@ use x509_cert::time::Validity; use sush_client::context::Authz; use sush_client::{Client, ResponseValue}; use sush_common::authn::{Challenge, ChallengeResponse, Credentials, Identity, Nonce, RequestKey}; -use sush_common::codephrases::generate_id; +use sush_common::codephrases::Codephrase; use sush_common::jobs::{JobId, JobStartRequest, VerifiedJob}; use sush_common::keys::{EphemeralKey, KeyType, Signer}; use sush_common::targets::{Cubbies, Target}; @@ -110,7 +110,7 @@ impl SignJobRequest for EphemeralKey { pub fn ephemeral_test_subject() -> Name { let mut buf = [0; 8]; OsRng.fill_bytes(&mut buf); - let id = generate_id(); + let id = Codephrase::random().truncate(); format!("CN=Ephemeral Test Key {id},O=Oxide Computer Company,C=US") .parse() .unwrap() @@ -145,7 +145,7 @@ pub fn test_pki(prefix: &'static str) -> (TempDir, Utf8PathBuf) { } pub async fn fake_identity(key: &mut EphemeralKey) -> Identity { - let nonce = Nonce::generate(); + let nonce = Nonce::random(); let challenge = Challenge::new(nonce.clone()); let response = ChallengeResponse::new(challenge, RequestKey::new().verifier()); let signed = key.sign(response).await.unwrap();