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

feat: verify expected PeerId as part of security handshake #4864

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
6748e17
feat: MVP verification of the expected `PeerId` as part of a protocol…
denis2glez Nov 14, 2023
dbedc1d
chore: fix a typo in the doc comments
denis2glez Nov 15, 2023
d800320
fix: introduce `Builder::authenticate2` as an alternative code path
denis2glez Nov 16, 2023
af48270
docs: improve comments related to `Builder::authenticate2`
denis2glez Nov 16, 2023
31c7e61
fix: add `Send` constraint to `Builder::authenticate2` parameters
denis2glez Nov 17, 2023
5c5b728
refactor: split `SecurityUpgrade` into two alternative traits
denis2glez Nov 17, 2023
b686801
docs: mark items related to `Builder::authenticate2` as `#[doc(hidden)]`
denis2glez Nov 17, 2023
c0fad3a
feat: impl `InboundSecurityUpgrade`/`OutboundSecurityUpgrade` for TLS
denis2glez Nov 17, 2023
399666b
feat: impl `InboundSecurityUpgrade`/`OutboundSecurityUpgrade` for Noise
denis2glez Nov 17, 2023
71e0ac8
chore: minor doc comments fixes
denis2glez Nov 19, 2023
5500663
refactor: remove the `PeerId` parameter in `InboundSecurityUpgrade`
denis2glez Nov 19, 2023
a726083
docs: fix several issues in the comments
denis2glez Nov 22, 2023
514b220
refactor: delegate `upgrade_inbound` and `upgrade_outbound` methods
denis2glez Nov 22, 2023
66df049
fix: references to actual and remote `PeerId` respectively
denis2glez Nov 22, 2023
d8440e7
chore: keep grouping of module and use declarations
denis2glez Nov 24, 2023
c386182
fix: make client and server config ad-hoc in `secure_outbound` method
denis2glez Nov 24, 2023
3eb5f1f
fix: use the wording of `actual` and `expected` PeerId
denis2glez Nov 27, 2023
2ede11c
refactor: store the `identity::Keypair` within `Config`
denis2glez Nov 27, 2023
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
87 changes: 84 additions & 3 deletions core/src/transport/upgrade.rs
denis2glez marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,11 @@ use crate::{
TransportError, TransportEvent,
},
upgrade::{
self, apply_inbound, apply_outbound, InboundConnectionUpgrade, InboundUpgradeApply,
OutboundConnectionUpgrade, OutboundUpgradeApply, UpgradeError,
self, apply_inbound, apply_outbound, EitherSecurityFuture, InboundConnectionUpgrade,
InboundSecurityUpgrade, InboundUpgradeApply, OutboundConnectionUpgrade,
OutboundSecurityUpgrade, OutboundUpgradeApply, UpgradeError,
},
Negotiated,
Negotiated, UpgradeInfo,
};
use futures::{prelude::*, ready};
use libp2p_identity::PeerId;
Expand Down Expand Up @@ -113,6 +114,53 @@ where
version,
))
}

/// Upgrades the transport to perform authentication of the remote.
///
/// The supplied upgrade receives the I/O resource `C` and must
/// produce a pair `(PeerId, D)`, where `D` is a new I/O resource.
/// The upgrade must thus at a minimum identify the remote, which typically
/// involves the use of a cryptographic authentication protocol in the
/// context of establishing a secure channel.
///
/// ## Transitions
///
/// * I/O upgrade: `C -> (PeerId, D)`.
/// * Transport output: `C -> (PeerId, D)`
///
/// ## Alternative state
///
/// This function is an alternative code path to [`Builder::authenticate`]
/// given the supplied upgrade implements [`InboundSecurityUpgrade`]/[`OutboundSecurityUpgrade`]
/// instead of [`InboundConnectionUpgrade`]/[`OutboundConnectionUpgrade`].
#[doc(hidden)]
pub fn authenticate2<C, D, U, E>(
denis2glez marked this conversation as resolved.
Show resolved Hide resolved
self,
upgrade: U,
) -> Authenticated<AndThen<T, impl FnOnce(C, ConnectedPoint) -> Authenticate2<C, U> + Clone>>
where
T: Transport<Output = C>,
C: AsyncRead + AsyncWrite + Unpin + Send + 'static,
D: AsyncRead + AsyncWrite + Unpin,
U: InboundSecurityUpgrade<Negotiated<C>, Output = (PeerId, D), Error = E> + Send + 'static,
U: OutboundSecurityUpgrade<Negotiated<C>, Output = (PeerId, D), Error = E>
+ Clone
+ Send
+ 'static,
E: Error + 'static,
<U as UpgradeInfo>::Info: Send,
<U as InboundSecurityUpgrade<Negotiated<C>>>::Future: Send,
<U as OutboundSecurityUpgrade<Negotiated<C>>>::Future: Send,
<<U as UpgradeInfo>::InfoIter as std::iter::IntoIterator>::IntoIter: Send,
{
let version = self.version;
Authenticated(Builder::new(
self.inner.and_then(move |conn, endpoint| Authenticate2 {
inner: upgrade::secure(conn, upgrade, endpoint, version),
}),
version,
))
}
}

/// An upgrade that authenticates the remote peer, typically
Expand Down Expand Up @@ -147,6 +195,39 @@ where
}
}

/// An upgrade that authenticates the remote peer, typically
/// in the context of negotiating a secure channel.
///
/// Configured through [`Builder::authenticate2`].
#[doc(hidden)]
#[pin_project::pin_project]
pub struct Authenticate2<C, U>
where
C: AsyncRead + AsyncWrite + Unpin,
U: InboundSecurityUpgrade<Negotiated<C>> + OutboundSecurityUpgrade<Negotiated<C>>,
{
#[pin]
inner: EitherSecurityFuture<C, U>,
}

impl<C, U> Future for Authenticate2<C, U>
where
C: AsyncRead + AsyncWrite + Unpin,
U: InboundSecurityUpgrade<Negotiated<C>>
+ OutboundSecurityUpgrade<
Negotiated<C>,
Output = <U as InboundSecurityUpgrade<Negotiated<C>>>::Output,
Error = <U as InboundSecurityUpgrade<Negotiated<C>>>::Error,
>,
{
type Output = <EitherSecurityFuture<C, U> as Future>::Output;

fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
let this = self.project();
Future::poll(this.inner, cx)
}
}

/// An upgrade that negotiates a (sub)stream multiplexer on
/// top of an authenticated transport.
///
Expand Down
43 changes: 43 additions & 0 deletions core/src/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,17 @@ mod either;
mod error;
mod pending;
mod ready;
mod secure;
mod select;

pub(crate) use apply::{
apply, apply_inbound, apply_outbound, InboundUpgradeApply, OutboundUpgradeApply,
};
pub(crate) use error::UpgradeError;
pub(crate) use secure::{secure, EitherSecurityFuture};

use futures::future::Future;
use libp2p_identity::PeerId;

pub use self::{
denied::DeniedUpgrade, pending::PendingUpgrade, ready::ReadyUpgrade, select::SelectUpgrade,
Expand Down Expand Up @@ -152,3 +156,42 @@ pub trait OutboundConnectionUpgrade<T>: UpgradeInfo {
/// The `info` is the identifier of the protocol, as produced by `protocol_info`.
fn upgrade_outbound(self, socket: T, info: Self::Info) -> Self::Future;
}

/// Possible security upgrade on an inbound connection
pub trait InboundSecurityUpgrade<T>: UpgradeInfo {
/// Output after the upgrade has been successfully negotiated and the handshake performed.
type Output;
/// Possible error during the handshake.
type Error;
/// Future that performs the handshake with the remote.
type Future: Future<Output = Result<(PeerId, Self::Output), Self::Error>>;

/// After we have determined that the remote supports one of the protocols we support, this
/// method is called to start the handshake.
///
/// The `info` is the identifier of the protocol, as produced by `protocol_info`.
fn secure_inbound(self, socket: T, info: Self::Info) -> Self::Future;
}

/// Possible security upgrade on an outbound connection
pub trait OutboundSecurityUpgrade<T>: UpgradeInfo {
/// Output after the upgrade has been successfully negotiated and the handshake performed.
type Output;
/// Possible error during the handshake.
type Error;
/// Future that performs the handshake with the remote.
type Future: Future<Output = Result<(PeerId, Self::Output), Self::Error>>;

/// After we have determined that the remote supports one of the protocols we support, this
/// method is called to start the handshake.
///
/// The `info` is the identifier of the protocol, as produced by `protocol_info`. Security
/// transports use the optional `expected_peer_id` parameter on outgoing upgrades to validate
/// the expected `PeerId`.
fn secure_outbound(
self,
socket: T,
info: Self::Info,
expected_peer_id: Option<PeerId>,
) -> Self::Future;
}
107 changes: 107 additions & 0 deletions core/src/upgrade/secure.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
//! After a successful protocol negotiation as part of the upgrade process, the [`InboundSecurityUpgrade::secure_inbound`]
//! or [`OutboundSecurityUpgrade::secure_outbound`] method is called and a [`Future`] that performs a
//! handshake is returned.

use std::iter::IntoIterator;

use crate::upgrade::UpgradeError;
use crate::UpgradeInfo;
use crate::{connection::ConnectedPoint, Negotiated};
use futures::future::{BoxFuture, Either};
use futures::prelude::*;

use libp2p_identity::PeerId;
use multiaddr::Protocol;
use multistream_select::Version;

use super::{InboundSecurityUpgrade, OutboundSecurityUpgrade};

/// An inbound or outbound security upgrade.
pub(crate) type EitherSecurityFuture<C, U> =
Either<InboundSecurityFuture<C, U>, OutboundSecurityFuture<C, U>>;

/// An inbound security upgrade represented by an owned trait object `Future`.
pub(crate) type InboundSecurityFuture<C, U> = BoxFuture<
'static,
Result<
(PeerId, <U as InboundSecurityUpgrade<Negotiated<C>>>::Output),
UpgradeError<<U as InboundSecurityUpgrade<Negotiated<C>>>::Error>,
>,
>;

/// An outbound security upgrade represented by an owned trait object `Future`.
pub(crate) type OutboundSecurityFuture<C, U> = BoxFuture<
'static,
Result<
(
PeerId,
<U as OutboundSecurityUpgrade<Negotiated<C>>>::Output,
),
UpgradeError<<U as OutboundSecurityUpgrade<Negotiated<C>>>::Error>,
>,
>;

/// Applies a security upgrade to the inbound and outbound direction of a connection or substream.
pub(crate) fn secure<C, U>(
conn: C,
up: U,
cp: ConnectedPoint,
v: Version,
) -> EitherSecurityFuture<C, U>
where
C: AsyncRead + AsyncWrite + Unpin + Send + 'static,
U: InboundSecurityUpgrade<Negotiated<C>>
+ OutboundSecurityUpgrade<Negotiated<C>>
+ Send
+ 'static,
<U as UpgradeInfo>::Info: Send,
<U as InboundSecurityUpgrade<Negotiated<C>>>::Future: Send,
<U as OutboundSecurityUpgrade<Negotiated<C>>>::Future: Send,
<<U as UpgradeInfo>::InfoIter as IntoIterator>::IntoIter: Send,
{
match cp {
ConnectedPoint::Dialer { role_override, .. } if role_override.is_dialer() => Either::Right(
async move {

Check failure on line 64 in core/src/upgrade/secure.rs

View workflow job for this annotation

GitHub Actions / clippy (nightly-2023-09-10)

future cannot be sent between threads safely

Check failure on line 64 in core/src/upgrade/secure.rs

View workflow job for this annotation

GitHub Actions / clippy (nightly-2023-09-10)

future cannot be sent between threads safely

Check failure on line 64 in core/src/upgrade/secure.rs

View workflow job for this annotation

GitHub Actions / Compile with MSRV

future cannot be sent between threads safely

Check failure on line 64 in core/src/upgrade/secure.rs

View workflow job for this annotation

GitHub Actions / Compile with MSRV

future cannot be sent between threads safely
let peer_id = cp
.get_remote_address()
.iter()
.find_map(|protocol| match protocol {
Protocol::P2p(peer_id) => Some(peer_id),
_ => None,
});
let (info, stream) =
multistream_select::dialer_select_proto(conn, up.protocol_info(), v).await?;
let name = info.as_ref().to_owned();
match up.secure_outbound(stream, info, peer_id).await {
Ok(x) => {
tracing::trace!(up=%name, "Secured outbound stream");
Ok(x)
}
Err(e) => {
tracing::trace!(up=%name, "Failed to secure outbound stream");
Err(UpgradeError::Apply(e))
}
}
}
.boxed(),
),
_ => Either::Left(
async move {

Check failure on line 89 in core/src/upgrade/secure.rs

View workflow job for this annotation

GitHub Actions / clippy (nightly-2023-09-10)

future cannot be sent between threads safely

Check failure on line 89 in core/src/upgrade/secure.rs

View workflow job for this annotation

GitHub Actions / clippy (nightly-2023-09-10)

future cannot be sent between threads safely

Check failure on line 89 in core/src/upgrade/secure.rs

View workflow job for this annotation

GitHub Actions / Compile with MSRV

future cannot be sent between threads safely

Check failure on line 89 in core/src/upgrade/secure.rs

View workflow job for this annotation

GitHub Actions / Compile with MSRV

future cannot be sent between threads safely
let (info, stream) =
multistream_select::listener_select_proto(conn, up.protocol_info()).await?;
let name = info.as_ref().to_owned();
match up.secure_inbound(stream, info).await {
Ok(x) => {
tracing::trace!(up=%name, "Secured inbound stream");
Ok(x)
}
Err(e) => {
tracing::trace!(up=%name, "Failed to secure inbound stream");
Err(UpgradeError::Apply(e))
}
}
}
.boxed(),
),
}
}
68 changes: 59 additions & 9 deletions transports/noise/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,12 @@ pub use io::Output;
use crate::handshake::State;
use crate::io::handshake;
use crate::protocol::{noise_params_into_builder, AuthenticKeypair, Keypair, PARAMS_XX};
use futures::future::BoxFuture;
use futures::prelude::*;
use libp2p_core::upgrade::{InboundConnectionUpgrade, OutboundConnectionUpgrade};
use libp2p_core::upgrade::{
InboundConnectionUpgrade, InboundSecurityUpgrade, OutboundConnectionUpgrade,
OutboundSecurityUpgrade,
};
use libp2p_core::UpgradeInfo;
use libp2p_identity as identity;
use libp2p_identity::PeerId;
Expand Down Expand Up @@ -179,7 +183,33 @@ where
type Error = Error;
type Future = Pin<Box<dyn Future<Output = Result<Self::Output, Self::Error>> + Send>>;

fn upgrade_inbound(self, socket: T, _: Self::Info) -> Self::Future {
fn upgrade_inbound(self, socket: T, info: Self::Info) -> Self::Future {
InboundSecurityUpgrade::secure_inbound(self, socket, info)
}
}

impl<T> OutboundConnectionUpgrade<T> for Config
where
T: AsyncRead + AsyncWrite + Unpin + Send + 'static,
{
type Output = (PeerId, Output<T>);
type Error = Error;
type Future = Pin<Box<dyn Future<Output = Result<Self::Output, Self::Error>> + Send>>;

fn upgrade_outbound(self, socket: T, info: Self::Info) -> Self::Future {
OutboundSecurityUpgrade::secure_outbound(self, socket, info, None)
}
}

impl<T> InboundSecurityUpgrade<T> for Config
where
T: AsyncRead + AsyncWrite + Unpin + Send + 'static,
{
type Output = Output<T>;
type Error = Error;
type Future = BoxFuture<'static, Result<(PeerId, Self::Output), Self::Error>>;

fn secure_inbound(self, socket: T, _: Self::Info) -> Self::Future {
async move {
let mut state = self.into_responder(socket)?;

Expand All @@ -189,21 +219,27 @@ where

let (pk, io) = state.finish()?;

Ok((pk.to_peer_id(), io))
let peer_id = pk.to_peer_id();
Ok((peer_id, io))
}
.boxed()
}
}

impl<T> OutboundConnectionUpgrade<T> for Config
impl<T> OutboundSecurityUpgrade<T> for Config
where
T: AsyncRead + AsyncWrite + Unpin + Send + 'static,
{
type Output = (PeerId, Output<T>);
type Output = Output<T>;
type Error = Error;
type Future = Pin<Box<dyn Future<Output = Result<Self::Output, Self::Error>> + Send>>;

fn upgrade_outbound(self, socket: T, _: Self::Info) -> Self::Future {
type Future = BoxFuture<'static, Result<(PeerId, Self::Output), Self::Error>>;

fn secure_outbound(
self,
socket: T,
_: Self::Info,
expected_peer_id: Option<PeerId>,
) -> Self::Future {
async move {
let mut state = self.into_initiator(socket)?;

Expand All @@ -213,7 +249,16 @@ where

let (pk, io) = state.finish()?;

Ok((pk.to_peer_id(), io))
let peer_id = pk.to_peer_id();
match expected_peer_id {
Some(expected_peer_id) if expected_peer_id != peer_id => {
Err(Error::PeerIdMismatch {
peer_id,
expected_peer_id,
})
}
_ => Ok((peer_id, io)),
}
}
.boxed()
}
Expand Down Expand Up @@ -243,6 +288,11 @@ pub enum Error {
SigningError(#[from] libp2p_identity::SigningError),
#[error("Expected WebTransport certhashes ({}) are not a subset of received ones ({})", certhashes_to_string(.0), certhashes_to_string(.1))]
UnknownWebTransportCerthashes(HashSet<Multihash<64>>, HashSet<Multihash<64>>),
#[error("Invalid peer ID (actual {peer_id:?}, expected {expected_peer_id:?})")]
PeerIdMismatch {
peer_id: PeerId,
expected_peer_id: PeerId,
},
}

#[derive(Debug, thiserror::Error)]
Expand Down
Loading
Loading