hs-v3: Don't BUG() if descriptor is found on SOCKS connection retry

When retrying all SOCKS connection because new directory information just
arrived, do not BUG() if a connection in state AP_CONN_STATE_RENDDESC_WAIT is
found to have a usable descriptor.

There is a rare case when this can happen as detailed in #28669 so the right
thing to do is put that connection back in circuit wait state so the
descriptor can be retried.

Fixes #28669

Signed-off-by: David Goulet <dgoulet@torproject.org>

2 files changed, 19 insertions, 6 deletions

diff --git a/changes/ticket28669 b/changes/ticket28669
new file mode 100644
index 0000000000..32c6114ffc
--- /dev/null
+++ b/changes/ticket28669
@@ -0,0 +1,6 @@
+  o Minor bugfix (hidden service v3, client):
+    - Avoid a BUG() stacktrace in case a SOCKS connection is found waiting for
+      the descriptor while we do have it in the cache. There is a rare case
+      when this can happen. Now, tor will recover and retry the descriptor.
+      Fixes bug 28669; bugfix on 0.3.2.4-alpha.
+    
diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c
index 22aacdfe99..5fded92fe3 100644
--- a/src/feature/hs/hs_client.c
+++ b/src/feature/hs/hs_client.c
@@ -296,12 +296,19 @@ retry_all_socks_conn_waiting_for_desc(void)
 
     /* Order a refetch in case it works this time. */
     status = hs_client_refetch_hsdesc(&edge_conn->hs_ident->identity_pk);
-    if (BUG(status == HS_CLIENT_FETCH_HAVE_DESC)) {
-      /* This case is unique because it can NOT happen in theory. Once we get
-       * a new descriptor, the HS client subsystem is notified immediately and
-       * the connections waiting for it are handled which means the state will
-       * change from renddesc wait state. Log this and continue to next
-       * connection. */
+    if (status == HS_CLIENT_FETCH_HAVE_DESC) {
+      /* This is a rare case where a SOCKS connection is in state waiting for
+       * a descriptor but we do have it in the cache.
+       *
+       * This can happen is tor comes back from suspend where it previously
+       * had the descriptor but the intro points were not usuable. Once it
+       * came back to life, the intro point failure cache was cleaned up and
+       * thus the descriptor became usable again leaving us in this code path.
+       *
+       * We'll mark the connection as waiting for a circuit so the descriptor
+       * can be retried. This is safe because a connection in state waiting
+       * for a descriptor can not be in the entry connection pending list. */
+      mark_conn_as_waiting_for_circuit(base_conn, approx_time());
       continue;
     }
     /* In the case of an error, either all SOCKS connections have been