[tor-commits] [tor/master] hs-v3: Don't BUG() if descriptor is found on SOCKS connection retry

[nickm at torproject.org]

commit cec616a0c8ff060cb722e54342fd30aeab3ad285
Author: David Goulet <dgoulet at torproject.org>
Date:   Tue Dec 4 14:27:46 2018 -0500

    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 at torproject.org>
---
 changes/ticket28669        |  6 ++++++
 src/feature/hs/hs_client.c | 19 +++++++++++++------
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/changes/ticket28669 b/changes/ticket28669
new file mode 100644
index 000000000..32c6114ff
--- /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 22aacdfe9..5fded92fe 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