Make private cookies cookie marked as secure by default

[atezet]

Existing Functionality

Rocket is a framework using "secure defaults". However, cookies are not marked as secure by default.

The functionality to mark a cookie as secure is there, but when libraries set cookies for you it is nearly impossible to mark it as secure. For example https://github.com/jebrosen/rocket_oauth2/blob/0.5.0-rc.1/src/lib.rs#L693 is not marked as secure and cannot be done in any trivial way.

Suggested Changes

Mark private cookies as secure by default. I.e. add 3 lines of code to: https://github.com/SergioBenitez/Rocket/blob/v0.5.0-rc.2/core/lib/src/cookies.rs#L497

Alternatives Considered

This could probably be done using some proxy like nginx, but it would be very hacky. When not using a proxy it is hard to do when using existing libraries like Rocket OAUTH2.

Additional Context

Options to consider:

• only do so when a secure connection is used
• only make it the default for production builds

[SergioBenitez]

We could absolutely do this, but only if we are absolutely certain that the application is being served over TLS. Otherwise, we risk creating cookies that are unusable. I would be in favor of this change.

[atezet]

So apparently, browsers do not allow to set secure cookies over an insecure connection (makes sense), so it would indeed make the code setting the cookies useless for insecure connections. I am not sure if it is possible to detect if a connection runs over TLS in a reliable manner (e.g. if a reverse proxy is used), but if there is I'd like to hear about it.

So the only viable option I see now is to use a configuration option that toggles compatibility with insecure connections, but I'm not sure what your vision is on these kind of options.

[atezet]

So, I've looked into some other frameworks, and Django has the following options regarding TLS: https://docs.djangoproject.com/en/4.1/topics/security/#ssl-https

In particular, the following setting looks like a proper solution on how to implement this in a workable way: https://docs.djangoproject.com/en/4.1/ref/settings/#std-setting-SECURE_PROXY_SSL_HEADER

[atezet]

Apparently, using this header is also the way Kubernetes ingress and mainstream load balancers do this: https://cloud.google.com/load-balancing/docs/https#target-proxies

[SergioBenitez]

I'd be in favor of enabling this conservatively. That is, as long as we're as certain as we can be that the connection is being served over a secure tunnel. It seems this means that either: 1) Rocket was launched with TLS enabled and is running. 2) A (default to None) configurable header is set that indicates the connection is being proxied over a secure tunnel.

[atezet]

Thanks for implementing this change. However, this now only works when Rocket is handling the TLS. I am trying to find some time to make this work with a proxy handling the TLS (through a configurable header, defaulting to None)

[SergioBenitez]

Thanks for implementing this change. However, this now only works when Rocket is handling the TLS. I am trying to find some time to make this work with a proxy handling the TLS (through a configurable header, defaulting to None)

Yes, this is a conservative change that improves the status quo. Please feel free to implement the proxy header version as well. Doing so would mean adding another field to the TlsConfig structure for the header name. The field would be of similar type and implementation to the recently added ip_header configuration value. That commit would be a great guide for how this should be implemented: 9377af5978c2505911769bb4f873b2a800e8e35a.

[atezet]

Thanks for the guidance. I quickly had a look at it already this morning and it seems a lot easier than I thought. That probably makes it easier to find the time to implement it

[atezet]

I did some work in #2519 but there are some things that are not 100% clear to me:

• How is the default for the setting determined?
• Right now I only support Protocols "http" and "https", but for some non-standard formed headers "on" is used: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto#examples , what do you think?
• What do you think of the other choices I made?

I still need to use it when setting the secure flag for cookies, but I thought I'd first get this right.

[atezet]

I added the functionality to actually set the cookie as secure by default (if the added options are enabled).