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

LDAP authentication server does not support Sub CA #3742

Closed
jpawlowski opened this issue Oct 2, 2019 · 15 comments
Closed

LDAP authentication server does not support Sub CA #3742

jpawlowski opened this issue Oct 2, 2019 · 15 comments
Assignees
Labels
cleanup Low impact changes
Milestone

Comments

@jpawlowski
Copy link

jpawlowski commented Oct 2, 2019

Today, configuring an LDAP server with either TLS or StartTLS will only work when the Root CA can be set directly. It will not work when dealing with an issuing / sub CA as the Auth/LDAP.php script currently only writes the sub CA into the file in /var/run/certs.

A workaround today is to create a single entry in System > Trust > Authorities and put both the Root CA and Sub CA certificates into the Certificate data field. Choosing this particular entry in LDAP server configuration will allow to connect to the server. (Hint: It might be required to reboot your device as the trust store somehow seems to get confused when playing with such configuration too much. Rebooting will give you a clean state before actually testing the LDAP server configuration).

I am not sure about the side effects to the trust store to be honest. One is for example a cosmetic one where it will not show the correct issuer:
image


If the Auth/LDAP.php script could include all certificates of this particular trust chain, that would be preferred. Maybe this can be considered in one of the next

@jpawlowski jpawlowski changed the title LDAP server does not support Sub CA LDAP authentication server does not support Sub CA Oct 2, 2019
@fichtner
Copy link
Member

fichtner commented Oct 2, 2019

If you have a 19.7.4 can you try to hardcode LDAP to /etc/ssl/cert.pem ? it should have all the CAs and the default ones now.

@jpawlowski
Copy link
Author

Indeed, changing

ldap_set_option(null, LDAP_OPT_X_TLS_CACERTFILE, "/var/run/certs/{$this->ldapCAcert}.ca");

to /etc/ssl/cert.pem will make that happen.
That would make quite a lot of stuff obsolete in the code I guess. Also, it would no longer be needed to choose the Peer Certificate Authority in the frontend. Instead, a simple reminder about adding private CAs to the trust store would be sufficient.

@AdSchellevis
Copy link
Member

@jpawlowski Sounds like a good idea to me, can you confirm that your issue is solved when LDAP_OPT_X_TLS_CACERTFILE points to /etc/ssl/cert.pem ? I'll gladly remove some code here.

@jpawlowski
Copy link
Author

I can confirm this, it is working on 4 devices now.

@AdSchellevis
Copy link
Member

ok, thanks, I'll prepare a patch later.

@AdSchellevis AdSchellevis self-assigned this Oct 4, 2019
@AdSchellevis AdSchellevis added the cleanup Low impact changes label Oct 4, 2019
@AdSchellevis
Copy link
Member

@jpawlowski can you try b2affd1 ?

I've dropped LDAP_OPT_X_TLS_CACERTDIR as well, since it doesn't seem to be required when pointing to a cacert file.

@fichtner fichtner added this to the 20.1 milestone Oct 4, 2019
@jpawlowski
Copy link
Author

Just applied the patch to one of the machines, deleted the chain certificate from the trust store, and saved the LDAP server configuration (just to ensure the new XML config file structure).

Afterwards, I was still able to connect to the LDAP server as intended so the patch seems to work fine. Many thanks, Ad!

@AdSchellevis
Copy link
Member

@jpawlowski thanks for confirming, can I close the issue? It will move into a production version later.

@jpawlowski
Copy link
Author

will just close it myself, thanks for politely asking ;)

@jpawlowski
Copy link
Author

@AdSchellevis just to let you know that there are coming up some warning messages in the reporter:

[04-Oct-2019 21:49:18 Europe/Berlin] PHP Warning:  implode(): Invalid arguments passed in /usr/local/www/system_authservers.php on line 245

Everything is working fine, though.

@AdSchellevis
Copy link
Member

@jpawlowski not related, but 09c34b2 should fix it.

fichtner pushed a commit that referenced this issue Oct 9, 2019
@jpawlowski
Copy link
Author

Too bad this didn’t make it into the new minor release (while other LDAP fixes did) @fichtner

@fichtner
Copy link
Member

Too many changes here... we just pushed it 1 release back to give others the opportunity to test with the package mirror shipped opnsense-devel which is our default policy for backports. Exceptions apply, but it’s good to not forget rules completely. 😊

@jpawlowski
Copy link
Author

This was not about the package, it was about the patches to Auth/LDAP and stuff in b2affd1

@fichtner
Copy link
Member

fichtner commented Oct 11, 2019 via email

fichtner pushed a commit that referenced this issue Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Low impact changes
Development

No branches or pull requests

3 participants