(SUDO) Re: Bug in ldap.c - Fixed

[Aaron Spangler]

Andreas,

I added the HAVE_LBER & HAVE_LDAP_INITIALIZE.
You need to tweek config.h after running ./config.  Eventually config.h will 
autosense which LDAP library you have.

I also fixed the null pointer bug while debugging and you don't have (or don't 
have access to) the cn=defaults,ou=Sudoers.  The fix is slightly different 
than what you submitted. 

^^^ Please test the new code if you could.

Also I added a runtime option of ldap_version which defaults to LDAPv3.  This 
my default mimics pam_ldap and nss_ldap better now and makes OpenLDAP server 
work out of the box.

I did not implement a timeout option yet.  I can put it in with the failover 
stuff.  Please give me an example of how your /etc/ldap.conf lists failover 
so that I can write failover code.

Thanks again for all your hard work.

 -Aaron


On Thursday 13 November 2003 04:46 am, Andreas.Bussjaeger at t-systems.com wrote:
> Hi Aaron,
>
> I found a little bug in ldap.c while testing ACI's for my sudoers tree
> (only a special sudo-LDAP-User is allowed to browse the sudoers tree) and
> having the wrong ACI settings (which means I get no result searching for
> cn=defaults,$SUDOERS_BASE).
>
> My diffs (including the ldap_initialize stuff):
>
> Addition to config.h:
>
> #define HAVE_LBER
>
> We should add this and "#define HAVE_LDAP_INITIALIZE" via configure script
> later.
>
> diff -c ldap.c.orig ldap.c
> *** ldap.c.orig Fri Nov  7 09:47:24 2003
> --- ldap.c      Thu Nov 13 10:44:49 2003
> ***************
> *** 74,79 ****
> --- 74,82 ----
>
>
>   #ifdef HAVE_LDAP
> + #ifdef HAVE_LBER
> + #include <lber.h>
> + #endif
>   #include <ldap.h>
>
>   #ifndef LDAP_CONFIG
> ***************
> *** 537,542 ****
> --- 540,546 ----
>     /* Used for searches */
>     LDAPMessage *result=NULL;
>     LDAPMessage *entry=NULL;
> +   char *dn;
>     /* used to parse attributes */
>     char *f;
>     /* temp/final return values */
> ***************
> *** 551,557 ****
>
>
>     /* attempt connect */
> !   if (ldap_conf.uri) {
>
>       if (ldap_conf.debug>1) fprintf(stderr,
>              "ldap_initialize(ld,%s)\n",ldap_conf.uri);
> --- 555,562 ----
>
>
>     /* attempt connect */
> ! #ifdef HAVE_LDAP_INITIALIZE
> !     if (ldap_conf.uri) {
>
>       if (ldap_conf.debug>1) fprintf(stderr,
>              "ldap_initialize(ld,%s)\n",ldap_conf.uri);
> ***************
> *** 562,569 ****
>              rc,ldap_err2string(rc));
>         return VALIDATE_ERROR;
>       }
> !   } else if (ldap_conf.host) {
> !
>       if (ldap_conf.debug>1) fprintf(stderr,
>              "ldap_init(%s,%d)\n",ldap_conf.host,ldap_conf.port);
>
> --- 567,576 ----
>              rc,ldap_err2string(rc));
>         return VALIDATE_ERROR;
>       }
> !    } else if (ldap_conf.host) {
> ! #else
> !   if (ldap_conf.host) {
> ! #endif /* HAVE_LDAP_INITIALIZE */
>       if (ldap_conf.debug>1) fprintf(stderr,
>              "ldap_init(%s,%d)\n",ldap_conf.host,ldap_conf.port);
>
> ***************
> *** 575,581 ****
>       }
>     }
>
> !   /* Acutally connect */
>
>     rc=ldap_simple_bind_s(ld,ldap_conf.binddn,ldap_conf.bindpw);
>     if(rc){
> --- 582,588 ----
>       }
>     }
>
> !   /* Actually connect */
>
>     rc=ldap_simple_bind_s(ld,ldap_conf.binddn,ldap_conf.bindpw);
>     if(rc){
> ***************
> *** 586,592 ****
>
>     if (ldap_conf.debug) printf("ldap_bind() ok\n");
>
> -
>     /* Parse Default Options */
>
>     rc=ldap_search_s(ld,ldap_conf.base,LDAP_SCOPE_ONELEVEL,
> --- 593,598 ----
> ***************
> *** 593,603 ****
>                "cn=defaults",NULL,0,&result);
>     if (!rc) {
>       entry=ldap_first_entry(ld,result);
> !     if (ldap_conf.debug) printf("found:%s\n",ldap_get_dn(ld,entry));
> !     sudo_ldap_parse_options(ld,entry);
> !   } else {
> !     if (ldap_conf.debug) printf("no options found\n");
>     }
>
>     if (result) ldap_msgfree(result);
>     result=NULL;
> --- 599,618 ----
>                "cn=defaults",NULL,0,&result);
>     if (!rc) {
>       entry=ldap_first_entry(ld,result);
> !     dn=ldap_get_dn(ld,entry);
> !     if (dn) {
> !       if (ldap_conf.debug) {
> !         printf("found:%s\n",dn);
> !       }
> !       sudo_ldap_parse_options(ld,entry);
> !     }
> !     else {
> !       rc=1;
> !     }
>     }
> +   if (rc) {
> +     if (ldap_conf.debug) printf("no options found !\n");
> +   }
>
>     if (result) ldap_msgfree(result);
>     result=NULL;
>
> Your "printf("found:%s\n",ldap_get_dn(ld,entry));" without checking the
> search result correctly caused a segmentation fault.
>
> What do you think about using the "timout"-version of ldap_search_s (just
> to be save when the ldap server crashes or is no longer connectable via
> network) ?
>
> int ldap_search_st(LDAP *ld, char  *base,  int  scope,  char
>      *filter,   char  *attrs[],  int  attrsonly,  struct  timeval
>      *timeout, LDAPMessage **res);
>
> Another idea: We should be able to define a couple of ldap servers (we
> actually have one master and three slaves) in ldap.conf and failover when
> using ldap_init(), etc.
>
> Regards,
>
> Andreas
> ---
> Andreas Bußjäger
> für
> Special Tasks & Projects (ST&P)
> Projekt Toll-Collect - UNIX
> T-Systems International GmbH
> Hausadresse: Dachauer Straße 651, 80995 München
> Postanschrift: Postfach 50 01 30, 80971 München
> Telefon: +49 89 1011-3034
> Telefax: +49 89 1011-2622
> E-Fax:    +49 1805 3344911312
> E-Mail: Andreas.Bussjaeger at t-systems.com
> Internet: http://www.t-systems.com <http://www.t-systems.com>