search

perwendel / spark

A simple expressive web framework for java. Spark has a kotlin DSL https://github.com/perwendel/spark-kotlin
Apache License 2.0
9.64k stars 1.56k forks source link

Set HttpOnly for session cookie due to improvement of security. #965

Closed M-Razavi closed 6 years ago

M-Razavi commented 6 years ago

Cookies are usually created by a server, passed to the browser and then passed back. Now it is possible to create and manipulate Cookies using JavaScript which can be helpful but can also be a security hole. So an HttpOnly Cookie is only accessible by the server, or in other words it is not accessible from client side JavaScript which protects your site from some forms of XSS attacks. So the Browser will store and return an HttpOnly Cookie but it will not alter it or allow you to create it on the client; an HttpOnly Cookie must be created on the server. Overall, for security improvement cookies must be HttpOnly.

asolntsev commented 6 years ago

@jakaarl I believe we should merge this pr ASAP. This is a huge security hole in all web applications written in Spark. What is the most important, adding HttpOnly breaks vulnerabilities. No valid code should ever use session cookie from JavaScript code. If somebody has such code, let's break it. It's the moment when framework author can decide. Security has much higher priority over somebody's ugly legacy code.

Let's the force be with you! :)

P.S. Those who don't have access to their source code just will not upgrade to the latest Spark version. As simple as that.

joatmon commented 6 years ago

There are plenty of use cases for using session cookies to SHARE information with the broweser. For example, any time you use a session cookie for authentication, you are vulnerable to Cross Site Request Forgery attacks. One way to mitigate these attacks is to provide a XSRF cookie that can be read by the browser.

https://stormpath.com/blog/angular-xsrf https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet

asolntsev commented 6 years ago

@joatmon yes, sure, it's absolutely ok to use some cookies in JS (for example, CSRF token). But not for session cookie. It definitely must be "httpOnly"!

In this pull request, we are talking specifically about session cookie.

asolntsev commented 6 years ago

@jakaarl @joatmon So?...

asolntsev commented 6 years ago

@jakaarl @joatmon ping

jakaarl commented 6 years ago

Secure by default is generally the way to go, but as I said, a way to opt out would be nice. Anyway, IIRC only @perwendel and @tipsy can accept pull requests.

M-Razavi commented 6 years ago

@perwendel @tipsy ping

tipsy commented 6 years ago

Seems like a good idea, but it's not backwards compatible.

M-Razavi commented 6 years ago

It seems unit tests were not written correctly(All checks passed and there is not any conflict) or maybe there are some imaginary checks which are not visible publicly.

asolntsev commented 6 years ago

@perwendel @tipsy @jakaarl @joatmon @M-Razavi

I have added tests and made it possible to disable HttpOnly flag. Please see my PR with these changes: https://github.com/perwendel/spark/pull/1027

tipsy commented 6 years ago

For it to be backwards compatible it should really be the other way, but IMO this is fine. @perwendel will have to decide.

asolntsev commented 6 years ago

@tipsy I agree, formally it's not backward compatible: the default behaviour has been changed. And I believe it's good in the name of security. :)

Those who want the previous behaviour will be able to achivee that.