Opened 15 years ago

Last modified 14 years ago

#99 closed defect (Won't fix)

connect.c "let's try to resolve it anyway, why not" bug

Reported by: skquinn Owned by:
Priority: Very Low Milestone:
Component: Torify Version:
Severity: Keywords:
Cc: skquinn, nickm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Shun-ichi Goto's connect.c program 1.90 tries to resolve addresses locally even when specifically told to resolve remotely only (as we discussed in IRC).

[Automatically added by flyspray2trac: Operating System: All]

Child Tickets

Change History (8)

comment:1 Changed 15 years ago by nickm

Has anyone tried contacting Shun-ichi Goto about this issue?

comment:2 Changed 14 years ago by nickm

For reference, here's the IRC discussion in question.

[DrHouston] okay I have a question about Shun-ichi Goto's connect.c program
[DrHouston] I can get it to connect to hidden services fine, but I can't get it
to not leak DNS requests
[DrHouston] at least not without installing BIND and putting in empty zones for
.onion and .exit (that's one way to squash them!)
[arma] oh, does it try doing the resolve first, and then doing socks4a second?
[DrHouston] yes
[DrHouston] -R remote is supposed to stop that, I think
[DrHouston] but it doesn't
[arma] that sucks.
[arma] have you looked at the code for -R?
[arma] it could just be a bug.
[arma] socat had exactly this bug too.
[arma] people consider it a feature if you optimize by trying the resolve locally
[DrHouston] it's only a feature if you don't mind giving yourself up
[weasel] but we have nothing to hide, do we?
[arma] well, not everybody uses these things for tor
[DrHouston] "gee, this guy just tried to resolve 6sxoyfb3h2nvok2d.onion, and here's a bunch of Tor port 9001 traffic, gee, I wonder what it is?"
* You have new email.
[DrHouston] true, that
[DrHouston] maybe the concept is just too new that it's actually a bad thing to
try the resolve locally first
* You have new email.
[arma] right.
[arma] is that what -R does, in the code?
[DrHouston] that's what the docs say -R does
* You have new email.
[DrHouston] I'm not sure how to follow the rest of the code
[DrHouston] I tried simply taking out the function that does the call to gethostbyname
[arma] url for connect.c? i'll take a look briefly
[DrHouston] no dice.
[DrHouston] http://www.taiyo.co.jp/~gotoh/ssh/connect.c
[DrHouston] and s/c$/html/ if you want to read the docs
[arma] so your command line is what?
[DrHouston] connect -R remote -4 -S localhost:9050 %h %p
[DrHouston] with %h and %p being filled-in by ssh
[arma] does it also leak when you ask for socks 5?
[DrHouston] yes
* You have new email.
[arma] win32?
* Darven (~darven@…) has joined channel #tor
[DrHouston] no this is Debian
[arma] yes, i think it has this bug.
[arma] foo.
[arma] well, design flaw
[arma] oh.
[arma] maybe not. hard to tell. complex program for its size, isn't it.
[arma] would help if it had some fucking comments in the code.
[DrHouston] oh I agree
[arma] in open_connection,
[arma] it seems to have a clause that tries to sneak in a direct connection even if you didn't ask for one, just in case it works.
[DrHouston] oh, that's bad
[DrHouston] if I ask to go through a proxy, it's for a reason
[DrHouston] okay, local_resolve is where the DNS requests happen, right?
[arma] it seems so.
[DrHouston] because I blew that away and I still get DNS requests going out
[arma] perhaps you didn't recompile or something?
[DrHouston] no I recompiled
[DrHouston] wait
[DrHouston] ERROR: can't resolve hostname: 127.0.0.1
[DrHouston] sigh
[arma] the first half of local_resolve does IPs
[DrHouston] okay, my C is very rusty
[DrHouston] maybe I/we should write e-mails to Mr. Goto (what a name for a programmer, eh) about this
[arma] yea. probably with a patch is better.
[DrHouston] I'm reading through open_connection now
[DrHouston] it looks to me like it only tries a direct address if it's in a list you give it
[arma] does connect.c work for more than one connection at a time?
[arma] it seems to be caching results, and remembering which ones allowed direct connect
[DrHouston] I would assume it's designed to run one instance per connection
[DrHouston] DEBUG: socks_resolve=REMOTE (2)
[arma] sure, it plans to do it remote
[DrHouston] DEBUG: dest_host=6z
sn.onion
[DrHouston] DEBUG: resolving host by name: 6z
sn.onion
[arma] but that's the key: see the use of dest_host in open_connection,
[arma] not the 'host' that is passed in?
[DrHouston] ah
[DrHouston] yeah
[arma] it tries to be smarter than you.
[weasel] which sw are we talking about?
[arma] [DrHouston] http://www.taiyo.co.jp/~gotoh/ssh/connect.c
[weasel] ah
[DrHouston] silicon is rarely smarter than protoplasm. I'm sorry.
[arma] it apparently has the socat "let you resolve remotely, but try it locally first anyway because hey why not" bug
[weasel] probably a common design flaw in socks implementations
[arma] yep.
[weasel] because really, why not :)
[arma] exactly.
[DrHouston] well they make the assumption that DNS requests won't go past the local LAN
[DrHouston] running a node trying to connect via Tor, this assumption blows up in a thousand pieces
[weasel] :)
[arma] well, it's a fine assumption for connect.c. the code works fine if the resolve fails.
[DrHouston] well, at least the bug is known
[DrHouston] thank God for GPL'd software
[arma] to the three of us
[arma] we should get him to do a revision, with a patch we supply.
[DrHouston] ok
[DrHouston] any overwhelming reason to keep a lid on this bug?
[arma] put it on our bugtracker?
[arma] nope, shout it far and wide if that helps anything
[arma] if you put it on our bugtracker, in the 'torify' category, we'll see what we can do
[DrHouston] ok
[DrHouston] I'd use socat instead but it's a clumsy piece of crap by comparison
[arma] one day we'll get around to rewriting all the software

comment:3 Changed 14 years ago by phobos

Is this still an issue?

comment:4 Changed 14 years ago by skquinn

I haven't had time to play with it lately, but might give it another whirl in a few days.

comment:5 Changed 14 years ago by skquinn

Okay. This patch appears to at least not break anything. I can't successfully connect to a hidden service hosted on my own computer, though; can someone else test?

--- connect.c 2005-12-03 04:13:32.000000000 -0600
+++ connect_nolocal.c 2005-12-03 04:16:36.000000000 -0600
@@ -1636,17 +1636,7 @@

addr->sin_family = AF_INET;
addr->sin_addr.s_addr = inet_addr(host);

} else {

  • debug("resolving host by name: %s\n", host);
  • ent = gethostbyname (host);
  • if ( ent ) {
  • memcpy (&addr->sin_addr, ent->h_addr, ent->h_length);
  • addr->sin_family = ent->h_addrtype;
  • debug("resolved: %s (%s)\n",
  • host, inet_ntoa(addr->sin_addr));
  • } else {
  • debug("failed to resolve locally.\n");
  • return -1; /* failed */
  • }

+ return -1; /* castrated version */

}
return 0; /* good */

}

comment:6 Changed 14 years ago by nickm

(decremented severity since this isn't a tor bug. Could somebody confirm that the original
author isn't taking patches, and if so, fork?)

comment:7 Changed 13 years ago by nickm

It seems unclear whether:

1) This is still an issue
2) Anybody is depending on connect.c any longer
3) We are recommending connect.c any longer
4) The author is taking patches
5) Anybody cares enough to fork if the author is not taking patches

I'll close this in a bit unless somebody answers some of those with a "yes". I do not think
that a forked connect.c is likely to ship with Tor soon.

comment:8 Changed 13 years ago by nickm

flyspray2trac: bug closed.
Not our software, and it's not clear anybody is using it these days. If somebody cases, they should talk to the original author, or fork the code.

Note: See TracTickets for help on using tickets.