Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#2279 closed defect (fixed)

Duplicate call to connection_mark_for_close

Reported by: Kiyoo Owned by:
Priority: Medium Milestone: Tor: 0.2.2.x-final
Component: Core Tor/Tor Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

I get lots (about twice an hour) of these in my logfile:
[Warning] Bug: Duplicate call to connection_mark_for_close at connection.c:1175 (first at connection_edge.c:1618)

tor version: 0.2.1.27

my torrc:
SocksPort 9050
SocksListenAddress 127.0.0.1
AutomapHostsOnResolve 1
TransPort 9040
DNSPort 53
Log notice syslog
ControlPort 9051
HashedControlPassword xxxxxxxxxxxxxxxx
DataDirectory /var/lib/tor/.tor
User toranon

Child Tickets

Change History (17)

comment:1 Changed 10 years ago by nickm

Hm. connection.c:1175 is the one here, inside connection_handle_listener_read:

  if (connection_init_accepted_conn(newconn, conn->type) < 0) {
    connection_mark_for_close(newconn);
    return 0;
  }

and connection_edge.c:1618 is the one here, inside connection_ap_handle_rewrite_and_attach():

    conn->_base.state = AP_CONN_STATE_CIRCUIT_WAIT;
    if ((circ && connection_ap_handshake_attach_chosen_circuit(
                   conn, circ, cpath) < 0) ||
        (!circ &&
         connection_ap_handshake_attach_circuit(conn) < 0)) {
      if (!conn->_base.marked_for_close)
        connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH);
      return -1;
    }

So at first glance, something keeps a new client stream from getting attached, and it gets marked twice. The clear fix seems to be adding a check in connection_handle_listener read to avoid the double-mark more cleanly. But there's a root problem here that doesn't seem to be visible from the original bug description: why is connection_ap_handshake_attach{_chosen}_circuit() failing in the first place?

Kiyoo, do you have any other messages in your log that could be related to this?

comment:2 Changed 10 years ago by Kiyoo

Now that I get more of these warnings I see the following pattern:

  • [Notice] No Tor server allows exit to [scrubbed]:xxxx. Rejecting.

always followed by the mentioned Warning:

  • [Warning] Bug: Duplicate call to connection_mark_for_close at connection.c:1175 (first at connection_edge.c:1618)

So the log file looks like:

[Notice] No Tor server allows...
[Warning] Bug: Duplicate...
[Notice] No Tor server allows...
[Warning] Bug: Duplicate...
[Notice] No Tor server allows...
[Warning] Bug: Duplicate...
...

HTH

comment:3 Changed 10 years ago by nickm

Interesting. I'm curious as to whether you are really trying to connect to things that no Tor exit supports, or whether there is something else going on.

I wonder if the addresses follow any particular pattern. You can see the actual addresses by setting "SafeLogging 0" in your torrc file so that it shows you addresses instead of "[scrubbed". Are they mostly of the form "127.192.x.x" (which would mean they are 'virtual' addresses as returned by AutomapHostsOnResolve), or are they mostly real IPs?

Also, are the ports you are seeing this message for real ports that you're trying to connect to, or do they seem at all "weird"?

Obviously, please don't actually tell me the addresses you're connecting to. Once you've had a look, you'll probably want to turn SafeLogging back on and maybe delete your logs.

comment:4 Changed 10 years ago by nickm

On IRC, boboper says:

2279 happens for CONN_TYPE_AP_TRANS_LISTENER. 
                 connection_ap_process_transparent() returns -1 and conn 
                 already closen

comment:5 Changed 10 years ago by rransom

Kiyoo, what operating system are you using?

comment:6 Changed 10 years ago by Kiyoo

I set SafeLogging to 0, but currently do not see these Warnings any more.
I'll report back and answer your questions as soon as I see these warnings again.
This host runs a Linux kernel.

comment:7 Changed 10 years ago by nickm

Milestone: Tor: 0.2.2.x-final

Pegging this bug to 0.2.2.x-final. If analysis shows it to be more serious than just the double-mark warning, we should think about a backport to 0.2.1.x instead.

comment:8 Changed 10 years ago by Kiyoo

Is it possible that this error gets triggered by a specific (maybe incorrect) iptables redirect setup?

While this errors were occurring constantly I used the following rule to redirect all traffic to tor:
(NAT table)
-A OUTPUT -p tcp -j REDIRECT --to-ports 9040

since then I changed it because I thought that traffic generated by the tor daemon itself should not be redirected to the TransPort...
(NAT table)
-A OUTPUT -m owner ! --uid-owner toranon -p tcp -j REDIRECT --to-ports 9040

comment:9 Changed 10 years ago by nickm

Status: newneeds_review

See branch bug2279 in my public repository.

It has two parts-- the first part fixes the duplicate mark. The second part adds a check for client attempts to connect to a local address via a random exit, and rejects such connections earlier with a more useful message.

comment:10 in reply to:  9 Changed 10 years ago by rransom

Replying to nickm:

The second part adds a check for client attempts to connect to a local address via a random exit, and rejects such connections earlier with a more useful message.

We need a torrc option to allow attempts to connect to such addresses (most obviously for private networks, but there are uses for this even when a Tor client is retrieving consensuses from the public Tor network).

comment:11 Changed 10 years ago by nickm

What other uses do you have in mind?

comment:12 in reply to:  11 Changed 10 years ago by rransom

Replying to nickm:

What other uses do you have in mind?

A user could configure a non-published Tor relay whose exit policy permits some non-routable addresses, load its descriptor into a Tor client using the control port, and build a circuit to the unpublished relay to connect to a somewhat hidden network.

comment:13 Changed 10 years ago by nickm

Added a configuration option as suggested.

comment:14 Changed 10 years ago by nickm

Aded a manpage entry for it too. Fuzzel on #tor-dev suggests that "Private" is a better name than "Internal" here. But if we change "Internal" to "Private", we'll need to make sure to change ClientDNSRejectPrivateAddresses too, and add an alias for the old name.

comment:15 Changed 10 years ago by rransom

In commit 411ec3c0f8cd, a log message says "NatdPort" and a comment says "NATd"; the capitalization should match the man page, which uses "NATDPort" and "NATD". (Unless NATd is the correct capitalization, in which case we need another global capitalization-canonicalization patch.)

In the man page chunk in commit 30111a3a0199, s/a controller/or &/ ; also, fulfil seems to be a less common spelling than fulfill (at least in American English).

Other than that, looks good.

comment:16 Changed 10 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

hm, looks like the BSD manpage says "natd"; all of our config options start with uppercase as we spell them, so NatdPort might be okay, or natdPort if we're being good and hackerly. I'm going to leave our NATD mistake as is for now, and open a ticket ( #2507 ) to fix it in 0.2.3.x.

Fixing the rest of that, tweaking the manpage a little more, and merging. Thanks for having a look!

comment:17 Changed 8 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.