-
Notifications
You must be signed in to change notification settings - Fork 9
auth: allow cross-site session cookie for federation #444
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
base: master
Are you sure you want to change the base?
Conversation
Set SESSION_COOKIE_SAMESITE to None whenever OpenStack federation and OIDC auth is enabled so the browser keeps the session during the WebSSO callback. Without this, the session cookie is stripped when Keystone runs on a different domain, and the flow ends with ?code=invalid_authentication_method. Fixes #443 Signed-off-by: Bartosz Bezak <[email protected]>
|
tested locally, looks ok: |
|
As discussed elsewhere, I think this fix opens us up to CSRF exploits such as a malicious site being able to send state changing requests (e.g. DELETE to |
maybe we should store tokens server-side instead of sending it via cookie? |
|
I've noticed that there's also a separate |
JohnGarbutt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit worried this opens us up to nasty CSRF attacks, and we have quite a lot of functional federation setups today with the current settings... I think because the all come from the same base site address.
Can we just make this easier to override to start with, till we work this one out in more details? (Clearly you totally need this for a scenario you are testing).
| CSRF_COOKIE_NAME: {{ . }} | ||
| {{- end }} | ||
| {{- if or (and (eq .Values.authentication.type "openstack") (.Values.authentication.openstack.federated.enabled)) (eq .Values.authentication.type "oidc") }} | ||
| SESSION_COOKIE_SAMESITE: None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want this on by default, as this is working fine for all our current federation customers, and gives us better security I think.
For cases where you need to allow it, we should try and make this easier to change.
I think we assumed this was related to a problem when SSL wasn't turned on, but maybe the problem was deeper than we first thought, and was actually this one. Which makes sense.
I don't believe that changes things, the attack is still likely to work, I believe, as you still have to set a cookie to look up the details on the server side. |
|
Just to follow up here, is there not a config we can add to trust a specific extra domain, because I think in this case, its simply that federation is from a different domain, that we could allow explicitly? |
Set SESSION_COOKIE_SAMESITE to None whenever OpenStack federation and OIDC auth is enabled so the browser keeps the session during the WebSSO callback. Without this, the session cookie is stripped when Keystone runs on a different domain, and the flow ends with ?code=invalid_authentication_method.
Fixes #443