Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(log): Stop logging peer IP addresses, to protect user privacy #6662

Merged
merged 18 commits into from
May 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 22 additions & 13 deletions zebra-network/src/address_book.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@ use tracing::Span;
use zebra_chain::parameters::Network;

use crate::{
constants, meta_addr::MetaAddrChange, protocol::external::canonical_socket_addr,
types::MetaAddr, AddressBookPeers, PeerAddrState,
constants,
meta_addr::MetaAddrChange,
protocol::external::{canonical_peer_addr, canonical_socket_addr},
types::MetaAddr,
AddressBookPeers, PeerAddrState, PeerSocketAddr,
};

#[cfg(test)]
mod tests;

Expand Down Expand Up @@ -66,7 +70,7 @@ pub struct AddressBook {
/// We reverse the comparison order, because the standard library
/// ([`BTreeMap`](std::collections::BTreeMap)) sorts in ascending order, but
/// [`OrderedMap`] sorts in descending order.
by_addr: OrderedMap<SocketAddr, MetaAddr, Reverse<MetaAddr>>,
by_addr: OrderedMap<PeerSocketAddr, MetaAddr, Reverse<MetaAddr>>,

/// The local listener address.
local_listener: SocketAddr,
Expand Down Expand Up @@ -182,7 +186,7 @@ impl AddressBook {
let addrs = addrs
.into_iter()
.map(|mut meta_addr| {
meta_addr.addr = canonical_socket_addr(meta_addr.addr);
meta_addr.addr = canonical_peer_addr(meta_addr.addr);
meta_addr
})
.filter(|meta_addr| meta_addr.address_is_valid_for_outbound(network))
Expand Down Expand Up @@ -219,11 +223,16 @@ impl AddressBook {
///
/// This address contains minimal state, but it is not sanitized.
pub fn local_listener_meta_addr(&self) -> MetaAddr {
MetaAddr::new_local_listener_change(&self.local_listener)
MetaAddr::new_local_listener_change(self.local_listener)
.into_new_meta_addr()
.expect("unexpected invalid new local listener addr")
}

/// Get the local listener [`SocketAddr`].
pub fn local_listener_socket_addr(&self) -> SocketAddr {
self.local_listener
}

/// Get the contents of `self` in random order with sanitized timestamps.
pub fn sanitized(&self, now: chrono::DateTime<Utc>) -> Vec<MetaAddr> {
use rand::seq::SliceRandom;
Expand Down Expand Up @@ -257,8 +266,8 @@ impl AddressBook {
/// Look up `addr` in the address book, and return its [`MetaAddr`].
///
/// Converts `addr` to a canonical address before looking it up.
pub fn get(&mut self, addr: &SocketAddr) -> Option<MetaAddr> {
let addr = canonical_socket_addr(*addr);
pub fn get(&mut self, addr: PeerSocketAddr) -> Option<MetaAddr> {
let addr = canonical_peer_addr(*addr);

// Unfortunately, `OrderedMap` doesn't implement `get`.
let meta_addr = self.by_addr.remove(&addr);
Expand All @@ -278,7 +287,7 @@ impl AddressBook {
/// All changes should go through `update`, so that the address book
/// only contains valid outbound addresses.
///
/// Change addresses must be canonical `SocketAddr`s. This makes sure that
/// Change addresses must be canonical `PeerSocketAddr`s. This makes sure that
/// each address book entry has a unique IP address.
///
/// # Security
Expand All @@ -287,11 +296,11 @@ impl AddressBook {
/// to the address book. This prevents rapid reconnections to the same peer.
///
/// As an exception, this function can ignore all changes for specific
/// [`SocketAddr`]s. Ignored addresses will never be used to connect to
/// [`PeerSocketAddr`]s. Ignored addresses will never be used to connect to
/// peers.
#[allow(clippy::unwrap_in_result)]
pub fn update(&mut self, change: MetaAddrChange) -> Option<MetaAddr> {
let previous = self.get(&change.addr());
let previous = self.get(change.addr());

let _guard = self.span.enter();

Expand Down Expand Up @@ -378,7 +387,7 @@ impl AddressBook {
/// All address removals should go through `take`, so that the address
/// book metrics are accurate.
#[allow(dead_code)]
fn take(&mut self, removed_addr: SocketAddr) -> Option<MetaAddr> {
fn take(&mut self, removed_addr: PeerSocketAddr) -> Option<MetaAddr> {
let _guard = self.span.enter();

let instant_now = Instant::now();
Expand All @@ -399,9 +408,9 @@ impl AddressBook {
}
}

/// Returns true if the given [`SocketAddr`] is pending a reconnection
/// Returns true if the given [`PeerSocketAddr`] is pending a reconnection
/// attempt.
pub fn pending_reconnection_addr(&mut self, addr: &SocketAddr) -> bool {
pub fn pending_reconnection_addr(&mut self, addr: PeerSocketAddr) -> bool {
let meta_addr = self.get(addr);

let _guard = self.span.enter();
Expand Down
14 changes: 7 additions & 7 deletions zebra-network/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ use crate::{
DEFAULT_CRAWL_NEW_PEER_INTERVAL, DNS_LOOKUP_TIMEOUT, INBOUND_PEER_LIMIT_MULTIPLIER,
OUTBOUND_PEER_LIMIT_MULTIPLIER,
},
protocol::external::canonical_socket_addr,
BoxError,
protocol::external::{canonical_peer_addr, canonical_socket_addr},
BoxError, PeerSocketAddr,
};

#[cfg(test)]
Expand Down Expand Up @@ -145,7 +145,7 @@ impl Config {
}

/// Resolve initial seed peer IP addresses, based on the configured network.
pub async fn initial_peers(&self) -> HashSet<SocketAddr> {
pub async fn initial_peers(&self) -> HashSet<PeerSocketAddr> {
Config::resolve_peers(&self.initial_peer_hostnames().iter().cloned().collect()).await
}

Expand All @@ -154,7 +154,7 @@ impl Config {
///
/// If DNS resolution fails or times out for all peers, continues retrying
/// until at least one peer is found.
async fn resolve_peers(peers: &HashSet<String>) -> HashSet<SocketAddr> {
async fn resolve_peers(peers: &HashSet<String>) -> HashSet<PeerSocketAddr> {
use futures::stream::StreamExt;

if peers.is_empty() {
Expand Down Expand Up @@ -196,7 +196,7 @@ impl Config {
/// `max_retries` times.
///
/// If DNS continues to fail, returns an empty list of addresses.
async fn resolve_host(host: &str, max_retries: usize) -> HashSet<SocketAddr> {
async fn resolve_host(host: &str, max_retries: usize) -> HashSet<PeerSocketAddr> {
for retries in 0..=max_retries {
if let Ok(addresses) = Config::resolve_host_once(host).await {
return addresses;
Expand Down Expand Up @@ -225,13 +225,13 @@ impl Config {
///
/// If `host` is a DNS name, performs DNS resolution with a timeout of a few seconds.
/// If DNS resolution fails or times out, returns an error.
async fn resolve_host_once(host: &str) -> Result<HashSet<SocketAddr>, BoxError> {
async fn resolve_host_once(host: &str) -> Result<HashSet<PeerSocketAddr>, BoxError> {
let fut = tokio::net::lookup_host(host);
let fut = tokio::time::timeout(DNS_LOOKUP_TIMEOUT, fut);

match fut.await {
Ok(Ok(ip_addrs)) => {
let ip_addrs: Vec<SocketAddr> = ip_addrs.map(canonical_socket_addr).collect();
let ip_addrs: Vec<PeerSocketAddr> = ip_addrs.map(canonical_peer_addr).collect();

// if we're logging at debug level,
// the full list of IP addresses will be shown in the log message
Expand Down
12 changes: 7 additions & 5 deletions zebra-network/src/isolated.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Creating isolated connections to specific peers.

use std::{future::Future, net::SocketAddr};
use std::future::Future;

use futures::future::TryFutureExt;
use tokio::io::{AsyncRead, AsyncWrite};
Expand All @@ -11,7 +11,7 @@ use zebra_chain::{chain_tip::NoChainTip, parameters::Network};
use crate::{
peer::{self, Client, ConnectedAddr, HandshakeRequest},
peer_set::ActiveConnectionCounter,
BoxError, Config, Request, Response,
BoxError, Config, PeerSocketAddr, Request, Response,
};

// Wait until `arti-client`'s dependency `x25519-dalek v1.2.0` is updated to a higher version. (#5492)
Expand Down Expand Up @@ -126,7 +126,7 @@ where
/// Prefer `connect_isolated_tor` if available.
pub fn connect_isolated_tcp_direct(
network: Network,
addr: SocketAddr,
addr: impl Into<PeerSocketAddr>,
user_agent: String,
) -> impl Future<Output = Result<Client, BoxError>> {
let nil_inbound_service =
Expand All @@ -146,7 +146,7 @@ pub fn connect_isolated_tcp_direct(
/// which makes it stand out from other isolated connections from other peers.
pub fn connect_isolated_tcp_direct_with_inbound<InboundService>(
network: Network,
addr: SocketAddr,
addr: impl Into<PeerSocketAddr>,
user_agent: String,
inbound_service: InboundService,
) -> impl Future<Output = Result<Client, BoxError>>
Expand All @@ -155,7 +155,9 @@ where
Service<Request, Response = Response, Error = BoxError> + Clone + Send + 'static,
InboundService::Future: Send,
{
tokio::net::TcpStream::connect(addr)
let addr = addr.into();

tokio::net::TcpStream::connect(*addr)
.err_into()
.and_then(move |tcp_stream| {
connect_isolated_with_inbound(network, tcp_stream, user_agent, inbound_service)
Expand Down
4 changes: 2 additions & 2 deletions zebra-network/src/isolated/tests/vectors.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Fixed test vectors for isolated Zebra connections.

use std::{net::SocketAddr, task::Poll, time::Duration};
use std::{task::Poll, time::Duration};

use futures::stream::StreamExt;
use tokio_util::codec::Framed;
Expand Down Expand Up @@ -151,7 +151,7 @@ async fn check_version_message<PeerTransport>(
//
// SECURITY TODO: check if the timestamp field can be zeroed, to remove another distinguisher (#3300)

let mut fixed_isolated_addr: SocketAddr = "0.0.0.0:0".parse().unwrap();
let mut fixed_isolated_addr: PeerSocketAddr = "0.0.0.0:0".parse().unwrap();
fixed_isolated_addr.set_port(network.default_port());

// Required fields should be accurate and match most other peers.
Expand Down
7 changes: 4 additions & 3 deletions zebra-network/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,16 +166,17 @@ pub use crate::isolated::tor::connect_isolated_tor;
pub use crate::isolated::tor::connect_isolated_tor_with_inbound;

#[cfg(any(test, feature = "proptest-impl"))]
pub use crate::isolated::{
connect_isolated_tcp_direct_with_inbound, connect_isolated_with_inbound,
pub use crate::{
isolated::{connect_isolated_tcp_direct_with_inbound, connect_isolated_with_inbound},
protocol::external::canonical_peer_addr,
};

pub use crate::{
address_book::AddressBook,
address_book_peers::AddressBookPeers,
config::Config,
isolated::{connect_isolated, connect_isolated_tcp_direct},
meta_addr::PeerAddrState,
meta_addr::{PeerAddrState, PeerSocketAddr},
peer::{Client, ConnectedAddr, ConnectionInfo, HandshakeError, PeerError, SharedPeerError},
peer_set::init,
policies::RetryLimit,
Expand Down
Loading