Skip to content

Commit e843c91

Browse files
JOHNJOHN
authored andcommitted
feat: improve error handling and add path validation
- Change derive_x25519_for_device_epoch to return Result - Add structured RateLimited error with retry_after_ms field - Add path validation in StorageBackedMessaging::new() - Update all call sites and tests
1 parent 236f60e commit e843c91

19 files changed

+256
-58
lines changed

examples/error_handling.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,10 +181,19 @@ fn main() {
181181
// In production: back off, respect limits
182182
}
183183

184-
NoiseError::RateLimited(msg) => {
184+
NoiseError::RateLimited {
185+
message,
186+
retry_after_ms,
187+
} => {
185188
// Rate limited - retry after delay
186-
eprintln!("[RATE_LIMITED] {}: {} - will retry later", context, msg);
187-
// In production: parse retry-after and wait
189+
if let Some(delay_ms) = retry_after_ms {
190+
eprintln!(
191+
"[RATE_LIMITED] {}: {} - retry after {}ms",
192+
context, message, delay_ms
193+
);
194+
} else {
195+
eprintln!("[RATE_LIMITED] {}: {} - will retry later", context, message);
196+
}
188197
}
189198

190199
NoiseError::MaxSessionsExceeded => {

fuzz/fuzz_targets/fuzz_handshake.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@ impl RingKeyProvider for FuzzRing {
1818
device_id: &[u8],
1919
epoch: u32,
2020
) -> Result<[u8; 32], NoiseError> {
21-
Ok(pubky_noise::kdf::derive_x25519_for_device_epoch(
22-
&self.seed, device_id, epoch,
23-
))
21+
pubky_noise::kdf::derive_x25519_for_device_epoch(&self.seed, device_id, epoch)
2422
}
2523

2624
fn ed25519_pubkey(&self, _kid: &str) -> Result<[u8; 32], NoiseError> {

fuzz/fuzz_targets/fuzz_kdf.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,17 @@ struct KdfInput {
1616

1717
fuzz_target!(|input: KdfInput| {
1818
// Test derive_x25519_for_device_epoch
19-
let sk = derive_x25519_for_device_epoch(&input.seed, &input.device_id, input.epoch);
19+
let sk = derive_x25519_for_device_epoch(&input.seed, &input.device_id, input.epoch)
20+
.expect("HKDF should never fail with valid inputs");
2021

2122
// Verify the key is clamped correctly (X25519 requirements)
2223
assert_eq!(sk[0] & 7, 0, "Low 3 bits should be cleared");
2324
assert_eq!(sk[31] & 128, 0, "High bit should be cleared");
2425
assert_eq!(sk[31] & 64, 64, "Bit 6 should be set");
2526

2627
// Verify determinism
27-
let sk2 = derive_x25519_for_device_epoch(&input.seed, &input.device_id, input.epoch);
28+
let sk2 = derive_x25519_for_device_epoch(&input.seed, &input.device_id, input.epoch)
29+
.expect("HKDF should never fail with valid inputs");
2830
assert_eq!(sk, sk2);
2931

3032
// Test x25519_pk_from_sk doesn't panic

fuzz/fuzz_targets/fuzz_noise_link.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,7 @@ impl RingKeyProvider for FuzzRing {
2020
device_id: &[u8],
2121
epoch: u32,
2222
) -> Result<[u8; 32], NoiseError> {
23-
Ok(pubky_noise::kdf::derive_x25519_for_device_epoch(
24-
&self.seed, device_id, epoch,
25-
))
23+
pubky_noise::kdf::derive_x25519_for_device_epoch(&self.seed, device_id, epoch)
2624
}
2725

2826
fn ed25519_pubkey(&self, _kid: &str) -> Result<[u8; 32], NoiseError> {

src/errors.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,13 @@ pub enum NoiseError {
6666
#[error("policy violation: {0}")]
6767
Policy(String),
6868

69-
#[error("rate limited: {0}")]
70-
RateLimited(String),
69+
#[error("rate limited: {message}")]
70+
RateLimited {
71+
message: String,
72+
/// Optional retry delay in milliseconds.
73+
/// When present, the client should wait at least this long before retrying.
74+
retry_after_ms: Option<u64>,
75+
},
7176

7277
#[error("maximum sessions exceeded for this identity")]
7378
MaxSessionsExceeded,
@@ -108,7 +113,7 @@ impl NoiseError {
108113
Self::IdentityVerify => NoiseErrorCode::IdentityVerify,
109114
Self::RemoteStaticMissing => NoiseErrorCode::RemoteStaticMissing,
110115
Self::Policy(_) => NoiseErrorCode::Policy,
111-
Self::RateLimited(_) => NoiseErrorCode::RateLimited,
116+
Self::RateLimited { .. } => NoiseErrorCode::RateLimited,
112117
Self::MaxSessionsExceeded => NoiseErrorCode::MaxSessionsExceeded,
113118
Self::SessionExpired(_) => NoiseErrorCode::SessionExpired,
114119
Self::InvalidPeerKey => NoiseErrorCode::InvalidPeerKey,
@@ -133,18 +138,15 @@ impl NoiseError {
133138
Self::Network(_)
134139
| Self::Timeout(_)
135140
| Self::ConnectionReset(_)
136-
| Self::RateLimited(_)
141+
| Self::RateLimited { .. }
137142
| Self::Storage(_)
138143
)
139144
}
140145

141146
/// Returns a suggested retry delay in milliseconds, if applicable.
142147
pub fn retry_after_ms(&self) -> Option<u64> {
143148
match self {
144-
Self::RateLimited(msg) => {
145-
// Try to parse retry-after from message if encoded
146-
msg.parse().ok()
147-
}
149+
Self::RateLimited { retry_after_ms, .. } => *retry_after_ms,
148150
Self::Network(_) | Self::Timeout(_) | Self::ConnectionReset(_) => Some(1000),
149151
Self::Storage(_) => Some(500),
150152
_ => None,

src/ffi/config.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::ffi::errors::FfiNoiseError;
12
use crate::ffi::types::FfiMobileConfig;
23
use crate::mobile_manager::MobileConfig;
34

@@ -30,13 +31,23 @@ pub fn performance_config() -> FfiMobileConfig {
3031
.into()
3132
}
3233

34+
/// Derive an X25519 device key from seed, device ID, and epoch.
35+
///
36+
/// # Errors
37+
///
38+
/// Returns `FfiNoiseError::Other` if key derivation fails (extremely rare).
3339
#[uniffi::export]
34-
pub fn derive_device_key(seed: Vec<u8>, device_id: Vec<u8>, epoch: u32) -> Vec<u8> {
40+
pub fn derive_device_key(
41+
seed: Vec<u8>,
42+
device_id: Vec<u8>,
43+
epoch: u32,
44+
) -> Result<Vec<u8>, FfiNoiseError> {
3545
let mut seed_arr = [0u8; 32];
3646
if seed.len() >= 32 {
3747
seed_arr.copy_from_slice(&seed[0..32]);
3848
}
39-
crate::kdf::derive_x25519_for_device_epoch(&seed_arr, &device_id, epoch).to_vec()
49+
let sk = crate::kdf::derive_x25519_for_device_epoch(&seed_arr, &device_id, epoch)?;
50+
Ok(sk.to_vec())
4051
}
4152

4253
#[uniffi::export]

src/ffi/errors.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,11 @@ pub enum FfiNoiseError {
3939
Decryption { message: String },
4040

4141
#[error("Rate limited: {message}")]
42-
RateLimited { message: String },
42+
RateLimited {
43+
message: String,
44+
/// Optional retry delay in milliseconds.
45+
retry_after_ms: Option<u64>,
46+
},
4347

4448
#[error("Maximum sessions exceeded")]
4549
MaxSessionsExceeded,
@@ -69,7 +73,13 @@ impl From<NoiseError> for FfiNoiseError {
6973
NoiseError::Timeout(msg) => FfiNoiseError::Timeout { message: msg },
7074
NoiseError::Storage(msg) => FfiNoiseError::Storage { message: msg },
7175
NoiseError::Decryption(msg) => FfiNoiseError::Decryption { message: msg },
72-
NoiseError::RateLimited(msg) => FfiNoiseError::RateLimited { message: msg },
76+
NoiseError::RateLimited {
77+
message,
78+
retry_after_ms,
79+
} => FfiNoiseError::RateLimited {
80+
message,
81+
retry_after_ms,
82+
},
7383
NoiseError::MaxSessionsExceeded => FfiNoiseError::MaxSessionsExceeded,
7484
NoiseError::SessionExpired(msg) => FfiNoiseError::SessionExpired { message: msg },
7585
NoiseError::ConnectionReset(msg) => FfiNoiseError::ConnectionReset { message: msg },

src/kdf.rs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,34 @@
1+
use crate::errors::NoiseError;
12
use hkdf::Hkdf;
23
use sha2::Sha512;
34
use zeroize::Zeroizing;
45

5-
pub fn derive_x25519_for_device_epoch(seed: &[u8; 32], device_id: &[u8], epoch: u32) -> [u8; 32] {
6+
/// Derive an X25519 secret key from a seed, device ID, and epoch.
7+
///
8+
/// Uses HKDF-SHA512 with a fixed salt to derive keys deterministically.
9+
/// The resulting key is clamped for X25519 compatibility.
10+
///
11+
/// # Errors
12+
///
13+
/// Returns `NoiseError::Other` if HKDF expansion fails (should never happen
14+
/// with valid 32-byte output, but we handle it explicitly for robustness).
15+
pub fn derive_x25519_for_device_epoch(
16+
seed: &[u8; 32],
17+
device_id: &[u8],
18+
epoch: u32,
19+
) -> Result<[u8; 32], NoiseError> {
620
let salt = b"pubky-noise-x25519:v1";
721
let hk = Hkdf::<Sha512>::new(Some(salt), seed);
822
let mut info = Vec::with_capacity(device_id.len() + 4);
923
info.extend_from_slice(device_id);
1024
info.extend_from_slice(&epoch.to_le_bytes());
1125
let mut sk = [0u8; 32];
12-
hk.expand(&info, &mut sk).expect("hkdf expand");
26+
hk.expand(&info, &mut sk)
27+
.map_err(|e| NoiseError::Other(format!("HKDF expand failed: {:?}", e)))?;
1328
sk[0] &= 248;
1429
sk[31] &= 127;
1530
sk[31] |= 64;
16-
sk
31+
Ok(sk)
1732
}
1833

1934
pub fn x25519_pk_from_sk(sk: &[u8; 32]) -> [u8; 32] {

src/mobile_manager.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -453,8 +453,8 @@ impl<R: RingKeyProvider> NoiseManager<R> {
453453
.get(session_id)
454454
.ok_or_else(|| NoiseError::Other("Session state not found".to_string()))?;
455455

456-
let mut messaging =
457-
StorageBackedMessaging::new(link, session, public_client, write_path, read_path);
456+
let messaging =
457+
StorageBackedMessaging::new(link, session, public_client, write_path, read_path)?;
458458

459459
// Configure with retry settings appropriate for mobile
460460
let retry_config = RetryConfig {
@@ -468,7 +468,7 @@ impl<R: RingKeyProvider> NoiseManager<R> {
468468
operation_timeout_ms: 30000,
469469
};
470470

471-
messaging = messaging
471+
let messaging = messaging
472472
.with_counters(state.write_counter, state.read_counter)
473473
.with_retry_config(retry_config);
474474

src/pubky_ring.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@ impl RingKeyProvider for PubkyRingProvider {
2727
epoch: u32,
2828
) -> Result<[u8; 32], NoiseError> {
2929
let seed = self.keypair.secret_key();
30-
let sk = crate::kdf::derive_x25519_for_device_epoch(&seed, device_id, epoch);
31-
Ok(sk)
30+
crate::kdf::derive_x25519_for_device_epoch(&seed, device_id, epoch)
3231
}
3332

3433
fn ed25519_pubkey(&self, _kid: &str) -> Result<[u8; 32], NoiseError> {

0 commit comments

Comments
 (0)