Opened 8 years ago

Closed 3 years ago

#3967 closed enhancement (duplicate)

new identity feature could support ControlSocket

Reported by: T(A)ILS developers Owned by: mikeperry
Priority: Low Milestone:
Component: Applications/Torbutton Version:
Severity: Normal Keywords: AffectsTails
Cc: anonym, intrigeri, lunar, proper@…, gk, poncho@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

New identity feature only supports ControlPort. Supporting ControlSocket as well would make it easier to integrate with Debian systems that have Tor 0.2.2.x with ControlSocket enabled by default.

Child Tickets

Attachments (1)

0001-Implement-NEWNYM-through-ControlSocket.v0.patch (7.6 KB) - added by lunar 8 years ago.
Implement NEWNYM through ControlSocket (v0 patch)

Download all attachments as: .zip

Change History (21)

comment:1 Changed 8 years ago by T(A)ILS developers

Cc: amnesia@… added

Changed 8 years ago by lunar

Implement NEWNYM through ControlSocket (v0 patch)

comment:2 Changed 8 years ago by lunar

Status: newneeds_review

I have just attached a first working attempt of implementing NEWNYM through ControlSocket.

This is a very crude hack using js-ctypes but it is working nevertheless. It currently hardcode almost everything, but I'd like to have Mike's opinion before improving this very first attempt.

comment:3 in reply to:  2 Changed 8 years ago by mikeperry

Cc: lunar added

Replying to lunar:

I have just attached a first working attempt of implementing NEWNYM through ControlSocket.

This is a very crude hack using js-ctypes but it is working nevertheless. It currently hardcode almost everything, but I'd like to have Mike's opinion before improving this very first attempt.

Woah.. Did you try to create a file-input-stream and file-output-stream xpcom component instead? Is there a reason you can't treat the filesystem socket as normal file io? Or perhaps pass the nsIFileInputStream to the binary-input-stream, like I did with the socket transport service socket in torbutton_send_ctrl_cmd()?

Looks like https://developer.mozilla.org/en/Code_snippets/File_I%2f%2fO has some examples. They all warn about doing sync file IO from the main thread, but ctypes is going to be just as bad, and more fragile.

comment:4 Changed 8 years ago by lunar

I have just noticed that 'inputStream' and 'outputStream' are not local variables in torbutton_send_ctrl_cmd(). Is that intended?

comment:5 in reply to:  4 Changed 8 years ago by mikeperry

Replying to lunar:

I have just noticed that 'inputStream' and 'outputStream' are not local variables in torbutton_send_ctrl_cmd(). Is that intended?

Nope. Thanks. Fixed.

comment:6 Changed 8 years ago by lunar

Unix domain sockets really are sockets. The file system handle is to be treated as an address and cannot be used as a usual file. So nsIFile{Input,Output}Stream will not work.

If I tried to look into Firefox's internals to see if there was a way to open a AF_UNIX socket, and it looks like the code is there in NSPR, but not there is no XPCOM interface. There is also no XPCOM interface that would receive an already open file descriptor, even if there is an underlying function that might be used in the netwerk library.

Pending you do not judge this hack as very wrong, I think I'd like to ship it (after some more refinements) in the Debian package, even if it might not be welcome in the standard Torbutton. Can you think of a reason not do to so?

comment:7 Changed 8 years ago by mikeperry

Ugh. I thought we were dealing with named pipes here. Why did debian/tor settle on unix domain sockets instead of a named pipe on the filesystem?

Aside from that, the hack definitely needs to be hidden behind some debian-specific check that prevents the code from even entering the try block unless we're damn sure it is a debian system we're dealing with.

comment:8 Changed 8 years ago by lunar

Named pipes that works both read/write are not well defined accross architectures. The default behaviour is also to have them block until the other end of the pipe is opened. IIRC, some systems implement non-blocking mode, but those are also quite architecture dependent.

comment:9 Changed 8 years ago by mikeperry

"why did god make unix this way? :("

Anyways, I guess Tor uses sockets, not named pipes, so the discussion is probably irrelevant.

If you can wrap this codepath in a pref or other check so that it is only active on Debian and/or Iceweasel systems, I'll merge it. I just fear merging it on other platforms that may have a different libc location/name, etc. Or, perhaps just make it fail without a log notice in those cases?

comment:10 Changed 7 years ago by cypherpunks

Isn't the authentication cookie unnecessary here? You can control access to the ControlSocket simply by setting appropriate file permissions on the socket file (i.e. the user running firefox needs RW permissions to /var/run/tor/control). Surely that's the whole advantage of using unix domain sockets in the first place: you don't need an external authentication mechanism because you can rely on unix file permissions. Or am I missing something?

comment:11 Changed 7 years ago by proper

Cc: proper@… added
Status: needs_reviewneeds_revision

Mike looked at it (reviewed) and said, he'd accept the patch if there were changes. Subsequently the next state should be needs_revision.

comment:12 Changed 5 years ago by intrigeri

Cc: anonym intrigeri added; amnesia@… removed

comment:13 Changed 5 years ago by gk

Cc: gk added

comment:14 in reply to:  6 Changed 5 years ago by gk

Replying to lunar:

If I tried to look into Firefox's internals to see if there was a way to open a AF_UNIX socket, and it looks like the code is there in NSPR, but not there is no XPCOM interface.

FWIW that changed when https://bugzilla.mozilla.org/show_bug.cgi?id=892114 landed a while ago.

comment:15 Changed 4 years ago by poncho

Cc: poncho@… added

comment:16 Changed 4 years ago by cypherpunks

Keywords: AffectTails added

comment:17 Changed 4 years ago by cypherpunks

Keywords: AffectsTails added; AffectTails removed

comment:18 Changed 3 years ago by mcs

Severity: Normal

I think this bug can be resolved as a duplicate of #14271, which is closed. Does anyone disagree?

comment:19 Changed 3 years ago by intrigeri

Yes, it looks like a subset of #14271.

comment:20 Changed 3 years ago by mcs

Resolution: duplicate
Status: needs_revisionclosed
Note: See TracTickets for help on using tickets.