Bug 554

Summary:	sudoers ldap: wrong sequence of init operations
Product:	Sudo	Reporter:	Jan Vcelak <jvcelak>
Component:	Sudoers	Assignee:	Todd C. Miller <Todd.Miller>
Status:	RESOLVED FIXED
Severity:	normal
Priority:	low
Version:	1.8.4
Hardware:	PC
OS:	Linux
Attachments:	proposed patch Alternate patch to fix sudo TLS problem

Description Jan Vcelak 2012-04-20 09:31:10 MDT

Created attachment 340 [details]
proposed patch

OpenLDAP by default uses global TLS context, which is shared in the whole application. All TLS options are copied to a handle options when ldap_initialize() is called. Sudo initializes the handle, and then sets the global options. Including the TLS settings. Which means that the global TLS options are different from the options associated with the LDAP handle.

When the connection is created, global TLS context is created from global options. But because the options in handle are wrong, hostname verification is always triggered, even if "tls_checkpeer" is sudo is set to "no".

This causes a problem with Mozilla NSS crypto backend in OpenLDAP, where this sequence of operations makes "tls_checkpeer no" not effective.

I reported this to OpenLDAP issue tracker, but I was explained, that this is a bug in sudo. For details see:
http://www.openldap.org/its/index.cgi?findid=7240

So there are two possibilities:

1. reorder the operations:
- apply all global LDAP settings:  ldap_set_option(NULL, opt, &val)
- initialize the handle: ldap_initialize(&ld, url)
- apply all handle specific options: ldap_set_option(ld, opt, &val)

2. request non-global TLS context:
- do everything as it is now, except that LDAP_OPT_X_TLS_* options should be applied non-globally: ldap_set_option(ld, opt, &val)
- create the TLS context after setting the options, before connecting:
ldap_set_option(ld, LDAP_OPT_X_TLS_NEWCTX, &is_server);

My patch goes the second way. I found it less intrusive, and the table with options in sudo is iterated just once.

Feel free to modify the patch as needed.

Comment 1 Todd C. Miller 2012-04-23 11:54:14 MDT

Created attachment 342 [details]
Alternate patch to fix sudo TLS problem

I'd rather just split up global and per-connection options and apply them separately.  Can you try the attached path and verify that it solves the problem?

Comment 2 Jan Vcelak 2012-04-23 14:23:38 MDT

(In reply to comment #1)
> Created attachment 342 [details]
> Alternate patch to fix sudo TLS problem
> 
> I'd rather just split up global and per-connection options and apply
> them separately.  Can you try the attached path and verify that it
> solves the problem?

I will try and let you know. Hopefully today.

Comment 3 Jan Vcelak 2012-04-23 17:09:26 MDT

(In reply to comment #1)
> Created attachment 342 [details]
> Alternate patch to fix sudo TLS problem

I tried your patch with sudo-1.8.3p1-6.fc17 (from Fedora Rawhide), it applied with a little changes. However the patch is causing a segfault.

The segfault is caused by passing uninitialized ld pointer to sudo_ldap_set_options_global() function. In fact, there is no need to pass the LDAP handle because ldap_set_option() should not be given a handle to apply the global options.

I suggest this change in your patch:

--- sudo-sudoers-ldap.patch
+++ sudo-sudoers-ldap.patch.fixed
@@ -344,7 +344,7 @@
 + * Set LDAP options based on the global config table.
 + */
 +static int
-+sudo_ldap_set_options_global(LDAP *ld)
++sudo_ldap_set_options_global(void)
 +{
 +    int rc;
 +    debug_decl(sudo_ldap_set_options_global, SUDO_DEBUG_LDAP)
@@ -356,7 +356,7 @@
 +#endif
 +
 +    /* Parse global LDAP options table. */
-+    rc = sudo_ldap_set_options_table(ld, ldap_conf_global);
++    rc = sudo_ldap_set_options_table(NULL, ldap_conf_global);
 +    if (rc == -1)
 +      debug_return_int(-1);
 +    debug_return_int(0);
@@ -383,7 +383,7 @@
      }
  
 +    /* Set global LDAP options */
-+    if (sudo_ldap_set_options_global(ld) < 0)
++    if (sudo_ldap_set_options_global() < 0)
 +      debug_return_int(-1);
 +
      /* Connect to LDAP server */

After this change, it works as expected. If you want to be sure, I can test with latest sudo from the repository. And with OpenLDAP with both OpenSSL and Mozilla NSS crypto backends.

Comment 4 Todd C. Miller 2012-04-23 19:58:21 MDT

Good catch, I had intended to make sudo_ldap_set_options_global() take no arguments but goofed up when splitting things out.  I've fixed that in the sudo source repo.  This will be part of the next sudo 1.8.5 release candidate.

Comment 5 Jan Vcelak 2012-04-24 03:36:23 MDT

Great! Thank you, Todd.

Comment 6 Todd C. Miller 2012-05-17 11:09:48 MDT

Fixed in sudo 1.8.5.