Skip to content

Commit c8e9425

Browse files
msardaramarkpmartonmicpapalhackeramitkumaramitami2
authored
feat: remove UUID v4 constraint on link_id, require non-empty string (agntcy#1713)
# Description The link_id field no longer needs to be a valid UUID v4. Any non-empty string is now accepted. This relaxes validation in both the config and datapath crates. ## Type of Change - [ ] Bugfix - [x] New Feature - [ ] Breaking Change - [x] Refactor - [ ] Documentation - [ ] Other (please describe) ## Checklist - [x] I have read the [contributing guidelines](/agntcy/repo-template/blob/main/CONTRIBUTING.md) - [x] Existing issues have been referenced (where applicable) - [x] I have verified this change is not present in other open pull requests - [x] Functionality is documented - [x] All code style checks pass - [x] New code contribution is covered by automated tests - [x] All new and existing tests pass --------- Signed-off-by: Mauro Sardara <msardara@cisco.com> Signed-off-by: Mark Marton <mark.p.marton@gmail.com> Signed-off-by: Michele Papalini <micpapal@cisco.com> Signed-off-by: amitami2 <amitami2@cisco.com> Signed-off-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com> Signed-off-by: Janos Sarusi-Kis <janossk@cisco.com> Signed-off-by: Mark Marton <30534230+markpmarton@users.noreply.github.com> Co-authored-by: Mark Marton <30534230+markpmarton@users.noreply.github.com> Co-authored-by: Michele Papalini <49271675+micpapal@users.noreply.github.com> Co-authored-by: Amit kumar <amit9116260192@gmail.com> Co-authored-by: amitami2 <amitami2@cisco.com> Co-authored-by: Sam Betts <1769706+Tehsmash@users.noreply.github.com> Co-authored-by: János Sarusi-Kis <janossk@cisco.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 87890e9 commit c8e9425

7 files changed

Lines changed: 42 additions & 96 deletions

File tree

data-plane/core/config/src/client.rs

Lines changed: 3 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ pub struct ClientConfig {
285285
pub metadata: Option<MetadataMap>,
286286

287287
/// Link identifier for this connection, used during link negotiation.
288-
/// Must be a valid UUID v4. Defaults to a randomly generated UUID v4.
288+
/// Defaults to a randomly generated UUID v4.
289289
#[serde(default = "default_link_id")]
290290
pub link_id: String,
291291

@@ -369,13 +369,6 @@ impl std::fmt::Display for ClientConfig {
369369
}
370370
}
371371

372-
pub fn is_valid_uuid_v4(s: &str) -> bool {
373-
match uuid::Uuid::parse_str(s) {
374-
Ok(id) => id.get_version() == Some(uuid::Version::Random),
375-
Err(_) => false,
376-
}
377-
}
378-
379372
impl Configuration for ClientConfig {
380373
type Error = ConfigError;
381374

@@ -384,11 +377,6 @@ impl Configuration for ClientConfig {
384377
return Err(ConfigError::MissingEndpoint);
385378
}
386379

387-
// Validate link_id is a UUID v4
388-
if !is_valid_uuid_v4(&self.link_id) {
389-
return Err(ConfigError::InvalidLinkId);
390-
}
391-
392380
// Validate the client configuration
393381
self.tls_setting.validate()?;
394382
validate_endpoint_scheme(&self.endpoint)?;
@@ -742,24 +730,9 @@ mod tests {
742730
}
743731

744732
#[test]
745-
fn test_validate_rejects_non_uuid_link_id() {
746-
let mut config = ClientConfig::with_endpoint("http://localhost:1234");
747-
config.link_id = "not-a-uuid".to_string();
748-
assert!(matches!(config.validate(), Err(ConfigError::InvalidLinkId)));
749-
}
750-
751-
#[test]
752-
fn test_validate_rejects_non_v4_uuid() {
733+
fn test_validate_accepts_any_link_id() {
753734
let mut config = ClientConfig::with_endpoint("http://localhost:1234");
754-
// Version 1 UUID.
755-
config.link_id = "00000000-0000-1000-8000-000000000000".to_string();
756-
assert!(matches!(config.validate(), Err(ConfigError::InvalidLinkId)));
757-
}
758-
759-
#[test]
760-
fn test_validate_accepts_default_v4_link_id() {
761-
// default_link_id() generates a v4 UUID; validation must pass.
762-
let config = ClientConfig::with_endpoint("http://localhost:1234");
735+
config.link_id = "my-custom-link-id".to_string();
763736
assert!(config.validate().is_ok());
764737
}
765738
}

data-plane/core/config/src/errors.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,6 @@ pub enum ConfigError {
108108
#[error("resolution error")]
109109
ResolutionError,
110110

111-
// Link negotiation
112-
#[error("link_id must be a valid UUID v4")]
113-
InvalidLinkId,
114-
115111
// ServerHandler routing
116112
#[error("server handler does not provide gRPC routes, but transport is gRPC")]
117113
HandlerMissingGrpcSupport,

data-plane/core/config/src/grpc/client.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
pub use crate::client::{
55
AuthenticationConfig, BackoffConfig, ClientConfig, KeepaliveConfig, TransportChannel,
6-
is_valid_uuid_v4,
76
};
87

98
use std::str::FromStr;

data-plane/core/config/src/schema/client-config.schema.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@
7272
]
7373
},
7474
"link_id": {
75-
"description": "Link identifier for this connection, used during link negotiation.\nMust be a valid UUID v4. Defaults to a randomly generated UUID v4.",
75+
"description": "Link identifier for this connection, used during link negotiation.\nDefaults to a randomly generated UUID v4.",
7676
"type": "string",
7777
"default": "5809da77-44e1-4b56-b221-de07c3b641e9"
7878
},

data-plane/core/datapath/src/connection.rs

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::api::proto::dataplane::v1::Message;
77
use aws_lc_rs::agreement::EphemeralPrivateKey;
88
use parking_lot::RwLock;
99
use semver::Version;
10-
use slim_config::client::{ClientConfig, is_valid_uuid_v4};
10+
use slim_config::client::ClientConfig;
1111
use std::net::SocketAddr;
1212
use tokio::sync::mpsc;
1313
use tokio_util::sync::CancellationToken;
@@ -228,15 +228,15 @@ impl Connection {
228228

229229
/// Atomically complete link negotiation on the server (incoming) path.
230230
///
231-
/// Validates `link_id` as a UUID v4 and stores it together with `version` under one lock.
232-
/// Returns `false` if `link_id` is not a valid UUID v4 or negotiation is already complete
231+
/// Validates `link_id` is non-empty and stores it together with `version` under one lock.
232+
/// Returns `false` if `link_id` is empty or negotiation is already complete
233233
/// (replay protection).
234234
pub fn complete_negotiation_as_server(&self, link_id: &str, version: Version) -> bool {
235235
let mut state = self.negotiation.write();
236236
if state.remote_slim_version.is_some() {
237237
return false;
238238
}
239-
if !is_valid_uuid_v4(link_id) {
239+
if link_id.is_empty() {
240240
return false;
241241
}
242242
state.link_id = Some(link_id.to_string());
@@ -347,21 +347,19 @@ mod tests {
347347
}
348348

349349
#[test]
350-
fn test_complete_negotiation_as_server_stores_valid_uuid() {
350+
fn test_complete_negotiation_as_server_stores_link_id() {
351351
let conn = server_conn();
352-
let id = uuid::Uuid::new_v4().to_string();
352+
let id = "my-custom-link-id";
353353
let v = Version::parse("1.2.3").unwrap();
354-
assert!(conn.complete_negotiation_as_server(&id, v.clone()));
355-
assert_eq!(conn.link_id(), Some(id));
354+
assert!(conn.complete_negotiation_as_server(id, v.clone()));
355+
assert_eq!(conn.link_id(), Some(id.to_string()));
356356
assert_eq!(conn.remote_slim_version(), Some(v));
357357
}
358358

359359
#[test]
360-
fn test_complete_negotiation_as_server_rejects_invalid_uuid() {
360+
fn test_complete_negotiation_as_server_rejects_empty_link_id() {
361361
let conn = server_conn();
362-
assert!(
363-
!conn.complete_negotiation_as_server("not-a-uuid", Version::parse("1.0.0").unwrap())
364-
);
362+
assert!(!conn.complete_negotiation_as_server("", Version::parse("1.0.0").unwrap()));
365363
assert!(conn.link_id().is_none());
366364
assert!(conn.remote_slim_version().is_none());
367365
}

data-plane/core/datapath/src/header_mac.rs

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ use std::cell::RefCell;
1515

1616
use aws_lc_rs::hmac;
1717
use thiserror::Error;
18-
use uuid::Uuid;
1918

2019
use crate::api::proto::dataplane::v1::{Name, SlimHeader};
2120

@@ -31,8 +30,8 @@ const TAG_LEN: usize = 32;
3130
pub enum HeaderMacError {
3231
#[error("header_mac key must be at least {MIN_KEY_LEN} bytes")]
3332
KeyTooShort,
34-
#[error("invalid link_id for header MAC")]
35-
InvalidLinkId,
33+
#[error("link_id must not be empty")]
34+
EmptyLinkId,
3635
#[error("missing SLIM header integrity tag")]
3736
MissingTag,
3837
#[error("invalid integrity tag length")]
@@ -73,13 +72,15 @@ impl HeaderMacSession {
7372
header: &mut SlimHeader,
7473
link_id: &str,
7574
) -> Result<(), HeaderMacError> {
75+
if link_id.is_empty() {
76+
return Err(HeaderMacError::EmptyLinkId);
77+
}
7678
header.header_mac = None;
77-
let uuid = link_uuid_bytes(link_id)?;
7879
PREIMAGE_BUF.with(|cell| {
7980
let mut buf = cell.borrow_mut();
8081
buf.clear();
81-
reserve_preimage_upper_bound(&mut buf, header);
82-
write_preimage(&mut buf, header, &uuid);
82+
reserve_preimage_upper_bound(&mut buf, header, link_id);
83+
write_preimage(&mut buf, header, link_id.as_bytes());
8384
let tag = hmac::sign(&self.key, buf.as_slice());
8485
header.header_mac = Some(Vec::from(tag.as_ref()));
8586
Ok(())
@@ -92,19 +93,21 @@ impl HeaderMacSession {
9293
header: &SlimHeader,
9394
link_id: &str,
9495
) -> Result<(), HeaderMacError> {
96+
if link_id.is_empty() {
97+
return Err(HeaderMacError::EmptyLinkId);
98+
}
9599
let tag = header
96100
.header_mac
97101
.as_deref()
98102
.ok_or(HeaderMacError::MissingTag)?;
99103
if tag.len() != TAG_LEN {
100104
return Err(HeaderMacError::InvalidTagLength);
101105
}
102-
let uuid = link_uuid_bytes(link_id)?;
103106
PREIMAGE_BUF.with(|cell| {
104107
let mut buf = cell.borrow_mut();
105108
buf.clear();
106-
reserve_preimage_upper_bound(&mut buf, header);
107-
write_preimage(&mut buf, header, &uuid);
109+
reserve_preimage_upper_bound(&mut buf, header, link_id);
110+
write_preimage(&mut buf, header, link_id.as_bytes());
108111
hmac::verify(&self.key, buf.as_slice(), tag)
109112
.map_err(|_| HeaderMacError::VerificationFailed)
110113
})
@@ -113,19 +116,18 @@ impl HeaderMacSession {
113116

114117
/// Ensure `buf` can hold the worst-case preimage without reallocation during `write_preimage`.
115118
#[inline]
116-
fn reserve_preimage_upper_bound(buf: &mut Vec<u8>, hdr: &SlimHeader) {
117-
let need = preimage_upper_bound(hdr);
119+
fn reserve_preimage_upper_bound(buf: &mut Vec<u8>, hdr: &SlimHeader, link_id: &str) {
120+
let need = preimage_upper_bound(hdr, link_id);
118121
let cap = buf.capacity();
119122
if need > cap {
120123
buf.reserve(need - cap);
121124
}
122125
}
123126

124127
#[inline]
125-
fn preimage_upper_bound(header: &SlimHeader) -> usize {
126-
/// Byte size of `link_uuid` field.
127-
/// * 16 bytes for `&[u8; 16]`.
128-
const LINK_UUID_SIZE: usize = 16;
128+
fn preimage_upper_bound(header: &SlimHeader, link_id: &str) -> usize {
129+
/// Byte size of the link_id length prefix (u32).
130+
const LINK_ID_LEN_PREFIX: usize = 4;
129131

130132
/// Byte size of `fanout`, serialized by [`to_le_bytes`]
131133
/// * 4 bytes for `u32`
@@ -152,7 +154,8 @@ fn preimage_upper_bound(header: &SlimHeader) -> usize {
152154
const TTL_SIZE: usize = 4;
153155

154156
DOMAIN_V1.len()
155-
+ LINK_UUID_SIZE
157+
+ LINK_ID_LEN_PREFIX
158+
+ link_id.len()
156159
+ FANOUT_SIZE
157160
+ RECV_FROM_SIZE
158161
+ FORWARD_TO_SIZE
@@ -194,12 +197,6 @@ fn encoded_name_upper_bound(name_opt: &Option<Name>) -> usize {
194197
}
195198
}
196199

197-
fn link_uuid_bytes(link_id: &str) -> Result<[u8; 16], HeaderMacError> {
198-
Uuid::parse_str(link_id)
199-
.map(|u| *u.as_bytes())
200-
.map_err(|_| HeaderMacError::InvalidLinkId)
201-
}
202-
203200
#[inline]
204201
fn push_bytes(buf: &mut Vec<u8>, data: &[u8]) {
205202
buf.extend_from_slice(&(data.len() as u32).to_le_bytes());
@@ -261,10 +258,11 @@ fn push_encoded_name(buf: &mut Vec<u8>, n: &Option<Name>) {
261258
}
262259
}
263260

264-
/// Canonical preimage: domain || link_uuid || routing header fields (no incoming_conn, no tag).
265-
fn write_preimage(buf: &mut Vec<u8>, hdr: &SlimHeader, link_uuid: &[u8; 16]) {
261+
/// Canonical preimage: domain || len(link_id) || link_id || routing header fields (no incoming_conn, no tag).
262+
fn write_preimage(buf: &mut Vec<u8>, hdr: &SlimHeader, link_id_bytes: &[u8]) {
266263
buf.extend_from_slice(DOMAIN_V1);
267-
buf.extend_from_slice(link_uuid);
264+
buf.extend_from_slice(&(link_id_bytes.len() as u32).to_le_bytes());
265+
buf.extend_from_slice(link_id_bytes);
268266
push_encoded_name(buf, &hdr.source);
269267
push_encoded_name(buf, &hdr.destination);
270268
push_bytes(buf, hdr.identity.as_bytes());
@@ -280,6 +278,7 @@ mod tests {
280278
use super::*;
281279
use crate::api::proto::dataplane::v1::{EncodedName, Name, NameId, StringName};
282280
use crate::messages::utils::DEFAULT_TTL;
281+
use uuid::Uuid;
283282

284283
fn test_key() -> Vec<u8> {
285284
b"01234567890123456789012345678901".to_vec()

data-plane/core/datapath/src/message_processing.rs

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ impl MessageProcessor {
323323
};
324324
let link_id = conn
325325
.link_id()
326-
.filter(|id| slim_config::grpc::client::is_valid_uuid_v4(id))
326+
.filter(|id| !id.is_empty())
327327
.ok_or(DataPathError::HeaderMacAwaitingLinkNegotiation(conn_index))?;
328328
mac.verify_slim_header(header, &link_id)
329329
.map_err(DataPathError::HeaderIntegrity)
@@ -710,7 +710,7 @@ impl MessageProcessor {
710710
let link_id = conn
711711
.link_id()
712712
.or_else(|| conn.config_data().map(|c| c.link_id.clone()))
713-
.filter(|id| slim_config::grpc::client::is_valid_uuid_v4(id));
713+
.filter(|id| !id.is_empty());
714714
if let Some(ref id) = link_id {
715715
let header = msg.get_slim_header_mut();
716716

@@ -1801,7 +1801,6 @@ impl DataPlaneService for MessageProcessor {
18011801
#[cfg(test)]
18021802
mod tests {
18031803
use slim_config::client::ClientConfig;
1804-
use slim_config::grpc::client::is_valid_uuid_v4;
18051804
use std::sync::Arc;
18061805
use std::time::Duration;
18071806

@@ -1868,24 +1867,6 @@ mod tests {
18681867
assert_failed_subscription_ack_is_sent(false).await;
18691868
}
18701869

1871-
#[test]
1872-
fn test_is_valid_uuid_v4_accepts_v4() {
1873-
let id = uuid::Uuid::new_v4().to_string();
1874-
assert!(is_valid_uuid_v4(&id));
1875-
}
1876-
1877-
#[test]
1878-
fn test_is_valid_uuid_v4_rejects_non_uuid_string() {
1879-
assert!(!is_valid_uuid_v4("not-a-uuid"));
1880-
assert!(!is_valid_uuid_v4(""));
1881-
}
1882-
1883-
#[test]
1884-
fn test_is_valid_uuid_v4_rejects_non_v4_uuid() {
1885-
// Version 1 UUID (time-based).
1886-
assert!(!is_valid_uuid_v4("00000000-0000-1000-8000-000000000000"));
1887-
}
1888-
18891870
// ── handle_link_message ───────────────────────────────────────────────────
18901871

18911872
#[tokio::test]
@@ -2036,11 +2017,11 @@ mod tests {
20362017
}
20372018

20382019
#[tokio::test]
2039-
async fn test_handle_link_negotiation_server_invalid_uuid_ignored() {
2020+
async fn test_handle_link_negotiation_server_empty_link_id_ignored() {
20402021
let processor = MessageProcessor::new();
20412022
let (conn_id, _rx) = make_server_conn(&processor);
20422023
let payload = LinkNegotiationPayload {
2043-
link_id: "not-a-uuid".into(),
2024+
link_id: "".into(),
20442025
slim_version: "1.0.0".into(),
20452026
is_reply: false,
20462027
link_ecdh_public_key: vec![],

0 commit comments

Comments
 (0)