CookieConfiguration should default to secure configuration and require user opt-out

[JLLeitschuh]

Ktor Version

master

Feedback

Currently, the configuration defaults for the Session CookieConfiguration defaults to an insecure configuration.

The power of defaults cannot be overstated in a security context.

Instead of defaulting to being insecure, the session cookie should instead default to secure.

Requiring the user to opt-out of the security of these Cookie protections means far fewer default servers will be vulnerable to HTTP downgrade attacks exposing session cookies and will prevent XXS attacks from being leveraged to steal sessions.

https://github.com/ktorio/ktor/blob/583760a04d07c985376ad2f19e656d365adb9a0f/ktor-server/ktor-server-core/jvm/src/io/ktor/sessions/SessionTransportCookie.kt#L83-L86

https://github.com/ktorio/ktor/blob/583760a04d07c985376ad2f19e656d365adb9a0f/ktor-server/ktor-server-core/jvm/src/io/ktor/sessions/SessionTransportCookie.kt#L88-L91

This is not technically a "security vulnerability", it's just an insecure default. This is not unique to Ktor, many libraries default to this "insecure by default" model, but there's no reason why Ktor needs to help perpetuate the insecurity.

I might classify this under CWE-276: Incorrect Default Permissions.

[cy6erGn0m]

I completely agree with httpOnly option that could be safely made true by default. The first one can't be true by default since it will never work in development environment that is very annoying, so all developers will simply disable this option forever and most likely there will be tons of open tickets about "sessions do not work"

[JLLeitschuh]

@cy6erGn0m (The start of a potentially bad thought) Is there any good way to detect a development vs production environment and enable this?

Or perhaps auto enable when TLS is enabled?

This might all just be a terrible idea, and I respect that.

I completely agree with httpOnly option that could be safely made true by default.

I agree, thank you for being willing to make the change.

[cy6erGn0m]

The other problem is that making it secure = true by default will lead to not working sessions over non-tls connector without any diagnostics so one wouldn't be able to understand why it doesn't work. Always throwing an exception at attempt to set session would be too rough...

[JLLeitschuh]

@cy6erGn0m Can you add a disclaimer to the documentation with a big warning telling users to enable this feature before shipping to production?

[cy6erGn0m]

Yes, good idea

[oleg-larshin]

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.