authentication support: work has begun!

[Jason A. Donenfeld]

On Wed, Jan 15, 2014 at 10:28 AM, Peter Wu <lekensteyn at gmail.com> wrote:
>
> 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.
>

Yes, of course. I was aware of this too. Clearly the current spitting out
of headers is unfinished. In the first place, I wouldn't be spitting the
password and username post directly out in the cookies ("-- We wouldn't
actually want to set these cookies... Just for testing." in the comments),
let alone leaving everything completely unvalidated. Rest assured, this
won't make it to the master branch.


>
> 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.
>

I had planned on this too. A good suggestion. None of the cookie generation
is complete right now.


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

Not completely though. I've taken careful precaution to ensure that caching
from the cgit end stays in tact. If the cookie is authenticated, then cgit
is able to serve up the cached pages from its own cache. If the cookie is
unauthenticated, then, yes, it displays an uncached version of the "please
authenticate" page.

I did not check the ramification this has on nginx's handling of HTTP
caching of resources, however. Can you elaborate on this, if it's a
problem, and how to mitigate it?


>
> 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.
>

Obviously. All authentication mechanisms in the browser that do not go over
HTTPS are vulnerable. If you have HTTPS, then excellent. If you don't, then
you should be aware of how vulnerable you will be without it. There will be
a note in the documentation about this naturally.

The HMAC mention doesn't have to do with cleartext vs non-cleartext. It's
for this reason -- I'm not going to be relying in an "auth=1" cookie for
authentication passing. This is trivially spoofable. Instead, there's going
to be something like "${username}|${expiration-unix-time}|${salt}|${hmac}",
so that such state is stored in the cookie itself, but then validated
server side using a secret.


>
> 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.


If the script fails to load due to a syntax error, cgit will bail out. It
"fails safe" in this regard.


Thanks for your feedback! I'll ping you when I've finished the web side of
things and you can let me know if it's satisfactory.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20140115/5dcd4a62/attachment.html>