Commit ff05ed61 authored by Nick Mathewson's avatar Nick Mathewson 🤹
Browse files

Merge branch 'accessors' into 'main'

Abolish some accessors in drmgr

See merge request !423
parents 0fdbe701 25dab822
Loading
Loading
Loading
Loading
+11 −24
Original line number Diff line number Diff line
@@ -6,7 +6,6 @@
use derive_builder::Builder;
use serde::Deserialize;
use tor_llcrypto::pk::rsa::RsaIdentity;
use tor_netdoc::doc::authcert::{AuthCert, AuthCertKeyIds};

/// A single authority that signs a consensus directory.
//
@@ -22,7 +21,7 @@ pub struct Authority {
    /// A SHA1 digest of the DER-encoded long-term v3 RSA identity key for
    /// this authority.
    // TODO: It would be lovely to use a better hash for these identities.
    v3ident: RsaIdentity,
    pub(crate) v3ident: RsaIdentity,
}

impl Authority {
@@ -33,27 +32,6 @@ impl Authority {
    pub fn builder() -> AuthorityBuilder {
        AuthorityBuilder::default()
    }
    /// Return the (human-readable) name for this authority.
    pub fn name(&self) -> &str {
        self.name.as_ref()
    }
    /// Return the v3 identity key of this certificate.
    ///
    /// This is the identity of the >=2048-bit RSA key that the
    /// authority uses to sign documents; it is distinct from its
    /// identity keys that it uses when operating as a relay.
    pub fn v3ident(&self) -> &RsaIdentity {
        &self.v3ident
    }
    /// Return true if this authority matches a given certificate.
    pub fn matches_cert(&self, cert: &AuthCert) -> bool {
        &self.v3ident == cert.id_fingerprint()
    }

    /// Return true if this authority matches a given key ID.
    pub fn matches_keyid(&self, id: &AuthCertKeyIds) -> bool {
        self.v3ident == id.id_fingerprint
    }
}

/// Return a vector of the default directory authorities.
@@ -94,6 +72,15 @@ impl AuthorityBuilder {
mod test {
    #![allow(clippy::unwrap_used)]
    use super::*;
    use tor_netdoc::doc::authcert::AuthCertKeyIds;

    impl Authority {
        /// Return true if this authority matches a given key ID.
        fn matches_keyid(&self, id: &AuthCertKeyIds) -> bool {
            self.v3ident == id.id_fingerprint
        }
    }

    #[test]
    fn authority() {
        let key1: RsaIdentity = [9_u8; 20].into();
@@ -104,7 +91,7 @@ mod test {
            .build()
            .unwrap();

        assert_eq!(auth.v3ident(), &key1);
        assert_eq!(&auth.v3ident, &key1);

        let keyids1 = AuthCertKeyIds {
            id_fingerprint: key1,
+14 −19
Original line number Diff line number Diff line
@@ -40,7 +40,10 @@ pub struct NetworkConfig {
    /// affect future download attempts only.
    #[serde(default = "fallbacks::default_fallbacks")]
    #[builder(default = "fallbacks::default_fallbacks()")]
    fallback_caches: Vec<FallbackDir>,
    #[serde(rename = "fallback_caches")]
    #[builder_field_attr(serde(rename = "fallback_caches"))]
    #[builder(setter(name = "fallback_caches"))]
    pub(crate) fallbacks: Vec<FallbackDir>,

    /// List of directory authorities which we expect to sign consensus
    /// documents.
@@ -51,13 +54,13 @@ pub struct NetworkConfig {
    /// This section cannot be changed in a running Arti client.
    #[serde(default = "crate::authority::default_authorities")]
    #[builder(default = "crate::authority::default_authorities()")]
    authorities: Vec<Authority>,
    pub(crate) authorities: Vec<Authority>,
}

impl Default for NetworkConfig {
    fn default() -> Self {
        NetworkConfig {
            fallback_caches: fallbacks::default_fallbacks(),
            fallbacks: fallbacks::default_fallbacks(),
            authorities: crate::authority::default_authorities(),
        }
    }
@@ -68,20 +71,12 @@ impl NetworkConfig {
    pub fn builder() -> NetworkConfigBuilder {
        NetworkConfigBuilder::default()
    }
    /// Return the configured directory authorities
    pub(crate) fn authorities(&self) -> &[Authority] {
        &self.authorities[..]
    }
    /// Return the configured fallback directories
    pub(crate) fn fallbacks(&self) -> &[FallbackDir] {
        &self.fallback_caches[..]
    }
}

impl NetworkConfigBuilder {
    /// Check that this builder will give a reasonable network.
    fn validate(&self) -> std::result::Result<(), ConfigBuildError> {
        if self.authorities.is_some() && self.fallback_caches.is_none() {
        if self.authorities.is_some() && self.fallbacks.is_none() {
            return Err(ConfigBuildError::Inconsistent {
                fields: vec!["authorities".to_owned(), "fallbacks".to_owned()],
                problem: "Non-default authorities are use, but the fallback list is not overridden"
@@ -225,12 +220,12 @@ impl DirMgrConfig {

    /// Return a slice of the configured authorities
    pub fn authorities(&self) -> &[Authority] {
        self.network_config.authorities()
        &self.network_config.authorities
    }

    /// Return the configured set of fallback directories
    pub fn fallbacks(&self) -> &[FallbackDir] {
        self.network_config.fallbacks()
        &self.network_config.fallbacks
    }

    /// Return set of configured networkstatus parameter overrides.
@@ -252,7 +247,7 @@ impl DirMgrConfig {
        DirMgrConfig {
            cache_path: self.cache_path.clone(),
            network_config: NetworkConfig {
                fallback_caches: new_config.network_config.fallback_caches.clone(),
                fallbacks: new_config.network_config.fallbacks.clone(),
                authorities: self.network_config.authorities.clone(),
            },
            schedule_config: new_config.schedule_config.clone(),
@@ -355,8 +350,8 @@ mod test {
        // with nothing set, we get the default.
        let mut bld = NetworkConfig::builder();
        let cfg = bld.build().unwrap();
        assert_eq!(cfg.authorities().len(), dflt.authorities.len());
        assert_eq!(cfg.fallbacks().len(), dflt.fallback_caches.len());
        assert_eq!(cfg.authorities.len(), dflt.authorities.len());
        assert_eq!(cfg.fallbacks.len(), dflt.fallbacks.len());

        // with any authorities set, the fallback list _must_ be set
        // or the build fails.
@@ -382,8 +377,8 @@ mod test {
            .build()
            .unwrap()]);
        let cfg = bld.build().unwrap();
        assert_eq!(cfg.authorities().len(), 2);
        assert_eq!(cfg.fallbacks().len(), 1);
        assert_eq!(cfg.authorities.len(), 2);
        assert_eq!(cfg.fallbacks.len(), 1);

        Ok(())
    }
+1 −1
Original line number Diff line number Diff line
@@ -149,7 +149,7 @@ impl<DM: WriteNetDir> GetConsensusState<DM> {
                .config()
                .authorities()
                .iter()
                .map(|auth| *auth.v3ident())
                .map(|auth| auth.v3ident)
                .collect();
            let after = writedir
                .netdir()
+1 −0
Original line number Diff line number Diff line
@@ -52,6 +52,7 @@ arti-client:
tor-dirmgr:
  new-api: DirMgrConfig object now has accessors.
  DirMgrCfg: totally changed, builder abolished.
  Authority, NetworkConfig: removed several accessors for these config elements.

tor-circmgr:
  CircMgrCfg: totally changed, builder abolished.