Skip to content

Commit

Permalink
address code review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
neonphog committed Apr 23, 2024
1 parent 8f9b50e commit 58e4c11
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 24 deletions.
53 changes: 34 additions & 19 deletions rust/sbd-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand All @@ -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 {
Expand All @@ -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()
}
Expand All @@ -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 {
Expand All @@ -64,6 +78,7 @@ impl std::fmt::Debug for PubKey {
}
}

/// defined by sbd spec
const CMD_PREFIX: &[u8; 28] = &[0; 28];

enum MsgType<'t> {
Expand All @@ -89,56 +104,56 @@ pub struct Msg(pub Vec<u8>);
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<MsgType<'_>> {
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..],
})
}
}
Expand Down
4 changes: 2 additions & 2 deletions rust/sbd-client/src/raw_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => (),
Expand Down
6 changes: 3 additions & 3 deletions rust/sbd-client/src/send_buf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?;
Expand All @@ -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"));
}

Expand All @@ -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);
Expand Down

0 comments on commit 58e4c11

Please sign in to comment.