Minor refactoring on channel_get_remote_addr_if_possible()

* We no longer call this an optional method
* We document that it returns the real address, not a canonical one.
* We have it try harder if the real address hasn't been set yet.

4 files changed, 42 insertions, 23 deletions

diff --git a/src/core/or/channel.c b/src/core/or/channel.c
index 20ccf41306..deb27a4d78 100644
--- a/src/core/or/channel.c
+++ b/src/core/or/channel.c
@@ -2835,22 +2835,24 @@ channel_get_canonical_remote_descr,(channel_t *chan))
 }
 
 /**
- * Get remote address if possible.
+ * Get the remote address for this channel, if possible.
  *
  * Write the remote address out to a tor_addr_t if the underlying transport
  * supports this operation, and return 1.  Return 0 if the underlying transport
  * doesn't let us do this.
+ *
+ * Always returns the "real" address of the peer -- the one we're connected to
+ * on the internet.
  */
 MOCK_IMPL(int,
-channel_get_addr_if_possible,(const channel_t *chan, tor_addr_t *addr_out))
+channel_get_addr_if_possible,(const channel_t *chan,
+                              tor_addr_t *addr_out))
 {
   tor_assert(chan);
   tor_assert(addr_out);
+  tor_assert(chan->get_remote_addr);
 
-  if (chan->get_remote_addr)
-    return chan->get_remote_addr(chan, addr_out);
-  /* Else no support, method not implemented */
-  else return 0;
+  return chan->get_remote_addr(chan, addr_out);
 }
 
 /**
diff --git a/src/core/or/channel.h b/src/core/or/channel.h
index 5fe1fb9cc4..9aa59da2cb 100644
--- a/src/core/or/channel.h
+++ b/src/core/or/channel.h
@@ -329,11 +329,9 @@ struct channel_t {
    */
   double (*get_overhead_estimate)(channel_t *);
   /*
-   * Ask the underlying transport what the remote endpoint address is, in
-   * a tor_addr_t.  This is optional and subclasses may leave this NULL.
-   * If they implement it, they should write the address out to the
-   * provided tor_addr_t *, and return 1 if successful or 0 if no address
-   * available.
+   * Ask the underlying transport what the remote endpoint address is, in a
+   * tor_addr_t.  Write the address out to the provided tor_addr_t *, and
+   * return 1 if successful or 0 if no address available.
    */
   int (*get_remote_addr)(const channel_t *, tor_addr_t *);
   int (*get_transport_name)(channel_t *chan, char **transport_out);
diff --git a/src/core/or/channeltls.c b/src/core/or/channeltls.c
index e4dbdda7db..8273235efb 100644
--- a/src/core/or/channeltls.c
+++ b/src/core/or/channeltls.c
@@ -103,9 +103,8 @@ static void channel_tls_close_method(channel_t *chan);
 static const char * channel_tls_describe_transport_method(channel_t *chan);
 static void channel_tls_free_method(channel_t *chan);
 static double channel_tls_get_overhead_estimate_method(channel_t *chan);
-static int
-channel_tls_get_remote_addr_method(const channel_t *chan,
-                                   tor_addr_t *addr_out);
+static int channel_tls_get_remote_addr_method(const channel_t *chan,
+                                              tor_addr_t *addr_out);
 static int
 channel_tls_get_transport_name_method(channel_t *chan, char **transport_out);
 static const char *
@@ -512,24 +511,35 @@ channel_tls_get_overhead_estimate_method(channel_t *chan)
  * Get the remote address of a channel_tls_t.
  *
  * This implements the get_remote_addr method for channel_tls_t; copy the
- * remote endpoint of the channel to addr_out and return 1 (always
- * succeeds for this transport).
+ * remote endpoint of the channel to addr_out and return 1.  (Always
+ * succeeds if this channel is attached to an OR connection.)
+ *
+ * Always returns the real address of the peer, not the canonical address.
  */
 static int
-channel_tls_get_remote_addr_method(const channel_t *chan, tor_addr_t *addr_out)
+channel_tls_get_remote_addr_method(const channel_t *chan,
+                                   tor_addr_t *addr_out)
 {
-  int rv = 0;
   const channel_tls_t *tlschan = BASE_CHAN_TO_TLS((channel_t*) chan);
 
   tor_assert(tlschan);
   tor_assert(addr_out);
 
-  if (tlschan->conn) {
+  if (tlschan->conn == NULL) {
+    tor_addr_make_unspec(addr_out);
+    return 0;
+  }
+
+  if (! tor_addr_is_null(&tlschan->conn->real_addr)) {
+    /* They want the real address, and real_addr is set. */
     tor_addr_copy(addr_out, &(tlschan->conn->real_addr));
-    rv = 1;
-  } else tor_addr_make_unspec(addr_out);
+  } else {
+    /* We'll have to give them the nominal address, which hopefully has
+    * not been overwritten yet. */
+    tor_addr_copy(addr_out, &TO_CONN(tlschan->conn)->addr);
+  }
 
-  return rv;
+  return 1;
 }
 
 /**
diff --git a/src/test/test_channel.c b/src/test/test_channel.c
index 2b723b4a8d..efe195eac4 100644
--- a/src/test/test_channel.c
+++ b/src/test/test_channel.c
@@ -166,13 +166,21 @@ static const char *
 chan_test_get_remote_descr(channel_t *ch, int flags)
 {
   tt_assert(ch);
-  tt_int_op(flags & ~(GRD_FLAG_ORIGINAL | GRD_FLAG_ADDR_ONLY), OP_EQ, 0);
+  tt_int_op(flags & ~(GRD_FLAG_ORIGINAL), OP_EQ, 0);
 
  done:
   return "Fake channel for unit tests; no real endpoint";
 }
 
 static int
+chan_test_get_remote_addr(const channel_t *ch, tor_addr_t *out)
+{
+  (void)ch;
+  tor_addr_from_ipv4h(out, 0x7f000001);
+  return 1;
+}
+
+static int
 chan_test_num_cells_writeable(channel_t *ch)
 {
   tt_assert(ch);
@@ -269,6 +277,7 @@ new_fake_channel(void)
   chan->close = chan_test_close;
   chan->num_cells_writeable = chan_test_num_cells_writeable;
   chan->get_remote_descr = chan_test_get_remote_descr;
+  chan->get_remote_addr = chan_test_get_remote_addr;
   chan->write_packed_cell = chan_test_write_packed_cell;
   chan->write_var_cell = chan_test_write_var_cell;
   chan->state = CHANNEL_STATE_OPEN;