txtorcon icon indicating copy to clipboard operation
txtorcon copied to clipboard

Warn users / sysadmins of potential Twisted endpoint-using application security risk.

Open nathan-at-least opened this issue 9 years ago • 4 comments

This isn't an issue specific to txtorcon per se, but general to twisted endpoints. However, I believe txtorcon should educate its own users about this issue, and perhaps attempt to mitigate it for the Tor endpoints.

While chatting about integrating Tor into Foolscap, we realized that the endpoint client specification strings are dangerous for a certain class of application, such as those often supported by Foolscap (see foolscap #203 comment 47).

The issue is that some applications may naively use endpoint specifiers similar to URLs where they are treated as public addresses passed around in an application (over the net). Foolscap applications often pass "FURLs" around which are URLs that specify a capability for remote procedure calls to an object. For this kind of application, a client endpoint specifier is dangerous, because it allows the creator of the endpoint specification to control sensitive client-side behavior.

A useful case is on Getting Connected with Endpoints for the SSL client example:

ssl:host=twistedmatrix.com:port=443:caCertsDir=/etc/ssl/certs

For an application like a browser (or many "p2p" style applications) letting a remote entity specify this is disasterous, since they can control the CA trust root. Worse may be possible, such as by specifying a confidential file path as a certificate, which may transmit the file contents as part of an SSL handshake.

This issue is exacerbated by the "plugin" nature of endpoint parsers. If a naive application is installed and there are also SSL or Tor endpoints, then an attacker may be able to exploit that naive application. The application author may have never even heard of Tor, so we cannot rely on them to protect against this. The user may not realize that installing the naive application, then separately installing txtorcon would be sufficient to trigger this case.

One mitigating factor may be that few applications use Twisted endpoints, and hopefully none of them do so naively.

nathan-at-least avatar Mar 19 '15 17:03 nathan-at-least

I agree that passing un-sanitized strings as endpoint specifiers may indeed result in surprising behavior.

Any code that uses Twisted endpoints would end up passing the strings to clientFromString or serverFromString and receive back an IStreamClientEndpoint (or IStreamServerEndpoint). You could then whitelist acceptable endpoint subclasses.

Alternatively, you could whitelist acceptable prefix-strings, since (I believe) this is all Twisted is doing to identify parsers. So endpointstring.split(':')[0] in whitelist would be sufficient to check. Looking at the returned object would obviously be slightly better, as from the string-checking you still don't know precisely which parser Twisted will call.

Do you have a suggestion as to what txtorcon can/should do?

e.g. a pull-request suggesting documentation changes, or ...? Would an example of doing the above, with a Sphinx/rST note calling it out ... somewhere ... be sufficient? (I guess I don't know where that "somewhere" should be for maximum effectiveness).

meejah avatar Mar 19 '15 19:03 meejah

Note that you should treat a colon (:) as illegal for individual args as you're building up (sanitized) endpoint strings, as Twisted uses those to delimit options in the strings.

meejah avatar Mar 19 '15 19:03 meejah

You're right that there's not clear criteria for closing this ticket... Hrm... I'll think about it.

nathan-at-least avatar Mar 19 '15 21:03 nathan-at-least

Bump: this is an interesting issue, but I don't have a good answer yet. Since it was filed, though, the documentation (on master) is mostly-new. Perhaps now there's a good place to call this out in a "note:" or similar.

meejah avatar Feb 23 '17 00:02 meejah