From 2743fb077862f9228ca0b7d1b9056b4253cdcc70 Mon Sep 17 00:00:00 2001 From: pennae Date: Sun, 17 Jul 2022 14:30:31 +0200 Subject: remove SecretBytes there's no benefit to keeping it around, the zeroing behavior it had was never any good and without it it's just a fancy [u8; N] --- src/api/auth/account.rs | 6 +-- src/api/auth/invite.rs | 4 +- src/api/auth/password.rs | 6 +-- src/crypto.rs | 99 +++++++++++------------------------------------- 4 files changed, 30 insertions(+), 85 deletions(-) diff --git a/src/api/auth/account.rs b/src/api/auth/account.rs index 56ec717..9d2a19e 100644 --- a/src/api/auth/account.rs +++ b/src/api/auth/account.rs @@ -11,7 +11,7 @@ use serde::{Deserialize, Serialize}; use validator::Validate; use crate::api::{Empty, EMPTY}; -use crate::crypto::{KeyFetchToken, SessionToken}; +use crate::crypto::{random_bytes, KeyFetchToken, SessionToken}; use crate::db::{Db, DbConn}; use crate::mailer::Mailer; use crate::push::PushClient; @@ -21,7 +21,7 @@ use crate::Config; use crate::{ api::{auth, serialize_dt}, auth::{AuthSource, Authenticated}, - crypto::{AuthPW, KeyBundle, KeyFetchReq, SecretBytes, SessionCredentials}, + crypto::{AuthPW, KeyBundle, KeyFetchReq, SessionCredentials}, types::{KeyFetchID, OauthToken, SecretKey, User, UserID, VerifyHash}, }; @@ -122,7 +122,7 @@ pub(crate) async fn create( .await?; let auth_at = db.add_session(session.token_id.clone(), &uid, session.req_hmac_key, false, None).await?; - let verify_code = hex::encode(&SecretBytes::<16>::generate().0); + let verify_code = hex::encode(&random_bytes::<16>()); db.add_verify_code(&uid, &session.token_id, &verify_code).await?; // NOTE we send the email in this context rather than a spawn to signal // send errors to the client. diff --git a/src/api/auth/invite.rs b/src/api/auth/invite.rs index f2c6ad8..e70c3d6 100644 --- a/src/api/auth/invite.rs +++ b/src/api/auth/invite.rs @@ -3,7 +3,7 @@ use chrono::{Duration, Utc}; use rocket::{http::uri::Reference, serde::json::Json, State}; use serde::{Deserialize, Serialize}; -use crate::{api::auth, auth::Authenticated, crypto::SecretBytes, db::DbConn, Config}; +use crate::{api::auth, auth::Authenticated, crypto::random_bytes, db::DbConn, Config}; use super::WithVerifiedFxaLogin; @@ -12,7 +12,7 @@ pub(crate) async fn generate_invite_link( cfg: &Config, ttl: Duration, ) -> anyhow::Result> { - let code = base64::encode_config(&SecretBytes::<32>::generate().0, URL_SAFE_NO_PAD); + let code = base64::encode_config(&random_bytes::<32>(), URL_SAFE_NO_PAD); db.add_invite_code(&code, Utc::now() + ttl).await?; Reference::parse_owned(format!("{}/#/register/{}", cfg.location, code)) .map_err(|e| anyhow!("url building failed at {e}")) diff --git a/src/api/auth/password.rs b/src/api/auth/password.rs index 79b7587..d1455e4 100644 --- a/src/api/auth/password.rs +++ b/src/api/auth/password.rs @@ -10,8 +10,8 @@ use crate::{ api::auth, auth::{AuthSource, Authenticated}, crypto::{ - AccountResetReq, AccountResetToken, AuthPW, KeyBundle, KeyFetchReq, KeyFetchToken, - PasswordChangeReq, PasswordChangeToken, SecretBytes, + random_bytes, AccountResetReq, AccountResetToken, AuthPW, KeyBundle, KeyFetchReq, + KeyFetchToken, PasswordChangeReq, PasswordChangeToken, }, db::{Db, DbConn}, mailer::Mailer, @@ -192,7 +192,7 @@ pub(crate) async fn forgot_start( return Err(auth::Error::UnverifiedAccount); } - let forgot_code = hex::encode(SecretBytes::<16>::generate().0); + let forgot_code = hex::encode(random_bytes::<16>()); let forgot_token = PasswordChangeToken::generate(); let forgot_req = PasswordChangeReq::derive_from_forgot_token(&forgot_token); db.add_password_change( diff --git a/src/crypto.rs b/src/crypto.rs index d681e86..2063c9b 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -36,39 +36,6 @@ fn xor(l: &[u8; N], r: &[u8; N]) -> [u8; N] { result } -#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(try_from = "String", into = "String")] -pub struct SecretBytes(pub [u8; N]); - -impl Debug for SecretBytes { - fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { - fmt.write_fmt(format_args!("SecretBytes {{ raw: {} }}", hex::encode(&self.0))) - } -} - -impl From> for String { - fn from(sb: SecretBytes) -> Self { - hex::encode(&sb.0) - } -} - -impl TryFrom for SecretBytes { - type Error = hex::FromHexError; - - fn try_from(value: String) -> Result { - let mut result = Self([0; N]); - hex::decode_to_slice(value, &mut result.0)?; - Ok(result) - } -} - -impl From> for Output { - fn from(s: SecretBytes<32>) -> Output { - #[allow(clippy::unwrap_used)] - Output::new(&s.0).unwrap() - } -} - mod from_hkdf { use hkdf::Hkdf; use sha2::Sha256; @@ -78,7 +45,6 @@ mod from_hkdf { // and copies never fail mod private { pub trait Seal {} - impl Seal for super::super::SecretBytes {} impl Seal for [u8; N] {} impl Seal for (L, R) {} } @@ -88,14 +54,6 @@ mod from_hkdf { fn from_hkdf(bytes: &[u8]) -> Self; } - impl FromHkdf for super::SecretBytes { - const SIZE: usize = N; - fn from_hkdf(bytes: &[u8]) -> Self { - #[allow(clippy::unwrap_used)] - Self(bytes.try_into().unwrap()) - } - } - impl FromHkdf for [u8; N] { const SIZE: usize = N; fn from_hkdf(bytes: &[u8]) -> Self { @@ -124,19 +82,8 @@ mod from_hkdf { use from_hkdf::from_hkdf; -impl SecretBytes { - pub fn generate() -> Self { - let mut result = Self([0; N]); - rand::rngs::OsRng.fill_bytes(&mut result.0); - result - } -} - #[derive(Debug, Deserialize, Serialize)] -#[serde(transparent)] -pub(crate) struct AuthPW { - pw: SecretBytes<32>, -} +pub(crate) struct AuthPW(#[serde(with = "as_hex")] [u8; 32]); pub(crate) struct StretchedPW { pw: Output, @@ -148,15 +95,16 @@ impl AuthPW { let mut buf = [0; Salt::MAX_LENGTH]; let params = scrypt::Params::new(16, 8, 1)?; let salt = salt.b64_decode(&mut buf)?; - scrypt(&self.pw.0, salt, ¶ms, &mut result)?; + scrypt(&self.0, salt, ¶ms, &mut result)?; Ok(StretchedPW { pw: Output::new(&result)? }) } } impl StretchedPW { pub fn verify_hash(&self) -> Output { - let raw: SecretBytes<32> = from_hkdf(self.pw.as_bytes(), &[NAMESPACE, b"verifyHash"]); - raw.into() + let raw: [u8; 32] = from_hkdf(self.pw.as_bytes(), &[NAMESPACE, b"verifyHash"]); + #[allow(clippy::unwrap_used)] + Output::new(&raw).unwrap() } fn wrap_wrap_key(&self) -> [u8; 32] { @@ -217,7 +165,7 @@ impl KeyFetchToken { pub(crate) struct KeyFetchReq { pub token_id: KeyFetchID, pub req_hmac_key: HawkKey, - key_request_key: SecretBytes<32>, + key_request_key: [u8; 32], } impl KeyFetchReq { @@ -233,21 +181,21 @@ impl KeyFetchReq { pub fn derive_resp(&self) -> KeyFetchResp { let (resp_hmac_key, resp_xor_key) = - from_hkdf(&self.key_request_key.0, &[NAMESPACE, b"account/keys"]); + from_hkdf(&self.key_request_key, &[NAMESPACE, b"account/keys"]); KeyFetchResp { resp_hmac_key, resp_xor_key } } } pub(crate) struct KeyFetchResp { - resp_hmac_key: SecretBytes<32>, - resp_xor_key: SecretBytes<64>, + resp_hmac_key: [u8; 32], + resp_xor_key: [u8; 64], } impl KeyFetchResp { pub fn wrap_keys(&self, keys: &KeyBundle) -> WrappedKeyBundle { - let ciphertext = xor(&self.resp_xor_key.0, &keys.to_bytes().0); + let ciphertext = xor(&self.resp_xor_key, &keys.to_bytes()); #[allow(clippy::unwrap_used)] - let mut hmac = Hmac::::new_from_slice(&self.resp_hmac_key.0).unwrap(); + let mut hmac = Hmac::::new_from_slice(&self.resp_hmac_key).unwrap(); hmac.update(&ciphertext); let hmac = *hmac.finalize().into_bytes().as_ref(); WrappedKeyBundle { ciphertext, hmac } @@ -260,10 +208,10 @@ pub(crate) struct KeyBundle { } impl KeyBundle { - pub fn to_bytes(&self) -> SecretBytes<64> { - let mut result = SecretBytes([0; 64]); - result.0[0..32].copy_from_slice(&self.ka.0); - result.0[32..].copy_from_slice(&self.wrap_kb.0); + pub fn to_bytes(&self) -> [u8; 64] { + let mut result = [0; 64]; + result[0..32].copy_from_slice(&self.ka.0); + result[32..].copy_from_slice(&self.wrap_kb.0); result } } @@ -355,7 +303,7 @@ mod test { types::SecretKey, }; - use super::{AuthPW, SecretBytes}; + use super::AuthPW; #[test] fn test_derive_session() { @@ -386,17 +334,17 @@ mod test { hex!("87b8937f61d38d0e 29cd2d5600b3f4da 0aa48ac41de36a0e fe84bb4a9872ceb7") ); assert_eq!( - key_fetch.key_request_key.0, + key_fetch.key_request_key, hex!("14f338a9e8c6324d 9e102d4e6ee83b20 9796d5c74bb734a4 10e729e014a4a546") ); let resp = key_fetch.derive_resp(); assert_eq!( - resp.resp_hmac_key.0, + resp.resp_hmac_key, hex!("f824d2953aab9faf 51a1cb65ba9e7f9e 5bf91c8d8fd1ac1c 8c2d31853a8a1210") ); assert_eq!( - resp.resp_xor_key.0, + resp.resp_xor_key, hex!( "ce7d7aa77859b235 9932970bbe2101f2 e80d01faf9191bd5 ee52181d2f0b7809 8281ba8cff392543 3a89f7c3095e0c89 900a469d60790c83 3281c4df1a11c763" @@ -412,7 +360,7 @@ mod test { )), }; assert_eq!( - bundle.to_bytes().0, + bundle.to_bytes(), hex!( "2021222324252627 28292a2b2c2d2e2f 3031323334353637 38393a3b3c3d3e3f 7effe354abecbcb2 34a8dfc2d7644b4a d339b525589738f2 d27341bb8622ecd8" @@ -443,11 +391,8 @@ mod test { #[test] fn test_stretch() -> anyhow::Result<()> { - let auth_pw = AuthPW { - pw: SecretBytes(hex!( - "247b675ffb4c4631 0bc87e26d712153a be5e1c90ef00a478 4594f97ef54f2375" - )), - }; + let auth_pw = + AuthPW(hex!("247b675ffb4c4631 0bc87e26d712153a be5e1c90ef00a478 4594f97ef54f2375")); let stretched = auth_pw.stretch( SaltString::b64_encode(&hex!( -- cgit v1.2.3