Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Next

* Adds a validated MD5 authentication string type that zeroizes its key on drop

## [0.1.7] - 2026-08-13

* Adds validated unicast link-local IPv4, IPv6, and dual-stack address types
Expand Down
21 changes: 21 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ ipnetwork = { version = "0.21.1", optional = true }
macaddr = { version = "1.0.1", optional = true }
sha1 = { version = "0.11.0", optional = true }
rand = { version = "0.10.2", optional = true }
zeroize = { version = "1.9.0", features = ["zeroize_derive"] }

[dev-dependencies]
expectorate = "1.2.0"
Expand Down
13 changes: 13 additions & 0 deletions all_schemas.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,19 @@
"version": "0.1.0"
}
},
"Md5AuthString": {
"title": "An MD5 authentication string",
"description": "A nonempty printable ASCII string of at most 80 bytes",
"type": "string",
"maxLength": 80,
"minLength": 1,
"pattern": "^[ -~]+$",
"x-rust-type": {
"crate": "oxnet",
"path": "oxnet::Md5AuthString",
"version": "0.1.8"
}
},
"UnicastLinkLocalIpAddr": {
"oneOf": [
{
Expand Down
3 changes: 3 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

mod ipaddr;
mod ipnet;
mod md5;
mod multicast;
#[cfg(feature = "schemars")]
mod schema_util;
Expand All @@ -24,6 +25,8 @@ pub use ipaddr::{
#[cfg(feature = "ula")]
pub use ipnet::{UlaBuildError, UlaBuilder};

pub use md5::{Md5AuthString, Md5AuthStringError};

pub use multicast::MulticastMac;

pub use sockaddr::{SocketAddrJson, SocketAddrV4Json, SocketAddrV6Json};
243 changes: 243 additions & 0 deletions src/md5.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,243 @@
// Copyright 2026 Oxide Computer Company

use std::hash::{Hash, Hasher};
use zeroize::{ZeroizeOnDrop, Zeroizing};

/// An MD5 authentication key represented as a printable ASCII string.
///
/// The key contains between 1 and 80 bytes, inclusive, and every byte is in
/// the printable ASCII range (`0x20..=0x7e`). This follows the recommendation
/// for TCP MD5 keys in RFC 2385 section 4.5.
///
/// The [`Debug`](std::fmt::Debug) implementation redacts the key, and its
/// allocation is zeroized when the value is dropped. Converting it into a
/// [`String`] transfers responsibility for zeroizing that allocation to the
/// caller. Its serialized representation contains the key as a plain string.
Comment on lines +12 to +15

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// The [`Debug`](std::fmt::Debug) implementation redacts the key, and its
/// allocation is zeroized when the value is dropped. Converting it into a
/// [`String`] transfers responsibility for zeroizing that allocation to the
/// caller. Its serialized representation contains the key as a plain string.
/// The [`Debug`](std::fmt::Debug) implementation redacts the key, and its
/// allocation is zeroed when the value is dropped. Converting it into a
/// [`String`] should therefore be done with caution since no zeroing may occur.

This raises a question for me. Where are we using this type where we are ensuring that places where the string occurs are zeroed? For example, if this is ever formatted into a query string or POST body by progenitor we're almost certainly not zeroing it. Is this even useful at all?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently this type isn't used anywhere, but the intention is to integrate it into the mgd API as an optional parameter for BGP peers as a replacement for the current raw String. The flow of this data currently is that it comes in via dropshot, gets stored in a config struct owned by a per BGP peer, and that data is copied out when we make a call into libnet to interact with a PF_KEY socket. I don't believe the libnet type uses zeroize, but that could always be updated to match.

I'm not as familiar with the dropshot side of things or what would be needed to provide similar zeroing.

As I said before, I'm not so strongly opinionated here as to say zeroing is a must. I would be okay removing it if the consensus is that the utility is limited / non-existent.

#[derive(Clone, Eq, PartialEq, ZeroizeOnDrop)]
pub struct Md5AuthString(Zeroizing<String>);

impl Md5AuthString {
/// Maximum key length in bytes.
pub const MAX_LEN: usize = 80;

/// Creates an MD5 authentication string after validating its contents.
pub fn new(source: String) -> Result<Self, Md5AuthStringError> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we prefer new to TryFrom?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not particularly. I'd lean towards having both over just one though.

let source = Zeroizing::new(source);

if source.is_empty() {
return Err(Md5AuthStringError::Empty);
}

if source.len() > Self::MAX_LEN {
return Err(Md5AuthStringError::TooLong { len: source.len() });
}

if !source.chars().all(|c| c.is_ascii_graphic() || c == ' ') {
return Err(Md5AuthStringError::NotPrintableAscii);
}

Ok(Self(source))
}

/// Returns the key as a byte slice.
pub fn as_bytes(&self) -> &[u8] {
self.0.as_bytes()
}

/// Returns the key as a string slice.
pub fn as_str(&self) -> &str {
&self.0
}

/// Returns the underlying string, transferring responsibility for
/// zeroizing it to the caller.
Comment on lines +52 to +53

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Returns the underlying string, transferring responsibility for
/// zeroizing it to the caller.
/// Returns the underlying string, transferring responsibility for
/// zeroing it to the caller.

pub fn into_inner(mut self) -> String {
std::mem::take(&mut *self.0)
}
}

impl Hash for Md5AuthString {
fn hash<H: Hasher>(&self, state: &mut H) {
self.as_str().hash(state);
}
}

impl TryFrom<String> for Md5AuthString {
type Error = Md5AuthStringError;

fn try_from(source: String) -> Result<Self, Self::Error> {
Self::new(source)
}
}

impl From<Md5AuthString> for String {
fn from(source: Md5AuthString) -> Self {
source.into_inner()
}
}

impl std::fmt::Debug for Md5AuthString {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str("Md5AuthString(<redacted>)")
}
}

#[cfg(feature = "serde")]
impl<'de> serde::Deserialize<'de> for Md5AuthString {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
let source = <String as serde::Deserialize>::deserialize(deserializer)?;
Self::new(source).map_err(serde::de::Error::custom)
}
}

#[cfg(feature = "serde")]
impl serde::Serialize for Md5AuthString {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
serializer.serialize_str(self.as_str())
}
}

#[cfg(feature = "schemars")]
impl schemars::JsonSchema for Md5AuthString {
fn schema_name() -> String {
"Md5AuthString".to_string()
}

fn json_schema(_: &mut schemars::gen::SchemaGenerator) -> schemars::schema::Schema {
schemars::schema::SchemaObject {
metadata: Some(Box::new(schemars::schema::Metadata {
title: Some("An MD5 authentication string".to_string()),
description: Some(
"A nonempty printable ASCII string of at most 80 bytes".to_string(),
),
..Default::default()
})),
instance_type: Some(schemars::schema::InstanceType::String.into()),
string: Some(Box::new(schemars::schema::StringValidation {
max_length: Some(Self::MAX_LEN as u32),
min_length: Some(1),
pattern: Some(r"^[ -~]+$".to_string()),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely non-obvious and needs a comment

})),
extensions: crate::schema_util::extension("Md5AuthString", "0.1.8"),
..Default::default()
}
.into()
}
}

impl std::error::Error for Md5AuthStringError {}

/// An error returned when an MD5 authentication string violates its required
/// invariants.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum Md5AuthStringError {
/// The string is empty.
Empty,
/// The string exceeds [`Md5AuthString::MAX_LEN`] bytes.
TooLong {
/// The actual string length in bytes.
len: usize,
},
/// The string contains a byte outside the printable ASCII range.
NotPrintableAscii,
}

impl std::fmt::Display for Md5AuthStringError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Empty => write!(f, "MD5 auth string must not be empty"),
Self::TooLong { len } => write!(
f,
"MD5 auth string length must be <= {}, found {len}",
Md5AuthString::MAX_LEN
),
Self::NotPrintableAscii => write!(
f,
"MD5 auth string must be fully comprised of printable ASCII characters"
),
}
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn accepts_printable_ascii_within_length_limit() {
for byte in b' '..=b'~' {
let source = char::from(byte).to_string();
assert_eq!(Md5AuthString::new(source.clone()).unwrap().as_str(), source);
}

let source = "x".repeat(Md5AuthString::MAX_LEN);
let key = Md5AuthString::new(source.clone()).unwrap();
assert_eq!(key.as_str(), source);
assert_eq!(key.as_bytes(), source.as_bytes());
assert_eq!(String::from(key), source);
}

#[test]
fn rejects_strings_outside_invariants() {
assert_eq!(
Md5AuthString::new(String::new()),
Err(Md5AuthStringError::Empty)
);

let len = Md5AuthString::MAX_LEN + 1;
assert_eq!(
Md5AuthString::new("x".repeat(len)),
Err(Md5AuthStringError::TooLong { len })
);

for source in ["line\nfeed", "tab\tkey", "nul\0key", "non-ASCII-é"] {
assert_eq!(
Md5AuthString::new(source.to_string()),
Err(Md5AuthStringError::NotPrintableAscii)
);
}
}

#[test]
fn debug_redacts_inner_string() {
let key = Md5AuthString::new("super secret".to_string()).unwrap();
assert_eq!(format!("{key:?}"), "Md5AuthString(<redacted>)");
}

#[cfg(all(feature = "serde", feature = "schemars"))]
#[test]
fn serde_round_trip_preserves_invariants() {
let key = Md5AuthString::new("secret key".to_string()).unwrap();
let json = serde_json::to_string(&key).unwrap();
assert_eq!(json, r#""secret key""#);
assert_eq!(serde_json::from_str::<Md5AuthString>(&json).unwrap(), key);

assert!(serde_json::from_str::<Md5AuthString>(r#"""#).is_err());
assert!(serde_json::from_str::<Md5AuthString>(r#""line\nfeed""#).is_err());
}

#[cfg(feature = "schemars")]
#[test]
fn json_schema_matches_invariants() {
let schema = schemars::schema_for!(Md5AuthString);
let validation = schema.schema.string.expect("string validation");

assert_eq!(validation.min_length, Some(1));
assert_eq!(validation.max_length, Some(Md5AuthString::MAX_LEN as u32));
assert_eq!(validation.pattern.as_deref(), Some(r"^[ -~]+$"));
assert_eq!(
schema.schema.extensions.get("x-rust-type"),
Some(&serde_json::json!({
"crate": "oxnet",
"version": "0.1.8",
"path": "oxnet::Md5AuthString",
}))
);
}
}
1 change: 1 addition & 0 deletions src/schema_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ mod tests {
let _ = gen.subschema_for::<UnicastLinkLocalIpAddr>();
let _ = gen.subschema_for::<UnicastLinkLocalIpv4Addr>();
let _ = gen.subschema_for::<UnicastLinkLocalIpv6Addr>();
let _ = gen.subschema_for::<Md5AuthString>();

/// Object to validate types with inlined schemas.
#[derive(schemars::JsonSchema)]
Expand Down
Loading