"Missing string query parameter b'RelayState'" during SAML re-authentication

[sephii]

Description

When using the SAML re-authentication flow (eg. when deleting devices from an account), the page with the text "A client is trying to remove device(s) from your account. To confirm this action, re-authenticate with single sign-on. If you did not expect this, your account may be compromised!" redirects to the IDP page without the RelayState parameter, causing the authentication to redirect to /_matrix/saml2/authn_response without any RelayState parameter, causing a 400 error with Missing string query parameter b'RelayState'.

i think the problem comes from this line, which initiates the redirect with an empty URL (used as RelayState by handle_redirect_request).

SSO login works fine on the other hand, where RelayState is correctly added to the IDP URL. I'm using Google as a SAML IDP.

Steps to reproduce

• Log in on riot web with SAML auth (SAML being the only authentication scheme enabled)
• Go to your user settings, go to the "security and privacy tab"
• Select a session to delete, click on the "delete" button
• A modal asking you to use SSO to continue opens, click on the SSO button
• A page asking you to confirm you want to delete these devices opens, click on the link
• You're redirected to the SAML login page, without a RelayState parameter
• Log in on the IDP, you're redirected to /_matrix/saml2/authn_response without a RelayState parameter, and you get the 400 error described.

i'd expect the authentication to succeed.

Version information

• Homeserver: https://matrix.asso-pea.ch/
• Version: 1.13.0rc1
• Client: riot-web 1.6.0
• Install method: pip
• Platform: Debian buster

[anoadragon453]

Added the release-blocker tag as I believe this affects things that have been added in the v1.13.0 release cycle.

Supposedly the reason behind the empty string there is that in that case there isn't any reason to perform a redirect. It's worth noting that Google's SAML wasn't tested in development of this feature, but obviously we want it to work :)

@sephii do you have any insight about what that string should be in Google's case?

[sephii]

@anoadragon453 I have no clue what it should theoretically be. 😅 But I can tell you what happens when I click the "Sign in with single sign-on" button on Riot web, which works. Clicking this button makes a GET HTTP request to https://matrix.mydomain.com/_matrix/client/r0/login/sso/redirect?redirectUrl=https%3A%2F%2Friot.mydomain.com%2F%3Fhomeserver%3Dhttps%253A%252F%252Fmatrix.mydomain.com%26identityServer%3Dhttps%253A%252F%252Fvector.im which responds with a redirect to https://accounts.google.com/o/saml2/idp?idpid=redacted&SAMLRequest=redacted&RelayState=https%3A%2F%2Friot.mydomain.com%2F%3Fhomeserver%3Dhttps%253A%252F%252Fmatrix.mydomain.com%26identityServer%3Dhttps%253A%252F%252Fvector.im.

So it seems like the login/sso/redirect endpoint uses the redirectUrl param as RelayState. i hope this helps, let me know if you need more info!

[clokep]

I took another look at this using Auth0 and also didn't run into this issue. I unfortunately don't have access to a G Suite account so can't test this.

We might be able to just fill in anything there, even if we never used it? There isn't an obvious value to fill in to redirect to in this case.

[artooro]

We are also getting hit by this issue.

If I take the link "re-authenticate with single sign-on" and add &RelayState=https://riot.mydomain.com to the end of the URL it works fine.

[artooro]

For SSO the client_redirect_url is set via

args = request.args
        if b"redirectUrl" not in args:
            return 400, "Redirect URL not specified for SSO auth"
        client_redirect_url = args[b"redirectUrl"][0]

here:

synapse/synapse/rest/client/v1/login.py

Line 401 in 13a8276

args = request.args

But for the fallback used by cross-signing

synapse/synapse/rest/client/v2_alpha/auth.py

Line 175 in 13a8276

client_redirect_url = ""

it's client_redirect_url = "" which is one part of the problem. If I set the value of client_redirect_url to self.hs.config.public_baseurl then the authentication works, but it does not redirect back to the Riot app as you'd expect.
I'm not sure at this point how to set the client_redirect_url value so that it works with Riot desktop.

[clokep]

During the SSO fallback it is not expected to redirect anywhere at the end of it, a page gets served saying "you can close this now" (with some JavaScript that would allow a client to automatically close the page). This is why the client doesn't provide a redirect URL in those cases and why it was left blank. Using something like the homeserver URL wouldn't really make sense, but sounds better than using a Riot URL.

[clokep]

For reference:

• SSO login flow: https://matrix.org/docs/spec/client_server/r0.6.0#sso-client-login
• The fallback docs: https://matrix.org/docs/spec/client_server/r0.6.0#fallback

[artooro]

@clokep ok in that case, my change is working for the Riot-web and possibly the best resolution for this specific issue.

The issue where Riot-desktop does not appear to work with sso fallback might be another issue.

[JonathanReifer]

Well described above, I can confirm this is an issue for both riot-desktop and riot-web with synapse 1.13 authenticating against a lemonldap-ng saml provider (in addition to the above reference to google). Haven't tried the hacky work around yet. As mentioned above, if the sso fallback flow is not expecting a url to redirect back to then imo either handle_saml_response should handle the response with an empty relaystate or client_redirect_url should get set (to something to prevent the fatal error).

[oblak-be]

we have the same issue authenticating against keycloak!

[jlusky]

Same issue with Azure AD IDP

[sephii]

If I understand #7484 (comment) correctly, it means the error is just cosmetic and the authentication succeeds even if the redirect fails, and the whole thing should work anyway?

It tested again and I still can't delete devices on riot-web 1.6 with SSO. Here's what I did:

• Go to Settings > Security & Privacy
• Select 1 unused session
• Click the button "Delete 1 session"
• Click on "Single Sign On" button from the "Confirm deleting this session by using Single Sign On to prove your identity." dialog
• At this point, a new tab with "A client is trying to remove device(s) from your account. To confirm this action, re-authenticate with single sign-on. If you did not expect this, your account may be compromised!" opens. If I go back to the riot-web tab, I now see the "Delete session" button in the dialog, with a message "Click the button below to confirm deleting this session."
• Since clicking on the button doesn't do anything, I go back to the SSO auth tab and click on the "re-authenticate with single sign-on" link
• I log in with my Google Account
• I get the "Missing string query parameter b'RelayState'" error
• I close the window and go back to the riot-web tab where the dialog with the "Delete session" button is still open
• I click the "Delete session" button
• Nothing happens

Am I doing something wrong?

[clokep]

If I understand #7484 (comment) correctly, it means the error is just cosmetic and the authentication succeeds even if the redirect fails, and the whole thing should work anyway?

My comment was describing why the code does not provide a redirect URL from the client. It was not meant to imply that this should "work anyway". The error message is from the IdP rejecting authentication (I assume at least, I haven't been able to reproduce this).

[clokep]

Since I've been unable to reproduce -- would any of the reporters here be able to try code off of a pull requests if I put up a potential fix?

[kheiken]

Since I've been unable to reproduce -- would any of the reporters here be able to try code off of a pull requests if I put up a potential fix?

Absolutely no problem, got an SSO setup in a test environment where I can reproduce this bug.

[clokep]

Did some more reading on this to remember exactly what the RelayState is here, my understanding is that it is a value the service provider passes to the identity provider which gets echoed back after authentication. This can be anything and does not need to be provided, unfortunately Google's troubleshooting documentation seems to imply that they require it:

The SAML 2.0 specification requires that Identity Providers retrieve and send back a RelayState URL parameter from Resource Providers (such as G Suite). G Suite provides this value to the Identity Provider in the SAML Request, and the exact contents can differ in every login. For authentication to complete successfully, the exact RelayState must be returned in the SAML Response. According to the SAML standard specification, your Identity Provider should not modify the RelayState during the login flow.

• Diagnose this issue further by capturing HTTP headers during a login attempt. Extract the RelayState from the HTTP headers with both the SAML Request and Response, and make sure that the RelayState values in the Request and Response match.
• Most commercially-available or open-source SSO Identity Providers transmit the RelayState seamlessly by default. For optimum security and reliability, we recommend that you use one of these existing solutions and cannot offer support for your own custom SSO software.

I believe providing anything there will fix the issue, even if the value is unused.

Anyway, I put up a fix as commit f4b450b15cdef1b650b9bf70ac52f988e05fe40a, it would be nice if someone could test. I based this on 1.13.0 so it is essentially 1.13.0 + one commit and should be fairly safe to run.

[kheiken]

With the proposed patch I can successfully re-authenticate using SSO/SAML and e.g. delete client sessions. So thank you very much for this fix.

But it only works when using Riot in the web browser.

When I try to delete a client session using the Electron version of Riot (1.6.1), I get the prompt to use Single Sign-On to re-authenticate, which opens a browser. I authenticate and am presented with the Matrix message:

Thank you

You may now close this window and return to the application

The Riot application however does not register that I just re-authenticated using SSO. The prompt to re-authenticate just remains open.
I am not sure if this needs to be done on the Synapse- or Riot-side of things. In a way I would expect Matrix to redirect the user to a riot:// URL, right?

[clokep]

The Riot desktop issue is a separate problem, see element-hq/element-web#13719.

[clokep]

I should also say -- thanks for verifying that this solution worked! 👍 I'll clean-up the PR and we'll get it in the next release version!

[clokep]

@richvdh also provided a definition RelayState from one of the SAML docs:

RelayState data MAY be included with a SAML protocol message transmitted with this binding

[sephii]

Works for me too, thanks!

[clokep]

This should be fixed in 1.14.0. Thanks for all the help debugging this! 🥳