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

Encrypted/Authenticated Cookies #1668

Open
MaxGabriel opened this issue Apr 21, 2020 · 2 comments
Open

Encrypted/Authenticated Cookies #1668

MaxGabriel opened this issue Apr 21, 2020 · 2 comments

Comments

@MaxGabriel
Copy link
Member

MaxGabriel commented Apr 21, 2020

Yesod currently uses encrypted and authenticated cookies for sessions, which prevents users from either reading or forging them.

Yesod doesn't offer this for regular cookies, but regular cookies often serve important purposes that wouldn't be tied to a session cookie. For example, a session cookie might expire after 2 hours, whereas a cookie to remember your device (and thus not trigger 2FA) might last months. For this reason, a Yesod user might use a different cookie.

Overall, I think cookie tampering represents a potential security vulnerability to Yesod users, and the documentation should warn that it's trivial for clients to forge non-session cookies, and suggest encrypting/authenticating the cookies if users want to prevent this. I generally think that web frameworks should attempt to educate their users about HTTP stuff like this.

I think it could also be good for Yesod to introduce APIs to allow users to seamlessly set cookies with encrypted payloads, as well as lookup data from encrypted ones. This would match other web frameworks like Ruby on Rails support for encrypted/authenticated cookies. This helps make the secure path the an easy option for Yesod users.

I consider this a more rare use case, but it may also be valuable to offer only authenticated (not encrypted cookies). Rails offers this as signed cookies. I am not sure how common this use case is, but I imagine the idea is that it allows the client to read (but not modify) the cookie values. (Having trouble coming up with a good use case for this. Maybe something like you use a cookie to give temporarily elevated permissions to the client, and you want them to be able to read and display what those permissions are?).

Thoughts on this?


Implementation discussion

As far as APIs go, encrypted/authenticated cookies might look something like:

  • setEncryptedCookie — takes a regular SetCookie and encrypts its payload
  • lookupEncryptedCookie—looks up a cookie, and attempts to decrypt its payload. Potentially this function would differentiate between no cookie/cookie but decryption failure/successful decryption?

lookupCookies would maintain its current behavior, but potentially it should be documented that it does not decrypt cookie values for you.

You could argue that functions like setCookie should be renamed to setUnencryptedCookie or something to make people cognizant of the potential for users to modify them. Thoughts on this? Initially I thought this was too extreme, but now I'm wavering.

As far as the actual encryption/decryption, yesod-core currently integrates the clientSession package to encrypt session cookies. I know there is a little debate on clientsession about changing its encryption function, but I would lean towards using the same encryption that clientsession uses so it's simple to Yesod users what encryption function is being used. The existing APIs of clientsession are sufficient to implement encrypted+authenticated cookies, but not sufficient for authenticated cookies alone.

I would imagine a new function on the Yesod typeclass would be added for handling encryption/decryption of cookies, similar to the session backend one? That might potentially be confusing, that the encryption of session cookies could be different from regular ones (though not sure why people would do this in practice). This function could default to using clientsession, using the same file path for the key as regular session cookies.

@snoyberg
Copy link
Member

I don't have any strong opinions here. Documenting that cookies are unencrypted by default seems fine, but there's also already quite a bit of security knowledge around writing web apps that is assumed in using Yesod, so this may be boiling the ocean. I'm also not certain what use cases encrypted cookies are intended to cover that sessions don't cover.

I use cookies for things like user settings (e.g. overriding language) or setting redirect destinations, things which have no security implications. Everything else goes in a session (either client or server side).

@MaxGabriel
Copy link
Member Author

Following up on this, I implemented this in our production codebase and it's working well!


I don't have a lot of use cases thought up, but to give two examples of what we're using cookies for besides sessions:

  1. Using a 30 day cookie for remembering 2FA. This cookie naturally has to be separate from the session cookie, which expires much more quickly.

  2. Using a permanent cookie to identify users across logins. On signin, we check if the browser has this permanent cookie, and if so store the (userId, cookieValue) in the database. If not, we assign a permanent cookie to the browser with a new random value. This is intended as an anti-fraud measure; we can then see that two users have logged in from the same browser, so if one of them commits fraud, we know to go investigate the other one (since it's likely the same person).

In both these cases, it's not critical to use an encrypted cookie, but it is nice security hygiene that we can trust the values we get are ones we've set


I looked into implementing this in Yesod. It would look something like this:

  1. Add a new data type called EncryptedCookieBackend that has encrypt/decrypt functions
data DecryptCookieResult =
      DecryptionFailure Text ByteString
    | DecryptionSuccess ByteString

data EncryptedCookieBackend = EncryptedCookieBackend
    { ecbEncryptCookie :: ByteString -> IO ByteString -- TODO: Bad naming b/c "ecb" is a mode of encryption
    , ecbDecryptCookie :: ByteString -> IO DecryptCookieResult
    }
  1. Add a new function to the Yesod typeclass:
makeEncryptedCookieBackend :: site -> IO (Maybe EncryptedCookieBackend)

This would be similar to makeSessionBackend: it would default to use clientsession's functions to encrypt/decrypt (possibly with the same key file as is used for sessions). Nothing could be returned to opt-out of the addition, in which case I'm not sure what functions like lookupEncryptedCookie would do. Possibly throw an exception?

  1. Add a new field to YesodRunnerEnv called yreEncryptedCookieBackend:
data YesodRunnerEnv site = YesodRunnerEnv
    { yreLogger         :: !Logger
    , yreSite           :: !site
    , yreSessionBackend :: !(Maybe SessionBackend)
    , yreEncryptedCookieBackend :: !(Maybe EncryptedCookieBackend)
    , yreGen            :: !(IO Int)
    -- ^ Generate a random number
    , yreGetMaxExpires  :: !(IO Text)
    }
  1. The field from YesodRunnerEnv would be used to populate a new field on RunHandlerEnv, similar to how maxExpires is

  2. That field would be looked up with askHandlerEnv and used to implement functions like setEncryptedCookie and lookupEncryptedCookie.

Overall, I think this would be relatively simple to implement but is somewhat invasive, so not sure if other people would want this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants