>From a4c09c8e15967606dc32b10c52e209bceae9e317 Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzidek@redhat.com>
Date: Wed, 8 Aug 2012 14:25:54 +0200
Subject: [PATCH] If the same server is in both primary and secondary server
 list, we must only add it to the primary list.

Also SRV resolution for backup servers should not be permitted.

https://fedorahosted.org/sssd/ticket/1463
---
 src/providers/ad/ad_common.c     | 54 +++++++++++++++++++++++----------
 src/providers/ipa/ipa_common.c   | 57 ++++++++++++++++++++++++-----------
 src/providers/krb5/krb5_common.c | 58 +++++++++++++++++++++++++-----------
 src/providers/ldap/ldap_common.c | 64 +++++++++++++++++++++++++++-------------
 4 files changed, 162 insertions(+), 71 deletions(-)

diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c
index a4703ee..acc89d8 100644
--- a/src/providers/ad/ad_common.c
+++ b/src/providers/ad/ad_common.c
@@ -147,31 +147,30 @@ ad_resolve_callback(void *private_data, struct fo_server *server);
 static errno_t
 ad_servers_init(TALLOC_CTX *mem_ctx,
                 struct be_ctx *bectx,
-                const char *servers,
+                char **servers,
                 struct ad_options *options,
                 bool primary)
 {
     size_t i;
-    errno_t ret;
-    char **list;
+    errno_t ret = 0;
     char *ad_domain;
     TALLOC_CTX *tmp_ctx;
 
     tmp_ctx = talloc_new(NULL);
     if (!tmp_ctx) return ENOMEM;
 
-    /* Split the server list */
-    ret = split_on_separator(tmp_ctx, servers, ',', true, &list, NULL);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to parse server list!\n"));
-        goto done;
-    }
-
     ad_domain = dp_opt_get_string(options->basic, AD_DOMAIN);
 
     /* Add each of these servers to the failover service */
-    for (i = 0; list[i]; i++) {
-        if (be_fo_is_srv_identifier(list[i])) {
+    for (i = 0; servers[i]; i++) {
+        if (be_fo_is_srv_identifier(servers[i])) {
+            if (!primary) {
+                DEBUG(SSSDBG_MINOR_FAILURE,
+                      ("Failed to add server [%s] to failover service: "
+                       "SRV resolution only allowed for primary servers!\n",
+                       servers[i]));
+                continue;
+            }
             ret = be_fo_add_srv_server(bectx, AD_SERVICE_NAME, "ldap",
                                        ad_domain, BE_FO_PROTO_TCP,
                                        false, NULL);
@@ -186,13 +185,13 @@ ad_servers_init(TALLOC_CTX *mem_ctx,
             continue;
         }
 
-        ret = be_fo_add_server(bectx, AD_SERVICE_NAME, list[i], 0, NULL, primary);
+        ret = be_fo_add_server(bectx, AD_SERVICE_NAME, servers[i], 0, NULL, primary);
         if (ret && ret != EEXIST) {
             DEBUG(SSSDBG_FATAL_FAILURE, ("Failed to add server\n"));
             goto done;
         }
 
-        DEBUG(SSSDBG_CONF_SETTINGS, ("Added failover server %s\n", list[i]));
+        DEBUG(SSSDBG_CONF_SETTINGS, ("Added failover server %s\n", servers[i]));
     }
 done:
     talloc_free(tmp_ctx);
@@ -210,6 +209,9 @@ ad_failover_init(TALLOC_CTX *mem_ctx, struct be_ctx *bectx,
     TALLOC_CTX *tmp_ctx;
     struct ad_service *service;
     char *realm;
+    char **primary_list;
+    char **backup_list;
+    char **back_only_list;
 
     tmp_ctx = talloc_new(mem_ctx);
     if (!tmp_ctx) return ENOMEM;
@@ -279,13 +281,33 @@ ad_failover_init(TALLOC_CTX *mem_ctx, struct be_ctx *bectx,
         }
     }
 
-    ret = ad_servers_init(mem_ctx, bectx, primary_servers, options, true);
+    /* split primary_servers into a list */
+    ret = split_on_separator(tmp_ctx, primary_servers, ',', true,
+                             &primary_list, NULL);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to parse server list!\n"));
+        goto done;
+    }
+
+    ret = ad_servers_init(mem_ctx, bectx, primary_list, options, true);
     if (ret != EOK) {
         goto done;
     }
 
     if (backup_servers) {
-        ret = ad_servers_init(mem_ctx, bectx, backup_servers, options, false);
+        /* split backup_servers into a list */
+        ret = split_on_separator(tmp_ctx, backup_servers, ',', true,
+                                 &backup_list, NULL);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to parse server list!\n"));
+            goto done;
+        }
+
+        /* Use only those backup servers not included in primary_list */
+        diff_string_lists(tmp_ctx, primary_list, backup_list, NULL,
+                          &back_only_list, NULL);
+
+        ret = ad_servers_init(mem_ctx, bectx, back_only_list, options, false);
         if (ret != EOK) {
             goto done;
         }
diff --git a/src/providers/ipa/ipa_common.c b/src/providers/ipa/ipa_common.c
index b5f6337..b72797a 100644
--- a/src/providers/ipa/ipa_common.c
+++ b/src/providers/ipa/ipa_common.c
@@ -800,13 +800,12 @@ static void ipa_resolve_callback(void *private_data, struct fo_server *server)
 errno_t ipa_servers_init(struct be_ctx *ctx,
                          struct ipa_service *service,
                          struct ipa_options *options,
-                         const char *servers,
+                         char **servers,
                          bool primary)
 {
     TALLOC_CTX *tmp_ctx;
-    char **list = NULL;
     char *ipa_domain;
-    int ret;
+    int ret = 0;
     int i;
 
     tmp_ctx = talloc_new(NULL);
@@ -814,19 +813,20 @@ errno_t ipa_servers_init(struct be_ctx *ctx,
         return ENOMEM;
     }
 
-    /* split server parm into a list */
-    ret = split_on_separator(tmp_ctx, servers, ',', true, &list, NULL);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to parse server list!\n"));
-        goto done;
-    }
-
     /* now for each one add a new server to the failover service */
-    for (i = 0; list[i]; i++) {
+    for (i = 0; servers[i]; i++) {
+
+        talloc_steal(service, servers[i]);
 
-        talloc_steal(service, list[i]);
+        if (be_fo_is_srv_identifier(servers[i])) {
+            if (!primary) {
+                DEBUG(SSSDBG_MINOR_FAILURE,
+                      ("Failed to add server [%s] to failover service: "
+                       "SRV resolution only allowed for primary servers!\n",
+                       servers[i]));
+                continue;
+            }
 
-        if (be_fo_is_srv_identifier(list[i])) {
             ipa_domain = dp_opt_get_string(options->basic, IPA_DOMAIN);
             ret = be_fo_add_srv_server(ctx, "IPA", "ldap", ipa_domain,
                                        BE_FO_PROTO_TCP, false, NULL);
@@ -839,13 +839,13 @@ errno_t ipa_servers_init(struct be_ctx *ctx,
             continue;
         }
 
-        ret = be_fo_add_server(ctx, "IPA", list[i], 0, NULL, primary);
+        ret = be_fo_add_server(ctx, "IPA", servers[i], 0, NULL, primary);
         if (ret && ret != EEXIST) {
             DEBUG(SSSDBG_FATAL_FAILURE, ("Failed to add server\n"));
             goto done;
         }
 
-        DEBUG(SSSDBG_TRACE_FUNC, ("Added Server %s\n", list[i]));
+        DEBUG(SSSDBG_TRACE_FUNC, ("Added Server %s\n", servers[i]));
     }
 
 done:
@@ -863,6 +863,9 @@ int ipa_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx,
     struct ipa_service *service;
     char *realm;
     int ret;
+    char **primary_list;
+    char **backup_list;
+    char **back_only_list;
 
     tmp_ctx = talloc_new(NULL);
     if (!tmp_ctx) {
@@ -930,13 +933,33 @@ int ipa_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx,
         }
     }
 
-    ret = ipa_servers_init(ctx, service, options, primary_servers, true);
+    /* split primary_servers into a list */
+    ret = split_on_separator(tmp_ctx, primary_servers, ',', true,
+                             &primary_list, NULL);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to parse server list!\n"));
+        goto done;
+    }
+
+    ret = ipa_servers_init(ctx, service, options, primary_list, true);
     if (ret != EOK) {
         goto done;
     }
 
     if (backup_servers) {
-        ret = ipa_servers_init(ctx, service, options, backup_servers, false);
+        /* split backup_servers into a list */
+        ret = split_on_separator(tmp_ctx, backup_servers, ',', true,
+                                 &backup_list, NULL);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to parse server list!\n"));
+            goto done;
+        }
+
+        /* Use only those backup servers not included in primary_list */
+        diff_string_lists(tmp_ctx, primary_list, backup_list, NULL,
+                          &back_only_list, NULL);
+
+        ret = ipa_servers_init(ctx, service, options, back_only_list, false);
         if (ret != EOK) {
             goto done;
         }
diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c
index ad79db9..9577f58 100644
--- a/src/providers/krb5/krb5_common.c
+++ b/src/providers/krb5/krb5_common.c
@@ -468,12 +468,11 @@ static void krb5_resolve_callback(void *private_data, struct fo_server *server)
 errno_t krb5_servers_init(struct be_ctx *ctx,
                           struct krb5_service *service,
                           const char *service_name,
-                          const char *servers,
+                          char **servers,
                           bool primary)
 {
     TALLOC_CTX *tmp_ctx;
-    char **list = NULL;
-    errno_t ret;
+    errno_t ret = 0;
     int i;
     char *port_str;
     long port;
@@ -486,22 +485,23 @@ errno_t krb5_servers_init(struct be_ctx *ctx,
         return ENOMEM;
     }
 
-    ret = split_on_separator(tmp_ctx, servers, ',', true, &list, NULL);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to parse server list!\n"));
-        goto done;
-    }
+    for (i = 0; servers[i]; i++) {
 
-    for (i = 0; list[i]; i++) {
-
-        talloc_steal(service, list[i]);
-        server_spec = talloc_strdup(service, list[i]);
+        talloc_steal(service, servers[i]);
+        server_spec = talloc_strdup(service, servers[i]);
         if (!server_spec) {
             ret = ENOMEM;
             goto done;
         }
 
         if (be_fo_is_srv_identifier(server_spec)) {
+            if (!primary) {
+                DEBUG(SSSDBG_MINOR_FAILURE,
+                      ("Failed to add server [%s] to failover service: "
+                       "SRV resolution only allowed for primary servers!\n",
+                       servers[i]));
+                continue;
+            }
             ret = be_fo_add_srv_server(ctx, service_name, service_name, NULL,
                                        BE_FO_PROTO_UDP, true, NULL);
             if (ret) {
@@ -551,20 +551,20 @@ errno_t krb5_servers_init(struct be_ctx *ctx,
 
                 port = servent->s_port;
             } else {
-                DEBUG(SSSDBG_CRIT_FAILURE, ("Unsupported port specifier in [%s].\n", list[i]));
+                DEBUG(SSSDBG_CRIT_FAILURE, ("Unsupported port specifier in [%s].\n", servers[i]));
                 ret = EINVAL;
                 goto done;
             }
         }
 
         ret = be_fo_add_server(ctx, service_name, server_spec, (int) port,
-                               list[i], primary);
+                               servers[i], primary);
         if (ret && ret != EEXIST) {
             DEBUG(SSSDBG_FATAL_FAILURE, ("Failed to add server\n"));
             goto done;
         }
 
-        DEBUG(SSSDBG_TRACE_FUNC, ("Added Server %s\n", list[i]));
+        DEBUG(SSSDBG_TRACE_FUNC, ("Added Server %s\n", servers[i]));
     }
 
 done:
@@ -581,6 +581,9 @@ int krb5_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx,
     TALLOC_CTX *tmp_ctx;
     struct krb5_service *service;
     int ret;
+    char **primary_list;
+    char **backup_list;
+    char **back_only_list;
 
     tmp_ctx = talloc_new(memctx);
     if (!tmp_ctx) {
@@ -626,13 +629,34 @@ int krb5_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx,
         }
     }
 
-    ret = krb5_servers_init(ctx, service, service_name, primary_servers, true);
+    /* split primary_servers into a list */
+    ret = split_on_separator(tmp_ctx, primary_servers, ',', true,
+                             &primary_list, NULL);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to parse server list!\n"));
+        goto done;
+    }
+
+    ret = krb5_servers_init(ctx, service, service_name, primary_list, true);
     if (ret != EOK) {
         goto done;
     }
 
     if (backup_servers) {
-        ret = krb5_servers_init(ctx, service, service_name, backup_servers, false);
+        /* split backup_servers into a list */
+        ret = split_on_separator(tmp_ctx, backup_servers, ',', true,
+                                 &backup_list, NULL);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to parse server list!\n"));
+            goto done;
+        }
+
+        /* Use only those backup servers not included in primary_list */
+        diff_string_lists(tmp_ctx, primary_list, backup_list, NULL,
+                          &back_only_list, NULL);
+
+        ret = krb5_servers_init(ctx, service, service_name, back_only_list,
+                                false);
         if (ret != EOK) {
             goto done;
         }
diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index 1fc5ebb..6778ebb 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -1109,14 +1109,13 @@ errno_t sdap_urls_init(struct be_ctx *ctx,
                        struct sdap_service *service,
                        const char *service_name,
                        const char *dns_service_name,
-                       const char *urls,
+                       char **urls,
                        bool primary)
 {
     TALLOC_CTX *tmp_ctx;
     char *srv_user_data;
-    char **list = NULL;
     LDAPURLDesc *lud;
-    errno_t ret;
+    errno_t ret = 0;
     int i;
 
     tmp_ctx = talloc_new(NULL);
@@ -1124,17 +1123,17 @@ errno_t sdap_urls_init(struct be_ctx *ctx,
         return ENOMEM;
     }
 
-
-    /* split server parm into a list */
-    ret = split_on_separator(tmp_ctx, urls, ',', true, &list, NULL);
-    if (ret != EOK) {
-        DEBUG(1, ("Failed to parse server list!\n"));
-        goto done;
-    }
-
     /* now for each URI add a new server to the failover service */
-    for (i = 0; list[i]; i++) {
-        if (be_fo_is_srv_identifier(list[i])) {
+    for (i = 0; urls[i]; i++) {
+        if (be_fo_is_srv_identifier(urls[i])) {
+            if (!primary) {
+                DEBUG(SSSDBG_MINOR_FAILURE,
+                      ("Failed to add server [%s] to failover service: "
+                       "SRV resolution only allowed for primary servers!\n",
+                       urls[i]));
+                continue;
+            }
+
             if (!dns_service_name) {
                 DEBUG(0, ("Missing DNS service name for service [%s].\n",
                           service_name));
@@ -1159,26 +1158,26 @@ errno_t sdap_urls_init(struct be_ctx *ctx,
             continue;
         }
 
-        ret = ldap_url_parse(list[i], &lud);
+        ret = ldap_url_parse(urls[i], &lud);
         if (ret != LDAP_SUCCESS) {
-            DEBUG(0, ("Failed to parse ldap URI (%s)!\n", list[i]));
+            DEBUG(0, ("Failed to parse ldap URI (%s)!\n", urls[i]));
             ret = EINVAL;
             goto done;
         }
 
         if (lud->lud_host == NULL) {
             DEBUG(2, ("The LDAP URI (%s) did not contain a host name\n",
-                      list[i]));
+                      urls[i]));
             ldap_free_urldesc(lud);
             continue;
         }
 
-        DEBUG(6, ("Added URI %s\n", list[i]));
+        DEBUG(6, ("Added URI %s\n", urls[i]));
 
-        talloc_steal(service, list[i]);
+        talloc_steal(service, urls[i]);
 
         ret = be_fo_add_server(ctx, service->name, lud->lud_host,
-                               lud->lud_port, list[i], primary);
+                               lud->lud_port, urls[i], primary);
         ldap_free_urldesc(lud);
         if (ret) {
             goto done;
@@ -1198,6 +1197,9 @@ int sdap_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx,
     TALLOC_CTX *tmp_ctx;
     struct sdap_service *service;
     int ret;
+    char **primary_list;
+    char **backup_list;
+    char **back_only_list;
 
     tmp_ctx = talloc_new(NULL);
     if (!tmp_ctx) {
@@ -1237,15 +1239,35 @@ int sdap_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx,
         }
     }
 
+    /* split primary_urls into a list */
+    ret = split_on_separator(tmp_ctx, urls, ',', true,
+                             &primary_list, NULL);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to parse server list!\n"));
+        goto done;
+    }
+
     ret = sdap_urls_init(ctx, service, service_name, dns_service_name,
-                         urls, true);
+                         primary_list, true);
     if (ret != EOK) {
         goto done;
     }
 
     if (backup_urls) {
+        /* split backup_servers into a list */
+        ret = split_on_separator(tmp_ctx, backup_urls, ',', true,
+                                 &backup_list, NULL);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to parse server list!\n"));
+            goto done;
+        }
+
+        /* Use only those backup servers not included in primary_list */
+        diff_string_lists(tmp_ctx, primary_list, backup_list, NULL,
+                          &back_only_list, NULL);
+
         ret = sdap_urls_init(ctx, service, service_name, dns_service_name,
-                             backup_urls, false);
+                             back_only_list, false);
         if (ret != EOK) {
             goto done;
         }
-- 
1.7.11.2

