From 39c3a5dc38827aa7733897ec6af9c71abb01e111 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 12:57:51 +0100 Subject: [PATCH 01/30] tor-netdoc: parse2: Add missing ItemObjectParseable to prelude And remove a then-unneeded import in a using module. --- crates/tor-netdoc/src/doc/microdesc.rs | 2 -- crates/tor-netdoc/src/parse2/internal_prelude.rs | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/tor-netdoc/src/doc/microdesc.rs b/crates/tor-netdoc/src/doc/microdesc.rs index 740ebde00a..44892efc77 100644 --- a/crates/tor-netdoc/src/doc/microdesc.rs +++ b/crates/tor-netdoc/src/doc/microdesc.rs @@ -33,8 +33,6 @@ use std::sync::Arc; use std::sync::LazyLock; use std::time; -use crate::parse2::ItemObjectParseable; - #[cfg(feature = "build_docs")] mod build; diff --git a/crates/tor-netdoc/src/parse2/internal_prelude.rs b/crates/tor-netdoc/src/parse2/internal_prelude.rs index f545150a60..e8478b6406 100644 --- a/crates/tor-netdoc/src/parse2/internal_prelude.rs +++ b/crates/tor-netdoc/src/parse2/internal_prelude.rs @@ -42,7 +42,7 @@ pub use super::{ structural::{StopAt, StopPredicate}, traits::{ IsStructural, ItemArgumentParseable, ItemValueParseable, NetdocParseable, - NetdocParseableFields, + NetdocParseableFields, ItemObjectParseable, }, }; pub use crate::types::Unknown; -- GitLab From a963188b0dd77acec7adbfcf89c462145e2d21a6 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 15:10:51 +0100 Subject: [PATCH 02/30] tor-netdoc: parse2: Add missing ItemObjectParseable to prelude (fmt) --- crates/tor-netdoc/src/parse2/internal_prelude.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/tor-netdoc/src/parse2/internal_prelude.rs b/crates/tor-netdoc/src/parse2/internal_prelude.rs index e8478b6406..5702b557c6 100644 --- a/crates/tor-netdoc/src/parse2/internal_prelude.rs +++ b/crates/tor-netdoc/src/parse2/internal_prelude.rs @@ -41,8 +41,8 @@ pub use super::{ }, structural::{StopAt, StopPredicate}, traits::{ - IsStructural, ItemArgumentParseable, ItemValueParseable, NetdocParseable, - NetdocParseableFields, ItemObjectParseable, + IsStructural, ItemArgumentParseable, ItemObjectParseable, ItemValueParseable, + NetdocParseable, NetdocParseableFields, }, }; pub use crate::types::Unknown; -- GitLab From 551609259391998d62d0578487f1f57f2a00037e Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 12:58:56 +0100 Subject: [PATCH 03/30] tor-netdoc: parse2: Improve another error span for missing trait impl --- crates/tor-netdoc/src/parse2/derive.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/tor-netdoc/src/parse2/derive.rs b/crates/tor-netdoc/src/parse2/derive.rs index bf5d3f65e5..73f26bd000 100644 --- a/crates/tor-netdoc/src/parse2/derive.rs +++ b/crates/tor-netdoc/src/parse2/derive.rs @@ -1306,7 +1306,7 @@ define_derive_deftly! { return Err(EP::ObjectIncorrectLabel) } } else { - selector.check_label(object.label())?; + selector.${paste_spanned $fname check_label}(object.label())?; }} ${if fmeta(netdoc(with)) { ${fmeta(netdoc(with)) as path}::${paste_spanned $fname try_from} -- GitLab From dd0a53e9c1de003717c69266d2418f1bf2f978c0 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 15:53:01 +0100 Subject: [PATCH 04/30] tor-netdoc: types.rs: Add a missing export --- crates/tor-netdoc/src/types.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/tor-netdoc/src/types.rs b/crates/tor-netdoc/src/types.rs index 8cfab65646..ca7416da25 100644 --- a/crates/tor-netdoc/src/types.rs +++ b/crates/tor-netdoc/src/types.rs @@ -22,6 +22,7 @@ pub use misc::RetainedOrderVec; pub use misc::{ContactInfo, InvalidNickname, Nickname, NotPresent, NumericBoolean, Unknown}; pub use misc::{Hostname, InternetHost, InvalidHostname, InvalidInternetHost}; +pub use misc::InvalidContactInfo; pub use misc::RsaSha1Signature; pub use misc::{B16, B16U, B64, FixedB16U, FixedB64}; pub use misc::{Base64Fingerprint, Fingerprint, Ignored, IgnoredItemOrObjectValue, SpFingerprint}; -- GitLab From f600cc908542f475537084d1e6b874305384c53f Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 15:50:44 +0100 Subject: [PATCH 05/30] tor-netdoc: netsatus: glob import from types, not types::misc This way we import via the public names, which is more sensible, and we include things that aren't in misc. The whole of types is pretty good as a glob import here. --- crates/tor-netdoc/src/doc/netstatus.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index 920848a32e..5a9128e965 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -73,7 +73,7 @@ use crate::parse2::{ self, ArgumentStream, ErrorProblem, IsStructural, ItemStream, ItemValueParseable, KeywordRef, NetdocParseable, StopAt, }; -use crate::types::misc::*; +use crate::types::*; use crate::types::relay_flags::{self, DocRelayFlags}; use crate::util::PeekableIterator; use crate::{Error, KeywordEncodable, NetdocErrorKind as EK, NormalItemArgument, Pos, Result}; -- GitLab From 93c18a201ffe85350bc856dd33b28ef3833ed030 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 15:02:29 +0100 Subject: [PATCH 06/30] tor-netdoc: derive Display for DirectorySignatureHashAlgo --- crates/tor-netdoc/src/util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/tor-netdoc/src/util.rs b/crates/tor-netdoc/src/util.rs index 317fa29001..c20cd496b9 100644 --- a/crates/tor-netdoc/src/util.rs +++ b/crates/tor-netdoc/src/util.rs @@ -107,7 +107,7 @@ fn test_as_mut_compiles() { // 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)] + #[derive(Clone, Copy, Debug, Eq, PartialEq, strum::Display, strum::EnumString, Deftly)] $($attrs)* #[non_exhaustive] #[strum(serialize_all = "snake_case")] -- GitLab From 99701cd01561d3e25bcc3263ff8f0f8e0da21806 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 10:53:15 +0100 Subject: [PATCH 07/30] tor-netdoc: Unify authcert test modules Abolish the parse2_test and encode_test modules. These mostly existed because of cfg gates. Now, we can unify the test module namespace, and also remove an indentation level (that will come next, with rustfmt). --- crates/tor-netdoc/src/doc/authcert.rs | 50 ++++++++++----------------- 1 file changed, 18 insertions(+), 32 deletions(-) diff --git a/crates/tor-netdoc/src/doc/authcert.rs b/crates/tor-netdoc/src/doc/authcert.rs index 0021b8809b..662b8c2937 100644 --- a/crates/tor-netdoc/src/doc/authcert.rs +++ b/crates/tor-netdoc/src/doc/authcert.rs @@ -715,7 +715,19 @@ mod test { #![allow(clippy::needless_pass_by_value)] //! use super::*; - use crate::{Error, Pos}; + use crate::{ + Pos, + parse2::{ErrorProblem, ParseError, ParseInput, VerifyFailed, parse_netdoc}, + types, + }; + use humantime::parse_rfc3339; + use std::result::Result; + use std::{ + net::{Ipv4Addr, SocketAddrV4}, + str::FromStr, + }; + use tor_basic_utils::test_rng; + const TESTDATA: &str = include_str!("../../testdata/authcert1.txt"); fn bad_data(fname: &str) -> String { @@ -730,7 +742,7 @@ mod test { } #[test] - fn parse_one() -> Result<()> { + fn parse_one() -> crate::Result<()> { use tor_checkable::{SelfSigned, Timebound}; let cert = AuthCert::parse(TESTDATA)? .check_signature() @@ -752,7 +764,7 @@ mod test { #[test] fn parse_bad() { - fn check(fname: &str, err: &Error) { + fn check(fname: &str, err: &crate::Error) { let contents = bad_data(fname); let cert = AuthCert::parse(&contents); assert!(cert.is_err()); @@ -792,7 +804,7 @@ mod test { let mut data = "<><><<><>\nfingerprint ABC\n".to_string(); data += TESTDATA; - let res: Vec> = AuthCert::parse_multiple(&data).unwrap().collect(); + let res: Vec> = AuthCert::parse_multiple(&data).unwrap().collect(); // We should recover from the failed case and read the next data fine. assert!(res[0].is_err()); @@ -805,7 +817,7 @@ mod test { let mut data = bad_data("bad-version"); data += TESTDATA; - let res: Vec> = AuthCert::parse_multiple(&data).unwrap().collect(); + let res: Vec> = AuthCert::parse_multiple(&data).unwrap().collect(); // We should recover from the failed case and read the next data fine. assert!(res[0].is_err()); @@ -813,23 +825,6 @@ mod test { assert_eq!(res.len(), 2); } - mod parse2_test { - use super::{AuthCert, AuthCertUnverified, AuthCertVersion, CrossCert, CrossCertObject}; - - use std::{ - net::{Ipv4Addr, SocketAddrV4}, - str::FromStr, - time::{Duration, SystemTime}, - }; - - use crate::{ - parse2::{self, ErrorProblem, NetdocUnverified, ParseError, ParseInput, VerifyFailed}, - types::{self, Iso8601TimeSp}, - }; - - use derive_deftly::Deftly; - use tor_llcrypto::pk::rsa::{self, RsaIdentity}; - // === AUTHCERT D190BF3B00E311A9AEB6D62B51980E9B2109BAD1 === // These values come from testdata2/keys/authority_certificate. const DIR_KEY_PUBLISHED: &str = "2000-01-01 00:00:05"; @@ -1188,17 +1183,9 @@ mzMT023bleZ574az+117yNAr6XbIgqQfzbySzVLPXM8ZN9BrGR40KDZ2638ZJjRu VerifyFailed::VerifyFailed ); } - } - - #[cfg(feature = "incomplete")] - mod encode_test { - use super::*; - use crate::parse2::{ParseInput, parse_netdoc}; - use humantime::parse_rfc3339; - use std::result::Result; - use tor_basic_utils::test_rng; #[test] + #[cfg(feature = "incomplete")] fn roundtrip() -> Result<(), anyhow::Error> { let mut rng = test_rng::testing_rng(); let k_auth_id_rsa = rsa::KeyPair::generate(&mut rng)?; @@ -1231,5 +1218,4 @@ mzMT023bleZ574az+117yNAr6XbIgqQfzbySzVLPXM8ZN9BrGR40KDZ2638ZJjRu assert_eq!(input_value, reparsed_value); Ok(()) } - } } -- GitLab From 5057470bfa1070840914fd98bc5ffae08282d5dc Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 12:10:27 +0100 Subject: [PATCH 08/30] tor-netdoc: Unify authcert test modules (fmt) --- crates/tor-netdoc/src/doc/authcert.rs | 621 +++++++++++++------------- 1 file changed, 308 insertions(+), 313 deletions(-) diff --git a/crates/tor-netdoc/src/doc/authcert.rs b/crates/tor-netdoc/src/doc/authcert.rs index 662b8c2937..553081ab53 100644 --- a/crates/tor-netdoc/src/doc/authcert.rs +++ b/crates/tor-netdoc/src/doc/authcert.rs @@ -825,13 +825,13 @@ mod test { assert_eq!(res.len(), 2); } - // === AUTHCERT D190BF3B00E311A9AEB6D62B51980E9B2109BAD1 === - // These values come from testdata2/keys/authority_certificate. - const DIR_KEY_PUBLISHED: &str = "2000-01-01 00:00:05"; - const DIR_KEY_EXPIRES: &str = "2001-01-01 00:00:05"; - const FINGERPRINT: &str = "D190BF3B00E311A9AEB6D62B51980E9B2109BAD1"; - const DIR_ADDRESS: SocketAddrV4 = SocketAddrV4::new(Ipv4Addr::new(127, 0, 0, 1), 7100); - const DIR_IDENTITY_KEY: &str = " + // === AUTHCERT D190BF3B00E311A9AEB6D62B51980E9B2109BAD1 === + // These values come from testdata2/keys/authority_certificate. + const DIR_KEY_PUBLISHED: &str = "2000-01-01 00:00:05"; + const DIR_KEY_EXPIRES: &str = "2001-01-01 00:00:05"; + const FINGERPRINT: &str = "D190BF3B00E311A9AEB6D62B51980E9B2109BAD1"; + const DIR_ADDRESS: SocketAddrV4 = SocketAddrV4::new(Ipv4Addr::new(127, 0, 0, 1), 7100); + const DIR_IDENTITY_KEY: &str = " -----BEGIN RSA PUBLIC KEY----- MIIBigKCAYEAt0rXD+1gYwKFAxrO4uNHQ9dQVUOGx5FxkioYNSct5Z3JU00dTKNJ jt4OGkFYwixWwk6KLDOiB+I/q9YIdA1NlQ5R3Hz8jjvFPVl0JQQm2LYzdSzv7/CZ @@ -844,7 +844,7 @@ yfwfiPN6FQWlPyMCEB81HerCn03Zi5WgQLGo7PAeO4LFrLrU16DUC5/oJENeHs0T hWBehR+ct4OJAgMBAAE= -----END RSA PUBLIC KEY----- "; - const DIR_SIGNING_KEY: &str = " + const DIR_SIGNING_KEY: &str = " -----BEGIN RSA PUBLIC KEY----- MIIBCgKCAQEAtPF94+bThLI28kn6e+MmUECMMJ5UBlnQ+Mvwn8Zd85awPQTDz5Wu 13sZDN3nWnhgSuP5q/WDYc5GPPtQdSWBiG1nJA2XLgEHTHf29iGZ+jAoGfIMJvBV @@ -854,7 +854,7 @@ d7kQzUHbVP0KmYGK4qYntGDfP4g9SmpBoUUHyP3j9en9S6PMYv8m1YFO7M7JKu6Q dQZfGTxj9C/0b/jRklgn5JlKAl9eJQvCdwIDAQAB -----END RSA PUBLIC KEY----- "; - const DIR_CROSS_CERT_OBJECT: &str = " + const DIR_CROSS_CERT_OBJECT: &str = " -----BEGIN ID SIGNATURE----- NBaPdBNCNMah6cklrALzj0RdHymF/jPGOv9NmeqaXc0uTN06S/BlVM/xTjilu+dj sjPuT0BQL4/ZWyZR+R+gJJojKYILSId4IQ1elzRSxpFN+u2u/ZEmS6SR2SwpA05A @@ -864,358 +864,353 @@ mzMT023bleZ574az+117yNAr6XbIgqQfzbySzVLPXM8ZN9BrGR40KDZ2638ZJjRu 8HK5TzuknWlkRv3hCyRX+g== -----END ID SIGNATURE----- "; - const AUTHCERT_RAW: &str = include_str!("../../testdata2/keys/authority_certificate"); - /// A system time in the range of [`DIR_KEY_PUBLISHED`] and [`DIR_KEY_EXPIRES`]. - /// - /// Constructed by ourselves to have a time point we can use for testing - /// timestamp verification. - const VALID_SYSTEM_TIME: &str = "2000-06-01 00:00:00"; - - // === AUTHCERT 0B8997614EC647C1C6B6A044E2B5408F0B823FB0 === - // This values come from ../../testdata2/cached-certs--1 - // A different authority certificate different from the one above. - const ALTERNATIVE_AUTHCERT_RAW: &str = include_str!("../../testdata2/cached-certs--1"); - - /// Converts a string in the [`Iso8601TimeSp`] format to [`SystemTime`]. - /// - /// This functions panics in the case the input is malformatted. - fn to_system_time(s: &str) -> SystemTime { - Iso8601TimeSp::from_str(s).unwrap().0 - } + const AUTHCERT_RAW: &str = include_str!("../../testdata2/keys/authority_certificate"); + /// A system time in the range of [`DIR_KEY_PUBLISHED`] and [`DIR_KEY_EXPIRES`]. + /// + /// Constructed by ourselves to have a time point we can use for testing + /// timestamp verification. + const VALID_SYSTEM_TIME: &str = "2000-06-01 00:00:00"; - /// Converts a PEM encoded RSA Public key to an [`rsa::PublicKey`]. - /// - /// This function panics in the case the input is malformatted. - fn pem_to_rsa_pk(s: &str) -> rsa::PublicKey { - rsa::PublicKey::from_der(pem::parse(s).unwrap().contents()).unwrap() - } + // === AUTHCERT 0B8997614EC647C1C6B6A044E2B5408F0B823FB0 === + // This values come from ../../testdata2/cached-certs--1 + // A different authority certificate different from the one above. + const ALTERNATIVE_AUTHCERT_RAW: &str = include_str!("../../testdata2/cached-certs--1"); + + /// Converts a string in the [`Iso8601TimeSp`] format to [`SystemTime`]. + /// + /// This functions panics in the case the input is malformatted. + fn to_system_time(s: &str) -> SystemTime { + Iso8601TimeSp::from_str(s).unwrap().0 + } + + /// Converts a PEM encoded RSA Public key to an [`rsa::PublicKey`]. + /// + /// This function panics in the case the input is malformatted. + fn pem_to_rsa_pk(s: &str) -> rsa::PublicKey { + rsa::PublicKey::from_der(pem::parse(s).unwrap().contents()).unwrap() + } - /// Converts a hex-encoded RSA identity to an [`RsaIdentity`]. - /// - /// This function panics in the case the input is malformatted. - fn to_rsa_id(s: &str) -> RsaIdentity { - RsaIdentity::from_hex(s).unwrap() + /// Converts a hex-encoded RSA identity to an [`RsaIdentity`]. + /// + /// This function panics in the case the input is malformatted. + fn to_rsa_id(s: &str) -> RsaIdentity { + RsaIdentity::from_hex(s).unwrap() + } + + /// Tests whether a [`DirKeyCrossCert`] can be parsed properly. + #[test] + fn dir_auth_cross_cert() { + #[derive(Debug, Clone, PartialEq, Eq, Deftly)] + #[derive_deftly(NetdocParseable)] + struct Dummy { + dir_key_crosscert: CrossCert, } - /// Tests whether a [`DirKeyCrossCert`] can be parsed properly. - #[test] - fn dir_auth_cross_cert() { - #[derive(Debug, Clone, PartialEq, Eq, Deftly)] - #[derive_deftly(NetdocParseable)] - struct Dummy { - dir_key_crosscert: CrossCert, - } + // "Encodes" a DIR_CROSS_CERT_OBJECT by simply removing the lines + // indicating the BEGIN and END, as the purpose is to test multiple + // labels. + let encoded = DIR_CROSS_CERT_OBJECT + .lines() + .filter(|line| !line.starts_with("-----")) + .collect::>() + .join("\n"); + let decoded = pem::parse(DIR_CROSS_CERT_OBJECT) + .unwrap() + .contents() + .to_vec(); - // "Encodes" a DIR_CROSS_CERT_OBJECT by simply removing the lines - // indicating the BEGIN and END, as the purpose is to test multiple - // labels. - let encoded = DIR_CROSS_CERT_OBJECT - .lines() - .filter(|line| !line.starts_with("-----")) - .collect::>() - .join("\n"); - let decoded = pem::parse(DIR_CROSS_CERT_OBJECT) - .unwrap() - .contents() - .to_vec(); - - // Try with `SIGNATURE`. - let cert = format!( - "dir-key-crosscert\n-----BEGIN SIGNATURE-----\n{encoded}\n-----END SIGNATURE-----" - ); - let res = parse2::parse_netdoc::(&ParseInput::new(&cert, "")).unwrap(); - assert_eq!( - res, - Dummy { - dir_key_crosscert: CrossCert { - signature: CrossCertObject(decoded.clone()) - } - } - ); - - // Try with `ID SIGNATURE`. - let cert = format!( - "dir-key-crosscert\n-----BEGIN ID SIGNATURE-----\n{encoded}\n-----END ID SIGNATURE-----" - ); - let res = parse2::parse_netdoc::(&ParseInput::new(&cert, "")).unwrap(); - assert_eq!( - res, - Dummy { - dir_key_crosscert: CrossCert { - signature: CrossCertObject(decoded.clone()) - } + // Try with `SIGNATURE`. + let cert = format!( + "dir-key-crosscert\n-----BEGIN SIGNATURE-----\n{encoded}\n-----END SIGNATURE-----" + ); + let res = parse2::parse_netdoc::(&ParseInput::new(&cert, "")).unwrap(); + assert_eq!( + res, + Dummy { + dir_key_crosscert: CrossCert { + signature: CrossCertObject(decoded.clone()) } - ); - - // Try with different label and fail. - let cert = - format!("dir-key-crosscert\n-----BEGIN WHAT-----\n{encoded}\n-----END WHAT-----"); - let res = parse2::parse_netdoc::(&ParseInput::new(&cert, "")); - match res { - Err(ParseError { - problem: ErrorProblem::ObjectIncorrectLabel, - doctype: "dir-key-crosscert", - file: _, - lno: 1, - column: None, - }) => {} - other => panic!("not expected error {other:#?}"), } + ); - // Try with extra args. - let cert = format!( - "dir-key-crosscert arg1\n-----BEGIN ID SIGNATURE-----\n{encoded}\n-----END ID SIGNATURE-----" - ); - let res = parse2::parse_netdoc::(&ParseInput::new(&cert, "")); - match res { - Err(ParseError { - problem: ErrorProblem::UnexpectedArgument { column: 19 }, - doctype: "dir-key-crosscert", - file: _, - lno: 1, - column: Some(19), - }) => {} - other => panic!("not expected error {other:#?}"), + // Try with `ID SIGNATURE`. + let cert = format!( + "dir-key-crosscert\n-----BEGIN ID SIGNATURE-----\n{encoded}\n-----END ID SIGNATURE-----" + ); + let res = parse2::parse_netdoc::(&ParseInput::new(&cert, "")).unwrap(); + assert_eq!( + res, + Dummy { + dir_key_crosscert: CrossCert { + signature: CrossCertObject(decoded.clone()) + } } + ); + + // Try with different label and fail. + let cert = + format!("dir-key-crosscert\n-----BEGIN WHAT-----\n{encoded}\n-----END WHAT-----"); + let res = parse2::parse_netdoc::(&ParseInput::new(&cert, "")); + match res { + Err(ParseError { + problem: ErrorProblem::ObjectIncorrectLabel, + doctype: "dir-key-crosscert", + file: _, + lno: 1, + column: None, + }) => {} + other => panic!("not expected error {other:#?}"), } - #[test] - fn dir_auth_cert() { - let res = - parse2::parse_netdoc::(&ParseInput::new(AUTHCERT_RAW, "")) - .unwrap(); - assert_eq!( - *res.inspect_unverified().0, - AuthCert { - dir_key_certificate_version: AuthCertVersion::V3, - dir_address: Some(DIR_ADDRESS), - fingerprint: types::Fingerprint(to_rsa_id(FINGERPRINT)), - dir_key_published: Iso8601TimeSp(to_system_time(DIR_KEY_PUBLISHED)), - dir_key_expires: Iso8601TimeSp(to_system_time(DIR_KEY_EXPIRES)), - dir_identity_key: pem_to_rsa_pk(DIR_IDENTITY_KEY), - dir_signing_key: pem_to_rsa_pk(DIR_SIGNING_KEY), - dir_key_crosscert: CrossCert { - signature: CrossCertObject( - pem::parse(DIR_CROSS_CERT_OBJECT) - .unwrap() - .contents() - .to_vec() - ) - }, - __non_exhaustive: (), - } - ); + // Try with extra args. + let cert = format!( + "dir-key-crosscert arg1\n-----BEGIN ID SIGNATURE-----\n{encoded}\n-----END ID SIGNATURE-----" + ); + let res = parse2::parse_netdoc::(&ParseInput::new(&cert, "")); + match res { + Err(ParseError { + problem: ErrorProblem::UnexpectedArgument { column: 19 }, + doctype: "dir-key-crosscert", + file: _, + lno: 1, + column: Some(19), + }) => {} + other => panic!("not expected error {other:#?}"), } + } - #[test] - fn dir_auth_signature() { - let res = - parse2::parse_netdoc::(&ParseInput::new(AUTHCERT_RAW, "")) - .unwrap(); + #[test] + fn dir_auth_cert() { + let res = + parse2::parse_netdoc::(&ParseInput::new(AUTHCERT_RAW, "")).unwrap(); + assert_eq!( + *res.inspect_unverified().0, + AuthCert { + dir_key_certificate_version: AuthCertVersion::V3, + dir_address: Some(DIR_ADDRESS), + fingerprint: types::Fingerprint(to_rsa_id(FINGERPRINT)), + dir_key_published: Iso8601TimeSp(to_system_time(DIR_KEY_PUBLISHED)), + dir_key_expires: Iso8601TimeSp(to_system_time(DIR_KEY_EXPIRES)), + dir_identity_key: pem_to_rsa_pk(DIR_IDENTITY_KEY), + dir_signing_key: pem_to_rsa_pk(DIR_SIGNING_KEY), + dir_key_crosscert: CrossCert { + signature: CrossCertObject( + pem::parse(DIR_CROSS_CERT_OBJECT) + .unwrap() + .contents() + .to_vec() + ) + }, + __non_exhaustive: (), + } + ); + } - // Test a valid signature. + #[test] + fn dir_auth_signature() { + let res = + parse2::parse_netdoc::(&ParseInput::new(AUTHCERT_RAW, "")).unwrap(); + + // Test a valid signature. + res.clone() + .verify( + &[to_rsa_id(FINGERPRINT)], + Duration::ZERO, + Duration::ZERO, + to_system_time(VALID_SYSTEM_TIME), + ) + .unwrap(); + + // Test with an invalid authority. + assert_eq!( res.clone() .verify( - &[to_rsa_id(FINGERPRINT)], + &[], Duration::ZERO, Duration::ZERO, to_system_time(VALID_SYSTEM_TIME), ) - .unwrap(); - - // Test with an invalid authority. - assert_eq!( - res.clone() - .verify( - &[], - Duration::ZERO, - Duration::ZERO, - to_system_time(VALID_SYSTEM_TIME), - ) - .unwrap_err(), - VerifyFailed::InsufficientTrustedSigners - ); - - // Test a key too far in the future. - assert_eq!( - res.clone() - .verify( - &[to_rsa_id(FINGERPRINT)], - Duration::ZERO, - Duration::ZERO, - SystemTime::UNIX_EPOCH, - ) - .unwrap_err(), - VerifyFailed::TooNew - ); + .unwrap_err(), + VerifyFailed::InsufficientTrustedSigners + ); - // Test an almost too new. + // Test a key too far in the future. + assert_eq!( res.clone() .verify( &[to_rsa_id(FINGERPRINT)], Duration::ZERO, Duration::ZERO, - to_system_time(DIR_KEY_PUBLISHED), + SystemTime::UNIX_EPOCH, ) - .unwrap(); - - // Now fail when we are 1s below ... - assert_eq!( - res.clone() - .verify( - &[to_rsa_id(FINGERPRINT)], - Duration::ZERO, - Duration::ZERO, - to_system_time(DIR_KEY_PUBLISHED) - Duration::from_secs(1), - ) - .unwrap_err(), - VerifyFailed::TooNew - ); + .unwrap_err(), + VerifyFailed::TooNew + ); - // ... but succeed again with a clock skew tolerance. + // Test an almost too new. + res.clone() + .verify( + &[to_rsa_id(FINGERPRINT)], + Duration::ZERO, + Duration::ZERO, + to_system_time(DIR_KEY_PUBLISHED), + ) + .unwrap(); + + // Now fail when we are 1s below ... + assert_eq!( res.clone() .verify( &[to_rsa_id(FINGERPRINT)], - Duration::from_secs(1), + Duration::ZERO, Duration::ZERO, to_system_time(DIR_KEY_PUBLISHED) - Duration::from_secs(1), ) - .unwrap(); - - // Test a key too old. - assert_eq!( - res.clone() - .verify( - &[to_rsa_id(FINGERPRINT)], - Duration::ZERO, - Duration::ZERO, - SystemTime::UNIX_EPOCH - .checked_add(Duration::from_secs(2000000000)) - .unwrap(), - ) - .unwrap_err(), - VerifyFailed::TooOld - ); + .unwrap_err(), + VerifyFailed::TooNew + ); - // Test an almost too old. + // ... but succeed again with a clock skew tolerance. + res.clone() + .verify( + &[to_rsa_id(FINGERPRINT)], + Duration::from_secs(1), + Duration::ZERO, + to_system_time(DIR_KEY_PUBLISHED) - Duration::from_secs(1), + ) + .unwrap(); + + // Test a key too old. + assert_eq!( res.clone() .verify( &[to_rsa_id(FINGERPRINT)], Duration::ZERO, Duration::ZERO, - to_system_time(DIR_KEY_EXPIRES), + SystemTime::UNIX_EPOCH + .checked_add(Duration::from_secs(2000000000)) + .unwrap(), ) - .unwrap(); - - // Now fail when we are 1s above ... - assert_eq!( - res.clone() - .verify( - &[to_rsa_id(FINGERPRINT)], - Duration::ZERO, - Duration::ZERO, - to_system_time(DIR_KEY_EXPIRES) + Duration::from_secs(1), - ) - .unwrap_err(), - VerifyFailed::TooOld - ); + .unwrap_err(), + VerifyFailed::TooOld + ); - // ... but succeed again with a clock skew tolerance. + // Test an almost too old. + res.clone() + .verify( + &[to_rsa_id(FINGERPRINT)], + Duration::ZERO, + Duration::ZERO, + to_system_time(DIR_KEY_EXPIRES), + ) + .unwrap(); + + // Now fail when we are 1s above ... + assert_eq!( res.clone() .verify( - &[to_rsa_id(FINGERPRINT)], - Duration::ZERO, - Duration::from_secs(1), - to_system_time(DIR_KEY_EXPIRES) + Duration::from_secs(1), - ) - .unwrap(); - - // Check with non-matching fingerprint and long-term identity key. - let mut cert = - parse2::parse_netdoc::(&ParseInput::new(AUTHCERT_RAW, "")) - .unwrap(); - let alternative_cert = parse2::parse_netdoc::(&ParseInput::new( - ALTERNATIVE_AUTHCERT_RAW, - "", - )) - .unwrap(); - cert.body.dir_identity_key = alternative_cert.body.dir_identity_key.clone(); - assert_eq!( - cert.verify( - &[to_rsa_id(FINGERPRINT)], - Duration::ZERO, - Duration::ZERO, - to_system_time(VALID_SYSTEM_TIME), - ) - .unwrap_err(), - VerifyFailed::Inconsistent - ); - - // Check invalid cross-cert. - let mut cert = - parse2::parse_netdoc::(&ParseInput::new(AUTHCERT_RAW, "")) - .unwrap(); - cert.body.dir_key_crosscert = alternative_cert.body.dir_key_crosscert.clone(); - assert_eq!( - cert.verify( - &[to_rsa_id(FINGERPRINT)], - Duration::ZERO, - Duration::ZERO, - to_system_time(VALID_SYSTEM_TIME), - ) - .unwrap_err(), - VerifyFailed::VerifyFailed - ); - - // Check outer signature. - let mut cert = - parse2::parse_netdoc::(&ParseInput::new(AUTHCERT_RAW, "")) - .unwrap(); - cert.sigs = alternative_cert.sigs.clone(); - assert_eq!( - cert.verify( &[to_rsa_id(FINGERPRINT)], Duration::ZERO, Duration::ZERO, - to_system_time(VALID_SYSTEM_TIME), + to_system_time(DIR_KEY_EXPIRES) + Duration::from_secs(1), ) .unwrap_err(), - VerifyFailed::VerifyFailed - ); - } + VerifyFailed::TooOld + ); - #[test] - #[cfg(feature = "incomplete")] - fn roundtrip() -> Result<(), anyhow::Error> { - let mut rng = test_rng::testing_rng(); - let k_auth_id_rsa = rsa::KeyPair::generate(&mut rng)?; - let k_auth_sign_rsa = rsa::KeyPair::generate(&mut rng)?; - - let secs = |s| Duration::from_secs(s); - let now = parse_rfc3339("1993-01-01T00:00:00Z")?; - let published = now - secs(1000); - let expires = published + secs(86400); - let tolerance = secs(10); - - let input_value = AuthCert { - dir_address: Some("192.0.2.17:7000".parse()?), - ..AuthCert::new_base(&k_auth_id_rsa, &k_auth_sign_rsa, published, expires)? - }; - dbg!(&input_value); - - let encoded = input_value.encode_sign(&k_auth_id_rsa)?; - - let reparsed_uv: AuthCertUnverified = - parse_netdoc(&ParseInput::new(encoded.as_ref(), ""))?; - let reparsed_value = reparsed_uv.verify( - &[k_auth_id_rsa.to_public_key().to_rsa_identity()], - tolerance, - tolerance, - now, - )?; - dbg!(&reparsed_value); - - assert_eq!(input_value, reparsed_value); - Ok(()) - } + // ... but succeed again with a clock skew tolerance. + res.clone() + .verify( + &[to_rsa_id(FINGERPRINT)], + Duration::ZERO, + Duration::from_secs(1), + to_system_time(DIR_KEY_EXPIRES) + Duration::from_secs(1), + ) + .unwrap(); + + // Check with non-matching fingerprint and long-term identity key. + let mut cert = + parse2::parse_netdoc::(&ParseInput::new(AUTHCERT_RAW, "")).unwrap(); + let alternative_cert = parse2::parse_netdoc::(&ParseInput::new( + ALTERNATIVE_AUTHCERT_RAW, + "", + )) + .unwrap(); + cert.body.dir_identity_key = alternative_cert.body.dir_identity_key.clone(); + assert_eq!( + cert.verify( + &[to_rsa_id(FINGERPRINT)], + Duration::ZERO, + Duration::ZERO, + to_system_time(VALID_SYSTEM_TIME), + ) + .unwrap_err(), + VerifyFailed::Inconsistent + ); + + // Check invalid cross-cert. + let mut cert = + parse2::parse_netdoc::(&ParseInput::new(AUTHCERT_RAW, "")).unwrap(); + cert.body.dir_key_crosscert = alternative_cert.body.dir_key_crosscert.clone(); + assert_eq!( + cert.verify( + &[to_rsa_id(FINGERPRINT)], + Duration::ZERO, + Duration::ZERO, + to_system_time(VALID_SYSTEM_TIME), + ) + .unwrap_err(), + VerifyFailed::VerifyFailed + ); + + // Check outer signature. + let mut cert = + parse2::parse_netdoc::(&ParseInput::new(AUTHCERT_RAW, "")).unwrap(); + cert.sigs = alternative_cert.sigs.clone(); + assert_eq!( + cert.verify( + &[to_rsa_id(FINGERPRINT)], + Duration::ZERO, + Duration::ZERO, + to_system_time(VALID_SYSTEM_TIME), + ) + .unwrap_err(), + VerifyFailed::VerifyFailed + ); + } + + #[test] + #[cfg(feature = "incomplete")] + fn roundtrip() -> Result<(), anyhow::Error> { + let mut rng = test_rng::testing_rng(); + let k_auth_id_rsa = rsa::KeyPair::generate(&mut rng)?; + let k_auth_sign_rsa = rsa::KeyPair::generate(&mut rng)?; + + let secs = |s| Duration::from_secs(s); + let now = parse_rfc3339("1993-01-01T00:00:00Z")?; + let published = now - secs(1000); + let expires = published + secs(86400); + let tolerance = secs(10); + + let input_value = AuthCert { + dir_address: Some("192.0.2.17:7000".parse()?), + ..AuthCert::new_base(&k_auth_id_rsa, &k_auth_sign_rsa, published, expires)? + }; + dbg!(&input_value); + + let encoded = input_value.encode_sign(&k_auth_id_rsa)?; + + let reparsed_uv: AuthCertUnverified = + parse_netdoc(&ParseInput::new(encoded.as_ref(), ""))?; + let reparsed_value = reparsed_uv.verify( + &[k_auth_id_rsa.to_public_key().to_rsa_identity()], + tolerance, + tolerance, + now, + )?; + dbg!(&reparsed_value); + + assert_eq!(input_value, reparsed_value); + Ok(()) + } } -- GitLab From 212d53d3c4aabbd732abda109b9c18c250411d4b Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 15:16:20 +0100 Subject: [PATCH 09/30] tor-netdoc: Introduce DigestAlgoInSignature parsing/encoding type And change the type of the field in Signature. We are going to abandon the almost completely manual parsing approach taken in poc's NdiDirectorySignature. That parsing approach was heavily influenced by poc's struct layout, which in turn was influenced by the way that previously, hashes were inside signatures. Instead we'll derive ItemValueParseable from Signature, and semi-manually implement the hash update. This type is the one that embodies the `directory-signature` item's parsing strangeness: the optional initial positional argument. --- crates/tor-netdoc/src/doc/netstatus.rs | 62 ++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 3 deletions(-) diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index 5a9128e965..bfd36d5a68 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -65,13 +65,14 @@ pub use proto_statuses_parse2_encode::ProtoStatusesNetdocParseAccumulator; use crate::doc::authcert::EncodedAuthCert; use crate::doc::authcert::{AuthCert, AuthCertKeyIds}; -use crate::encode::{ItemValueEncodable, NetdocEncodable, NetdocEncoder}; +use crate::encode::{ItemArgument, ItemEncoder, ItemValueEncodable, NetdocEncodable, NetdocEncoder}; use crate::parse::keyword::Keyword; use crate::parse::parser::{Section, SectionRules, SectionRulesBuilder}; use crate::parse::tokenize::{Item, ItemResult, NetDocReader}; use crate::parse2::{ self, ArgumentStream, ErrorProblem, IsStructural, ItemStream, ItemValueParseable, KeywordRef, NetdocParseable, StopAt, + ArgumentError, ItemArgumentParseable, }; use crate::types::*; use crate::types::relay_flags::{self, DocRelayFlags}; @@ -523,6 +524,60 @@ define_directory_signature_hash_algo! { #[derive_deftly_adhoc] // TODO DIRAUTH; suppresses complaints about attrs used only in poc } +/// `algorithm` field in a `directory-signature` item +/// +/// This is extremely bizarre: it's an *optional item at the start of the arguments*! +// TODO SPEC #350 +/// +/// So we parse it with some kind of nightmarish lookahead. +/// +/// Additionally, to be able to convey the signatures accurately, without breaking them, +/// we must remember whether the argument was present. +/// +/// +#[derive(Debug, Clone, derive_more::Deref, derive_more::DerefMut)] +#[allow(clippy::exhaustive_structs)] +pub struct DigestAlgoInSignature(pub Option>); + +impl ItemArgumentParseable for DigestAlgoInSignature { + fn from_args<'s>(args: &mut ArgumentStream<'s>) -> StdResult { + let v = if (|| { + let s = args.clone().next()?; + // Treat it as a fingerprint if it doesn't have any non-hex characters + // (including lowercase ones). If we reuse this item for new algorithms + // they should have at least one letter g-z in their name. + s.chars().all(|c| c.is_ascii_hexdigit()).then_some(()) + })() + .is_some() + { + // next argument looks enough like a fingerprint that we don't treat as an algo name + None + } else { + Some(KeywordOrString::from_args(args)?) + }; + Ok(DigestAlgoInSignature(v)) + } +} +impl ItemArgument for DigestAlgoInSignature { + fn write_arg_onto(&self, out: &mut ItemEncoder<'_>) -> StdResult<(), Bug> { + if let Some(y) = &self.0 { + y.write_arg_onto(out)?; + } + Ok(()) + } +} +impl DigestAlgoInSignature { + /// Return the actual algorithm + /// + /// This handles the defaulting, where an absent argument means `sha1`. + pub fn algorithm(&self) -> &KeywordOrString { + self.as_ref() + .unwrap_or(&KeywordOrString::Known(DirectorySignatureHashAlgo::Sha1)) + } +} + +impl NormalItemArgument for DirectorySignatureHashAlgo {} + /// The signature of a single directory authority on a networkstatus document. #[derive(Debug, Clone)] #[non_exhaustive] @@ -531,7 +586,7 @@ pub struct Signature { /// /// Currently sha1 and sh256 are recognized. Here we only support /// sha256. - pub digest_algo: KeywordOrString, + pub digest_algo: DigestAlgoInSignature, /// Fingerprints of the keys for the authority that made /// this signature. pub key_ids: AuthCertKeyIds, @@ -1463,6 +1518,7 @@ impl Signature { }; let digest_algo = digest_algo.to_string().parse().void_unwrap(); + let digest_algo = DigestAlgoInSignature(Some(digest_algo)); let id_fingerprint = id_fp.parse::()?.into(); let sk_fingerprint = sk_fp.parse::()?.into(); let key_ids = AuthCertKeyIds { @@ -1574,7 +1630,7 @@ impl SignatureGroup { use DirectorySignatureHashAlgo as DSHA; use KeywordOrString as KOS; - let d: Option<&[u8]> = match sig.digest_algo { + let d: Option<&[u8]> = match sig.digest_algo.algorithm() { 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. -- GitLab From c2d5e236923fbf2c3e00868636ad54a878629d14 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 15:11:56 +0100 Subject: [PATCH 10/30] tor-netdoc: impl parsing/encoding for AuthCertKeyIds in directory-signature This module will be used when we derive ItemValueParseable on Signature. --- crates/tor-netdoc/src/doc/authcert.rs | 69 ++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/crates/tor-netdoc/src/doc/authcert.rs b/crates/tor-netdoc/src/doc/authcert.rs index 553081ab53..95b9127952 100644 --- a/crates/tor-netdoc/src/doc/authcert.rs +++ b/crates/tor-netdoc/src/doc/authcert.rs @@ -7,12 +7,13 @@ //! signing keys to sign votes and consensuses. use crate::batching_split_before::IteratorExt as _; -use crate::encode::{Bug, ItemObjectEncodable, NetdocEncodable, NetdocEncoder}; +use crate::encode::{Bug, ItemArgument, ItemEncoder, ItemObjectEncodable, NetdocEncodable, NetdocEncoder}; use crate::parse::keyword::Keyword; use crate::parse::parser::{Section, SectionRules}; use crate::parse::tokenize::{ItemResult, NetDocReader}; use crate::parse2::{ self, ItemObjectParseable, NetdocUnverified as _, sig_hashes::Sha1WholeKeywordLine, + ArgumentError, ArgumentStream, ItemArgumentParseable, }; use crate::types::misc::{Fingerprint, Iso8601TimeSp, RsaPublicParse1Helper, RsaSha1Signature}; use crate::util::str::Extent; @@ -438,6 +439,45 @@ impl AuthCert { } } +/// Parsing/encoding module for `AuthCertKeyIds` as found in `directory-signature` +/// +/// Use with `#[deftly(netdoc(with = ...))]` when deriving +/// `ItemValueParseable` and `ItemValueEncodable`. +/// +/// +// +// Currently the only use site is `netstatus::Signature`. +// If we find this is being used in many places, and is therefore a standard thing, +// we should arrange for the derives to be able to derive from an argument collection, +// and use that. +#[allow(dead_code)] // XXXX +pub(crate) mod keyids_directory_signature_args { + use super::*; + use std::result::Result; + + /// Parse + pub(crate) fn from_args<'s>( + args: &mut ArgumentStream<'s>, + ) -> Result { + let mut fp = || Ok::<_, ArgumentError>(Fingerprint::from_args(args)?.0); + Ok(AuthCertKeyIds { + id_fingerprint: fp()?, + sk_fingerprint: fp()?, + }) + } + + /// Encode + pub(crate) fn write_arg_onto( + self_: &AuthCertKeyIds, + out: &mut ItemEncoder<'_>, + ) -> Result<(), Bug> { + let mut fp = |id| Fingerprint(id).write_arg_onto(out); + fp(self_.id_fingerprint)?; + fp(self_.sk_fingerprint)?; + Ok(()) + } +} + /// Pseudo-Signature of the long-term identity key by the medium-term key. /// /// This type does not implement `SignatureItemParseable` because that trait @@ -1179,6 +1219,33 @@ mzMT023bleZ574az+117yNAr6XbIgqQfzbySzVLPXM8ZN9BrGR40KDZ2638ZJjRu ); } + #[test] + fn keyids_for_directory_signature() -> anyhow::Result<()> { + #[derive(Deftly)] + #[derive_deftly(NetdocEncodable, NetdocParseable)] + struct Doc { + intro: (), + ids: Item, + } + #[derive(Deftly)] + #[derive_deftly(ItemValueEncodable, ItemValueParseable)] + struct Item { + #[deftly(netdoc(with = keyids_directory_signature_args))] + ids: AuthCertKeyIds, + } + + let text = r#"intro +ids 1234567812345678123456781234567812345678 ABCDABCDABCDABCDABCDABCDABCDABCDABCDABCD +"#; + let doc = parse2::parse_netdoc::(&ParseInput::new(text, ""))?; + let mut re_encode = NetdocEncoder::new(); + doc.encode_unsigned(&mut re_encode)?; + let re_encode = re_encode.finish()?; + + assert_eq!(text, re_encode); + Ok(()) + } + #[test] #[cfg(feature = "incomplete")] fn roundtrip() -> Result<(), anyhow::Error> { -- GitLab From 51289624bc158a8360489b35f42d6c58e893c08e Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 17:26:41 +0100 Subject: [PATCH 11/30] tor-netdoc: derive ItemValue parsing/encoding for Signature This doesn't let us do any verification or anything. --- crates/tor-netdoc/src/doc/authcert.rs | 1 - crates/tor-netdoc/src/doc/netstatus.rs | 13 ++++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/crates/tor-netdoc/src/doc/authcert.rs b/crates/tor-netdoc/src/doc/authcert.rs index 95b9127952..79ab00c3f9 100644 --- a/crates/tor-netdoc/src/doc/authcert.rs +++ b/crates/tor-netdoc/src/doc/authcert.rs @@ -450,7 +450,6 @@ impl AuthCert { // If we find this is being used in many places, and is therefore a standard thing, // we should arrange for the derives to be able to derive from an argument collection, // and use that. -#[allow(dead_code)] // XXXX pub(crate) mod keyids_directory_signature_args { use super::*; use std::result::Result; diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index bfd36d5a68..fdfbb522a7 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -64,7 +64,7 @@ pub use proto_statuses_parse2_encode::ProtoStatusesNetdocParseAccumulator; #[cfg(feature = "incomplete")] use crate::doc::authcert::EncodedAuthCert; -use crate::doc::authcert::{AuthCert, AuthCertKeyIds}; +use crate::doc::authcert::{self, AuthCert, AuthCertKeyIds}; use crate::encode::{ItemArgument, ItemEncoder, ItemValueEncodable, NetdocEncodable, NetdocEncoder}; use crate::parse::keyword::Keyword; use crate::parse::parser::{Section, SectionRules, SectionRulesBuilder}; @@ -74,7 +74,7 @@ use crate::parse2::{ NetdocParseable, StopAt, ArgumentError, ItemArgumentParseable, }; -use crate::types::*; +use crate::types::{self, *}; use crate::types::relay_flags::{self, DocRelayFlags}; use crate::util::PeekableIterator; use crate::{Error, KeywordEncodable, NetdocErrorKind as EK, NormalItemArgument, Pos, Result}; @@ -579,7 +579,12 @@ impl DigestAlgoInSignature { impl NormalItemArgument for DirectorySignatureHashAlgo {} /// The signature of a single directory authority on a networkstatus document. -#[derive(Debug, Clone)] +/// +/// Implements `ItemValueParseable` which parses without hashing anything; +/// this is mostly useful for use by the `SignatureItemParseable` implementation. +// XXXX that impl doesn't exist yet +#[derive(Debug, Clone, Deftly)] +#[derive_deftly(ItemValueEncodable, ItemValueParseable)] #[non_exhaustive] pub struct Signature { /// The name of the digest algorithm used to make the signature. @@ -589,8 +594,10 @@ pub struct Signature { pub digest_algo: DigestAlgoInSignature, /// Fingerprints of the keys for the authority that made /// this signature. + #[deftly(netdoc(with = authcert::keyids_directory_signature_args))] pub key_ids: AuthCertKeyIds, /// The signature itself. + #[deftly(netdoc(object(label = "SIGNATURE"), with = types::raw_data_object))] pub signature: Vec, } -- GitLab From 0ad67f5b7a4faefc0f9dc38283be811dff3437aa Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 12:41:07 +0100 Subject: [PATCH 12/30] tor-netdoc: netstatus signing algorithms: Improve a doc comment There isn't a "this" here; self is DirectorySignaturesHashesAccu. --- crates/tor-netdoc/src/parse2/poc/netstatus.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/tor-netdoc/src/parse2/poc/netstatus.rs b/crates/tor-netdoc/src/parse2/poc/netstatus.rs index ba6bfed62f..cbeca84bfc 100644 --- a/crates/tor-netdoc/src/parse2/poc/netstatus.rs +++ b/crates/tor-netdoc/src/parse2/poc/netstatus.rs @@ -122,7 +122,7 @@ define_derive_deftly! { }) } - /// Return the hash value for this algorithm, as a slice + /// Return the hash value for a specific algorithm, as a slice /// /// `None` if the value wasn't computed. /// That shouldn't happen. -- GitLab From 2f4be3aac51b0f0206cf7c8b755e3d7c1b4889a6 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 15:43:25 +0100 Subject: [PATCH 13/30] tor-netdoc: netstatus signature hashing: Add TODO re unnamed sha1 We're going to fix this in this branch, as part of promoting the poc code to prod. --- crates/tor-netdoc/src/parse2/poc/netstatus.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/crates/tor-netdoc/src/parse2/poc/netstatus.rs b/crates/tor-netdoc/src/parse2/poc/netstatus.rs index cbeca84bfc..dd5b357f60 100644 --- a/crates/tor-netdoc/src/parse2/poc/netstatus.rs +++ b/crates/tor-netdoc/src/parse2/poc/netstatus.rs @@ -97,6 +97,21 @@ define_derive_deftly! { ${vattrs doc} $FNAME: Option<[u8; ${vmeta(hash_len) as expr}]>, ) + +/* + // XXXX actually implement this + + /// `sha1` but without the algorithm name + /// + /// This is needed because the hash includes the whole signature item keyword line, + /// and therefore a signature with the `sha1` explicitly stated, + /// and one without, have different hashes! + /// + /// So we mustn't use the `sha1` field for both implicit and explicit use of SHA-1, + /// or multiple signatures with different syntax would overwrite each others' + /// different hashes. + pub(crate) sha1_unnamed: Option<[u8; 20]>, +*/ } impl DirectorySignaturesHashesAccu { @@ -127,6 +142,7 @@ define_derive_deftly! { /// `None` if the value wasn't computed. /// That shouldn't happen. fn hash_slice_for_verification(&self, algo: $ttype) -> Option<&[u8]> { + // XXXX handle sha1_unnamed correctly match algo { $( $vtype => Some(self.$FNAME.as_ref()?), ) } -- GitLab From 6fa4858fceadafa9e018412f88cb551f30ef2313 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 16:45:45 +0100 Subject: [PATCH 14/30] tor-netdoc: netstatus signature hashing: Add TODO re wasted work --- 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 dd5b357f60..e99dd7ed0a 100644 --- a/crates/tor-netdoc/src/parse2/poc/netstatus.rs +++ b/crates/tor-netdoc/src/parse2/poc/netstatus.rs @@ -129,6 +129,7 @@ define_derive_deftly! { let mut h = tor_llcrypto::d::$vname::new(); h.update(body.body().body()); h.update(body.signature_item_kw_spc); + // XXXX Unconditionally write. This can involve wasted work. self.$FNAME = Some(h.finalize().into()); $vtype } -- GitLab From 1dbe2b5f8702fc77e36b65f2df06fcb60cd61daf Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 17:08:17 +0100 Subject: [PATCH 15/30] tor-netdoc: Temporary conversions between the two DirectorySignatureHashAlgos Because we have two DirectorySignatureHashAlgo types because of the macrology problem, we need to introduce temporary conversions, to let us move code from poc to prod. --- crates/tor-netdoc/src/parse2/poc/netstatus.rs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/crates/tor-netdoc/src/parse2/poc/netstatus.rs b/crates/tor-netdoc/src/parse2/poc/netstatus.rs index e99dd7ed0a..154fc19f30 100644 --- a/crates/tor-netdoc/src/parse2/poc/netstatus.rs +++ b/crates/tor-netdoc/src/parse2/poc/netstatus.rs @@ -10,6 +10,8 @@ use doc::netstatus::{ConsensusAuthoritySection, VoteAuthoritySection}; mod ns_per_flavour_macros; pub use ns_per_flavour_macros::*; +use crate::doc::netstatus::DirectorySignatureHashAlgo as ProdAlgo; // XXXX remove + ns_per_flavour_macros::ns_export_flavoured_types! { NetworkStatus, NetworkStatusUnverified, Router, } @@ -149,6 +151,24 @@ define_derive_deftly! { ) } } } + + // XXXX we have two DirectorySignatureHashAlgo types, which we, briefly, + // need to convert between! + impl From for DirectorySignatureHashAlgo { + fn from(other: ProdAlgo) -> Self { + match other { $( + ProdAlgo::$vname => DirectorySignatureHashAlgo::$vname, + ) } + } + } + // XXXX + impl From for ProdAlgo { + fn from(other: DirectorySignatureHashAlgo) -> Self { + match other { $( + DirectorySignatureHashAlgo::$vname => ProdAlgo::$vname, + ) } + } + } } define_directory_signature_hash_algo! { -- GitLab From d46bbe7bb4cbb27cff6f2198b0de06941968401d Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 15:38:49 +0100 Subject: [PATCH 16/30] tor-netdoc: Make some items temporarily pub(crate) --- crates/tor-netdoc/src/parse2/poc/netstatus.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/tor-netdoc/src/parse2/poc/netstatus.rs b/crates/tor-netdoc/src/parse2/poc/netstatus.rs index 154fc19f30..60a93ed582 100644 --- a/crates/tor-netdoc/src/parse2/poc/netstatus.rs +++ b/crates/tor-netdoc/src/parse2/poc/netstatus.rs @@ -97,7 +97,8 @@ define_derive_deftly! { pub struct DirectorySignaturesHashesAccu { $( ${vattrs doc} - $FNAME: Option<[u8; ${vmeta(hash_len) as expr}]>, + // XXXX make no longer pub(crate) + pub(crate) $FNAME: Option<[u8; ${vmeta(hash_len) as expr}]>, ) /* @@ -144,7 +145,8 @@ define_derive_deftly! { /// /// `None` if the value wasn't computed. /// That shouldn't happen. - fn hash_slice_for_verification(&self, algo: $ttype) -> Option<&[u8]> { + // XXXX make no longer pub(crate) + pub(crate) fn hash_slice_for_verification(&self, algo: $ttype) -> Option<&[u8]> { // XXXX handle sha1_unnamed correctly match algo { $( $vtype => Some(self.$FNAME.as_ref()?), -- GitLab From ea5d42d936c841836dfda5050ec905be517b919b Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 16:53:11 +0100 Subject: [PATCH 17/30] tor-netdoc: Promote poc's DirectorySignaturesHashesAccu to prod There are definitely some things wrong with this, but they all have MR-blocking TODOs so we can promote this type now. Largely code motion. Review with --color-moved. --- crates/tor-netdoc/src/doc/netstatus.rs | 40 ++++++++++++++++++- crates/tor-netdoc/src/parse2/poc/netstatus.rs | 30 +------------- 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index fdfbb522a7..7b2cdf8200 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -520,8 +520,46 @@ impl ConsensusFlavor { } } +define_derive_deftly! { + /// Bespoke derives applied to [`DirectorySignatureHashAlgo`] + /// + /// Generates: + /// + /// * `pub struct DirectorySignaturesHashesAccu` + DirectorySignaturesHashesAccu: + + ${define FNAME ${paste ${snake_case $vname}} } + + /// `directory-signature`a hash algorithm argument + #[derive(Clone, Copy, Default, Debug, Eq, PartialEq, Deftly)] + #[derive_deftly(AsMutSelf)] + #[non_exhaustive] + pub struct DirectorySignaturesHashesAccu { + $( + ${vattrs doc} + // XXXX make no longer pub(crate) + pub(crate) $FNAME: Option<[u8; ${vmeta(hash_len) as expr}]>, + ) + +/* + // XXXX actually implement this + + /// `sha1` but without the algorithm name + /// + /// This is needed because the hash includes the whole signature item keyword line, + /// and therefore a signature with the `sha1` explicitly stated, + /// and one without, have different hashes! + /// + /// So we mustn't use the `sha1` field for both implicit and explicit use of SHA-1, + /// or multiple signatures with different syntax would overwrite each others' + /// different hashes. + pub(crate) sha1_unnamed: Option<[u8; 20]>, +*/ + } +} + define_directory_signature_hash_algo! { - #[derive_deftly_adhoc] // TODO DIRAUTH; suppresses complaints about attrs used only in poc + #[derive_deftly(DirectorySignaturesHashesAccu)] } /// `algorithm` field in a `directory-signature` item diff --git a/crates/tor-netdoc/src/parse2/poc/netstatus.rs b/crates/tor-netdoc/src/parse2/poc/netstatus.rs index 60a93ed582..b479f13aad 100644 --- a/crates/tor-netdoc/src/parse2/poc/netstatus.rs +++ b/crates/tor-netdoc/src/parse2/poc/netstatus.rs @@ -5,7 +5,7 @@ use super::*; use crate::doc::{self, authcert}; use crate::types; use authcert::AuthCert as DirAuthKeyCert; -use doc::netstatus::{ConsensusAuthoritySection, VoteAuthoritySection}; +use doc::netstatus::{ConsensusAuthoritySection, DirectorySignaturesHashesAccu, VoteAuthoritySection}; mod ns_per_flavour_macros; pub use ns_per_flavour_macros::*; @@ -90,33 +90,6 @@ define_derive_deftly! { ${define FNAME ${paste ${snake_case $vname}} } - /// `directory-signature`a hash algorithm argument - #[derive(Clone, Copy, Default, Debug, Eq, PartialEq, Deftly)] - #[derive_deftly(AsMutSelf)] - #[non_exhaustive] - pub struct DirectorySignaturesHashesAccu { - $( - ${vattrs doc} - // XXXX make no longer pub(crate) - pub(crate) $FNAME: Option<[u8; ${vmeta(hash_len) as expr}]>, - ) - -/* - // XXXX actually implement this - - /// `sha1` but without the algorithm name - /// - /// This is needed because the hash includes the whole signature item keyword line, - /// and therefore a signature with the `sha1` explicitly stated, - /// and one without, have different hashes! - /// - /// So we mustn't use the `sha1` field for both implicit and explicit use of SHA-1, - /// or multiple signatures with different syntax would overwrite each others' - /// different hashes. - pub(crate) sha1_unnamed: Option<[u8; 20]>, -*/ - } - impl DirectorySignaturesHashesAccu { /// If `algorithm` is an algorithm name, calculate the hash /// @@ -175,6 +148,7 @@ define_derive_deftly! { define_directory_signature_hash_algo! { #[derive_deftly(DirectorySignatureHashesAccu)] + #[derive_deftly_adhoc] // XXXX suppress unknown attr errors } /// Unsupported `vote-status` value -- GitLab From 5aa7b82f5ec8469981226ff7dcfebb39fb846013 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 17:08:52 +0100 Subject: [PATCH 18/30] tor-netdoc: Introduce DirectorySignaturesHashesAccu::update_from This now contains the hashing code that was in poc. In poc it was entangled with the signature type, mostly because once upon a time the hash was inside the signature. In the real code we can make it a standalone method. It takes DigestAlgoInSignature so that it will be able to handle the explicit vs implicit sha1 anomaly correctly - but it can't do that yet because the necessary field doesn't exist yet. Also, we are going to want to make it a bit lazier - preserve the MR-blocking todo for that. A handful of lines are being actually moved verbatim, so --color-moved may help a bit. --- crates/tor-netdoc/src/doc/netstatus.rs | 44 +++++++++++++++++-- crates/tor-netdoc/src/parse2/poc/netstatus.rs | 25 +++++------ 2 files changed, 53 insertions(+), 16 deletions(-) diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index 7b2cdf8200..a4225d247d 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -71,7 +71,7 @@ use crate::parse::parser::{Section, SectionRules, SectionRulesBuilder}; use crate::parse::tokenize::{Item, ItemResult, NetDocReader}; use crate::parse2::{ self, ArgumentStream, ErrorProblem, IsStructural, ItemStream, ItemValueParseable, KeywordRef, - NetdocParseable, StopAt, + NetdocParseable, SignatureHashInputs, SignatureItemParseable, StopAt, UnparsedItem, ArgumentError, ItemArgumentParseable, }; use crate::types::{self, *}; @@ -525,7 +525,8 @@ define_derive_deftly! { /// /// Generates: /// - /// * `pub struct DirectorySignaturesHashesAccu` + /// * [`DirectorySignaturesHashesAccu`] + /// * [`DirectorySignaturesHashesAccu::update_from`] DirectorySignaturesHashesAccu: ${define FNAME ${paste ${snake_case $vname}} } @@ -556,6 +557,43 @@ define_derive_deftly! { pub(crate) sha1_unnamed: Option<[u8; 20]>, */ } + + impl DirectorySignaturesHashesAccu { + /// Calculate the hash for a signature item and update this accumulator + // XXXX make no longer pub(crate) + pub(crate) fn update_from( + &mut self, + algo: &DigestAlgoInSignature, + body: &SignatureHashInputs, + ) { + // Update the hash in self.$UPDATE according to algorithm $AGLO + // (uses dynamic bindings of those parameters) + ${define HASH { + // XXXX indentation is wrong + let mut h = tor_llcrypto::d::$ALGO::new(); + h.update(body.body().body()); + h.update(body.signature_item_kw_spc); + // XXXX Unconditionally write. This can involve wasted work. + self.$UPDATE = Some(h.finalize().into()); + }} + + match &**algo { + $( + Some(KeywordOrString::Known($vtype)) => { + ${define UPDATE $FNAME} + ${define ALGO $vname} + $HASH + } + ) + None => { + ${define UPDATE sha1} // XXXX wrong + ${define ALGO Sha1} + $HASH + } + Some(KeywordOrString::Unknown(..)) => {} + } + } + } } define_directory_signature_hash_algo! { @@ -1358,7 +1396,7 @@ mod parse2_impls { use super::*; pub(super) use parse2::{ ArgumentError as AE, ArgumentStream, ErrorProblem as EP, ItemArgumentParseable, - ItemValueParseable, NetdocParseableFields, UnparsedItem, + ItemValueParseable, NetdocParseableFields, }; use std::result::Result; diff --git a/crates/tor-netdoc/src/parse2/poc/netstatus.rs b/crates/tor-netdoc/src/parse2/poc/netstatus.rs index b479f13aad..e8894504cd 100644 --- a/crates/tor-netdoc/src/parse2/poc/netstatus.rs +++ b/crates/tor-netdoc/src/parse2/poc/netstatus.rs @@ -99,19 +99,18 @@ define_derive_deftly! { algorithm: &str, body: &SignatureHashInputs, ) -> Option<$ttype> { - Some(match algorithm { - $( - ${concat $FNAME} => { - let mut h = tor_llcrypto::d::$vname::new(); - h.update(body.body().body()); - h.update(body.signature_item_kw_spc); - // XXXX Unconditionally write. This can involve wasted work. - self.$FNAME = Some(h.finalize().into()); - $vtype - } - ) - _ => return None, - }) + let Ok(algo) = algorithm.parse() + else { return None }; + + // XXXX this is just wrong, but our tests don't catch it right now, + // and we are about to delete this whole function. + let algo_is = doc::netstatus::DigestAlgoInSignature(Some( + types::KeywordOrString::Known(algo) + )); + + self.update_from(&algo_is, body); + + Some(algo.into()) } /// Return the hash value for a specific algorithm, as a slice -- GitLab From 8ff11d455af1f6baa145b00acafda073e1ea0207 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 17:17:21 +0100 Subject: [PATCH 19/30] tor-netdoc: impl SignatureItemParseable for netstatus::Signature This is now pretty straightforward. The functions it calls need some work - we have blocking TODOs for that. --- crates/tor-netdoc/src/doc/netstatus.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index a4225d247d..e53c85265d 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -658,7 +658,6 @@ impl NormalItemArgument for DirectorySignatureHashAlgo {} /// /// Implements `ItemValueParseable` which parses without hashing anything; /// this is mostly useful for use by the `SignatureItemParseable` implementation. -// XXXX that impl doesn't exist yet #[derive(Debug, Clone, Deftly)] #[derive_deftly(ItemValueEncodable, ItemValueParseable)] #[non_exhaustive] @@ -677,6 +676,20 @@ pub struct Signature { pub signature: Vec, } +impl SignatureItemParseable for Signature { + type HashAccu = DirectorySignaturesHashesAccu; + + fn from_unparsed_and_body( + item: UnparsedItem, + body: &SignatureHashInputs<'_>, + hash: &mut Self::HashAccu, + ) -> StdResult { + let signature = Signature::from_unparsed(item)?; + hash.update_from(&signature.digest_algo, body); + Ok(signature) + } +} + /// A collection of signatures that can be checked on a networkstatus document #[derive(Debug, Clone)] #[non_exhaustive] -- GitLab From 8bcf8d44f085f268ae21c1c69cb1cf026dd27dee Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 17:15:30 +0100 Subject: [PATCH 20/30] tor-netdoc: Promote hash_slice_for_verification to prod This function is not right, yet. We're going to fix it later. Review with --color-moved. --- crates/tor-netdoc/src/doc/netstatus.rs | 12 ++++++++++++ crates/tor-netdoc/src/parse2/poc/netstatus.rs | 14 +------------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index e53c85265d..c44f78db0f 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -593,6 +593,18 @@ define_derive_deftly! { Some(KeywordOrString::Unknown(..)) => {} } } + + /// Return the hash value for a specific algorithm, as a slice + /// + /// `None` if the value wasn't computed. + /// That shouldn't happen. + // XXXX make no longer pub(crate) + pub(crate) fn hash_slice_for_verification(&self, algo: $ttype) -> Option<&[u8]> { + // XXXX handle sha1_unnamed correctly + match algo { $( + $vtype => Some(self.$FNAME.as_ref()?), + ) } + } } } diff --git a/crates/tor-netdoc/src/parse2/poc/netstatus.rs b/crates/tor-netdoc/src/parse2/poc/netstatus.rs index e8894504cd..0fc773b0d8 100644 --- a/crates/tor-netdoc/src/parse2/poc/netstatus.rs +++ b/crates/tor-netdoc/src/parse2/poc/netstatus.rs @@ -112,18 +112,6 @@ define_derive_deftly! { Some(algo.into()) } - - /// Return the hash value for a specific algorithm, as a slice - /// - /// `None` if the value wasn't computed. - /// That shouldn't happen. - // XXXX make no longer pub(crate) - pub(crate) fn hash_slice_for_verification(&self, algo: $ttype) -> Option<&[u8]> { - // XXXX handle sha1_unnamed correctly - match algo { $( - $vtype => Some(self.$FNAME.as_ref()?), - ) } - } } // XXXX we have two DirectorySignatureHashAlgo types, which we, briefly, @@ -254,7 +242,7 @@ fn verify_general_timeless( }; let h = hashes - .hash_slice_for_verification(*hash_algo) + .hash_slice_for_verification((*hash_algo).into()) .ok_or(VF::Bug)?; let () = cert.dir_signing_key.verify(h, rsa_signature)?; -- GitLab From f33cdcf44c4654e7043f7e93baa05eb7318ab005 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 17:15:34 +0100 Subject: [PATCH 21/30] tor-netdoc: Abolish poc's netstatus signature type Use prod's Signature instead. This gets rid of: * The old parsing code. We have a new approach based on ItemValueParseable, KeywordOrString and and DigestAlgoInSignature. * The duplicate DirectorySignaturesHashesAccu and its temporary conversions. poc's verify_timeless function needs a little adjustment for the new struct layout. --- crates/tor-dirserver/src/mirror/operation.rs | 16 +- crates/tor-netdoc/src/doc/netstatus.rs | 1 + crates/tor-netdoc/src/parse2/poc/netstatus.rs | 175 ++---------------- 3 files changed, 19 insertions(+), 173 deletions(-) diff --git a/crates/tor-dirserver/src/mirror/operation.rs b/crates/tor-dirserver/src/mirror/operation.rs index 08b49679ea..8dacfccdf7 100644 --- a/crates/tor-dirserver/src/mirror/operation.rs +++ b/crates/tor-dirserver/src/mirror/operation.rs @@ -37,7 +37,7 @@ use tor_netdoc::{ }, parse2::{ self, - poc::netstatus::{cons, md, NdiDirectorySignature}, + poc::netstatus::{cons, md}, NetdocParseable, NetdocUnverified, ParseInput, }, }; @@ -719,19 +719,7 @@ impl FlavoredConsensusSigned { Self::Md(md) => &md.sigs.sigs.directory_signature, }; sigs.iter() - .filter_map(|sig| match sig { - NdiDirectorySignature::Known { - h_kp_auth_id_rsa, - h_kp_auth_sign_rsa, - .. - } => Some(AuthCertKeyIds { - id_fingerprint: *h_kp_auth_id_rsa, - sk_fingerprint: *h_kp_auth_sign_rsa, - }), - // TODO DIRMIRROR: This is inappropriate, but because we are - // using poc, we have to refactor this either way. - _ => None, - }) + .map(|sig| sig.key_ids) .collect() } } diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index c44f78db0f..68773bc204 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -527,6 +527,7 @@ define_derive_deftly! { /// /// * [`DirectorySignaturesHashesAccu`] /// * [`DirectorySignaturesHashesAccu::update_from`] + /// * [`DirectorySignaturesHashesAccu::hash_slice_for_verification`] DirectorySignaturesHashesAccu: ${define FNAME ${paste ${snake_case $vname}} } diff --git a/crates/tor-netdoc/src/parse2/poc/netstatus.rs b/crates/tor-netdoc/src/parse2/poc/netstatus.rs index 0fc773b0d8..322347c65f 100644 --- a/crates/tor-netdoc/src/parse2/poc/netstatus.rs +++ b/crates/tor-netdoc/src/parse2/poc/netstatus.rs @@ -3,15 +3,14 @@ use super::*; use crate::doc::{self, authcert}; -use crate::types; -use authcert::AuthCert as DirAuthKeyCert; +use crate::types::{self, KeywordOrString}; +use authcert::{AuthCertKeyIds, AuthCert as DirAuthKeyCert}; use doc::netstatus::{ConsensusAuthoritySection, DirectorySignaturesHashesAccu, VoteAuthoritySection}; +pub use doc::netstatus::Signature as NdiDirectorySignature; mod ns_per_flavour_macros; pub use ns_per_flavour_macros::*; -use crate::doc::netstatus::DirectorySignatureHashAlgo as ProdAlgo; // XXXX remove - ns_per_flavour_macros::ns_export_flavoured_types! { NetworkStatus, NetworkStatusUnverified, Router, } @@ -46,98 +45,6 @@ pub struct NdiR { pub identity: String, // In non-demo, use a better type } -/// `directory-signature` value -#[derive(Debug, Clone)] -#[non_exhaustive] -pub enum NdiDirectorySignature { - /// Known "hash function" name - Known { - /// Hash algorithm - hash_algo: DirectorySignatureHashAlgo, - /// H(KP\_auth\_id\_rsa) - h_kp_auth_id_rsa: pk::rsa::RsaIdentity, - /// H(kp\_auth\_sign\_rsa) - h_kp_auth_sign_rsa: pk::rsa::RsaIdentity, - /// RSA signature - rsa_signature: Vec, - }, - /// Unknown "hash function" name - /// - /// TODO torspec#350; - /// might have been an unknown algorithm, or might be invalid hex, or something. - Unknown {}, -} -define_derive_deftly! { - /// Ad-hoc derives for [`DirectorySignatureHash`] impls, avoiding copypasta bugs - /// - /// # Input - /// - /// * `pub enum DirectorySignatureHashAlgo` - /// * Unit variants - /// * Each variant with `#[deftly(hash_len = "N")]` - /// where `N` is the digest length in bytes. - /// - /// # Generated code - /// - /// * `pub enum DirectorySignaturesHashesAccu`, - /// with each variant a 1-tuple containing `Option<[u8; N]>`. - /// (These are `None` if this hash has not been computed yet.) - /// - /// * `DirectorySignaturesHashesAccu::parse_keyword_and_hash` - /// - /// * `DirectorySignaturesHashesAccu::hash_slice_for_verification` - DirectorySignatureHashesAccu expect items, beta_deftly: - - ${define FNAME ${paste ${snake_case $vname}} } - - impl DirectorySignaturesHashesAccu { - /// If `algorithm` is an algorithm name, calculate the hash - /// - /// Otherwise, return `None`. - fn parse_keyword_and_hash( - &mut self, - algorithm: &str, - body: &SignatureHashInputs, - ) -> Option<$ttype> { - let Ok(algo) = algorithm.parse() - else { return None }; - - // XXXX this is just wrong, but our tests don't catch it right now, - // and we are about to delete this whole function. - let algo_is = doc::netstatus::DigestAlgoInSignature(Some( - types::KeywordOrString::Known(algo) - )); - - self.update_from(&algo_is, body); - - Some(algo.into()) - } - } - - // XXXX we have two DirectorySignatureHashAlgo types, which we, briefly, - // need to convert between! - impl From for DirectorySignatureHashAlgo { - fn from(other: ProdAlgo) -> Self { - match other { $( - ProdAlgo::$vname => DirectorySignatureHashAlgo::$vname, - ) } - } - } - // XXXX - impl From for ProdAlgo { - fn from(other: DirectorySignatureHashAlgo) -> Self { - match other { $( - DirectorySignatureHashAlgo::$vname => ProdAlgo::$vname, - ) } - } - } -} - -define_directory_signature_hash_algo! { - #[derive_deftly(DirectorySignatureHashesAccu)] - #[derive_deftly_adhoc] // XXXX suppress unknown attr errors -} - /// Unsupported `vote-status` value /// /// This message is not normally actually shown since our `ErrorProblem` doesn't contain it. @@ -146,60 +53,6 @@ define_directory_signature_hash_algo! { #[error("invalid value for vote-status in network status document")] pub struct InvalidNetworkStatusVoteStatus {} -impl SignatureItemParseable for NdiDirectorySignature { - type HashAccu = DirectorySignaturesHashesAccu; - - // TODO torspec#350. That's why this manual impl is needed - fn from_unparsed_and_body<'s>( - mut input: UnparsedItem<'s>, - document_body: &SignatureHashInputs<'_>, - hashes: &mut DirectorySignaturesHashesAccu, - ) -> Result { - let object = input.object(); - let args = input.args_mut(); - let maybe_algorithm = args.clone().next().ok_or(EP::MissingArgument { - field: "algorithm/h_kp_auth_id_rsa", - })?; - - let hash_algo = - if let Some(algo) = hashes.parse_keyword_and_hash(maybe_algorithm, document_body) { - let _: &str = args.next().expect("we just peeked"); - algo - } else if maybe_algorithm - .find(|c: char| !c.is_ascii_hexdigit()) - .is_some() - { - // Not hex. Must be some unknown algorithm. - // There might be Object, but don't worry if not. - return Ok(NdiDirectorySignature::Unknown {}); - } else { - hashes - .parse_keyword_and_hash("sha1", document_body) - .expect("sha1 is not valid?") - }; - - let rsa_signature = object.ok_or(EP::MissingObject)?.decode_data()?; - - let mut fingerprint_arg = |field: &'static str| { - (|| { - args.next() - .ok_or(AE::Missing)? - .parse::() - .map_err(|_e| AE::Invalid) - .map(pk::rsa::RsaIdentity::from) - })() - .map_err(args.error_handler(field)) - }; - - Ok(NdiDirectorySignature::Known { - hash_algo, - rsa_signature, - h_kp_auth_id_rsa: fingerprint_arg("h_kp_auth_id_rsa")?, - h_kp_auth_sign_rsa: fingerprint_arg("h_kp_auth_sign_rsa")?, - }) - } -} - /// Meat of the verification functions for network documents /// /// Checks that at least `threshold` members of `trusted` @@ -217,13 +70,17 @@ fn verify_general_timeless( let mut ok = HashSet::::new(); for sig in signatures { - match sig { - NdiDirectorySignature::Known { - hash_algo, - h_kp_auth_id_rsa, - h_kp_auth_sign_rsa, - rsa_signature, - } => { + let NdiDirectorySignature { + digest_algo: hash_algo, + key_ids: AuthCertKeyIds { + id_fingerprint: h_kp_auth_id_rsa, + sk_fingerprint: h_kp_auth_sign_rsa, + }, + signature: rsa_signature, + } = sig; + + match hash_algo.algorithm() { + KeywordOrString::Known(hash_algo) => { let Some(authority) = ({ trusted .iter() @@ -242,14 +99,14 @@ fn verify_general_timeless( }; let h = hashes - .hash_slice_for_verification((*hash_algo).into()) + .hash_slice_for_verification(*hash_algo) .ok_or(VF::Bug)?; let () = cert.dir_signing_key.verify(h, rsa_signature)?; ok.insert(*authority); } - NdiDirectorySignature::Unknown { .. } => {} + KeywordOrString::Unknown(..) => {} } } -- GitLab From ea42c37e098b2a7b56b94de84ec44b2968c999b4 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 16:48:30 +0100 Subject: [PATCH 22/30] tor-netdoc: Fix signature of hash_slice_for_verification (pre-fmt) --- crates/tor-netdoc/src/doc/netstatus.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index 68773bc204..6dcc08d189 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -600,7 +600,10 @@ define_derive_deftly! { /// `None` if the value wasn't computed. /// That shouldn't happen. // XXXX make no longer pub(crate) - pub(crate) fn hash_slice_for_verification(&self, algo: $ttype) -> Option<&[u8]> { + pub(crate) fn hash_slice_for_verification( + &self, + algo: $ttype, + ) -> Option<&[u8]> { // XXXX handle sha1_unnamed correctly match algo { $( $vtype => Some(self.$FNAME.as_ref()?), -- GitLab From 038c01aa4ba8501b8d107a27ccfd577ccf3a4ac4 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 17:30:36 +0100 Subject: [PATCH 23/30] tor-netdoc: Fix signature of hash_slice_for_verification In order to handle marked vs unmarked SHA1 correctly, it needs the original DigestAlgoInSignature. The only call site is in poc's verification code. --- crates/tor-netdoc/src/doc/netstatus.rs | 13 ++++++++----- crates/tor-netdoc/src/parse2/poc/netstatus.rs | 11 ++--------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index 6dcc08d189..142e434d49 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -602,12 +602,15 @@ define_derive_deftly! { // XXXX make no longer pub(crate) pub(crate) fn hash_slice_for_verification( &self, - algo: $ttype, + algo: &DigestAlgoInSignature, ) -> Option<&[u8]> { - // XXXX handle sha1_unnamed correctly - match algo { $( - $vtype => Some(self.$FNAME.as_ref()?), - ) } + match &**algo { + $( + Some(KeywordOrString::Known($vtype)) => Some(self.$FNAME.as_ref()?), + ) + None => Some(self.sha1.as_ref()?), // XXXX handle this correctly + Some(KeywordOrString::Unknown(..)) => None, + } } } } diff --git a/crates/tor-netdoc/src/parse2/poc/netstatus.rs b/crates/tor-netdoc/src/parse2/poc/netstatus.rs index 322347c65f..2610baa9a2 100644 --- a/crates/tor-netdoc/src/parse2/poc/netstatus.rs +++ b/crates/tor-netdoc/src/parse2/poc/netstatus.rs @@ -3,7 +3,7 @@ use super::*; use crate::doc::{self, authcert}; -use crate::types::{self, KeywordOrString}; +use crate::types; use authcert::{AuthCertKeyIds, AuthCert as DirAuthKeyCert}; use doc::netstatus::{ConsensusAuthoritySection, DirectorySignaturesHashesAccu, VoteAuthoritySection}; pub use doc::netstatus::Signature as NdiDirectorySignature; @@ -79,8 +79,7 @@ fn verify_general_timeless( signature: rsa_signature, } = sig; - match hash_algo.algorithm() { - KeywordOrString::Known(hash_algo) => { + if let Some(h) = hashes.hash_slice_for_verification(hash_algo) { let Some(authority) = ({ trusted .iter() @@ -98,15 +97,9 @@ fn verify_general_timeless( continue; }; - let h = hashes - .hash_slice_for_verification(*hash_algo) - .ok_or(VF::Bug)?; - let () = cert.dir_signing_key.verify(h, rsa_signature)?; ok.insert(*authority); - } - KeywordOrString::Unknown(..) => {} } } -- GitLab From 038f4de001a5576fa6c94a35ca0599e946ac8d01 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 17:32:03 +0100 Subject: [PATCH 24/30] tor-netdoc: Fix handling of explicit vs implicit dir sig SHA1 hash Now that everything is prepared, we can add the sha1_unnamed field in the hash accumulator. --- crates/tor-netdoc/src/doc/netstatus.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index 142e434d49..ba688ec753 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -543,9 +543,6 @@ define_derive_deftly! { pub(crate) $FNAME: Option<[u8; ${vmeta(hash_len) as expr}]>, ) -/* - // XXXX actually implement this - /// `sha1` but without the algorithm name /// /// This is needed because the hash includes the whole signature item keyword line, @@ -556,7 +553,6 @@ define_derive_deftly! { /// or multiple signatures with different syntax would overwrite each others' /// different hashes. pub(crate) sha1_unnamed: Option<[u8; 20]>, -*/ } impl DirectorySignaturesHashesAccu { @@ -587,7 +583,7 @@ define_derive_deftly! { } ) None => { - ${define UPDATE sha1} // XXXX wrong + ${define UPDATE sha1_unnamed} ${define ALGO Sha1} $HASH } @@ -608,7 +604,7 @@ define_derive_deftly! { $( Some(KeywordOrString::Known($vtype)) => Some(self.$FNAME.as_ref()?), ) - None => Some(self.sha1.as_ref()?), // XXXX handle this correctly + None => Some(self.sha1_unnamed.as_ref()?), Some(KeywordOrString::Unknown(..)) => None, } } -- GitLab From d7ce8e22f3ed49ce8f4db4a0a35899a93bed4c8a Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 17:40:38 +0100 Subject: [PATCH 25/30] tor-netdoc: directory-signature: don't re-hash unnecessarily --- crates/tor-netdoc/src/doc/netstatus.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index ba688ec753..0ee5305da2 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -566,12 +566,13 @@ define_derive_deftly! { // Update the hash in self.$UPDATE according to algorithm $AGLO // (uses dynamic bindings of those parameters) ${define HASH { - // XXXX indentation is wrong + // Avoid recalculating if we don't need to + self.$UPDATE.get_or_insert_with(|| { let mut h = tor_llcrypto::d::$ALGO::new(); h.update(body.body().body()); h.update(body.signature_item_kw_spc); - // XXXX Unconditionally write. This can involve wasted work. - self.$UPDATE = Some(h.finalize().into()); + h.finalize().into() + }); }} match &**algo { -- GitLab From 255eba7270ad74ffb6c499ae471ec5a2e090d70e Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 17:43:36 +0100 Subject: [PATCH 26/30] tor-netdoc: re-privatise some methods hash_slice_for_verification is still needed by code in poc, that we're not replacing in this MR. So that TODO gets downgraded, instead. --- crates/tor-netdoc/src/doc/netstatus.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index 0ee5305da2..19e6351967 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -539,8 +539,7 @@ define_derive_deftly! { pub struct DirectorySignaturesHashesAccu { $( ${vattrs doc} - // XXXX make no longer pub(crate) - pub(crate) $FNAME: Option<[u8; ${vmeta(hash_len) as expr}]>, + $FNAME: Option<[u8; ${vmeta(hash_len) as expr}]>, ) /// `sha1` but without the algorithm name @@ -552,13 +551,12 @@ define_derive_deftly! { /// So we mustn't use the `sha1` field for both implicit and explicit use of SHA-1, /// or multiple signatures with different syntax would overwrite each others' /// different hashes. - pub(crate) sha1_unnamed: Option<[u8; 20]>, + sha1_unnamed: Option<[u8; 20]>, } impl DirectorySignaturesHashesAccu { /// Calculate the hash for a signature item and update this accumulator - // XXXX make no longer pub(crate) - pub(crate) fn update_from( + fn update_from( &mut self, algo: &DigestAlgoInSignature, body: &SignatureHashInputs, @@ -596,7 +594,7 @@ define_derive_deftly! { /// /// `None` if the value wasn't computed. /// That shouldn't happen. - // XXXX make no longer pub(crate) + // TODO DIRAUTH make private when poc's verification is abolished pub(crate) fn hash_slice_for_verification( &self, algo: &DigestAlgoInSignature, -- GitLab From 9f0ea2b71e52d57d6a8e25479ec384cb30311c44 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 17:45:06 +0100 Subject: [PATCH 27/30] tor-netdoc: Turn DirectorySignatureHashAlgo back into a normal struct Abolish the terrible macro. Review with --color-moved-ws=allow-indentation-change --color-moved. --- crates/tor-netdoc/src/doc/netstatus.rs | 14 +++++++++++-- crates/tor-netdoc/src/util.rs | 29 -------------------------- 2 files changed, 12 insertions(+), 31 deletions(-) diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index 19e6351967..f92a2a0e58 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -610,8 +610,18 @@ define_derive_deftly! { } } -define_directory_signature_hash_algo! { - #[derive_deftly(DirectorySignaturesHashesAccu)] +/// `directory-signature` hash algorithm argument +#[derive(Clone, Copy, Debug, Eq, PartialEq, strum::Display, strum::EnumString, Deftly)] +#[derive_deftly(DirectorySignaturesHashesAccu)] +#[non_exhaustive] +#[strum(serialize_all = "snake_case")] +pub enum DirectorySignatureHashAlgo { + /// SHA-1 + #[deftly(hash_len = "20")] + Sha1, + /// SHA-256 + #[deftly(hash_len = "32")] + Sha256, } /// `algorithm` field in a `directory-signature` item diff --git a/crates/tor-netdoc/src/util.rs b/crates/tor-netdoc/src/util.rs index c20cd496b9..5f0df0b59e 100644 --- a/crates/tor-netdoc/src/util.rs +++ b/crates/tor-netdoc/src/util.rs @@ -91,32 +91,3 @@ 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::Display, 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 1153b25ffc98ec8cddf61fc5c03f0a896587bce7 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 29 Apr 2026 18:24:16 +0100 Subject: [PATCH 28/30] tor-netdoc: Apply deferred rustfmt churn --- crates/tor-dirserver/src/mirror/operation.rs | 4 +- crates/tor-netdoc/src/doc/authcert.rs | 8 ++- crates/tor-netdoc/src/doc/netstatus.rs | 12 ++-- crates/tor-netdoc/src/parse2/poc/netstatus.rs | 55 ++++++++++--------- 4 files changed, 42 insertions(+), 37 deletions(-) diff --git a/crates/tor-dirserver/src/mirror/operation.rs b/crates/tor-dirserver/src/mirror/operation.rs index 8dacfccdf7..b40790b48a 100644 --- a/crates/tor-dirserver/src/mirror/operation.rs +++ b/crates/tor-dirserver/src/mirror/operation.rs @@ -718,9 +718,7 @@ impl FlavoredConsensusSigned { Self::Ns(ns) => &ns.sigs.sigs.directory_signature, Self::Md(md) => &md.sigs.sigs.directory_signature, }; - sigs.iter() - .map(|sig| sig.key_ids) - .collect() + sigs.iter().map(|sig| sig.key_ids).collect() } } diff --git a/crates/tor-netdoc/src/doc/authcert.rs b/crates/tor-netdoc/src/doc/authcert.rs index 79ab00c3f9..c2ea67f0a2 100644 --- a/crates/tor-netdoc/src/doc/authcert.rs +++ b/crates/tor-netdoc/src/doc/authcert.rs @@ -7,13 +7,15 @@ //! signing keys to sign votes and consensuses. use crate::batching_split_before::IteratorExt as _; -use crate::encode::{Bug, ItemArgument, ItemEncoder, ItemObjectEncodable, NetdocEncodable, NetdocEncoder}; +use crate::encode::{ + Bug, ItemArgument, ItemEncoder, ItemObjectEncodable, NetdocEncodable, NetdocEncoder, +}; use crate::parse::keyword::Keyword; use crate::parse::parser::{Section, SectionRules}; use crate::parse::tokenize::{ItemResult, NetDocReader}; use crate::parse2::{ - self, ItemObjectParseable, NetdocUnverified as _, sig_hashes::Sha1WholeKeywordLine, - ArgumentError, ArgumentStream, ItemArgumentParseable, + self, ArgumentError, ArgumentStream, ItemArgumentParseable, ItemObjectParseable, + NetdocUnverified as _, sig_hashes::Sha1WholeKeywordLine, }; use crate::types::misc::{Fingerprint, Iso8601TimeSp, RsaPublicParse1Helper, RsaSha1Signature}; use crate::util::str::Extent; diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index f92a2a0e58..02251e3153 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -65,17 +65,19 @@ pub use proto_statuses_parse2_encode::ProtoStatusesNetdocParseAccumulator; use crate::doc::authcert::EncodedAuthCert; use crate::doc::authcert::{self, AuthCert, AuthCertKeyIds}; -use crate::encode::{ItemArgument, ItemEncoder, ItemValueEncodable, NetdocEncodable, NetdocEncoder}; +use crate::encode::{ + ItemArgument, ItemEncoder, ItemValueEncodable, NetdocEncodable, NetdocEncoder, +}; use crate::parse::keyword::Keyword; use crate::parse::parser::{Section, SectionRules, SectionRulesBuilder}; use crate::parse::tokenize::{Item, ItemResult, NetDocReader}; use crate::parse2::{ - self, ArgumentStream, ErrorProblem, IsStructural, ItemStream, ItemValueParseable, KeywordRef, - NetdocParseable, SignatureHashInputs, SignatureItemParseable, StopAt, UnparsedItem, - ArgumentError, ItemArgumentParseable, + self, ArgumentError, ArgumentStream, ErrorProblem, IsStructural, ItemArgumentParseable, + ItemStream, ItemValueParseable, KeywordRef, NetdocParseable, SignatureHashInputs, + SignatureItemParseable, StopAt, UnparsedItem, }; -use crate::types::{self, *}; use crate::types::relay_flags::{self, DocRelayFlags}; +use crate::types::{self, *}; use crate::util::PeekableIterator; use crate::{Error, KeywordEncodable, NetdocErrorKind as EK, NormalItemArgument, Pos, Result}; use std::collections::{BTreeSet, HashMap, HashSet}; diff --git a/crates/tor-netdoc/src/parse2/poc/netstatus.rs b/crates/tor-netdoc/src/parse2/poc/netstatus.rs index 2610baa9a2..8b911406c8 100644 --- a/crates/tor-netdoc/src/parse2/poc/netstatus.rs +++ b/crates/tor-netdoc/src/parse2/poc/netstatus.rs @@ -4,9 +4,11 @@ use super::*; use crate::doc::{self, authcert}; use crate::types; -use authcert::{AuthCertKeyIds, AuthCert as DirAuthKeyCert}; -use doc::netstatus::{ConsensusAuthoritySection, DirectorySignaturesHashesAccu, VoteAuthoritySection}; +use authcert::{AuthCert as DirAuthKeyCert, AuthCertKeyIds}; pub use doc::netstatus::Signature as NdiDirectorySignature; +use doc::netstatus::{ + ConsensusAuthoritySection, DirectorySignaturesHashesAccu, VoteAuthoritySection, +}; mod ns_per_flavour_macros; pub use ns_per_flavour_macros::*; @@ -72,34 +74,35 @@ fn verify_general_timeless( for sig in signatures { let NdiDirectorySignature { digest_algo: hash_algo, - key_ids: AuthCertKeyIds { - id_fingerprint: h_kp_auth_id_rsa, - sk_fingerprint: h_kp_auth_sign_rsa, - }, + key_ids: + AuthCertKeyIds { + id_fingerprint: h_kp_auth_id_rsa, + sk_fingerprint: h_kp_auth_sign_rsa, + }, signature: rsa_signature, } = sig; if let Some(h) = hashes.hash_slice_for_verification(hash_algo) { - let Some(authority) = ({ - trusted - .iter() - .find(|trusted| **trusted == *h_kp_auth_id_rsa) - }) else { - // unknown kp_auth_id_rsa, ignore it - continue; - }; - let Some(cert) = ({ - certs - .iter() - .find(|cert| cert.dir_signing_key.to_rsa_identity() == *h_kp_auth_sign_rsa) - }) else { - // no cert for this kp_auth_sign_rsa, ignore it - continue; - }; - - let () = cert.dir_signing_key.verify(h, rsa_signature)?; - - ok.insert(*authority); + let Some(authority) = ({ + trusted + .iter() + .find(|trusted| **trusted == *h_kp_auth_id_rsa) + }) else { + // unknown kp_auth_id_rsa, ignore it + continue; + }; + let Some(cert) = ({ + certs + .iter() + .find(|cert| cert.dir_signing_key.to_rsa_identity() == *h_kp_auth_sign_rsa) + }) else { + // no cert for this kp_auth_sign_rsa, ignore it + continue; + }; + + let () = cert.dir_signing_key.verify(h, rsa_signature)?; + + ok.insert(*authority); } } -- GitLab From a1357f069b8af4de5a4efaa2a2ca5442f67e49fa Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Tue, 5 May 2026 15:22:56 +0100 Subject: [PATCH 29/30] tor-netdoc: Replace an IEFE with use of .and_then() As per https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/3937#note_3405033 Suggested-by: Clara Engler --- crates/tor-netdoc/src/doc/netstatus.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index 02251e3153..295e917841 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -643,13 +643,14 @@ pub struct DigestAlgoInSignature(pub Option(args: &mut ArgumentStream<'s>) -> StdResult { - let v = if (|| { - let s = args.clone().next()?; + let v = if + args.clone().next() // Treat it as a fingerprint if it doesn't have any non-hex characters // (including lowercase ones). If we reuse this item for new algorithms // they should have at least one letter g-z in their name. + .and_then(|s| { s.chars().all(|c| c.is_ascii_hexdigit()).then_some(()) - })() + }) .is_some() { // next argument looks enough like a fingerprint that we don't treat as an algo name -- GitLab From a257e73c502d420d01eead1793ba7ed223aa5931 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Tue, 5 May 2026 15:24:02 +0100 Subject: [PATCH 30/30] tor-netdoc: Replace an IEFE with use of .and_then() (fmt) --- crates/tor-netdoc/src/doc/netstatus.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index 295e917841..985bde6b22 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -643,15 +643,14 @@ pub struct DigestAlgoInSignature(pub Option(args: &mut ArgumentStream<'s>) -> StdResult { - let v = if - args.clone().next() + let v = if args + .clone() + .next() // Treat it as a fingerprint if it doesn't have any non-hex characters // (including lowercase ones). If we reuse this item for new algorithms // they should have at least one letter g-z in their name. - .and_then(|s| { - s.chars().all(|c| c.is_ascii_hexdigit()).then_some(()) - }) - .is_some() + .and_then(|s| s.chars().all(|c| c.is_ascii_hexdigit()).then_some(())) + .is_some() { // next argument looks enough like a fingerprint that we don't treat as an algo name None -- GitLab