diff --git a/crates/agent-identity-conformance/COVERAGE.md b/crates/agent-identity-conformance/COVERAGE.md index 6732d0749..e36216d9a 100644 --- a/crates/agent-identity-conformance/COVERAGE.md +++ b/crates/agent-identity-conformance/COVERAGE.md @@ -14,6 +14,8 @@ The `p_*` tests check an authority and the `a_*` tests the agent; the tests name | §2 `u` read as text: `user@`, empty user name, `@` in path, non-ASCII host | `a_pending_file_deleted_on_permanent_error` | | §2.1 precedence of `token_url_mismatch` | `p_enroll_csr_url_refusal_precedes_key_lookup_and_token_state` (over the key lookup, a revoked token and an exhausted one), `p_enroll_csr_url_refusal_precedes_token_expiry`; `p_enroll_csr_must_name_token_url` (`invalid_request` for a CSR that also has a bad self-signature) | | §3 metadata limits | `p_metadata_limits`, `p_channel_hello_invalid_metadata_rejected` | +| §3 invisible format characters | `p_metadata_invisible_format_characters` (each end of every listed range refused in the hostname, and U+202E in an unknown key, at enrollment without consuming a token use; each code point just outside a range and the unlisted `Cf` code points U+0600, U+110BD and U+1D173 accepted and stored as sent), `p_channel_hello_invalid_metadata_rejected` (U+E007F in `Hello`); the harness test `invisible_format_character_lists_match_the_contract` pins both lists | +| §3.1 invisible format characters removed, empty keys `unknown` or omitted | `a_metadata_invisible_format_characters_removed`, through the metadata override, which also checks that the agent keeps the code points beside the list and omits an optional key (`domain`) left empty; the metadata check of every agent test refuses a listed code point in any value the agent sent, which the harness test `agent_metadata_check_refuses_listed_format_characters_only` pins | | §4 certificates, §5.1 trust anchors | `p_trust_anchor_lists_roots`, `p_renew_rejects_sha384_csr`, `p_compressed_point_csr_rejected`, `p_csr_bad_self_signature_rejected`, the `csr` vectors; `p_failed_enrollment_consumes_nothing` sends the `bad_self_signature` and `ecdsa_with_sha384` vectors at enrollment, where their enrollment URL (`https://example.test/dvls`) is not the target's `u`, and §2.1 orders `invalid_request` before the `token_url_mismatch` it would draw, so it isolates their rules there | | §4 chain signature algorithms, CA basicConstraints and keyCertSign | the chain check of `p_trust_anchor_lists_roots`, `p_reset_rebases_root_validity`, `p_renew_happy_path`, `p_renew_within_grace`, `p_rotation_publishes_both_roots_and_issues_from_new`, `p_rotation_grace_renewal_after_early_completion` and `p_rotation_old_root_cert_renewable_after_deadline`, which verifies the root's self-signature too; the harness tests `chain_verifies_p384_intermediate_and_refuses_other_algorithms` and `chain_requires_ca_extensions_on_every_issuer` pin that it accepts a P-384 issuer and refuses another curve and each missing CA extension | | §4 leaf extensions | the leaf profile check of `p_trust_anchor_lists_roots` (enroll), `p_renew_happy_path` and `p_renew_within_grace` (renew); the harness test `leaf_extensions_outside_the_profile_are_named` pins that the check refuses basicConstraints and names it, and refuses a leaf without the enrollment URL extension, with two, with a critical one, with one holding a `UTF8String` or naming another URL | @@ -47,6 +49,9 @@ The `p_*` tests check an authority and the `a_*` tests the agent; the tests name - Verified by review only: the randomness of secrets, nonces and challenges, and the stability of `authority_id` across restores. - Known gaps, with no test yet: - the §1 conventions beyond the error body, timestamps, repeated members, nesting depth, invalid Unicode and canonical `base64`; + - a renew or check-in body whose metadata holds a control character or an invisible format character (§3), which `p_metadata_limits` and `p_metadata_invisible_format_characters` send only at enrollment and `p_channel_hello_invalid_metadata_rejected` only in `Hello`; + - a code point inside a listed range of §3 other than its ends; + - the agent's removal of C0, DEL and C1 control characters (§3.1), and its removal of invisible format characters at renew, check-in and `Hello`, because no test sets such a value in the metadata override before those sends; - an `agent_channel_url` holding an `@`, which the agent treats as absent (§5.2, §7.2), because no mock fault delivers such a URL; - the shape of the agent's own CSR (§4: one attribute, an `extensionRequest` holding only the enrollment URL extension) and the URL its renewal CSR names, because the mock reports no CSR it received; the mock accepting the agent's enrollments shows only that their CSRs name the token's `u` where the mock reads it; - the agent's check of an enrollment that returns a known authority ID from another base URL ("Pending-enrollment files" in `crates/agent-identity/INTENT.md`), such as a chain through a P-384 CA accepted and an RSA-signed chain refused with `authority_conflict`, because a mock serves one base URL and issues only from its own root. diff --git a/crates/agent-identity-conformance/src/agent.rs b/crates/agent-identity-conformance/src/agent.rs index 1c3268fe1..0f182b1d4 100644 --- a/crates/agent-identity-conformance/src/agent.rs +++ b/crates/agent-identity-conformance/src/agent.rs @@ -23,8 +23,9 @@ use tokio::process::{Child, Command}; use x509_cert::Certificate; use crate::client::{ - Target, Token, count, decoded_certificate, expect_error, expect_status, field, token_bag, token_base_url, - with_bag_url, with_upper_case_scheme, + Target, Token, count, decoded_certificate, expect_error, expect_status, field, + invisible_format_character_range_ends, is_invisible_format_character, token_bag, token_base_url, + unlisted_format_character_neighbors, with_bag_url, with_upper_case_scheme, }; use crate::dvls::{CertificateStatus, IdentityQuery, IdentityRecord, IdentityStatus, RotationDeadline}; use crate::{Context, KeyBackend}; @@ -234,6 +235,10 @@ fn validate_agent_metadata(metadata: &Value, agent_version: Option<&str>) -> any .all(|character| character >= '\u{20}' && !('\u{7f}'..='\u{9f}').contains(&character)), "agent metadata {key} contains a C0/C1 control character" ); + ensure!( + !value.chars().any(is_invisible_format_character), + "agent metadata {key} contains an invisible format character" + ); total_bytes += key.len() + value.len(); } ensure!(total_bytes <= 16 * 1024, "agent metadata exceeds 16 KiB"); @@ -843,10 +848,18 @@ impl AgentCase { } fn set_metadata_hostname(&mut self, hostname: &str) -> anyhow::Result<()> { + self.set_metadata_override(&json!({ "hostname": hostname }))?; + self.metadata_hostname = hostname.to_owned(); + Ok(()) + } + + /// Replaces the metadata override with `values`, which the agent reads before its next send. + /// + /// It leaves `metadata_hostname`, the hostname that the metadata checks expect, to the caller. + fn set_metadata_override(&self, values: &Value) -> anyhow::Result<()> { let replacement = self.path().join("identity-metadata-override-next.json"); - std::fs::write(&replacement, serde_json::to_vec(&json!({ "hostname": hostname }))?)?; + std::fs::write(&replacement, serde_json::to_vec(values)?)?; std::fs::rename(&replacement, self.metadata_override_path())?; - self.metadata_hostname = hostname.to_owned(); Ok(()) } @@ -4473,6 +4486,51 @@ pub(crate) async fn a_channel_connected_and_metadata(ctx: Context) -> anyhow::Re .await } +/// CONTRACT.agent-identity.md §3.1: the agent "removes [...] the invisible format characters of §3" from each value it sends, and "A required key that ends up empty gets the value `unknown`". +/// +/// The metadata override replaces collected values before each send (the agent's intent, "Configuration"), so the values this test sets go through that removal. +/// The hostname holds every end of a listed range before, between and after code points that the list leaves out, which the agent keeps. +/// The OS name, a required key, and the domain, an optional one, hold only listed code points, so the agent sends `unknown` for the first and omits the second. +pub(crate) async fn a_metadata_invisible_format_characters_removed(ctx: Context) -> anyhow::Result<()> { + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §agent-metadata + // oracle: intent devolutions-agent/src/identity/INTENT.md §Configuration + with_agent(ctx, None, true, async move |case| { + let target = case.ctx.target.clone(); + let listed = invisible_format_character_range_ends(); + let only_listed = listed.iter().collect::(); + let mut hostname = format!("{only_listed}host"); + let mut expected = "host".to_owned(); + for (separator, kept) in listed.iter().cycle().zip(unlisted_format_character_neighbors()) { + hostname.push(*separator); + hostname.push(kept); + expected.push(kept); + } + hostname.push_str("name"); + hostname.push_str(&only_listed); + expected.push_str("name"); + case.set_metadata_override(&json!({ "hostname": hostname, "os_name": only_listed, "domain": only_listed }))?; + let (id, _) = case.enroll_new(&target, 1).await?; + let record = case.identity_record(&target, &id).await?; + ensure!( + record.metadata_value("hostname") == Some(&*expected), + "agent sent hostname {:?} instead of {expected:?}", + record.metadata_value("hostname") + ); + ensure!( + record.metadata_value("os_name") == Some("unknown"), + "agent sent os_name {:?} for a value made only of invisible format characters", + record.metadata_value("os_name") + ); + ensure!( + record.metadata_value("domain").is_none(), + "agent sent domain {:?} for a value made only of invisible format characters", + record.metadata_value("domain") + ); + Ok(()) + }) + .await +} + pub(crate) async fn a_make_before_break_on_renewal(ctx: Context) -> anyhow::Result<()> { // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §agent-renewal // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §channel-termination @@ -6038,6 +6096,37 @@ mod tests { use super::*; + #[test] + fn agent_metadata_check_refuses_listed_format_characters_only() { + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata + let metadata = |hostname: &str| { + let mut metadata = json!({ + "hostname": hostname, + "os_name": "os", + "os_version": "1", + "arch": std::env::consts::ARCH, + "agent_version": "1", + }); + // The check compares `machine_id` with the host's on Linux, so the fixture carries the host's. + if cfg!(target_os = "linux") + && let Ok(machine_id) = std::fs::read_to_string("/etc/machine-id") + { + metadata["machine_id"] = machine_id.trim().into(); + } + metadata + }; + validate_agent_metadata(&metadata("host\u{2065}name"), None).expect("U+2065 is outside the list"); + for character in ['\u{00AD}', '\u{E007F}'] { + let refused = validate_agent_metadata(&metadata(&format!("host{character}name")), None) + .expect_err(&format!("U+{:04X} is listed", u32::from(character))); + assert!( + refused.to_string().contains("invisible format character"), + "U+{:04X} was refused for another reason: {refused}", + u32::from(character) + ); + } + } + #[test] fn key_names_require_this_runs_prefix_and_a_canonical_uuid() -> anyhow::Result<()> { // oracle: intent crates/device-key/INTENT.md §Key name diff --git a/crates/agent-identity-conformance/src/client.rs b/crates/agent-identity-conformance/src/client.rs index b48344098..ee7b37503 100644 --- a/crates/agent-identity-conformance/src/client.rs +++ b/crates/agent-identity-conformance/src/client.rs @@ -1,3 +1,4 @@ +use std::ops::RangeInclusive; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; @@ -464,10 +465,124 @@ pub(crate) fn decoded_certificate(certificate: &str) -> anyhow::Result> .context("decode certificate") } +/// The invisible format characters that no metadata value holds, as [CONTRACT.agent-identity.md] §3 "Metadata" lists them. +/// +/// [CONTRACT.agent-identity.md]: ../../../docs/agent-identity/CONTRACT.agent-identity.md +// oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata +const INVISIBLE_FORMAT_CHARACTERS: [RangeInclusive; 11] = [ + '\u{00AD}'..='\u{00AD}', + '\u{061C}'..='\u{061C}', + '\u{180E}'..='\u{180E}', + '\u{200B}'..='\u{200F}', + '\u{202A}'..='\u{202E}', + '\u{2060}'..='\u{2064}', + '\u{2066}'..='\u{206F}', + '\u{FEFF}'..='\u{FEFF}', + '\u{FFF9}'..='\u{FFFB}', + '\u{E0001}'..='\u{E0001}', + '\u{E0020}'..='\u{E007F}', +]; + +pub(crate) fn is_invisible_format_character(character: char) -> bool { + INVISIBLE_FORMAT_CHARACTERS + .iter() + .any(|range| range.contains(&character)) +} + +/// The first and last code point of each range of [`INVISIBLE_FORMAT_CHARACTERS`], in order. +/// +/// An implementation that holds the list as ranges goes wrong at their ends, so these are the code points worth sending. +/// +/// A single code point is both ends of its range, and appears once. +pub(crate) fn invisible_format_character_range_ends() -> Vec { + let mut ends = INVISIBLE_FORMAT_CHARACTERS + .iter() + .flat_map(|range| [*range.start(), *range.end()]) + .collect::>(); + ends.dedup(); + ends +} + +/// Code points that a metadata value may hold although an implementation could take them for invisible format characters. +/// +/// They are each code point just outside a range of [`INVISIBLE_FORMAT_CHARACTERS`], which a range off by one refuses, and format characters (`Cf`) that the closed list leaves out, which a filter by general category refuses. +pub(crate) fn unlisted_format_character_neighbors() -> Vec { + let mut neighbors = INVISIBLE_FORMAT_CHARACTERS + .iter() + .flat_map(|range| [u32::from(*range.start()) - 1, u32::from(*range.end()) + 1]) + .filter_map(char::from_u32) + .filter(|character| !is_invisible_format_character(*character)) + .collect::>(); + // U+2065 is just outside both U+2060–U+2064 and U+2066–U+206F. + neighbors.dedup(); + // oracle: spec Unicode 15.0.0 UnicodeData.txt §General_Category + // U+0600 ARABIC NUMBER SIGN, U+110BD KAITHI NUMBER SIGN and U+1D173 MUSICAL SYMBOL BEGIN BEAM are each `Cf`. + neighbors.extend(['\u{0600}', '\u{110BD}', '\u{1D173}']); + neighbors +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn invisible_format_character_lists_match_the_contract() { + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata + assert_eq!( + invisible_format_character_range_ends(), + [ + '\u{00AD}', + '\u{061C}', + '\u{180E}', + '\u{200B}', + '\u{200F}', + '\u{202A}', + '\u{202E}', + '\u{2060}', + '\u{2064}', + '\u{2066}', + '\u{206F}', + '\u{FEFF}', + '\u{FFF9}', + '\u{FFFB}', + '\u{E0001}', + '\u{E0020}', + '\u{E007F}', + ] + ); + let neighbors = unlisted_format_character_neighbors(); + assert_eq!( + neighbors, + [ + '\u{00AC}', + '\u{00AE}', + '\u{061B}', + '\u{061D}', + '\u{180D}', + '\u{180F}', + '\u{200A}', + '\u{2010}', + '\u{2029}', + '\u{202F}', + '\u{205F}', + '\u{2065}', + '\u{2070}', + '\u{FEFE}', + '\u{FF00}', + '\u{FFF8}', + '\u{FFFC}', + '\u{E0000}', + '\u{E0002}', + '\u{E001F}', + '\u{E0080}', + '\u{0600}', + '\u{110BD}', + '\u{1D173}', + ] + ); + assert!(!neighbors.into_iter().any(is_invisible_format_character)); + } + #[test] fn error_body_server_time_must_end_in_z() -> anyhow::Result<()> { // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §errors diff --git a/crates/agent-identity-conformance/src/main.rs b/crates/agent-identity-conformance/src/main.rs index 0c8ddac62..350bc2243 100644 --- a/crates/agent-identity-conformance/src/main.rs +++ b/crates/agent-identity-conformance/src/main.rs @@ -810,6 +810,7 @@ const TESTS: &[Test] = &[ p!(p_enroll_certificate_key_reuse_rules), p!(p_renew_certificate_key_reuse_rules), p!(p_metadata_limits), + p!(p_metadata_invisible_format_characters), p!(p_friendly_name_format), p!(p_friendly_name_white_space_and_truncation), p!(p_duplicate_json_members_rejected), @@ -970,6 +971,7 @@ const TESTS: &[Test] = &[ a!(a_confirm_expired_pending_beyond_grace_rejects, mock_only), a!(a_expired_current_renews_in_grace_or_rejects_beyond, mock_only).channel(), a!(a_channel_connected_and_metadata).channel(), + a!(a_metadata_invisible_format_characters_removed), a!(a_make_before_break_on_renewal).channel(), a!(a_old_key_deleted_after_confirm_without_channel, mock_only).channel(), a!(a_request_renewal_connected).channel().need(Need::LongLeaves), diff --git a/crates/agent-identity-conformance/src/protocol.rs b/crates/agent-identity-conformance/src/protocol.rs index af310360a..bb08982b3 100644 --- a/crates/agent-identity-conformance/src/protocol.rs +++ b/crates/agent-identity-conformance/src/protocol.rs @@ -26,7 +26,8 @@ use x509_cert::ext::pkix::{ use crate::client::{ Identity, Reply, Target, Token, count, decoded_certificate, expect_error, expect_status, field, - https_authority_and_path, token_bag, token_base_url, with_upper_case_scheme, + https_authority_and_path, invisible_format_character_range_ends, token_bag, token_base_url, + unlisted_format_character_neighbors, with_upper_case_scheme, }; use crate::dvls::{ self, CertificateStatus, Credential, IdentityQuery, IdentityRecord, IdentityStatus, Permission, Refusal, @@ -1163,6 +1164,60 @@ pub(crate) async fn p_metadata_limits(ctx: Context) -> anyhow::Result<()> { Ok(()) } +/// CONTRACT.agent-identity.md §3: a metadata value holds "no invisible format character", a closed list of code points. +/// +/// An authority that holds the list as ranges goes wrong at their ends, so each refused enrollment holds one end of a range, inserted into the hostname of the accepted control and otherwise identical to it. +/// The control hostname holds each code point that [`unlisted_format_character_neighbors`] names, and so does an unknown key per code point, which §3 stores as-is. +/// One more refused enrollment holds a listed code point in an unknown key, since §3 applies to every value. +pub(crate) async fn p_metadata_invisible_format_characters(ctx: Context) -> anyhow::Result<()> { + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata + let neighbors = unlisted_format_character_neighbors().into_iter().collect::(); + let mut control = serde_json::Map::new(); + control.insert("hostname".to_owned(), json!(format!("host{neighbors}name"))); + for character in neighbors.chars() { + control.insert( + format!("u{:04x}", u32::from(character)), + json!(format!("host{character}name")), + ); + } + let mut refused = Vec::new(); + for character in invisible_format_character_range_ends() { + let mut metadata = control.clone(); + metadata.insert("hostname".to_owned(), json!(format!("host{character}{neighbors}name"))); + refused.push((format!("hostname holding U+{:04X}", u32::from(character)), metadata)); + } + let mut metadata = control.clone(); + metadata.insert("custom".to_owned(), json!("host\u{202E}name")); + refused.push(("unknown key holding U+202E".to_owned(), metadata)); + // One use per refused enrollment, so that an authority counting them still leaves the control a use, and the count names the fault. + let token = token(&ctx, refused.len() as u64 + 1).await?; + for (case, metadata) in refused { + let reply = ctx + .target + .enroll(&token.text, &KeyPair::generate()?, &Value::Object(metadata)) + .await?; + expect_error(&reply, 400, "invalid_request").context(case)?; + } + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §token-rules + ensure!( + ctx.target.management().token(&token.id).await?.used_count == 0, + "an enrollment refused for an invisible format character consumed a token use" + ); + let key = KeyPair::generate()?; + let control = Value::Object(control); + let reply = ctx.target.enroll(&token.text, &key, &control).await?; + let enrolled = Identity::from_enrollment(&token.text, key, &reply) + .context("enrollment holding only code points outside the list of invisible format characters")?; + let record = identity_record(&ctx.target, &enrolled.identity_id).await?; + for (name, value) in control.as_object().into_iter().flatten() { + ensure!( + record.metadata_value(name) == value.as_str(), + "metadata {name} was not stored as sent" + ); + } + Ok(()) +} + pub(crate) async fn p_friendly_name_format(ctx: Context) -> anyhow::Result<()> { // oracle: intent docs/agent-identity/INTENT.agent-identity.md §Enrollment // oracle: intent Devolutions/RDM@fe54fbb839b5f42b3278eb1b0d6b8989051e4a19:DVLS/Server/server/Common/AgentIdentity/INTENT.md §Enrollment tokens @@ -3481,6 +3536,8 @@ pub(crate) async fn p_channel_hello_invalid_metadata_rejected(ctx: Context) -> a ("an uppercase key", vec![("Hostname", "host")]), ("a 65-character key", vec![(long_key.as_str(), "host")]), ("a control character", vec![("hostname", "bad\u{7}host")]), + // The last listed code point is outside the Basic Multilingual Plane, where a check over UTF-16 code units goes wrong. + ("an invisible format character", vec![("hostname", "bad\u{E007F}host")]), ("a 1025-byte value", vec![("hostname", long_value.as_str())]), ("33 keys", numbered_keys.iter().map(|key| (key.as_str(), "v")).collect()), ( @@ -7618,8 +7675,13 @@ pub(crate) async fn p_friendly_name_white_space_and_truncation(ctx: Context) -> // oracle: intent docs/agent-identity/INTENT.agent-identity.md §Enrollment let cases = [ ("\u{3000}\u{a0}name\u{2003}\u{1680}", "name".to_owned()), - // U+200B ZERO WIDTH SPACE is not `White_Space`. - ("\u{200b}name\u{200b}", "\u{200b}name\u{200b}".to_owned()), + // U+0600 ARABIC NUMBER SIGN and U+1D173 MUSICAL SYMBOL BEGIN BEAM are format characters (`Cf`) but neither `White_Space` nor listed in CONTRACT.agent-identity.md §3, so a hostname may hold them, and a trim that also removes format characters fails here. + // U+0600 is one UTF-16 code unit and U+1D173 two, so a trim over code units fails on the first and a trim over scalar values on both. + // oracle: spec Unicode 18.0.0 PropList.txt §White_Space + // oracle: spec Unicode 15.0.0 UnicodeData.txt §General_Category + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata + ("\u{0600}name\u{0600}", "\u{0600}name\u{0600}".to_owned()), + ("\u{1D173}name\u{1D173}", "\u{1D173}name\u{1D173}".to_owned()), (&*format!("{}{}", " ".repeat(10), "x".repeat(300)), "x".repeat(255)), ( &*format!("{}\u{3000}{}", "a".repeat(254), "b".repeat(10)), diff --git a/crates/agent-identity-mock/src/name_eval.rs b/crates/agent-identity-mock/src/name_eval.rs index b8f00401d..06e0a7f97 100644 --- a/crates/agent-identity-mock/src/name_eval.rs +++ b/crates/agent-identity-mock/src/name_eval.rs @@ -1,7 +1,7 @@ //! Metadata limits ([CONTRACT.agent-identity.md][contract], "3. Metadata") and friendly-name format evaluation //! ([shared intent][intent], "Enrollment"). //! -//! [contract]: ../../../docs/agent-identity/CONTRACT.agent-identity.md#3-metadata +//! [contract]: ../../../docs/agent-identity/CONTRACT.agent-identity.md#metadata //! [intent]: ../../../docs/agent-identity/INTENT.agent-identity.md#enrollment use serde_json::{Map, Value}; @@ -49,6 +49,11 @@ pub(crate) fn validate_metadata(metadata: &Map) -> Result<(), Str { return Err(format!("metadata value for {key:?} contains control characters")); } + if value.chars().any(is_invisible_format_character) { + return Err(format!( + "metadata value for {key:?} contains invisible format characters" + )); + } total += key.len() + value.len(); } if total > MAX_METADATA_TOTAL_BYTES { @@ -57,6 +62,28 @@ pub(crate) fn validate_metadata(metadata: &Map) -> Result<(), Str Ok(()) } +/// Whether `c` is an invisible format character that [CONTRACT.agent-identity.md][contract] ("3. Metadata") refuses in a metadata value. +/// +/// The contract lists them as code points rather than as Unicode general category `Cf`, so that the agent and every authority agree on the set whatever Unicode version their runtimes ship. +/// +/// [contract]: ../../../docs/agent-identity/CONTRACT.agent-identity.md#metadata +fn is_invisible_format_character(c: char) -> bool { + matches!( + c, + '\u{ad}' + | '\u{61c}' + | '\u{180e}' + | '\u{200b}'..='\u{200f}' + | '\u{202a}'..='\u{202e}' + | '\u{2060}'..='\u{2064}' + | '\u{2066}'..='\u{206f}' + | '\u{feff}' + | '\u{fff9}'..='\u{fffb}' + | '\u{e0001}' + | '\u{e0020}'..='\u{e007f}' + ) +} + fn valid_metadata_key(key: &str) -> bool { let mut chars = key.chars(); match chars.next() { @@ -170,8 +197,8 @@ mod tests { Map::from_iter([("hostname".to_owned(), Value::String(value.to_owned()))]) } - /// CONTRACT.agent-identity.md §3 limits: at most 32 keys, each matching `^[a-z][a-z0-9_]{0,63}$`, each value at - /// most 1024 UTF-8 bytes without a C0, DEL or C1 control character, and at most 16 KiB of keys and values in all. + /// CONTRACT.agent-identity.md §3 limits: at most 32 keys, each matching `^[a-z][a-z0-9_]{0,63}$`, each value at most 1024 UTF-8 bytes without a C0, DEL or C1 control character or a listed invisible format character, and at most 16 KiB of keys and values in all. + /// The tests below cover the invisible format characters. // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata #[rstest] #[case::keys_32(entries(32, 1), true)] @@ -193,6 +220,55 @@ mod tests { assert_eq!(validate_metadata(&metadata).is_ok(), accepted); } + /// Whether a value holding `c` between other characters is accepted. + fn accepts_value_holding(c: char) -> bool { + validate_metadata(&with_value(&format!("host{c}name"))).is_ok() + } + + /// CONTRACT.agent-identity.md §3 refuses a value holding an invisible format character, which it lists as these code points and ranges, so the code points on either side of each are accepted. + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata + #[rstest] + #[case::soft_hyphen('\u{ad}', '\u{ad}')] + #[case::arabic_letter_mark('\u{61c}', '\u{61c}')] + #[case::mongolian_vowel_separator('\u{180e}', '\u{180e}')] + #[case::zero_width_and_directional_marks('\u{200b}', '\u{200f}')] + #[case::directional_embeddings_and_overrides('\u{202a}', '\u{202e}')] + #[case::word_joiner_and_invisible_operators('\u{2060}', '\u{2064}')] + #[case::directional_isolates_and_deprecated_formats('\u{2066}', '\u{206f}')] + #[case::zero_width_no_break_space('\u{feff}', '\u{feff}')] + #[case::interlinear_annotations('\u{fff9}', '\u{fffb}')] + #[case::language_tag('\u{e0001}', '\u{e0001}')] + #[case::tags('\u{e0020}', '\u{e007f}')] + fn listed_format_characters_are_refused_but_not_their_neighbors(#[case] first: char, #[case] last: char) { + for c in first..=last { + assert!(!accepts_value_holding(c), "U+{:04X} is accepted", u32::from(c)); + } + let before = char::from_u32(u32::from(first) - 1).expect("not a surrogate"); + let after = char::from_u32(u32::from(last) + 1).expect("not a surrogate"); + for c in [before, after] { + assert!(accepts_value_holding(c), "U+{:04X} is refused", u32::from(c)); + } + } + + /// CONTRACT.agent-identity.md §3 refuses only the invisible format characters it lists as code points, so a code point of Unicode general category `Cf` that it leaves out is accepted. + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata + #[rstest] + #[case::arabic_number_signs('\u{600}', '\u{605}')] + #[case::arabic_end_of_ayah('\u{6dd}', '\u{6dd}')] + #[case::syriac_abbreviation_mark('\u{70f}', '\u{70f}')] + #[case::arabic_pound_and_piastre_marks_above('\u{890}', '\u{891}')] + #[case::arabic_disputed_end_of_ayah('\u{8e2}', '\u{8e2}')] + #[case::kaithi_number_sign('\u{110bd}', '\u{110bd}')] + #[case::kaithi_number_sign_above('\u{110cd}', '\u{110cd}')] + #[case::egyptian_hieroglyph_format_controls('\u{13430}', '\u{1343f}')] + #[case::shorthand_format_controls('\u{1bca0}', '\u{1bca3}')] + #[case::musical_symbol_beams_and_phrases('\u{1d173}', '\u{1d17a}')] + fn format_characters_the_contract_leaves_out_are_accepted(#[case] first: char, #[case] last: char) { + for c in first..=last { + assert!(accepts_value_holding(c), "U+{:04X} is refused", u32::from(c)); + } + } + /// [Shared intent][intent] ("Enrollment"): "The result is trimmed, truncated to 255 Unicode scalar values without splitting one, /// and trimmed again, so a stored name never starts or ends with white space (Unicode `White_Space`); /// an empty result falls back to the identity ID." @@ -204,7 +280,7 @@ mod tests { #[case::trimmed_before_truncation(format!(" {}", "a".repeat(255)), "a".repeat(255))] #[case::truncated_by_scalar_value("é".repeat(300), "é".repeat(255))] #[case::unicode_white_space("\u{3000}host\u{a0}\u{2029}".to_owned(), "host".to_owned())] - #[case::not_white_space("\u{200b}host".to_owned(), "\u{200b}host".to_owned())] + #[case::not_white_space("\u{2800}host".to_owned(), "\u{2800}host".to_owned())] #[case::empty_falls_back_to_identity_id("\u{3000} ".to_owned(), uuid::Uuid::nil().to_string())] fn friendly_name_is_trimmed_truncated_and_trimmed_again(#[case] hostname: String, #[case] expected: String) { let metadata = Map::from_iter([("hostname".to_owned(), Value::String(hostname))]); diff --git a/crates/agent-identity/src/metadata.rs b/crates/agent-identity/src/metadata.rs index 438d86c3a..583c2c3ed 100644 --- a/crates/agent-identity/src/metadata.rs +++ b/crates/agent-identity/src/metadata.rs @@ -44,13 +44,41 @@ pub fn collect(agent_version: &str, override_path: Option<&Utf8Path>) -> Metadat sanitize(values) } +/// Removes from each value the characters that [CONTRACT.agent-identity.md §3 "Metadata"][metadata] forbids, then +/// applies the length and empty-value rules of [§3.1 "Metadata sent by the agent"][agent-metadata]. +/// +/// The forbidden characters are the contract's closed list of code points, not a Unicode category lookup, +/// because the contract fixes them as code points so that the agent and every authority agree whatever Unicode +/// version their runtimes ship. +/// +/// [metadata]: ../../../docs/agent-identity/CONTRACT.agent-identity.md#metadata +/// [agent-metadata]: ../../../docs/agent-identity/CONTRACT.agent-identity.md#agent-metadata fn sanitize(values: Metadata) -> Metadata { values .into_iter() .filter_map(|(key, value)| { let sanitized: String = value .chars() - .filter(|ch| !matches!(ch, '\u{0}'..='\u{1f}' | '\u{7f}'..='\u{9f}')) + .filter(|ch| { + !matches!( + ch, + // C0, DEL and C1 control characters. + '\u{0}'..='\u{1F}' + | '\u{7F}'..='\u{9F}' + // Invisible format characters. + | '\u{AD}' + | '\u{61C}' + | '\u{180E}' + | '\u{200B}'..='\u{200F}' + | '\u{202A}'..='\u{202E}' + | '\u{2060}'..='\u{2064}' + | '\u{2066}'..='\u{206F}' + | '\u{FEFF}' + | '\u{FFF9}'..='\u{FFFB}' + | '\u{E0001}' + | '\u{E0020}'..='\u{E007F}' + ) + }) .scan(0, |length, ch| { *length += ch.len_utf8(); (*length <= 1024).then_some(ch) @@ -190,8 +218,104 @@ fn machine_id() -> Option { #[cfg(test)] mod tests { + use rstest::rstest; + use super::*; + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §agent-metadata + #[rstest] + #[case::c0('\u{0}', '\u{1F}')] + #[case::del_and_c1('\u{7F}', '\u{9F}')] + #[case::soft_hyphen('\u{AD}', '\u{AD}')] + #[case::arabic_letter_mark('\u{61C}', '\u{61C}')] + #[case::mongolian_vowel_separator('\u{180E}', '\u{180E}')] + #[case::zero_width_and_directional_marks('\u{200B}', '\u{200F}')] + #[case::directional_embeddings_and_overrides('\u{202A}', '\u{202E}')] + #[case::word_joiner_and_invisible_operators('\u{2060}', '\u{2064}')] + #[case::directional_isolates_and_deprecated_controls('\u{2066}', '\u{206F}')] + #[case::byte_order_mark('\u{FEFF}', '\u{FEFF}')] + #[case::interlinear_annotations('\u{FFF9}', '\u{FFFB}')] + #[case::language_tag('\u{E0001}', '\u{E0001}')] + #[case::tags('\u{E0020}', '\u{E007F}')] + fn removes_each_forbidden_code_point(#[case] first: char, #[case] last: char) { + for ch in first..=last { + let code = u32::from(ch); + assert_eq!(sanitized_hostname(format!("a{ch}b")), "ab", "U+{code:04X}"); + } + } + + // Each case is the run of allowed code points from just after one forbidden range to just before the next, or to + // the last code point. + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §agent-metadata + #[rstest] + #[case::after_c0('\u{20}', '\u{7E}')] + #[case::after_c1('\u{A0}', '\u{AC}')] + #[case::after_soft_hyphen('\u{AE}', '\u{61B}')] + #[case::after_arabic_letter_mark('\u{61D}', '\u{180D}')] + #[case::after_mongolian_vowel_separator('\u{180F}', '\u{200A}')] + #[case::after_directional_marks('\u{2010}', '\u{2029}')] + #[case::after_directional_overrides('\u{202F}', '\u{205F}')] + #[case::after_invisible_operators('\u{2065}', '\u{2065}')] + #[case::after_deprecated_controls('\u{2070}', '\u{FEFE}')] + #[case::after_byte_order_mark('\u{FF00}', '\u{FFF8}')] + #[case::after_interlinear_annotations('\u{FFFC}', '\u{E0000}')] + #[case::after_language_tag('\u{E0002}', '\u{E001F}')] + #[case::after_tags('\u{E0080}', '\u{10FFFF}')] + fn keeps_code_points_just_outside_forbidden_ranges(#[case] first: char, #[case] last: char) { + for ch in [first, last] { + let code = u32::from(ch); + assert_eq!(sanitized_hostname(format!("a{ch}b")), format!("a{ch}b"), "U+{code:04X}"); + } + } + + // These are the format characters (`Cf`) that the contract's list leaves out. + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §agent-metadata + #[rstest] + #[case::arabic_number_signs('\u{600}', '\u{605}')] + #[case::arabic_end_of_ayah('\u{6DD}', '\u{6DD}')] + #[case::syriac_abbreviation_mark('\u{70F}', '\u{70F}')] + #[case::arabic_currency_marks_above('\u{890}', '\u{891}')] + #[case::arabic_disputed_end_of_ayah('\u{8E2}', '\u{8E2}')] + #[case::kaithi_number_sign('\u{110BD}', '\u{110BD}')] + #[case::kaithi_number_sign_above('\u{110CD}', '\u{110CD}')] + #[case::egyptian_hieroglyph_format_controls('\u{13430}', '\u{1343F}')] + #[case::shorthand_format_controls('\u{1BCA0}', '\u{1BCA3}')] + #[case::musical_symbol_format_controls('\u{1D173}', '\u{1D17A}')] + fn keeps_format_characters_the_contract_leaves_out(#[case] first: char, #[case] last: char) { + for ch in first..=last { + let code = u32::from(ch); + assert_eq!(sanitized_hostname(format!("a{ch}b")), format!("a{ch}b"), "U+{code:04X}"); + } + } + + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §agent-metadata + #[test] + fn removes_format_characters_before_truncating_to_1024_bytes() { + assert_eq!( + sanitized_hostname(format!("\u{200B}{}", "a".repeat(1024))), + "a".repeat(1024) + ); + } + + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §agent-metadata + #[test] + fn replaces_a_required_value_of_only_format_characters_with_unknown() { + assert_eq!( + sanitized_hostname("\u{200B}\u{2066}\u{FEFF}\u{E0041}".into()), + "unknown" + ); + } + + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §agent-metadata + #[test] + fn omits_an_optional_value_of_only_format_characters() { + let values = Metadata::from([("domain".to_owned(), "\u{AD}\u{202E}\u{E007F}".to_owned())]); + assert!(!sanitize(values).contains_key("domain")); + } + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §agent-metadata #[test] fn sanitizes_all_values_with_utf8_limits_and_required_fallback() { @@ -262,4 +386,9 @@ mod tests { ); assert!(smbios_uuid(&raw[..raw.len() - 1]).is_none()); } + + fn sanitized_hostname(value: String) -> String { + let mut values = sanitize(Metadata::from([("hostname".to_owned(), value)])); + values.remove("hostname").expect("hostname is a required key") + } } diff --git a/docs/agent-identity/CONTRACT.agent-identity.md b/docs/agent-identity/CONTRACT.agent-identity.md index e3e552fd5..07ea88db0 100644 --- a/docs/agent-identity/CONTRACT.agent-identity.md +++ b/docs/agent-identity/CONTRACT.agent-identity.md @@ -105,7 +105,8 @@ Unless stated otherwise, implementations must follow these conventions. - JSON object of string → string. - Known keys: `hostname`, `fqdn`, `domain`, `os_name`, `os_version`, `arch`, `agent_version`, `machine_id`. -- Limits: ≤ 32 keys; key matches `^[a-z][a-z0-9_]{0,63}$`; value ≤ 1024 UTF-8 bytes with no C0, DEL (U+007F) or C1 control characters; whole object ≤ 16 KiB, measured as the sum of the UTF-8 byte lengths of all keys and values. +- Limits: ≤ 32 keys; key matches `^[a-z][a-z0-9_]{0,63}$`; value ≤ 1024 UTF-8 bytes with no C0, DEL (U+007F) or C1 control characters and no invisible format character (U+00AD, U+061C, U+180E, U+200B–U+200F, U+202A–U+202E, U+2060–U+2064, U+2066–U+206F, U+FEFF, U+FFF9–U+FFFB, U+E0001, U+E0020–U+E007F); whole object ≤ 16 KiB, measured as the sum of the UTF-8 byte lengths of all keys and values. +- Invisible format characters are excluded so that no value can reorder or hide text where an administrator reads it, such as a friendly name evaluated from metadata. - `metadata` is required in enroll, renew and check-in bodies and may be an empty object; an absent or non-object value → `invalid_request`. - Anything else (nested values, non-strings, limits exceeded) → `invalid_request`. - Unknown keys within these limits are stored as-is and never used for authorization. @@ -126,7 +127,7 @@ Unless stated otherwise, implementations must follow these conventions. - `agent_version`: the agent's own product version string, the same one the binary's version resource and package report. - `machine_id`: Windows SMBIOS system UUID, lowercase hyphenated; Linux `/etc/machine-id` as-is; macOS `IOPlatformUUID`, lowercase hyphenated. - The agent never sends a value that violates §3: - - It removes C0, DEL and C1 control characters and truncates to 1024 UTF-8 bytes on a character boundary. + - It removes C0, DEL and C1 control characters and the invisible format characters of §3, and truncates to 1024 UTF-8 bytes on a character boundary. - An optional key whose value is empty after that is omitted. - A required key that ends up empty gets the value `unknown`. diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Bodies/AgentMetadataTests.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Bodies/AgentMetadataTests.cs index 5d73aeb50..a9ddc9ee9 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Bodies/AgentMetadataTests.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Bodies/AgentMetadataTests.cs @@ -16,6 +16,7 @@ public class AgentMetadataTests ["key outside the pattern"] = [Entry($"{LeakedKey}\n", LeakedValue)], ["repeated key"] = [Entry(LeakedKey, "first"), Entry(LeakedKey, LeakedValue)], ["value with a control character"] = [Entry(LeakedKey, $"{LeakedValue}\u0007")], + ["value with an invisible format character"] = [Entry(LeakedKey, $"{LeakedValue}\u202E")], ["value that is not Unicode"] = [Entry(LeakedKey, $"{LeakedValue}\uD800")], ["value of 1025 UTF-8 bytes"] = [Entry(LeakedKey, LeakedValue + new string('a', 1025 - LeakedValue.Length))], @@ -27,6 +28,38 @@ .. Enumerable.Range(0, 17) ], }; + // The invisible format characters that CONTRACT.agent-identity.md §3 lists, as inclusive ranges. + private static readonly (int First, int Last)[] InvisibleFormatRanges = + [ + (0x00AD, 0x00AD), + (0x061C, 0x061C), + (0x180E, 0x180E), + (0x200B, 0x200F), + (0x202A, 0x202E), + (0x2060, 0x2064), + (0x2066, 0x206F), + (0xFEFF, 0xFEFF), + (0xFFF9, 0xFFFB), + (0xE0001, 0xE0001), + (0xE0020, 0xE007F), + ]; + + // The format characters (`Cf`) that the contract's list leaves out, + // as inclusive ranges. + private static readonly (int First, int Last)[] UnlistedFormatRanges = + [ + (0x0600, 0x0605), + (0x06DD, 0x06DD), + (0x070F, 0x070F), + (0x0890, 0x0891), + (0x08E2, 0x08E2), + (0x110BD, 0x110BD), + (0x110CD, 0x110CD), + (0x13430, 0x1343F), + (0x1BCA0, 0x1BCA3), + (0x1D173, 0x1D17A), + ]; + public static TheoryData FailureNames => new(Failures.Keys); // Metadata may be an empty object. @@ -207,6 +240,41 @@ public void AcceptsValueWithCharacterNextToAControlRange(int codePoint) AssertValid(AgentMetadata.Validate([new("hostname", $"host{(char)codePoint}name")])); } + // No invisible format character, including the tag characters past U+FFFF, which UTF-16 writes as a surrogate pair. + public static TheoryData InvisibleFormatCharacters => new(CodePoints(InvisibleFormatRanges)); + + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata + [Theory] + [MemberData(nameof(InvisibleFormatCharacters))] + public void RejectsValueWithInvisibleFormatCharacter(int codePoint) + { + AssertInvalid(AgentMetadata.Validate([new("hostname", $"host{char.ConvertFromUtf32(codePoint)}name")])); + } + + // The neighbors of the invisible format ranges, which the list leaves out. + public static TheoryData CharactersNextToAnInvisibleFormatRange => + new(InvisibleFormatRanges.SelectMany(range => new[] { range.First - 1, range.Last + 1 }).Distinct()); + + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata + [Theory] + [MemberData(nameof(CharactersNextToAnInvisibleFormatRange))] + public void AcceptsValueWithCharacterNextToAnInvisibleFormatRange(int codePoint) + { + AssertValid(AgentMetadata.Validate([new("hostname", $"host{char.ConvertFromUtf32(codePoint)}name")])); + } + + // The list is closed, so a value may hold the format characters it leaves out, + // which a check of the runtime's Format category would refuse. + public static TheoryData UnlistedFormatCharacters => new(CodePoints(UnlistedFormatRanges)); + + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata + [Theory] + [MemberData(nameof(UnlistedFormatCharacters))] + public void AcceptsValueWithUnlistedFormatCharacter(int codePoint) + { + AssertValid(AgentMetadata.Validate([new("hostname", $"host{char.ConvertFromUtf32(codePoint)}name")])); + } + // CONTRACT.agent-identity.md §3 measures a value in UTF-8 bytes. // A string holding an unpaired surrogate is not Unicode text, so it has no UTF-8 encoding to measure or store. // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata @@ -267,19 +335,21 @@ public void FailureDetailHoldsNoKeyOrValue(string failure) Assert.DoesNotContain(LeakedValue, result.FailureDetail); } - // The control character is part of the value, so the detail holds neither the character + // The refused character is part of the value, so the detail holds neither the character // nor its code point in a hexadecimal rendering such as U+0007, 0x07, 0x0007 or 0007. // oracle: intent dotnet/Devolutions.Agent.Identity.Authority/INTENT.md §Failure details [Theory] [MemberData(nameof(ControlCharacters))] - public void FailureDetailDoesNotNameTheControlCharacter(int codePoint) + [MemberData(nameof(InvisibleFormatCharacters))] + public void FailureDetailDoesNotNameTheRefusedCharacter(int codePoint) { - MetadataValidationResult result = AgentMetadata.Validate([Entry("hostname", $"host{(char)codePoint}name")]); + string character = char.ConvertFromUtf32(codePoint); + MetadataValidationResult result = AgentMetadata.Validate([Entry("hostname", $"host{character}name")]); string[] renderings = [$"U+{codePoint:X4}", $"0x{codePoint:X2}", $"0x{codePoint:X4}", $"{codePoint:X4}"]; // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata AssertInvalid(result); - Assert.DoesNotContain(((char)codePoint).ToString(), result.FailureDetail, StringComparison.Ordinal); + Assert.DoesNotContain(character, result.FailureDetail, StringComparison.Ordinal); Assert.All( renderings, rendering => Assert.DoesNotContain(rendering, result.FailureDetail, StringComparison.OrdinalIgnoreCase)); @@ -336,4 +406,9 @@ private static string Repeat(string unit, int count) { return string.Concat(Enumerable.Repeat(unit, count)); } + + private static IEnumerable CodePoints((int First, int Last)[] ranges) + { + return ranges.SelectMany(range => Enumerable.Range(range.First, range.Last - range.First + 1)); + } } diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Bodies/RequestBodyTests.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Bodies/RequestBodyTests.cs index 011bbe8f0..18f051da8 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Bodies/RequestBodyTests.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Bodies/RequestBodyTests.cs @@ -128,6 +128,7 @@ public void RejectsMetadataValueThatIsNotAString(string metadata) { "key ending in a line feed", """{"hostname\n": "host"}""" }, { "C0 control character in a value", """{"hostname": "a\u0007b"}""" }, { "C1 control character in a value", """{"hostname": "a\u0085b"}""" }, + { "invisible format character in a value", """{"hostname": "a\u202Eb"}""" }, { "value of 1025 UTF-8 bytes", $$"""{"hostname": "{{string.Concat(Enumerable.Repeat("\u00E9", 512))}}a"}""" }, }; diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Channel/ChannelHandshakeTests.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Channel/ChannelHandshakeTests.cs index 07eccae74..cbc852506 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Channel/ChannelHandshakeTests.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Channel/ChannelHandshakeTests.cs @@ -25,6 +25,7 @@ public class ChannelHandshakeTests { ["a key outside the pattern"] = new($"{LeakedKey}\n", LeakedValue), ["a value with a control character"] = new(LeakedKey, $"{LeakedValue}\u0007"), + ["a value with an invisible format character"] = new(LeakedKey, $"{LeakedValue}\u202E"), ["a value of 1025 UTF-8 bytes"] = new(LeakedKey, LeakedValue + new string('a', 1025 - LeakedValue.Length)), }; diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Bodies/AgentMetadata.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Bodies/AgentMetadata.cs index e6a4427f9..4d1cd9f4d 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Bodies/AgentMetadata.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Bodies/AgentMetadata.cs @@ -45,8 +45,10 @@ private AgentMetadata(OrderedDictionary entries) /// /// /// Valid metadata has at most 32 keys, each matching ^[a-z][a-z0-9_]{0,63}$ and appearing once. - /// Each value is Unicode text of at most 1024 UTF-8 bytes without C0, DEL or C1 control characters, - /// and the UTF-8 lengths of all keys and values add up to at most 16 KiB. + /// Each value is Unicode text of at most 1024 UTF-8 bytes without C0, DEL or C1 control characters + /// or the invisible format characters U+00AD, U+061C, U+180E, U+200B to U+200F, U+202A to U+202E, + /// U+2060 to U+2064, U+2066 to U+206F, U+FEFF, U+FFF9 to U+FFFB, U+E0001 and U+E0020 to U+E007F. + /// The UTF-8 lengths of all keys and values add up to at most 16 KiB. /// /// The metadata entries, in the order received. /// An entry has a null key or value. @@ -164,6 +166,11 @@ private static bool IsValidKey(string key) return "holds a control character"; } + if (IsInvisibleFormatCharacter(rune.Value)) + { + return "holds an invisible format character"; + } + utf8Length += rune.Utf8SequenceLength; if (utf8Length > MaxValueBytes) { @@ -175,4 +182,20 @@ private static bool IsValidKey(string key) return null; } + + // The invisible format characters of the protocol, which could reorder or hide text where an administrator reads it. + // The protocol fixes them as code points rather than as the Format category, + // so that the agent and every authority agree whatever Unicode version their runtimes ship. + private static bool IsInvisibleFormatCharacter(int codePoint) + { + return codePoint is 0x00AD or 0x061C or 0x180E + or (>= 0x200B and <= 0x200F) + or (>= 0x202A and <= 0x202E) + or (>= 0x2060 and <= 0x2064) + or (>= 0x2066 and <= 0x206F) + or 0xFEFF + or (>= 0xFFF9 and <= 0xFFFB) + or 0xE0001 + or (>= 0xE0020 and <= 0xE007F); + } } diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.csproj b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.csproj index 0e5268263..2f6cecac4 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.csproj +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.csproj @@ -14,7 +14,7 @@ Agent-facing protocol rules shared by every Devolutions Agent Identity authority © Devolutions Inc. All rights reserved. Devolutions.Agent.Identity.Authority - 2026.10.27 + 2026.10.28 MIT OR Apache-2.0 README.md https://github.com/Devolutions/devolutions-gateway.git