From 8f1a71850c67db51262c52b7719475232df5a3a4 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Wed, 28 Jun 2023 17:14:08 +0100
Subject: [PATCH] tor-hsclient: Handle handshake completion error correctly

---
 crates/tor-hsclient/src/connect.rs | 16 ++++++++++------
 crates/tor-hsclient/src/err.rs     | 21 +++++++++++++++++++++
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/crates/tor-hsclient/src/connect.rs b/crates/tor-hsclient/src/connect.rs
index dd2b0b9ae..2d3c4382e 100644
--- a/crates/tor-hsclient/src/connect.rs
+++ b/crates/tor-hsclient/src/connect.rs
@@ -1187,12 +1187,16 @@ impl<'c, R: Runtime, M: MocksForConnect<R>> Context<'c, R, M> {
         // Try to complete the cryptographic handshake.
         let keygen = handshake_state
             .client_receive_rend(rend2_msg.handshake_info())
-            .map_err(into_internal!(
-                "ACTUALLY this is a protocol violation, make a better error" // TODO HS
-            ))?;
-        // TODO HS: make sure that we do the correct error recovery from the
-        // above error.  Either the onion service has failed, or the rendezvous
-        // point has misbehaved, or we have used the wrong handshake_state.
+            // If this goes wrong. either the onion service has mangled the crypto,
+            // or the rendezvous point has misbehaved (that that is possible is a protocol bug),
+            // or we have used the wrong handshake_state (let's assume that's not true).
+            //
+            // If this happens we'll go and try another RPT.
+            .map_err(|error| FAE::RendezvousCompletionHandshake {
+                error,
+                intro_index,
+                rend_pt: rend_pt.clone(),
+            })?;
 
         // TODO HS: Generate this more sensibly!
         let params = CircParameters::default();
diff --git a/crates/tor-hsclient/src/err.rs b/crates/tor-hsclient/src/err.rs
index ce2b4b583..6a767c189 100644
--- a/crates/tor-hsclient/src/err.rs
+++ b/crates/tor-hsclient/src/err.rs
@@ -255,6 +255,23 @@ pub enum FailedAttemptError {
         rend_pt: Redacted<RsaIdentity>,
     },
 
+    /// Error processing rendezvous completion
+    ///
+    /// This is might be the fault of the hidden service or the rendezvous point.
+    #[error("Rendezvous completion end-to-end crypto handshake failed")]
+    RendezvousCompletionHandshake {
+        /// What happened
+        #[source]
+        error: tor_proto::Error,
+
+        /// The index of the IPT in the list of IPTs in the descriptor
+        intro_index: IntroPtIndex,
+
+        /// Which relay did we choose for rendezvous point
+        // TODO #813 this should be Redacted<RelayDescription> or something
+        rend_pt: RendPtIdentityForError,
+    },
+
     /// Internal error
     #[error("{0}")]
     Bug(#[from] Bug),
@@ -272,6 +289,7 @@ impl FailedAttemptError {
         match self {
             FAE::UnusableIntro { intro_index, .. }
             | FAE::RendezvousCompletionCircuit { intro_index, .. }
+            | FAE::RendezvousCompletionHandshake { intro_index, .. }
             | FAE::RendezvousCompletionTimeout { intro_index, .. }
             | FAE::IntroductionCircuitObtain { intro_index, .. }
             | FAE::IntroductionExchange { intro_index, .. }
@@ -303,6 +321,8 @@ impl HasRetryTime for FailedAttemptError {
             FAE::RendezvousEstablishTimeout { .. }
             | FAE::RendezvousCompletionTimeout { .. }
             | FAE::IntroductionTimeout { .. } => RT::AfterWaiting,
+            // If service didn't cause this, it was the RPT, so prefer to try another RPT
+            FAE::RendezvousCompletionHandshake { error: _e, .. } => RT::Never,
             // Bespoke
             FAE::Bug(_) => RT::Never,
         }
@@ -397,6 +417,7 @@ impl HasKind for FailedAttemptError {
             FAE::RendezvousCircuitObtain { error, .. } => error.kind(),
             FAE::RendezvousEstablish { error, .. } => error.kind(),
             FAE::RendezvousCompletionCircuit { error, .. } => error.kind(),
+            FAE::RendezvousCompletionHandshake { error, .. } => error.kind(),
             FAE::RendezvousEstablishTimeout { .. } => EK::TorNetworkTimeout,
             FAE::IntroductionCircuitObtain { error, .. } => error.kind(),
             FAE::IntroductionExchange { error, .. } => error.kind(),