tor-proto: Add ClientCirc::start_conversation().

This will enable hidden services to send `RENDEZVOUS1` messages to the
`N`th hop of the circuit rather than the `N + 1`th virtual one (which
can only used after the client and service have completed the
introduction handshake).

This also deprecates `start_conversation_last_hop`.

Closes #959
This commit is contained in:
Gabriela Moldovan 2023-07-31 19:53:30 +01:00
parent c23e85270e
commit ed5a051ebe
No known key found for this signature in database
GPG Key ID: 3946E0ADE72BAC99
6 changed files with 37 additions and 22 deletions

View File

@ -1009,7 +1009,7 @@ impl<'c, R: Runtime, M: MocksForConnect<R>> Context<'c, R, M> {
.await .await
.map_err(handle_proto_error)?; .map_err(handle_proto_error)?;
// `start_conversation_last_hop` returns as soon as the control message has been sent. // `start_conversation` returns as soon as the control message has been sent.
// We need to obtain the RENDEZVOUS_ESTABLISHED message, which is "returned" via the oneshot. // We need to obtain the RENDEZVOUS_ESTABLISHED message, which is "returned" via the oneshot.
let _: RendezvousEstablished = rend_established_rx.recv(handle_proto_error).await?; let _: RendezvousEstablished = rend_established_rx.recv(handle_proto_error).await?;
@ -1337,7 +1337,7 @@ trait MocksForConnect<R>: Clone {
/// Mock for `HsCircPool` /// Mock for `HsCircPool`
/// ///
/// Methods start with `m_` to avoid the following problem: /// Methods start with `m_` to avoid the following problem:
/// `ClientCirc::start_conversation_last_hop` (say) means /// `ClientCirc::start_conversation` (say) means
/// to use the inherent method if one exists, /// to use the inherent method if one exists,
/// but will use a trait method if there isn't an inherent method. /// but will use a trait method if there isn't an inherent method.
/// ///
@ -1435,7 +1435,8 @@ impl MockableClientCirc for ClientCirc {
msg: Option<AnyRelayMsg>, msg: Option<AnyRelayMsg>,
reply_handler: impl MsgHandler + Send + 'static, reply_handler: impl MsgHandler + Send + 'static,
) -> tor_proto::Result<Self::Conversation<'_>> { ) -> tor_proto::Result<Self::Conversation<'_>> {
ClientCirc::start_conversation_last_hop(self, msg, reply_handler).await let last_hop = self.last_hop_num()?;
ClientCirc::start_conversation(self, msg, reply_handler, last_hop).await
} }
type Conversation<'r> = tor_proto::circuit::Conversation<'r>; type Conversation<'r> = tor_proto::circuit::Conversation<'r>;

View File

@ -42,7 +42,7 @@ hs-client = ["hs-common"]
hs-service = ["hs-common", "__is_experimental"] hs-service = ["hs-common", "__is_experimental"]
hs-common = ["tor-hscrypto"] hs-common = ["tor-hscrypto"]
experimental-api = ["__is_experimental"] experimental-api = ["__is_experimental"]
# start_conversation_last_hop etc.; TODO HS should be renamed # start_conversation etc.; TODO HS should be renamed
send-control-msg = ["visibility"] send-control-msg = ["visibility"]
stream-ctrl = ["__is_experimental"] stream-ctrl = ["__is_experimental"]
# Enable testing-only APIs. APIs under this feature are not # Enable testing-only APIs. APIs under this feature are not

View File

@ -8,3 +8,6 @@ BREAKING: `ClientCirc::allow_stream_requests` now expects `self` to be
`&Arc<ClientCirc>` `&Arc<ClientCirc>`
ADDED: `HopNum` is now public ADDED: `HopNum` is now public
ADDED: `ClientCirc::last_hop_num` ADDED: `ClientCirc::last_hop_num`
DEPRECATED: `ClientCirc::start_conversation_last_hop()`
ADDED: `ClientCirc::start_conversation()` to eventually replace
`ClientCirc::start_conversation_last_hop()`

View File

@ -339,7 +339,7 @@ impl ClientCirc {
&self.channel &self.channel
} }
/// Start an ad-hoc protocol exchange to the final hop on this circuit /// Start an ad-hoc protocol exchange to the specified hop on this circuit
/// ///
/// To use this: /// To use this:
/// ///
@ -347,7 +347,7 @@ impl ClientCirc {
/// the outcome of your conversation, /// the outcome of your conversation,
/// and bundle it into a [`MsgHandler`]. /// and bundle it into a [`MsgHandler`].
/// ///
/// 1. Call `start_conversation_last_hop`. /// 1. Call `start_conversation`.
/// This will install a your handler, for incoming messages, /// This will install a your handler, for incoming messages,
/// and send the outgoing message (if you provided one). /// and send the outgoing message (if you provided one).
/// After that, each message on the circuit /// After that, each message on the circuit
@ -355,7 +355,7 @@ impl ClientCirc {
/// is passed to your provided `reply_handler`. /// is passed to your provided `reply_handler`.
/// ///
/// 2. Possibly call `send_msg` on the [`Conversation`], /// 2. Possibly call `send_msg` on the [`Conversation`],
/// from the call site of `start_conversation_last_hop`, /// from the call site of `start_conversation`,
/// possibly multiple times, from time to time, /// possibly multiple times, from time to time,
/// to send further desired messages to the peer. /// to send further desired messages to the peer.
/// ///
@ -390,7 +390,7 @@ impl ClientCirc {
/// while the conversation is in progress. /// while the conversation is in progress.
/// ///
/// After the conversation has finished, the circuit may be extended. /// After the conversation has finished, the circuit may be extended.
/// Or, `start_conversation_last_hop` may be called again; /// Or, `start_conversation` may be called again;
/// but, in that case there will be a gap between the two conversations, /// but, in that case there will be a gap between the two conversations,
/// during which no `MsgHandler` is installed, /// during which no `MsgHandler` is installed,
/// and unexpected incoming messages would close the circuit. /// and unexpected incoming messages would close the circuit.
@ -399,7 +399,7 @@ impl ClientCirc {
/// ///
/// ## Precise definition of the lifetime of a conversation /// ## Precise definition of the lifetime of a conversation
/// ///
/// A conversation is in progress from entry to `start_conversation_last_hop`, /// A conversation is in progress from entry to `start_conversation`,
/// until entry to the body of the [`MsgHandler::handle_msg`] /// until entry to the body of the [`MsgHandler::handle_msg`]
/// call which returns [`ConversationFinished`](MetaCellDisposition::ConversationFinished). /// call which returns [`ConversationFinished`](MetaCellDisposition::ConversationFinished).
/// (*Entry* since `handle_msg` is synchronously embedded /// (*Entry* since `handle_msg` is synchronously embedded
@ -415,11 +415,24 @@ impl ClientCirc {
// //
// TODO hs: it might be nice to avoid exposing tor-cell APIs in the // TODO hs: it might be nice to avoid exposing tor-cell APIs in the
// tor-proto interface. // tor-proto interface.
//
// TODO hs: Possibly this function should use
// HopNum or similar to indicate which hop we're talking to, rather than
// just doing "the last hop".
#[cfg(feature = "send-control-msg")] #[cfg(feature = "send-control-msg")]
pub async fn start_conversation(
&self,
msg: Option<tor_cell::relaycell::msg::AnyRelayMsg>,
reply_handler: impl MsgHandler + Send + 'static,
hop_num: HopNum,
) -> Result<Conversation<'_>> {
let handler = Box::new(msghandler::UserMsgHandler::new(hop_num, reply_handler));
let conversation = Conversation(self);
conversation.send_internal(msg, Some(handler)).await?;
Ok(conversation)
}
/// Start an ad-hoc protocol exchange to the final hop on this circuit
///
/// See the [`ClientCirc::start_conversation`] docs for more information.
#[cfg(feature = "send-control-msg")]
#[deprecated(since = "0.13.0", note = "Use start_conversation instead.")]
pub async fn start_conversation_last_hop( pub async fn start_conversation_last_hop(
&self, &self,
msg: Option<tor_cell::relaycell::msg::AnyRelayMsg>, msg: Option<tor_cell::relaycell::msg::AnyRelayMsg>,
@ -432,10 +445,8 @@ impl ClientCirc {
.path .path
.last_hop_num() .last_hop_num()
.ok_or_else(|| internal!("no last hop index"))?; .ok_or_else(|| internal!("no last hop index"))?;
let handler = Box::new(msghandler::UserMsgHandler::new(last_hop, reply_handler));
let conversation = Conversation(self); self.start_conversation(msg, reply_handler, last_hop).await
conversation.send_internal(msg, Some(handler)).await?;
Ok(conversation)
} }
/// Tell this circuit to begin allowing the final hop of the circuit to try /// Tell this circuit to begin allowing the final hop of the circuit to try
@ -795,7 +806,7 @@ impl ClientCirc {
/// Handle to use during an ongoing protocol exchange with a circuit's last hop /// Handle to use during an ongoing protocol exchange with a circuit's last hop
/// ///
/// This is obtained from [`ClientCirc::start_conversation_last_hop`], /// This is obtained from [`ClientCirc::start_conversation`],
/// and used to send messages to the last hop relay. /// and used to send messages to the last hop relay.
/// ///
/// See also [`ConversationInHandler`], which is a type used for the same purpose /// See also [`ConversationInHandler`], which is a type used for the same purpose
@ -817,7 +828,7 @@ impl Conversation<'_> {
/// Send a `SendMsgAndInstallHandler` to the reactor and wait for the outcome /// Send a `SendMsgAndInstallHandler` to the reactor and wait for the outcome
/// ///
/// The guts of `start_conversation_last_hop` and `Conversation::send_msg` /// The guts of `start_conversation` and `Conversation::send_msg`
async fn send_internal( async fn send_internal(
&self, &self,
msg: Option<tor_cell::relaycell::msg::AnyRelayMsg>, msg: Option<tor_cell::relaycell::msg::AnyRelayMsg>,

View File

@ -1,5 +1,5 @@
//! A message handler trait for use with //! A message handler trait for use with
//! [`ClientCirc::start_conversation_last_hop`](super::ClientCirc::start_conversation_last_hop). //! [`ClientCirc::start_conversation`](super::ClientCirc::start_conversation).
//! //!
//! Although this is similar to `stream::cmdcheck`, I am deliberately leaving //! Although this is similar to `stream::cmdcheck`, I am deliberately leaving
//! them separate. Conceivably they should be unified at some point down the //! them separate. Conceivably they should be unified at some point down the
@ -18,7 +18,7 @@ use super::{ConversationInHandler, MetaCellDisposition};
/// circuit, and delivers them to a client if so. /// circuit, and delivers them to a client if so.
/// ///
/// The handler is supplied to /// The handler is supplied to
/// [`ClientCirc::start_conversation_last_hop`](super::ClientCirc::start_conversation_last_hop). It /// [`ClientCirc::start_conversation`](super::ClientCirc::start_conversation). It
/// is used to check any incoming message whose stream ID is 0, and which would /// is used to check any incoming message whose stream ID is 0, and which would
/// otherwise not be accepted on a given circuit. /// otherwise not be accepted on a given circuit.
/// ///

View File

@ -287,7 +287,7 @@ impl CircHop {
/// This is passed to `MsgHandler::handle_msg`. /// This is passed to `MsgHandler::handle_msg`.
/// ///
/// See also [`ConversationInHandler`], which is a type used for the same purpose /// See also [`ConversationInHandler`], which is a type used for the same purpose
/// but available to the caller of `start_conversation_last_hop` /// but available to the caller of `start_conversation`
// //
// This is the subset of the arguments to MetaCellHandler::handle_msg // This is the subset of the arguments to MetaCellHandler::handle_msg
// which are needed to be able to call send_relay_cell. // which are needed to be able to call send_relay_cell.