Blob Blame History Raw
From 6a77f9470ccd3fc90d850b71b0f0564573342c01 Mon Sep 17 00:00:00 2001
From: Tomas Halman <thalman@redhat.com>
Date: Mon, 29 Oct 2018 14:47:08 +0100
Subject: [PATCH] DYNDNS: SSSD does not batch DDNS update requests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

SSSD includes a 'send' command in between each record modification
and does not batch DDNS update requests. This is problematic in
complex AD environments because those requests may not be processed
by the same server.

Now forward zone update is done in two steps - one per
protocol family. If dyndns_update_per_family is set
to false, update is performed in single step.

Resolves:
https://github.com/SSSD/sssd/issues/4823

Reviewed-by: Pavel Březina <pbrezina@redhat.com>
(cherry picked from commit 5565dd365e704f6ded4f95db5bfbefd5dffc888b)

Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
---
 src/man/sssd-ad.5.xml            |  15 +++
 src/man/sssd-ipa.5.xml           |  15 +++
 src/providers/ad/ad_opts.c       |   1 +
 src/providers/be_dyndns.c        | 203 ++++++++++++++++++++-----------
 src/providers/be_dyndns.h        |  10 +-
 src/providers/ipa/ipa_opts.c     |   1 +
 src/providers/ldap/sdap_dyndns.c | 121 ++----------------
 src/tests/cmocka/test_dyndns.c   |  98 +++++++++++++--
 8 files changed, 264 insertions(+), 200 deletions(-)

diff --git a/src/man/sssd-ad.5.xml b/src/man/sssd-ad.5.xml
index 9e9e52eb3..6fc57ca21 100644
--- a/src/man/sssd-ad.5.xml
+++ b/src/man/sssd-ad.5.xml
@@ -1066,6 +1066,21 @@ ad_gpo_map_deny = +my_pam_service
                     </listitem>
                 </varlistentry>
 
+                <varlistentry>
+                    <term>dyndns_update_per_family (boolean)</term>
+                    <listitem>
+                        <para>
+                            DNS update is by default performed in two steps -
+                            IPv4 update and then IPv6 update. In some cases
+                            it might be desirable to perform IPv4 and IPv6
+                            update in single step.
+                        </para>
+                        <para>
+                            Default: true
+                        </para>
+                    </listitem>
+                </varlistentry>
+
                 <xi:include xmlns:xi="http://www.w3.org/2001/XInclude" href="include/override_homedir.xml" />
                 <xi:include xmlns:xi="http://www.w3.org/2001/XInclude" href="include/homedir_substring.xml" />
 
diff --git a/src/man/sssd-ipa.5.xml b/src/man/sssd-ipa.5.xml
index e46957d5f..2f1450213 100644
--- a/src/man/sssd-ipa.5.xml
+++ b/src/man/sssd-ipa.5.xml
@@ -313,6 +313,21 @@
                     </listitem>
                 </varlistentry>
 
+                <varlistentry>
+                    <term>dyndns_update_per_family (boolean)</term>
+                    <listitem>
+                        <para>
+                            DNS update is by default performed in two steps -
+                            IPv4 update and then IPv6 update. In some cases
+                            it might be desirable to perform IPv4 and IPv6
+                            update in single step.
+                        </para>
+                        <para>
+                            Default: true
+                        </para>
+                    </listitem>
+                </varlistentry>
+
                 <varlistentry>
                     <term>ipa_deskprofile_search_base (string)</term>
                     <listitem>
diff --git a/src/providers/ad/ad_opts.c b/src/providers/ad/ad_opts.c
index f181e5073..f2596a935 100644
--- a/src/providers/ad/ad_opts.c
+++ b/src/providers/ad/ad_opts.c
@@ -282,6 +282,7 @@ struct sdap_attr_map ad_autofs_entry_map[] = {
 
 struct dp_option ad_dyndns_opts[] = {
     { "dyndns_update", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE },
+    { "dyndns_update_per_family", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
     { "dyndns_refresh_interval", DP_OPT_NUMBER, { .number = 86400 }, NULL_NUMBER },
     { "dyndns_iface", DP_OPT_STRING, NULL_STRING, NULL_STRING },
     { "dyndns_ttl", DP_OPT_NUMBER, { .number = 3600 }, NULL_NUMBER },
diff --git a/src/providers/be_dyndns.c b/src/providers/be_dyndns.c
index ebe1fcd16..0907f9a53 100644
--- a/src/providers/be_dyndns.c
+++ b/src/providers/be_dyndns.c
@@ -267,72 +267,80 @@ done:
 
 static char *
 nsupdate_msg_add_fwd(char *update_msg, struct sss_iface_addr *addresses,
-                     const char *hostname, int ttl, uint8_t remove_af)
+                     const char *hostname, int ttl, uint8_t remove_af, bool update_per_family)
 {
     struct sss_iface_addr *new_record;
     char ip_addr[INET6_ADDRSTRLEN];
+    char *updateipv4 = talloc_strdup(update_msg, "");
+    char *updateipv6 = talloc_strdup(update_msg, "");
     errno_t ret;
 
-    /* A addresses first */
     /* Remove existing entries as needed */
     if (remove_af & DYNDNS_REMOVE_A) {
-        update_msg = talloc_asprintf_append(update_msg,
+        updateipv4 = talloc_asprintf_append(updateipv4,
                                             "update delete %s. in A\n",
                                             hostname);
-        if (update_msg == NULL) {
+        if (updateipv4 == NULL) {
             return NULL;
         }
     }
-    DLIST_FOR_EACH(new_record, addresses) {
-        if (new_record->addr->ss_family == AF_INET) {
-            ret = addr_to_str(new_record->addr, ip_addr, INET6_ADDRSTRLEN);
-            if (ret != EOK) {
-                DEBUG(SSSDBG_MINOR_FAILURE, "addr_to_str failed: %d:[%s],\n",
-                      ret, sss_strerror(ret));
-                return NULL;
-            }
-
-            /* Format the record update */
-            update_msg = talloc_asprintf_append(update_msg,
-                                                "update add %s. %d in %s %s\n",
-                                                hostname, ttl, "A", ip_addr);
-            if (update_msg == NULL) {
-                return NULL;
-            }
-        }
-    }
-    update_msg = talloc_asprintf_append(update_msg, "send\n");
 
-    /* AAAA addresses next */
-    /* Remove existing entries as needed */
     if (remove_af & DYNDNS_REMOVE_AAAA) {
-        update_msg = talloc_asprintf_append(update_msg,
+        updateipv6 = talloc_asprintf_append(updateipv6,
                                             "update delete %s. in AAAA\n",
                                             hostname);
-        if (update_msg == NULL) {
+        if (updateipv6 == NULL) {
             return NULL;
         }
     }
+
     DLIST_FOR_EACH(new_record, addresses) {
-        if (new_record->addr->ss_family == AF_INET6) {
-            ret = addr_to_str(new_record->addr, ip_addr, INET6_ADDRSTRLEN);
-            if (ret != EOK) {
-                DEBUG(SSSDBG_MINOR_FAILURE, "addr_to_str failed: %d:[%s],\n",
-                      ret, sss_strerror(ret));
+        ret = addr_to_str(new_record->addr, ip_addr, INET6_ADDRSTRLEN);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_MINOR_FAILURE, "addr_to_str failed: %d:[%s],\n",
+                  ret, sss_strerror(ret));
+            return NULL;
+        }
+
+        switch (new_record->addr->ss_family) {
+        case AF_INET:
+            updateipv4 = talloc_asprintf_append(updateipv4,
+                                                "update add %s. %d in %s %s\n",
+                                                hostname, ttl, "A", ip_addr);
+            if (updateipv4 == NULL) {
                 return NULL;
             }
 
-            /* Format the record update */
-            update_msg = talloc_asprintf_append(update_msg,
+            break;
+        case AF_INET6:
+            updateipv6 = talloc_asprintf_append(updateipv6,
                                                 "update add %s. %d in %s %s\n",
                                                 hostname, ttl, "AAAA", ip_addr);
-            if (update_msg == NULL) {
+            if (updateipv6 == NULL) {
                 return NULL;
             }
+
+            break;
         }
     }
 
-    return talloc_asprintf_append(update_msg, "send\n");
+    if (update_per_family && updateipv4[0] && updateipv6[0]) {
+        /* update per family and both families present */
+        return talloc_asprintf_append(update_msg,
+                                            "%s"
+                                            "send\n"
+                                            "%s"
+                                            "send\n",
+                                            updateipv4,
+                                            updateipv6);
+    }
+
+    return talloc_asprintf_append(update_msg,
+                                  "%s"
+                                  "%s"
+                                  "send\n",
+                                  updateipv4,
+                                  updateipv6);
 }
 
 static uint8_t *nsupdate_convert_address(struct sockaddr_storage *add_address)
@@ -355,46 +363,86 @@ static uint8_t *nsupdate_convert_address(struct sockaddr_storage *add_address)
     return addr;
 }
 
-static char *nsupdate_msg_add_ptr(char *update_msg,
-                                  struct sockaddr_storage *address,
-                                  const char *hostname,
-                                  int ttl,
-                                  bool delete)
+static char *
+nsupdate_msg_add_ptr(char *update_msg, struct sss_iface_addr *addresses,
+                     const char *hostname, int ttl, uint8_t remove_af,
+                     bool update_per_family)
 {
-    char *strptr;
+    char *updateipv4 = talloc_strdup(update_msg, "");
+    char *updateipv6 = talloc_strdup(update_msg, "");
+    char *ptr;
+    struct sss_iface_addr *address_it;
     uint8_t *addr;
 
-    addr = nsupdate_convert_address(address);
-    if (addr == NULL) {
+    if (!updateipv4 || !updateipv6) {
         return NULL;
     }
 
-    strptr = resolv_get_string_ptr_address(update_msg, address->ss_family,
-                                           addr);
-    if (strptr == NULL) {
-        return NULL;
-    }
+    DLIST_FOR_EACH(address_it, addresses) {
+        addr = nsupdate_convert_address(address_it->addr);
+        if (addr == 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);
-    } 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);
+        ptr = resolv_get_string_ptr_address(update_msg, address_it->addr->ss_family,
+                                            addr);
+        if (ptr == NULL) {
+            return NULL;
+        }
+
+        switch (address_it->addr->ss_family) {
+        case AF_INET:
+            if (remove_af & DYNDNS_REMOVE_A) {
+                updateipv4 = talloc_asprintf_append(updateipv4,
+                                                    "update delete %s in PTR\n",
+                                                    ptr);
+                if (updateipv4 == NULL) {
+                    return NULL;
+                }
+            }
+
+            updateipv4 = talloc_asprintf_append(updateipv4,
+                                                "update add %s %d in PTR %s.\n",
+                                                ptr, ttl, hostname);
+            break;
+        case AF_INET6:
+            if (remove_af & DYNDNS_REMOVE_AAAA) {
+                updateipv6 = talloc_asprintf_append(updateipv6,
+                                                    "update delete %s in PTR\n",
+                                                    ptr);
+                if (updateipv6 == NULL) {
+                    return NULL;
+                }
+            }
+            updateipv6 = talloc_asprintf_append(updateipv6,
+                                                "update add %s %d in PTR %s.\n",
+                                                ptr, ttl, hostname);
+            break;
+        }
+
+        talloc_free(ptr);
+        if (!updateipv4 || !updateipv6) {
+            return NULL;
+        }
     }
 
-    talloc_free(strptr);
-    if (update_msg == NULL) {
-        return NULL;
+    if (update_per_family && updateipv4[0] && updateipv6[0]) {
+        /* update per family and both families present */
+        return talloc_asprintf_append(update_msg,
+                                      "%s"
+                                      "send\n"
+                                      "%s"
+                                      "send\n",
+                                      updateipv4,
+                                      updateipv6);
     }
 
-    return update_msg;
+    return talloc_asprintf_append(update_msg,
+                                  "%s"
+                                  "%s"
+                                  "send\n",
+                                  updateipv4,
+                                  updateipv6);
 }
 
 static char *
@@ -464,6 +512,7 @@ be_nsupdate_create_fwd_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,
+                           bool update_per_family,
                            char **_update_msg)
 {
     int ret;
@@ -485,7 +534,7 @@ be_nsupdate_create_fwd_msg(TALLOC_CTX *mem_ctx, const char *realm,
     }
 
     update_msg = nsupdate_msg_add_fwd(update_msg, addresses, hostname,
-                                      ttl, remove_af);
+                                      ttl, remove_af, update_per_family);
     if (update_msg == NULL) {
         ret = ENOMEM;
         goto done;
@@ -506,28 +555,32 @@ 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,
-                           struct sockaddr_storage *address,
-                           bool delete,
+                           const char *servername,
+                           const char *hostname, const unsigned int ttl,
+                           uint8_t remove_af, struct sss_iface_addr *addresses,
+                           bool update_per_family,
                            char **_update_msg)
 {
     errno_t ret;
     char *update_msg;
+    TALLOC_CTX *tmp_ctx;
 
     /* in some cases realm could have been NULL if we weren't using TSIG */
     if (hostname == NULL) {
         return EINVAL;
     }
 
-    update_msg = nsupdate_msg_create_common(mem_ctx, realm, servername);
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) return ENOMEM;
+
+    update_msg = nsupdate_msg_create_common(tmp_ctx, realm, servername);
     if (update_msg == NULL) {
         ret = ENOMEM;
         goto done;
     }
 
-    update_msg = nsupdate_msg_add_ptr(update_msg, address, hostname, ttl,
-                                      delete);
+    update_msg = nsupdate_msg_add_ptr(update_msg, addresses, hostname,
+                                      ttl, remove_af, update_per_family);
     if (update_msg == NULL) {
         ret = ENOMEM;
         goto done;
@@ -540,9 +593,10 @@ be_nsupdate_create_ptr_msg(TALLOC_CTX *mem_ctx, const char *realm,
           update_msg);
 
     ret = ERR_OK;
-    *_update_msg = update_msg;
+    *_update_msg = talloc_steal(mem_ctx, update_msg);
 
 done:
+    talloc_free(tmp_ctx);
     return ret;
 }
 
@@ -1196,6 +1250,7 @@ be_nsupdate_check(void)
 
 static struct dp_option default_dyndns_opts[] = {
     { "dyndns_update", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
+    { "dyndns_update_per_family", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
     { "dyndns_refresh_interval", DP_OPT_NUMBER, NULL_NUMBER, NULL_NUMBER },
     { "dyndns_iface", DP_OPT_STRING, NULL_STRING, NULL_STRING },
     { "dyndns_ttl", DP_OPT_NUMBER, { .number = 1200 }, NULL_NUMBER },
diff --git a/src/providers/be_dyndns.h b/src/providers/be_dyndns.h
index 9f39e5d48..48f4597bb 100644
--- a/src/providers/be_dyndns.h
+++ b/src/providers/be_dyndns.h
@@ -49,6 +49,7 @@ struct be_nsupdate_ctx {
 
 enum dp_dyndns_opts {
     DP_OPT_DYNDNS_UPDATE,
+    DP_OPT_DYNDNS_UPDATE_PER_FAMILY,
     DP_OPT_DYNDNS_REFRESH_INTERVAL,
     DP_OPT_DYNDNS_IFACE,
     DP_OPT_DYNDNS_TTL,
@@ -92,14 +93,15 @@ be_nsupdate_create_fwd_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,
+                           bool update_per_family,
                            char **_update_msg);
 
 errno_t
 be_nsupdate_create_ptr_msg(TALLOC_CTX *mem_ctx, const char *realm,
-                           const char *servername, const char *hostname,
-                           const unsigned int ttl,
-                           struct sockaddr_storage *address,
-                           bool delete,
+                           const char *servername,
+                           const char *hostname, const unsigned int ttl,
+                           uint8_t remove_af, struct sss_iface_addr *addresses,
+                           bool update_per_family,
                            char **_update_msg);
 
 /* Returns:
diff --git a/src/providers/ipa/ipa_opts.c b/src/providers/ipa/ipa_opts.c
index c43b21778..3b395150c 100644
--- a/src/providers/ipa/ipa_opts.c
+++ b/src/providers/ipa/ipa_opts.c
@@ -56,6 +56,7 @@ struct dp_option ipa_basic_opts[] = {
 
 struct dp_option ipa_dyndns_opts[] = {
     { "dyndns_update", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
+    { "dyndns_update_per_family", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
     { "dyndns_refresh_interval", DP_OPT_NUMBER, NULL_NUMBER, NULL_NUMBER },
     { "dyndns_iface", DP_OPT_STRING, NULL_STRING, NULL_STRING },
     { "dyndns_ttl", DP_OPT_NUMBER, { .number = 1200 }, NULL_NUMBER },
diff --git a/src/providers/ldap/sdap_dyndns.c b/src/providers/ldap/sdap_dyndns.c
index 20d97ca41..1f7155a3d 100644
--- a/src/providers/ldap/sdap_dyndns.c
+++ b/src/providers/ldap/sdap_dyndns.c
@@ -55,13 +55,12 @@ struct sdap_dyndns_update_state {
     struct sss_iface_addr *dns_addrlist;
     uint8_t remove_af;
 
+    bool update_per_family;
     bool update_ptr;
     bool check_diff;
     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);
@@ -72,12 +71,6 @@ 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);
 
 static bool should_retry(int nsupdate_ret, int child_status)
 {
@@ -113,6 +106,7 @@ sdap_dyndns_update_send(TALLOC_CTX *mem_ctx,
         return NULL;
     }
     state->check_diff = check_diff;
+    state->update_per_family = dp_opt_get_bool(opts, DP_OPT_DYNDNS_UPDATE_PER_FAMILY);
     state->update_ptr = dp_opt_get_bool(opts, DP_OPT_DYNDNS_UPDATE_PTR);
     state->hostname = hostname;
     state->realm = realm;
@@ -123,8 +117,6 @@ 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);
@@ -346,6 +338,7 @@ sdap_dyndns_update_step(struct tevent_req *req)
                                      state->hostname,
                                      state->ttl, state->remove_af,
                                      state->addresses,
+                                     state->update_per_family,
                                      &state->update_msg);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "Can't get addresses for DNS update\n");
@@ -400,15 +393,6 @@ 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);
@@ -417,50 +401,6 @@ 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)
 {
@@ -469,7 +409,6 @@ sdap_dyndns_update_ptr_step(struct tevent_req *req)
     const char *servername;
     const char *realm;
     struct tevent_req *subreq;
-    struct sockaddr_storage *address;
 
     state = tevent_req_data(req, struct sdap_dyndns_update_state);
 
@@ -480,13 +419,11 @@ sdap_dyndns_update_ptr_step(struct tevent_req *req)
         realm = state->realm;
     }
 
-    address = sss_iface_addr_get_address(state->ptr_addr_iter);
-    if (address == NULL) {
-        return EIO;
-    }
-
-    ret = be_nsupdate_create_ptr_msg(state, realm, servername, state->hostname,
-                                     state->ttl, address, state->del_phase,
+    ret = be_nsupdate_create_ptr_msg(state, realm, servername,
+                                     state->hostname,
+                                     state->ttl, state->remove_af,
+                                     state->addresses,
+                                     state->update_per_family,
                                      &state->update_msg);
 
     if (ret != EOK) {
@@ -532,55 +469,13 @@ 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)
 {
diff --git a/src/tests/cmocka/test_dyndns.c b/src/tests/cmocka/test_dyndns.c
index 8888b5371..57180291e 100644
--- a/src/tests/cmocka/test_dyndns.c
+++ b/src/tests/cmocka/test_dyndns.c
@@ -385,7 +385,7 @@ void dyndns_test_create_fwd_msg(void **state)
 
     ret = be_nsupdate_create_fwd_msg(dyndns_test_ctx, NULL, NULL, "bran_stark",
                                      1234, DYNDNS_REMOVE_A | DYNDNS_REMOVE_AAAA,
-                                     addrlist, &msg);
+                                     addrlist, true, &msg);
     assert_int_equal(ret, EOK);
 
     assert_string_equal(msg,
@@ -397,11 +397,24 @@ void dyndns_test_create_fwd_msg(void **state)
                         "send\n");
     talloc_zfree(msg);
 
+    ret = be_nsupdate_create_fwd_msg(dyndns_test_ctx, NULL, NULL, "bran_stark",
+                                     1234, DYNDNS_REMOVE_A | DYNDNS_REMOVE_AAAA,
+                                     addrlist, false, &msg);
+    assert_int_equal(ret, EOK);
+
+    assert_string_equal(msg,
+                        "\nupdate delete bran_stark. in A\n"
+                        "update add bran_stark. 1234 in A 192.168.0.2\n"
+                        "update delete bran_stark. in AAAA\n"
+                        "update add bran_stark. 1234 in AAAA 2001:cdba::555\n"
+                        "send\n");
+    talloc_zfree(msg);
+
     /* fallback case realm and server */
     ret = be_nsupdate_create_fwd_msg(dyndns_test_ctx, "North", "Winterfell",
                                      "bran_stark",
                                      1234, DYNDNS_REMOVE_A | DYNDNS_REMOVE_AAAA,
-                                     addrlist, &msg);
+                                     addrlist, true, &msg);
     assert_int_equal(ret, EOK);
 
     assert_string_equal(msg,
@@ -423,7 +436,7 @@ void dyndns_test_create_fwd_msg(void **state)
     ret = be_nsupdate_create_fwd_msg(dyndns_test_ctx, "North", NULL,
                                      "bran_stark",
                                      1234, DYNDNS_REMOVE_A | DYNDNS_REMOVE_AAAA,
-                                     addrlist, &msg);
+                                     addrlist, true, &msg);
     assert_int_equal(ret, EOK);
 
     assert_string_equal(msg,
@@ -444,7 +457,7 @@ void dyndns_test_create_fwd_msg(void **state)
     ret = be_nsupdate_create_fwd_msg(dyndns_test_ctx, NULL, "Winterfell",
                                      "bran_stark",
                                      1234, DYNDNS_REMOVE_A | DYNDNS_REMOVE_AAAA,
-                                     addrlist, &msg);
+                                     addrlist, true, &msg);
     assert_int_equal(ret, EOK);
 
     assert_string_equal(msg,
@@ -461,7 +474,7 @@ void dyndns_test_create_fwd_msg(void **state)
     /* remove just A */
     ret = be_nsupdate_create_fwd_msg(dyndns_test_ctx, NULL, NULL, "bran_stark",
                                      1234, DYNDNS_REMOVE_A,
-                                     addrlist, &msg);
+                                     addrlist, true, &msg);
     assert_int_equal(ret, EOK);
 
     assert_string_equal(msg,
@@ -475,7 +488,7 @@ void dyndns_test_create_fwd_msg(void **state)
     /* remove just AAAA */
     ret = be_nsupdate_create_fwd_msg(dyndns_test_ctx, NULL, NULL, "bran_stark",
                                      1234, DYNDNS_REMOVE_AAAA,
-                                     addrlist, &msg);
+                                     addrlist, true, &msg);
     assert_int_equal(ret, EOK);
 
     assert_string_equal(msg,
@@ -519,7 +532,7 @@ void dyndns_test_create_fwd_msg_mult(void **state)
 
     ret = be_nsupdate_create_fwd_msg(dyndns_test_ctx, NULL, NULL, "bran_stark",
                                      1234, DYNDNS_REMOVE_A | DYNDNS_REMOVE_AAAA,
-                                     addrlist, &msg);
+                                     addrlist, true, &msg);
     assert_int_equal(ret, EOK);
 
     assert_string_equal(msg,
@@ -564,7 +577,7 @@ void dyndns_test_create_fwd_msg_A(void **state)
 
     ret = be_nsupdate_create_fwd_msg(dyndns_test_ctx, NULL, NULL, "bran_stark",
                                      1234, DYNDNS_REMOVE_A | DYNDNS_REMOVE_AAAA,
-                                     addrlist, &msg);
+                                     addrlist, true, &msg);
     assert_int_equal(ret, EOK);
 
     assert_string_equal(msg,
@@ -608,7 +621,7 @@ void dyndns_test_create_fwd_msg_AAAA(void **state)
 
     ret = be_nsupdate_create_fwd_msg(dyndns_test_ctx, NULL, NULL, "bran_stark",
                                      1234, DYNDNS_REMOVE_A | DYNDNS_REMOVE_AAAA,
-                                     addrlist, &msg);
+                                     addrlist, true, &msg);
     assert_int_equal(ret, EOK);
 
     assert_string_equal(msg,
@@ -624,6 +637,70 @@ void dyndns_test_create_fwd_msg_AAAA(void **state)
     assert_true(check_leaks_pop(dyndns_test_ctx) == true);
 }
 
+void dyndns_test_create_ptr_msg(void **state)
+{
+    errno_t ret;
+    char *msg;
+    struct sss_iface_addr *addrlist;
+    int i;
+
+    check_leaks_push(dyndns_test_ctx);
+
+     /* getifaddrs is called twice in sss_get_dualstack_addresses() */
+    for (i = 0; i < 2; i++) {
+        will_return_getifaddrs("eth0", "192.168.0.2", AF_INET);
+        will_return_getifaddrs("eth0", "192.168.0.1", AF_INET);
+        will_return_getifaddrs("eth0", "2001:cdba::555", AF_INET6);
+        will_return_getifaddrs("eth0", "2001:cdba::444", AF_INET6);
+        will_return_getifaddrs(NULL, NULL, 0); /* sentinel */
+    }
+
+    struct sockaddr_in sin;
+    memset(&sin, 0, sizeof (sin));
+    sin.sin_family = AF_INET;
+    sin.sin_addr.s_addr = inet_addr ("192.168.0.2");
+    ret = sss_get_dualstack_addresses(dyndns_test_ctx,
+                                      (struct sockaddr *) &sin,
+                                      &addrlist);
+    assert_int_equal(ret, EOK);
+
+    ret = be_nsupdate_create_ptr_msg(dyndns_test_ctx, NULL, NULL, "bran_stark",
+                                     1234, DYNDNS_REMOVE_A | DYNDNS_REMOVE_AAAA,
+                                     addrlist, true, &msg);
+    assert_int_equal(ret, EOK);
+    assert_string_equal(msg,
+                        "\nupdate delete 1.0.168.192.in-addr.arpa. in PTR\n"
+                        "update add 1.0.168.192.in-addr.arpa. 1234 in PTR bran_stark.\n"
+                        "update delete 2.0.168.192.in-addr.arpa. in PTR\n"
+                        "update add 2.0.168.192.in-addr.arpa. 1234 in PTR bran_stark.\n"
+                        "send\n"
+                        "update delete 4.4.4.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.a.b.d.c.1.0.0.2.ip6.arpa. in PTR\n"
+                        "update add 4.4.4.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.a.b.d.c.1.0.0.2.ip6.arpa. 1234 in PTR bran_stark.\n"
+                        "update delete 5.5.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.a.b.d.c.1.0.0.2.ip6.arpa. in PTR\n"
+                        "update add 5.5.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.a.b.d.c.1.0.0.2.ip6.arpa. 1234 in PTR bran_stark.\n"
+                        "send\n");
+    talloc_zfree(msg);
+
+    ret = be_nsupdate_create_ptr_msg(dyndns_test_ctx, NULL, NULL, "bran_stark",
+                                     1234, DYNDNS_REMOVE_A | DYNDNS_REMOVE_AAAA,
+                                     addrlist, false, &msg);
+    assert_int_equal(ret, EOK);
+    assert_string_equal(msg,
+                        "\nupdate delete 1.0.168.192.in-addr.arpa. in PTR\n"
+                        "update add 1.0.168.192.in-addr.arpa. 1234 in PTR bran_stark.\n"
+                        "update delete 2.0.168.192.in-addr.arpa. in PTR\n"
+                        "update add 2.0.168.192.in-addr.arpa. 1234 in PTR bran_stark.\n"
+                        "update delete 4.4.4.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.a.b.d.c.1.0.0.2.ip6.arpa. in PTR\n"
+                        "update add 4.4.4.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.a.b.d.c.1.0.0.2.ip6.arpa. 1234 in PTR bran_stark.\n"
+                        "update delete 5.5.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.a.b.d.c.1.0.0.2.ip6.arpa. in PTR\n"
+                        "update add 5.5.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.a.b.d.c.1.0.0.2.ip6.arpa. 1234 in PTR bran_stark.\n"
+                        "send\n");
+    talloc_zfree(msg);
+
+    talloc_free(addrlist);
+    assert_true(check_leaks_pop(dyndns_test_ctx) == true);
+}
+
 void dyndns_test_dualstack(void **state)
 {
     errno_t ret;
@@ -1052,6 +1129,9 @@ int main(int argc, const char *argv[])
         cmocka_unit_test_setup_teardown(dyndns_test_create_fwd_msg_AAAA,
                                         dyndns_test_setup,
                                         dyndns_test_teardown),
+        cmocka_unit_test_setup_teardown(dyndns_test_create_ptr_msg,
+                                        dyndns_test_setup,
+                                        dyndns_test_teardown),
     };
 
     /* Set debug level to invalid value so we can decide if -d 0 was used. */
-- 
2.21.1