BUG/MINOR: resolvers: don't lower the case of binary DNS format

The server's "hostname_dn" is in Domain Name format, not a pure string, as
converted by resolv_str_to_dn_label(). It is made of lower-case string
components delimited by binary lengths, e.g. <0x03>www<0x07>haproxy<0x03)org.
As such it must not be lowercased again in srv_state_srv_update(), because
1) it's useless on the name components since already done, and 2) because
it would replace component lengths 97 and above by 32-char shorter ones.
Granted, not many domain names have that large components so the risk is
very low but the operation is always wrong anyway. This was brought in
2.5 by commit 3406766d57 ("MEDIUM: resolvers: add a ref between servers
and srv request or used SRV record").

In the same vein, let's fix the confusing strcasecmp() that are applied
to this binary format, and use memcmp() instead. Here there's basically
no risk to incorrectly match the wrong record, but that test alone is
confusing enough to provoke the existence of the bug above.

Finally let's update the component for that field to mention that it's
in this format and already lower cased.

Better not backport this, the risk of facing this bug is almost zero, and
every time we touch such files something breaks for bad reasons.

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index f74f46f06aa7adcf7eba565b3707be45409318c4..ed3c36e957a715ff3b962897151b39ba4beea1c5 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -444,7 +444,7 @@ struct server {
        char *lastaddr;                         /* the address string provided by the server-state file */
        struct resolv_options resolv_opts;
        int hostname_dn_len;                    /* string length of the server hostname in Domain Name format */
-       char *hostname_dn;                      /* server hostname in Domain Name format */
+       char *hostname_dn;                      /* server hostname in Domain Name format (name is lower cased) */
        char *hostname;                         /* server hostname */
        struct sockaddr_storage init_addr;      /* plain IP address specified on the init-addr line */
        unsigned int init_addr_methods;         /* initial address setting, 3-bit per method, ends at 0, enough to store 10 entries */

diff --git a/src/server.c b/src/server.c
index 6d74a3f192601363b5dea6d1a5cf36d544b5bf5f..6326dadcbc3ab1ace353e3659c0e8f6d6a7420f1 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -4995,7 +4995,7 @@ struct server *snr_check_ip_callback(struct server *srv, void *ip, unsigned char
                HA_SPIN_LOCK(SERVER_LOCK, &tmpsrv->lock);
                if ((tmpsrv->hostname_dn == NULL) ||
                    (srv->hostname_dn_len != tmpsrv->hostname_dn_len) ||
-                   (strcasecmp(srv->hostname_dn, tmpsrv->hostname_dn) != 0) ||
+                   (memcmp(srv->hostname_dn, tmpsrv->hostname_dn, srv->hostname_dn_len) != 0) ||
                    (srv->puid == tmpsrv->puid)) {
                        HA_SPIN_UNLOCK(SERVER_LOCK, &tmpsrv->lock);
                        continue;
@@ -5082,7 +5082,7 @@ int srv_set_fqdn(struct server *srv, const char *hostname, int resolv_locked)
        if (resolution &&
            resolution->hostname_dn &&
            resolution->hostname_dn_len == hostname_dn_len &&
-           strcasecmp(resolution->hostname_dn, hostname_dn) == 0)
+           memcmp(resolution->hostname_dn, hostname_dn, hostname_dn_len) == 0)
                goto end;
 
        resolv_unlink_resolution(srv->resolv_requester);

diff --git a/src/server_state.c b/src/server_state.c
index 2bbcb46d978b9372d38c040dad3ef6b4e2472e2b..21170f8d295f0fe4cc88dc833f8a097cfb48953c 100644 (file)
--- a/src/server_state.c
+++ b/src/server_state.c
@@ -393,8 +393,6 @@ static void srv_state_srv_update(struct server *srv, int version, char **params)
         */
        else if (fqdn && !srv->hostname && srvrecord) {
                int res;
-               int i;
-               char *tmp;
 
                /* we can't apply previous state if SRV record has changed */
                if (!srv->srvrq) {
@@ -417,13 +415,7 @@ static void srv_state_srv_update(struct server *srv, int version, char **params)
                 * since this server has an hostname
                 */
                LIST_DEL_INIT(&srv->srv_rec_item);
-               srv->host_dn.key = tmp = strdup(srv->hostname_dn);
-
-               /* convert the key in lowercase because tree
-                * lookup is case sensitive but we don't care
-                */
-               for (i = 0; tmp[i]; i++)
-                       tmp[i] = tolower((unsigned char)tmp[i]);
+               srv->host_dn.key = strdup(srv->hostname_dn);
 
                /* insert in tree and set the srvrq expiration date */
                ebis_insert(&srv->srvrq->named_servers, &srv->host_dn);