GETINFO response parser doesn't handle AF_UNIX entries.

torbutton issues a GETINFO command to validate the configured socks port, but the response parser doesn't handle AF_UNIX entries.

[07-21 19:17:44] Torbutton WARN: unexpected tor response: net/listeners/socks="127.0.0.1:9050" "unix:/var/run/tor/socks"

This currently doesn't matter because the number of people using an AF_UNIX capable Tor Browser is ~1, but it is something to note for the future sandboxing efforts.

added TorBrowserTeam201609R in Legacy / Trac component::applications/tor browser in Legacy / Trac owner::tbb-team in Legacy / Trac parent::14270 in Legacy / Trac priority::very low in Legacy / Trac resolution::fixed in Legacy / Trac severity::minor in Legacy / Trac status::closed in Legacy / Trac tbb-sandboxing in Legacy / Trac tbb-torbutton in Legacy / Trac type::defect in Legacy / Trac labels

Whoops, the command that's puking is GETINFO, not PROTOCOLINFO.

Trac:
Summary: PROTOCOLINFO response parser doesn't handle AF_UNIX entries. to GETINFO response parser doesn't handle AF_UNIX entries.
Description: torbutton issues a PROTOCOLINFO command to validate the configured socks port, but the response parser doesn't handle AF_UNIX entries.

[07-21 19:17:44] Torbutton WARN: unexpected tor response: net/listeners/socks="127.0.0.1:9050" "unix:/var/run/tor/socks"

This currently doesn't matter because the number of people using an AF_UNIX capable Tor Browser is ~1, but it is something to note for the future sandboxing efforts.

to

torbutton issues a GETINFO command to validate the configured socks port, but the response parser doesn't handle AF_UNIX entries.

[07-21 19:17:44] Torbutton WARN: unexpected tor response: net/listeners/socks="127.0.0.1:9050" "unix:/var/run/tor/socks"

This currently doesn't matter because the number of people using an AF_UNIX capable Tor Browser is ~1, but it is something to note for the future sandboxing efforts.

Trac:
Parent: N/A to legacy/trac#14270 (moved)

Trac:
Component: Applications/Torbutton to Applications/Tor Browser
Owner: N/A to tbb-team
Keywords: N/A deleted, tbb-torbutton added

Trac:
Cc: N/A to brade, mcs

Kathy and I decided to begin our UNIX domain socket journey by fixing this bug. Here is a patch: https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug19733-01&id=db21620ac199bd4bc25939f14c6caaaa44093b89

Trac:
Keywords: N/A deleted, TorBrowserTeam201608R added
Status: new to needs_review

Mozilla changed their implementation to use file: URLs instead of simple paths (see https://bugzilla.mozilla.org/show_bug.cgi?id=1211567#c22). We will revise this patch accordingly.

Trac:
Status: needs_review to needs_revision

Here is a new patch: https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug19733-02&id=f1932e73ee5199969f49ce595fb30ec14e76cb52

Trac:
Status: needs_revision to needs_review

Moving review tickets to September

Trac:
Keywords: TorBrowserTeam201608R deleted, TorBrowserTeam201609R added

Replying to mcs:

Here is a new patch: https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug19733-02&id=f1932e73ee5199969f49ce595fb30ec14e76cb52 Minor things:

+ if (socksAddr && (socksAddr.substr(0, 5) == "file:")) {

Can this be sockAddr.startsWith("file:")?

+ foundSocksListener = (socketPath == path);

+          foundSocksListener = ((socksAddr == torSocksAddr) &&
+                                (socksPort == torSocksPort));

I would suggest using === (triple equals) for safety.

Replying to arthuredelstein:

Replying to mcs:

Here is a new patch: https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug19733-02&id=f1932e73ee5199969f49ce595fb30ec14e76cb52 Minor things: {{{

• if (socksAddr && (socksAddr.substr(0, 5) == "file:")) { }}} Can this be sockAddr.startsWith("file:")?

Done.

{{{

• foundSocksListener = (socketPath == path); }}} {{{

•      foundSocksListener = ((socksAddr == torSocksAddr) &&

•                            (socksPort == torSocksPort));

}}} I would suggest using === (triple equals) for safety.

OK. We fixed another place too where == was used instead of ===. While testing, we discovered that file: URLs that begin with file:/// were not handled correctly (and they should begin that way). So we rewrote the code that converts the network.proxy.socks value to a file path. Please review: https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug19733-03&id=a07ceb4e7974a1369a1d6484cbfce0a7ec9e503f

Looks good with one nit: I guess removing the very first line in torbutton.js is just an oversight? I think we should keep that one. :)

Trac:
Keywords: TorBrowserTeam201609R deleted, TorBrowserTeam201609 added
Status: needs_review to needs_revision

Replying to gk:

Looks good with one nit: I guess removing the very first line in torbutton.js is just an oversight? I think we should keep that one. :)

Yes indeed. Here is a revised patch: https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug19733-04&id=5ea022aadf6416a1f046eac47a3ec28ea2a5dd7d

Trac:
Keywords: TorBrowserTeam201609 deleted, TorBrowserTeam201609R added
Status: needs_revision to needs_review

Thanks, applied with commit 5ea022aadf6416a1f046eac47a3ec28ea2a5dd7d.

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

Trac:
Keywords: tbb-sandbox deleted, tbb-sandboxing added

closed

mentioned in issue legacy/trac#20951 (moved)

moved from legacy/trac#19733 (moved)

added Bug label and removed 1 deleted label

added 1 deleted label and removed 1 deleted label

added Security label and removed 1 deleted label