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

secure cookie is encoded to base64 twice #36

Closed
miekg opened this issue Jan 27, 2016 · 5 comments
Closed

secure cookie is encoded to base64 twice #36

miekg opened this issue Jan 27, 2016 · 5 comments

Comments

@miekg
Copy link

miekg commented Jan 27, 2016

On https://github.com/gorilla/securecookie/blob/master/securecookie.go#L272, there is

b = decode(b)

and a few lines down it is encoded again to base64.

Why is it encoded to base64 on line 272? The raw bytes of b can be used to run the HMAC, it does not have to ascii.

@elithrar elithrar added the bug label Jan 27, 2016
@shore
Copy link

shore commented Jan 27, 2016

I'm not speaking for the authors, but given that the cookie is structured text (pipe-delimited in this case), the first encoding ensures that the data does not contain the delimiting character. This protects a user from surprising decode failures if the data they're storing in the cookie happens to include the delimiter, no?

@miekg
Copy link
Author

miekg commented Jan 27, 2016

That's a good point... The downside is that when you do this you blow up the cookie size and we started hitting the 4k-ish limit.

@miekg
Copy link
Author

miekg commented Jan 27, 2016

A hack around this would be to prefix the thing with indices of each of the pieces of data.

@elithrar
Copy link
Contributor

That would be a breaking change at this point (aka unable to parse existing
cookies).

If you are storing reasonably large amounts of data in a cookie, have you
considered moving to a server-side approach using (e.g.) Redis or BoltDB?

On Wed, Jan 27, 2016 at 10:20 AM Miek Gieben notifications@github.com
wrote:

A hack around this would be to prefix the thing with indices of each of
the pieces of data.


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

@elithrar
Copy link
Contributor

elithrar commented Jun 5, 2016

Closing due to inactivity. A fix would be breaking.

@elithrar elithrar closed this as completed Jun 5, 2016
@elithrar elithrar self-assigned this Jun 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants