Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a way to encrypt cookies #28

Closed
stephanebastian opened this issue Dec 18, 2014 · 19 comments
Closed

Provide a way to encrypt cookies #28

stephanebastian opened this issue Dec 18, 2014 · 19 comments

Comments

@stephanebastian
Copy link
Contributor

No description provided.

@stephanebastian
Copy link
Contributor Author

Never mind. It's already implemented in CookieHandlerImpl

@purplefox
Copy link
Contributor

Hmm, I actually removed this in a recent commit.

Are you referring to signing cookies? (Signed cookies values are not encrypted)

@purplefox purplefox reopened this Dec 18, 2014
@stephanebastian
Copy link
Contributor Author

Hi Tim,

Yes I am referring to encrypting cookies (via a Java Cipher for instance)
However the more I thing about it and the more I feel it's more than just encrypting cookies. IMHO it would be nice to somehow 'configure' Apex with some kind of Cipher.
Then each time something needs to be 'secured', it's done using the Cipher that's configured.
For instance, cookies could be automatically encrypted/decrypted using the configured Cipher.
Others use cases could be to encrypt/decrypt sensitive form fields maybe also some specific url parameters.

Does it make sens?

@aliakhtar
Copy link

Others use cases could be to encrypt/decrypt sensitive form fields

You should just use HTTPS for that.

@stephanebastian
Copy link
Contributor Author

Sorry for the late answer, it was prety hectic lately.

The main issue with 'regular' cookies is that they can be modified client-side. If you use HTTPS it prevents people from eyedropping the data (including cookies) transmitted back and forth between the client and the server. IMHO HTTPS is a must have in almost any web applications.
However, HTTPS does not prevent a user from modifying cookie data client-side.

Encrypting cookies (AES 256, CBC for instance) prevents users from tampering data stored inside cookies. In some case, it's perfectly OK to use plain-text cookies. In others cases, we really want to make sure that cookie data can't be modified in any way.
IMHO, Vertx should support this use case and let developers decide whether they need to encrypt cookies or not.

From a technical stanpoint, it's a no brainer to support cookie encryption. The 'challenge' though is to make sure that people can use whatever encryption technology they want (AES, Blowfich, HMAc + AES, etc...)

I propose we add the interface CookieCodec. We provide a couple of must-have implementations of that interface.

We also need to update the CookieHandler as follow:
'''
@VertxGen
public interface CookieHandler extends Handler {

/**

  • Create a cookie handler
    *

  • @return the cookie handler
    */
    static CookieHandler create() {
    return new CookieHandlerImpl();
    }

    /**

  • Create a cookie handler using the specified CookieCodec
    *

  • @return the cookie handler
    */
    static CookieHandler create(CookieCodec codec) {
    return new CookieHandlerImpl(codec);
    }
    }
    '''

If there is an interest from others people I can work on this and submit a PR quickly.
Let me know.

@aliakhtar
Copy link

+1, I'm in favor.

@purplefox
Copy link
Contributor

I think this would be a nice feature, although usually this requirement can be designed away by never storing important information in a cookie and just using an id to lookup the session on the server side.

E.g. the Vert.x session cookies are cryptographically secure random GUIDs so it doesn't matter if they are tampered with.

@aliakhtar
Copy link

The issue with looking up the info on server is, 1) you take up server
memory, 2) It can make scaling out to other servers difficult.

I think it would be nice to offer support for both server as well as client
sessions, to let people choose whichever one works for them.

On Mon, Mar 23, 2015 at 4:05 PM, Tim Fox notifications@github.com wrote:

I think this would be a nice feature, although usually this requirement
can be designed away by never storing important information in a cookie and
just using an id to lookup the session on the server side.

E.g. the Vert.x session cookies are cryptographically secure random GUIDs
so it doesn't matter if they are tampered with.


Reply to this email directly or view it on GitHub
#28 (comment).

@purplefox purplefox added ready and removed ready labels Mar 23, 2015
@leolux
Copy link
Contributor

leolux commented Mar 30, 2015

A little security improvement could be to set the HttpOnly flag in order to prevent cookie stealing by an XSS attack (https://www.owasp.org/index.php/HttpOnly).

stephanebastian pushed a commit to stephanebastian/vertx-web that referenced this issue Apr 2, 2015
Added a new interface Crypto that can be used to encrypt/decrypt String (not only Cookies)
Provided an implementation of the Crypto interface based on AES/CBC/PADDING + HMAC
Modified existing Cookie classes to add the Crypto feature

Signed-off-by: Stephane Bastian <stephane.bastian@monpetitguide.com>
@apatrida
Copy link

as @leolux says, HttpOnly should be default options for session cookies to help mitigate attacks using the cookie and scripting. In additional the other parts of this issue such as encryption are valuable.

@pmlopes
Copy link
Member

pmlopes commented Aug 18, 2015

Instead of encrypted cookies we can sign cookies which is common on other frameworks such as node, Django, php. That would be easier to implement since it is just a endHandler that when active will sign the cookie and a validator when reading the cookie.

That and using httpsOnly would already give you a high level of trust.

The problem of encrypting a cookie is that clients will not be able to read the cookie anymore unless there is a decrypter at the client side. if you have a decrypted in javascript if will probably make it way to easy to break the security since the scripts would be available freely.

@pmlopes pmlopes removed the deferred label Feb 5, 2016
@noguespi
Copy link

What is the state of this feature ? Can we sign or encrypt cookies ?

Now, using an encrypted (or just signed) cookies to store the session is the default of most web frameworks... The main advantage of storing session in a cookie is because it's more easier to scale (and it doesn't make the app less secure, I would even say it makes it more secure because sessions are not stored in a centralized place).

Using httpOnly should be the default too. A well coded web application rarely need to access cookie in JavaScript (and if it's needed it can be enable because it's just an option). Also we should have ability to mark the cookie as "secure" when used in conjonction with https.

@pmlopes
Copy link
Member

pmlopes commented Mar 24, 2016

@serphacker it is not implemented since the discussion was still open and no consensus was achieved. Also if we're at it, it could be nice to revisit the session code and see if we could implement a cookie based session storage since at the moment you can only store your sessions (backend wise) using a local or shared map.

@noguespi
Copy link

Yes it is more about providing a cookie based session system than a "whole cookie encryption". If the session value is stored client side (in a cookie) it should be :

  • signed, mandatory to avoid session tempering from client, ie setting admin=true in the cookie.
  • encrypted, optionally, but will improve security by hiding session variables
  • timestamped, here it's not just about setting a cookie expiration time. If your session contains userid=1 and its cookie expired, it will be deleted from your browser. But, nothing prevent an attacker to resend the same value (in a fresh cookie) 10 years later, and it would still be valid, that's why we may need a timestamp in the session itself.
  • httpOnly
  • secure (optionally)

I did take a look at SessionStore, SessionHandler etc. and it would need a bit of refactoring to support CookieSession.

@pmlopes
Copy link
Member

pmlopes commented Mar 30, 2016

@serphacker are you volunteering for a PR for this then?

@noguespi
Copy link

I can't now, I don't know yet if I switch from ninjaframework to vertx-web for our next project. So don't count on me now.

@pmlopes
Copy link
Member

pmlopes commented Mar 30, 2016

@serphacker ok no problem, i'll mark it as "help wanted" since we are currently hands full with other issues and soon adding support for HTTP2 so this is currently not critical.

@elad-yosifon
Copy link

@serphacker @purplefox @pmlopes I'm actually implementing a similar solution on a private project..
I might consider writing it as a PR to the vertx-web repo..
Can you clarify what is the needed spec?

@pmlopes pmlopes added the wontfix label Mar 1, 2019
@pmlopes
Copy link
Member

pmlopes commented Mar 1, 2019

Closing as there's no activity for more than 1 year. Cookies can be updated or modified easily with a custom handler.

@pmlopes pmlopes closed this as completed Mar 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

8 participants