Merge branch 'idx' into 'main'

tor-netdir: Try to abolish unqualified uses of idx

See merge request tpo/core/arti!1083
This commit is contained in:
gabi-250 2023-03-27 17:38:31 +00:00
commit a282e1f300
2 changed files with 71 additions and 68 deletions

View File

@ -65,7 +65,10 @@ pub(crate) struct HsDirRing {
}
/// Compute the [`HsDirIndex`] for a given relay.
pub(crate) fn relay_index(kp_relayid_ed: &Ed25519Identity, params: &HsDirParams) -> HsDirIndex {
pub(crate) fn relay_hsdir_index(
kp_relayid_ed: &Ed25519Identity,
params: &HsDirParams,
) -> HsDirIndex {
// rend-spec-v3 2.2.3 "hsdir_index(node)"
//
// hsdir_index(node) = H("node-idx" | node_identity |
@ -85,7 +88,7 @@ pub(crate) fn relay_index(kp_relayid_ed: &Ed25519Identity, params: &HsDirParams)
}
/// Compute the starting [`HsDirIndex`] for a given descriptor replica.
pub(crate) fn service_index(
pub(crate) fn service_hsdir_index(
kp_hs_blind_id: &HsBlindId,
replica: u8,
params: &HsDirParams,
@ -162,8 +165,8 @@ impl HsDirRing {
let reuse_index_values = prev_ring
.ring
.iter()
.filter_map(|(hsdir_index, rsi)| {
Some((prev_netdir.md_by_idx(*rsi)?.ed25519_id(), hsdir_index))
.filter_map(|(hsdir_index, rsidx)| {
Some((prev_netdir.md_by_rsidx(*rsidx)?.ed25519_id(), hsdir_index))
})
.collect();
Some(reuse_index_values)
@ -172,19 +175,19 @@ impl HsDirRing {
let mut new_ring: Vec<_> = this_netdir
.all_hsdirs()
.map(|(rsi, relay)| {
.map(|(rsidx, relay)| {
let ed_id = relay.md.ed25519_id();
let hsdir_index = reuse_index_values
.get(ed_id)
.cloned()
.cloned()
.unwrap_or_else(|| relay_index(ed_id, &new_params));
(hsdir_index, rsi)
.unwrap_or_else(|| relay_hsdir_index(ed_id, &new_params));
(hsdir_index, rsidx)
})
.collect();
// rsi are all different, so no need to think about comparing them
new_ring.sort_by_key(|(hsdir_index, _rsi)| *hsdir_index);
// rsidx are all different, so no need to think about comparing them
new_ring.sort_by_key(|(hsdir_index, _rsidx)| *hsdir_index);
HsDirRing {
ring: new_ring,
@ -192,20 +195,20 @@ impl HsDirRing {
}
}
/// Find the location or (notional) insertion point for `idx` within `ring`.
fn find_pos(&self, idx: HsDirIndex) -> usize {
/// Find the location or (notional) insertion point for `hsdir_index` within `ring`.
fn find_pos(&self, hsdir_index: HsDirIndex) -> usize {
// TODO hs implement this
todo!()
}
/// Yield items from `ring` starting with `idx`, wrapping around once when we
/// Yield items from `ring` starting with `hsdir_index`, wrapping around once when we
/// reach the end, and yielding no element more than once.
pub(crate) fn ring_items_at(
&self,
idx: HsDirIndex,
hsdir_index: HsDirIndex,
) -> impl Iterator<Item = &(HsDirIndex, RouterStatusIdx)> {
let idx = self.find_pos(idx);
self.ring[idx..].iter().chain(&self.ring[..idx])
let pos = self.find_pos(hsdir_index);
self.ring[pos..].iter().chain(&self.ring[..pos])
}
/// Return the time period for which this ring applies.
@ -255,7 +258,7 @@ mod test {
{
let kp_hs_blind_id = [0x42; 32].into();
let replica = 1;
let got = service_index(&kp_hs_blind_id, replica, &params);
let got = service_hsdir_index(&kp_hs_blind_id, replica, &params);
assert_eq!(
hex::encode(got.as_ref()),
"37e5cbbd56a22823714f18f1623ece5983a0d64c78495a8cfab854245e5f9a8a",
@ -265,7 +268,7 @@ mod test {
// relay_index AKA hsdir_index
{
let kp_relayid_ed = [0x42; 32].into();
let got = relay_index(&kp_relayid_ed, &params);
let got = relay_hsdir_index(&kp_relayid_ed, &params);
assert_eq!(
hex::encode(got.as_ref()),
"db475361014a09965e7e5e4d4a25b8f8d4b8f16cb1d8a7e95eed50249cc1a2d5",

View File

@ -295,7 +295,7 @@ pub struct NetDir {
mds: TiVec<RouterStatusIdx, Option<Arc<Microdesc>>>,
/// Map from SHA256 of _missing_ microdescriptors to the index of their
/// corresponding routerstatus.
rs_idx_by_missing: HashMap<MdDigest, RouterStatusIdx>,
rsidx_by_missing: HashMap<MdDigest, RouterStatusIdx>,
/// Map from ed25519 identity to index of the routerstatus.
///
/// Note that we don't know the ed25519 identity of a relay until
@ -304,16 +304,16 @@ pub struct NetDir {
///
/// # Implementation note
///
/// For this field, and for `rs_idx_by_rsa`, and for
/// `MdEntry::*::rsa_idx`, it might be cool to have references instead.
/// For this field, and for `rsidx_by_rsa`,
/// it might be cool to have references instead.
/// But that would make this into a self-referential structure,
/// which isn't possible in safe rust.
rs_idx_by_ed: HashMap<Ed25519Identity, RouterStatusIdx>,
rsidx_by_ed: HashMap<Ed25519Identity, RouterStatusIdx>,
/// Map from RSA identity to index of the routerstatus.
///
/// This is constructed at the same time as the NetDir object, so it
/// can be immutable.
rs_idx_by_rsa: Arc<HashMap<RsaIdentity, RouterStatusIdx>>,
rsidx_by_rsa: Arc<HashMap<RsaIdentity, RouterStatusIdx>>,
/// Hash ring(s) describing the onion service directory.
///
@ -619,16 +619,16 @@ impl PartialNetDir {
let n_relays = consensus.c_relays().len();
let rs_idx_by_missing = consensus
let rsidx_by_missing = consensus
.c_relays()
.iter_enumerated()
.map(|(rs_idx, rs)| (*rs.md_digest(), rs_idx))
.map(|(rsidx, rs)| (*rs.md_digest(), rsidx))
.collect();
let rs_idx_by_rsa = consensus
let rsidx_by_rsa = consensus
.c_relays()
.iter_enumerated()
.map(|(rs_idx, rs)| (*rs.rsa_identity(), rs_idx))
.map(|(rsidx, rs)| (*rs.rsa_identity(), rsidx))
.collect();
#[cfg(feature = "hs-common")]
@ -645,9 +645,9 @@ impl PartialNetDir {
consensus: Arc::new(consensus),
params,
mds: vec![None; n_relays].into(),
rs_idx_by_missing,
rs_idx_by_rsa: Arc::new(rs_idx_by_rsa),
rs_idx_by_ed: HashMap::with_capacity(n_relays),
rsidx_by_missing,
rsidx_by_rsa: Arc::new(rsidx_by_rsa),
rsidx_by_ed: HashMap::with_capacity(n_relays),
#[cfg(feature = "hs-common")]
hsdir_rings,
weights,
@ -738,20 +738,20 @@ impl NetDir {
/// Return true if we wanted it, and false otherwise.
#[allow(clippy::missing_panics_doc)] // Can't panic on valid object.
fn add_arc_microdesc(&mut self, md: Arc<Microdesc>) -> bool {
if let Some(rs_idx) = self.rs_idx_by_missing.remove(md.digest()) {
assert_eq!(self.c_relays()[rs_idx].md_digest(), md.digest());
if let Some(rsidx) = self.rsidx_by_missing.remove(md.digest()) {
assert_eq!(self.c_relays()[rsidx].md_digest(), md.digest());
// There should never be two approved MDs in the same
// consensus listing the same ID... but if there is,
// we'll let the most recent one win.
self.rs_idx_by_ed.insert(*md.ed25519_id(), rs_idx);
self.rsidx_by_ed.insert(*md.ed25519_id(), rsidx);
// Happy path: we did indeed want this one.
self.mds[rs_idx] = Some(md);
self.mds[rsidx] = Some(md);
// Save some space in the missing-descriptor list.
if self.rs_idx_by_missing.len() < self.rs_idx_by_missing.capacity() / 4 {
self.rs_idx_by_missing.shrink_to_fit();
if self.rsidx_by_missing.len() < self.rsidx_by_missing.capacity() / 4 {
self.rsidx_by_missing.shrink_to_fit();
}
return true;
@ -763,13 +763,13 @@ impl NetDir {
/// Construct a (possibly invalid) Relay object from a routerstatus and its
/// index within the consensus.
fn relay_from_rs_and_idx<'a>(
fn relay_from_rs_and_rsidx<'a>(
&'a self,
rs: &'a netstatus::MdConsensusRouterStatus,
rs_idx: RouterStatusIdx,
rsidx: RouterStatusIdx,
) -> UncheckedRelay<'a> {
debug_assert_eq!(self.c_relays()[rs_idx].rsa_identity(), rs.rsa_identity());
let md = self.mds[rs_idx].as_deref();
debug_assert_eq!(self.c_relays()[rsidx].rsa_identity(), rs.rsa_identity());
let md = self.mds[rsidx].as_deref();
if let Some(md) = md {
debug_assert_eq!(rs.md_digest(), md.digest());
}
@ -800,7 +800,7 @@ impl NetDir {
// do so many hashtable lookups.
self.c_relays()
.iter_enumerated()
.map(move |(idx, rs)| self.relay_from_rs_and_idx(rs, idx))
.map(move |(rsidx, rs)| self.relay_from_rs_and_rsidx(rs, rsidx))
}
/// Return an iterator over all usable Relays.
pub fn relays(&self) -> impl Iterator<Item = Relay<'_>> {
@ -809,8 +809,8 @@ impl NetDir {
/// Look up a relay's `MicroDesc` by its `RouterStatusIdx`
#[cfg_attr(not(feature = "hs-common"), allow(dead_code))]
pub(crate) fn md_by_idx(&self, rsi: RouterStatusIdx) -> Option<&Microdesc> {
self.mds.get(rsi)?.as_deref()
pub(crate) fn md_by_rsidx(&self, rsidx: RouterStatusIdx) -> Option<&Microdesc> {
self.mds.get(rsidx)?.as_deref()
}
/// Return a relay matching a given identity, if we have a
@ -830,10 +830,10 @@ impl NetDir {
let id = id.into();
let answer = match id {
RelayIdRef::Ed25519(ed25519) => {
let rs_idx = *self.rs_idx_by_ed.get(ed25519)?;
let rs = self.c_relays().get(rs_idx).expect("Corrupt index");
let rsidx = *self.rsidx_by_ed.get(ed25519)?;
let rs = self.c_relays().get(rsidx).expect("Corrupt index");
self.relay_from_rs_and_idx(rs, rs_idx).into_relay()?
self.relay_from_rs_and_rsidx(rs, rsidx).into_relay()?
}
RelayIdRef::Rsa(rsa) => self
.by_rsa_id_unchecked(rsa)
@ -919,7 +919,7 @@ impl NetDir {
(Some(r), Some(e)) => self.id_pair_listed(e, r),
(Some(r), None) => Some(self.rsa_id_is_listed(r)),
(None, Some(e)) => {
if self.rs_idx_by_ed.contains_key(e) {
if self.rsidx_by_ed.contains_key(e) {
Some(true)
} else {
None
@ -932,10 +932,10 @@ impl NetDir {
/// Return a (possibly unusable) relay with a given RSA identity.
#[allow(clippy::missing_panics_doc)] // Can't panic on valid object.
fn by_rsa_id_unchecked(&self, rsa_id: &RsaIdentity) -> Option<UncheckedRelay<'_>> {
let rs_idx = *self.rs_idx_by_rsa.get(rsa_id)?;
let rs = self.c_relays().get(rs_idx).expect("Corrupt index");
let rsidx = *self.rsidx_by_rsa.get(rsa_id)?;
let rs = self.c_relays().get(rsidx).expect("Corrupt index");
assert_eq!(rs.rsa_identity(), rsa_id);
Some(self.relay_from_rs_and_idx(rs, rs_idx))
Some(self.relay_from_rs_and_rsidx(rs, rsidx))
}
/// Return the relay with a given RSA identity, if we have one
/// and it is usable.
@ -953,11 +953,11 @@ impl NetDir {
/// The results are not returned in any particular order.
#[cfg(feature = "hs-common")]
fn all_hsdirs(&self) -> impl Iterator<Item = (RouterStatusIdx, Relay<'_>)> {
self.c_relays().iter_enumerated().filter_map(|(rsi, rs)| {
let relay = self.relay_from_rs_and_idx(rs, rsi);
self.c_relays().iter_enumerated().filter_map(|(rsidx, rs)| {
let relay = self.relay_from_rs_and_rsidx(rs, rsidx);
relay.is_hsdir_for_ring().then(|| ())?;
let relay = relay.into_relay()?;
Some((rsi, relay))
Some((rsidx, relay))
})
}
@ -1256,13 +1256,13 @@ impl NetDir {
impl MdReceiver for NetDir {
fn missing_microdescs(&self) -> Box<dyn Iterator<Item = &MdDigest> + '_> {
Box::new(self.rs_idx_by_missing.keys())
Box::new(self.rsidx_by_missing.keys())
}
fn add_microdesc(&mut self, md: Microdesc) -> bool {
self.add_arc_microdesc(Arc::new(md))
}
fn n_missing(&self) -> usize {
self.rs_idx_by_missing.len()
self.rsidx_by_missing.len()
}
}
@ -1586,8 +1586,8 @@ mod test {
let (consensus, microdescs) = construct_network().unwrap();
let mut dir = PartialNetDir::new(consensus.clone(), Some(&low_threshold));
for (idx, md) in microdescs.iter().enumerate() {
if idx % 7 == 2 {
for (pos, md) in microdescs.iter().enumerate() {
if pos % 7 == 2 {
continue; // skip a few relays.
}
dir.add_microdesc(md.clone());
@ -1628,8 +1628,8 @@ mod test {
// But if we try again with a slightly higher threshold...
let mut dir = PartialNetDir::new(consensus, Some(&high_threshold));
for (idx, md) in microdescs.into_iter().enumerate() {
if idx % 7 == 2 {
for (pos, md) in microdescs.into_iter().enumerate() {
if pos % 7 == 2 {
continue; // skip a few relays.
}
dir.add_microdesc(md);
@ -1758,10 +1758,10 @@ mod test {
#[test]
fn relay_funcs() {
let (consensus, microdescs) = construct_custom_network(|idx, nb| {
if idx == 15 {
let (consensus, microdescs) = construct_custom_network(|pos, nb| {
if pos == 15 {
nb.rs.add_or_port("[f0f0::30]:9001".parse().unwrap());
} else if idx == 20 {
} else if pos == 20 {
nb.rs.add_or_port("[f0f0::3131]:9001".parse().unwrap());
}
})
@ -1853,8 +1853,8 @@ mod test {
// make a netdir where relays 10-19 are badexit, and everybody
// exits to 443 on IPv6.
use tor_netdoc::doc::netstatus::RelayFlags;
let netdir = construct_custom_netdir(|idx, nb| {
if (10..20).contains(&idx) {
let netdir = construct_custom_netdir(|pos, nb| {
if (10..20).contains(&pos) {
nb.rs.add_flags(RelayFlags::BAD_EXIT);
}
nb.md.parse_ipv6_policy("accept 443").unwrap();
@ -1903,8 +1903,8 @@ mod test {
#[test]
fn test_by_id() {
// Make a netdir that omits the microdescriptor for 0xDDDDDD...
let netdir = construct_custom_netdir(|idx, mut nb| {
nb.omit_md = idx == 13;
let netdir = construct_custom_netdir(|pos, mut nb| {
nb.omit_md = pos == 13;
})
.unwrap();
@ -2011,8 +2011,8 @@ mod test {
#[test]
fn family_list() {
let netdir = construct_custom_netdir(|idx, n| {
if idx == 0x0a {
let netdir = construct_custom_netdir(|pos, n| {
if pos == 0x0a {
n.md.family(
"$0B0B0B0B0B0B0B0B0B0B0B0B0B0B0B0B0B0B0B0B \
$0C0C0C0C0C0C0C0C0C0C0C0C0C0C0C0C0C0C0C0C \
@ -2020,7 +2020,7 @@ mod test {
.parse()
.unwrap(),
);
} else if idx == 0x0c {
} else if pos == 0x0c {
n.md.family("$0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A".parse().unwrap());
}
})