Hi Lukas,
I sent a response on July 6th but perhaps there was an issue with the
mailing list or some reason it did not go through.
Updated patch attached.
I moved the resolv_is_address() function declaration into the
async_resolv.h file(so that it could be included in the
cmocka/test_resolv_fake.c test but I am not sure if this is the
correct approach). I also made the assumption of including my tests
in the already existing test_resolv_fake.c file instead of a
different file.
Also, I wasn't sure whether to use SSSDBG_MINOR_FAILURE or
SSSDBG_CONF_SETTINGS debug log level.
I am sure there are some corrections to make so I appreciate any
feedback.
Kind regards,
Justin Stephenson
On 08/05/2016 11:49 AM, Lukas Slebodnik wrote:
On (26/02/16 09:47), Justin Stephenson wrote:
Thanks a lot for the feedback, I am new to the cmocka framework so I will
take some time to learn it and respond after amending the patch.
Justin,
do you plan to update the patch?
LS
>From fd737c446f6aad4d8d2bfe4cb4f1477d02646618 Mon Sep 17 00:00:00 2001
From: Justin Stephenson <[email protected]>
Date: Wed, 6 Jul 2016 11:15:59 -0400
Subject: [PATCH] Resolves: https://fedorahosted.org/sssd/ticket/2789
ipa and ad provider initialization will provide a warning if IP address is used
for hard-coded server values, this may cause issues with GSSAPI
---
src/providers/ad/ad_common.c | 9 +++++++++
src/providers/ipa/ipa_common.c | 9 +++++++++
src/resolv/async_resolv.c | 5 ++---
src/resolv/async_resolv.h | 3 +++
src/tests/cmocka/test_resolv_fake.c | 26 ++++++++++++++++++++++++++
5 files changed, 49 insertions(+), 3 deletions(-)
diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c
index 9f9f9f1bdac2dc47bf18f9727ccd3b4675a2bb06..cb6942b9c3e4c1a6c79140000581b047078f2320 100644
--- a/src/providers/ad/ad_common.c
+++ b/src/providers/ad/ad_common.c
@@ -489,6 +489,7 @@ _ad_servers_init(struct ad_service *service,
bool primary)
{
size_t i;
+ int j;
errno_t ret = 0;
char **list;
struct ad_server_data *sdata;
@@ -504,6 +505,14 @@ _ad_servers_init(struct ad_service *service,
goto done;
}
+ for (j = 0; list[j]; j++) {
+ if (resolv_is_address(list[j])) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "ad_server [%s] is detected as IP address, "
+ "this can cause GSSAPI problems\n", list[j]);
+ }
+ }
+
/* Add each of these servers to the failover service */
for (i = 0; list[i]; i++) {
if (be_fo_is_srv_identifier(list[i])) {
diff --git a/src/providers/ipa/ipa_common.c b/src/providers/ipa/ipa_common.c
index b15ccc6ee87002a5ccb2ce8b2c195372f6b5eecd..209c03400c5db1d300d01f954f15868e6204bec6 100644
--- a/src/providers/ipa/ipa_common.c
+++ b/src/providers/ipa/ipa_common.c
@@ -831,6 +831,7 @@ static errno_t _ipa_servers_init(struct be_ctx *ctx,
char *ipa_domain;
int ret = 0;
int i;
+ int j;
tmp_ctx = talloc_new(NULL);
if (!tmp_ctx) {
@@ -844,6 +845,14 @@ static errno_t _ipa_servers_init(struct be_ctx *ctx,
goto done;
}
+ for (j = 0; list[j]; j++) {
+ if (resolv_is_address(list[j])) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "ipa_server [%s] is detected as IP address, "
+ "this can cause GSSAPI problems\n", list[j]);
+ }
+ }
+
/* now for each one add a new server to the failover service */
for (i = 0; list[i]; i++) {
diff --git a/src/resolv/async_resolv.c b/src/resolv/async_resolv.c
index 58d5c6e550bb34cbaa50517323133fad4f900980..86a07023a9508a53f1093c5e4206af4027f46788 100644
--- a/src/resolv/async_resolv.c
+++ b/src/resolv/async_resolv.c
@@ -1062,8 +1062,7 @@ resolv_gethostbyname_address(TALLOC_CTX *mem_ctx, const char *address,
struct resolv_hostent **_rhostent);
static inline int
resolv_gethostbyname_family_init(enum restrict_family family_order);
-static bool
-resolv_is_address(const char *name);
+
static errno_t
resolv_gethostbyname_step(struct tevent_req *req);
@@ -1133,7 +1132,7 @@ fail:
return NULL;
}
-static bool
+bool
resolv_is_address(const char *name)
{
struct addrinfo hints;
diff --git a/src/resolv/async_resolv.h b/src/resolv/async_resolv.h
index 876abff14e6e22c67585a476a7169423281f01c7..0b819217277e4249909e26ba0c60b2b16258ec6a 100644
--- a/src/resolv/async_resolv.h
+++ b/src/resolv/async_resolv.h
@@ -189,4 +189,7 @@ errno_t resolv_discover_srv_recv(TALLOC_CTX *mem_ctx,
uint32_t *_ttl,
char **_dns_domain);
+bool
+resolv_is_address(const char *name);
+
#endif /* __ASYNC_RESOLV_H__ */
diff --git a/src/tests/cmocka/test_resolv_fake.c b/src/tests/cmocka/test_resolv_fake.c
index a1e9ee4cba9b17d506eeae7e6088de28c301364d..ed1f28096ef096dfd541112457daaebcdefbd95b 100644
--- a/src/tests/cmocka/test_resolv_fake.c
+++ b/src/tests/cmocka/test_resolv_fake.c
@@ -31,6 +31,7 @@
#include <stdlib.h>
#include <resolv.h>
+#include "resolv/async_resolv.h"
#include "tests/cmocka/common_mock.h"
#include "tests/cmocka/common_mock_resp.h"
@@ -333,6 +334,30 @@ void test_resolv_fake_srv(void **state)
assert_int_equal(ret, ERR_OK);
}
+void test_resolv_is_address(void **state)
+{
+ bool ret;
+
+ ret = resolv_is_address("10.192.211.37");
+ assert_true(ret);
+
+ ret = resolv_is_address("127.0.0.1");
+ assert_true(ret);
+
+ ret = resolv_is_address("2001:0db8:85a3:0000:0000:8a2e:0370:7334");
+ assert_true(ret);
+
+ ret = resolv_is_address("sssd.ldap.com");
+ assert_false(ret);
+
+ ret = resolv_is_address("testhostname");
+ assert_false(ret);
+
+ ret = resolv_is_address("localhost");
+ assert_false(ret);
+
+}
+
int main(int argc, const char *argv[])
{
int rv;
@@ -348,6 +373,7 @@ int main(int argc, const char *argv[])
cmocka_unit_test_setup_teardown(test_resolv_fake_srv,
test_resolv_fake_setup,
test_resolv_fake_teardown),
+ cmocka_unit_test(test_resolv_is_address),
};
/* Set debug level to invalid value so we can deside if -d 0 was used. */
--
2.7.4
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]
