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

[security] This image creates two Admin accounts #161

Closed
ChessSpider opened this issue Nov 8, 2017 · 15 comments · Fixed by #264
Closed

[security] This image creates two Admin accounts #161

ChessSpider opened this issue Nov 8, 2017 · 15 comments · Fixed by #264

Comments

@ChessSpider
Copy link

ChessSpider commented Nov 8, 2017

Hi!
I noticed unexpected behavior while using your OpenLDAP image..

After changing the passwords of cn=admin,dc=example,dc=org you can still login using the old password with full rights. The reason is that this image defines RootDN and RootPW in DN: olcDatabase={1}hdb,cn=config as cn=admin,dc=example,dc=org. This is a separate login.

steps to reproduce

Start the stuff

me@VirtualBox:~/test/docker-openldap/example$ docker-compose up 

Login via PHPLDapAdmin

username: cn=admin,dc=example,dc=org
password: admin

Change the password of cn=admin,dc=example,dc=org to example

Use hash {SSHA}bbbIClpMGqvOpa5Yc9x+VS/9PN1czs1B
Or create a new SSHA hash here: http://projects.marsching.org/weave4j/util/genpassword.php (not for production)

Save.

Login again with cn=admin,dc=example,dc=org with the old password admin

Expected behavior:

Login fails

Observed behavior

Login succeeds, given full root access.
(Note, you can also login with cn=admin,dc=example,dc=org using the new password example. You're now logged in as a user and the ACL as defined in 02-security.ldif will give you full access)

Reason why

In DN: olcDatabase={1}hdb,cn=config database the RootDN is defined as cn=admin,dc=example,dc=org with the initial password set in RootPW. This is a real root-user with full access that bypasses all ACL. This password is not changed when you change the password of cn=admin,dc=example,dc=org.

Note, to login to the cn=config database you need the config credentials. By default in this image:
Username: cn=admin,cn=config
Password: config

Impact

If people start the image with the default password and change the password of cn=admin,dc=example,dc=org to something more secure later then they're gonna have a bad time.

Suggested fix

I really think there's no need for the user cn=admin,dc=example,dc=org with the ACL giving all permissions. Just simply rely on the RootDN in DN: olcDatabase={1}hdb,cn=config and explain people how cn=config works.

@osixia
Copy link
Collaborator

osixia commented Nov 9, 2017

Hello,
thanks for reporting this.

Indeed we can add information how to change the admin password. Pull request welcomed.

Maybe adapt your issue title to something less worrying and more accurate.

@ChessSpider
Copy link
Author

ChessSpider commented Nov 9, 2017

Hi thanks for the response 👍 . I want to emphasize I like and appreciate the work you've done in creating and maintaining this image.

Any suggestions for title? I will remove the exclamation mark at the end but it is security related and the image by default does create two admins.

Adding documentation to change the admin password will not fix this vulnerability for existing users of your image. Also, your image should not create two admin accounts during bootstrap.

Any current user of the image may be impacted. In that light, it might be an option to create a script which removes the cn=admin,{{ LDAP_BASE_DN}} user to mitigate the risk. Or remove the RootDN. Or sync the password of the cn=admin,{{LDAP_BASE_DN}} user to RootDN.

I don't think there is any good fix for the current users of your image that works best for all your users, and they should be informed so they can decide the best action. What do you think?

@ChessSpider ChessSpider changed the title [SECURITY] This image creates two Admin accounts ! [SECURITY] This image creates two Admin accounts Nov 9, 2017
@ChessSpider ChessSpider changed the title [SECURITY] This image creates two Admin accounts [security] This image creates two Admin accounts Nov 9, 2017
@osixia
Copy link
Collaborator

osixia commented Nov 9, 2017

Oops sorry,
your title is accurate i read a bit too fast, i just make the full test and your right this is a major issue.

Yes users should be informed, i'm going to add a warning on top of the readme linking to this issue,
maybe we can wait for some user inputs to choose the best option ?

@ChessSpider
Copy link
Author

ChessSpider commented Nov 9, 2017

Hi thanks for the confirmation. It's a difficult issue. I think most people will use this image and just let it running, without looking here again. Waiting has an obvious risk, but rushing into a decision may cause a lot of damage.

I suggest this fix for the next update:

Create a shell script which executes automatically, which:

  1. Prints a warning informing them of this issue
  2. And either:
    2.1 If both accounts exist: Takes the password from cn=admin,{{LDAP_BASE_DN}} and sets it as RootPW in olcDatabase={1}hdb,cn=config. Then remove cn=admin,{{LDAP_BASE_DN}}
    2.1 If cn=admin,{{BASE_DN}} is missing and RootPW of olcDatabase={1}hdb,cn=config is admin AND RootDN is cn=admin,{{BASE_DN}}, then remove RootDN and RootPW from olcDatabase={1}hdb,cn=config,
    2.1 If RootDN/RootPW doesn't exist, do nothing

I think this script should create the situation which people expect. What do you think?

@osixia
Copy link
Collaborator

osixia commented Nov 9, 2017

not sure to understand this point:
"If cn=admin,{{BASE_DN}} is missing and RootPW of olcDatabase={1}hdb,cn=config is admin, remove RootDN and RootPW from olcDatabase={1}hdb,cn=config"

this leads to no admin account ? Except with the config password ?

@ChessSpider
Copy link
Author

ChessSpider commented Nov 9, 2017

Correct. My reasoning is that, if the user removed the cn=admin,{{BASE_DN}} user it does not want to use that admin account. Perhaps it has created a new admin account with different ACLs.

Of course it is a problem if the user wants to use the RootDN account. However the user could re-add the RootDN account easily.

--
edit; i changed it to the following:
2.1 If cn=admin,{{BASE_DN}} is missing and RootPW of olcDatabase={1}hdb,cn=config is admin AND RootDN cn=admin,{{BASE_DN}}, then remove RootDN and RootPW from olcDatabase={1}hdb,cn=config

@osixia
Copy link
Collaborator

osixia commented Nov 9, 2017

Sounds good to me :)

Just maybe not do this automatically but ask for user consent by adding a boolean environment variable like FIX_SECURITY_ISSUE_161

So when the user start the container with next image version:

  • If FIX_SECURITY_ISSUE_161 is undefined or something else than true or false and we are in case 1 or 2 :
    we print a warning message that explain the problem and the fix that will be applied and ask the user to set FIX_SECURITY_ISSUE_161 to true if want to fix this automatically or false to discard this warning
  • if FIX_SECURITY_ISSUE_161 is true we apply the fix
  • if FIX_SECURITY_ISSUE_161 is false we do nothing

What do you think?

@ChessSpider
Copy link
Author

Hi! I can definitely understand the sentiment of not wanting to break things or change things without permission. However considering how most people use Docker as a "start & forget" application I have serious doubts if that warning will reach everyone who is impacted.

It's ultimately your decision, but I think mitigating the security issue should be the first priority. I think the shell-script as I described minimizes any negative consequences. One can always login locally and re-add a RootDN w/ RootPW if required.

I would prefer that bit of manual labour over having (the risk of) an arbitrary person logging in as admin on my LDAP :).

@osixia osixia added bug and removed docs labels Nov 10, 2017
@osixia
Copy link
Collaborator

osixia commented Nov 10, 2017

Hello,
Ok to make it autcomatically in a first time.

We will make a release candidate with that and see feedback :)

@ChessSpider
Copy link
Author

Any progress on the fix? :)

@osixia
Copy link
Collaborator

osixia commented Nov 28, 2017

we have no time to work on this for now :(
any help would be appreciated

@lknock
Copy link

lknock commented Dec 12, 2017

These are the same issue, no? https://www.openldap.org/lists/openldap-technical/201212/msg00225.html

It seems there is an authentication identity for root access if you set olcRootDN and olcRootPW. It is not a user. This means you can still create a user with the same DN, which is why it appears there are two admins or one admin with two passwords when you do this.

If olcRootPW is not set, they advise to create the admin user. Otherwise, it is not even necessary. So it's either one or the other then.

@BertrandGouny
Copy link
Member

BertrandGouny commented Feb 16, 2018

This behavior occurs also on a regular debian install of slapd (debian strech, openldap 2.4.44)
i guess this should be reported upstream ? But not sure if this a Openldap or phpLDAPAdmin issue.

Does anyone have an idea how to fix this on fresh install ?

@Bessonov
Copy link

Run in this issue. Is there any way to configure admin password in a secure way? Because LDAP_ADMIN_PASSWORD isn't encrypted.

@BertrandGouny
Copy link
Member

You can set the admin password in a *.startup.yaml environment file, that is deleted right after startup scripts. Or if you use kubernetes run an init container with command line option -p and LDAP_ADMIN_PASSWORD environment variable that will just bootstrap openldap config and do not run openldap. Then run a regular pod container using the same ldap config and data dirs

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.

4 participants