Opened 7 years ago

Closed 5 years ago

Last modified 3 years ago

#6031 closed enhancement (duplicate)

Distinguish when a Tor HS is "not found" vs "not reachable" (exists / does not exists)

Reported by: naif Owned by:
Priority: Medium Milestone: Tor: 0.2.6.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-hs
Cc: nickm, sebastian, rransom Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Tor2web software (but also on other software) need to provide a different response depending if a Tor Hidden Service that a user is trying to access exists or not.

The error handling revealed that today there's no specific return code to distinguish between:

  • TorHS does not exists
  • TorHS exists (and then it maybe reachable or not)

So in order to mimic the error-response typical of internet environment:

  • Host not found (TorHS does not exists)
  • Host unreachable (Tor HS exists but is not reachable)

After a discussion on IRC with Nick and sebastian the following conclusion arised:

nickm: so, this concept of ==== "We can have "exists" mean "This HS has been running recently, and has been attempting to provide service", though and implement that by saying that an HS "exists" if we find a descriptor for it, and doesn't exist if we don't. ==== is already "available" os must be implemented?

nickm
naif: I don't think that's exposed from Tor right now.

This ticket is to provide an implementation for that feature.

Whenever possible we should use a Return Code from Socks Server following SOCKS protocol so that a Socks Client can parse the result and know that connection to this TorHS is "Not Found".

Child Tickets

Change History (27)

comment:1 Changed 7 years ago by runa

I don't think we should implement something that would allow others to enumerate hidden services (it would take forever and would probably be a waste of time, but would be possible). Please correct me if I'm wrong here.

comment:2 Changed 7 years ago by runa

rransom corrected me and pointed out that looking up every possible HS address is not the way to enumerate all running HSes. I'm still not sure if we should distinguish between no HS and HS but not reachable, but I do see an advantage from the usability side of things.

comment:3 Changed 7 years ago by nickm_mobile

The way to expose this is , I think, is to have the client report a different error (possibly via SOCKS response if there's a plausibly standards-compliant way to do that) in the case where a hidden service connection fails because we can't get a descriptor than we report in the case where we can't introduce and rendezvous.

comment:4 Changed 7 years ago by hellais

Would it make sense to map the standard SOCKS5 reply field X'04' "Host unreachable" to "I can't get a descriptor" and X'03' "Network unreachable" [1] in case the connection to the intro and rendezvous fails?

[1] http://www.ietf.org/rfc/rfc1928.txt

comment:5 Changed 7 years ago by nickm_mobile

Sadly, the RFC doesn't appear to describe a meaning for each of those. Typically, i'd expect that network unreachable would mean that we couldn't connect to Tor at all, or something like that.

comment:6 Changed 7 years ago by naif

Well,

i think that the various way to check this could be:

a) SOCKS level

Given that there is no documented specific reply for that, we could use reply

o X'09' to X'FF' unassigned

that are unassigned by the RFC and give the meaning of "Host not found".

This means that the socks client must manage this new REP code.

b) Tor CP level

Extend messages so that if a Circuit is created manually from Tor Control Port, a Tor CP client can know if a TorHS exists or not

This would mean that a client, before connecting to a TorHS host, should go trough Tor CP and manually establish a circuit.

c) Tor DNS Resolver level

We may leverage the DNS Resolver of Tor, to handle such situation when we have a configuration like:

VirtualAddrNetwork 10.192.0.0/10
AutomapHostsOnResolve 1
DNSPort 53

In that case we may handle the "Host Not Found" condition case.

This would mean that a client, before connecting to a TorHS host, should go trough DNS query.

I would suggest to goes for an unassigned SOCKS REP code.

comment:7 Changed 7 years ago by hellais

I wrote a patch for this, you can find it inside of the feature-6031 branch here: https://gitweb.torproject.org/user/art/tor.git/shortlog/refs/heads/feature-6031

The behavior I ended up choosing was REP code 0x23 for HS_NOT_FOUND and 0x24 for HS_UNREACHABLE.

I have a few questions though:
Is it sufficient to have reached to the end of rend_client_refetch_v2_renddesc() to determine that the HS does not exist?

For passing to rend_client_desc_trynow (that from what I understand, must be called to close also the pending connections) the fact that I am failing because the HS does not exist I replace the query with the string "hostnotfound". This is kind of hackish and may create problems when invoking rend_cmp_service_ids in rend_client_desc_trynow, what is the better way to do this?

Is overall the strategy for determining if a HS exists or is unreachable valid?

comment:8 in reply to:  7 Changed 7 years ago by rransom

Status: newneeds_revision

Replying to hellais:

I wrote a patch for this, you can find it inside of the feature-6031 branch here: https://gitweb.torproject.org/user/art/tor.git/shortlog/refs/heads/feature-6031

The behavior I ended up choosing was REP code 0x23 for HS_NOT_FOUND and 0x24 for HS_UNREACHABLE.

I have a few questions though:
Is it sufficient to have reached to the end of rend_client_refetch_v2_renddesc() to determine that the HS does not exist?

Probably.

For passing to rend_client_desc_trynow (that from what I understand, must be called to close also the pending connections) the fact that I am failing because the HS does not exist I replace the query with the string "hostnotfound". This is kind of hackish and may create problems when invoking rend_cmp_service_ids in rend_client_desc_trynow, what is the better way to do this?

s/kind of hackish/completely and utterly broken/

  • Throw away that patch entirely.
  • In rend_client_desc_trynow, replace “if (rend_cache_” through “{” with:
        rend_cache_lookup_entry(rend_data->onion_address, -1, &entry);
        if (entry != NULL &&
            rend_client_any_intro_points_usable(entry)) {
    
  • Before the corresponding “} else {”, add:
        } else if (entry == NULL) {
          /* We couldn't get a descriptor for this HS at all; maybe it doesn't exist. */
          /* FIXME CLOSE STREAMS WITH APPROPRIATE REASON */
    
  • After the “} else {”, add:
          /* We got a descriptor, but either (a) the HS published no intro points (i.e. it was shut down cleanly) or (b) we tried all the intro points it listed, and they failed. */
    
  • Fix the FIXME comment.
  • Wrap comments to 79 columns or less, so that make check-spaces won't complain.
  • Actually test your patch at least once or twice.
  • Remember to mark the ticket as needs_review once you've pushed a new branch.

Is overall the strategy for determining if a HS exists or is unreachable valid?

Probably.

comment:9 Changed 7 years ago by hellais

Status: needs_revisionneeds_review

Thanks for your review!

Here is my second attempt at this patch: https://gitweb.torproject.org/user/art/tor.git/shortlog/refs/heads/feature6031-2.

I have tested it and it appears to be working as it should.

comment:10 Changed 7 years ago by nickm

Milestone: Tor: 0.2.4.x-final

comment:11 Changed 7 years ago by andrea

Status: needs_reviewneeds_revision

I've tested this and it seems to work as advertised. Two issues, though:

First, RFC 1928 does not define status codes 0x23 or 0x24, and describes all status codes >= 0x09 as simply 'unassigned'. It requires that clients treat any status other than 0x00 as an error and disconnect, so the scope for pathological behavior *from conforming clients* is limited here, but do we know how well clients behave in practice?

This may at least lead to some rather non-transparent error messages for the user. In testing, status code 0x23 causes privoxy to report "Privoxy was unable to socks5-forward your request http://uhd2bclqqu2sfch6.onion/ through 127.0.0.1: SOCKS5 negotiation protocol error", for example.

Without this patch it produces "Privoxy was unable to socks5-forward your request http://uhd2bclqqu2sfch6.onion/ through 127.0.0.1: SOCKS5 host unreachable", which seems to me to have a more recognizable relation to the actual error condition.

The tor log messages are perfectly clear in this case, though.

Second, this fails make check-spaces: src/or/or.h line 627 and src/or/reasons.c line 76 are wider than 80 characters, and src/or/reasons.c line 140 is a second consecutive blank line.

comment:12 Changed 7 years ago by naif

Imho we could, as a next step:

a) Provide a patch to the SOCKS RFC on IETF website

b) Document this change to the Tor Wiki

c) Announce that change to tor's socks client

Post to the mailing lists / tickets systems of most used Tor's community socks client (Firefox, Privoxy, etc) to announce the improvements.

d) Eventually make this behavior configurable and by default disabled

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

Replying to andrea:

I've tested this and it seems to work as advertised. Two issues, though:

First, RFC 1928 does not define status codes 0x23 or 0x24, and describes all status codes >= 0x09 as simply 'unassigned'. It requires that clients treat any status other than 0x00 as an error and disconnect, so the scope for pathological behavior *from conforming clients* is limited here, but do we know how well clients behave in practice?

What would you think about using "Host unreachable" for the case where lookup fails and "connection refused" for the case where introduction/rendezvous fails? I don't think we'd be doing a great violence to either. (If only they had thought to introduce "name lookup failed" when they introduced hostname addresses. But there's no use crying over bad specs.)

comment:14 Changed 7 years ago by naif

mmm but "connection refused" should be already used today when:

  • lookup is ok
  • introduction/redenzvous is ok
  • circuit is established
  • TCP port redirected on the Tor Hidden Service port-redirection is closed (and receive a "connection refused" from TCP-level-meaning of that).

If we use "connection refused" we would not be able to detect the condition previously defined.

The real matter is that socks does not let us to handle "dns resolution error" that is the stuff we are trying simulate.

So i think that for that reason we would still need to use one of the unassigned/reserved socks error codes, and eventually update various libraries / internet draft stuff, to include a generic "host not found" that could apply to both TorHS and to normal hostname.

What do you think?

comment:15 Changed 7 years ago by naif

What if we would use:

  • "Host unreachable" when introduction/rendezvous fails
  • "Address type not supported" when lookup does not work

The "Address type not supported" seems to me enough unusual to happen but enough "similar" in a possible interpretation of "Tor HS not found" ?

comment:16 Changed 7 years ago by nickm

Keywords: tor-hs added

comment:17 Changed 7 years ago by nickm

Component: Tor Hidden ServicesTor

comment:18 Changed 6 years ago by nickm

Milestone: Tor: 0.2.4.x-finalTor: 0.2.5.x-final

After discussion with Andrea, we think the likeliest way to make this actually mergeable is to give clients some way to signal that they can support an extended set of error codes when they do the initial socks handshake. Otherwise, we'll be deviating from spec in the presence of clients who are trying to follow it.

comment:19 Changed 6 years ago by naif

Well,

this feature is now used, as far as i know, only by Tor2web.

Do we want to make it specific, as a config+compile-time flag for Tor2webMode of Tor?

Or maybe we can just make it a configurable variable, available for any use with something like:

ExtendedSocksMessages 1

to enable it.

What do you think?

comment:20 Changed 6 years ago by nickm

I think it needs to be a per-connection thing in the protocol, not a global thing in the tor process. Otherwise you need *all* of your apps to support this, or you can't turn it on.

The most reasonable place for an app to signal its support here would be by grabbing an unused SOCKS5 authentication type code, and have clients advertise that type only if they support whatever extensions we want to add.

comment:21 Changed 6 years ago by naif

(ab)using protocol for fun and profit.

I like this approach.

http://www.ietf.org/rfc/rfc1928.txt

We may use:

o X'80' to X'FE' RESERVED FOR PRIVATE METHODS

And just pickup the first one (X'80) as a private method. ?

comment:22 Changed 5 years ago by nickm

Milestone: Tor: 0.2.5.x-finalTor: 0.2.6.x-final

Moving nearly all needs_revision tickets into 0.2.6, as now untimely for 0.2.5.

comment:23 Changed 5 years ago by nickm

Resolution: duplicate
Status: needs_revisionclosed

The canonical fix for this one is going to be as part of an implementation for proposal 229; see #12456.

("Duplicate" is not exactly the best status here, but it seems the least wrong.)

comment:24 Changed 4 years ago by evilaliv3

in relation to this ticket, i'm noticing a strange behaviour of the tor repository.

while patching tor for tor2web uses we were using the patch provided by hellais that was available at the following url: https://gitweb.torproject.org/user/art/tor.git/patch/f6d3dc3d9e0e70f2c553ce254b49630bd98910e9?hp=ca525db02dbb026bda4305881476dada754c3ca3a

now it seems that the repo is providing always the same content regardless of the uri that one may write.

comment:25 Changed 4 years ago by nickm

It appears to be in a branch called "feature6031-2" in that repository.

comment:26 Changed 4 years ago by cypherpunks

To make it easier for other people to find the appropriate patch: https://gitweb.torproject.org/user/art/tor.git/patch/?id=f6d3dc3d9e0e70f2c553ce254b49630bd98910e9 - works fine.

comment:27 Changed 3 years ago by evilaliv3

Severity: Normal
Note: See TracTickets for help on using tickets.