diff --git a/crates/tor-dirserver/src/mirror/operation.rs b/crates/tor-dirserver/src/mirror/operation.rs index 08b49679ea0452c0211514e13175cf06a7305048..b40790b48a4718aa59ee6df1ba611543036889e1 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, }, }; @@ -718,21 +718,7 @@ impl FlavoredConsensusSigned { Self::Ns(ns) => &ns.sigs.sigs.directory_signature, 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, - }) - .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 0021b8809bcfe15b80209a58d9764677f143aff1..c2ea67f0a22ac976035dc7f711296876712ed55b 100644 --- a/crates/tor-netdoc/src/doc/authcert.rs +++ b/crates/tor-netdoc/src/doc/authcert.rs @@ -7,12 +7,15 @@ //! 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, + self, ArgumentError, ArgumentStream, ItemArgumentParseable, ItemObjectParseable, + NetdocUnverified as _, sig_hashes::Sha1WholeKeywordLine, }; use crate::types::misc::{Fingerprint, Iso8601TimeSp, RsaPublicParse1Helper, RsaSha1Signature}; use crate::util::str::Extent; @@ -438,6 +441,44 @@ 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. +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 @@ -715,7 +756,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 +783,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 +805,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 +845,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 +858,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,30 +866,13 @@ 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"; - 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 @@ -849,7 +885,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 @@ -859,7 +895,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 @@ -869,367 +905,380 @@ 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_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] - fn dir_auth_signature() { - let res = - parse2::parse_netdoc::(&ParseInput::new(AUTHCERT_RAW, "")) - .unwrap(); + #[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 a valid signature. + // 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 + ); + + // Test an almost too new. + res.clone() + .verify( + &[to_rsa_id(FINGERPRINT)], + Duration::ZERO, + Duration::ZERO, + to_system_time(DIR_KEY_PUBLISHED), + ) + .unwrap(); - // ... but succeed again with a clock skew tolerance. + // 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 + ); + + // Test an almost too old. + res.clone() + .verify( + &[to_rsa_id(FINGERPRINT)], + Duration::ZERO, + Duration::ZERO, + to_system_time(DIR_KEY_EXPIRES), + ) + .unwrap(); - // ... but succeed again with a clock skew tolerance. + // 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 + ); + + // ... 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] + 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")] - 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] - 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(()) - } + 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(()) } } diff --git a/crates/tor-netdoc/src/doc/microdesc.rs b/crates/tor-netdoc/src/doc/microdesc.rs index 740ebde00ad00748cfb996b87b787b8b5c584d24..44892efc776d63ab53d677dfa7a675df255e5168 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/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index 920848a32e300abd53bece37455866cdac86c455..985bde6b22055ff8ff24e830f9753fcc617237ea 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -64,17 +64,20 @@ pub use proto_statuses_parse2_encode::ProtoStatusesNetdocParseAccumulator; #[cfg(feature = "incomplete")] use crate::doc::authcert::EncodedAuthCert; -use crate::doc::authcert::{AuthCert, AuthCertKeyIds}; -use crate::encode::{ItemValueEncodable, NetdocEncodable, NetdocEncoder}; +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}; use crate::parse::tokenize::{Item, ItemResult, NetDocReader}; use crate::parse2::{ - self, ArgumentStream, ErrorProblem, IsStructural, ItemStream, ItemValueParseable, KeywordRef, - NetdocParseable, StopAt, + self, ArgumentError, ArgumentStream, ErrorProblem, IsStructural, ItemArgumentParseable, + ItemStream, ItemValueParseable, KeywordRef, NetdocParseable, SignatureHashInputs, + SignatureItemParseable, StopAt, UnparsedItem, }; -use crate::types::misc::*; 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}; @@ -519,26 +522,200 @@ impl ConsensusFlavor { } } -define_directory_signature_hash_algo! { - #[derive_deftly_adhoc] // TODO DIRAUTH; suppresses complaints about attrs used only in poc +define_derive_deftly! { + /// Bespoke derives applied to [`DirectorySignatureHashAlgo`] + /// + /// Generates: + /// + /// * [`DirectorySignaturesHashesAccu`] + /// * [`DirectorySignaturesHashesAccu::update_from`] + /// * [`DirectorySignaturesHashesAccu::hash_slice_for_verification`] + 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} + $FNAME: Option<[u8; ${vmeta(hash_len) as expr}]>, + ) + + /// `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. + sha1_unnamed: Option<[u8; 20]>, + } + + impl DirectorySignaturesHashesAccu { + /// Calculate the hash for a signature item and update this accumulator + 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 { + // 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); + h.finalize().into() + }); + }} + + match &**algo { + $( + Some(KeywordOrString::Known($vtype)) => { + ${define UPDATE $FNAME} + ${define ALGO $vname} + $HASH + } + ) + None => { + ${define UPDATE sha1_unnamed} + ${define ALGO Sha1} + $HASH + } + 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. + // TODO DIRAUTH make private when poc's verification is abolished + pub(crate) fn hash_slice_for_verification( + &self, + algo: &DigestAlgoInSignature, + ) -> Option<&[u8]> { + match &**algo { + $( + Some(KeywordOrString::Known($vtype)) => Some(self.$FNAME.as_ref()?), + ) + None => Some(self.sha1_unnamed.as_ref()?), + Some(KeywordOrString::Unknown(..)) => None, + } + } + } +} + +/// `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 +/// +/// 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 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 + 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)] +/// +/// Implements `ItemValueParseable` which parses without hashing anything; +/// this is mostly useful for use by the `SignatureItemParseable` implementation. +#[derive(Debug, Clone, Deftly)] +#[derive_deftly(ItemValueEncodable, ItemValueParseable)] #[non_exhaustive] pub struct Signature { /// The name of the digest algorithm used to make the 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. + #[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, } +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] @@ -1258,7 +1435,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; @@ -1463,6 +1640,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 +1752,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. diff --git a/crates/tor-netdoc/src/parse2/derive.rs b/crates/tor-netdoc/src/parse2/derive.rs index bf5d3f65e53c049dca99dabcb3b8250adf9e8067..73f26bd000dc504dc2c74aaef80aa163c737e31d 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} diff --git a/crates/tor-netdoc/src/parse2/internal_prelude.rs b/crates/tor-netdoc/src/parse2/internal_prelude.rs index f545150a60e71813822dd71ec4c2d0304776bd75..5702b557c67da43cab1c97fc0bc25a34fb2a7f01 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, + IsStructural, ItemArgumentParseable, ItemObjectParseable, ItemValueParseable, + NetdocParseable, NetdocParseableFields, }, }; pub use crate::types::Unknown; diff --git a/crates/tor-netdoc/src/parse2/poc/netstatus.rs b/crates/tor-netdoc/src/parse2/poc/netstatus.rs index ba6bfed62f945515b8bbd02fd1ceaf793d58ab94..8b911406c8e927311e53ef318fcfb01615593293 100644 --- a/crates/tor-netdoc/src/parse2/poc/netstatus.rs +++ b/crates/tor-netdoc/src/parse2/poc/netstatus.rs @@ -4,8 +4,11 @@ use super::*; use crate::doc::{self, authcert}; use crate::types; -use authcert::AuthCert as DirAuthKeyCert; -use doc::netstatus::{ConsensusAuthoritySection, 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::*; @@ -44,100 +47,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}} } - - /// `directory-signature`a hash algorithm argument - #[derive(Clone, Copy, Default, Debug, Eq, PartialEq, Deftly)] - #[derive_deftly(AsMutSelf)] - #[non_exhaustive] - pub struct DirectorySignaturesHashesAccu { - $( - ${vattrs doc} - $FNAME: Option<[u8; ${vmeta(hash_len) as expr}]>, - ) - } - - 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> { - 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); - self.$FNAME = Some(h.finalize().into()); - $vtype - } - ) - _ => return None, - }) - } - - /// Return the hash value for this algorithm, as a slice - /// - /// `None` if the value wasn't computed. - /// That shouldn't happen. - fn hash_slice_for_verification(&self, algo: $ttype) -> Option<&[u8]> { - match algo { $( - $vtype => Some(self.$FNAME.as_ref()?), - ) } - } - } -} - -define_directory_signature_hash_algo! { - #[derive_deftly(DirectorySignatureHashesAccu)] -} - /// Unsupported `vote-status` value /// /// This message is not normally actually shown since our `ErrorProblem` doesn't contain it. @@ -146,60 +55,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,39 +72,37 @@ 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 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 h = hashes - .hash_slice_for_verification(*hash_algo) - .ok_or(VF::Bug)?; + 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; + + 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)?; + let () = cert.dir_signing_key.verify(h, rsa_signature)?; - ok.insert(*authority); - } - NdiDirectorySignature::Unknown { .. } => {} + ok.insert(*authority); } } diff --git a/crates/tor-netdoc/src/types.rs b/crates/tor-netdoc/src/types.rs index 8cfab65646efde0a1e203fb27f7c62e67fb381d3..ca7416da25d2792a29f54b0a63264fec894a8811 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}; diff --git a/crates/tor-netdoc/src/util.rs b/crates/tor-netdoc/src/util.rs index 317fa29001f06f0cd2351615dd35abadba6b976d..5f0df0b59eb0841134fe64cb387ed789cf651a54 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::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, - } -} }