We sometimes run into situations where one of the three servers a ypldap
can talk to will accept a TCP connection but won't do TLS properly, or won't
perform LDAP searches.  ypldap currently only tries servers until one accepts
the connection, so when this happens, it is less successful at updating than
it could be.

The diff below adjusts the ldap update code so it tries servers until it
either successfully queries one or it runs out of addresses to try.
If a server breaks after returning partial results, the ldap process will
still send what it got to the main process.  If the ldap process then gets
full results from another server, those will overwrite the partial results,
and if it doesn't, the main process will discard the partial results when it
gets a 'trash update' message from the ldap process.

While here, the diff also adds the server address to log messages about
servers not working, so it's easier to figure out what's going wrong.

ok?

Index: ldapclient.c
===================================================================
RCS file: /cvs/src/usr.sbin/ypldap/ldapclient.c,v
retrieving revision 1.46
diff -u -p -r1.46 ldapclient.c
--- ldapclient.c        13 Oct 2022 04:55:33 -0000      1.46
+++ ldapclient.c        3 Feb 2023 03:58:17 -0000
@@ -53,50 +53,10 @@ int client_build_req(struct idm *, struc
            int, int);
 int    client_search_idm(struct env *, struct idm *, struct aldap *,
            char **, char *, int, int, enum imsg_type);
-int    client_try_idm(struct env *, struct idm *);
+int    client_try_idm(struct env *, struct idm *, struct ypldap_addr *);
 void   client_addr_init(struct idm *);
 int    client_addr_free(struct idm *);
 
-struct aldap   *client_aldap_open(struct ypldap_addr_list *);
-
-/*
- * dummy wrapper to provide aldap_init with its fd's.
- */
-struct aldap *
-client_aldap_open(struct ypldap_addr_list *addr)
-{
-       int                      fd = -1;
-       struct ypldap_addr      *p;
-       struct aldap            *al;
-
-       TAILQ_FOREACH(p, addr, next) {
-               char                     hbuf[NI_MAXHOST], sbuf[NI_MAXSERV];
-               struct sockaddr         *sa = (struct sockaddr *)&p->ss;
-
-               if (getnameinfo(sa, SA_LEN(sa), hbuf, sizeof(hbuf), sbuf,
-                       sizeof(sbuf), NI_NUMERICHOST | NI_NUMERICSERV))
-                               errx(1, "could not get numeric hostname");
-
-               if ((fd = socket(sa->sa_family, SOCK_STREAM, 0)) == -1)
-                       return NULL;
-
-               if (connect(fd, sa, SA_LEN(sa)) == 0)
-                       break;
-
-               log_warn("connect to %s port %s failed", hbuf, sbuf);
-               close(fd);
-               fd = -1;
-       }
-
-       if (fd == -1)
-               return NULL;
-
-       al = aldap_init(fd);
-       if (al == NULL)
-               close(fd);
-       return al;
-}
-
 void
 client_addr_init(struct idm *idm)
 {
@@ -241,8 +201,12 @@ client_dispatch_dns(int fd, short events
        }
 
        TAILQ_FOREACH(idm, &env->sc_idms, idm_entry) {
-               if (client_try_idm(env, idm) == -1)
-                       idm->idm_state = STATE_LDAP_FAIL;
+               TAILQ_FOREACH(h, &idm->idm_addr, next) {
+                       if (client_try_idm(env, idm, h) == -1)
+                               idm->idm_state = STATE_LDAP_FAIL;
+                       else
+                               break;
+               }
 
                if (idm->idm_state < STATE_LDAP_DONE)
                        wait_cnt++;
@@ -585,17 +549,36 @@ fail:
 }
 
 int
-client_try_idm(struct env *env, struct idm *idm)
+client_try_idm(struct env *env, struct idm *idm, struct ypldap_addr *addr)
 {
        const char              *where;
+       char                     hbuf[NI_MAXHOST], sbuf[NI_MAXSERV];
        char                    *attrs[ATTR_MAX+1];
+       int                      fd = -1;
        int                      i, j;
+       struct sockaddr         *sa = (struct sockaddr *)&addr->ss;
        struct aldap_message    *m;
        struct aldap            *al;
 
+       if (getnameinfo(sa, SA_LEN(sa), hbuf, sizeof(hbuf), sbuf,
+           sizeof(sbuf), NI_NUMERICHOST | NI_NUMERICSERV))
+               errx(1, "could not get numeric hostname");
+
        where = "connect";
-       if ((al = client_aldap_open(&idm->idm_addr)) == NULL)
+       if ((fd = socket(sa->sa_family, SOCK_STREAM, 0)) == -1)
+               return (-1);
+
+       if (connect(fd, sa, SA_LEN(sa)) != 0) {
+               log_warn("connect to %s port %s failed", hbuf, sbuf);
+               close(fd);
+               return (-1);
+       }
+
+       al = aldap_init(fd);
+       if (al == NULL) {
+               close(fd);
                return (-1);
+       }
 
        if (idm->idm_flags & F_STARTTLS) {
                log_debug("requesting starttls");
@@ -625,8 +608,8 @@ client_try_idm(struct env *env, struct i
                if (aldap_tls(al, idm->idm_tls_config, idm->idm_name) < 0) {
                        const char *err;
                        aldap_get_errno(al, &err);
-                       log_warnx("TLS handshake with %s failed: %s",
-                           idm->idm_name, err);
+                       log_warnx("TLS handshake with %s(%s) failed: %s",
+                           idm->idm_name, hbuf, err);
                        goto bad;
                }
        }
@@ -653,8 +636,8 @@ client_try_idm(struct env *env, struct i
                where = "bind response";
                rc = aldap_get_resultcode(m);
                if (rc != LDAP_SUCCESS) {
-                       log_warnx("LDAP bind with %s failed: result code %d",
-                           idm->idm_name, rc);
+                       log_warnx("LDAP bind with %s(%s) failed: result code"
+                           " %d", idm->idm_name, hbuf, rc);
                        aldap_freemsg(m);
                        goto bad;
                }
@@ -703,7 +686,8 @@ client_try_idm(struct env *env, struct i
        return (0);
 bad:
        aldap_close(al);
-       log_debug("directory %s errored out in %s", idm->idm_name, where);
+       log_debug("directory %s(%s) errored out in %s", idm->idm_name, hbuf,
+           where);
        return (-1);
 }
 

Reply via email to