diff --git a/crates/tor-netdoc/semver.md b/crates/tor-netdoc/semver.md index acd988c26a323e298d885b4e5c77a8ecba401a0f..2d136d3f6211fcae7f289cf00f6f3fd5e91febfe 100644 --- a/crates/tor-netdoc/semver.md +++ b/crates/tor-netdoc/semver.md @@ -1,3 +1,4 @@ +ADDED: `NetdocEncodableFields` for `AddrPolicy` ADDED: `encode::ItemArgument` for `SpFingerprint` ADDED: `ItemValueEncodable` for `ExtraInfoDigests` ADDED: `ItemValueEncodable` for `RouterDescIntroItem` diff --git a/crates/tor-netdoc/src/types/policy.rs b/crates/tor-netdoc/src/types/policy.rs index 0e342aea7d796e5ccbc893b7048c57cec4016ae0..a538ba3fe5a2f47c1a2468a02927eb23944cc436 100644 --- a/crates/tor-netdoc/src/types/policy.rs +++ b/crates/tor-netdoc/src/types/policy.rs @@ -81,13 +81,13 @@ pub struct PortRange { impl PortRange { /// Create a new port range spanning from lo to hi, asserting that /// the correct invariants hold. - fn new_unchecked(lo: u16, hi: u16) -> Self { + const fn new_unchecked(lo: u16, hi: u16) -> Self { assert!(lo != 0); assert!(lo <= hi); PortRange { lo, hi } } /// Create a port range containing all ports. - pub fn new_all() -> Self { + pub const fn new_all() -> Self { PortRange::new_unchecked(1, 65535) } /// Create a new PortRange. diff --git a/crates/tor-netdoc/src/types/policy/addrpolicy.rs b/crates/tor-netdoc/src/types/policy/addrpolicy.rs index e81d4eb696630cef49a46329a99aab21c5fe35c5..86c161c0ae12ebf7b5977937f4530c7c40fcfd6b 100644 --- a/crates/tor-netdoc/src/types/policy/addrpolicy.rs +++ b/crates/tor-netdoc/src/types/policy/addrpolicy.rs @@ -5,7 +5,10 @@ use std::fmt::Display; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; use std::str::FromStr; +use itertools::chain; + use crate::NormalItemArgument; +use crate::encode::NetdocEncodableFields; use crate::parse2::{ ErrorProblem as EP, ItemArgumentParseable, ItemStream, KeywordRef, NetdocParseableFields, UnparsedItem, @@ -108,6 +111,39 @@ impl NetdocParseableFields for AddrPolicy { } } +impl NetdocEncodableFields for AddrPolicy { + fn encode_fields(&self, out: &mut crate::encode::NetdocEncoder) -> Result<(), tor_error::Bug> { + // The order of this field is significant, meaning we have to emit the + // values as they are. The spec also strongly recommends a trailing + // `accept *:*` or `reject *:*`. To comply with this, we check for + // an existing final rule with an ALL pattern and add a `reject *:*` + // if that is not the case. This is not super nice and ideally we would + // do this somewhere in the type construction, but we cannot do some + // right now because the legacy parser accumulates it as it is. + const ALL: AddrPortPattern = AddrPortPattern::new_all(); + const DEFAULT_DENY: AddrPolicyRule = AddrPolicyRule { + kind: RuleKind::Reject, + pattern: ALL, + }; + + // Add default deny in case of an absent trailing ALL. + let default_deny = match self.rules.last() { + // Do nothing if there already is a trailing ALL. + Some(AddrPolicyRule { + kind: _, + pattern: ALL, + }) => None, + // Add a default deny to the end. + _ => Some(&DEFAULT_DENY), + }; + + for rule in chain!(&self.rules, default_deny) { + out.push_raw_string(&format_args!("{} {}\n", rule.kind, rule.pattern)); + } + Ok(()) + } +} + /// A single rule in an address policy. /// /// Contains a pattern and what to do with things that match it. @@ -161,7 +197,7 @@ pub struct AddrPortPattern { impl AddrPortPattern { /// Return an AddrPortPattern matching all targets. - pub fn new_all() -> Self { + pub const fn new_all() -> Self { Self { pattern: IpPattern::Star, ports: PortRange::new_all(), @@ -326,6 +362,8 @@ mod test { #![allow(clippy::needless_pass_by_value)] #![allow(clippy::string_slice)] // See arti#2571 //! + use crate::encode::{NetdocEncodable, NetdocEncoder}; + use super::*; #[test] @@ -451,10 +489,7 @@ mod test { #[test] fn parse2() { - use crate::{ - parse2::{self, ParseInput}, - types::Ignored, - }; + use crate::parse2::{self, ParseInput}; use derive_deftly::Deftly; const RULES: &str = "\ @@ -468,16 +503,16 @@ mod test { reject *:*\n"; #[derive(Deftly)] - #[derive_deftly(NetdocParseable)] + #[derive_deftly(NetdocParseable, NetdocEncodable)] struct Wrapper { #[allow(dead_code)] - intro: Ignored, + intro: (), #[deftly(netdoc(flatten))] ipv4_policy: AddrPolicy, } let wrapper = parse2::parse_netdoc::(&ParseInput::new(RULES, "")).unwrap(); - let ap = wrapper.ipv4_policy; + let ap = wrapper.ipv4_policy.clone(); assert_eq!( ap.allows_sockaddr(&"1.1.1.1:80".parse().unwrap()), @@ -504,5 +539,38 @@ mod test { ap.allows_sockaddr(&"1.1.1.1:70".parse().unwrap()), Some(RuleKind::Reject) ); + + // Do round-trip encoding. + let mut enc = NetdocEncoder::default(); + wrapper.encode_unsigned(&mut enc).unwrap(); + assert_eq!(RULES, enc.finish().unwrap()); + + // Test default deny. + let accept_all = { + let mut ap = AddrPolicy::new(); + ap.push(RuleKind::Accept, AddrPortPattern::new_all()); + ap + }; + let reject_all = { + let mut ap = AddrPolicy::new(); + ap.push(RuleKind::Reject, AddrPortPattern::new_all()); + ap + }; + + let tests = [ + (accept_all.clone(), "accept *:*"), // do not add default deny to existing + (reject_all.clone(), "reject *:*"), // do not add default deny to existing + (AddrPolicy::new(), "reject *:*"), // add default deny to empty + ]; + for (input, expected_tail) in tests { + let mut enc = NetdocEncoder::default(); + Wrapper { + intro: (), + ipv4_policy: input, + } + .encode_unsigned(&mut enc) + .unwrap(); + assert_eq!(expected_tail, enc.finish().unwrap().lines().last().unwrap()); + } } }