On 19:46 +0100 / 23 Jul, Luke Plant wrote:
> On 23/07/12 14:24, Rohan Jain wrote:
> > With this, attacker won't be able to directly set arbitrary tokens on
> > other sub domains through cookies, they will need a signature of the
> > token with the form which is to be verified against the cookie.
> > Plus it also puts a limit on the duration a token stays valid on the
> > server side.
> >
> > Yes, still with this, someone can spoof the whole pair using a separate
> > legitimate session. So really it doesn't make it completely secure,
> > just makes it little difficult for the attacker.
>
> So, to make it clear:
>
> Attacker controls evil.example.com, and wants to attack example.com. By
> appropriate setting of a cookie, and by providing a matching token in
> the form, they can forge a request to example.com
>
> With your proposed change, they are in no way hampered. Using HMAC to do
> sign the value, and a timestamp, as you mention, they can simply
> regularly directly contact example.com and pull in a valid token/cookie
> value pair, which they can use since there is no correlation to the
> session of the person being attacked. I'm pretty sure this can be done
> entirely in Javascript too.
>
> Yes, this change would make it "a little difficult". But the value of
> "little" is very small. It's like suggesting we add ROT 13 encryption to
> one of the values - sure it makes it a "little" more difficult, but it
> does not *materially* affect the feasibility of the attack. The
> resources they need are identical: the ability to read the Django source
> code, and the subdomain control that they have in both cases.
>
> On the other hand, this adds significant complication to our code, which
> is the last thing you need for security related code. I'm -1 on this
> change. I did highlight all these things before.
>
>
> > If it weren't for the possibility of attacker injecting cookies from
> > other subdomains, I think CSRF token should be a fine check for
> > CSRF.
> >
> > That is why I am siding on adding referer checking in case of non
> > https scheme requests too.
>
> I really don't think we can consider this - for HTTP, proxies can and do
> strip the referer header. Quoting from Barth, Jackon and Mitchell:
>
> <<<
> Over HTTP, sites cannot afford
> to block requests that lack a Referer header because
> they would cease to be compatible with the sizable
> percentage (roughly 3–11%) of users
> >>
>
> This makes strict referer checking a non-started, and lax referer
> checking (only check it if it is present) has known flaws.
Thanks for this quote. I didn't know about this paper and the fact
that the web behaves differentially with respect to referer checking
in http from https earlier. So, yes relying on it isn't the best idea.
I had one more idea, "Pluggable CSRF checkers".
Currently, the CSRF middleware has two kinds of checks, referer (for
https) and secret validation token (common). These with origin header
based checker (if we add it) come in conditional blocks, making
switching them difficult. So what I propose to do is decouple their
logic from CSRF middleware and each of them provide a checker. It goes
like this:
A setting for configuring global CSRF checkers:
CSRF_CHECKERS = {
'django.middleware.csrf.checkers.OriginChecker',
# This one can be strict for https and lax for http
'django.middleware.csrf.checkers.RefererChecker',
# contrib.sessions could provide a csrf checker maintained
# with sessions. This stores the token in session data.
'django.contrib.sessions.csrf_checkers.SessionChecker'
}
Views can have their own custom checkers:
from django.view.decorators.csrf import csrf_checkers
@csrf_checkers(
'django.middleware.csrf.checkers.OriginChecker',
'django.middleware.csrf.checkers.TokenChecker',
)
def foo(request):
'''This view needs token and origin for CSRF checks. For some
reason session can't be used here.'''
@csrf_checkers(
'django.middleware.csrf.checkers.OriginChecker',
'django.middleware.csrf.checkers.SignedTokenChecker',
)
def bar(request):
'''This view for some reason needs time signed token and
origin for CSRF checks.'''
A base checker class:
class BaseCSRFChecker(object):
def __init__(request, *args, **kwargs):
self.request = request
def _accept(self, request):
request.csrf_processing_done = True
return None
def _reject(self, request, reason):
return _get_failure_view()(request, reason=reason)
def _check(strict=True):
raise NotImplementedError ("Checkers must implement a checking
mechanism.")
def check(strict=True):
# Checkers will implement the method in their
self._check(strict=strict)
With this I can see some benefits:
- More explicit about the presence of CSRF checking.
- Easier to modify CSRF behaviour in the future.
- Possibility of using contrib.sessions for CSRF, while not
completely tying CSRF to the app.
- Custom checkers can make people prevent using csrf_exempt for some
views (Maybe something which isn't supposed to be accessed with
cookies).
This might look like too much overkill or complication but I think if
not making it faster, this at least won't add any overhead to the
middleware.
--
Thanks
Rohan
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
