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

Session Cookie Missing 'Secure' Attribute #2887

Open
kayvanaarssen opened this issue Dec 13, 2024 · 6 comments
Open

Session Cookie Missing 'Secure' Attribute #2887

kayvanaarssen opened this issue Dec 13, 2024 · 6 comments

Comments

@kayvanaarssen
Copy link

kayvanaarssen commented Dec 13, 2024

We got this in a report (Pentest)
Is the fix: https://docs.pwpush.com/docs/other-configurations/ setting - SECRET_KEY_BASE
Or do we need to do more?

Finding;

Not including the "secure" attribute in a session cookie introduces a significant security risk. Without this attribute, the cookie is transmitted over both secure (HTTPS) and unsecure (HTTP) connections, making it susceptible to interception by attackers on unsecured networks. This vulnerability exposes sensitive session information, such as user authentication tokens, to potential eavesdropping and unauthorized access. Attackers could exploit this weakness to hijack user sessions, leading to unauthorized account access and potential misuse of personal data. Including the "secure" attribute is crucial to ensure that session cookies are transmitted exclusively over encrypted channels, mitigating the risk of data interception and enhancing overall security.

A session cookie is a small piece of data that a website temporarily stores on a user's device during their visit. It is commonly used to temporarily store information about the user's session, such as login credentials or preferences. Session cookies are designed to be temporary and are typically deleted when the user closes their browser. The "secure" attribute, when applied to a session cookie, ensures it's only transmitted over secure, encrypted connections (HTTPS). This adds an extra layer of protection against potential security threats during data exchange between the user's browser and the server.

  • Enable HTTPS for your website to ensure secure communication.
  • Explicitly set the "secure" attribute when creating or updating the session cookie.
  • Implement secure practices for storing session cookies on the server side.
@pglombardo
Copy link
Owner

Good point. Should be fixed in #2902 which I'll test and release soon.

@kayvanaarssen
Copy link
Author

kayvanaarssen commented Dec 19, 2024

Good point. Should be fixed in #2902 which I'll test and release soon.

@pglombardo Great news, sorry I missed your reply 2 days ago #BusyTimes
Is there anything we need to set after the update to get this to work?

@jaychinut
Copy link

jaychinut commented Dec 22, 2024

Glad to see this being worked on as I also had this concern. I was able to set the secure flag by using FORCE_SSL and assume_ssl (due to the load balancer in front of my setup) in config/environments/production.rb. However, I see that FORCE_SSL is being deprecated soon.

# Assume all access to the app is happening through a SSL-terminating reverse proxy.
# Can be used together with config.force_ssl for Strict-Transport-Security and secure cookies.
config.assume_ssl = ENV.key?("ASSUME_SSL")

# Force all access to the app over SSL, use Strict-Transport-Security, and use secure cookies.
config.force_ssl = ENV.key?("FORCE_SSL")

@kayvanaarssen
Copy link
Author

@pglombardo Is the setting that @jaychinut suggest enough to get it working?
Of are there more settings we need to set in the .env?

@pglombardo
Copy link
Owner

FORCE_SSL is an older setting in Ruby on Rails (the backend framework used). It was going to be deprecated from the framework at one point and then it wasn't. So Password Pusher will follow. As it looks right now, it won't be deprecated after all.

assume_ssl is a newer setting that I haven't added environment variable support for yet.

Besides those two, I believe we also need the session cookie changes in #2902. The current issue that I need to confirm/research is that if I enable secure cookies, it will likely break any users who are running pwpush internally over HTTP.

With the list of options growing every release, I might have to make some opinionated decisions in the v2 docker containers which is coming eventually.

I'm thinking maybe make the Docker container SSL enabled by default by bundling Caddy directly in the container. All security options enabled by default. Then an option to instead make everything HTTP for those who want to use their own proxy.

So a slight ramble but there is a bit of complexity I have to work though.

I'll try to get #2902 out soon.

@kayvanaarssen
Copy link
Author

That's a good idea, also if you make it still an option in the environment than people can still choose. And make a best practice with the enabled option for an Secure Environment with NGINX etc.

Keep up the good work! But the secure cookies part - is becoming critical with Pentests etc.

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