Opened 7 years ago

Closed 7 years ago

#7112 closed defect (fixed)

tor_addr_is_internal_() - non-IP address of type 0

Reported by: mazda Owned by:
Priority: High Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor_addr_is_internal
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Configure options:
nfigure --enable-static-tor --with-libevent-dir=/usr/lib --with-openssl-dir=/usr/local/src/openssl-1.0.1c/ --with-zlib-dir=/usr/local/src/zlib-1.2.7

Getting this error in the syslog:
Tor[8400]: tor_addr_is_internal_(): Bug: tor_addr_is_internal() called from src/or/config.c:1991 with a non-IP address of type 0

Startup log:
Oct 14 15:35:27 faravahar Tor[8400]: Using OpenSSL engine RSAX engine support [rsax] for RSA
Oct 14 15:35:27 faravahar Tor[8400]: Your Tor server's identity key fingerprint is 'Faravahar CF6D0AAFB385BE71B8E111FC5CFF4B47923733BC'
Oct 14 15:35:27 faravahar Tor[8400]: Parsing GEOIP file /usr/local/share/tor/geoip.
Oct 14 15:35:27 faravahar Tor[8400]: Configured to measure statistics. Look for the *-stats files that will first be written to the data directory in 24 hours from now.
Oct 14 15:35:28 faravahar Tor[8400]: Choosing expected valid-after time as 2012-10-14 16:00:00: consensus_set=1, interval=3600
Oct 14 15:35:28 faravahar Tor[8400]: This version of Tor (0.2.4.3-alpha-dev) is newer than any recommended version, according to the directory authorities. Recommended versions are: 0.2.2.39,0.2.3.22-rc,0.2.4.3-alpha
Oct 14 15:35:31 faravahar Tor[8400]: We now have enough directory information to build circuits.
Oct 14 15:35:31 faravahar Tor[8400]: Bootstrapped 80%: Connecting to the Tor network.
Oct 14 15:35:31 faravahar Tor[8400]: Bootstrapped 85%: Finishing handshake with first hop.
Oct 14 15:35:45 faravahar Tor[8400]: All routers are down or won't exit -- choosing a doomed exit at random.
Oct 14 15:35:45 faravahar Tor[8400]: failed to choose an exit server
Oct 14 15:35:57 faravahar Tor[8400]: Bootstrapped 90%: Establishing a Tor circuit.
Oct 14 15:36:17 faravahar Tor[8400]: Self-testing indicates your ORPort is reachable from the outside. Excellent. Publishing server descriptor.
Oct 14 15:36:18 faravahar Tor[8400]: Tor has successfully opened a circuit. Looks like client functionality is working.
Oct 14 15:36:18 faravahar Tor[8400]: Bootstrapped 100%: Done.
Oct 14 15:36:18 faravahar Tor[8400]: tor_addr_is_internal_(): Bug: tor_addr_is_internal() called from src/or/config.c:1991 with a non-IP address of type 0
Oct 14 15:36:50 Tor[8400]: last message repeated 10 times
Oct 14 15:36:50 faravahar Tor[8400]: Self-testing indicates your DirPort is reachable from the outside. Excellent.
Oct 14 15:36:52 faravahar Tor[8400]: tor_addr_is_internal_(): Bug: tor_addr_is_internal() called from src/or/config.c:1991 with a non-IP address of type 0
Oct 14 15:38:00 Tor[8400]: last message repeated 14 times
Oct 14 15:39:05 Tor[8400]: last message repeated 11 times
Oct 14 15:39:51 Tor[8400]: last message repeated 6 times
Oct 14 15:39:51 faravahar Tor[8400]: Performing bandwidth self-test...done.
Oct 14 15:39:58 faravahar Tor[8400]: tor_addr_is_internal_(): Bug: tor_addr_is_internal() called from src/or/config.c:1991 with a non-IP address of type 0
Oct 14 15:41:05 Tor[8400]: last message repeated 12 times
Oct 14 15:42:05 Tor[8400]: last message repeated 5 times
Oct 14 15:43:05 Tor[8400]: last message repeated 16 times
Oct 14 15:44:05 Tor[8400]: last message repeated 12 times
Oct 14 15:45:05 Tor[8400]: last message repeated 11 times
Oct 14 15:46:05 Tor[8400]: last message repeated 6 times
Oct 14 15:47:05 Tor[8400]: last message repeated 10 times
Oct 14 15:48:06 Tor[8400]: last message repeated 15 times
Oct 14 15:49:06 Tor[8400]: last message repeated 8 times
Oct 14 15:50:01 Tor[8400]: last message repeated 11 times

Child Tickets

Change History (21)

comment:1 Changed 7 years ago by arma

There are some hints on #7086. I'm going to close that bug and we can use this one as the ticket.

comment:2 Changed 7 years ago by arma

Milestone: Tor: 0.2.4.x-final
Priority: normalmajor

comment:3 Changed 7 years ago by arma

Version: Tor: 0.2.4.3-alpha

(the bug is on master, not 0.2.4.3-alpha)

comment:4 Changed 7 years ago by andrea

This gets triggered when we have an anonymized directory connection linked to an exit connection, as created in connection_exit_connect_dir(); write_http_response_header_impl() then calls is_local_addr() on a zeroed-out address field and emits the warning. Best fix is to track which server-side dir_connection_ts are anonymous and avoid calling is_local_addr() on them - the is_local_dir() test controls writing an X-Your-Address-Is HTTP header, and that's not meaningful anyway in this case.

There's already a dirconn_direct field which gets set to false on the client side in the anonymous directory connection case, but which as far as I can tell is always zero on the server side. It has a bunch of associated comments about that being a bad name. I propose renaming it to dirconn_anonymous, inverting the sense in the client case, and setting it on the server side from connection_exit_connect_dir(). Then we can test it in write_http_response_header_impl() and avoid calling is_local_addr().

comment:5 Changed 7 years ago by andrea

Status: newneeds_review

Fix implemented in my bug7112 branch.

comment:6 Changed 7 years ago by nickm

"anonymous" isn't a great name either, though. Linked directory connections *can* be anonymous (as in the case of hidden service directory operations) or they can be non-anonymous (as in the case of the connections we make over one-hop circuits when fetching directory information directly.)

I think maybe "via_tor" is better?

comment:7 Changed 7 years ago by nickm

oh duh forget that.

It *is* a matter of being anonymized or not

comment:8 Changed 7 years ago by nickm

But connection_exit_connect_dir() appears to be called for _all_ connections received over a begindir stream, which includes both anonymized and non-anonymized connections. So that's not right.

comment:9 Changed 7 years ago by nickm

I seriously wonder why we can't just copy the address over in connection_exit_connect_dir().

comment:10 Changed 7 years ago by nickm

Aha. We do copy the address over... but only conn->address, and not the conn->addr field.

Okay, I've make a slightly less bold approach based on your analysis of the bug. Instead of suppressing the output on all tunneled directory connections, I've decided to include it on all tunneled directory connections, and trust that the client's use of the still-misnamed direct_connection field will make the client ignore that field on anonymized tunneled connections.

This approach is "slightly less bold" because it leaves the addr field of the directory connection alone, and instead finds out the right value to report just as it is about to report the value. That way if anything is relying on these addr fields being null, it won't break.

The computation for "the right address" is a little tricky, since it isn't in dir_conn, and it isn't in the linked exit connection either. No, for the apparent peer address, we need to ask the channel.

See branch 'bug7112_v2' in my public repository -- does that seem right?

One remaining mystery is why this warning appeared about when we merged the channel code; I don't see any change that would make it start happening.

comment:11 in reply to:  10 Changed 7 years ago by nickm

Status: needs_reviewneeds_revision

Replying to nickm:

One remaining mystery is why this warning appeared about when we merged the channel code; I don't see any change that would make it start happening.

Oh hey. Instead of copying the circuit's or_connection's address into the edge_connection's address in connection_exit_begin_conn, we instead assign the output of channel_get_actual_remote_descr(). But that includes a port. I wonder if something else was parsing that...

And hey, check out this part of 4768c0efe3e9471cc367c3740d1a4ba0ab79626c:

   if (rh.command == RELAY_COMMAND_BEGIN_DIR) {
     tor_assert(or_circ);
-    if (or_circ->p_conn && !tor_addr_is_null(&or_circ->p_conn->real_addr))
-      tor_addr_copy(&n_stream->_base.addr, &or_circ->p_conn->real_addr);
     return connection_exit_connect_dir(n_stream);
   }

There's the bug!

comment:12 Changed 7 years ago by nickm

Status: needs_revisionneeds_review

I think I have a fix for this, and a couple related issues now. Indeed, your investigation put me on the right track.

Have a look at bug7112_v3 in my public repo?

comment:13 Changed 7 years ago by andrea

That looks good to me; nice catch with channel_dump_statistics() BTW.

comment:14 Changed 7 years ago by arma

I just tried running what I think is nickm/bug7112_v3 on moria1, and I still get a stream of

Oct 18 18:20:29.118 [warn] tor_addr_is_internal_(): Bug: tor_addr_is_internal() called from src/or/config.c:1994 with a non-IP address of type 0
Oct 18 18:20:29.279 [warn] tor_addr_is_internal_(): Bug: tor_addr_is_internal() called from src/or/config.c:1994 with a non-IP address of type 0
Oct 18 18:20:29.429 [warn] tor_addr_is_internal_(): Bug: tor_addr_is_internal() called from src/or/config.c:1994 with a non-IP address of type 0
...

This is on Tor v0.2.4.3-alpha-dev (git-64676d05714a8248).

comment:15 in reply to:  14 Changed 7 years ago by andrea

Replying to arma:

This is on Tor v0.2.4.3-alpha-dev (git-64676d05714a8248).

Hmm, it looks to me like nickm/bug7112_v3 is 850c9901445a1d2a4ce7ecf16e03f939fcd821d6. though.

comment:16 Changed 7 years ago by arma

Great. How come 'make distclean' doesn't rm src/or/micro-revision.i? Is that worth a separate ticket, or am I just being hasty and doing something wrong?

Once I make distclean and rm that file and then autogen and configure and make, I get

Tor v0.2.4.3-alpha-dev (git-850c9901445a1d2a)

and the stream of bug lines remains.

comment:17 in reply to:  16 ; Changed 7 years ago by nickm

Status: needs_reviewneeds_revision

Replying to arma:

Great. How come 'make distclean' doesn't rm src/or/micro-revision.i? Is that worth a separate ticket, or am I just being hasty and doing something wrong?

It could be a separate ticket, but don't expect it fixed before december fwiw.

Once I make distclean and rm that file and then autogen and configure and make, I get

Tor v0.2.4.3-alpha-dev (git-850c9901445a1d2a)

and the stream of bug lines remains.

Well that's just screwed up. I wonder where I went wrong.

comment:18 in reply to:  16 Changed 7 years ago by andrea

Replying to arma:

Great. How come 'make distclean' doesn't rm src/or/micro-revision.i? Is that worth a separate ticket, or am I just being hasty and doing something wrong?

I think it's definitely wrong if make distclean doesn't kill a generated file.

Once I make distclean and rm that file and then autogen and configure and make, I get

Tor v0.2.4.3-alpha-dev (git-850c9901445a1d2a)

and the stream of bug lines remains.

Hold on, I'll see if I can repro with that branch.

comment:19 in reply to:  17 Changed 7 years ago by arma

Replying to nickm:

Replying to arma:

Great. How come 'make distclean' doesn't rm src/or/micro-revision.i? Is that worth a separate ticket, or am I just being hasty and doing something wrong?

It could be a separate ticket, but don't expect it fixed before december fwiw.

Opened #7143 for what appears to be a separate issue.

comment:20 Changed 7 years ago by nickm

Status: needs_revisionneeds_review

Okay, we found another bug that was keeping the fix for the other bug working. My bug7112_v3 branch now has andrea's fix for it.

comment:21 Changed 7 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

This time for sure. Andrea says it tests out okay. Merged.

Note: See TracTickets for help on using tickets.