Ponder this SSRF talk

[njsmith]

https://www.blackhat.com/docs/us-17/thursday/us-17-Tsai-A-New-Era-Of-SSRF-Exploiting-URL-Parser-In-Trending-Programming-Languages.pdf

h11 in client mode refuses to let you send a Host: header with an embedded newline, which protects against some of the attacks they're talking about. (This apparently distinguishes us from every Python HTTP library they studied.)

Do we validate outgoing request targets? (The / part in GET /?)

Is there anything else in there that's in scope for us? We certainly can't do anything about SNI. We don't actually deal with URLs, though I have been thinking that we should perhaps do more validation of incoming request targets in the rare case where there's something useful to validate (like checking that the authority if given matches the Host:).

The IDNA stuff is definitely not in scope for h11, but I'll note in passing that I'm confused: their table doesn't match the behavior of the idna module at all. The "uts46" column matches what you see with uts46=True, transitional=True (which is a mode that UTS-46 tries hard to dissuade you from using -- they want you to use uts46=True, transitional=False). If you use uts46=True, transitional=False, then the circled letters one gives google.com but the \u200D one is an error and baß.de gives xn--ba-hia.de. And if you use pure IDNA2008 (which literally nobody does, it errors out on capital letters), then the \u200D one still gives an error.

[njsmith]

It looks like we currently don't validate outgoing request targets:

In [8]: c = h11.Connection(our_role=h11.CLIENT)

In [9]: c.send(h11.Request(method="GET", target="/hello HTTP/1.1\r\nxx-evil: acc
   ...: omplished\r\nversion:", headers=[("Host", "victim.com")]))
Out[9]: 
b'GET /hello HTTP/1.1\r\nxx-evil: accomplished\r\nversion: HTTP/1.1\r\n'
b'host: victim.com\r\n\r\n'

We probably should do something here, though I'm not quite sure what. The spec does give ABNF for the request-target, but I'm not at all sure that people respect it in practice (and it's also quite complicated, relying on references to internal productions in RFC 3986).

We could at least validate that it doesn't contain space/carriage return/newline.

[njsmith]

Here's a recent discussion about target validation in CPython (got a CVE etc.): https://bugs.python.org/issue36276

They link to this recent golang bug fix, which AFAICT rules out all ascii control characters from URLs, so including both targets and hosts: golang/go@829c5df

We should apply some kind of validation to outgoing targets, at a minimum ruling out \0, \n, \r, (space).

[njsmith]

We added more restrictions on targets in #69. In particular it should now be impossible to do header-stuffing by setting a target containing metacharacters, which is the really nasty security issue. So I guess this can be closed.