Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Renders a security warning on every request #158

Open
dmcinnes opened this issue Jun 18, 2015 · 6 comments
Open

Renders a security warning on every request #158

dmcinnes opened this issue Jun 18, 2015 · 6 comments

Comments

@dmcinnes
Copy link
Contributor

Around Rack 1.5.2 a warning message was added if Rack::Session::Cookie does not have the secret option set:

https://github.com/rack/rack/blob/master/lib/rack/session/cookie.rb#L108-L116

Identity is now seeing this in the logs:

SECURITY WARNING: No secret option provided to Rack::Session::Cookie.
This poses a security threat. It is strongly recommended that you
provide a secret to prevent exploits that may be possible from crafted
cookies. This will not be supported in future versions of Rack, and
future versions will even invalidate your existing user cookies.

Called from: /Users/dmcinnes/.rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/rack-1.6.2/lib/rack/builder.rb:86:in `new'.

Though annoying, it can be safely ignored because Identity supplies FernetCookieCoder as the session cookie coder which handles the encryption piece.

Unfortunately there is no easy way to disable the warning short of disabling all warnings in rack.

@brandur
Copy link
Contributor

brandur commented Jun 18, 2015

Yeah, I can see where Rack is coming from here, but this one is super annoying :/

Any thoughts on what we should do to solve it? We could just include a signature as well, but it seems somewhat wasteful just to bypass and error message which is being wrongly injected. We could also see if we can get an upstream patch into Rack.

@dmcinnes
Copy link
Contributor Author

I looked into just supplying the signature but it will encode it twice :/
Getting a patch into rack is probably the right way to go if this pattern is a common one, I imagine some sort of flag that suppresses the warning will be the easiest choice.

@dmcinnes
Copy link
Contributor Author

Opened a PR on Rack to allow disabling of the warning for cases like these: rack/rack#900

@rwz
Copy link
Contributor

rwz commented Jun 19, 2015

👍

@brandur
Copy link
Contributor

brandur commented Jun 19, 2015

@dmcinnes Nice :)

@dmcinnes
Copy link
Contributor Author

dmcinnes commented Sep 3, 2015

The rack PR has been merged! Once a Rack release is made we can update to fix this.

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

No branches or pull requests

3 participants