Made some progress in the development branch.

commit:
https://github.com/Whonix/control-port-filter-python/commit/83ec5e3f90539571b90137ce2e9f1b0c2d95dcc8

file:
https://github.com/Whonix/control-port-filter-python/blob/83ec5e3f90539571b90137ce2e9f1b0c2d95dcc8/usr/sbin/cpfpd

The trick is to not close the socket to Tor's real control port.

Function handle does now keep looping in fh.readline(). Now function handle also passes the socket to answer the workstation directly to do_request_real. Now in do_request_real there is another loop waiting for what Tor is (maybe) going to answer later (events) using readh.readline().

These two loops are currently blocking each other. So after the request addonion ... or so was passed, the requester (workstation) will send new control port commands in vain since we keep lurking in do_request_real.

So function do_request_real should probably become a child thread so it won't be blocking function handle.

https://github.com/Whonix/control-port-filter-python/blob/master/usr/sbin/cpfpd

Could you have a look please @marmarek? Should I go for a child thread or two instances of asyncore? I am trying to combine it with the existing StreamServer, but I don't know if either approach is a dead end.

Big progress.

• https://github.com/Whonix/control-port-filter-python/commit/e073189f6f96380b66b577d02cb6f87fe4761a82
• https://github.com/Whonix/control-port-filter-python/blob/development/usr/sbin/cpfpd

Still a work in progress and a lot cleanup of debug output and other todos but the far worst part is done.

Commented in that last commit. Generally looks good. Few things to consider:

• switching to asyncore completely - even for handling new incoming connections, replacing StreamServer - not sure if worth the effort, but will make the code even cleaner. Generally better use one API for handling sockets at the time.
• filtering responses (is it needed?)

(There is also almost no exception handling atm, but that is easy to add once the other stuff is sorted out.)

• switching to asyncore completely - even for handling new incoming connections, replacing StreamServer - not sure if worth the effort, but will make the code even cleaner.

Generally better use one API for handling sockets at the time.

Yes, generally I agree, practically I don't know if my python skills allow me to do that yet.

@marmarek wrote in https://github.com/Whonix/control-port-filter-python/commit/e073189f6f96380b66b577d02cb6f87fe4761a82#commitcomment-19064635

This looks a bit weird - handling one side with asyncore, but the other one with direct select call. I think you can handle both with asyncore - it looks like asyncore.dispatcher can accept existing socket in __init__ parameter.
This way you'll also not need that 1 sec timeout on each side.

I am absolutely not proud of that loop indeed. Too hacky. In this case, a time.wait is mostly a sign, that no proper asynchronous event / callback based handling has been implemented. Looks weird. A code bug, even if there are no functionality or security issues from it. (Does not cause high cpu yet, but I have to keep monitoring this does not waste cpu/hdd/ram.)

I try to sketch how the Tor Control Protocol filtering works...

• 1) client connects to cpfpy -> cpfpy accepts connection and keeps it open -> maybe filtered -> connect to Tor's real ControlPort -> Tor answers instantly and connection stays open -> relay answer back to client

• 2) now, maybe after an unknown time (event) Tor sends more data in the same connection -> needs to somehow also relayed back to the client

• 3) client sends more data in the same connection -> again needs to be forwarded to Tor like 1)

• 4) a separate client connection comes in

Maybe I am overthinking it, but the combination of 1), 2), 3) and 4) is difficult for me.

Let's say we had two asyncore loops. Then the client handler (in case of 3)) would have to talk to the Tor handler. Is that even possible? And if so, possible without mixing up different incoming client connections (in case of 4))?

Would these two asyncore loops have to communicate through a third asyncore loop using internal sockets?

In T448#10296, @marmarek wrote:

filtering responses (is it needed?)

We filter, control what does into Tor. And Tor is trusted. So what comes out of Tor should be safe. I haven't heard any argument how filtering Tor's responses would be useful.

In T448#10296, @marmarek wrote:

Commented in that last commit. Generally looks good.

Glad to hear! Thanks for looking!

More progress. Now there is just a single Tor connection per client. Not a as before with a bug. (Before there was one new Tor connection per client command.)

• https://github.com/Whonix/control-port-filter-python/commit/b9ea6c0519f2e2f9f5b10d52c8fdd8275deb988f
• https://github.com/Whonix/control-port-filter-python/blob/development/usr/sbin/cpfpd

For the case that Tor stops the connection (upgrade, shutdown or w/e)...

In Tor connection hander, in function handle_close I am doing self.client_socket.close().

Do you think this can be spotted in the client connection handler (function stream_server_handler)?That would be good, so I could also break the stream_server_handler` loop and close the client connection so the client application does not end up with a stale Tor control port connection.

Or asking another way around. How could the Tor connection hander communicate back to the client handler?

There is another much bigger issue, a deal breaker. Simultaneous connections (multiple applications) are no longer possible. No idea why. Looks like I have to port everything to asyncore.

Working on it, will push something in half an hour.

https://github.com/Whonix/control-port-filter-python/pull/3

Awesome! Works great!

In T448#10318, @Patrick wrote:

Awesome! Works great!

That was tested using two instances of nc and setevents stream,

However, does not yet work for real world applications (Tor Browser). Getting fragmented.

authen
ticate

2016-09-18 03:02:11,354 - CPFP 12520 log - DEBUG - Trying to start Tor control port filter on    IP 10.137.6.1 port 9052
2016-09-18 03:02:11,354 - CPFP 12520 log - DEBUG - Tor control port filter started, listening on IP 10.137.6.1 port 9052
2016-09-18 03:02:19,659 - CPFP 12520 log - DEBUG - handle_connect() done
2016-09-18 03:02:19,660 - CPFP 12520 log - DEBUG - stream_server_handler: Getting command on sock: <__main__.ControlPortFilter connected 10.137.6.41:44448 at 0x7f1d1e655200>
2016-09-18 03:02:19,660 - CPFP 12520 log - INFO - stream_server_handler: request: authen
2016-09-18 03:02:19,660 - CPFP 12520 log - DEBUG - stream_server_handler: Getting command on sock: <__main__.ControlPortFilter connected 10.137.6.41:44448 at 0x7f1d1e655200>
2016-09-18 03:02:19,660 - CPFP 12520 log - INFO - stream_server_handler: request: ticate "
2016-09-18 03:02:19,660 - CPFP 12520 log - DEBUG - handle_close() start
2016-09-18 03:02:19,660 - CPFP 12520 log - DEBUG - handle_close(): answer by Tor: 
2016-09-18 03:02:19,661 - CPFP 12520 log - DEBUG - handle_close(): Closing connection. self.client_socket.close()
2016-09-18 03:02:19,661 - CPFP 12520 log - DEBUG - handle_close(): Closed connection on sock. self.client_socket: <__main__.ControlPortFilter 10.137.6.41:44448 at 0x7f1d1e655200>

Somehow we need to read until the delimiter \r\n.

https://github.com/Whonix/control-port-filter-python/pull/4

Thanks @marmarek for helping with this :)

Progress, but still some side effects to be ironed out.

• Tor Browser control port commands work now.
• nc connection to cpfpy no longer works. (Should work, because it also works with real Tor Control Port.)
• Running tor-arm through cpfpy with filtering disabled crashes arm in konsole. So something significant that cpfpy replies differs from what the real Tor control port would reply.
• onionshare is capable to create its ephemeral Tor hidden service, but that onion is not actually reachable from Tor Browser. Could be Whonix firewall issues (unloaded all workstation firewall rules for now) or could be a cpfpy issue.

In T448#10358, @Patrick wrote:

• onionshare is capable to create its ephemeral Tor hidden service, but that onion is not actually reachable from Tor Browser. Could be Whonix firewall issues (unloaded all workstation firewall rules for now) or could be a cpfpy issue.

One reason why onionshare cannot work has just now been described here:
https://github.com/micahflee/onionshare/issues/220#issuecomment-248650516

One way would be getting these feature requests integrated into onionshare.

• https://github.com/micahflee/onionshare/issues/220#issuecomment-248650516
• https://github.com/micahflee/onionshare/issues/220

But then we would have to make similar modifications into all applications making use of Tor ephemeral services. Perhaps it is better to modify cpfpy so it automatically adds Target=<workstation IP> if it recognized the add_onion command.

There is another blocker. cpfpy now consistently uses 100% cpu.

http://www.gossamer-threads.com/lists/python/python/118657

I'm on it.

https://github.com/Whonix/control-port-filter-python/pull/5

nc connection to cpfpy no longer works. (Should work, because it also works with real Tor Control Port.)

This is because cpfpy now expects CRLF (just LF is not enough). According to spec, it is ok:

Wherever CRLF is specified to be accepted from the controller, Tor MAY also
accept LF. Tor, however, MUST NOT generate LF instead of CRLF.
Controllers SHOULD always send CRLF.

Use telnet, it works for me.

Cannot stop being amazed! :)

Yes, telnet is fine, never mind nc. (Using this for debugging only anyhow.)

For debugging purposes, I entered add_onion new:best port=80,10.137.6.41:80 into cpfpy in combination with our hidden web server instructions, which worked.

I also sufficiently hacked things on my local computer to have a onionshare proof of concept.

Now, there are just two pieces missing for this to go into production:

• a) injecting IP in cpfpy into add_onion
• b) onionshare http listener should listen on all interfaces (at least the external eth0 one)

To hack around a), luckily onionshare on restart always takes port 17600. So I could hardcoded into cpfpy.

request = "add_onion new:best port=80,10.137.6.41:17600"

I try to implement this string manipulation for a) on the fly so it works for any port, any application.

To hack around b), inside the workstation:

socat TCP-LISTEN:17600,bind=10.137.6.41,fork TCP:127.0.0.1:17600

Finding a generic solution for b) that is a bit harder.

• 1) The onionshare developer could be open to add a feature to have the webserver listen on all interfaces when it detects being run inside Whonix.
  • Not great, because not generic. We would require similar feature requests from other applications that use Tor hidden services.
• 2) Ship socat listeners by default that listen in onionshare range from 17600 to 17650.
  • Also not generic.
  • Loads of socat listeners. At some point they could even eat too much RAM if they become too many.
  • Could be made conditional by only loading these listeners if onionshare is installed. (Not great when installed inside template and not all Whonix-Workstations use onionshare.)
• 3) Can you think of any other solution?

Are network namespaces (ip-netns) of any use here? It can manipulate any application's view of the local network stack even if its not aware or supports the feature:

http://man7.org/linux/man-pages/man8/ip-netns.8.html

I don't know how ip-netns could be used here.

For linux it's a strange question to ask, looks like. Most server software supports choosing bind addresses. Local interfaces, all interfaces, selected interfaces. Just the Tor ephemeral onion options come with no such option since from there view that is pointless, even dangerous. So the question to ask is like "my server application can only bind on localhost, how can I let it bind localhost but actually have it bind on external network?" Therefore there may be no obvious tool for this purpose.

C code and LD_PRELOAD might do the magic here.

Perhaps https://github.com/yongboy/bindp can already do? It's very small, still C.

bindp even works!

BIND_ADDR="10.137.6.41" LD_PRELOAD=/home/user/bindp/libindp.so onionshare a

It would require wrappers being installed. Similar to uwt. Not pretty. Still not generic, requires one wrapper per application. (I doubt there can be a generic solution.)

What's better?

• Implement the workarounds in Whonix? Then once Whonix 14 is released, we could provide instructions how users can install onionshare, ricochet from jessie-backports.
• Or work with application developers like onionshare, ricochet so they would add modes for Whonix (or more generically called, Tor isolating proxy systems or so). Until we can preinstall onionshare, ricochet we should wait until Debian version 9 codename Stretch anyhow.

redirect application listening on localhost to listening on external interface / alternative to bindp ( libindp.so ):
http://unix.stackexchange.com/questions/311492/redirect-application-listening-on-localhost-to-listening-on-external-interface

cpfpy restart is broken.

2016-09-22 14:25:18,931 - CPFP 20603 log - INFO - Trying to start Tor control port filter on    IP 10.137.6.1 port 9052
2016-09-22 14:25:18,931 - CPFP 20603 log - INFO - Tor control port filter started, listening on IP 10.137.6.1 port 9052
2016-09-22 14:26:23,537 - CPFP 20603 log - INFO - Sigint received.
2016-09-22 14:26:23,538 - CPFP 20603 log - INFO - End.
2016-09-22 14:26:24,653 - CPFP 20606 log - INFO - Trying to start Tor control port filter on    IP 10.137.6.1 port 9052
2016-09-22 14:26:24,653 - CPFP 20606 log - CRITICAL - Server error [Errno 98] Address already in use
2016-09-22 14:26:24,653 - CPFP 20606 log - CRITICAL - Exiting.

Any idea why out sigterm / sigint / keyboard interrupt handlers do not work? They are invoked, asyncore.close_all() is run, but it does not actually close the listener.

I'm seeing iptables rules for redirecting localhost ports to an external interface. Benefit is that no wrappers are required but a port specific rule for each application supported.

https://superuser.com/questions/440324/iptables-how-to-forward-all-external-ports-to-one-local-port

EDIT:

https://superuser.com/a/807612

You may need to enable sysctl -w net.ipv4.conf.all.route_localnet=1 to allow such rules to work.

Implement the workarounds in Whonix? Then once Whonix 14 is released, we could provide instructions how users can install onionshare, ricochet from jessie-backports.

+1

I think solving this in Whonix instead of waiting on upstream cooperation which may be delayed or sometimes not possible is better.

In T448#10376, @HulaHoop wrote:

I'm seeing iptables rules for redirecting localhost ports to an external interface. Benefit is that no wrappers are required but a port specific rule for each application supported.

https://superuser.com/questions/440324/iptables-how-to-forward-all-external-ports-to-one-local-port

EDIT:

https://superuser.com/a/807612

You may need to enable sysctl -w net.ipv4.conf.all.route_localnet=1 to allow such rules to work.

That doesn't work. Steps to try this out:

https://www.whonix.org/wiki/Dev/Port_Redirection

I was wrong. Made a mistake in the port number. It does work.

Question is, if setting sudo sysctl -w net.ipv4.conf.all.route_localnet=1 is sane or has any adverse effects?

Whonix-Workstation IP injection into add_onion implemented.

https://github.com/Whonix/control-port-filter-python/commit/79ecc7cad1058b50036d1ecf3e98b5fcc3bb151b

Here are the security implications. I think they are important enough to warrant spinning off this sysctl tweak into its own package to prevent it being enabled on hosts that use the security/usability-misc packages on hosts (like mine):

https://unix.stackexchange.com/a/112232

By default this value is 0, which instructs the kernel to not route external traffic destined to 127.0.0.0/8. This is just for security as such traffic is not normal.

If you decide this is not acceptable try this alternative to sysctl altering:
https://unix.stackexchange.com/a/112213

Offtopic: In the iptables rules use eth0 to avoid conflicting addresses in case of multiple WSs on the same internal network.

That doesn't work. Same iptables command as net.ipv4.conf.all.route_localnet one.

What are the security implications of net.ipv4.conf.eth0.route_localnet=1 / route_localnet?
http://security.stackexchange.com/questions/137602/what-are-the-security-implications-of-net-ipv4-conf-eth0-route-localnet-1-rout

Merged into master branch since the remaining todo safely makes it into Whonix 14.

Security aspects...

Do you think Whonix-Workstation could spoof its client_ip and therefore lead to an security issue?

def handle_accept(self):
    conn, addr = self.accept()
    client_ip = str(addr[0])

We have to be careful about add_onion. We should not allow add_onion by default anyhow, because an open Tor hidden service makes certain attacks easier. So onionshare and similar applications will always need manual configuration. (Could be simplified one day with guis and whatnot.)

We have to be careful about add_onion. For those who enabled it... A possible attack:

A compromised Whonix-Workstation could open a Tor Hidden Service and then have it terminate on Whonix-Gateway, thereby connect to Tor's real control port. (credits: David Stainton)

Therefore and for other reasons, Tor ControlPort will be disabled in Whonix 14. (commit) CPFPY will authenticate to Tor using the Tor cookie authentication file method.

Also other ports that are normally firewalled matter.

sudo netstat -tulpen

Active Internet connections (only servers)
Proto Recv-Q Send-Q Local Address           Foreign Address         State       User       Inode       PID/Program name
tcp        0      0 10.137.6.1:9052         0.0.0.0:*               LISTEN      106        207067      32250/python    
tcp        0      0 127.0.0.1:4101          0.0.0.0:*               LISTEN      0          8599        219/brltty      
tcp        0      0 0.0.0.0:8082            0.0.0.0:*               LISTEN      0          12776       852/tinyproxy

• 9052 cpfpy itself, ok.
• 4101 brltty, bad. Perhaps no longer install brltty by default and sort that out once we work on accessibility support. (related: T457)
• 9052 tinyproxy [Qubes only], not great.

Perhaps we could restrict Tor from connecting to these ports? Limiting where user debian-tor may connect?

other TODO:

• log uncaught exceptions
• enforce max string length for incoming connections (done)
• option to use/not use add_onion IP injection feature (done)
• reject request if add_onion contains and IP of Whonix-Gateway (that includes 127.0.0.1)
• anything else?

• (Updated above post.)
• https://github.com/Whonix/control-port-filter-python/commit/f5a12054e9e63ee49cf35ba6c91b501b82b1c73c
• https://github.com/Whonix/control-port-filter-python/commit/ab11f9aeb122a5879ad70fec3b51cc81697c058f

Tails is having an interesting discussion about add_onion which really can be dangerous if not tamed:
https://labs.riseup.net/code/issues/7870#note-28

In T448#10396, @Patrick wrote:

Security aspects...

Do you think Whonix-Workstation could spoof its client_ip and therefore lead to an security issue?

Should be prevented by firewall. At least in Qubes case it is (in raw table, based on incoming interface).

Also make sure that add_onion is properly filtered/transformed. Some test cases:

• port parameter already contains IP
• multiple port parameters
• invalid/unknown parameters (should be filtered? rejected?), some could be harmful, if not now, maybe in future protocol versions

Also other ports that are normally firewalled matter.

sudo netstat -tulpen

Active Internet connections (only servers)
Proto Recv-Q Send-Q Local Address           Foreign Address         State       User       Inode       PID/Program name
tcp        0      0 10.137.6.1:9052         0.0.0.0:*               LISTEN      106        207067      32250/python    
tcp        0      0 127.0.0.1:4101          0.0.0.0:*               LISTEN      0          8599        219/brltty      
tcp        0      0 0.0.0.0:8082            0.0.0.0:*               LISTEN      0          12776       852/tinyproxy

• 9052 cpfpy itself, ok.
• 4101 brltty, bad. Perhaps no longer install brltty by default and sort that out once we work on accessibility support. (related: T457)
• 8082 tinyproxy [Qubes only], not great.

Should it be limited to templates only? This is normally done using qubes-firewall, but it is disabled on Whonix Gateway.

Perhaps we could restrict Tor from connecting to these ports? Limiting where user debian-tor may connect?

Good idea.

Do you think DaemonRunner is still of any use or could be removed?

In T448#10402, @marmarek wrote:

Also make sure that add_onion is properly filtered/transformed. Some test cases:

• port parameter already contains IP
• multiple port parameters
• invalid/unknown parameters (should be filtered? rejected?), some could be harmful, if not now, maybe in future protocol versions

Another test case: spaces in IP address. (Probably rejected by Tor.)

I am going to implement matching add_onion command (after IP injection) for all of Whonix-Gateway's IPs (which must include 127.0.0.1). If found, control command will be rejected. That way it should not be opening a listener on Whonix-Gateway. That should suffice?

Future protocol versions: we already do control what major Tor version flow into Whonix due to updates. And Tor control protocol updates fortunately only happen during major versions. It's on my list to keep reading Tor changelogs and monitor them for anything that effects Whonix / cpfpy.

• 8082 tinyproxy [Qubes only], not great.

Should it be limited to templates only? This is normally done using qubes-firewall, but it is disabled on Whonix Gateway.

Even if I have not heard and argument tinyproxy is easily exploited, I always thought it is a good idea to limit access to tinyproxy to TemplateVMs. This is not the case currently in Whonix. So not a big deal in the scope of this ticket. (We have existing Qubes tickets to improve that, qrexec based updates proxy / new firewall interface.)

I am looking for code to find out all local IPv4 and IPv6 addresses.

Does http://stackoverflow.com/a/10377262/2605155 look sane?

Just to be sure, flags sent by the WS are ignored/rewritten right?

For example the "NonAnonymous" flag will cause creation of a single hop service that is encrypted but not hidden.

https://gitweb.torproject.org/torspec.git/tree/control-spec.txt

For port choice do you think a port range whitelist is a good idea for more defense?

In T448#10419, @Patrick wrote:

Do you think DaemonRunner is still of any use or could be removed?

In theory it should handle things like "start"/"stop" actions, pid file, sigterm etc. But if we don't use any of this, probably can be removed. Also if it's always started from systemd, better let systemd manage it (i.e. run as foreground process, pidfile not needed).

In T448#10402, @marmarek wrote:

Also make sure that add_onion is properly filtered/transformed. Some test cases:

• port parameter already contains IP
• multiple port parameters
• invalid/unknown parameters (should be filtered? rejected?), some could be harmful, if not now, maybe in future protocol versions

Another test case: spaces in IP address. (Probably rejected by Tor.)

Yes. Probably this could fool filtering - like using 127 as address (which most software recognize as valid representation of 127.0.0.0).

I am going to implement matching add_onion command (after IP injection) for all of Whonix-Gateway's IPs (which must include 127.0.0.1). If found, control command will be rejected. That way it should not be opening a listener on Whonix-Gateway. That should suffice?

I'm not fully following. But isn't it better to do the other way around - allow only client IP and reject anything else?

Future protocol versions: we already do control what major Tor version flow into Whonix due to updates. And Tor control protocol updates fortunately only happen during major versions. It's on my list to keep reading Tor changelogs and monitor them for anything that effects Whonix / cpfpy.

Yes, that's important. But having a code to actually prevent such commands/options even when you miss some changelog entry would be better. Whitelisting better than blacklisting.

• 8082 tinyproxy [Qubes only], not great.

Should it be limited to templates only? This is normally done using qubes-firewall, but it is disabled on Whonix Gateway.

Even if I have not heard and argument tinyproxy is easily exploited, I always thought it is a good idea to limit access to tinyproxy to TemplateVMs. This is not the case currently in Whonix. So not a big deal in the scope of this ticket. (We have existing Qubes tickets to improve that, qrexec based updates proxy / new firewall interface.)

Yes.

- new configuration option ONION_REJECT_OWN_IPS (default: true), that will
  reject any add_onion requests that contain any IP that the gateway has
- reject request if add_onion IP injection failed rather than passing non-transformed request
- fix config parsing bug
  (fixed case when there is a config file but still some value undefined)
- log to stdout rather than /var/log/control-port-filter-python.log, so cpfpy
  output ends up in journal
- deprecated /var/log/control-port-filter-python.log
- no longer use DaemonRunner, no longer fork, leave that to systemd
- remove unused class UnexpectedAnswer(Exception)

https://github.com/Whonix/control-port-filter-python/commit/88aafe059bb05dd1175148d00f635db6ff020cbd

In T448#10422, @HulaHoop wrote:

Just to be sure, flags sent by the WS are ignored/rewritten right?

Excellent point!

For example the "NonAnonymous" flag will cause creation of a single hop service that is encrypted but not hidden.

Rewrite not for now. Will reject NonAnonymous. If there ever are any legitimate applications using flag NonAnonymous by default where upstream cooperation fails, we can still purge NonAnonymous from the request string.

Any other flags we should block?

For port choice do you think a port range whitelist is a good idea for more defense?

Not sure. I want to limit the amount of string parsing and string manipulation. Not for security, but to keep the code simpler. These ports are very application specific. Now that we have ONION_REJECT_OWN_IPS, that should not be necessary.

In T448#10436, @marmarek wrote:

Also make sure that add_onion is properly filtered/transformed. Some test cases:

• port parameter already contains IP
• multiple port parameters
• invalid/unknown parameters (should be filtered? rejected?), some could be harmful, if not now, maybe in future protocol versions

Another test case: spaces in IP address. (Probably rejected by Tor.)

Yes. Probably this could fool filtering - like using 127 as address (which most software recognize as valid representation of 127.0.0.0).

I am going to implement matching add_onion command (after IP injection) for all of Whonix-Gateway's IPs (which must include 127.0.0.1). If found, control command will be rejected. That way it should not be opening a listener on Whonix-Gateway. That should suffice?

I'm not fully following. But isn't it better to do the other way around - allow only client IP and reject anything else?

It's better, but also harder. Then it is needed to fully parse and understand the add_onion command from within python. Originally I wanted to prevent going into the deepness of the protocol from within cpfpy.

Future protocol versions: we already do control what major Tor version flow into Whonix due to updates. And Tor control protocol updates fortunately only happen during major versions. It's on my list to keep reading Tor changelogs and monitor them for anything that effects Whonix / cpfpy.

Yes, that's important. But having a code to actually prevent such commands/options even when you miss some changelog entry would be better. Whitelisting better than blacklisting.

Right. I will work on fully parsing the add_onion command and only sent what has been sanitized and reconstructed.

I'm not fully following. But isn't it better to do the other way around - allow only client IP and reject anything else?

+1

I think whitelisting is much safer than blacklisting anywhere. Imagine supporting IPv6 addresses someday and not updating the blacklist, then things can easily get by it or there is some obscure way to represent local addresses that we don't know about yet.

Manually hardcoding some subset of the 10.152.152.10 address range is better. I doubt users need more than 100 IPs on the internal network.

Any other flags we should block?

Lets do it with whitelists:

• KeyType - only allow NEW
• KeyBlob - only allow BEST
• Flag - only allow DiscardPK, Detach, BasicAuth

• VirtPort - port ranges are useful so an attacker cannot use up resources by enabling every port on the GW. IMO having this configurable .d style is good for customization.

• ClientName - only allow input formatted like so: "An identifier 1 to 16 characters long, using only characters in A-Za-z0-9+-_ (no spaces)."

• ClientBlob - need more info to make decisions about expected input size.

I think there should be some way to put an upper bound on the number of ephemeral HSes allowed to be created. A DoS that can exhaust the GW resources by spamming ADD_ONION until its RAM/CPU is used up is very bad and can eventually have network visible consequences.

I think by this point users should be warned not to run multiple internal nets with different trust levels behind the same GW because its not possible to stop a malicious process in one internal net asking about the ephemeral service of a process in the other. This affects Qubes-Whonix where such a feature is possible.

Parsing the whole add_onion command seems incredibly complex. There are too many combinations. I am not sure I can implement all sane features or if we should do that. Perhaps just a subset. Perhaps it just be tailored for real world applications.

In T448#10441, @HulaHoop wrote:

I'm not fully following. But isn't it better to do the other way around - allow only client IP and reject anything else?

+1

I think whitelisting is much safer than blacklisting anywhere. Imagine supporting IPv6 addresses someday and not updating the blacklist, then things can easily get by it or there is some obscure way to represent local addresses that we don't know about yet.

Currently IPv6 addresses are already enumerated. But the obscure representations are indeed an issue.

Manually hardcoding some subset of the 10.152.152.10 address range is better. I doubt users need more than 100 IPs on the internal network.

Any other flags we should block?

Lets do it with whitelists:

• KeyType - only allow NEW
• KeyBlob - only allow BEST

That would mean, no support for getting hidden services with saved private keys back online. I guess it's not what you had in mind?

From the examples: ADD_ONION RSA1024:[Blob Redacted] Port=80,192.168.1.1:8080

• Flag - only allow DiscardPK, Detach, BasicAuth

• VirtPort - port ranges are useful so an attacker cannot use up resources by enabling every port on the GW. IMO having this configurable .d style is good for customization.

VirtPort is the port at the Tor hidden service, not on the gateway. You are suggesting to limit target ports?

I think there should be some way to put an upper bound on the number of ephemeral HSes allowed to be created. A DoS that can exhaust the GW resources by spamming ADD_ONION until its RAM/CPU is used up is very bad and can eventually have network visible consequences.

Makes sense but seems hard to implement. cpfpy would have to globally track the number of HSes and make that somehow survive cpfpy restarts.

---

I think by this point users should be warned not to run multiple internal nets with different trust levels behind the same GW because its not possible to stop a malicious process in one internal net asking about the ephemeral service of a process in the other.

As far I understand, Tor ephemeral HSes in one Tor control connection (from ws one) cannot be touched from another Tor control connection. The flag Detach may or may not influence that.

Yes. I would go as far recommending against using the same gateway for hosting hidden services (let alone ephemeral hidden services) for other tasks such as simple browsing.

Parsing the whole add_onion command seems incredibly complex. There are too many combinations. I am not sure I can implement all sane features or if we should do that. Perhaps just a subset. Perhaps it just be tailored for real world applications.

Agreed. I think it only makes sense to accept predefined strings that have parameters ordered a certain way - there is a few of those in use right now anyway.

That would mean, no support for getting hidden services with saved private keys back online. I guess it's not what you had in mind?

Do you know if any onion apps use these older ciphers? I thought the ECC keys allowed completely offline HS keys. The idea here is to prevent a malicious process from downgrading the crypto used. If that has little realworld use then sure add support for RSA1024 too.

You are suggesting to limit target ports?

Yes.

Makes sense but seems hard to implement. cpfpy would have to globally track the number of HSes and make that somehow survive cpfpy restarts.

I thought about it more and it should be Tor that handles this not cpfpy. For example Tor rate limits newnym requests that are made too soon. I think this needs to be discussed with upstream.

I guess we don't get around unit testing now. Do you have any examples where I can define function names, inputs and expected outputs?

Wrote a test script for this purpose. Now everything is white listed. This is how I'd built it into cpfpy soon. More maliciously formed add_onion request string suggestions appreciated.

#!/usr/bin/python

import logging
import os
import sys

client_ip = "555.555.555.555"
ADD_ONION_INJECT_IP = True

#"ADD_ONION RSA1024:[Blob Redacted] Port=80,192.168.1.1:8080",

add_onion_test = [
 "ADD_ONION NEW:BEST Flags=DiscardPK Port=80",
 "ADD_ONION NEW:BEST Port=22 Port=80,8080",
 "ADD_ONION NEW:BEST Flags=DiscardPK,BasicAuth Port=22",
 "ADD_ONION NEW:BEST Flags=DiscardPK Port=22",
 "ADD_ONION NEW:BEST Flags=DiscardPK,NonAnonymous Port=22",
 "ADD_ONION NEW:BEST Flags=DiscardPK Port=22",
 "ADD_ONION NEW:BEST Flags=DiscardPK,NonAnonymous Port=22",
 "add_onion new:best port=80,17600",
 "add_onion new:best port=80,127.0.0.1:17600",
 "add_onion new:best port=8 0,17600"
 "add_onion new:best port=80 : 17600",
 "add_onion new:best port=80 : 17600 Flags=DiscardPK",
 "add_onion new:best port= 80:17600 Flags=DiscardPK",
];

## TODO: blob support
## TODO: do not lower KeyBlob
## TODO: do not lower ClientBlob
def add_onion_parse(request, client_ip, inject):
    ## ADD_ONION KeyType:KeyBlob
    ## Flags=Flag,Flag
    ## Port=VirtPort,TargetIP:TargetPort
    ## ClientAuth=ClientName
    ## ClientAuth=ClientName:ClientBlob

    request_processed = ""

    block_counter = 0
    for block in request.split(" "):
        block_counter = block_counter + 1
        #logger.info('block %s: %s' % (str(block_counter), block))
        if block_counter == 1 and block == "add_onion":
           request_processed = "add_onion"
           continue
        if block_counter == 2:
           KeyTyp = block.split(":")[0]
           KeyBlob = block.split(":")[1]
           if KeyTyp == "new":
               request_processed = request_processed + " " + "new"
           elif KeyTyp == "rsa1024":
               request_processed = request_processed + " " + "rsa1024"
           else:
               logger.error('KeyType not whitelisted! KeyTyp: %s' % (KeyType))
               return False
           request_processed = request_processed + ":" + KeyBlob
           continue

        first = block.split("=")[0]
        second = block.split("=")[1]
        if first == "flags":
            request_processed = request_processed + " " + "flags="
            for single_flag in second.split(","):
                #logger.info('single_flag: %s' % (single_flag))
                if single_flag == "discardpk":
                   request_processed = request_processed + "discardpk,"
                elif single_flag == "detach":
                   request_processed = request_processed + "detach,"
                elif single_flag == "basicauth":
                   request_processed = request_processed + "detach,"
                else:
                   logger.error('Flag not whitelisted! flag: %s' % (single_flag))
                   return False

            ## Remove extraneous ',' at the end.
            request_processed = request_processed[:-1]
        elif first == "port":
           request_processed = request_processed + " " + "port="
           port = second

           ##    port=22
           ## vs port=80,17600

           ## Format such as 'Port=80,192.168.1.1:8080' not yet supported.

           if len(port.split(",")) == 1:
               if not port.isdigit():
                   logger.error('port is no digit! port: %s' % (str(port)))
                   return False
               request_processed = request_processed + port + ","
           elif len(port.split(",")) == 2:
               port_counter = 0
               for port_single in port.split(","):
                  if not port_single.isdigit():
                     logger.error('port_single is no digit! port_single: %s' % (str(port_single)))
                     return False

                  #logger.info('port_single %s: %s' % (str(port_counter), port_single))
                  port_counter = port_counter + 1
                  if port_counter == 1:
                      request_processed = request_processed + port_single + ","
                  elif port_counter == 2:
                      if inject == True:
                          request_processed = request_processed + client_ip + ":" + port_single + ","
                      else:
                          request_processed = request_processed + ":" + port_single + ","
                  else:
                     logger.error('len(port.split(",")) too long!')
                     return False

           ## Remove extraneous ',' at the end.
           request_processed = request_processed[:-1]

    return request_processed


mypid = os.getpid()
log_prefix = "CPFP " + str(mypid) + " log"
logger = logging.getLogger(log_prefix)
logger.setLevel(logging.DEBUG)

formatter = logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s")
handler = logging.StreamHandler(sys.stdout)
handler.setFormatter(formatter)
logger.addHandler(handler)

for request in add_onion_test:
    logger.info('request_requested: %s' % (request))
    request = request.lower().strip()
    request_startswith = request.split(' ', 1)[0]
    if request_startswith == "add_onion":
        try:
            request_processed = add_onion_parse(request, client_ip, ADD_ONION_INJECT_IP)
        except:
            exception_msg = str(sys.exc_info()[0])
            logger.error('exception_msg: %s' % (exception_msg))
            request_processed = False
        logger.info('request_processed: %s' % (request_processed))
        print ""

2016-09-27 16:26:19,283 - CPFP 9680 log - INFO - request_requested: ADD_ONION NEW:BEST Flags=DiscardPK Port=80
2016-09-27 16:26:19,283 - CPFP 9680 log - INFO - request_processed: add_onion new:best flags=discardpk port=80

2016-09-27 16:26:19,283 - CPFP 9680 log - INFO - request_requested: ADD_ONION NEW:BEST Port=22 Port=80,8080
2016-09-27 16:26:19,283 - CPFP 9680 log - INFO - request_processed: add_onion new:best port=22 port=80,555.555.555.555:8080

2016-09-27 16:26:19,283 - CPFP 9680 log - INFO - request_requested: ADD_ONION NEW:BEST Flags=DiscardPK,BasicAuth Port=22
2016-09-27 16:26:19,283 - CPFP 9680 log - INFO - request_processed: add_onion new:best flags=discardpk,detach port=22

2016-09-27 16:26:19,283 - CPFP 9680 log - INFO - request_requested: ADD_ONION NEW:BEST Flags=DiscardPK Port=22
2016-09-27 16:26:19,283 - CPFP 9680 log - INFO - request_processed: add_onion new:best flags=discardpk port=22

2016-09-27 16:26:19,283 - CPFP 9680 log - INFO - request_requested: ADD_ONION NEW:BEST Flags=DiscardPK,NonAnonymous Port=22
2016-09-27 16:26:19,283 - CPFP 9680 log - ERROR - Flag not whitelisted! flag: nonanonymous
2016-09-27 16:26:19,283 - CPFP 9680 log - INFO - request_processed: False

2016-09-27 16:26:19,283 - CPFP 9680 log - INFO - request_requested: ADD_ONION NEW:BEST Flags=DiscardPK Port=22
2016-09-27 16:26:19,283 - CPFP 9680 log - INFO - request_processed: add_onion new:best flags=discardpk port=22

2016-09-27 16:26:19,283 - CPFP 9680 log - INFO - request_requested: ADD_ONION NEW:BEST Flags=DiscardPK,NonAnonymous Port=22
2016-09-27 16:26:19,283 - CPFP 9680 log - ERROR - Flag not whitelisted! flag: nonanonymous
2016-09-27 16:26:19,284 - CPFP 9680 log - INFO - request_processed: False

2016-09-27 16:26:19,284 - CPFP 9680 log - INFO - request_requested: add_onion new:best port=80,17600
2016-09-27 16:26:19,284 - CPFP 9680 log - INFO - request_processed: add_onion new:best port=80,555.555.555.555:17600

2016-09-27 16:26:19,284 - CPFP 9680 log - INFO - request_requested: add_onion new:best port=80,127.0.0.1:17600
2016-09-27 16:26:19,284 - CPFP 9680 log - ERROR - port_single is no digit! port_single: 127.0.0.1:17600
2016-09-27 16:26:19,284 - CPFP 9680 log - INFO - request_processed: False

2016-09-27 16:26:19,284 - CPFP 9680 log - INFO - request_requested: add_onion new:best port=8 0,17600add_onion new:best port=80 : 17600
2016-09-27 16:26:19,284 - CPFP 9680 log - ERROR - exception_msg: <type 'exceptions.IndexError'>
2016-09-27 16:26:19,284 - CPFP 9680 log - INFO - request_processed: False

2016-09-27 16:26:19,284 - CPFP 9680 log - INFO - request_requested: add_onion new:best port=80 : 17600 Flags=DiscardPK
2016-09-27 16:26:19,284 - CPFP 9680 log - ERROR - exception_msg: <type 'exceptions.IndexError'>
2016-09-27 16:26:19,284 - CPFP 9680 log - INFO - request_processed: False

2016-09-27 16:26:19,284 - CPFP 9680 log - INFO - request_requested: add_onion new:best port= 80:17600 Flags=DiscardPK
2016-09-27 16:26:19,284 - CPFP 9680 log - ERROR - port is no digit! port: 
2016-09-27 16:26:19,284 - CPFP 9680 log - INFO - request_processed: False

In T448#10444, @HulaHoop wrote:

That would mean, no support for getting hidden services with saved private keys back online. I guess it's not what you had in mind?

Do you know if any onion apps use these older ciphers? I thought the ECC keys allowed completely offline HS keys. The idea here is to prevent a malicious process from downgrading the crypto used. If that has little realworld use then sure add support for RSA1024 too.

It doesn't look like ECC HS keys are implemented yet.

You are suggesting to limit target ports?

Yes.

Why? If we make double sure the target IP will be the client IP who sent the add_onion command...

Port ranges are very application specific.

And limiting them does not help. The adversary could also bind to an allowed port and then just using something like socat to redirect it to a port of his choice.

Makes sense but seems hard to implement. cpfpy would have to globally track the number of HSes and make that somehow survive cpfpy restarts.

I thought about it more and it should be Tor that handles this not cpfpy. For example Tor rate limits newnym requests that are made too soon. I think this needs to be discussed with upstream.

Yes. Can you take this upstream please?

The list_ephemeral_hidden_services parameter can be used to ask Tor to list all ephemeral services. By default it shows those created by a controller with the detached boolean as optionally including services who's continuation isn't tied to a controller.

Down the page:
detached (bool) -- continue this hidden service even after this control connection is closed if True

So if detached is allowed in cpfp-py, the number of services including them must be queried.

https://stem.torproject.org/api/control.html

Idea: Every time an ADD_ONION request hits cpfp, it checks the list to see how many ephemeral services are active and if the number exceeds a preconfigured limit, the request is silently dropped (optionally with a generic log message if you think that is useful).

If this is implemented successfully you can parse the number of ports provided in an ADD_ONION request and make sure they are not excessive - only 3 or 4 allowed per request and a total of 10 services allowed - 30-40 ports maximum open at any time is not so bad and is more flexible than an arbitrarily fixed port range.

In T448#10482, @Patrick wrote:

I guess we don't get around unit testing now. Do you have any examples where I can define function names, inputs and expected outputs?

Take a look at https://docs.python.org/2/library/unittest.html
Generally, add a new class inheriting from unittest.TestCase, then add method(s) named test_something and use self.assertEqual(expected_result, actual_result) (or other method - see linked documentation). Then call it: python -m unittest path/to/file

Some example: https://github.com/marmarek/qubes-core-admin/blob/core3-firewall/qubes/tests/firewall.py

It's easier to write unit tests when code is sufficiently divided to functions: for example have separate prepare_response and send_response instead of prepare_and_send_response. But even in the later case, you can create wrapper class (just for testing purpose) to override self.send with something that will just save the data for later checking.

It is common practice to have tests in separate file, but that is possible only when the actual code is installed as a python module, not directly in executable. Not sure if worth changing it here, probably not.

Unit test suggestions:

Include some strings with:

• excessive characters for each parameter - imagine case of adversary injecting a load of rubbish when keyblob is supported
• invalid port numbers - ports that don't exist
• a query stress test - bombarding cpfp with a very high volume of valid requests. Does cpfp fail gracefully? How does it cope with a simulated DDoS?

https://lists.torproject.org/pipermail/tor-dev/2016-September/011477.html

EDIT:

How to change the number of open files limit in Linux:

https://stackoverflow.com/a/923369

In T448#10484, @HulaHoop wrote:

Idea: Every time an ADD_ONION request hits cpfp, it checks the list to see how many ephemeral services are active and if the number exceeds a preconfigured limit, the request is silently dropped (optionally with a generic log message if you think that is useful).

Created T564 for it.

If this is implemented successfully you can parse the number of ports provided in an ADD_ONION request and make sure they are not excessive - only 3 or 4 allowed per request and a total of 10 services allowed - 30-40 ports maximum open at any time is not so bad and is more flexible than an arbitrarily fixed port range.

Only 5 concurrent connections at the same time are allowed:
https://github.com/Whonix/control-port-filter-python/blob/6a131266a8dc8f98ff22a3b83fae9d43e38b3127/usr/sbin/cpfpd#L468-L470

I don't think I'll implement creating multiple virtual ports or multiple target ports in a single add_onion command at this time. Gets too complex. And there is no incentive since no real world applications are doing this. Let's move add_onion command parsing to T562.

In T448#10490, @HulaHoop wrote:

Unit test suggestions:

Include some strings with:

• excessive characters for each parameter - imagine case of adversary injecting a load of rubbish when keyblob is supported

Excessively long inputs are already rejected earlier than parsing add_onion.

https://github.com/Whonix/control-port-filter-python/blob/6a131266a8dc8f98ff22a3b83fae9d43e38b3127/usr/sbin/cpfpd#L272

KeyBlob won't be supported soon since no real world applications are using it. Also probably neither ClientName / ClientBlob will be supported. But I added a comment so we not forget to impose maximum string lengths for them if/when it gets implemented.

• invalid port numbers - ports that don't exist

Good idea. Added to T562.

• a query stress test - bombarding cpfp with a very high volume of valid requests. Does cpfp fail gracefully? How does it cope with a simulated DDoS?

Created T565 for it.