From 4c75ba0cb8c036e4b330f1f10740e7c29e8379de Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 27 Apr 2026 15:43:55 +0100 Subject: [PATCH 1/7] tor-netdoc: Move FromStr and Display from NormalItemArgument to impls This allows implementing NormalItemArgument for types that can only be parsed, or only displayed - or other combinations. I noticed this restriction while inventing a type I later decided was unnecessary. I still think it's a good change. There is no practical impact elsewhere, since in practice downstream code implements NormalItemArgument rather than relying on it. --- crates/tor-netdoc/semver.md | 1 + crates/tor-netdoc/src/encode.rs | 2 +- crates/tor-netdoc/src/lib.rs | 8 +++++--- crates/tor-netdoc/src/parse2/traits.rs | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/tor-netdoc/semver.md b/crates/tor-netdoc/semver.md index bef63e1dda..f990cc1fa3 100644 --- a/crates/tor-netdoc/semver.md +++ b/crates/tor-netdoc/semver.md @@ -61,3 +61,4 @@ BREAKING: `ConsensusVoterInfo`; several field types changed ADDED: `ConsensusVoterInfo`: `ItemValueParseable`, `ItemValueEncodable`, `Constructor` ADDED: `VoteAuthorityEntry`, `VoteAuthoritySection` ADDED: `SupersededAuthorityKey`, `ConsensusAuthoritySection` +BREAKING: `NormalItemArgument` no longer has `FromStr` and `Display` as supertraits diff --git a/crates/tor-netdoc/src/encode.rs b/crates/tor-netdoc/src/encode.rs index bce2057355..03abc84707 100644 --- a/crates/tor-netdoc/src/encode.rs +++ b/crates/tor-netdoc/src/encode.rs @@ -230,7 +230,7 @@ impl Default for NetdocEncoder { } } -impl ItemArgument for T { +impl ItemArgument for T { fn write_arg_onto(&self, out: &mut ItemEncoder<'_>) -> Result<(), Bug> { (*self.to_string()).write_arg_onto(out) } diff --git a/crates/tor-netdoc/src/lib.rs b/crates/tor-netdoc/src/lib.rs index d20e980dec..9ecfb558d4 100644 --- a/crates/tor-netdoc/src/lib.rs +++ b/crates/tor-netdoc/src/lib.rs @@ -119,9 +119,11 @@ pub enum AllowAnnotations { /// A type that is represented as a single argument /// whose representation is as for the type's `FromStr` and `Display`. /// -/// Implementing this trait enables a blanket impl of `parse2::ItemArgumentParseable` -/// and `build::ItemArgument`. -pub trait NormalItemArgument: std::str::FromStr + std::fmt::Display {} +/// Implementing this trait enables a blanket impl of +/// [`parse2::ItemArgumentParseable`] (if `FromStr`) +/// and +/// [`encode::ItemArgument`] (if `Display`). +pub trait NormalItemArgument {} // TODO: should we implement ItemArgument for, say, tor_llcrypto::pk::rsa::RsaIdentity ? // It's not clear whether it's always formatted the same way in all parts of the spec. // The Display impl of RsaIdentity adds a `$` which is not supposed to be present diff --git a/crates/tor-netdoc/src/parse2/traits.rs b/crates/tor-netdoc/src/parse2/traits.rs index d584eff4b7..6826f9e08b 100644 --- a/crates/tor-netdoc/src/parse2/traits.rs +++ b/crates/tor-netdoc/src/parse2/traits.rs @@ -149,7 +149,7 @@ impl ItemArgumentParseable for Arc { } } -impl ItemArgumentParseable for T { +impl ItemArgumentParseable for T { fn from_args<'s>(args: &mut ArgumentStream<'s>) -> Result { let v = args .next() -- GitLab From 330fa170bb755934534b26627a4d75c7724083a4 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 27 Apr 2026 15:37:22 +0100 Subject: [PATCH 2/7] tor-netdoc: Improvement to Unknown API: make disregarding more explicit Make the operation of disregarding the possible existence of already-discarded information, more explicit. --- crates/tor-netdoc/semver.md | 2 ++ crates/tor-netdoc/src/types/misc.rs | 17 ++++++++++++++++- crates/tor-netdoc/src/types/relay_flags.rs | 1 + 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/tor-netdoc/semver.md b/crates/tor-netdoc/semver.md index f990cc1fa3..179836e24f 100644 --- a/crates/tor-netdoc/semver.md +++ b/crates/tor-netdoc/semver.md @@ -62,3 +62,5 @@ ADDED: `ConsensusVoterInfo`: `ItemValueParseable`, `ItemValueEncodable`, `Constr ADDED: `VoteAuthorityEntry`, `VoteAuthoritySection` ADDED: `SupersededAuthorityKey`, `ConsensusAuthoritySection` BREAKING: `NormalItemArgument` no longer has `FromStr` and `Display` as supertraits +BREAKING: `Unknown::as_ref` signature changed; `only_known` may be needed too now. +ADDED: `Unknown::only_known` diff --git a/crates/tor-netdoc/src/types/misc.rs b/crates/tor-netdoc/src/types/misc.rs index 3a3e8859fe..512235a7cf 100644 --- a/crates/tor-netdoc/src/types/misc.rs +++ b/crates/tor-netdoc/src/types/misc.rs @@ -803,7 +803,22 @@ impl Unknown { } /// Obtain an `Unknown` containing (maybe) a reference - pub fn as_ref(&self) -> Option<&T> { + pub fn as_ref(&self) -> Unknown<&T> { + match self { + Unknown::Discarded(_) => Unknown::Discarded(PhantomData), + #[cfg(feature = "retain-unknown")] + Unknown::Retained(t) => Unknown::Retained(t), + } + } + + /// Return the retained unknown data, giving `None` if none was saved + /// + /// This is the function for disregarding the possible previously existence + /// of now-discarded unknown (unrecognised) information. + /// + /// Use [`into_retained`](Self::into_retained) if it would be a bug + /// if unrecognised information had been previously discarded. + pub fn only_known(self) -> Option { match self { Unknown::Discarded(_) => None, #[cfg(feature = "retain-unknown")] diff --git a/crates/tor-netdoc/src/types/relay_flags.rs b/crates/tor-netdoc/src/types/relay_flags.rs index 4209d4cf09..7ba35afae3 100644 --- a/crates/tor-netdoc/src/types/relay_flags.rs +++ b/crates/tor-netdoc/src/types/relay_flags.rs @@ -301,6 +301,7 @@ mod parse2_impl { flags .unknown .as_ref() + .only_known() .map(|u| u.iter()) .into_iter() .flatten() -- GitLab From c67df7070159dc7b62769678f01a1c55db2246c6 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 27 Apr 2026 15:54:57 +0100 Subject: [PATCH 3/7] tor-netdoc: Improvement to Unknown API: make it exhaustive --- crates/tor-netdoc/semver.md | 1 + crates/tor-netdoc/src/types/misc.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/tor-netdoc/semver.md b/crates/tor-netdoc/semver.md index 179836e24f..8195ad4240 100644 --- a/crates/tor-netdoc/semver.md +++ b/crates/tor-netdoc/semver.md @@ -64,3 +64,4 @@ ADDED: `SupersededAuthorityKey`, `ConsensusAuthoritySection` BREAKING: `NormalItemArgument` no longer has `FromStr` and `Display` as supertraits BREAKING: `Unknown::as_ref` signature changed; `only_known` may be needed too now. ADDED: `Unknown::only_known` +ADDED: `Unknown` is now an exhaustive enum. diff --git a/crates/tor-netdoc/src/types/misc.rs b/crates/tor-netdoc/src/types/misc.rs index 512235a7cf..4794943636 100644 --- a/crates/tor-netdoc/src/types/misc.rs +++ b/crates/tor-netdoc/src/types/misc.rs @@ -772,7 +772,7 @@ mod ignored_impl { /// `Unknown` is not `Eq` or `Ord` because we won't want to relate a `Discarded` /// to a `Retained`. That would be a logic error. `partial_cmp` gives `None` for this. #[derive(Debug, PartialEq, Clone, Copy, Hash)] -#[non_exhaustive] +#[allow(clippy::exhaustive_enums)] // this isn't going to change pub enum Unknown { /// The parsing discarded unknown values and they are no longer available. Discarded(PhantomData), -- GitLab From 77cb35dadc2c6915f56de7df35cfec9aaecbfa02 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 27 Apr 2026 15:32:27 +0100 Subject: [PATCH 4/7] tor-netdoc: Introduce KeywordOrString We're going to use this for `directory-signature`'s hash algorithm, which the current code always treats as a string! --- crates/tor-netdoc/semver.md | 1 + crates/tor-netdoc/src/types.rs | 1 + crates/tor-netdoc/src/types/misc.rs | 46 +++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+) diff --git a/crates/tor-netdoc/semver.md b/crates/tor-netdoc/semver.md index 8195ad4240..1f0b46f47f 100644 --- a/crates/tor-netdoc/semver.md +++ b/crates/tor-netdoc/semver.md @@ -65,3 +65,4 @@ BREAKING: `NormalItemArgument` no longer has `FromStr` and `Display` as supertra BREAKING: `Unknown::as_ref` signature changed; `only_known` may be needed too now. ADDED: `Unknown::only_known` ADDED: `Unknown` is now an exhaustive enum. +ADDED: `KeywordOrString` diff --git a/crates/tor-netdoc/src/types.rs b/crates/tor-netdoc/src/types.rs index 78762d149b..8cfab65646 100644 --- a/crates/tor-netdoc/src/types.rs +++ b/crates/tor-netdoc/src/types.rs @@ -17,6 +17,7 @@ pub mod version; pub use embedded_cert::*; +pub use misc::KeywordOrString; pub use misc::RetainedOrderVec; pub use misc::{ContactInfo, InvalidNickname, Nickname, NotPresent, NumericBoolean, Unknown}; pub use misc::{Hostname, InternetHost, InvalidHostname, InvalidInternetHost}; diff --git a/crates/tor-netdoc/src/types/misc.rs b/crates/tor-netdoc/src/types/misc.rs index 4794943636..2e70147253 100644 --- a/crates/tor-netdoc/src/types/misc.rs +++ b/crates/tor-netdoc/src/types/misc.rs @@ -877,6 +877,52 @@ impl PartialOrd for Unknown { // ============================================================ +/// Known keyword (enum) value, or arbitrary string +/// +/// `T` should be a `Copy` enum with unit variants. +/// It should have appropriate `FromStr` and `Display`, +/// as well as [`NormalItemArgument`], impls. +/// +/// Then `KeywordOrString` will implement the same traits. +/// +/// Unlike [`Unknown`], unknown values are always retained as strings. +// +// `RelayFlags` has machinery for parsing flags and retaining unknown values, +// but it uses `Unknown` to maybe discard unknown flags, +// and it is generally quite a lot more complicated. +#[derive(Debug, PartialEq, Clone, Hash)] +#[allow(clippy::exhaustive_enums)] // this isn't going to change +pub enum KeywordOrString { + /// Known and recognised `T` + Known(T), + + /// Unknown value in arbitrary syntax + Unknown(String), +} + +impl NormalItemArgument for KeywordOrString {} + +impl Display for KeywordOrString { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + KeywordOrString::Known(t) => Display::fmt(t, f), + KeywordOrString::Unknown(s) => Display::fmt(s, f), + } + } +} + +impl FromStr for KeywordOrString { + type Err = Void; + fn from_str(s: &str) -> Result { + Ok(match s.parse() { + Ok(y) => KeywordOrString::Known(y), + Err(_) => KeywordOrString::Unknown(s.to_owned()), + }) + } +} + +// ============================================================ + /// A sequence of `T` items, with their order retained /// /// Normally when a `Vec` appears in a network document, -- GitLab From 24b773c0074d68b7ee6771d3652c7e3794a46130 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 27 Apr 2026 16:13:12 +0100 Subject: [PATCH 5/7] tor-netdoc: poc: make DirectorySignatureHashAlgo serialise properly --- crates/tor-netdoc/src/parse2/poc/netstatus.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/tor-netdoc/src/parse2/poc/netstatus.rs b/crates/tor-netdoc/src/parse2/poc/netstatus.rs index ffe05ef926..1d04b7c807 100644 --- a/crates/tor-netdoc/src/parse2/poc/netstatus.rs +++ b/crates/tor-netdoc/src/parse2/poc/netstatus.rs @@ -138,6 +138,7 @@ define_derive_deftly! { #[derive(Clone, Copy, Debug, Eq, PartialEq, strum::EnumString, Deftly)] #[derive_deftly(DirectorySignatureHashesAccu)] #[non_exhaustive] +#[strum(serialize_all = "snake_case")] pub enum DirectorySignatureHashAlgo { /// SHA-1 #[deftly(hash_len = "20")] -- GitLab From 3d1f272aab1c4815bef932c09efa5e12ed318a9c Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 11:28:02 +0100 Subject: [PATCH 6/7] tor-netdoc: Promote DirectorySignatureHashAlgo from poc as nasty macro We want DirectorySignatureHashAlgo for prod's directory Signature type. However, for complex cfg gate and macro scoping reasons, we can't just move the definition out of poc. Hence this rather unpleasant intermediate state. This will go away, and become normal again, when we can abolish poc's signatures and have poc use a prod signature type. Review with git show --color=always --color-moved-ws=allow-indentation-change --color-moved --- crates/tor-netdoc/src/parse2/poc/netstatus.rs | 14 ++------- crates/tor-netdoc/src/util.rs | 29 +++++++++++++++++++ 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/crates/tor-netdoc/src/parse2/poc/netstatus.rs b/crates/tor-netdoc/src/parse2/poc/netstatus.rs index 1d04b7c807..ba6bfed62f 100644 --- a/crates/tor-netdoc/src/parse2/poc/netstatus.rs +++ b/crates/tor-netdoc/src/parse2/poc/netstatus.rs @@ -134,18 +134,8 @@ define_derive_deftly! { } } -/// `directory-signature` hash algorithm argument -#[derive(Clone, Copy, Debug, Eq, PartialEq, strum::EnumString, Deftly)] -#[derive_deftly(DirectorySignatureHashesAccu)] -#[non_exhaustive] -#[strum(serialize_all = "snake_case")] -pub enum DirectorySignatureHashAlgo { - /// SHA-1 - #[deftly(hash_len = "20")] - Sha1, - /// SHA-256 - #[deftly(hash_len = "32")] - Sha256, +define_directory_signature_hash_algo! { + #[derive_deftly(DirectorySignatureHashesAccu)] } /// Unsupported `vote-status` value diff --git a/crates/tor-netdoc/src/util.rs b/crates/tor-netdoc/src/util.rs index 5f0df0b59e..317fa29001 100644 --- a/crates/tor-netdoc/src/util.rs +++ b/crates/tor-netdoc/src/util.rs @@ -91,3 +91,32 @@ fn test_as_mut_compiles() { let _: &mut S<()> = S { t: () }.as_mut(); } + +/// Define `DirectorySignatureHashAlgo`, for `directory-signature` items +/// +/// This macro exists to avoid clone-and-hack between poc and prod code. +/// +/// It is difficult for either of those modules to use the other's definition, +/// because they have different stability, different cfg gating, +/// and want to derive deftly differently. +/// +/// tl;dr: defining this type in doc/netstatus.rs would mean +/// the poc derived structs would end up in the prod module; +/// defining it in poc puts it behind the `incomplete` feature gate. +// +// TODO DIRAUTH after poc abolished, turn back into a normal struct DirectorySignatureHashAlgo +macro_rules! define_directory_signature_hash_algo { { $( $attrs:tt )* } => { + /// `directory-signature` hash algorithm argument + #[derive(Clone, Copy, Debug, Eq, PartialEq, strum::EnumString, Deftly)] + $($attrs)* + #[non_exhaustive] + #[strum(serialize_all = "snake_case")] + pub enum DirectorySignatureHashAlgo { + /// SHA-1 + #[deftly(hash_len = "20")] + Sha1, + /// SHA-256 + #[deftly(hash_len = "32")] + Sha256, + } +} } -- GitLab From c78493ff61a51b915f9ddbe2e293eba648da0be3 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 11:28:49 +0100 Subject: [PATCH 7/7] tor-netdoc: Use poc's DirectorySignatureHashAlgo This type is now directly suitable for use in directory signatures. While we're changing its type, rename the digest algorithm name field from `digestname` to `digest_algo`. (The spec says `algorithm` but I don't think we're going to reuse `direcctory-signature` for non-RSA signatures so this is just the hash algorithm.) --- crates/tor-netdoc/src/doc/netstatus.rs | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index d0b839dd0b..920848a32e 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -85,6 +85,7 @@ use std::sync::Arc; use std::{net, result, time}; use tor_error::{Bug, HasKind, bad_api_usage, internal}; use tor_protover::Protocols; +use void::ResultVoidExt as _; use derive_deftly::{Deftly, define_derive_deftly}; use digest::Digest; @@ -518,6 +519,10 @@ impl ConsensusFlavor { } } +define_directory_signature_hash_algo! { + #[derive_deftly_adhoc] // TODO DIRAUTH; suppresses complaints about attrs used only in poc +} + /// The signature of a single directory authority on a networkstatus document. #[derive(Debug, Clone)] #[non_exhaustive] @@ -526,7 +531,7 @@ pub struct Signature { /// /// Currently sha1 and sh256 are recognized. Here we only support /// sha256. - pub digestname: String, + pub digest_algo: KeywordOrString, /// Fingerprints of the keys for the authority that made /// this signature. pub key_ids: AuthCertKeyIds, @@ -1447,7 +1452,7 @@ impl Signature { .at_pos(item.pos())); } - let (alg, id_fp, sk_fp) = if item.n_args() > 2 { + let (digest_algo, id_fp, sk_fp) = if item.n_args() > 2 { ( item.required_arg(0)?, item.required_arg(1)?, @@ -1457,7 +1462,7 @@ impl Signature { ("sha1", item.required_arg(0)?, item.required_arg(1)?) }; - let digestname = alg.to_string(); + let digest_algo = digest_algo.to_string().parse().void_unwrap(); let id_fingerprint = id_fp.parse::()?.into(); let sk_fingerprint = sk_fp.parse::()?.into(); let key_ids = AuthCertKeyIds { @@ -1467,7 +1472,7 @@ impl Signature { let signature = item.obj("SIGNATURE")?; Ok(Signature { - digestname, + digest_algo, key_ids, signature, }) @@ -1566,9 +1571,12 @@ impl SignatureGroup { continue; } - let d: Option<&[u8]> = match sig.digestname.as_ref() { - "sha256" => self.sha256.as_ref().map(|a| &a[..]), - "sha1" => self.sha1.as_ref().map(|a| &a[..]), + use DirectorySignatureHashAlgo as DSHA; + use KeywordOrString as KOS; + + let d: Option<&[u8]> = match sig.digest_algo { + KOS::Known(DSHA::Sha256) => self.sha256.as_ref().map(|a| &a[..]), + KOS::Known(DSHA::Sha1) => self.sha1.as_ref().map(|a| &a[..]), _ => None, // We don't know how to find this digest. }; if d.is_none() { -- GitLab