From 00c51bf63f3df6d47dc9e8afb2be2e8d56436f7d Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Tue, 23 Aug 2022 17:22:59 +0100
Subject: [PATCH 01/12] tor-config misc tests: Add standard lint suppression
 block

---
 crates/tor-config/src/misc.rs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/crates/tor-config/src/misc.rs b/crates/tor-config/src/misc.rs
index a3e6f7a85..a6791c657 100644
--- a/crates/tor-config/src/misc.rs
+++ b/crates/tor-config/src/misc.rs
@@ -76,6 +76,14 @@ impl TryFrom<PaddingLevelSerde> for PaddingLevel {
 
 #[cfg(test)]
 mod test {
+    // @@ begin test lint list maintained by maint/add_warning @@
+    #![allow(clippy::bool_assert_comparison)]
+    #![allow(clippy::clone_on_copy)]
+    #![allow(clippy::dbg_macro)]
+    #![allow(clippy::print_stderr)]
+    #![allow(clippy::print_stdout)]
+    #![allow(clippy::unwrap_used)]
+    //! <!-- @@ end test lint list maintained by maint/add_warning @@ -->
     use super::*;
 
     #[derive(Debug, Deserialize, Serialize)]

From f58826812833f16edbd2488a5abf096eee6ee071 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Tue, 23 Aug 2022 14:19:28 +0100
Subject: [PATCH 02/12] tor-config: Provide misc::Listen

---
 Cargo.lock                    |   1 +
 crates/tor-config/Cargo.toml  |   1 +
 crates/tor-config/src/misc.rs | 310 ++++++++++++++++++++++++++++++++++
 3 files changed, 312 insertions(+)

diff --git a/Cargo.lock b/Cargo.lock
index 4e71c84cb..ab8ad119c 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -3553,6 +3553,7 @@ dependencies = [
  "directories",
  "dirs",
  "educe",
+ "either",
  "fs-mistrust",
  "itertools",
  "once_cell",
diff --git a/crates/tor-config/Cargo.toml b/crates/tor-config/Cargo.toml
index f6684f3d4..2301e285b 100644
--- a/crates/tor-config/Cargo.toml
+++ b/crates/tor-config/Cargo.toml
@@ -21,6 +21,7 @@ config = { version = "0.13", default-features = false, features = ["toml"] }
 derive_builder = { version = "0.11.2", package = "derive_builder_fork_arti" }
 directories = { version = "4", optional = true }
 educe = "0.4.6"
+either = "1"
 fs-mistrust = { path = "../fs-mistrust", version = "0.4.0" }
 itertools = "0.10.1"
 once_cell = "1"
diff --git a/crates/tor-config/src/misc.rs b/crates/tor-config/src/misc.rs
index a6791c657..703d32ee5 100644
--- a/crates/tor-config/src/misc.rs
+++ b/crates/tor-config/src/misc.rs
@@ -4,8 +4,13 @@
 //! and layers, but which don't depend on specific elements of the Tor system.
 
 use std::borrow::Cow;
+use std::iter;
+use std::net;
+use std::num::NonZeroU16;
 
 use educe::Educe;
+use either::Either;
+use itertools::Itertools;
 use serde::{Deserialize, Serialize};
 use strum::{Display, EnumString, IntoStaticStr};
 
@@ -74,6 +79,215 @@ impl TryFrom<PaddingLevelSerde> for PaddingLevel {
     }
 }
 
+/// Specification of (possibly) something to listen on (eg, a port, or some addresses/ports)
+///
+/// Can represent, at least:
+///  * "do not listen"
+///  * Listen on the following port on localhost (IPv6 and IPv4)
+///  * Listen on precisely the following address and port
+///  * Listen on several addresses/ports
+///
+/// Currently only IP (v6 and v4) is supported.
+#[derive(Clone, Hash, Debug, Ord, PartialOrd, Eq, PartialEq, Serialize, Deserialize)]
+#[serde(try_from = "ListenSerde", into = "ListenSerde")]
+#[derive(Educe)]
+#[educe(Default)]
+pub struct Listen(Vec<ListenItem>);
+
+impl Listen {
+    /// Create a new `Listen` specifying no addresses (no listening)
+    pub fn new_none() -> Listen {
+        Listen(vec![])
+    }
+
+    /// Create a new `Listen` specifying listening on a port on localhost
+    ///
+    /// Special case: if `port` is zero, specifies no listening.
+    pub fn new_localhost(port: u16) -> Listen {
+        Listen(port.try_into().ok().map(ListenItem::Localhost).into_iter().collect_vec())
+    }
+
+    /// Create a new `Listen`, possibly specifying listening on a port on localhost
+    ///
+    /// Special case: if `port` is `Some(0)`, also specifies no listening.
+    pub fn new_localhost_optional(port: Option<u16>) -> Listen {
+        Self::new_localhost(port.unwrap_or_default())
+    }
+
+    /// List the network socket addresses to listen on
+    ///
+    /// Fails if the listen spec involves listening on things other than IP addresses.
+    /// (Currently that is not possible.)
+    pub fn ip_addrs(
+        &self,
+    ) -> Result<impl Iterator<Item = net::SocketAddr> + '_, ListenUnsupported> {
+        Ok(self.0.iter().flat_map(|i| i.iter()))
+    }
+
+    /// Get the localhost port to listen on
+    ///
+    /// Returns `None` if listening is configured to be disabled.
+    ///
+    /// Fails, giving an unsupported error, if the configuration
+    /// isn't just "listen on a single localhost port".
+    pub fn localhost_port_deprecated(&self) -> Result<Option<u16>, ListenUnsupported> {
+        use ListenItem as LI;
+        Ok(match &*self.0 {
+            [] => None,
+            [LI::Localhost(port)] => Some((*port).into()),
+            _ => return Err(ListenUnsupported {}),
+        })
+    }
+}
+
+/// [`Listen`] configuration specified something not supported by application code
+#[derive(thiserror::Error, Debug, Clone)]
+#[non_exhaustive]
+#[error("Unsupported listening configration")]
+pub struct ListenUnsupported {}
+
+/// One item in the `Listen`
+///
+/// We distinguish `Localhost`,
+/// rather than just storing two `net:SocketAddr`,
+/// so that we can handle localhost (which means to address families) specially
+/// in order to implement `localhost_port_deprecated()`.
+#[derive(Clone, Hash, Debug, Ord, PartialOrd, Eq, PartialEq, Serialize, Deserialize)]
+#[serde(untagged)]
+enum ListenItem {
+    /// One port, both IPv6 and IPv4
+    Localhost(NonZeroU16),
+
+    /// Any other single socket address
+    General(net::SocketAddr),
+}
+
+impl ListenItem {
+    /// Return the `SocketAddr`s implied by this item
+    fn iter(&self) -> impl Iterator<Item = net::SocketAddr> + '_ {
+        use net::{IpAddr, Ipv4Addr, Ipv6Addr};
+        use ListenItem as LI;
+        match self {
+            &LI::Localhost(port) => Either::Left({
+                let port = port.into();
+                let addrs: [IpAddr; 2] = [Ipv6Addr::LOCALHOST.into(), Ipv4Addr::LOCALHOST.into()];
+                addrs
+                    .into_iter()
+                    .map(move |ip| net::SocketAddr::new(ip, port))
+            }),
+            LI::General(addr) => Either::Right(iter::once(addr).cloned()),
+        }
+    }
+}
+
+/// How we (de) serialize a [`Listen`]
+#[derive(Serialize, Deserialize)]
+#[serde(untagged)]
+enum ListenSerde {
+    /// for `listen = false` (in TOML syntax)
+    Bool(bool),
+
+    /// A bare item
+    One(ListenItemSerde),
+
+    /// An item in a list
+    List(Vec<ListenItemSerde>),
+}
+
+/// One item in the list of a list-ish `Listen`, or the plain value
+#[derive(Serialize, Deserialize)]
+#[serde(untagged)]
+enum ListenItemSerde {
+    /// An integer.
+    ///
+    /// When appearing "loose" (in ListenSerde::One), `0` is parsed as none.
+    Port(u16),
+
+    /// An string which will be parsed as an address and port
+    ///
+    /// When appearing "loose" (in ListenSerde::One), `""` is parsed as none.
+    String(String),
+}
+
+// This implementation isn't fallible, but clippy thinks it is because of the unwrap.
+// The unwrap is just there because we can't pattern-match on a Vec
+#[allow(clippy::fallible_impl_from)]
+impl From<Listen> for ListenSerde {
+    fn from(l: Listen) -> ListenSerde {
+        let l = l.0;
+        match l.len() {
+            0 => ListenSerde::Bool(false),
+            1 => ListenSerde::One(l.into_iter().next().expect("len=1 but no next").into()),
+            _ => ListenSerde::List(l.into_iter().map(Into::into).collect()),
+        }
+    }
+}
+impl From<ListenItem> for ListenItemSerde {
+    fn from(i: ListenItem) -> ListenItemSerde {
+        use ListenItem as LI;
+        use ListenItemSerde as LIS;
+        match i {
+            LI::Localhost(port) => LIS::Port(port.into()),
+            LI::General(addr) => LIS::String(addr.to_string()),
+        }
+    }
+}
+
+/// Listen configuration is invalid
+#[derive(thiserror::Error, Debug, Clone)]
+#[non_exhaustive]
+pub enum InvalidListen {
+    /// Bool was `true` but that's not an address.
+    #[error("Invalid listen specification: need actual addr/port, or `false`; not `true`")]
+    InvalidBool,
+
+    /// Specified listen was a string but couldn't parse to a [`net::SocketAddr`].
+    #[error("Invalid listen specification: failed to parse string: {0}")]
+    InvalidString(#[from] net::AddrParseError),
+
+    /// Specified listen was a list containing a zero integer
+    #[error("Invalid listen specification: zero (for no port) not permitted in list")]
+    ZeroPortInList,
+}
+impl TryFrom<ListenSerde> for Listen {
+    type Error = InvalidListen;
+
+    fn try_from(l: ListenSerde) -> Result<Listen, Self::Error> {
+        use ListenSerde as LS;
+        Ok(Listen(match l {
+            LS::Bool(false) => vec![],
+            LS::Bool(true) => return Err(InvalidListen::InvalidBool),
+            LS::One(i) if i.means_none() => vec![],
+            LS::One(i) => vec![i.try_into()?],
+            LS::List(l) => l.into_iter().map(|i| i.try_into()).try_collect()?,
+        }))
+    }
+}
+impl ListenItemSerde {
+    /// Is this item actually a sentinel, meaning "don't listen, disable this thing"?
+    ///
+    /// Allowed only bare, not in a list.
+    fn means_none(&self) -> bool {
+        use ListenItemSerde as LIS;
+        match self {
+            &LIS::Port(port) => port == 0,
+            LIS::String(s) => s.is_empty(),
+        }
+    }
+}
+impl TryFrom<ListenItemSerde> for ListenItem {
+    type Error = InvalidListen;
+
+    fn try_from(i: ListenItemSerde) -> Result<ListenItem, Self::Error> {
+        use ListenItem as LI;
+        use ListenItemSerde as LIS;
+        Ok(match i {
+            LIS::String(s) => LI::General(s.parse()?),
+            LIS::Port(p) => LI::Localhost(p.try_into().map_err(|_| InvalidListen::ZeroPortInList)?),
+        })
+    }
+}
+
 #[cfg(test)]
 mod test {
     // @@ begin test lint list maintained by maint/add_warning @@
@@ -90,6 +304,9 @@ mod test {
     struct TestConfigFile {
         #[serde(default)]
         padding: PaddingLevel,
+
+        #[serde(default)]
+        listen: Option<Listen>,
     }
 
     #[test]
@@ -117,4 +334,97 @@ mod test {
         chk_e(r#"padding = "unknown""#);
         chk_e(r#"padding = "Normal""#);
     }
+
+    #[test]
+    fn listen_parse() {
+        use net::{Ipv4Addr, Ipv6Addr, SocketAddr};
+        use ListenItem as LI;
+
+        let localhost6 = |p| SocketAddr::new(Ipv6Addr::LOCALHOST.into(), p);
+        let localhost4 = |p| SocketAddr::new(Ipv4Addr::LOCALHOST.into(), p);
+        let unspec6 = |p| SocketAddr::new(Ipv6Addr::UNSPECIFIED.into(), p);
+
+        #[allow(clippy::needless_pass_by_value)] // we do this for consistency
+        fn chk(
+            exp_i: Vec<ListenItem>,
+            exp_addrs: Result<Vec<SocketAddr>, ()>,
+            exp_lpd: Result<Option<u16>, ()>,
+            s: &str,
+        ) {
+            let tc: TestConfigFile = toml::from_str(s).expect(s);
+            let ll = tc.listen.unwrap();
+            eprintln!("s={:?} ll={:?}", &s, &ll);
+            assert_eq!(ll, Listen(exp_i));
+            assert_eq!(
+                ll.ip_addrs().map(|a| a.collect_vec()).map_err(|_| ()),
+                exp_addrs
+            );
+            assert_eq!(ll.localhost_port_deprecated().map_err(|_| ()), exp_lpd);
+        }
+
+        let chk_err = |exp, s: &str| {
+            let got: Result<TestConfigFile, _> = toml::from_str(s);
+            let got = got.expect_err(s).to_string();
+            assert!(got.contains(exp), "s={:?} got={:?} exp={:?}", s, got, exp);
+        };
+
+        let chk_none = |s: &str| {
+            chk(vec![], Ok(vec![]), Ok(None), &format!("listen = {}", s));
+            chk_err(
+                "", /* any error will do */
+                &format!("listen = [ {} ]", s),
+            );
+        };
+
+        let chk_1 = |v: ListenItem, addrs: Vec<SocketAddr>, port, s| {
+            chk(
+                vec![v.clone()],
+                Ok(addrs.clone()),
+                port,
+                &format!("listen = {}", s),
+            );
+            chk(
+                vec![v.clone()],
+                Ok(addrs.clone()),
+                port,
+                &format!("listen = [ {} ]", s),
+            );
+            chk(
+                vec![v, LI::Localhost(23.try_into().unwrap())],
+                Ok([addrs, vec![localhost6(23), localhost4(23)]]
+                    .into_iter()
+                    .flatten()
+                    .collect()),
+                Err(()),
+                &format!("listen = [ {}, 23 ]", s),
+            );
+        };
+
+        chk_none(r#""""#);
+        chk_none(r#"0"#);
+        chk_none(r#"false"#);
+        chk(vec![], Ok(vec![]), Ok(None), r#"listen = []"#);
+
+        chk_1(
+            LI::Localhost(42.try_into().unwrap()),
+            vec![localhost6(42), localhost4(42)],
+            Ok(Some(42)),
+            "42",
+        );
+        chk_1(
+            LI::General(unspec6(56)),
+            vec![unspec6(56)],
+            Err(()),
+            r#""[::]:56""#,
+        );
+
+        let chk_err_1 = |e, el, s| {
+            chk_err(e, &format!("listen = {}", s));
+            chk_err(el, &format!("listen = [ {} ]", s));
+            chk_err(el, &format!("listen = [ 23, {}, 77 ]", s));
+        };
+
+        chk_err_1("need actual addr/port", "did not match any variant", "true");
+        chk_err("did not match any variant", r#"listen = [ [] ]"#);
+    }
 }

From 29a24a9dcb3a3464c4ed5de4f24661433d57d077 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Tue, 23 Aug 2022 18:35:35 +0100
Subject: [PATCH 03/12] tor-config: Rename UnrecognizedKey to DisfavouredKey

We're going to want the to use the same type for deprecated keys.
---
 crates/tor-config/src/load.rs | 36 +++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/crates/tor-config/src/load.rs b/crates/tor-config/src/load.rs
index 93eaf3d4c..7aa780ac3 100644
--- a/crates/tor-config/src/load.rs
+++ b/crates/tor-config/src/load.rs
@@ -233,7 +233,7 @@ enum UnrecognizedKeys {
     /// The keys which remain unrecognized by any consumer
     ///
     /// If this is empty, we do not (need to) do any further tracking.
-    These(BTreeSet<UnrecognizedKey>),
+    These(BTreeSet<DisfavouredKey>),
 }
 use UnrecognizedKeys as UK;
 
@@ -247,7 +247,7 @@ impl UnrecognizedKeys {
     }
 
     /// Update in place, intersecting with `other`
-    fn intersect_with(&mut self, other: BTreeSet<UnrecognizedKey>) {
+    fn intersect_with(&mut self, other: BTreeSet<DisfavouredKey>) {
         match self {
             UK::AllKeys => *self = UK::These(other),
             UK::These(self_) => {
@@ -262,12 +262,12 @@ impl UnrecognizedKeys {
 ///
 /// `Display`s in an approximation to TOML format.
 #[derive(Debug, Clone, Hash, Eq, PartialEq, Ord, PartialOrd)]
-pub struct UnrecognizedKey {
+pub struct DisfavouredKey {
     /// Can be empty only before returned from this module
     path: Vec<PathEntry>,
 }
 
-/// Element of an UnrecognizedKey
+/// Element of an DisfavouredKey
 #[derive(Debug, Clone, Hash, Eq, PartialEq, Ord, PartialOrd)]
 enum PathEntry {
     /// Array index
@@ -285,7 +285,7 @@ enum PathEntry {
 fn resolve_inner<T>(
     input: config::Config,
     want_unrecognized: bool,
-) -> Result<(T, Vec<UnrecognizedKey>), ConfigResolveError>
+) -> Result<(T, Vec<DisfavouredKey>), ConfigResolveError>
 where
     T: Resolvable,
 {
@@ -337,7 +337,7 @@ where
 /// Deserialize and build overall configuration, reporting unrecognized keys in the return value
 pub fn resolve_return_unrecognized<T>(
     input: config::Config,
-) -> Result<(T, Vec<UnrecognizedKey>), ConfigResolveError>
+) -> Result<(T, Vec<DisfavouredKey>), ConfigResolveError>
 where
     T: Resolvable,
 {
@@ -388,8 +388,8 @@ where
     }
 }
 
-/// Turns a [`serde_ignored::Path`] (which is borrowed) into an owned `UnrecognizedKey`
-fn copy_path(mut path: &serde_ignored::Path) -> UnrecognizedKey {
+/// Turns a [`serde_ignored::Path`] (which is borrowed) into an owned `DisfavouredKey`
+fn copy_path(mut path: &serde_ignored::Path) -> DisfavouredKey {
     use serde_ignored::Path as SiP;
     use PathEntry as PE;
 
@@ -407,7 +407,7 @@ fn copy_path(mut path: &serde_ignored::Path) -> UnrecognizedKey {
         path = new_path;
     }
     descend.reverse();
-    UnrecognizedKey { path: descend }
+    DisfavouredKey { path: descend }
 }
 
 /// Computes the intersection, resolving ignorances at different depths
@@ -428,9 +428,9 @@ fn copy_path(mut path: &serde_ignored::Path) -> UnrecognizedKey {
 /// If the inputs are not minimal, the output may not be either
 /// (although `serde_ignored` gives us minimal sets, so that case is not important).
 fn intersect_unrecognized_lists(
-    al: BTreeSet<UnrecognizedKey>,
-    bl: BTreeSet<UnrecognizedKey>,
-) -> BTreeSet<UnrecognizedKey> {
+    al: BTreeSet<DisfavouredKey>,
+    bl: BTreeSet<DisfavouredKey>,
+) -> BTreeSet<DisfavouredKey> {
     //eprintln!("INTERSECT:");
     //for ai in &al { eprintln!("A: {}", ai); }
     //for bi in &bl { eprintln!("B: {}", bi); }
@@ -535,7 +535,7 @@ fn intersect_unrecognized_lists(
     output
 }
 
-impl Display for UnrecognizedKey {
+impl Display for DisfavouredKey {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         use PathEntry as PE;
         if self.path.is_empty() {
@@ -581,9 +581,9 @@ mod test {
     use derive_builder::Builder;
     use serde::{Deserialize, Serialize};
 
-    fn parse_test_set(l: &[&str]) -> BTreeSet<UnrecognizedKey> {
+    fn parse_test_set(l: &[&str]) -> BTreeSet<DisfavouredKey> {
         l.iter()
-            .map(|s| UnrecognizedKey {
+            .map(|s| DisfavouredKey {
                 path: s
                     .split('.')
                     .map(|s| PathEntry::MapEntry(s.into()))
@@ -645,7 +645,7 @@ mod test {
     #[test]
     fn test_display_key() {
         let chk = |exp, path: &[PathEntry]| {
-            assert_eq! { UnrecognizedKey { path: path.into() }.to_string(), exp };
+            assert_eq! { DisfavouredKey { path: path.into() }.to_string(), exp };
         };
         let me = |s: &str| PathEntry::MapEntry(s.into());
         use PathEntry::ArrayIndex as AI;
@@ -742,14 +742,14 @@ mod test {
             .unwrap();
         {
             // First try "A", then "C".
-            let res1: Result<((TestConfigA, TestConfigC), Vec<UnrecognizedKey>), _> =
+            let res1: Result<((TestConfigA, TestConfigC), Vec<DisfavouredKey>), _> =
                 resolve_return_unrecognized(cfg.clone());
             assert!(res1.is_err());
             assert!(matches!(res1, Err(ConfigResolveError::Deserialize(_))));
         }
         {
             // Now the other order: first try "C", then "A".
-            let res2: Result<((TestConfigC, TestConfigA), Vec<UnrecognizedKey>), _> =
+            let res2: Result<((TestConfigC, TestConfigA), Vec<DisfavouredKey>), _> =
                 resolve_return_unrecognized(cfg.clone());
             assert!(res2.is_err());
             assert!(matches!(res2, Err(ConfigResolveError::Deserialize(_))));

From 33358379f4bbbda3f4fd20bd5a413f3003e959cf Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Tue, 23 Aug 2022 18:51:23 +0100
Subject: [PATCH 04/12] tor-config: Introduce ResolutionResults

This will allow us to handle new kinds of warnigns etc.
---
 crates/arti/src/cfg.rs        | 13 ++++----
 crates/tor-config/semver.md   |  4 +++
 crates/tor-config/src/lib.rs  |  2 +-
 crates/tor-config/src/load.rs | 60 ++++++++++++++++++++++++-----------
 crates/tor-config/src/misc.rs |  8 ++++-
 5 files changed, 60 insertions(+), 27 deletions(-)

diff --git a/crates/arti/src/cfg.rs b/crates/arti/src/cfg.rs
index 4e7701ffe..a5f6b1cc9 100644
--- a/crates/arti/src/cfg.rs
+++ b/crates/arti/src/cfg.rs
@@ -178,6 +178,7 @@ mod test {
     use arti_client::config::TorClientConfigBuilder;
     use regex::Regex;
     use std::time::Duration;
+    use tor_config::load::ResolutionResults;
 
     use super::*;
 
@@ -208,14 +209,14 @@ mod test {
             // Also we should ideally test that every setting from the config appears here in
             // the file.  Possibly that could be done with some kind of stunt Deserializer,
             // but it's not trivial.
-            let (parsed, unrecognized): (ArtiCombinedConfig, _) =
-                tor_config::resolve_return_unrecognized(cfg).unwrap();
+            let results: ResolutionResults<ArtiCombinedConfig> =
+                tor_config::resolve_return_results(cfg).unwrap();
 
-            assert_eq!(&parsed, &default);
-            assert_eq!(&parsed, &empty_config);
+            assert_eq!(&results.value, &default);
+            assert_eq!(&results.value, &empty_config);
 
-            assert_eq!(unrecognized, &[]);
-            parsed
+            assert_eq!(results.unrecognized, &[]);
+            results.value
         };
 
         let _ = parses_to_defaults(ARTI_EXAMPLE_CONFIG);
diff --git a/crates/tor-config/semver.md b/crates/tor-config/semver.md
index 6d0fb2d24..a4c55f10a 100644
--- a/crates/tor-config/semver.md
+++ b/crates/tor-config/semver.md
@@ -4,3 +4,7 @@ ADDED: resolve_option_general
 ADDED: sources::FoundConfigFiles
 BREAKING: ConfigurationSources takes ConfigurationSource for files, not Paths
 BREAKING: ConfigurationSources::from_cmdline wants an iterator of defaults
+BREAKING: load::resolve_ignore_unrecognized renamed resolve_ignore_warnings
+BREAKING: load::resolve_return_unrecognized replaced with resolve_return_results
+BREAKING: load::UnrecognizedKey renamed to DisfavouredKey
+
diff --git a/crates/tor-config/src/lib.rs b/crates/tor-config/src/lib.rs
index ceacc9900..4f5303f7b 100644
--- a/crates/tor-config/src/lib.rs
+++ b/crates/tor-config/src/lib.rs
@@ -106,7 +106,7 @@ pub use cmdline::CmdLine;
 pub use config as config_crate;
 pub use educe;
 pub use err::{ConfigBuildError, ReconfigureError};
-pub use load::{resolve, resolve_ignore_unrecognized, resolve_return_unrecognized};
+pub use load::{resolve, resolve_ignore_warnings, resolve_return_results};
 pub use misc::*;
 pub use mut_cfg::MutCfg;
 pub use paste::paste;
diff --git a/crates/tor-config/src/load.rs b/crates/tor-config/src/load.rs
index 7aa780ac3..740124e4a 100644
--- a/crates/tor-config/src/load.rs
+++ b/crates/tor-config/src/load.rs
@@ -285,7 +285,7 @@ enum PathEntry {
 fn resolve_inner<T>(
     input: config::Config,
     want_unrecognized: bool,
-) -> Result<(T, Vec<DisfavouredKey>), ConfigResolveError>
+) -> Result<ResolutionResults<T>, ConfigResolveError>
 where
     T: Resolvable,
 {
@@ -298,9 +298,9 @@ where
         },
     };
 
-    let val = Resolvable::resolve(&mut lc)?;
+    let value = Resolvable::resolve(&mut lc)?;
 
-    let ign = match lc.unrecognized {
+    let unrecognized = match lc.unrecognized {
         UK::AllKeys => panic!("all unrecognized, as if we had processed nothing"),
         UK::These(ign) => ign,
     }
@@ -308,7 +308,10 @@ where
     .filter(|ip| !ip.path.is_empty())
     .collect_vec();
 
-    Ok((val, ign))
+    Ok(ResolutionResults {
+        value,
+        unrecognized,
+    })
 }
 
 /// Deserialize and build overall configuration from config sources
@@ -327,29 +330,43 @@ pub fn resolve<T>(input: config::Config) -> Result<T, ConfigResolveError>
 where
     T: Resolvable,
 {
-    let (val, ign) = resolve_inner(input, true)?;
-    for ign in ign {
+    let ResolutionResults {
+        value,
+        unrecognized,
+    } = resolve_inner(input, true)?;
+    for ign in unrecognized {
         warn!("unrecognized configuration key: {}", &ign);
     }
-    Ok(val)
+    Ok(value)
 }
 
 /// Deserialize and build overall configuration, reporting unrecognized keys in the return value
-pub fn resolve_return_unrecognized<T>(
+pub fn resolve_return_results<T>(
     input: config::Config,
-) -> Result<(T, Vec<DisfavouredKey>), ConfigResolveError>
+) -> Result<ResolutionResults<T>, ConfigResolveError>
 where
     T: Resolvable,
 {
     resolve_inner(input, true)
 }
 
+/// Results of a successful `resolve_return_disfavoured`
+#[derive(Debug, Clone)]
+#[non_exhaustive]
+pub struct ResolutionResults<T> {
+    /// The configuration, successfully parsed
+    pub value: T,
+
+    /// Any config keys which were found in the input, but not recognized (and so, ignored)
+    pub unrecognized: Vec<DisfavouredKey>,
+}
+
 /// Deserialize and build overall configuration, silently ignoring unrecognized config keys
-pub fn resolve_ignore_unrecognized<T>(input: config::Config) -> Result<T, ConfigResolveError>
+pub fn resolve_ignore_warnings<T>(input: config::Config) -> Result<T, ConfigResolveError>
 where
     T: Resolvable,
 {
-    Ok(resolve_inner(input, false)?.0)
+    Ok(resolve_inner(input, false)?.value)
 }
 
 impl<T> Resolvable for T
@@ -696,12 +713,17 @@ mod test {
             .build()
             .unwrap();
 
-        let _: (TestConfigA, TestConfigB) = resolve_ignore_unrecognized(cfg.clone()).unwrap();
+        let _: (TestConfigA, TestConfigB) = resolve_ignore_warnings(cfg.clone()).unwrap();
 
-        let ((a, b), ign): ((TestConfigA, TestConfigB), _) =
-            resolve_return_unrecognized(cfg).unwrap();
+        let resolved: ResolutionResults<(TestConfigA, TestConfigB)> =
+            resolve_return_results(cfg).unwrap();
+        let (a, b) = resolved.value;
 
-        let ign = ign.into_iter().map(|ik| ik.to_string()).collect_vec();
+        let ign = resolved
+            .unrecognized
+            .into_iter()
+            .map(|ik| ik.to_string())
+            .collect_vec();
 
         assert_eq! { &a, &TestConfigA { a: "hi".into() } };
         assert_eq! { &b, &TestConfigB { b: "".into() } };
@@ -742,15 +764,15 @@ mod test {
             .unwrap();
         {
             // First try "A", then "C".
-            let res1: Result<((TestConfigA, TestConfigC), Vec<DisfavouredKey>), _> =
-                resolve_return_unrecognized(cfg.clone());
+            let res1: Result<ResolutionResults<(TestConfigA, TestConfigC)>, _> =
+                resolve_return_results(cfg.clone());
             assert!(res1.is_err());
             assert!(matches!(res1, Err(ConfigResolveError::Deserialize(_))));
         }
         {
             // Now the other order: first try "C", then "A".
-            let res2: Result<((TestConfigC, TestConfigA), Vec<DisfavouredKey>), _> =
-                resolve_return_unrecognized(cfg.clone());
+            let res2: Result<ResolutionResults<(TestConfigC, TestConfigA)>, _> =
+                resolve_return_results(cfg.clone());
             assert!(res2.is_err());
             assert!(matches!(res2, Err(ConfigResolveError::Deserialize(_))));
         }
diff --git a/crates/tor-config/src/misc.rs b/crates/tor-config/src/misc.rs
index 703d32ee5..72ea44b22 100644
--- a/crates/tor-config/src/misc.rs
+++ b/crates/tor-config/src/misc.rs
@@ -104,7 +104,13 @@ impl Listen {
     ///
     /// Special case: if `port` is zero, specifies no listening.
     pub fn new_localhost(port: u16) -> Listen {
-        Listen(port.try_into().ok().map(ListenItem::Localhost).into_iter().collect_vec())
+        Listen(
+            port.try_into()
+                .ok()
+                .map(ListenItem::Localhost)
+                .into_iter()
+                .collect_vec(),
+        )
     }
 
     /// Create a new `Listen`, possibly specifying listening on a port on localhost

From 3af1f3e7126ded6688cfbf2603a8f41ea47391a6 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Tue, 23 Aug 2022 19:00:08 +0100
Subject: [PATCH 05/12] tor-config: Support tracking deprecated config keys

---
 crates/arti/src/cfg.rs        | 16 +++++---
 crates/tor-config/semver.md   |  2 +-
 crates/tor-config/src/load.rs | 77 +++++++++++++++++++++++++++++++----
 3 files changed, 80 insertions(+), 15 deletions(-)

diff --git a/crates/arti/src/cfg.rs b/crates/arti/src/cfg.rs
index a5f6b1cc9..3fc3d2d91 100644
--- a/crates/arti/src/cfg.rs
+++ b/crates/arti/src/cfg.rs
@@ -176,6 +176,7 @@ mod test {
 
     use arti_client::config::dir;
     use arti_client::config::TorClientConfigBuilder;
+    use itertools::Itertools;
     use regex::Regex;
     use std::time::Duration;
     use tor_config::load::ResolutionResults;
@@ -328,7 +329,6 @@ mod test {
 
     #[allow(clippy::dbg_macro)]
     fn exhaustive_1(example_file: &str, expect_missing: &[&str]) {
-        use itertools::Itertools;
         use serde_json::Value as JsValue;
         use std::collections::BTreeSet;
 
@@ -447,11 +447,17 @@ mod test {
 
     #[test]
     fn exhaustive() {
-        exhaustive_1(
-            ARTI_EXAMPLE_CONFIG,
-            // add *old*, obsoleted settings here
-            &[],
+        let mut deprecated = vec![];
+        <(ArtiConfig, TorClientConfig) as tor_config::load::Resolvable>::enumerate_deprecated_keys(
+            &mut |l| {
+                for k in l {
+                    deprecated.push(k.to_string());
+                }
+            },
         );
+        let deprecated = deprecated.iter().map(|s| &**s).collect_vec();
+
+        exhaustive_1(ARTI_EXAMPLE_CONFIG, &deprecated);
 
         exhaustive_1(
             OLDEST_SUPPORTED_CONFIG,
diff --git a/crates/tor-config/semver.md b/crates/tor-config/semver.md
index a4c55f10a..d30c39e13 100644
--- a/crates/tor-config/semver.md
+++ b/crates/tor-config/semver.md
@@ -7,4 +7,4 @@ BREAKING: ConfigurationSources::from_cmdline wants an iterator of defaults
 BREAKING: load::resolve_ignore_unrecognized renamed resolve_ignore_warnings
 BREAKING: load::resolve_return_unrecognized replaced with resolve_return_results
 BREAKING: load::UnrecognizedKey renamed to DisfavouredKey
-
+ADDED: Support for tracking deprecated config keys
diff --git a/crates/tor-config/src/load.rs b/crates/tor-config/src/load.rs
index 740124e4a..27b251ac4 100644
--- a/crates/tor-config/src/load.rs
+++ b/crates/tor-config/src/load.rs
@@ -157,6 +157,11 @@ pub trait Resolvable: Sized {
     // because that would somehow involve creating `Self` from `ResolveContext`
     // but `ResolveContext` is completely opaque outside this module.
     fn resolve(input: &mut ResolveContext) -> Result<Self, ConfigResolveError>;
+
+    /// Return a list of deprecated config keys, as "."-separated strings
+    fn enumerate_deprecated_keys<F>(f: &mut F)
+    where
+        F: FnMut(&'static [&'static str]);
 }
 
 /// Top-level configuration struct, made from a deserializable builder
@@ -175,6 +180,9 @@ pub trait TopLevel {
     ///
     /// Should satisfy `&'_ Self::Builder: Builder<Built=Self>`
     type Builder: DeserializeOwned;
+
+    /// Deprecated config keys, as "."-separates strings
+    const DEPRECATED_KEYS: &'static [&'static str] = &[];
 }
 
 /// `impl Resolvable for (A,B..) where A: Resolvable, B: Resolvable ...`
@@ -198,6 +206,10 @@ macro_rules! define_for_tuples {
             fn resolve(cfg: &mut ResolveContext) -> Result<Self, ConfigResolveError> {
                 Ok(( $( $A::resolve(cfg)?, )* ))
             }
+            fn enumerate_deprecated_keys<NF>(f: &mut NF)
+            where NF: FnMut(&'static [&'static str]) {
+                $( $A::enumerate_deprecated_keys(f); )*
+            }
         }
 
     };
@@ -258,7 +270,7 @@ impl UnrecognizedKeys {
     }
 }
 
-/// Key in config file(s) unrecognized by all Resolvables we obtained
+/// Key in config file(s) which is disfavoured (unrecognized or deprecated)
 ///
 /// `Display`s in an approximation to TOML format.
 #[derive(Debug, Clone, Hash, Eq, PartialEq, Ord, PartialOrd)]
@@ -284,14 +296,29 @@ enum PathEntry {
 /// Inner function used by all the `resolve_*` family
 fn resolve_inner<T>(
     input: config::Config,
-    want_unrecognized: bool,
+    want_disfavoured: bool,
 ) -> Result<ResolutionResults<T>, ConfigResolveError>
 where
     T: Resolvable,
 {
+    let mut deprecated = BTreeSet::new();
+
+    if want_disfavoured {
+        T::enumerate_deprecated_keys(&mut |l: &[&str]| {
+            for key in l {
+                match input.get(key) {
+                    Err(_) => {}
+                    Ok(serde::de::IgnoredAny) => {
+                        deprecated.insert(key);
+                    }
+                }
+            }
+        });
+    }
+
     let mut lc = ResolveContext {
         input,
-        unrecognized: if want_unrecognized {
+        unrecognized: if want_disfavoured {
             UK::AllKeys
         } else {
             UK::These(BTreeSet::new())
@@ -308,9 +335,21 @@ where
     .filter(|ip| !ip.path.is_empty())
     .collect_vec();
 
+    let deprecated = deprecated
+        .into_iter()
+        .map(|key| {
+            let path = key
+                .split('.')
+                .map(|e| PathEntry::MapEntry(e.into()))
+                .collect_vec();
+            DisfavouredKey { path }
+        })
+        .collect_vec();
+
     Ok(ResolutionResults {
         value,
         unrecognized,
+        deprecated,
     })
 }
 
@@ -333,7 +372,11 @@ where
     let ResolutionResults {
         value,
         unrecognized,
+        deprecated,
     } = resolve_inner(input, true)?;
+    for depr in deprecated {
+        warn!("deprecated configuration key: {}", &depr);
+    }
     for ign in unrecognized {
         warn!("unrecognized configuration key: {}", &ign);
     }
@@ -359,6 +402,9 @@ pub struct ResolutionResults<T> {
 
     /// Any config keys which were found in the input, but not recognized (and so, ignored)
     pub unrecognized: Vec<DisfavouredKey>,
+
+    /// Any config keys which were found, but have been declared deprecated
+    pub deprecated: Vec<DisfavouredKey>,
 }
 
 /// Deserialize and build overall configuration, silently ignoring unrecognized config keys
@@ -403,6 +449,13 @@ where
         let built = builder.build()?;
         Ok(built)
     }
+
+    fn enumerate_deprecated_keys<NF>(f: &mut NF)
+    where
+        NF: FnMut(&'static [&'static str]),
+    {
+        f(T::DEPRECATED_KEYS);
+    }
 }
 
 /// Turns a [`serde_ignored::Path`] (which is borrowed) into an owned `DisfavouredKey`
@@ -694,10 +747,14 @@ mod test {
     struct TestConfigB {
         #[builder(default)]
         b: String,
+
+        #[builder(default)]
+        old: bool,
     }
     impl_standard_builder! { TestConfigB }
     impl TopLevel for TestConfigB {
         type Builder = TestConfigBBuilder;
+        const DEPRECATED_KEYS: &'static [&'static str] = &["old"];
     }
 
     #[test]
@@ -705,6 +762,7 @@ mod test {
         let test_data = r#"
             wombat = 42
             a = "hi"
+            old = true
         "#;
         let source = config::File::from_str(test_data, config::FileFormat::Toml);
 
@@ -719,15 +777,16 @@ mod test {
             resolve_return_results(cfg).unwrap();
         let (a, b) = resolved.value;
 
-        let ign = resolved
-            .unrecognized
-            .into_iter()
-            .map(|ik| ik.to_string())
-            .collect_vec();
+        let mk_strings =
+            |l: Vec<DisfavouredKey>| l.into_iter().map(|ik| ik.to_string()).collect_vec();
+
+        let ign = mk_strings(resolved.unrecognized);
+        let depr = mk_strings(resolved.deprecated);
 
         assert_eq! { &a, &TestConfigA { a: "hi".into() } };
-        assert_eq! { &b, &TestConfigB { b: "".into() } };
+        assert_eq! { &b, &TestConfigB { b: "".into(), old: true } };
         assert_eq! { ign, &["wombat"] };
+        assert_eq! { depr, &["old"] };
 
         let _ = TestConfigA::builder();
         let _ = TestConfigB::builder();

From 846fe3d5207a86e3c73003288b0f1bdbb442e9fb Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Wed, 24 Aug 2022 12:08:15 +0100
Subject: [PATCH 06/12] tor-config: Provide resolve_alternative_specs

---
 crates/tor-config/src/lib.rs | 78 ++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/crates/tor-config/src/lib.rs b/crates/tor-config/src/lib.rs
index 4f5303f7b..fe4adbc09 100644
--- a/crates/tor-config/src/lib.rs
+++ b/crates/tor-config/src/lib.rs
@@ -106,6 +106,7 @@ pub use cmdline::CmdLine;
 pub use config as config_crate;
 pub use educe;
 pub use err::{ConfigBuildError, ReconfigureError};
+pub use itertools::Itertools;
 pub use load::{resolve, resolve_ignore_warnings, resolve_return_results};
 pub use misc::*;
 pub use mut_cfg::MutCfg;
@@ -231,6 +232,83 @@ where
     }
 }
 
+/// Helper for resolving a config item which can be specified in multiple ways
+///
+/// Useable when a single configuration item can be specified
+/// via multiple (alternative) input fields;
+/// Each input field which is actually present
+/// should be converted to the common output type,
+/// and then passed to this function,
+/// which will handle consistency checks and defaulting.
+///
+/// A common use case is deprecated field name/types.
+/// In that case, the deprecated field names should be added to the appropriate
+/// [`load::TopLevel::DEPRECATED_KEYS`].
+///
+/// `specified` should be an array (or other iterator) of `(key, Option<value>)`
+/// where `key` is the field name and
+/// `value` is that field from the builder,
+/// converted to the common output type `V`.
+///
+/// # Example
+///
+/// ```
+/// use derive_builder::Builder;
+/// use serde::{Deserialize, Serialize};
+/// use tor_config::{impl_standard_builder, ConfigBuildError, Listen, resolve_alternative_specs};
+///
+/// #[derive(Debug, Clone, Builder, Eq, PartialEq)]
+/// #[builder(build_fn(error = "ConfigBuildError"))]
+/// #[builder(derive(Debug, Serialize, Deserialize))]
+/// #[allow(clippy::option_option)]
+/// pub struct ProxyConfig {
+///    /// Addresses to listen on for incoming SOCKS connections.
+///    #[builder(field(build = r#"self.resolve_socks_port()?"#))]
+///    pub(crate) socks_listen: Listen,
+///
+///    /// Port to listen on (at localhost) for incoming SOCKS
+///    /// connections.
+///    #[builder(setter(strip_option), field(type = "Option<Option<u16>>", build = "()"))]
+///    pub(crate) socks_port: (),
+/// }
+/// impl_standard_builder! { ProxyConfig }
+///
+/// impl ProxyConfigBuilder {
+///     fn resolve_socks_port(&self) -> Result<Listen, ConfigBuildError> {
+///         resolve_alternative_specs(
+///             [
+///                 ("socks_listen", self.socks_listen.clone()),
+///                 ("socks_port", self.socks_port.map(Listen::new_localhost_optional)),
+///             ],
+///             || Listen::new_localhost(9150),
+///         )
+///     }
+/// }
+/// ```
+//
+// Testing: this is tested quit exhaustively in the context of the listen/port handling, in
+// crates/arti/src/cfg.rs.
+pub fn resolve_alternative_specs<V, K>(
+    specified: impl IntoIterator<Item = (K, Option<V>)>,
+    default: impl FnOnce() -> V,
+) -> Result<V, ConfigBuildError>
+where
+    K: Into<String>,
+    V: Eq,
+{
+    Ok(specified
+        .into_iter()
+        .filter_map(|(k, v)| Some((k, v?)))
+        .dedup_by(|(_, v1), (_, v2)| v1 == v2)
+        .at_most_one()
+        .map_err(|several| ConfigBuildError::Inconsistent {
+            fields: several.into_iter().map(|(k, _v)| k.into()).collect_vec(),
+            problem: "conflicting fields, specifying different values".into(),
+        })?
+        .map(|(_k, v)| v)
+        .unwrap_or_else(default))
+}
+
 /// Defines standard impls for a struct with a `Builder`, incl `Default`
 ///
 /// **Use this.**  Do not `#[derive(Builder, Default)]`.  That latter approach would produce

From 12476bf0d40ad24ccc8a6b9a41f183c2720caa90 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Wed, 24 Aug 2022 12:46:19 +0100
Subject: [PATCH 07/12] arti: cfg: Rename `*_port` to `*_listen` and change the
 type

This commit largely follows the example for resolve_alternative_specs.

The difference is that there are two fields, so we use a macro to
avoid recapitulating the field names.
---
 Cargo.lock                               |  1 +
 crates/arti/Cargo.toml                   |  2 +
 crates/arti/src/arti-example-config.toml |  4 +-
 crates/arti/src/cfg.rs                   | 78 +++++++++++++++++++++---
 crates/arti/src/lib.rs                   |  7 ++-
 5 files changed, 79 insertions(+), 13 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index ab8ad119c..1eea1a407 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -96,6 +96,7 @@ dependencies = [
  "libc",
  "notify",
  "once_cell",
+ "paste",
  "regex",
  "rlimit",
  "safelog",
diff --git a/crates/arti/Cargo.toml b/crates/arti/Cargo.toml
index 9386842f1..69f709c9a 100644
--- a/crates/arti/Cargo.toml
+++ b/crates/arti/Cargo.toml
@@ -46,9 +46,11 @@ derive_builder = { version = "0.11", package = "derive_builder_fork_arti" }
 educe = "0.4.6"
 fs-mistrust = { path = "../fs-mistrust", version = "0.4.0" }
 futures = "0.3.14"
+itertools = "0.10.1"
 libc = "0.2"
 notify = "4.0"
 once_cell = { version = "1", optional = true }
+paste = "1"
 rlimit = "0.8.3"
 safelog = { path = "../safelog", version = "0.1.0" }
 secmem-proc = { version = "0.1.1", optional = true }
diff --git a/crates/arti/src/arti-example-config.toml b/crates/arti/src/arti-example-config.toml
index 79befcd6b..64ca1442a 100644
--- a/crates/arti/src/arti-example-config.toml
+++ b/crates/arti/src/arti-example-config.toml
@@ -32,10 +32,10 @@
 # listen on localhost.
 #
 # Note that only one process can listen on a given port at a time.
-#socks_port = 9150
+#socks_listen = 9150
 
 # Port to use to listen for DNS requests.  0 means disabled.
-#dns_port = 0
+#dns_listen = 0
 
 # Configure logging
 [logging]
diff --git a/crates/arti/src/cfg.rs b/crates/arti/src/cfg.rs
index 3fc3d2d91..0a57da730 100644
--- a/crates/arti/src/cfg.rs
+++ b/crates/arti/src/cfg.rs
@@ -2,11 +2,14 @@
 //
 // (Thia module is called `cfg` to avoid name clash with the `config` crate, which we use.)
 
+use paste::paste;
+
 use derive_builder::Builder;
 use serde::{Deserialize, Serialize};
 
 use arti_client::TorClientConfig;
-use tor_config::{impl_standard_builder, ConfigBuildError};
+use tor_config::resolve_alternative_specs;
+pub(crate) use tor_config::{impl_standard_builder, ConfigBuildError, Listen};
 
 use crate::{LoggingConfig, LoggingConfigBuilder};
 
@@ -70,18 +73,70 @@ pub struct ApplicationConfig {
 }
 impl_standard_builder! { ApplicationConfig }
 
+/// Resolves values from `$field_listen` and `$field_port` (compat) into a `Listen`
+///
+/// For `dns` and `proxy`.
+///
+/// Handles defaulting, and normalisation, using `resolve_alternative_specs`
+/// and `Listen::new_localhost_option`.
+///
+/// Broken out into a macro so as to avoid having to state the field name four times,
+/// which is a recipe for programming slips.
+macro_rules! resolve_listen_port {
+    { $self:expr, $field:ident, $def_port:expr } => { paste!{
+        resolve_alternative_specs(
+            [
+                (
+                    concat!(stringify!($field), "_listen"),
+                    $self.[<$field _listen>].clone(),
+                ),
+                (
+                    concat!(stringify!($field), "_port"),
+                    $self.[<$field _port>].map(Listen::new_localhost_optional),
+                ),
+            ],
+            || Listen::new_localhost($def_port),
+        )?
+    } }
+}
+
 /// Configuration for one or more proxy listeners.
 #[derive(Debug, Clone, Builder, Eq, PartialEq)]
 #[builder(build_fn(error = "ConfigBuildError"))]
 #[builder(derive(Debug, Serialize, Deserialize))]
+#[allow(clippy::option_option)] // Builder port fields: Some(None) = specified to disable
 pub struct ProxyConfig {
-    /// Port to listen on (at localhost) for incoming SOCKS
-    /// connections.
-    #[builder(field(build = r#"tor_config::resolve_option(&self.socks_port, || Some(9150))"#))]
-    pub(crate) socks_port: Option<u16>,
+    /// Addresses to listen on for incoming SOCKS connections.
+    #[builder(field(build = r#"resolve_listen_port!(self, socks, 9150)"#))]
+    pub(crate) socks_listen: Listen,
+
+    /// Port to listen on (at localhost) for incoming SOCKS connections.
+    ///
+    /// This field is deprecated, and will, eventually, be removed.
+    /// Use `socks_listen` instead, which accepts the same values,
+    /// but which will also be able to support more flexible listening in the future.
+    #[builder(
+        setter(strip_option),
+        field(type = "Option<Option<u16>>", build = "()")
+    )]
+    #[builder_setter_attr(deprecated)]
+    pub(crate) socks_port: (),
+
+    /// Addresses to listen on for incoming DNS connections.
+    #[builder(field(build = r#"resolve_listen_port!(self, dns, 0)"#))]
+    pub(crate) dns_listen: Listen,
+
     /// Port to lisen on (at localhost) for incoming DNS connections.
-    #[builder(field(build = r#"tor_config::resolve_option(&self.dns_port, || None)"#))]
-    pub(crate) dns_port: Option<u16>,
+    ///
+    /// This field is deprecated, and will, eventually, be removed.
+    /// Use `dns_listen` instead, which accepts the same values,
+    /// but which will also be able to support more flexible listening in the future.
+    #[builder(
+        setter(strip_option),
+        field(type = "Option<Option<u16>>", build = "()")
+    )]
+    #[builder_setter_attr(deprecated)]
+    pub(crate) dns_port: (),
 }
 impl_standard_builder! { ProxyConfig }
 
@@ -146,6 +201,7 @@ impl_standard_builder! { ArtiConfig }
 
 impl tor_config::load::TopLevel for ArtiConfig {
     type Builder = ArtiConfigBuilder;
+    const DEPRECATED_KEYS: &'static [&'static str] = &["proxy.socks_port", "proxy.dns_port"];
 }
 
 /// Convenience alias for the config for a whole `arti` program
@@ -254,7 +310,7 @@ mod test {
         let mut bld = ArtiConfig::builder();
         let mut bld_tor = TorClientConfig::builder();
 
-        bld.proxy().socks_port(Some(9999));
+        bld.proxy().socks_listen(Listen::new_localhost(9999));
         bld.logging().console("warn");
 
         bld_tor.tor_network().set_authorities(vec![auth]);
@@ -462,7 +518,11 @@ mod test {
         exhaustive_1(
             OLDEST_SUPPORTED_CONFIG,
             // add *new*, not present in old file, settings here
-            &["application.allow_running_as_root"],
+            &[
+                "application.allow_running_as_root",
+                "proxy.socks_listen",
+                "proxy.dns_listen",
+            ],
         );
     }
 }
diff --git a/crates/arti/src/lib.rs b/crates/arti/src/lib.rs
index b454c61cf..c5f849143 100644
--- a/crates/arti/src/lib.rs
+++ b/crates/arti/src/lib.rs
@@ -509,14 +509,17 @@ where
     if let Some(proxy_matches) = matches.subcommand_matches("proxy") {
         let socks_port = match (
             proxy_matches.value_of("socks-port"),
-            config.proxy().socks_port,
+            config.proxy().socks_listen.localhost_port_deprecated()?,
         ) {
             (Some(p), _) => p.parse().expect("Invalid port specified"),
             (None, Some(s)) => s,
             (None, None) => 0,
         };
 
-        let dns_port = match (proxy_matches.value_of("dns-port"), config.proxy().dns_port) {
+        let dns_port = match (
+            proxy_matches.value_of("dns-port"),
+            config.proxy().dns_listen.localhost_port_deprecated()?,
+        ) {
             (Some(p), _) => p.parse().expect("Invalid port specified"),
             (None, Some(s)) => s,
             (None, None) => 0,

From 370330cb570dac1b717915c0f1787fef67fc744a Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Wed, 22 Jun 2022 18:59:17 +0100
Subject: [PATCH 08/12] arti cfg: Provide comprehensive tests for port
 listening

---
 crates/arti/src/cfg.rs | 117 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 116 insertions(+), 1 deletion(-)

diff --git a/crates/arti/src/cfg.rs b/crates/arti/src/cfg.rs
index 0a57da730..29825cd8b 100644
--- a/crates/arti/src/cfg.rs
+++ b/crates/arti/src/cfg.rs
@@ -232,8 +232,9 @@ mod test {
 
     use arti_client::config::dir;
     use arti_client::config::TorClientConfigBuilder;
-    use itertools::Itertools;
+    use itertools::{chain, Itertools};
     use regex::Regex;
+    use std::iter;
     use std::time::Duration;
     use tor_config::load::ResolutionResults;
 
@@ -383,6 +384,120 @@ mod test {
         assert_eq!(&config.proxy, proxy);
     }
 
+    /// Comprehensive tests for the various `socks_port` and `dns_port`
+    ///
+    /// The "this isn't set at all, just use the default" cases are tested elsewhere.
+    fn compat_ports_listen(
+        f: &str,
+        get_listen: &dyn Fn(&ArtiConfig) -> &Listen,
+        bld_get_port: &dyn Fn(&ArtiConfigBuilder) -> &Option<Option<u16>>,
+        bld_get_listen: &dyn Fn(&ArtiConfigBuilder) -> &Option<Listen>,
+        setter_port: &dyn Fn(&mut ArtiConfigBuilder, Option<u16>) -> &mut ProxyConfigBuilder,
+        setter_listen: &dyn Fn(&mut ArtiConfigBuilder, Listen) -> &mut ProxyConfigBuilder,
+    ) {
+        let from_toml = |s: &str| -> ArtiConfigBuilder {
+            let cfg: toml::Value = toml::from_str(dbg!(s)).unwrap();
+            let cfg: ArtiConfigBuilder = cfg.try_into().unwrap();
+            cfg
+        };
+
+        let conflicting_cfgs = [
+            format!("proxy.{}_port = 0 \n proxy.{}_listen = 200", f, f),
+            format!("proxy.{}_port = 100 \n proxy.{}_listen = 0", f, f),
+            format!("proxy.{}_port = 100 \n proxy.{}_listen = 200", f, f),
+        ];
+
+        let chk = |cfg: &ArtiConfigBuilder, expected: &Listen| {
+            dbg!(bld_get_listen(cfg), bld_get_port(cfg));
+            let cfg = cfg.build().unwrap();
+            assert_eq!(get_listen(&cfg), expected);
+        };
+
+        let check_setters = |port, expected: &_| {
+            for cfg in chain!(
+                iter::once(ArtiConfig::builder()),
+                conflicting_cfgs.iter().map(|cfg| from_toml(cfg)),
+            ) {
+                for listen in match port {
+                    None => vec![Listen::new_none(), Listen::new_localhost(0)],
+                    Some(port) => vec![Listen::new_localhost(port)],
+                } {
+                    let mut cfg = cfg.clone();
+                    setter_port(&mut cfg, dbg!(port));
+                    setter_listen(&mut cfg, dbg!(listen));
+                    chk(&cfg, expected);
+                }
+            }
+        };
+
+        {
+            let expected = Listen::new_localhost(100);
+
+            let cfg = from_toml(&format!("proxy.{}_port = 100", f));
+            assert_eq!(bld_get_port(&cfg), &Some(Some(100)));
+            chk(&cfg, &expected);
+
+            let cfg = from_toml(&format!("proxy.{}_listen = 100", f));
+            assert_eq!(bld_get_listen(&cfg), &Some(Listen::new_localhost(100)));
+            chk(&cfg, &expected);
+
+            let cfg = from_toml(&format!(
+                "proxy.{}_port = 100\n proxy.{}_listen = 100",
+                f, f
+            ));
+            chk(&cfg, &expected);
+
+            check_setters(Some(100), &expected);
+        }
+
+        {
+            let expected = Listen::new_none();
+
+            let cfg = from_toml(&format!("proxy.{}_port = 0", f));
+            chk(&cfg, &expected);
+
+            let cfg = from_toml(&format!("proxy.{}_listen = 0", f));
+            chk(&cfg, &expected);
+
+            let cfg = from_toml(&format!("proxy.{}_port = 0 \n proxy.{}_listen = 0", f, f));
+            chk(&cfg, &expected);
+
+            check_setters(None, &expected);
+        }
+
+        for cfg in &conflicting_cfgs {
+            let cfg = from_toml(cfg);
+            let err = dbg!(cfg.build()).unwrap_err();
+            assert!(err.to_string().contains("specifying different values"));
+        }
+    }
+
+    #[test]
+    #[allow(deprecated)]
+    fn ports_listen_socks() {
+        compat_ports_listen(
+            "socks",
+            &|cfg| &cfg.proxy.socks_listen,
+            &|bld| &bld.proxy.socks_port,
+            &|bld| &bld.proxy.socks_listen,
+            &|bld, arg| bld.proxy.socks_port(arg),
+            &|bld, arg| bld.proxy.socks_listen(arg),
+        );
+    }
+
+    #[test]
+    #[allow(deprecated)]
+    fn compat_ports_listen_dns() {
+        compat_ports_listen(
+            "dns",
+            &|cfg| &cfg.proxy.dns_listen,
+            &|bld| &bld.proxy.dns_port,
+            &|bld| &bld.proxy.dns_listen,
+            &|bld, arg| bld.proxy.dns_port(arg),
+            &|bld, arg| bld.proxy.dns_listen(arg),
+        );
+    }
+
     #[allow(clippy::dbg_macro)]
     fn exhaustive_1(example_file: &str, expect_missing: &[&str]) {
         use serde_json::Value as JsValue;

From 81bf8d5f4d7c79097aad55148e4fd6580e6daaa1 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Wed, 24 Aug 2022 16:04:53 +0100
Subject: [PATCH 09/12] tor-config; Listen: Return addresses in groups for
 error behaviour

Prompted by
  https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/602#note_2830766
---
 crates/tor-config/src/misc.rs | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/crates/tor-config/src/misc.rs b/crates/tor-config/src/misc.rs
index 72ea44b22..9df797f72 100644
--- a/crates/tor-config/src/misc.rs
+++ b/crates/tor-config/src/misc.rs
@@ -122,12 +122,22 @@ impl Listen {
 
     /// List the network socket addresses to listen on
     ///
+    /// Each returned item is a list of `SocketAddr`,
+    /// of which *at least one* must be successfully bound.
+    /// It is OK if the others (up to all but one of them)
+    /// fail with `EAFNOSUPPORT` ("Address family not supported").
+    /// This allows handling of support, or non-support,
+    /// for particular address faimilies, eg IPv6 vs IPv4 localhost.
+    ///
     /// Fails if the listen spec involves listening on things other than IP addresses.
     /// (Currently that is not possible.)
     pub fn ip_addrs(
         &self,
-    ) -> Result<impl Iterator<Item = net::SocketAddr> + '_, ListenUnsupported> {
-        Ok(self.0.iter().flat_map(|i| i.iter()))
+    ) -> Result<
+        impl Iterator<Item = impl Iterator<Item = net::SocketAddr> + '_> + '_,
+        ListenUnsupported,
+    > {
+        Ok(self.0.iter().map(|i| i.iter()))
     }
 
     /// Get the localhost port to listen on
@@ -353,7 +363,7 @@ mod test {
         #[allow(clippy::needless_pass_by_value)] // we do this for consistency
         fn chk(
             exp_i: Vec<ListenItem>,
-            exp_addrs: Result<Vec<SocketAddr>, ()>,
+            exp_addrs: Result<Vec<Vec<SocketAddr>>, ()>,
             exp_lpd: Result<Option<u16>, ()>,
             s: &str,
         ) {
@@ -362,7 +372,9 @@ mod test {
             eprintln!("s={:?} ll={:?}", &s, &ll);
             assert_eq!(ll, Listen(exp_i));
             assert_eq!(
-                ll.ip_addrs().map(|a| a.collect_vec()).map_err(|_| ()),
+                ll.ip_addrs()
+                    .map(|a| a.map(|l| l.collect_vec()).collect_vec())
+                    .map_err(|_| ()),
                 exp_addrs
             );
             assert_eq!(ll.localhost_port_deprecated().map_err(|_| ()), exp_lpd);
@@ -382,7 +394,7 @@ mod test {
             );
         };
 
-        let chk_1 = |v: ListenItem, addrs: Vec<SocketAddr>, port, s| {
+        let chk_1 = |v: ListenItem, addrs: Vec<Vec<SocketAddr>>, port, s| {
             chk(
                 vec![v.clone()],
                 Ok(addrs.clone()),
@@ -397,7 +409,7 @@ mod test {
             );
             chk(
                 vec![v, LI::Localhost(23.try_into().unwrap())],
-                Ok([addrs, vec![localhost6(23), localhost4(23)]]
+                Ok([addrs, vec![vec![localhost6(23), localhost4(23)]]]
                     .into_iter()
                     .flatten()
                     .collect()),
@@ -413,13 +425,13 @@ mod test {
 
         chk_1(
             LI::Localhost(42.try_into().unwrap()),
-            vec![localhost6(42), localhost4(42)],
+            vec![vec![localhost6(42), localhost4(42)]],
             Ok(Some(42)),
             "42",
         );
         chk_1(
             LI::General(unspec6(56)),
-            vec![unspec6(56)],
+            vec![vec![unspec6(56)]],
             Err(()),
             r#""[::]:56""#,
         );

From a6d7e38f6d6675f041cd92cce9307af0899f1a90 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Wed, 24 Aug 2022 19:19:15 +0100
Subject: [PATCH 10/12] tor-config Listen: Add a note about EADDRINUSE

Prompted by
  https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/602#note_2830848
---
 crates/tor-config/src/misc.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/crates/tor-config/src/misc.rs b/crates/tor-config/src/misc.rs
index 9df797f72..250d1dc99 100644
--- a/crates/tor-config/src/misc.rs
+++ b/crates/tor-config/src/misc.rs
@@ -128,6 +128,7 @@ impl Listen {
     /// fail with `EAFNOSUPPORT` ("Address family not supported").
     /// This allows handling of support, or non-support,
     /// for particular address faimilies, eg IPv6 vs IPv4 localhost.
+    /// Other errors (eg, `EADDRINUSE`) should always be treated as serious problems.
     ///
     /// Fails if the listen spec involves listening on things other than IP addresses.
     /// (Currently that is not possible.)

From 76066dac8142373883c5f4ca67df954f58adbc82 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Wed, 24 Aug 2022 19:22:54 +0100
Subject: [PATCH 11/12] tor-config Listen: Rename localhost_port_legacy (from
 _deprecated)

As per
  https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/602#note_2830847
---
 crates/arti/src/lib.rs        | 4 ++--
 crates/tor-config/src/misc.rs | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/crates/arti/src/lib.rs b/crates/arti/src/lib.rs
index c5f849143..01b746dbf 100644
--- a/crates/arti/src/lib.rs
+++ b/crates/arti/src/lib.rs
@@ -509,7 +509,7 @@ where
     if let Some(proxy_matches) = matches.subcommand_matches("proxy") {
         let socks_port = match (
             proxy_matches.value_of("socks-port"),
-            config.proxy().socks_listen.localhost_port_deprecated()?,
+            config.proxy().socks_listen.localhost_port_legacy()?,
         ) {
             (Some(p), _) => p.parse().expect("Invalid port specified"),
             (None, Some(s)) => s,
@@ -518,7 +518,7 @@ where
 
         let dns_port = match (
             proxy_matches.value_of("dns-port"),
-            config.proxy().dns_listen.localhost_port_deprecated()?,
+            config.proxy().dns_listen.localhost_port_legacy()?,
         ) {
             (Some(p), _) => p.parse().expect("Invalid port specified"),
             (None, Some(s)) => s,
diff --git a/crates/tor-config/src/misc.rs b/crates/tor-config/src/misc.rs
index 250d1dc99..a6da10b39 100644
--- a/crates/tor-config/src/misc.rs
+++ b/crates/tor-config/src/misc.rs
@@ -147,7 +147,7 @@ impl Listen {
     ///
     /// Fails, giving an unsupported error, if the configuration
     /// isn't just "listen on a single localhost port".
-    pub fn localhost_port_deprecated(&self) -> Result<Option<u16>, ListenUnsupported> {
+    pub fn localhost_port_legacy(&self) -> Result<Option<u16>, ListenUnsupported> {
         use ListenItem as LI;
         Ok(match &*self.0 {
             [] => None,
@@ -378,7 +378,7 @@ mod test {
                     .map_err(|_| ()),
                 exp_addrs
             );
-            assert_eq!(ll.localhost_port_deprecated().map_err(|_| ()), exp_lpd);
+            assert_eq!(ll.localhost_port_legacy().map_err(|_| ()), exp_lpd);
         }
 
         let chk_err = |exp, s: &str| {

From 3faf4475cce98b725efe393df3c9ceafb54340d8 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Wed, 24 Aug 2022 20:08:06 +0100
Subject: [PATCH 12/12] Bump toml dependency

We need
  60b874308e6792a73cc00517a60bbef60a12e3cc
  Mixed type arrays (#358)
for a test case in tor-config.

While we're here, drop the dupe entry in tor-config.

(In principle we could make this increase only in tor-config's
dev-dependencies, but that seems unnecessarily fiddly.)
---
 crates/arti/Cargo.toml        | 2 +-
 crates/fs-mistrust/Cargo.toml | 2 +-
 crates/tor-config/Cargo.toml  | 3 +--
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/crates/arti/Cargo.toml b/crates/arti/Cargo.toml
index 69f709c9a..ee913c4b9 100644
--- a/crates/arti/Cargo.toml
+++ b/crates/arti/Cargo.toml
@@ -71,7 +71,7 @@ visibility = { version = "0.0.1", optional = true }
 itertools = "0.10.1"
 regex = { version = "1", default-features = false, features = ["std"] }
 serde_json = "1.0.50"
-toml = "0.5"
+toml = "0.5.6"
 
 [target.'cfg(unix)'.dependencies]
 libc = { version = "0.2", default-features = false }
diff --git a/crates/fs-mistrust/Cargo.toml b/crates/fs-mistrust/Cargo.toml
index 77921700f..a25ddc4de 100644
--- a/crates/fs-mistrust/Cargo.toml
+++ b/crates/fs-mistrust/Cargo.toml
@@ -32,7 +32,7 @@ users = "0.11"
 anyhow = "1.0.23"
 serde_json = "1.0.50"
 tempfile = "3"
-toml = "0.5"
+toml = "0.5.6"
 [package.metadata.docs.rs]
 all-features = true
 rustdoc-args = ["--cfg", "docsrs"]
diff --git a/crates/tor-config/Cargo.toml b/crates/tor-config/Cargo.toml
index 2301e285b..86294b84c 100644
--- a/crates/tor-config/Cargo.toml
+++ b/crates/tor-config/Cargo.toml
@@ -32,7 +32,7 @@ serde_ignored = "0.1.3"
 shellexpand = { version = "2.1.2", optional = true }
 strum = { version = "0.24", features = ["derive"] }
 thiserror = "1"
-toml = "0.5"
+toml = "0.5.6"
 tor-basic-utils = { path = "../tor-basic-utils", version = "0.3.3" }
 tor-error = { path = "../tor-error", version = "0.3.2" }
 tracing = "0.1.18"
@@ -44,7 +44,6 @@ dirs = "4.0.0"
 rmp-serde = "1"
 serde_json = "1.0.50"
 tempfile = "3"
-toml = "0.5"
 tracing-test = "0.2"
 [package.metadata.docs.rs]
 all-features = true