From 58e4c1121c64ac29bd23918c160b47a8f3a6dedf Mon Sep 17 00:00:00 2001 From: neonphog Date: Tue, 23 Apr 2024 11:36:19 -0600 Subject: [PATCH] address code review comments --- rust/sbd-client/src/lib.rs | 53 ++++++++++++++++++++----------- rust/sbd-client/src/raw_client.rs | 4 +-- rust/sbd-client/src/send_buf.rs | 6 ++-- 3 files changed, 39 insertions(+), 24 deletions(-) diff --git a/rust/sbd-client/src/lib.rs b/rust/sbd-client/src/lib.rs index b00dabe..bd344a3 100644 --- a/rust/sbd-client/src/lib.rs +++ b/rust/sbd-client/src/lib.rs @@ -7,6 +7,18 @@ use std::sync::Arc; /// defined by the sbd spec const MAX_MSG_SIZE: usize = 20_000; +/// defined by ed25519 spec +const PK_SIZE: usize = 32; + +/// defined by ed25519 spec +const SIG_SIZE: usize = 64; + +/// sbd spec defines headers to be the same size as ed25519 pub keys +const HDR_SIZE: usize = PK_SIZE; + +/// defined by sbd spec +const NONCE_SIZE: usize = 32; + #[cfg(feature = "raw_client")] pub mod raw_client; #[cfg(not(feature = "raw_client"))] @@ -17,16 +29,18 @@ mod send_buf; /// Crypto to use. Note, the pair should be fresh for each new connection. pub trait Crypto { /// The pubkey. - fn pub_key(&self) -> &[u8; 32]; + fn pub_key(&self) -> &[u8; PK_SIZE]; /// Sign the nonce. - fn sign(&self, nonce: &[u8]) -> [u8; 64]; + fn sign(&self, nonce: &[u8]) -> [u8; SIG_SIZE]; } #[cfg(feature = "crypto")] mod default_crypto { + use super::*; + /// Default signer. Use a fresh one for every new connection. - pub struct DefaultCrypto([u8; 32], ed25519_dalek::SigningKey); + pub struct DefaultCrypto([u8; PK_SIZE], ed25519_dalek::SigningKey); impl Default for DefaultCrypto { fn default() -> Self { @@ -38,11 +52,11 @@ mod default_crypto { } impl super::Crypto for DefaultCrypto { - fn pub_key(&self) -> &[u8; 32] { + fn pub_key(&self) -> &[u8; PK_SIZE] { &self.0 } - fn sign(&self, nonce: &[u8]) -> [u8; 64] { + fn sign(&self, nonce: &[u8]) -> [u8; SIG_SIZE] { use ed25519_dalek::Signer; self.1.sign(nonce).to_bytes() } @@ -53,7 +67,7 @@ pub use default_crypto::*; /// Public key. #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct PubKey(pub [u8; 32]); +pub struct PubKey(pub [u8; PK_SIZE]); impl std::fmt::Debug for PubKey { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -64,6 +78,7 @@ impl std::fmt::Debug for PubKey { } } +/// defined by sbd spec const CMD_PREFIX: &[u8; 28] = &[0; 28]; enum MsgType<'t> { @@ -89,56 +104,56 @@ pub struct Msg(pub Vec); impl Msg { /// Get a reference to the slice containing the pubkey data. pub fn pub_key_ref(&self) -> &[u8] { - &self.0[..32] + &self.0[..PK_SIZE] } /// Extract a pubkey from the message. pub fn pub_key(&self) -> PubKey { - PubKey(self.0[..32].try_into().unwrap()) + PubKey(self.0[..PK_SIZE].try_into().unwrap()) } /// Get a reference to the slice containing the message data. pub fn message(&self) -> &[u8] { - &self.0[32..] + &self.0[PK_SIZE..] } // -- private -- // fn parse(&self) -> Result> { - if self.0.len() < 32 { + if self.0.len() < PK_SIZE { return Err(Error::other("invalid message length")); } if &self.0[..28] == CMD_PREFIX { - match &self.0[28..32] { + match &self.0[28..HDR_SIZE] { b"lbrt" => { - if self.0.len() != 32 + 4 { + if self.0.len() != HDR_SIZE + 4 { return Err(Error::other("invalid lbrt length")); } Ok(MsgType::LimitByteNanos(i32::from_be_bytes( - self.0[32..].try_into().unwrap(), + self.0[PK_SIZE..].try_into().unwrap(), ))) } b"lidl" => { - if self.0.len() != 32 + 4 { + if self.0.len() != HDR_SIZE + 4 { return Err(Error::other("invalid lidl length")); } Ok(MsgType::LimitIdleMillis(i32::from_be_bytes( - self.0[32..].try_into().unwrap(), + self.0[HDR_SIZE..].try_into().unwrap(), ))) } b"areq" => { - if self.0.len() != 32 + 32 { + if self.0.len() != HDR_SIZE + NONCE_SIZE { return Err(Error::other("invalid areq length")); } - Ok(MsgType::AuthReq(&self.0[32..])) + Ok(MsgType::AuthReq(&self.0[HDR_SIZE..])) } b"srdy" => Ok(MsgType::Ready), _ => Ok(MsgType::Unknown), } } else { Ok(MsgType::Msg { - pub_key: &self.0[..32], - message: &self.0[32..], + pub_key: &self.0[..PK_SIZE], + message: &self.0[PK_SIZE..], }) } } diff --git a/rust/sbd-client/src/raw_client.rs b/rust/sbd-client/src/raw_client.rs index 31e08a1..85611ea 100644 --- a/rust/sbd-client/src/raw_client.rs +++ b/rust/sbd-client/src/raw_client.rs @@ -185,12 +185,12 @@ impl Handshake { MsgType::LimitIdleMillis(l) => limit_idle_millis = l, MsgType::AuthReq(nonce) => { let sig = crypto.sign(nonce); - let mut auth_res = Vec::with_capacity(32 + 64); + let mut auth_res = Vec::with_capacity(HDR_SIZE + SIG_SIZE); auth_res.extend_from_slice(CMD_PREFIX); auth_res.extend_from_slice(b"ares"); auth_res.extend_from_slice(&sig); send.send(auth_res).await?; - bytes_sent += 32 + 64; + bytes_sent += HDR_SIZE + SIG_SIZE; } MsgType::Ready => break, MsgType::Unknown => (), diff --git a/rust/sbd-client/src/send_buf.rs b/rust/sbd-client/src/send_buf.rs index b00077f..abdb175 100644 --- a/rust/sbd-client/src/send_buf.rs +++ b/rust/sbd-client/src/send_buf.rs @@ -92,7 +92,7 @@ impl SendBuf { // first check if we need to keepalive if now - self.last_send >= self.idle_keepalive_nanos { - let mut data = Vec::with_capacity(32); + let mut data = Vec::with_capacity(HDR_SIZE); data.extend_from_slice(CMD_PREFIX); data.extend_from_slice(b"keep"); self.raw_send(now, data).await?; @@ -117,7 +117,7 @@ impl SendBuf { /// Note, you'll need to explicitly call `write_next_queued()` or /// make additional sends in order to get this queued data actually sent. pub async fn send(&mut self, pk: &PubKey, data: &[u8]) -> Result<()> { - if data.len() > MAX_MSG_SIZE - 32 { + if data.len() > MAX_MSG_SIZE - PK_SIZE { return Err(Error::other("message too large")); } @@ -128,7 +128,7 @@ impl SendBuf { self.write_next_queued().await?; } - let mut buf = Vec::with_capacity(32 + data.len()); + let mut buf = Vec::with_capacity(PK_SIZE + data.len()); buf.extend_from_slice(&pk.0[..]); buf.extend_from_slice(data); self.buf.push_back(buf);