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

Passwords for Database user and LDAP bind user are stored in plain text #15460

Closed
1 of 6 tasks
GeyserLaPunk opened this issue Apr 13, 2021 · 6 comments · Fixed by #15547
Closed
1 of 6 tasks

Passwords for Database user and LDAP bind user are stored in plain text #15460

GeyserLaPunk opened this issue Apr 13, 2021 · 6 comments · Fixed by #15547
Labels
topic/security Something leaks user information or is otherwise vulnerable. Should be fixed!

Comments

@GeyserLaPunk
Copy link

GeyserLaPunk commented Apr 13, 2021

  • Gitea version (or commit ref): 1.13.7
  • Git version: 2.14.0
  • Operating system: Windows 10/Server 2012
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
  • Log gist:

Bad security practice of storing passwords in plain text

Description

While application does provide a warning that it stores password in plain text, I think we all agree that this is a big security issue independently from how many privileges user has.
There is an existing module modules/secret/secret.go that allows for two way encryption. Why is it not used to encrypt password values stored in app.ini and in the database in case of LDAP auth.
...

Screenshots

@zeripath
Copy link
Contributor

It sounds like you have identified the issue and the solution. Perhaps you could provide a PR?

@GeyserLaPunk
Copy link
Author

It sounds like you have identified the issue and the solution. Perhaps you could provide a PR?

While I would like to help as much as I can, however I never wrote a line of code in GO, so until I'm comfortable doing it I should pass this opportunity to someone with more experience.

The logic should be pretty simple: you encrypt it as soon as you read it and decrypt it only when you need to use it.

@lunny
Copy link
Member

lunny commented Apr 14, 2021

I cannot know what do you mean. Which table and which column?

@GeyserLaPunk
Copy link
Author

GeyserLaPunk commented Apr 14, 2021

I cannot know what do you mean. Which table and which column?

Table: login_source,
Column: cfg

When login source is LDAP via BindDN bind password value within JSON string is plain text.
Not sure if any other sources have similar issue as I haven't tried them.

Also, database user password in app.ini database section PASSWD value is plain text as well.

@lunny lunny added the topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! label Apr 17, 2021
zeripath added a commit to zeripath/gitea that referenced this issue Apr 19, 2021
The LDAP source bind password are currently stored in plaintext in the db
This PR simply encrypts them with the setting.SECRET_KEY.

Fix go-gitea#15460

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath
Copy link
Contributor

That the configuration file contains necessary passwords for configuring the service is essentially unfixable - at some point these have to be unencrypted and the secrets have to become available some way.

I have however, put a PR up that encrypts the ldap bind password within the db.

@GeyserLaPunk
Copy link
Author

That the configuration file contains necessary passwords for configuring the service is essentially unfixable - at some point these have to be unencrypted and the secrets have to become available some way.

I would have to respectfully disagree with your statement. Any password should be encrypted the moment it was read during initial setup and held in the encrypted state in any long term storage (memory, text file, etc.). decryption should occur at the place of the immediate use, such as when connection to DB is happening. It just feels like a common sense.

I have however, put a PR up that encrypts the ldap bind password within the db.

That is a step in the right direction. Thank you.

zeripath added a commit that referenced this issue May 20, 2021
* Encrypt LDAP bind password in db with SECRET_KEY

The LDAP source bind password are currently stored in plaintext in the db
This PR simply encrypts them with the setting.SECRET_KEY.

Fix #15460

Signed-off-by: Andrew Thornton <art27@cantab.net>

* remove ui warning regarding unencrypted password

Co-authored-by: silverwind <me@silverwind.io>
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this issue Aug 10, 2021
* Encrypt LDAP bind password in db with SECRET_KEY

The LDAP source bind password are currently stored in plaintext in the db
This PR simply encrypts them with the setting.SECRET_KEY.

Fix go-gitea#15460

Signed-off-by: Andrew Thornton <art27@cantab.net>

* remove ui warning regarding unencrypted password

Co-authored-by: silverwind <me@silverwind.io>
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic/security Something leaks user information or is otherwise vulnerable. Should be fixed!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants