Skip to content

Commit

Permalink
udp: cookie fixups as suggested by Jose
Browse files Browse the repository at this point in the history
  • Loading branch information
da2ce7 committed Nov 19, 2024
1 parent e3562f0 commit a5e8796
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 148 deletions.
2 changes: 1 addition & 1 deletion src/bootstrap/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub fn check_seed() {
let seed = keys::Current::get_seed();
let instance = keys::Instance::get_seed();

assert_eq!(seed, instance, "maybe using zeroed see in production!?");
assert_eq!(seed, instance, "maybe using zeroed seed in production!?");
}

/// It initializes the application with the given configuration.
Expand Down
32 changes: 17 additions & 15 deletions src/servers/udp/connection_cookie.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ use aquatic_udp_protocol::ConnectionId as Cookie;
use cookie_builder::{assemble, decode, disassemble, encode};
use zerocopy::AsBytes;

use super::error::{self, Error};
use super::error::Error;
use crate::shared::crypto::keys::CipherArrayBlowfish;

/// Generates a new connection cookie.
Expand All @@ -106,6 +106,8 @@ pub fn make(fingerprint: u64, issue_at: f64) -> Result<Cookie, Error> {
Ok(zerocopy::FromBytes::read_from(cookie.as_slice()).expect("it should be the same size"))
}

use std::ops::Range;

/// Checks if the supplied `connection_cookie` is valid.
///
/// # Errors
Expand All @@ -114,32 +116,32 @@ pub fn make(fingerprint: u64, issue_at: f64) -> Result<Cookie, Error> {
///
/// # Panics
///
/// It would panic if cookie min value is larger than the max value.
pub fn check(cookie: &Cookie, fingerprint: u64, min: f64, max: f64) -> Result<f64, Error> {
assert!(min < max, "min is larger than max");
/// It would panic if the range is empty.
pub fn check(cookie: &Cookie, fingerprint: u64, valid_range: Range<f64>) -> Result<f64, Error> {
assert!(valid_range.is_empty(), "a range should be defined");

let cookie_bytes = CipherArrayBlowfish::from_slice(cookie.0.as_bytes());
let cookie_bytes = decode(*cookie_bytes);

let issue_time = disassemble(fingerprint, cookie_bytes);

if !issue_time.is_normal() {
return Err(Error::InvalidConnectionId {
bad_id: error::ConnectionCookie(*cookie),
return Err(Error::ConnectionIdNotNormal {
not_normal_value: issue_time,
});
}

if issue_time < min {
if issue_time < valid_range.start {
return Err(Error::ConnectionIdExpired {
bad_age: issue_time,
min_age: min,
expired_value: issue_time,
min_value: valid_range.start,
});
}

if issue_time > max {
if issue_time > valid_range.end {
return Err(Error::ConnectionIdFromFuture {
future_age: issue_time,
max_age: max,
future_value: issue_time,
max_value: valid_range.end,
});
}

Expand Down Expand Up @@ -262,7 +264,7 @@ mod tests {
let min = issue_at - 10.0;
let max = issue_at + 10.0;

let result = check(&cookie, fingerprint, min, max).unwrap();
let result = check(&cookie, fingerprint, min..max).unwrap();

// we should have exactly the same bytes returned
assert_eq!(result.to_ne_bytes(), issue_at.to_ne_bytes());
Expand All @@ -277,7 +279,7 @@ mod tests {
let min = issue_at + 10.0;
let max = issue_at + 20.0;

let result = check(&cookie, fingerprint, min, max).unwrap_err();
let result = check(&cookie, fingerprint, min..max).unwrap_err();

match result {
Error::ConnectionIdExpired { .. } => {} // Expected error
Expand All @@ -295,7 +297,7 @@ mod tests {
let min = issue_at - 20.0;
let max = issue_at - 10.0;

let result = check(&cookie, fingerprint, min, max).unwrap_err();
let result = check(&cookie, fingerprint, min..max).unwrap_err();

match result {
Error::ConnectionIdFromFuture { .. } => {} // Expected error
Expand Down
12 changes: 6 additions & 6 deletions src/servers/udp/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ pub enum Error {
#[error("the issue time should be a normal floating point number")]
InvalidCookieIssueTime { invalid_value: f64 },

#[error("connection id was decoded, but could not be understood")]
InvalidConnectionId { bad_id: ConnectionCookie },
#[error("connection id did not produce a normal value")]
ConnectionIdNotNormal { not_normal_value: f64 },

#[error("connection id was decoded, but was expired (too old)")]
ConnectionIdExpired { bad_age: f64, min_age: f64 },
#[error("connection id produced an expired value")]
ConnectionIdExpired { expired_value: f64, min_value: f64 },

#[error("connection id was decoded, but was invalid (from future)")]
ConnectionIdFromFuture { future_age: f64, max_age: f64 },
#[error("connection id produces a future value")]
ConnectionIdFromFuture { future_value: f64, max_value: f64 },

/// Error returned when the domain tracker returns an error.
#[error("tracker server error: {source}")]
Expand Down
Loading

0 comments on commit a5e8796

Please sign in to comment.