Enhanced patch for 23595 to handle IPv4 fixed addresses more

cleanly.

diff --git a/RELNOTES b/RELNOTES
index b05dbb875c56468ea8f0595aeda64445cc1868f5..8234ec765b1be38f0e48c204351c3094a621c98a 100644 (file)
--- a/RELNOTES
+++ b/RELNOTES
@@ -52,6 +52,9 @@ work on other platforms. Please report any problems and suggested fixes to
 - Add support for passing DDNS information to a DNS server over
   an IPv6 address.  [ISC-Bugs #22647]
 
+- Enhanced patch for 23595 to handle IPv4 fixed addresses more
+  cleanly.  [ISC-Bugs #23595]
+
                        Changes since 4.2.1
 
 ! In dhclient check the data for some string options for

diff --git a/includes/dhcpd.h b/includes/dhcpd.h
index ec03b2744f4dff3b89438ab25bdeb2b75a5b8986..a4d81f34b39061b06cc3ea94b0c9df17c5569102 100644 (file)
--- a/includes/dhcpd.h
+++ b/includes/dhcpd.h
@@ -1538,6 +1538,7 @@ struct ipv6_pool {
 #define DDNS_CLIENT_DID_UPDATE  0x10
 #define DDNS_EXECUTE_NEXT       0x20
 #define DDNS_ABORT              0x40
+#define DDNS_STATIC_LEASE       0x80
 
 /*
  * The following two groups are separate and we could reuse

diff --git a/server/ddns.c b/server/ddns.c
index 4149494e0a7f12cb47a2352a69f280e0d8485e87..b003bd0b1f81fbd71ddc0230cb6b95f66880eb08 100644 (file)
--- a/server/ddns.c
+++ b/server/ddns.c
@@ -126,10 +126,17 @@ ddns_updates(struct packet *packet, struct lease *lease, struct lease *old,
        }
        /* assume that we shall update both the A and ptr records */
        ddns_cb->flags = DDNS_UPDATE_ADDR | DDNS_UPDATE_PTR;
-       
+
+       /*
+        * For v4 we flag static leases so we don't try
+        * and manipulate the lease later.  For v6 we don't
+        * get static leases and don't need to flag them.
+        */
        if (lease != NULL) {
                scope = &(lease->scope);
                ddns_cb->address = lease->ip_addr;
+               if (lease->flags & STATIC_LEASE)
+                       ddns_cb->flags |= DDNS_STATIC_LEASE;
        } else if (lease6 != NULL) {
                scope = &(lease6->scope);
                memcpy(ddns_cb->address.iabuf, lease6->addr.s6_addr, 16);
@@ -726,6 +733,13 @@ ddns_update_lease_text(dhcp_ddns_cb_t        *ddns_cb,
        struct in6_addr        addr;
        struct data_string     lease_dhcid;
        
+       /*
+        * If the lease was static (for a fixed address)
+        * we don't need to do any work.
+        */
+       if (ddns_cb->flags & DDNS_STATIC_LEASE)
+               return (ISC_R_SUCCESS);
+
        if (inscope != NULL) {
                scope = inscope;
        } else if (ddns_cb->address.len == 4) {
@@ -839,8 +853,10 @@ update_lease_failed(struct lease *lease,
        sprintf(reason, "unknown");
        sprintf(lease_address, "unknown");
 
-       /* let's pretend that everything is ok, so we can continue for 
-          for information gathering purposes */
+       /*
+        * let's pretend that everything is ok, so we can continue for 
+        * information gathering purposes
+        */
        
        if (ddns_cb != NULL) {
                strncpy(lease_address, piaddr(ddns_cb->address), 
@@ -891,133 +907,138 @@ update_lease_failed(struct lease *lease,
 
 void
 safe_lease_update(struct lease *lease,
-                 struct iasubopt *lease6,
                  dhcp_ddns_cb_t *oldcb,
                  dhcp_ddns_cb_t *newcb,
                  const char *file, int line)
 {
-       char addrbuf[MAX_ADDRESS_STRING_LEN];
+       if (lease == NULL) {
+               /* should never get here */
+               log_fatal("Impossible condition at %s:%d (called from %s:%d).", 
+                         MDL, file, line);
+       }
 
-       if (lease != NULL) {
-               if ( (lease->ddns_cb == NULL) && (newcb == NULL) ) {
-                       /*
-                        * Trying to clean up pointer that is already null. We
-                        * are most likely trying to update wrong lease here.
-                        */
+       if ( (lease->ddns_cb == NULL) && (newcb == NULL) ) {
+               /*
+                * Trying to clean up pointer that is already null. We
+                * are most likely trying to update wrong lease here.
+                */
 
-                       /* 
-                        * this error message pops out during every DNS Update
-                        * for fixed leases. It is enabled only for debugging
-                        * DNS Updates. Investigation pending.
-                        */
-#if defined (DEBUG_DNS_UPDATES)
-                       log_error("%s(%d): Invalid lease update. Tried to "
-                                 "clear already NULL DDNS control block "
-                                 "pointer for lease %s.",
-                                 file, line, piaddr(lease->ip_addr) );
-#endif
+               /* 
+                * Previously this error message popped out during
+                * DNS update for fixed leases.  As we no longer
+                * try to update the lease for a fixed (static) lease
+                * this should not be a problem.
+                */
+               log_error("%s(%d): Invalid lease update. Tried to "
+                         "clear already NULL DDNS control block "
+                         "pointer for lease %s.",
+                         file, line, piaddr(lease->ip_addr) );
 
 #if defined (DNS_UPDATES_MEMORY_CHECKS)
-                       update_lease_failed(lease, NULL, oldcb, newcb, file,
-                                           line);
+               update_lease_failed(lease, NULL, oldcb, newcb, file, line);
 #endif
-                       /* 
-                        * May not reach this: update_lease_failed calls 
-                        * log_fatal.
-                        */ 
-                       return;
-               }
+               /* 
+                * May not reach this: update_lease_failed calls 
+                * log_fatal.
+                */ 
+               return;
+       }
 
-               if ( (lease->ddns_cb != NULL) && (lease->ddns_cb != oldcb) ) {
-                       /* 
-                        * There is existing cb structure, but it differs from
-                        * what we expected to see there. Most likely we are 
-                        * trying to update wrong lease. 
-                        */
-                       log_error("%s(%d): Failed to update internal lease "
-                                 "structure with DDNS control block. Existing"
-                                 " ddns_cb structure does not match "
-                                 "expectations.IPv4=%s, old ddns_cb=%p, tried"
-                                 "to update to new ddns_cb=%p", file, line,
-                                 piaddr(lease->ip_addr), oldcb,  newcb);
+       if ( (lease->ddns_cb != NULL) && (lease->ddns_cb != oldcb) ) {
+               /* 
+                * There is existing cb structure, but it differs from
+                * what we expected to see there. Most likely we are 
+                * trying to update wrong lease. 
+                */
+               log_error("%s(%d): Failed to update internal lease "
+                         "structure with DDNS control block. Existing"
+                         " ddns_cb structure does not match "
+                         "expectations.IPv4=%s, old ddns_cb=%p, tried"
+                         "to update to new ddns_cb=%p", file, line,
+                         piaddr(lease->ip_addr), oldcb,  newcb);
 
 #if defined (DNS_UPDATES_MEMORY_CHECKS)
-                       update_lease_failed(lease, NULL, oldcb, newcb, file,
-                                           line);
+               update_lease_failed(lease, NULL, oldcb, newcb, file, line);
 #endif
-                       /* 
-                        * May not reach this: update_lease_failed calls 
-                        * log_fatal.
-                        */ 
-                       return; 
-               }
-               /* additional IPv4 specific checks may be added here */
+               /* 
+                * May not reach this: update_lease_failed calls 
+                * log_fatal.
+                */ 
+               return; 
+       }
 
-               /* update the lease */
-               lease->ddns_cb = newcb;
-       } else if (lease6 != NULL) {
-               if ( (lease6->ddns_cb == NULL) && (newcb == NULL) ) {
-                       inet_ntop(AF_INET6, &lease6->addr, addrbuf, 
-                                 MAX_ADDRESS_STRING_LEN);
-                       /*
-                        * Trying to clean up pointer that is already null. We
-                        * are most likely trying to update wrong lease here.
-                        */
-#if defined (DEBUG_DNS_UPDATES)
-                       log_error("%s(%d): Failed to update internal lease "
-                                 "structure. Tried to clear already NULL "
-                                 "DDNS control block pointer for lease %s.",
-                                 file, line, addrbuf);
-#endif
+       /* additional IPv4 specific checks may be added here */
+
+       /* update the lease */
+       lease->ddns_cb = newcb;
+}
+
+void
+safe_lease6_update(struct iasubopt *lease6,
+                  dhcp_ddns_cb_t *oldcb,
+                  dhcp_ddns_cb_t *newcb,
+                  const char *file, int line)
+{
+       char addrbuf[MAX_ADDRESS_STRING_LEN];
+
+       if (lease6 == NULL) {
+               /* should never get here */
+               log_fatal("Impossible condition at %s:%d (called from %s:%d).", 
+                         MDL, file, line);
+       }
+
+       if ( (lease6->ddns_cb == NULL) && (newcb == NULL) ) {
+               inet_ntop(AF_INET6, &lease6->addr, addrbuf, 
+                         MAX_ADDRESS_STRING_LEN);
+               /*
+                * Trying to clean up pointer that is already null. We
+                * are most likely trying to update wrong lease here.
+                */
+               log_error("%s(%d): Failed to update internal lease "
+                         "structure. Tried to clear already NULL "
+                         "DDNS control block pointer for lease %s.",
+                         file, line, addrbuf);
 
 #if defined (DNS_UPDATES_MEMORY_CHECKS)
-                       update_lease_failed(NULL, lease6, oldcb, newcb, file,
-                                           line);
+               update_lease_failed(NULL, lease6, oldcb, newcb, file, line);
 #endif
 
-                       /* 
-                        * May not reach this: update_lease_failed calls 
-                        * log_fatal.
-                        */ 
-                       return; 
-               }
+               /* 
+                * May not reach this: update_lease_failed calls 
+                * log_fatal.
+                */ 
+               return; 
+       }
 
-               if ( (lease6->ddns_cb != NULL) && (lease6->ddns_cb != oldcb) ) {
-                       /* 
-                        * there is existing cb structure, but it differs from
-                        * what we expected to see there. Most likely we are 
-                        * trying to update wrong lease. 
-                        */
-                       inet_ntop(AF_INET6, &lease6->addr, addrbuf, 
-                                 MAX_ADDRESS_STRING_LEN);
+       if ( (lease6->ddns_cb != NULL) && (lease6->ddns_cb != oldcb) ) {
+               /* 
+                * there is existing cb structure, but it differs from
+                * what we expected to see there. Most likely we are 
+                * trying to update wrong lease. 
+                */
+               inet_ntop(AF_INET6, &lease6->addr, addrbuf, 
+                         MAX_ADDRESS_STRING_LEN);
 
-                       log_error("%s(%d): Failed to update internal lease "
-                                 "structure with DDNS control block. Existing"
-                                 " ddns_cb structure does not match "
-                                 "expectations.IPv6=%s, old ddns_cb=%p, tried"
-                                 "to update to new ddns_cb=%p", file, line,
-                                 addrbuf, oldcb,  newcb);
+               log_error("%s(%d): Failed to update internal lease "
+                         "structure with DDNS control block. Existing"
+                         " ddns_cb structure does not match "
+                         "expectations.IPv6=%s, old ddns_cb=%p, tried"
+                         "to update to new ddns_cb=%p", file, line,
+                         addrbuf, oldcb,  newcb);
 
 #if defined (DNS_UPDATES_MEMORY_CHECKS)
-                       update_lease_failed(NULL, lease6, oldcb, newcb, file, 
-                                           line);
+               update_lease_failed(NULL, lease6, oldcb, newcb, file, line);
 #endif
-                       /* 
-                        * May not reach this: update_lease_failed calls 
-                        * log_fatal.
-                        */ 
-                       return;
-               }
-               /* additional IPv6 specific checks may be added here */
-       
-               /* update the lease */
-               lease6->ddns_cb = newcb;
-
-       } else {
-               /* should never get here */
-               log_fatal("Impossible condition at %s:%d (called from %s:%d).", 
-                         MDL, file, line);
+               /* 
+                * May not reach this: update_lease_failed calls 
+                * log_fatal.
+                */ 
+               return;
        }
+       /* additional IPv6 specific checks may be added here */
+       
+       /* update the lease */
+       lease6->ddns_cb = newcb;
 }
 
 /*
@@ -1043,7 +1064,7 @@ ddns_update_lease_ptr(struct lease    *lease,
                      const char * file, int line)
 {
        char ddns_address[MAX_ADDRESS_STRING_LEN];
-       sprintf(ddns_address, "uknown");
+       sprintf(ddns_address, "unknown");
        if (ddns_cb) {
                strncpy(ddns_address, piaddr(ddns_cb->address), 
                        MAX_ADDRESS_STRING_LEN);
@@ -1053,14 +1074,23 @@ ddns_update_lease_ptr(struct lease    *lease,
                 file, line, ddns_cb, ddns_address );
 #endif
 
+       /*
+        * If the lease was static (for a fixed address)
+        * we don't need to do any work.
+        */
+       if (ddns_cb->flags & DDNS_STATIC_LEASE) {
+#if defined (DEBUG_DNS_UPDATES)
+               log_info("lease is static, returning");
+#endif
+               return (ISC_R_SUCCESS);
+       }
+
        if (lease != NULL) {
-               safe_lease_update(lease, NULL, ddns_cb, ddns_cb_set, 
+               safe_lease_update(lease, ddns_cb, ddns_cb_set, 
                                  file, line);
-               /* lease->ddns_cb = ddns_cb_set; */
        } else if (lease6 != NULL) {
-               safe_lease_update(NULL, lease6, ddns_cb, ddns_cb_set, 
+               safe_lease6_update(lease6, ddns_cb, ddns_cb_set, 
                                  file, line);
-               /* lease6->ddns_cb = ddns_cb_set; */
        } else if (ddns_cb->address.len == 4) {
                struct lease *find_lease = NULL;
                if (find_lease_by_ip_addr(&find_lease,
@@ -1071,17 +1101,14 @@ ddns_update_lease_ptr(struct lease    *lease,
                                 find_lease);
 #endif
                  
-                       /* find_lease->ddns_cb = ddns_cb_set; */
-                       safe_lease_update(find_lease, NULL, ddns_cb,
+                       safe_lease_update(find_lease, ddns_cb,
                                          ddns_cb_set, file, line);
                        lease_dereference(&find_lease, MDL);
                }
                else {
-#if defined (DEBUG_DNS_UPDATES)
                        log_error("%s(%d): ddns_update_lease_ptr failed. "
-                                 "Lease for %s not found. (Is it static lease?)",
+                                 "Lease for %s not found.",
                                  file, line, piaddr(ddns_cb->address));
-#endif
 
 #if defined (DNS_UPDATES_MEMORY_CHECKS)
                        update_lease_failed(NULL, NULL, ddns_cb, ddns_cb_set,
@@ -1120,7 +1147,7 @@ ddns_update_lease_ptr(struct lease    *lease,
                        return(ISC_R_FAILURE);
                }
 
-               if (iasubopt_hash_lookup(&find_lease6,  pool->leases,
+               if (iasubopt_hash_lookup(&find_lease6, pool->leases,
                                         &addr, 16, MDL)) {
                        find_lease6->ddns_cb = ddns_cb_set;
                        iasubopt_dereference(&find_lease6, MDL);
@@ -1599,10 +1626,17 @@ ddns_removals(struct lease    *lease,
        if (ddns_cb == NULL) {
                goto cleanup;
        }
-       
+
+       /*
+        * For v4 we flag static leases so we don't try
+        * and manipulate the lease later.  For v6 we don't
+        * get static leases and don't need to flag them.
+        */
        if (lease != NULL) {
                scope = &(lease->scope);
                ddns_cb->address = lease->ip_addr;
+               if (lease->flags & STATIC_LEASE)
+                       ddns_cb->flags |= DDNS_STATIC_LEASE;
        } else if (lease6 != NULL) {
                scope = &(lease6->scope);
                memcpy(&ddns_cb->address.iabuf, lease6->addr.s6_addr, 16);
@@ -1618,7 +1652,7 @@ ddns_removals(struct lease    *lease,
                goto cleanup;
 
        /* Assume that we are removing both records */
-       ddns_cb->flags = DDNS_UPDATE_ADDR | DDNS_UPDATE_PTR;
+       ddns_cb->flags |= DDNS_UPDATE_ADDR | DDNS_UPDATE_PTR;
 
        /* and that we want to do the add call */
        execute_add = ISC_R_SUCCESS;