Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

feature request: ability to set _xsrf cookie expiration #462

Open
tzuryby opened this Issue Feb 17, 2012 · 5 comments

Comments

Projects
None yet
2 participants

tzuryby commented Feb 17, 2012

I wanted to be able to set _xsrf expiration to 1 day rather than current 30 days.
I have search the code to see where this happens and found this.

    @property
    def xsrf_token(self):
        """The XSRF-prevention token for the current user/session.

        To prevent cross-site request forgery, we set an '_xsrf' cookie
        and include the same '_xsrf' value as an argument with all POST
        requests. If the two do not match, we reject the form submission
        as a potential forgery.

        See http://en.wikipedia.org/wiki/Cross-site_request_forgery
        """
        if not hasattr(self, "_xsrf_token"):
            token = self.get_cookie("_xsrf")
            if not token:
                token = binascii.b2a_hex(uuid.uuid4().bytes)
                expires_days = 30 if self.current_user else None
                self.set_cookie("_xsrf", token, expires_days=expires_days)
            self._xsrf_token = token
        return self._xsrf_token

So I wonder if there was a way to do so easily, e.g.

define("xsrf_cookie_expiration", default=1, help="_xsrf cookie expiration days", type=int)

and within the xsrf_token implementation:
expires_days = options.xsrf_cookie_expiration if self.current_user else None

Owner

bdarnell commented Feb 17, 2012

Just curious, why do you want to make the xsrf cookies expire faster? (I actually see an argument for making them last longer, perhaps session-indefinite, since I don't think we currently do a graceful rollover on expiration).

The tornado.options module is, well, optional, so we don't want to use it directly here, but an overridable method on RequestHandler would be OK.

tzuryby commented Feb 17, 2012

taking a fanatical approach toward security (paranoid mode), on a certain app, I wish to reset all footprint at client side every 24 hours, that perhaps includes regenerating cookie_secret at server side, and regenerating _xsrf.

I think I will override this in a way no expiration will be set at all (Session cookies like), and that will make all reset at browser close.

Owner

bdarnell commented Feb 19, 2012

OK. I tend to be skeptical of overly-frequent key rotation, but XSRF is one area where it makes sense (since to exploit a stolen xsrf cookie you have to get the victim to visit a page you control before the cookie expires). I do feel like we should do something about making rollover more graceful before encouraging shorter expiration times.

I'd advise against having cookies expire on browser close - that's a very unpredictable policy in practice, since some users restart their browsers frequently, while others run one browser instance for weeks or months.

tzuryby commented Feb 19, 2012

I completely agree with your second sentence, perhaps it shall be based on hours (12 or 24) rather than browser closed.
as for the first, well, it is up to you what shall go into mainstream ;-).

overloading it locally in my app only is not a big deal and can remain that way in my opinion forever ;-)

have a nice weekend.

Owner

bdarnell commented May 25, 2014

In addition to setting the expiration, it should also be possible to set the secure, httponly, and other flags for the xsrf cookie.

@bdarnell bdarnell added the web label Jul 16, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment