DDNS: execute nsupdate for single update of PTR rec

nsupdate fails definitely if any of update request fails when GSSAPI is used.

As tmp solution nsupdate is executed for each update.

Resolves:
https://fedorahosted.org/sssd/ticket/2783

Reviewed-by: Jakub Hrozek <jhrozek@redhat.com>

4 files changed, 219 insertions, 72 deletions

diff --git a/src/providers/dp_dyndns.c b/src/providers/dp_dyndns.c
index 0577743cb..50b087446 100644
--- a/src/providers/dp_dyndns.c
+++ b/src/providers/dp_dyndns.c
@@ -52,6 +52,25 @@ struct sss_iface_addr {
     struct sockaddr_storage *addr;
 };
 
+struct sockaddr_storage*
+sss_iface_addr_get_address(struct sss_iface_addr *address)
+{
+    if (address == NULL) {
+        return NULL;
+    }
+
+    return address->addr;
+}
+
+struct sss_iface_addr *sss_iface_addr_get_next(struct sss_iface_addr *address)
+{
+    if (address) {
+        return address->next;
+    }
+
+    return NULL;
+}
+
 void sss_iface_addr_concatenate(struct sss_iface_addr **list,
                                 struct sss_iface_addr *list2)
 {
@@ -293,80 +312,63 @@ nsupdate_msg_add_fwd(char *update_msg, struct sss_iface_addr *addresses,
     return talloc_asprintf_append(update_msg, "send\n");
 }
 
-static char *
-nsupdate_msg_add_ptr(char *update_msg, struct sss_iface_addr *addresses,
-                     const char *hostname, int ttl, uint8_t remove_af,
-                     struct sss_iface_addr *old_addresses)
+static uint8_t *nsupdate_convert_address(struct sockaddr_storage *add_address)
+{
+    uint8_t *addr;
+
+    switch(add_address->ss_family) {
+    case AF_INET:
+        addr = (uint8_t *) &((struct sockaddr_in *) add_address)->sin_addr;
+        break;
+    case AF_INET6:
+        addr = (uint8_t *) &((struct sockaddr_in6 *) add_address)->sin6_addr;
+        break;
+    default:
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unknown address family\n");
+        addr = NULL;
+        break;
+    }
+
+    return addr;
+}
+
+static char *nsupdate_msg_add_ptr(char *update_msg,
+                                  struct sockaddr_storage *address,
+                                  const char *hostname,
+                                  int ttl,
+                                  bool delete)
 {
-    struct sss_iface_addr *new_record, *old_record;
     char *strptr;
     uint8_t *addr;
 
-    DLIST_FOR_EACH(old_record, old_addresses) {
-        switch(old_record->addr->ss_family) {
-        case AF_INET:
-            if (!(remove_af & DYNDNS_REMOVE_A)) {
-                continue;
-            }
-            addr = (uint8_t *) &((struct sockaddr_in *) old_record->addr)->sin_addr;
-            break;
-        case AF_INET6:
-            if (!(remove_af & DYNDNS_REMOVE_AAAA)) {
-                continue;
-            }
-            addr = (uint8_t *) &((struct sockaddr_in6 *) old_record->addr)->sin6_addr;
-            break;
-        default:
-            DEBUG(SSSDBG_CRIT_FAILURE, "Unknown address family\n");
-            return NULL;
-        }
+    addr = nsupdate_convert_address(address);
+    if (addr == NULL) {
+        return NULL;
+    }
 
-        strptr = resolv_get_string_ptr_address(update_msg, old_record->addr->ss_family,
-                                               addr);
-        if (strptr == NULL) {
-            return NULL;
-        }
+    strptr = resolv_get_string_ptr_address(update_msg, address->ss_family,
+                                           addr);
+    if (strptr == NULL) {
+        return NULL;
+    }
 
+    if (delete) {
         /* example: update delete 38.78.16.10.in-addr.arpa. in PTR */
         update_msg = talloc_asprintf_append(update_msg,
                                             "update delete %s in PTR\n"
                                             "send\n",
                                             strptr);
-        talloc_free(strptr);
-        if (update_msg == NULL) {
-            return NULL;
-        }
-    }
-
-    /* example: update add 11.78.16.10.in-addr.arpa. 85000 in PTR testvm.example.com */
-    DLIST_FOR_EACH(new_record, addresses) {
-        switch(new_record->addr->ss_family) {
-        case AF_INET:
-            addr = (uint8_t *) &((struct sockaddr_in *) new_record->addr)->sin_addr;
-            break;
-        case AF_INET6:
-            addr = (uint8_t *) &((struct sockaddr_in6 *) new_record->addr)->sin6_addr;
-            break;
-        default:
-            DEBUG(SSSDBG_CRIT_FAILURE, "Unknown address family\n");
-            return NULL;
-        }
-
-        strptr = resolv_get_string_ptr_address(update_msg, new_record->addr->ss_family,
-                                               addr);
-        if (strptr == NULL) {
-            return NULL;
-        }
-
+    } else {
         /* example: update delete 38.78.16.10.in-addr.arpa. in PTR */
         update_msg = talloc_asprintf_append(update_msg,
                                             "update add %s %d in PTR %s.\n"
                                             "send\n",
                                             strptr, ttl, hostname);
-        talloc_free(strptr);
-        if (update_msg == NULL) {
-            return NULL;
-        }
+    }
+
+    talloc_free(strptr);
+    if (update_msg == NULL) {
+        return NULL;
     }
 
     return update_msg;
@@ -471,9 +473,9 @@ done:
 errno_t
 be_nsupdate_create_ptr_msg(TALLOC_CTX *mem_ctx, const char *realm,
                            const char *servername, const char *hostname,
-                           const unsigned int ttl, uint8_t remove_af,
-                           struct sss_iface_addr *addresses,
-                           struct sss_iface_addr *old_addresses,
+                           const unsigned int ttl,
+                           struct sockaddr_storage *address,
+                           bool delete,
                            char **_update_msg)
 {
     errno_t ret;
@@ -490,8 +492,8 @@ be_nsupdate_create_ptr_msg(TALLOC_CTX *mem_ctx, const char *realm,
         goto done;
     }
 
-    update_msg = nsupdate_msg_add_ptr(update_msg, addresses, hostname,
-                                      ttl, remove_af, old_addresses);
+    update_msg = nsupdate_msg_add_ptr(update_msg, address, hostname, ttl,
+                                      delete);
     if (update_msg == NULL) {
         ret = ENOMEM;
         goto done;
diff --git a/src/providers/dp_dyndns.h b/src/providers/dp_dyndns.h
index 9f72331b6..9f39e5d48 100644
--- a/src/providers/dp_dyndns.h
+++ b/src/providers/dp_dyndns.h
@@ -97,9 +97,9 @@ be_nsupdate_create_fwd_msg(TALLOC_CTX *mem_ctx, const char *realm,
 errno_t
 be_nsupdate_create_ptr_msg(TALLOC_CTX *mem_ctx, const char *realm,
                            const char *servername, const char *hostname,
-                           const unsigned int ttl, uint8_t remove_af,
-                           struct sss_iface_addr *addresses,
-                           struct sss_iface_addr *old_addresses,
+                           const unsigned int ttl,
+                           struct sockaddr_storage *address,
+                           bool delete,
                            char **_update_msg);
 
 /* Returns:
@@ -133,4 +133,11 @@ errno_t
 sss_get_dualstack_addresses(TALLOC_CTX *mem_ctx,
                             struct sockaddr *ss,
                             struct sss_iface_addr **_iface_addrs);
+
+struct sss_iface_addr *
+sss_iface_addr_get_next(struct sss_iface_addr *address);
+
+struct sockaddr_storage*
+sss_iface_addr_get_address(struct sss_iface_addr *address);
+
 #endif /* DP_DYNDNS_H_ */
diff --git a/src/providers/ldap/sdap_dyndns.c b/src/providers/ldap/sdap_dyndns.c
index 2a179fd1b..3a52a11d1 100644
--- a/src/providers/ldap/sdap_dyndns.c
+++ b/src/providers/ldap/sdap_dyndns.c
@@ -60,6 +60,8 @@ struct sdap_dyndns_update_state {
     enum be_nsupdate_auth auth_type;
     bool fallback_mode;
     char *update_msg;
+    struct sss_iface_addr *ptr_addr_iter;
+    bool del_phase;
 };
 
 static void sdap_dyndns_update_addrs_done(struct tevent_req *subreq);
@@ -70,6 +72,12 @@ static errno_t sdap_dyndns_update_step(struct tevent_req *req);
 static errno_t sdap_dyndns_update_ptr_step(struct tevent_req *req);
 static void sdap_dyndns_update_done(struct tevent_req *subreq);
 static void sdap_dyndns_update_ptr_done(struct tevent_req *subreq);
+static errno_t
+sdap_dyndns_next_ptr_record(struct sdap_dyndns_update_state *state,
+                            struct tevent_req *req);
+static struct sss_iface_addr*
+sdap_get_address_to_delete(struct sss_iface_addr *address_it,
+                           uint8_t remove_af);
 
 struct tevent_req *
 sdap_dyndns_update_send(TALLOC_CTX *mem_ctx,
@@ -106,6 +114,8 @@ sdap_dyndns_update_send(TALLOC_CTX *mem_ctx,
     state->ev = ev;
     state->opts = opts;
     state->auth_type = auth_type;
+    state->ptr_addr_iter = NULL;
+    state->del_phase = true;
 
     /* fallback servername is overriden by user option */
     conf_servername = dp_opt_get_string(opts, DP_OPT_DYNDNS_SERVER);
@@ -381,6 +391,16 @@ sdap_dyndns_update_done(struct tevent_req *subreq)
     }
 
     talloc_free(state->update_msg);
+
+    /* init iterator for addresses to be deleted */
+    state->ptr_addr_iter = sdap_get_address_to_delete(state->dns_addrlist,
+                                                      state->remove_af);
+    if (state->ptr_addr_iter == NULL) {
+        /* init iterator for addresses to be added */
+        state->del_phase = false;
+        state->ptr_addr_iter = state->addresses;
+    }
+
     ret = sdap_dyndns_update_ptr_step(req);
     if (ret != EOK) {
         tevent_req_error(req, ret);
@@ -389,6 +409,50 @@ sdap_dyndns_update_done(struct tevent_req *subreq)
     /* Execution will resume in sdap_dyndns_update_ptr_done */
 }
 
+
+static bool remove_addr(int address_family, uint8_t remove_af)
+{
+    bool ret = false;
+
+    switch(address_family) {
+    case AF_INET:
+        if (remove_af & DYNDNS_REMOVE_A) {
+            ret = true;
+        }
+        break;
+    case AF_INET6:
+        if (remove_af & DYNDNS_REMOVE_AAAA) {
+            ret = true;
+        }
+        break;
+    default:
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unknown address family\n");
+        ret = false;
+    }
+
+    return ret;
+}
+
+static struct sss_iface_addr*
+sdap_get_address_to_delete(struct sss_iface_addr *address_it,
+                           uint8_t remove_af)
+{
+    struct sockaddr_storage* address;
+
+    while (address_it != NULL) {
+        address = sss_iface_addr_get_address(address_it);
+
+        /* skip addresses that are not to be deleted */
+        if (remove_addr(address->ss_family, remove_af)) {
+            break;
+        }
+
+        address_it = sss_iface_addr_get_next(address_it);
+    }
+
+    return address_it;
+}
+
 static errno_t
 sdap_dyndns_update_ptr_step(struct tevent_req *req)
 {
@@ -396,6 +460,7 @@ sdap_dyndns_update_ptr_step(struct tevent_req *req)
     struct sdap_dyndns_update_state *state;
     const char *servername;
     struct tevent_req *subreq;
+    struct sockaddr_storage *address;
 
     state = tevent_req_data(req, struct sdap_dyndns_update_state);
 
@@ -405,11 +470,14 @@ sdap_dyndns_update_ptr_step(struct tevent_req *req)
         servername = state->servername;
     }
 
-    ret = be_nsupdate_create_ptr_msg(state, state->realm,
-                                     servername, state->hostname,
-                                     state->ttl, state->remove_af,
-                                     state->addresses, state->dns_addrlist,
-                                     &state->update_msg);
+    address = sss_iface_addr_get_address(state->ptr_addr_iter);
+    if (address == NULL) {
+        return EIO;
+    }
+
+    ret = be_nsupdate_create_ptr_msg(state, state->realm, servername,
+                                     state->hostname, state->ttl, address,
+                                     state->del_phase, &state->update_msg);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "Can't get addresses for DNS update\n");
         return ret;
@@ -454,13 +522,55 @@ sdap_dyndns_update_ptr_done(struct tevent_req *subreq)
             }
         }
 
+        ret = sdap_dyndns_next_ptr_record(state, req);
+        if (ret == EAGAIN) {
+            return;
+        }
+
         tevent_req_error(req, ret);
         return;
     }
 
+    ret = sdap_dyndns_next_ptr_record(state, req);
+    if (ret == EAGAIN) {
+        return;
+    }
+
     tevent_req_done(req);
 }
 
+static errno_t
+sdap_dyndns_next_ptr_record(struct sdap_dyndns_update_state *state,
+                            struct tevent_req *req)
+{
+    errno_t ret;
+
+    if (state->del_phase) {
+        /* iterate to next address to delete */
+        state->ptr_addr_iter = sdap_get_address_to_delete(
+            sss_iface_addr_get_next(state->ptr_addr_iter), state->remove_af);
+        if (state->ptr_addr_iter == NULL) {
+            /* init iterator for addresses to be added */
+            state->del_phase = false;
+            state->ptr_addr_iter = state->addresses;
+        }
+    } else {
+        /* iterate to next address to add */
+        state->ptr_addr_iter = sss_iface_addr_get_next(state->ptr_addr_iter);
+    }
+
+    if (state->ptr_addr_iter != NULL) {
+
+        state->fallback_mode = false;
+        ret = sdap_dyndns_update_ptr_step(req);
+        if (ret == EOK) {
+            return EAGAIN;
+        }
+    }
+
+    return EOK;
+}
+
 errno_t
 sdap_dyndns_update_recv(struct tevent_req *req)
 {
@@ -755,7 +865,6 @@ fail:
     return req;
 }
 
-
 static void
 sdap_dyndns_timer_conn_done(struct tevent_req *subreq)
 {
diff --git a/src/tests/cmocka/test_dyndns.c b/src/tests/cmocka/test_dyndns.c
index bed7565f4..815acf696 100644
--- a/src/tests/cmocka/test_dyndns.c
+++ b/src/tests/cmocka/test_dyndns.c
@@ -201,6 +201,32 @@ void will_return_getifaddrs(const char *ifname, const char *straddr,
     }
 }
 
+void dyndns_test_sss_iface_addr_get_misc(void **state)
+{
+    struct sss_iface_addr addrs[3];
+    struct sockaddr_storage ss[3];
+
+    addrs[0].prev = NULL;
+    addrs[0].next = &addrs[1];
+    addrs[0].addr = &ss[0];
+    addrs[1].prev = &addrs[0];
+    addrs[1].next = &addrs[2];
+    addrs[1].addr = &ss[1];
+    addrs[2].prev = &addrs[1];
+    addrs[2].next = NULL;
+    addrs[2].addr = &ss[2];
+
+    assert_ptr_equal(sss_iface_addr_get_address(NULL), NULL);
+    assert_ptr_equal(sss_iface_addr_get_address(&addrs[0]), &ss[0]);
+    assert_ptr_equal(sss_iface_addr_get_address(&addrs[1]), &ss[1]);
+    assert_ptr_equal(sss_iface_addr_get_address(&addrs[2]), &ss[2]);
+
+    assert_ptr_equal(sss_iface_addr_get_next(NULL), NULL);
+    assert_ptr_equal(sss_iface_addr_get_next(&addrs[0]), &addrs[1]);
+    assert_ptr_equal(sss_iface_addr_get_next(&addrs[1]), &addrs[2]);
+    assert_ptr_equal(sss_iface_addr_get_next(&addrs[2]), NULL);
+}
+
 void dyndns_test_get_ifaddr(void **state)
 {
     errno_t ret;
@@ -703,6 +729,9 @@ int main(int argc, const char *argv[])
 
     const struct CMUnitTest tests[] = {
         /* Utility functions unit test */
+        cmocka_unit_test_setup_teardown(dyndns_test_sss_iface_addr_get_misc,
+                                        dyndns_test_simple_setup,
+                                        dyndns_test_teardown),
         cmocka_unit_test_setup_teardown(dyndns_test_get_ifaddr,
                                         dyndns_test_simple_setup,
                                         dyndns_test_teardown),