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

Users import fails if StartTLS Transport is configured on the LDAP Server #3445

Closed
gvecchicert opened this issue Apr 26, 2019 · 23 comments

Comments

@gvecchicert
Copy link

commented Apr 26, 2019

If StartTLS Transport is configured on the LDAP Server, the User Import page (system_usermanager_import_ldap.php) will fail with error message:

Could not connect to the LDAP server. Please check your LDAP configuration.

but Testing credentials will not.

Configuring Transport to TCP - Standard will workaround the issue.

PS: Also see https://forum.opnsense.org/index.php?topic=12529.new

@AdSchellevis

This comment has been minimized.

Copy link
Member

commented Jun 20, 2019

@gvecchicert the connect sequence was a bit different in the import page, 58c3e6c should fix the issue

@AdSchellevis AdSchellevis self-assigned this Jun 20, 2019
@AdSchellevis AdSchellevis added cleanup and removed incomplete labels Jun 20, 2019
@gvecchicert

This comment has been minimized.

Copy link
Author

commented Jun 21, 2019

I will give it a try as soon as it will appear in the updates

@AdSchellevis

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

thanks in advance, just let us know if it doesn't solve your issue and I'll reopen the issue (I can't test this myself with StartTLS at the moment)

@gvecchicert

This comment has been minimized.

Copy link
Author

commented Jun 21, 2019

Ok.
When do you think it will be released?

@AdSchellevis

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

either in 19.7 (next month), or if @fichtner pulls it in earlier. you can also use opnsense-patch 58c3e6c to test before release.

@gvecchicert

This comment has been minimized.

Copy link
Author

commented Jun 24, 2019

either in 19.7 (next month), or if @fichtner pulls it in earlier. you can also use opnsense-patch 58c3e6c to test before release.

After patch, the issue persists.

@fichtner fichtner reopened this Jun 24, 2019
@fichtner fichtner modified the milestones: 19.7, 20.1 Jun 24, 2019
@fichtner fichtner added this to To Do in 20.1 Jul 18, 2019
@lbr88

This comment has been minimized.

Copy link

commented Sep 15, 2019

This issue still exists in OPNsense 19.7.4_1-amd64

@AdSchellevis

This comment has been minimized.

Copy link
Member

commented Oct 4, 2019

@lbr88 @gvecchicert can you try b2affd1 (from #3742)? It might be related.

@gvecchicert

This comment has been minimized.

Copy link
Author

commented Oct 9, 2019

Patch installed but issue persists:

fujiko

Does the web gui need to be restarted?

@AdSchellevis

This comment has been minimized.

Copy link
Member

commented Oct 9, 2019

@gvecchicert thanks for testing, a restart shouldn't be necessary, we probably have to wait for someone to replicate and do some more digging then. I don't see a reason why it won't work at the moment.

@gvecchicert

This comment has been minimized.

Copy link
Author

commented Oct 9, 2019

Anyway: how to restart web gui?

@AdSchellevis

This comment has been minimized.

Copy link
Member

commented Oct 9, 2019

pluginctl -c webgui

should do the trick

@gvecchicert

This comment has been minimized.

Copy link
Author

commented Oct 9, 2019

thanks @AdSchellevis : I confirm that issue persists even after web gui restart.

@lbr88

This comment has been minimized.

Copy link

commented Oct 9, 2019

Just applied the patch you mentioned and the issue persists:

Oct 9 14:39:31 opnsense: Could not startTLS on ldap connection [,Can't contact LDAP server]
Oct 9 14:39:02 opnsense: Could not startTLS on ldap connection [,Can't contact LDAP server]

When setting it back to standard TCP (no tls/ssl) it works again.

@AdSchellevis

This comment has been minimized.

Copy link
Member

commented Oct 9, 2019

If the issue only exists for the import form, but auth works like a charm (as mentioned in the issue), I have no idea at the moment. We should have to wait for someone with a setup and time to do some more detailed digging. We'll keep the issue open in the mean time.

@lbr88

This comment has been minimized.

Copy link

commented Oct 9, 2019

i did some digging and it appears the auth.inc isn't aware of the StartTLS property like the LDAP.php is:

$authcfg['ldap_full_url'] = "ldaps://";

This return a ldaps:// url instead of a ldap:// which appears to be why it fails:
from the $ldap_server object:
["ldap_full_url"]=> string(30) "ldaps://ldap.jumpcloud.com:389"

from the authenticator object:
["ldapBindURL":"OPNsense\Auth\LDAP":private]=> string(29) "ldap://ldap.jumpcloud.com:389"
["useStartTLS":"OPNsense\Auth\LDAP":private]=> bool(true)

@AdSchellevis

This comment has been minimized.

Copy link
Member

commented Oct 9, 2019

@lbr88 I totally seem to have missed this, let me prepare a patch to try.

@lbr88

This comment has been minimized.

Copy link

commented Oct 9, 2019

--- /root/auth.inc	2019-10-09 15:43:25.245143000 +0000
+++ /usr/local/etc/inc/auth.inc	2019-10-09 15:44:23.532387000 +0000
@@ -724,7 +724,7 @@ function auth_get_authserver($name)
             if ($authcfg['name'] == $name) {
                 if ($authcfg['type'] == 'ldap' || $authcfg['type'] == 'ldap-totp') {
                     // let's try to avoid regenerating the ldap url in every function.
-                    if (strstr($authcfg['ldap_urltype'], "Standard")) {
+                    if (strstr($authcfg['ldap_urltype'], "Standard") || strstr($authcfg['ldap_urltype'], "StartTLS")) {
                         $authcfg['ldap_full_url'] = "ldap://";
                     } else {
                         $authcfg['ldap_full_url'] = "ldaps://";

seems to do the trick.

@AdSchellevis

This comment has been minimized.

Copy link
Member

commented Oct 9, 2019

we probably better move the code out of auth.inc, ideally this part should also be handled via the ldap object, but since system_usermanager_import_ldap.php is the only consumer of the logic now, we best move it there.

@AdSchellevis

This comment has been minimized.

Copy link
Member

commented Oct 9, 2019

ok, this c4fba1c should fix it then.

opnsense-patch c4fba1c
@lbr88

This comment has been minimized.

Copy link

commented Oct 9, 2019

Couldn't you allow the Auth\LDAP connect function be called without any parameters? From what i can see the setProperties function sets the ldapBindURL property when creating the instance from the authenticator factory, so a call to connect without any parameters could just use the properties already set within the object?
Just a thought, don't know the implications as i dont know the codebase.

Your patch c4fba1c works. Thank you.

@AdSchellevis

This comment has been minimized.

Copy link
Member

commented Oct 9, 2019

@lbr88 that was my first idea as well, but the other caller (system_usermanager_settings_ldapacpicker.php) would still need to remap its post settings too in order to really have a single entry point. Maybe another day, thanks for confirming, closing the issue now.

20.1 automation moved this from To Do to Done Oct 9, 2019
@gvecchicert

This comment has been minimized.

Copy link
Author

commented Oct 9, 2019

I confirm patch works like a charm!

fichtner added a commit that referenced this issue Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
20.1
  
Done
4 participants
You can’t perform that action at this time.