From mboxrd@z Thu Jan  1 00:00:00 1970
From: lekensteyn at gmail.com (Peter Wu)
Date: Wed, 15 Jan 2014 10:28:41 +0100
Subject: authentication support: work has begun!
In-Reply-To: <CAHmME9pLfxyuEPYa2c92Gq4+O4aPkP+_pfUv-N1RMUPpPvVUjw@mail.gmail.com>
References: <CAHmME9pLfxyuEPYa2c92Gq4+O4aPkP+_pfUv-N1RMUPpPvVUjw@mail.gmail.com>
Message-ID: <14648906.FqII7cU9cN@al>

Hi,

On Wednesday 15 January 2014 02:02:13 Jason A. Donenfeld wrote:
> While still a horrendous mess, I've begun work adding authentication
> support, using our nice new lua filter system.
> 
> A sample script looks like this [at the moment]:
> 
> http://git.zx2c4.com/cgit/tree/filters/simple-authentication.lua?h=jd/authen
> tication
> 
> The full commit of this attrocity looks like this:
>    http://git.zx2c4.com/cgit/commit/?h=jd/authentication
> 
> It's far from finished or polished, but I thought I'd give everyone a
> preview of it. It's running on:
>     http://git.zx2c4.com/glouglou/log
> 
> Currently just enter anything you want as username and password. It will
> set a cookie. Check out the HTTP headers and response and everything to see
> what's happening.
> 
> Preliminary comments?

The script is vulnerable to header injection:

$ curl -i http://git.zx2c4.com/login -H 'Referer: x%0d\nX: 1' \
  -d 'username=1; path%3d/&password=%0aY: 2'
HTTP/1.1 302 Redirect
Server: ZX2C4 Web Server
Date: Wed, 15 Jan 2014 08:54:00 GMT
Transfer-Encoding: chunked
Connection: keep-alive
Keep-Alive: timeout=20
Location: x%0d\nX: 1
Set-Cookie: auth=1
Set-Cookie: username=1; path=/
Set-Cookie: password=
Y: 2

While the referrer part may not be that easily spoofable, the post fields are
a different matter. Speaking of the referrer header, that field might not be
set at all. What about making the URL available in a post field  "return-url"?
It still has to be validated though.

For the cookie, I suggest to add "; HttpOnly" to Set-Cookie to prevent cookie
theft through XSS. If possible, also add "; Secure" to prevent leakage through
HTTP when HTTPS is used.

An important consideration is caching. Adding the Set-Cookie header disables
caching for nginx at least, but other authenticated requests can still be
cached.

This authentication mechanism is unsafe if the transport is not encrypted
(i.e. use HTTPS!), passwords are then leaked in the air. You mentioned
using a HMAC, but what data do you want to protect? For best results,
some state has to be retained. The authentication does not have to rely on 
cookies either, it can use client SSL certificates too.

What if the script fails to load (syntax error)? Is access then granted to
everyone, silently ignoring the error? That would be bad, it should then
return an 500 Internal server error.

Regards,
Peter