Opened 11 years ago

Last modified 7 years ago

#646 closed defect (Fixed)

miscellaneous stream event bugs for DNS requests

Reported by: mwenge Owned by:
Priority: Low Milestone:
Component: Core Tor/Tor Version: 0.2.0.22-rc
Severity: Keywords:
Cc: mwenge, nickm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The response to a reverse lookup from the controller looks like:

650 ADDRMAP REVERSE[64.4.33.7] lc2.bay0.hotmail.com "2008-03-29 21:59:05" EXPIRES="2008-03-29 21:59:05"

Except when the result is already cached. Then it looks like:

650 ADDRMAP 64.4.33.7 lc2.bay0.hotmail.com "2008-03-29 21:59:05" EXPIRES="2008-03-29 21:59:05"

The 'REVERSE' is missing. Not sure which way is the 'correct' way - assume it's the non-cached response.

Also,

  • There's currently no 'NEW' stream event for DNS requests. Add one.
  • Cached reverse DNS requests get a 'CLOSE' event but no 'NEW' event. Just drop the 'CLOSE' event, since no conn is created.

Index: src/or/connection_edge.c
===================================================================
--- src/or/connection_edge.c (revision 14233)
+++ src/or/connection_edge.c (working copy)
@@ -1348,13 +1348,16 @@

&map_expires)) {

char *result = tor_strdup(socks->address);
/* remember _what_ is supposed to have been resolved. */

  • strlcpy(socks->address, orig_address, sizeof(socks->address));

+ tor_snprintf(socks->address, sizeof(socks->address), "REVERSE[%s]",
+ orig_address);

connection_ap_handshake_socks_resolved(conn, RESOLVED_TYPE_HOSTNAME,

strlen(result), result, -1,
map_expires);

+ strlcpy(socks->address, orig_address, sizeof(socks->address));

connection_mark_unattached_ap(conn,

  • END_STREAM_REASON_DONE |
  • END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED);

+ END_STREAM_REASON_DONE |
+ END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED |
+ END_STREAM_REASON_FLAG_ALREADY_SENT_CLOSED);

return 0;

}
if (options->ClientDNSRejectInternalAddresses) {

@@ -2084,9 +2087,11 @@

string_addr, payload_len) < 0)

return -1; /* circuit is closed, don't continue */

+ ap_conn->_base.address = tor_strdup("(Tor_internal)");

ap_conn->_base.state = AP_CONN_STATE_RESOLVE_WAIT;
log_info(LD_APP,"Address sent for resolve, ap socket %d, n_circ_id %d",

ap_conn->_base.s, circ->_base.n_circ_id);

+ control_event_stream_status(ap_conn, STREAM_EVENT_NEW, 0);

control_event_stream_status(ap_conn, STREAM_EVENT_SENT_RESOLVE, 0);
return 0;

}

[Automatically added by flyspray2trac: Operating System: All]

Child Tickets

Change History (5)

comment:1 Changed 11 years ago by nickm

These issues look important to fix. However, I worry about having socks->address change its value
once it's initially set. Why does the patch have to change the address back from REVERSE[x] to x
before calling mark_unattached_ap?

comment:2 Changed 11 years ago by mwenge

Hi Nick,

None that I can remember or establish now. I think I just didn't want to call mark_unattached with a modified
conn->socks_request->address that wasn't a valid IP address or URI. It doesn't seem to make any difference in practice.

comment:3 Changed 11 years ago by nickm

Applied as r14413 and r14414.

comment:4 Changed 11 years ago by nickm

flyspray2trac: bug closed.

comment:5 Changed 7 years ago by nickm

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