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

regression: login breaks for accounts with uppercase username #167

Closed
pedro-nonfree opened this issue Jul 17, 2022 · 7 comments
Closed

regression: login breaks for accounts with uppercase username #167

pedro-nonfree opened this issue Jul 17, 2022 · 7 comments

Comments

@pedro-nonfree
Copy link

pedro-nonfree commented Jul 17, 2022

Description

co-written with @evilham

Our ldap matrix instance did ldap case insensitive login which means that it was unaffected by #165

Steps to reproduce

With synapse 1.62.0 this change in matrix-synapse-ldap3 breaks ldap login for similar setups when historical users have uppercase letters.

This is a very old installation before https://spec.matrix.org/v1.3/appendices/#user-identifiers and it does not seem to be a migration to lowercase usernames.

Homeserver

selfhosted homeserver instance

Synapse Version

{"server_version":"1.62.0","python_version":"3.9.2"}

Installation Method

Debian packages from packages.matrix.org

Platform

Debian 11 Linux

Relevant log output

If you want to look at homeserver.log there are in element-hq/element-web#22859

Anything else that would be useful to know?

This is what we get on our database on a login attempt:

synapse=# select name from users where name='@SomeUser:matrix.example.com';
           name
---------------------------
 @SomeUser:matrix.example.com
(1 row)

synapse=# select user_id from access_tokens where id=TOKEN_ID;
          user_id
---------------------------
 @someuser:matrix.example.com
(1 row)

This inconsistency means that the INNER JOIN in this query fails and the login proceeds with trying the macaroon which is not going to work

On this line:

https://github.com/matrix-org/synapse/blob/96cf81e312407f0caba1b45ba9899906b1dcc098/synapse/storage/databases/main/registration.py#L547

And trying to fix the query with LOWER(users.name) is not enough. Client fails on filters endpoint with error 403 with response {"errcode":"M_FORBIDDEN","error":"Cannot create filters for other users"}. Other endpoints might have similar issues

As a temporary workaround we are rolling back the python package matrix-synapse-ldap3 this way (it's a debian package):

service matrix-synapse stop
/opt/venvs/matrix-synapse/bin/pip install matrix-synapse-ldap3==0.2.0
service matrix-synapse start

As shown in reproducing the bug, looks like it is a problem related to how synapse server deals with case insensitive user IDs and not to the matrix-synapse-ldap3 python package.

@richvdh
Copy link
Member

richvdh commented Jul 21, 2022

likely caused by #163 ?

@richvdh richvdh transferred this issue from matrix-org/synapse Jul 21, 2022
@H-Shay H-Shay self-assigned this Jul 21, 2022
@H-Shay
Copy link
Contributor

H-Shay commented Jul 21, 2022

Hello and thanks for reporting this. Just so I am clear (it's a little hard to follow the info across the issues), you upgraded to synapse 1.62.0, and after that login attempts for users with old, historical usernames containing capital letters began to fail, correct?
This is part I am a little unclear on: can you let me know exactly what the failure looks like? I.e. what error message you receive from the client and the relevant logs from the homeserver covering the login request, that would be extremely helpful.

@evilham
Copy link

evilham commented Jul 22, 2022

you upgraded to synapse 1.62.0, and after that login attempts for users with old, historical usernames containing capital letters began to fail, correct?

Correct.

Specifically matrix-synapse-ldap3==0.2.1 triggers (but does not cause) the issue with: #163

This is part I am a little unclear on: can you let me know exactly what the failure looks like? I.e. what error message you receive from the client and the relevant logs from the homeserver

It's mostly what we wrote on the issue; since what matrix-synapse-ldap3 returns as the username for the token doesn't match what synapse has in its database for the mxid (lowercase vs mixed case respectively), any INNER JOIN queries fail and it the result is as if the user does not exist. (see: https://github.com/matrix-org/synapse/blob/96cf81e312407f0caba1b45ba9899906b1dcc098/synapse/api/auth.py#L399 leading to https://github.com/matrix-org/synapse/blob/96cf81e312407f0caba1b45ba9899906b1dcc098/synapse/storage/databases/main/registration.py#L547 )

Since synapse has some fallback compatibility code, after the login fails it moves on to trying something with macaroons (see: https://github.com/matrix-org/synapse/blob/96cf81e312407f0caba1b45ba9899906b1dcc098/synapse/api/auth.py#L417 )
Due to this, the server logs show something misleading about a deserialisation issue and the macaroon not having the proper format; we can post the exception but it truly was a waste of time to follow that track, this bit of code should have never been executed.

In a nutshell

Depending on how you look at this, it can be a: SPEC bug, Synapse bug (I incline towards this), or a matrix-synapse-ldap3 issue (it seems like @richvdh inclines towards this).

  • When synapse was released spec was not finished and mxids allowed for upper case symbols
  • Users like with mixed case were created and are actively used today across the matrix network
  • spec was amended and now mxids do not really allow for upper case symbols, we are not aware of a migration path or compatibility layer
  • synapse doesn't seem to have a way to handle this properly in all cases
  • our particular server's LDAP handles login in a case-insensitive fashion for the username, which means it was not affected by Can login only with lowercase username #72
  • Can login only with lowercase username #72 is likely a real issue for people, regardless of whether or not it affected us
  • since it seems unclear how synapse deals or doesn't deal with mixed case mxids in these cases, we think it is unclear how matrix-synapse-ldap3 should treat this
  • That's why we originally filed against matrix-org/synapse

Cheers,

@H-Shay
Copy link
Contributor

H-Shay commented Jul 25, 2022

Thanks for the info-I just still have one question, which is: what error message/error code are you seeing when the request to /login fails?

@evilham
Copy link

evilham commented Jul 25, 2022

This meaningless thing:

2022-07-15 14:19:40,194 - synapse.storage.database - 803 - WARNING - sentinel - Starting db txn 'add_access_token_to_user' from sentinel context
2022-07-15 14:19:40,280 - synapse.api.auth - 450 - WARNING - GET-472531 - Invalid access token in auth: <class 'pymacaroons.exceptions.MacaroonDeserializationException'> cannot determine data format of binary-encoded macaroon.
2022-07-15 14:19:40,280 - synapse.http.server - 167 - INFO - GET-472531 - <SynapseRequest at 0x7f024d753d30 method='GET' uri='/_matrix/client/unstable/org.matrix.msc2697.v2/dehydrated_device?&_cacheBuster=6499453651040419' clientproto='HTTP/1.0' site='8008'> SynapseError: 401 - Invalid access token passed.
2022-07-15 14:19:40,455 - synapse.api.auth - 450 - WARNING - POST-472533 - Invalid access token in auth: <class 'pymacaroons.exceptions.MacaroonDeserializationException'> cannot determine data format of binary-encoded macaroon.
2022-07-15 14:19:40,455 - synapse.http.server - 167 - INFO - POST-472533 - <SynapseRequest at 0x7f027074aee0 method='POST' uri='/_matrix/client/r0/keys/upload?' clientproto='HTTP/1.0' site='8008'> SynapseError: 401 - Invalid access token passed.

As described, it tries the regular flow, then goes on with the macaroons, which ofc can't be deseralised because it's been ages since that got retired, so the result is: the token is not valid, aka https://http.cat/401

@H-Shay
Copy link
Contributor

H-Shay commented Jul 25, 2022

Great thanks!

@H-Shay
Copy link
Contributor

H-Shay commented Jul 29, 2022

We have landed a fix in #168 which should address the issue.

@H-Shay H-Shay closed this as completed Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants