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

Crowd Login - password always stored in plaintext in mongo and logged #7674

Closed
rndmh3ro opened this issue Aug 7, 2017 · 15 comments
Closed

Comments

@rndmh3ro
Copy link
Contributor

rndmh3ro commented Aug 7, 2017

Description:

When using Atlassian Crowd to let users login to Rocket.Chat, the password of the user is always stored in plaintext in the mongo database. This does not happen when using LDAP.

Server Setup Information:

  • Version of Rocket.Chat Server: 0.57.2
  • Operating System: Ubuntu 16.04.3 LTS
  • Deployment Method(snap/docker/tar/etc): snap
  • Number of Running Instances: 1
  • DB Replicaset Oplog: activated
  • Node Version: v4.2.6

Steps to Reproduce:

  1. Activate crowd login. It does not matter if deny unauthorized or sync users is set.
  2. Let user login. It does not matter if the user already exists or is newly created.
  3. Check the mongodb users collection, see that the password is stored in plaintext.
  4. Check the logs and see the password there in plaintext.

Expected behavior:

Password is not stored at all or hashed.

Actual behavior

Password is stored in plaintext.

@rndmh3ro
Copy link
Contributor Author

rndmh3ro commented Aug 7, 2017

I guess the syncing happens because of https://github.com/RocketChat/Rocket.Chat/blob/develop/packages/rocketchat-crowd/server/crowd.js#L128 and https://github.com/RocketChat/Rocket.Chat/blob/develop/packages/rocketchat-crowd/server/crowd.js#L152.
The sync seems to happen no matter if CROWD_Sync_User_Data is set or not.

However I could be totally wrong here.

@rndmh3ro
Copy link
Contributor Author

rndmh3ro commented Aug 8, 2017

ping @rufushonour, hope you're still working with rocket.chat! :)

@jsternadel
Copy link

I have patched this on my local instance running 0.65.2. I was not unable to reproduce #4 on your list. Even with all log levels enabled I never saw the plain text password.

I added several other features with my changes:

  • Call Accounts.setPassword whenever a Crowd user authenticates. This will allow Rocket.Chat authentication in the event Crowd is offline.
  • Added a Sync Interval setting to customize how often the Crowd user data sync happens
  • Added a Clean Usernames setting. Because Crowd will allow email as username I implemented a simple method to fix this. Basically split on the '@' and use the first section as the rocket chat username. Could use some guidance on how to make this better.
  • Added a crowd_username property which stores the actual crowd username and is now used for all syncing purposes. This will allow users to change their RC username and still be synced with Crowd.
  • Added a secondary sync method for attempting to find user by email. This was mainly to repair my own users that had changed their usernames. If more than one email is found it bails out but for the most part worked really well.
  • Added a Sync User Data button to the Atlassian Crowd admin ui. This is async and is used to run a Crowd data sync on demand.

The only thing that is a problem with what i have built is the way im saving the crowd password in conjunction with the Allow Password Change setting. This setting effectively has no effect. A user could change their password in RC but as soon as they logged in with their Crowd credentials the password is overwritten with the crowd password.

If anyone else is interested in these improvements I can submit a pull request

jsternadel pushed a commit to jsternadel/Rocket.Chat that referenced this issue Jun 24, 2018
- Added new setting for cleaning crowd usernames and creating an RC compliant name
- Added new setting for selecting crowd user data sync interval
- Added new button to force an on demand user data sync
- When a crowd user authenticates the native method for storing passwords is called allowing RC authentication via fallbackDefaultAccountSystem even if Crowd server is offline
@demonicblue
Copy link

@jsternadel Would be great to see these changes in upstream!
I'm hoping this will carry over removed and disabled users from crowd. Right now, it doesn't look like you can disable Rocket.Chat users through crowd.

@kos4live
Copy link
Contributor

I can't confirm/reproduce that passwords stored in plaintext, only bcrypt from login, but our crowd is connected to a active directory.

@jsternadel Maybe you want apply your additional changes to my fork #11483.

@piotrkochan
Copy link
Contributor

This should be released as fast as possible.

@jamesdeuchar
Copy link

Agreed this should be released as soon as possible. In the meantime, is there are recommended method of patching a docker install?

@ghost
Copy link

ghost commented Aug 6, 2018

Can confirm this is an issue and can see plaintext passwords stored in the MongoDB users collection. Using Crowd 3.2.2 with Crowd Internal Directory.

@jsternadel
Copy link

Here is a Gist with my current working changes:
https://gist.github.com/jsternadel/ffaadff287de75404032677a4595f629

The cleanUserName() method still needs some work. It should be trying to create a match from the global setting UTF8_Names_Validation. Unfortunately I haven't had time to try this out yet. Our Crowd instance allows emails to be used as usernames which doesn't work so well in RC.

The biggest change is using Accounts.setPassword which is the default way of saving user passwords that includes bcrypt encryption. This also allows RC to still be used if Crowd goes down since the fallback authentication mechanism actually works now.

There are also some new settings for invoking the cleanUserName() method as well as initiating a user sync from the "Atlassian Crowd" settings page.

Unfortunately my normal job is taking up all my dev time right now. Hopefully someone can use this to implement a permanent fix.

@jsternadel
Copy link

Here is a second Gist for anyone using Rocket Chat in a docker and want to know how to patch it with my latest code.
https://gist.github.com/jsternadel/09d2a068343f64146e98f675b20666e7

This will have to be done every time you update your container. I am currently on version 0.68.3 and this works for my environment.

@piotrkochan
Copy link
Contributor

piotrkochan commented Sep 3, 2018

@RocketChat any update?

@piotrkochan
Copy link
Contributor

Why do we need to patch it manually for so long, that's a SHAME

@Emersonyh-zz
Copy link

same issue here

@piotrkochan
Copy link
Contributor

@jsternadel Your patch works perfect, I only changed sync part - when user is not found in crowd I do not throw an error - instead just skip this person.

@rndmh3ro
Copy link
Contributor Author

@jsternadel Your patch works perfect, I only changed sync part - when user is not found in crowd I do not throw an error - instead just skip this person.

Why not create a PR then?

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

9 participants