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

Allow serverName to be specified in URL #17824

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

aaronraimist
Copy link
Collaborator

@aaronraimist aaronraimist commented Jun 29, 2021

Fixes #5469

This may not be exactly how you want to implement it (since it follows the same path as default_server_name which is deprecated) but it works. I'm not really sure why default_server_name is deprecated.

Try it:
https://localhost:8080/#/login?serverName=kde.org
https://localhost:8080/#/register?serverName=kde.org

Also works when the homeserver URL (without https://) is used instead of the server name:
https://localhost:8080/#/login?serverName=kde.modular.im

Also works with the defaultUsername query param
https://localhost:8080/#/login?defaultUsername=alice&serverName=raim.ist


This PR currently has none of the required changelog labels.

A reviewer can add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is, or add Type: [enhancement/defect/task] to the description and I'll add them for you.

Signed-off-by: Aaron Raimist <[email protected]>
@t3chguy t3chguy requested review from a team June 29, 2021 08:12
@t3chguy
Copy link
Member

t3chguy commented Jun 29, 2021

@vector-im/product this could potentially be abused where someone sends you a link including a serverName they control and if you don't notice the server isn't one you expect then your username/password may be sent to the wrong server.

@aaronraimist
Copy link
Collaborator Author

Definitely a valid concern but personally I don't see it as being a significantly different risk from someone sending you a link to app.el3ment.io which is configured to use to evil.com's server.

Or the risk of sending matrix.org your username and password when you meant to use your personal server.

The ultimate solution is to stop people from ever being able to log in with just a localpart and always requiring the full username to be entered. (And doing .well-known lookup on registration)

@hex-m
Copy link

hex-m commented Jul 1, 2021

Thanks a lot! This would enable us to use a single element-web instance for our homeservers. see also : element-hq/element-meta#1580

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code seems fine, though the product team's review will be a lot more interesting.

src/vector/app.tsx Outdated Show resolved Hide resolved
src/vector/app.tsx Show resolved Hide resolved
aaronraimist and others added 4 commits July 22, 2021 02:07
Co-authored-by: Travis Ralston <[email protected]>
Co-authored-by: Travis Ralston <[email protected]>
defaultUsername was added in matrix-org/matrix-react-sdk#5674. I'm not aware of any others but they may exist.

Signed-off-by: Aaron Raimist <[email protected]>
@dch
Copy link

dch commented Aug 27, 2021

fwiw this would be a major usability step forward for new signups - I'm rolling this out to some non-tech friends and without this, its a lot of screenshots, careful settings, and .. mess. This will be a huge improvement!

src/vector/app.tsx Outdated Show resolved Hide resolved
@hex-m
Copy link

hex-m commented Dec 9, 2021

For a client-agnostic approach this could be an enhancement of the Matrix-URI-scheme.

Such a link would tell a Matrix client to start the registration or login flow, optionally prefill the username/localpart, optionally include the registration token and do that on the specified server.

@pReya
Copy link

pReya commented Dec 13, 2022

@turt2live Any chance you could give an update here? Any chance this PR can get reviewed by the product team?

@daniellekirkwood daniellekirkwood added X-Needs-Product More input needed from the Product team and removed X-Needs-Product More input needed from the Product team labels Mar 21, 2023
@semiviral
Copy link

semiviral commented Jul 29, 2023

Are there any plans to get this branch merged in? It's been sitting here for two years, and the original issue is almost 6 years old.

If there are any blockers, could they be discussed & run by the community so perhaps someone could contribute the remaining requirements to get this pushed through?

@t3chguy t3chguy requested a review from a team July 31, 2023 07:52
@t3chguy
Copy link
Member

t3chguy commented Jul 31, 2023

@vector-im/security how do you feel about being able to specify homeserver URL in the Element URL in the context of phishing and other such attacks

@davidegirardi
Copy link

I can't build your fork with the latest develop branches of matrix-js-sdk and matrix-react-sdk. Maybe I'm missing something in my dev VM.
Anyhow, I imagine you can see the homeserver name when you try to login or register and, as a phishing vector, it is not that different from using other methods as outlined above.

As an extra guard rail, we could force the user to type their MXID when using this parameter. This will be an extra layer if an attacker tries to exploit a legitimate element-web installation for phishing. I imagine this also voids the quality of life improvement of this PR, right?

@t3chguy
Copy link
Member

t3chguy commented Jul 31, 2023

Anyhow, I imagine you can see the homeserver name when you try to login or register and, as a phishing vector, it is not that different from using other methods as outlined above.

@davidegirardi
A problematic vector I can see is around password managers, given they'd key your secrets on the origin URL (e.g. app.element.io) and you could specify metrix.org or something equally similar under your control in order to steal primary account credentials.

we could force the user to type their MXID when using this parameter

At which point, given if you type your MXID your server is auto-discovered, is there any point to this?

I imagine this also voids the quality of life improvement of this PR, right?

Quite

@davidegirardi
Copy link

A problematic vector I can see is around password managers, given they'd key your secrets on the origin URL (e.g. app.element.io) and you could specify metrix.org or something equally similar under your control in order to steal primary account credentials.

I agree and there are concrete possibilities of quite a few users of password managers to autotype only the local part of the MXID.

We would be basically creating a variation of an open redirect but for credential submission. I think the QoL does not balance the risk for the people who actually need that QoL improvement.
Typing or autotyping the full MXID does not require to manually change the server.

I'm voting no to this.

@t3chguy
Copy link
Member

t3chguy commented Jul 31, 2023

I think the only way this can work safely is if any url-suggested homeserver comes with a chunky warning interstitial.

@davidegirardi
Copy link

I think the only way this can work safely is if any url-suggested homeserver comes with a chunky warning interstitial.

Isn't that going to:

  • Make people used to the warning
  • Make people believe we have a broken/insecure login?

I am also thinking about the role of the move to OIDC in this. I don't know how the flow will look like but since the authentication will happen in the OpenID provider, this QoL improvement starts reducing its return value.

@t3chguy
Copy link
Member

t3chguy commented Jul 31, 2023

I guess this problem gets easier with OIDC, as your password manager will be keyed to the server's auth domain rather than the client domain.

this QoL improvement starts reducing its return value.

I disagree, I think the QoL stays the same (no need to input server manually) and password manager has all the odds to protect you by not suggesting you enter your creds into a phishing server

@semiviral
Copy link

semiviral commented Aug 2, 2023

A problematic vector I can see is around password managers, given they'd key your secrets on the origin URL (e.g. app.element.io) and you could specify metrix.org or something equally similar under your control in order to steal primary account credentials.

How exactly does this differ to registers the actual metrix.org and impersonating the website itself? Or any other website, for that matter?

Additionally, if one is using a password manager, there's A) little chance that the credentials transfer to more personal platforms, and B) a great chance that the password manager itself has warned the user about the possibility of credential theft with auto-logins

Additionally additionally, the responsibility of the server owner is handling / dealing with this sort of thing. My server uses SSO (via Keycloak, Authelia, etc) as its only login method, because the built-in password protocol in Matrix is simply not suited for a large userbase deployment (I believe there are discussions open for this, moving to an open authentication standard as the default for the protocol).

Overall, what I'm seeing with this argument is a simple misunderstanding of how phishing is executed and combated; and a harmful one, because simply defaulting to matrix.org creates a larger and larger vector for cyber security threats as most users naturally default to registering with whatever server the client suggests.

@semiviral
Copy link

semiviral commented Aug 2, 2023

Point of fact, this is from the last TWIM:

Random MSC of the Week

The random MSC of the week is... MSC3062: Bot verification!

This MSC describes a method for verifying (cross-signing) the devices of a bot user, and how verification of that sort could be done. Obviously it wouldn't make much sense to verify emoji with a bot. Instead, this MSC suggests that the bot provide a URL to present to the user. If the URL appears trustworthy (those who would control this URL should also be in charge of this bot), then the user can choose to continue the verification.

The user's Matrix client would then make a request to the URL with details of the verification. If the server responds successfully, some cryptographic magic happens, and your client will consider the bot verified!

This is essentially tying a bot's verification with control of a domain's DNS, which I think is a smart way to do things. But you do need to watch out for those pesky UTF-8 control characters when asking the user to verify the URL!

The expectation is on the application UI to make the URL very clear, and the expectation is on the user to verify if that URL is the one they are expecting.

@t3chguy
Copy link
Member

t3chguy commented Aug 3, 2023

How exactly does this differ to registers the actual metrix.org and impersonating the website itself? Or any other website, for that matter?

Your password manager is smart enough to not suggest a password for matrix.org when browsing metrix.org - it is not smart enough to see the server picker and apply the same logic there.

The expectation is on the application UI to make the URL very clear, and the expectation is on the user to verify if that URL is the one they are expecting.

Definitely, but common security sense also has to apply, users aren't all the most security cautious and can get very easily desensitised to small differences like matrix vs metrix or worse yet if extended unicode URLs are used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bring back the hs_url parameter and document it
10 participants