Hello Jakub,
thanks for the comments, I have updated first 4th patches as you
proposed, but I need some clarification of the last patch, please see
comments inline:
This relates to the 3rd patch.
I think we can just remove this note. If you like, we can say
something like "Choose the interface or a list of interfaces...".
</para>
<para>
Default: Use the IP address of the AD LDAP
connection
</para>
+ <para>
+ Example: dyndns_iface = em1, vnet1, vnet2
+ </para>
</listitem>
</varlistentry>
Please note that in attached patches I changed the default.
From 8680c9df4accf269bdc9d2cb6c3440cbb0dc8405 Mon Sep 17 00:00:00 2001
From: Pavel Reichl<[email protected]>
Date: Tue, 14 Jul 2015 09:56:59 -0400
Subject: [PATCH 5/5] DYNDNS: support for dualstack
When dyndns_iface option was not used, address of connection to LDAP
was used. This patch proposes following change:
* Interface containing address of connection is found.
* All A and AAAA addresses of this interface are collected.
* Collected addresses are sent during DDNS update.
* Function sss_iface_addr_add() is removed.
Resolves:
https://fedorahosted.org/sssd/ticket/2558
---
src/providers/dp_dyndns.c | 150 +++++++++++++++++++++++++++------
src/providers/dp_dyndns.h | 8 +-
src/providers/ldap/sdap_dyndns.c | 20 ++---
src/tests/cmocka/test_dyndns.c | 178 +++++++++++++++++++++++++++++++++++++++
4 files changed, 317 insertions(+), 39 deletions(-)
diff --git a/src/providers/dp_dyndns.c b/src/providers/dp_dyndns.c
index
a1533387c1fc980713fbc01c25efba2fbaed4a43..4310a7c6c74787eada934e56380375611801245b
100644
--- a/src/providers/dp_dyndns.c
+++ b/src/providers/dp_dyndns.c
@@ -55,31 +55,6 @@ void sss_iface_addr_concatenate(struct sss_iface_addr **list,
DLIST_CONCATENATE((*list), list2, struct sss_iface_addr*);
}
-struct sss_iface_addr *
-sss_iface_addr_add(TALLOC_CTX *mem_ctx, struct sss_iface_addr **list,
- struct sockaddr_storage *ss)
-{
- struct sss_iface_addr *address;
-
- address = talloc(mem_ctx, struct sss_iface_addr);
- if (address == NULL) {
- return NULL;
- }
-
- address->addr = talloc_memdup(address, ss,
- sizeof(struct sockaddr_storage));
- if(address->addr == NULL) {
- talloc_zfree(address);
- return NULL;
- }
-
- /* steal old dlist to the new head */
- talloc_steal(address, *list);
- DLIST_ADD(*list, address);
-
- return address;
-}
-
errno_t
sss_iface_addr_list_as_str_list(TALLOC_CTX *mem_ctx,
struct sss_iface_addr *ifaddr_list,
@@ -1252,3 +1227,128 @@ errno_t be_nsupdate_init_timer(struct be_nsupdate_ctx
*ctx,
return ERR_OK;
}
+
+static errno_t match_ip(const struct sockaddr *sa,
+ const struct sockaddr *sb,
+ bool *_res)
Don't you think it would be simpler to just return bool? For cases where
Yes, it would be simpler, but I prefer to know that address is from
family that is not supported by the function. But I can change it if you
still prefer that.
the address family is totally different, can you just return false?
I actually think this is how function works (at least for address
families known to function)
+{
+ size_t addrsize;
+ bool res;
+ errno_t ret;
+ const void *addr_a;
+ const void *addr_b;
+
+ if (sa->sa_family == AF_INET) {
+ addrsize = sizeof(struct in_addr);
+ addr_a = (const void *) &((const struct sockaddr_in *) sa)->sin_addr;
+ addr_b = (const void *) &((const struct sockaddr_in *) sb)->sin_addr;
+ } else if (sa->sa_family == AF_INET6) {
+ addrsize = sizeof(struct in6_addr);
+ addr_a = (const void *) &((const struct sockaddr_in6 *) sa)->sin6_addr;
+ addr_b = (const void *) &((const struct sockaddr_in6 *) sb)->sin6_addr;
Hmm, I was surprised I couldn't find any existing function or macro to
compare a sockaddr or a sockaddr_storage except IN6_ARE_ADDR_EQUAL..
+ } else {
+ ret = EINVAL;
+ goto done;
unsupported address family
+ }
+
+ if (sa->sa_family != sb->sa_family) {
+ ret = EOK;
+ res = false;
+ goto done;
+ }
For different families I return false.
+
+ res = memcmp(addr_a, addr_b, addrsize) == 0;
+ ret = EOK;
+
+done:
+ if (ret == EOK) {
+ *_res = res;
+ }
I think a more idiomatic way is to call this assignment to output
pointer along with setting ret = EOK.
Yes, it is. The reason I don't follow this idiomatic way here is the
fact that "ret = EOK" is written at 2 places in the function and I don't
want to duplicate code. But it would be quite minimal change here. Still
I personally prefer it the way it is now. You decide :-).
+ return ret;
+}
+
+static errno_t find_iface_by_addr(TALLOC_CTX *mem_ctx,
+ const struct sockaddr *ss,
+ const char **_iface_name)
+{
+ struct ifaddrs *ifaces = NULL;
+ struct ifaddrs *ifa;
+ errno_t ret;
+
+ ret = getifaddrs(&ifaces);
+ if (ret == -1) {
+ ret = errno;
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Could not read interfaces [%d][%s]\n", ret, sss_strerror(ret));
+ goto done;
+ }
+
+ for (ifa = ifaces; ifa != NULL; ifa = ifa->ifa_next) {
+ bool ip_matched;
+
+ /* Some interfaces don't have an ifa_addr */
+ if (!ifa->ifa_addr) continue;
+
+ ret = match_ip(ss, ifa->ifa_addr, &ip_matched);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, "match_ip failed: %d:[%s]\n",
+ ret, sss_strerror(ret));
+ goto done;
+ }
+ if (ip_matched) {
+ const char *iface_name;
Please put a blank line here. Actually, I'm not thrilled to have the EOK
branch in the middle of the function. Normally, you want to outmost
branch to be the success one...here it's ENOENT..
I'm sorry I fail to see any more elegant way how to rewrite the function.
+ iface_name = talloc_strdup(mem_ctx, ifa->ifa_name);
+ if (iface_name == NULL) {
+ ret = ENOMEM;
+ } else {
+ *_iface_name = iface_name;
+ ret = EOK;
I can add break here
+ }
+ goto done;
+ }
+ }
+ ret = ENOENT;
and test here "iface_name != NUL => EOK", if you think this will improve
the readability of the function.
+
+done:
+ freeifaddrs(ifaces);
+ return ret;
+}
+
+errno_t sss_get_dualstack_addresses(TALLOC_CTX *mem_ctx,
+ struct sockaddr *ss,
+ struct sss_iface_addr **_iface_addrs)
+{
+ struct sss_iface_addr *iface_addrs;
+ const char *iface_name = NULL;
+ TALLOC_CTX *tmp_ctx;
+ errno_t ret;
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ ret = find_iface_by_addr(tmp_ctx, ss, &iface_name);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE, "find_iface_by_addr failed: %d:[%s]\n",
+ ret, sss_strerror(ret));
+ goto done;
+ }
+
+ ret = sss_iface_addr_list_get(tmp_ctx, iface_name, &iface_addrs);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "sss_iface_addr_list_get failed: %d:[%s]\n",
+ ret, sss_strerror(ret));
+ goto done;
+ }
+
+done:
+ if (ret == EOK) {
+ *_iface_addrs = talloc_steal(mem_ctx, iface_addrs);
Again, it would be more idiomatic and consistent with the rest of the
code to write:
ret = EOK
*_iface_addrs = <assign>
done:
OK, I'll fix it.
+ }
+
+ talloc_free(tmp_ctx);
+ return ret;
+}
The rest looks OK to me. I've started Coverity scan.
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>From 166f56fb9d749c5ee72d61d8e2e5f1c9c6dafb5b Mon Sep 17 00:00:00 2001
From: Pavel Reichl <[email protected]>
Date: Wed, 8 Jul 2015 09:01:24 -0400
Subject: [PATCH 1/5] DYNDNS: sss_iface_addr_list_get return ENOENT
If none of eligible interfaces matches ifname then ENOENT is returned.
Resolves:
https://fedorahosted.org/sssd/ticket/2549
---
src/providers/dp_dyndns.c | 13 +++++++++++--
src/providers/ldap/sdap_dyndns.c | 6 +++++-
src/tests/cmocka/test_dyndns.c | 20 ++++++++++++++++++++
3 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/src/providers/dp_dyndns.c b/src/providers/dp_dyndns.c
index 1cac3d0fae2454cea823ed640a4325f27580353f..2ac43a108ff6197d9e2662198a6da976ca348e76 100644
--- a/src/providers/dp_dyndns.c
+++ b/src/providers/dp_dyndns.c
@@ -222,8 +222,17 @@ sss_iface_addr_list_get(TALLOC_CTX *mem_ctx, const char *ifname,
}
}
- ret = EOK;
- *_addrlist = addrlist;
+ if (addrlist != NULL) {
+ /* OK, some result was found */
+ ret = EOK;
+ *_addrlist = addrlist;
+ } else {
+ /* No result was found */
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "No IPs usable for DNS was found for interface: %s.\n", ifname);
+ ret = ENOENT;
+ }
+
done:
freeifaddrs(ifaces);
return ret;
diff --git a/src/providers/ldap/sdap_dyndns.c b/src/providers/ldap/sdap_dyndns.c
index 0d9c9205792062378aa25aad6ac706058001433a..e99a4f6687035928f6775c38b9df6b2a06d38f38 100644
--- a/src/providers/ldap/sdap_dyndns.c
+++ b/src/providers/ldap/sdap_dyndns.c
@@ -502,8 +502,12 @@ sdap_dyndns_get_addrs_send(TALLOC_CTX *mem_ctx,
if (iface) {
ret = sss_iface_addr_list_get(state, iface, &state->addresses);
if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE,
+ DEBUG(ret == ENOENT ? SSSDBG_MINOR_FAILURE : SSSDBG_OP_FAILURE,
"Cannot get list of addresses from interface %s\n", iface);
+ /* non critical failure */
+ if (ret == ENOENT) {
+ ret = EOK;
+ }
}
/* We're done. Just fake an async request completion */
goto done;
diff --git a/src/tests/cmocka/test_dyndns.c b/src/tests/cmocka/test_dyndns.c
index 689e333d4e68c4a2582894d06b8b7b20c76b9be8..3214e90c063ea9a4cf6f6bc6507bf4e37b7d23a4 100644
--- a/src/tests/cmocka/test_dyndns.c
+++ b/src/tests/cmocka/test_dyndns.c
@@ -247,6 +247,23 @@ void dyndns_test_get_multi_ifaddr(void **state)
assert_true(check_leaks_pop(dyndns_test_ctx) == true);
}
+void dyndns_test_get_ifaddr_enoent(void **state)
+{
+ errno_t ret;
+ struct sss_iface_addr *addrlist = NULL;
+
+ check_leaks_push(dyndns_test_ctx);
+ will_return_getifaddrs("eth0", "192.168.0.1");
+ will_return_getifaddrs("eth1", "192.168.0.2");
+ will_return_getifaddrs(NULL, NULL); /* sentinel */
+ ret = sss_iface_addr_list_get(dyndns_test_ctx, "non_existing_interface",
+ &addrlist);
+ assert_int_equal(ret, ENOENT);
+ talloc_free(addrlist);
+
+ assert_true(check_leaks_pop(dyndns_test_ctx) == true);
+}
+
void dyndns_test_ok(void **state)
{
struct tevent_req *req;
@@ -460,6 +477,9 @@ int main(int argc, const char *argv[])
cmocka_unit_test_setup_teardown(dyndns_test_get_multi_ifaddr,
dyndns_test_simple_setup,
dyndns_test_teardown),
+ cmocka_unit_test_setup_teardown(dyndns_test_get_ifaddr_enoent,
+ dyndns_test_simple_setup,
+ dyndns_test_teardown),
/* Dynamic DNS update unit tests*/
cmocka_unit_test_setup_teardown(dyndns_test_ok,
--
2.4.3
>From 18353c25b99674a524e4a459a710e091d6a68cc1 Mon Sep 17 00:00:00 2001
From: Pavel Reichl <[email protected]>
Date: Wed, 8 Jul 2015 09:08:03 -0400
Subject: [PATCH 2/5] DYNDNS: support mult. interfaces for dyndns_iface opt
Resolves:
https://fedorahosted.org/sssd/ticket/2549
---
src/man/sssd-ad.5.xml | 11 +++---
src/man/sssd-ipa.5.xml | 10 ++++--
src/providers/dp_dyndns.c | 6 ++++
src/providers/dp_dyndns.h | 4 +++
src/providers/ldap/sdap_dyndns.c | 72 +++++++++++++++++++++++++++++++++++-----
5 files changed, 87 insertions(+), 16 deletions(-)
diff --git a/src/man/sssd-ad.5.xml b/src/man/sssd-ad.5.xml
index 938a443e027b9bf83c75c240a7d6b2a0876b92c8..ff43ea37066514a87934d07b141e680416dcc05b 100644
--- a/src/man/sssd-ad.5.xml
+++ b/src/man/sssd-ad.5.xml
@@ -754,15 +754,16 @@ ad_gpo_map_deny = +my_pam_service
<listitem>
<para>
Optional. Applicable only when dyndns_update
- is true. Choose the interface whose IP address
- should be used for dynamic DNS updates.
- </para>
- <para>
- NOTE: This option currently supports only one interface.
+ is true. Choose the interface or a list of interfaces
+ whose IP addresses should be used for dynamic DNS
+ updates.
</para>
<para>
Default: Use the IP address of the AD LDAP connection
</para>
+ <para>
+ Example: dyndns_iface = em1, vnet1, vnet2
+ </para>
</listitem>
</varlistentry>
diff --git a/src/man/sssd-ipa.5.xml b/src/man/sssd-ipa.5.xml
index 0716b6235f93965170983856b930799bfded6258..d450c2fadbb1713096ff766bf536702195cfd137 100644
--- a/src/man/sssd-ipa.5.xml
+++ b/src/man/sssd-ipa.5.xml
@@ -166,11 +166,12 @@
<listitem>
<para>
Optional. Applicable only when dyndns_update
- is true. Choose the interface whose IP address
- should be used for dynamic DNS updates.
+ is true. Choose the interface or a list of interfaces
+ whose IP addresses should be used for dynamic DNS
+ updates.
</para>
<para>
- NOTE: This option currently supports only one interface.
+ NOTE: This option currently supports multiple interfaces.
</para>
<para>
NOTE: While it is still possible to use the old
@@ -181,6 +182,9 @@
<para>
Default: Use the IP address of the IPA LDAP connection
</para>
+ <para>
+ Example: dyndns_iface = em1, vnet1, vnet2
+ </para>
</listitem>
</varlistentry>
diff --git a/src/providers/dp_dyndns.c b/src/providers/dp_dyndns.c
index 2ac43a108ff6197d9e2662198a6da976ca348e76..76562840ef1d427629e41617b871caaedab779d4 100644
--- a/src/providers/dp_dyndns.c
+++ b/src/providers/dp_dyndns.c
@@ -49,6 +49,12 @@ struct sss_iface_addr {
struct sockaddr_storage *addr;
};
+void sss_iface_addr_concatenate(struct sss_iface_addr **list,
+ struct sss_iface_addr *list2)
+{
+ DLIST_CONCATENATE((*list), list2, struct sss_iface_addr*);
+}
+
struct sss_iface_addr *
sss_iface_addr_add(TALLOC_CTX *mem_ctx, struct sss_iface_addr **list,
struct sockaddr_storage *ss)
diff --git a/src/providers/dp_dyndns.h b/src/providers/dp_dyndns.h
index 23b833dace58a0ecbb1e2e21963a55186f1d06a8..deba112538ad22cd7f59be07934778ee9d4361e7 100644
--- a/src/providers/dp_dyndns.h
+++ b/src/providers/dp_dyndns.h
@@ -128,4 +128,8 @@ nsupdate_get_addrs_recv(struct tevent_req *req,
struct sss_iface_addr **_addrlist,
size_t *_count);
+void
+sss_iface_addr_concatenate(struct sss_iface_addr **list,
+ struct sss_iface_addr *list2);
+
#endif /* DP_DYNDNS_H_ */
diff --git a/src/providers/ldap/sdap_dyndns.c b/src/providers/ldap/sdap_dyndns.c
index e99a4f6687035928f6775c38b9df6b2a06d38f38..3b621e340121ca972c47f9a462923cee7219a818 100644
--- a/src/providers/ldap/sdap_dyndns.c
+++ b/src/providers/ldap/sdap_dyndns.c
@@ -482,6 +482,65 @@ static void sdap_dyndns_get_addrs_done(struct tevent_req *subreq);
static errno_t sdap_dyndns_add_ldap_conn(struct sdap_dyndns_get_addrs_state *state,
struct sdap_handle *sh);
+static errno_t get_ifaces_addrs(TALLOC_CTX *mem_ctx,
+ const char *iface,
+ struct sss_iface_addr **_result)
+{
+ struct sss_iface_addr *result_addrs = NULL;
+ struct sss_iface_addr *intf_addrs;
+ TALLOC_CTX *tmp_ctx;
+ char **list_of_intfs;
+ int num_of_intfs;
+ errno_t ret;
+ int i;
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ ret = ENONEN;
+ goto done;
+ }
+
+ ret = split_on_separator(tmp_ctx, iface, ',', true, true, &list_of_intfs,
+ &num_of_intfs);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "Parsing names of interfaces failed - %d:[%s].\n",
+ ret, sss_strerror(ret));
+ goto done;
+ }
+
+ for (i = 0; i < num_of_intfs; i++) {
+ ret = sss_iface_addr_list_get(tmp_ctx, list_of_intfs[i], &intf_addrs);
+ if (ret == EOK) {
+ if (result_addrs != NULL) {
+ /* If there is already an existing list, head of this existing
+ * list will be considered as parent talloc context for the
+ * new list.
+ */
+ talloc_steal(result_addrs, intf_addrs);
+ }
+ sss_iface_addr_concatenate(&result_addrs, intf_addrs);
+ } else if (ret == ENOENT) {
+ /* non-critical failure */
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "Cannot get interface %s or there are no addresses "
+ "bind to it.\n", list_of_intfs[i]);
+ } else {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Cannot get list of addresses from interface %s - %d:[%s]\n",
+ list_of_intfs[i], ret, sss_strerror(ret));
+ goto done;
+ }
+ }
+
+ ret = EOK;
+ *_result = talloc_steal(mem_ctx, result_addrs);
+
+done:
+ talloc_free(tmp_ctx);
+ return ret;
+}
+
static struct tevent_req *
sdap_dyndns_get_addrs_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
@@ -500,14 +559,11 @@ sdap_dyndns_get_addrs_send(TALLOC_CTX *mem_ctx,
}
if (iface) {
- ret = sss_iface_addr_list_get(state, iface, &state->addresses);
- if (ret != EOK) {
- DEBUG(ret == ENOENT ? SSSDBG_MINOR_FAILURE : SSSDBG_OP_FAILURE,
- "Cannot get list of addresses from interface %s\n", iface);
- /* non critical failure */
- if (ret == ENOENT) {
- ret = EOK;
- }
+ ret = get_ifaces_addrs(state, iface, &state->addresses);
+ if (ret != EOK || state->addresses == NULL) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "get_ifaces_addrs() failed: %d:[%s]\n",
+ ret, sss_strerror(ret));
}
/* We're done. Just fake an async request completion */
goto done;
--
2.4.3
>From 1def75b238ac8c9da397a14e074e30a50eaf4606 Mon Sep 17 00:00:00 2001
From: Pavel Reichl <[email protected]>
Date: Tue, 14 Jul 2015 04:21:34 -0400
Subject: [PATCH 3/5] DYNDNS: special value '*' for dyndns_iface option
Option dyndns_iface has now special value '*' which implies that IPs
from add interfaces should be sent during DDNS update.
---
src/man/sssd-ad.5.xml | 6 ++++--
src/man/sssd-ipa.5.xml | 9 ++++-----
src/providers/dp_dyndns.c | 20 ++++++++++++++++----
src/providers/ldap/sdap_dyndns.c | 2 +-
4 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/src/man/sssd-ad.5.xml b/src/man/sssd-ad.5.xml
index ff43ea37066514a87934d07b141e680416dcc05b..3cbc10520098372d984d00425d03832d002d6672 100644
--- a/src/man/sssd-ad.5.xml
+++ b/src/man/sssd-ad.5.xml
@@ -756,10 +756,12 @@ ad_gpo_map_deny = +my_pam_service
Optional. Applicable only when dyndns_update
is true. Choose the interface or a list of interfaces
whose IP addresses should be used for dynamic DNS
- updates.
+ updates. Special value <quote>*</quote> implies that
+ IPs from all interfaces should be used.
</para>
<para>
- Default: Use the IP address of the AD LDAP connection
+ Default: Use the IP addresses of the interface which
+ is used for AD LDAP connection
</para>
<para>
Example: dyndns_iface = em1, vnet1, vnet2
diff --git a/src/man/sssd-ipa.5.xml b/src/man/sssd-ipa.5.xml
index d450c2fadbb1713096ff766bf536702195cfd137..2e985991fde10827aff0e7c8e67f29a009683450 100644
--- a/src/man/sssd-ipa.5.xml
+++ b/src/man/sssd-ipa.5.xml
@@ -168,10 +168,8 @@
Optional. Applicable only when dyndns_update
is true. Choose the interface or a list of interfaces
whose IP addresses should be used for dynamic DNS
- updates.
- </para>
- <para>
- NOTE: This option currently supports multiple interfaces.
+ updates. Special value <quote>*</quote> implies that
+ IPs from all interfaces should be used.
</para>
<para>
NOTE: While it is still possible to use the old
@@ -180,7 +178,8 @@
in their config file.
</para>
<para>
- Default: Use the IP address of the IPA LDAP connection
+ Default: Use the IP addresses of the interface which
+ is used for IPA LDAP connection
</para>
<para>
Example: dyndns_iface = em1, vnet1, vnet2
diff --git a/src/providers/dp_dyndns.c b/src/providers/dp_dyndns.c
index 76562840ef1d427629e41617b871caaedab779d4..934d34c333857db21dcecdc97187fe2eb49e4d92 100644
--- a/src/providers/dp_dyndns.c
+++ b/src/providers/dp_dyndns.c
@@ -171,6 +171,19 @@ ok_for_dns(struct sockaddr *sa)
return true;
}
+static bool supported_address_family(sa_family_t sa_family)
+{
+ return sa_family == AF_INET || sa_family == AF_INET6;
+}
+
+/* MASK represents special value for matching all interfaces */
+#define MASK "*"
+
+static bool matching_name(const char *ifname, const char *ifname2)
+{
+ return (strcmp(MASK, ifname) == 0) || (strcasecmp(ifname, ifname2) == 0);
+}
+
/* Collect IP addresses associated with an interface */
errno_t
sss_iface_addr_list_get(TALLOC_CTX *mem_ctx, const char *ifname,
@@ -200,10 +213,9 @@ sss_iface_addr_list_get(TALLOC_CTX *mem_ctx, const char *ifname,
if (!ifa->ifa_addr) continue;
/* Add IP addresses to the list */
- if ((ifa->ifa_addr->sa_family == AF_INET ||
- ifa->ifa_addr->sa_family == AF_INET6) &&
- strcasecmp(ifa->ifa_name, ifname) == 0 &&
- ok_for_dns(ifa->ifa_addr)) {
+ if (supported_address_family(ifa->ifa_addr->sa_family)
+ && matching_name(ifname, ifa->ifa_name)
+ && ok_for_dns(ifa->ifa_addr)) {
/* Add this address to the IP address list */
address = talloc_zero(mem_ctx, struct sss_iface_addr);
diff --git a/src/providers/ldap/sdap_dyndns.c b/src/providers/ldap/sdap_dyndns.c
index 3b621e340121ca972c47f9a462923cee7219a818..f5929cff3db6f724efcedeb963e3a12d04f6e1d3 100644
--- a/src/providers/ldap/sdap_dyndns.c
+++ b/src/providers/ldap/sdap_dyndns.c
@@ -496,7 +496,7 @@ static errno_t get_ifaces_addrs(TALLOC_CTX *mem_ctx,
tmp_ctx = talloc_new(NULL);
if (tmp_ctx == NULL) {
- ret = ENONEN;
+ ret = ENOMEM;
goto done;
}
--
2.4.3
>From 60f14b58fce74518964d51478299cf1ca1aff5b9 Mon Sep 17 00:00:00 2001
From: Pavel Reichl <[email protected]>
Date: Wed, 15 Jul 2015 10:58:38 -0400
Subject: [PATCH 4/5] TESTS: dyndns tests support AAAA addresses
Resolves:
https://fedorahosted.org/sssd/ticket/2558
---
src/tests/cmocka/test_dyndns.c | 51 +++++++++++++++++++++++++++++++-----------
1 file changed, 38 insertions(+), 13 deletions(-)
diff --git a/src/tests/cmocka/test_dyndns.c b/src/tests/cmocka/test_dyndns.c
index 3214e90c063ea9a4cf6f6bc6507bf4e37b7d23a4..e9d42cea37472b38ae2eb900368f2ce3f409fefc 100644
--- a/src/tests/cmocka/test_dyndns.c
+++ b/src/tests/cmocka/test_dyndns.c
@@ -97,7 +97,9 @@ int __wrap_getifaddrs(struct ifaddrs **_ifap)
struct ifaddrs *ifap_head = NULL;
char *name;
char *straddr;
+ int ad_family;
struct sockaddr_in *sa;
+ void *dst;
while ((name = sss_mock_ptr_type(char *)) != NULL) {
straddr = sss_mock_ptr_type(char *);
@@ -105,6 +107,7 @@ int __wrap_getifaddrs(struct ifaddrs **_ifap)
errno = EINVAL;
goto fail;
}
+ ad_family = sss_mock_type(int);
ifap = talloc_zero(global_mock_context, struct ifaddrs);
if (ifap == NULL) {
@@ -127,15 +130,33 @@ int __wrap_getifaddrs(struct ifaddrs **_ifap)
/* Do not alocate directly on ifap->ifa_addr to
* avoid alignment warnings */
- sa = talloc(ifap, struct sockaddr_in);
+ if (ad_family == AF_INET) {
+ sa = talloc(ifap, struct sockaddr_in);
+ } else if (ad_family == AF_INET6) {
+ sa = (struct sockaddr_in *) talloc(ifap, struct sockaddr_in6);
+ } else {
+ errno = EINVAL;
+ goto fail;
+ }
+
if (sa == NULL) {
errno = ENOMEM;
goto fail;
}
- sa->sin_family = AF_INET;
+
+ sa->sin_family = ad_family;
+
+ if (ad_family == AF_INET) {
+ dst = &sa->sin_addr;
+ } else if (ad_family == AF_INET6) {
+ dst = &((struct sockaddr_in6 *)sa)->sin6_addr;
+ } else {
+ errno = EINVAL;
+ goto fail;
+ }
/* convert straddr into ifa_addr */
- if (inet_pton(AF_INET, straddr, &sa->sin_addr) != 1) {
+ if (inet_pton(ad_family, straddr, dst) != 1) {
goto fail;
}
@@ -167,12 +188,16 @@ static void dyndns_test_done(struct tevent_req *req)
ctx->tctx->done = true;
}
-void will_return_getifaddrs(const char *ifname, const char *straddr)
+void will_return_getifaddrs(const char *ifname, const char *straddr,
+ int af_family)
{
will_return(__wrap_getifaddrs, ifname);
if (ifname) {
will_return(__wrap_getifaddrs, straddr);
}
+ if (straddr) {
+ will_return(__wrap_getifaddrs, af_family);
+ }
}
void dyndns_test_get_ifaddr(void **state)
@@ -182,9 +207,9 @@ void dyndns_test_get_ifaddr(void **state)
char straddr[128];
check_leaks_push(dyndns_test_ctx);
- will_return_getifaddrs("eth0", "192.168.0.1");
- will_return_getifaddrs("eth1", "192.168.0.2");
- will_return_getifaddrs(NULL, NULL); /* sentinel */
+ will_return_getifaddrs("eth0", "192.168.0.1", AF_INET);
+ will_return_getifaddrs("eth1", "192.168.0.2", AF_INET);
+ will_return_getifaddrs(NULL, NULL, 0); /* sentinel */
ret = sss_iface_addr_list_get(dyndns_test_ctx, "eth0", &addrlist);
assert_int_equal(ret, EOK);
@@ -212,9 +237,9 @@ void dyndns_test_get_multi_ifaddr(void **state)
char straddr[128];
check_leaks_push(dyndns_test_ctx);
- will_return_getifaddrs("eth0", "192.168.0.2");
- will_return_getifaddrs("eth0", "192.168.0.1");
- will_return_getifaddrs(NULL, NULL); /* sentinel */
+ will_return_getifaddrs("eth0", "192.168.0.2", AF_INET);
+ will_return_getifaddrs("eth0", "192.168.0.1", AF_INET);
+ will_return_getifaddrs(NULL, NULL, 0); /* sentinel */
ret = sss_iface_addr_list_get(dyndns_test_ctx, "eth0", &addrlist);
assert_int_equal(ret, EOK);
@@ -253,9 +278,9 @@ void dyndns_test_get_ifaddr_enoent(void **state)
struct sss_iface_addr *addrlist = NULL;
check_leaks_push(dyndns_test_ctx);
- will_return_getifaddrs("eth0", "192.168.0.1");
- will_return_getifaddrs("eth1", "192.168.0.2");
- will_return_getifaddrs(NULL, NULL); /* sentinel */
+ will_return_getifaddrs("eth0", "192.168.0.1", AF_INET);
+ will_return_getifaddrs("eth1", "192.168.0.2", AF_INET);
+ will_return_getifaddrs(NULL, NULL, 0); /* sentinel */
ret = sss_iface_addr_list_get(dyndns_test_ctx, "non_existing_interface",
&addrlist);
assert_int_equal(ret, ENOENT);
--
2.4.3
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
