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

Replace current mechanism to store encrypted passwords #456

Open
stweil opened this issue Mar 3, 2016 · 16 comments
Open

Replace current mechanism to store encrypted passwords #456

stweil opened this issue Mar 3, 2016 · 16 comments

Comments

@stweil
Copy link
Member

stweil commented Mar 3, 2016

Currently, Goobi.Production stores DES encrypted passwords with a well known pass phrase. This approach has several problems:

  • As the pass phrase is well known, the encryption is not really useful.
  • The DES algorithm is no longer a good choice because of its weakness.
  • The source of class DesEncrypter has no known origin, so it should be removed.

The solution could be implemented in two steps.

  1. Replace DesEncrypter by a new implementation, maybe based on JCA. This can be tested by the existing test code, so it would be safe for the next version.
  2. Replace the DES encryption by something more secure. This involves an irreversible conversion of existing passwords and might be done in the next version.

See also previous discussion in PR #454.

@henning-gerhardt
Copy link
Collaborator

Some small hints and minds:

  • extending test scenarios to ensure correct usage of en- and decryption of passwords
  • changing pass phrase and / or encryption algorithm needs a migration tool as stored passwords must be migrated
  • ldap password transfer must be respected as know it used stored password

@stweil
Copy link
Member Author

stweil commented Dec 9, 2016

I suggest adding tags security and enhancement.

@matthias-ronge
Copy link
Collaborator

Minimum requirements to stored passwords are that they are stored hashed by a hash function that is cryptographically safe. (This is different from (reversible) encryption.) In addition, each password must be stored with an individual salt, making the generation of rainbow tables useless. Also, dumping and re-importing the user table must not break the hashes. (Both MySQL and Java handle illegal string content, but it cannot be restored from a MySQL dump, which is a text file.)

Since passwords cannot be decrypted, there is need for a recovery option. Typical solution is to e-mail the user a link with a one-time hash which allows them to reset their password. It should be clear that this password should only be usable once to change the password, and it should also expire after a short period (configurable, but I am talking about hours here). However, some desktop OSes (for example KDE) may call the link in the moment you click on it (probably to get the MIME type), to then decide to pass it to the browser. So the link must only be usable once to change the password, but as long as it was not used to change the password, the page must remain requestable until the time-out is reached. (What I want to say: The wrong solution is to make the page requestable only once.)

@matthias-ronge
Copy link
Collaborator

matthias-ronge commented Dec 5, 2017

When reworking this, issue #1151 should be solved, too.

@funkyfuture
Copy link

@matthias-ronge i'm a little confused about your statemnt that each password has to be hashed with an individual salt. that would probably be solved by storing the salts in the database. which would render the whole mechanism useless in case of a leaked database. afaik it's common to store one salt that are used for all hashings in a secure-as-possible-location.

@matthias-ronge
Copy link
Collaborator

No, as far as I understand it, the idea behind it, as I wrote earlier, is to make rainbow tables useless. Generating a rainbow table is still something that takes CPU time, but it's still a bit worthwhile if you only have to do it once, and once you have it, you can decrypt all passwords with it. But if you have to do that individually for each password, it makes it a lot more time-consuming again. While using an individual salt and saving it is trivial. The point is that if you could not prevent the burglary, you make the thieves at least as hard as possible.

@matthias-ronge
Copy link
Collaborator

This also applies to version 3, as the mechanism has not yet been replaced.

@matthias-ronge
Copy link
Collaborator

matthias-ronge commented Nov 4, 2020

The fact that we store our passwords reversibly encrypted is a security nightmare. We only save the passwords reversibly encrypted so that later an LDAP entry for a newly created user can be written. The LDAP entry is not written with the aim of allowing the user to log on to Production via LDAP (this can also be done via the database) but so that he or she can then log on to Samba via LDAP. We could have irreversibly hashed the passwords in the database if we did not write the LDAP group via a button, but if it happened when logging in.

Goal: If a user logs in and LDAP is active, first, the login via LDAP takes place as before. If it fails and the LDAP server is not read-only, it is checked whether the user actually exists in the LDAP at all. If not, it is checked whether the hash of the password matches the hash in the database, and if so, the user is logged in and, since the clear-text password is available, the LDAP entry is generated using that clear-text password. (At that moment, the password hash can also be deleted from the database because it is no longer needed and would not be corrected if the password was changed in the LDAP, which will likely lead to confusion later.) The button “Write LDAP entry” can be removed, too, and manually writing all users to LDAP when setting up a new instance is no longer necessary.

@matthias-ronge
Copy link
Collaborator

matthias-ronge commented Nov 6, 2020

Procedure proposal:

  1. Enable individual users are authenticated against database even when LDAP is on. (Enable individual users are authenticated against database even when LDAP is on #4014)
  2. Deactivate the LDAP group for the users supplied with the basic database. (Cf. also Improve example LDAP group #3304)
  3. Remove the LDAP on/off switch. (What about the LDAP on/off switch? #3336)
    (The ldap_use parameter in the kitodo_config.properties file should initially be retained so that users can initially remove their LDAP group after a migration. However, the default for this attribute can be set to true for new installations.)
  4. Change the creation of a user in LDAP to upon login. (Change the creation of a user in LDAP to upon login #4576)
  5. Remove the “Write LDAP entry” button.
  6. Add a function that deletes the password of users from the database the first time they successfully log in via LDAP. (covered in Change the creation of a user in LDAP to upon login #4576)
  7. Change the login so that the plain-text password is encrypted and compared against the stored encrypted password (and no longer the other way around).
  8. Add columns to the user table for the encryption algorithm and for an individual salt.
  9. Add a standardized irreversible hash function to hash the password. Make the question of which function is used for password checking dependent on what the database says for that user. Replace the passwords of the standard users in the delivered database with hashed ones.
  10. Change the functions for creating a user and for changing the password so that they use the irreversible hash function with securely random individual salt.
  11. Add a migration function that will use the old method to decrypt the passwords for all existing users, hash them with the new method and update the user table.

@henning-gerhardt
Copy link
Collaborator

@matthias-ronge I read now your goal to solve this issue and I have at least two questions:

  • Is it correct that after all your suggested changes are made that passwords only stored in database or LDAP and not anymore in both?
  • If passwords are only stored in LDAP then I need for my local development with productive data even a local LDAP server with a dump of the productive LDAP system and must change the URI to the LDAP system in the database?

@matthias-ronge
Copy link
Collaborator

First question: On a per-user basis, yes. For each user the password is either stored in the database (if the user does not have an LDAP group), otherwise it is stored in the LDAP. This makes sense because the user can always change his password in the LDAP in another way. However, the copy of the password in the database does not change synchronously, so that when the LDAP is switched off, the old password suddenly applies again after months or years. This has led to confusion in the past.

At the same time, mixed operation is possible, which was previously not possible. A user (e.g. an admin account) can always authenticate against the database, while other users are authenticated against LDAP. The user groups refer to the LDAP server, of which there can also be several in version 3, which can make sense with several clients.

Second question: It depends.

First of all, we have to distinguish two very different LDAP configurations.

Variant 1: The LDAP runs locally, Production has write access to the LDAP. The aim of the LDAP is that the users can log in with the password used in Production via Samba (or other Linux services: WebDav, FTP, Shell/SCP, depending on what is desired) using this middle ground, and if they change their password in Kitodo via the user dialog, the password that they have to use for these services also changes. However (depending on the configuration) the users may possibly also change their password in other ways (e.g. if they have access to the shell). (In this configuration, it makes no sense to have multiple LDAP servers.)

Variant 2: The LDAP runs centrally somewhere. It is an institute / university / company LDAP. Production has no write access to the LDAP. The users are created in another way (e.g. at the registration desk). The users cannot change their password using Production. (In this constellation, several LDAP servers can make sense if the users of different clients are to authenticate against different LDAPs.)

If I have read that correctly from your question, your test system is variant 2, which means you don't need a copy of the LDAP server, you just have to configure it as read-only. At the same time, you can also have users who authenticate locally and even don’t need to exist in the LDAP.

@henning-gerhardt
Copy link
Collaborator

Thank you for your explanations.

However, the copy of the password in the database does not change synchronously, so that when the LDAP is switched off, the old password suddenly applies again after months or years. This has led to confusion in the past.

This did only happen if you change the password in wrong way in 2.x. If you change the password in 2.x right / correct way database and LDAP would be synchronized changed.

A user (e.g. an admin account) can always authenticate against the database, while other users are authenticated against LDAP.

Good to know but I did not have this scenario as even admin users must authenticate against LDAP to use SAMBA and WebDAV services.

If I have read that correctly from your question, your test system is variant 2, which means you don't need a copy of the LDAP server, you just have to configure it as read-only. At the same time, you can also have users who authenticate locally and even don’t need to exist in the LDAP.

No, our test environment have a separated LDAP server as we did not use a centralized authentication system for Kitodo.Production.
In the past or in 2.x if I need a productive data in our test environment for testing new features or new versions of Kitodo.Production I copied everything: database, meta data files, ldap data, users, ... With the change in 3.x, that LDAP configuration is stored inside the database it would be more complicated to copy data from productive to test environment.
Even if I need productive data on my local development system to reproduce some kind of error then I need now even a local LDAP server as authentication against database is not longer possible with your changes.

@matthias-ronge
Copy link
Collaborator

This did only happen if you change the password in wrong way in 2.x. If you change the password in 2.x right / correct way database and LDAP would be synchronized changed.

If you create full Linux operating system users from Kitodo (with SSH access) and the PAM is configured correctly, the user can change his password on the shell and the password in the database is not updated. You may see this as the wrong way to go, but it is possible and we had seen the case on customer servers and it was confusing. That is why it makes sense to do without a password in the database if it is not used.

A user (e.g. an admin account) can always authenticate against the database, while other users are authenticated against LDAP.

Good to know but I did not have this scenario as even admin users must authenticate against LDAP to use SAMBA and WebDAV services.

This is about an over-admin who should be able to create clients and intervene for service/in case of incidents, while the users of different clients authenticate themselves via different (!) LDAP servers (or via the database, if a client does not have an LDAP server. In any case, this is about mixed operation. This approach is not necessary for a classic 1-institution instance.)

In the past or in 2.x if I need a productive data in our test environment for testing new features or new versions of Kitodo.Production I copied everything: database, meta data files, ldap data, users, ...

In that case you already have your own LDAP on the test system.

With the change in 3.x, that LDAP configuration is stored inside the database it would be more complicated to copy data from productive to test environment.

To make it easier for you, you can bend the DNS name of the LDAP server in /etc/hosts to the local IP address. Then you don't need to touch the database. (I've seen this on test systems before, it seems to be common practice.)

Even if I need productive data on my local development system to reproduce some kind of error then I need now even a local LDAP server as authentication against database is not longer possible with your changes.

No. All you have to do is set the LDAP group to null in the database for the user (or users) you want to test with and insert a (known) password hash.

UPDATE user SET ldapGroup_id = null, password = "OvEJ00yyYZQ=" WHERE id > 0;

@henning-gerhardt
Copy link
Collaborator

If you create full Linux operating system users from Kitodo (with SSH access) and the PAM is configured correctly, the user can change his password on the shell and the password in the database is not updated.

That is not my use case as our users are not be able to login through ssh or other shell login mechanism. Even they should only change password through the application.

To make it easier for you, you can bend the DNS name of the LDAP server in /etc/hosts to the local IP address. Then you don't need to touch the database. (I've seen this on test systems before, it seems to be common practice.)

Even this is not my use case. We are using real and correct dns entries and switching to tls protected ldap and mysql connections with correct signed certificates for every used system.

No. All you have to do is set the LDAP group to null in the database for the user (or users) you want to test with and insert a (known) password hash.

Good to know. Maybe this can work on a local development system if the developer remember to change the user information in the database.

@solth solth removed the 3.x label Jul 7, 2022
@matthias-ronge matthias-ronge added development fund 2024 A candidate for the Kitodo e.V. development fund. and removed 2.x labels Mar 5, 2024
@matthias-ronge
Copy link
Collaborator

I removed the 2.x label as this also affects version 3.x.

@solth
Copy link
Member

solth commented Mar 18, 2024

Votes: 5

@solth solth removed the development fund 2024 A candidate for the Kitodo e.V. development fund. label Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants