From f51280b0b490064131e85845eac8c25d66cfbc27 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Fri, 24 Feb 2017 13:55:47 +0100 Subject: [PATCH 1/8] krb5 locator: add support for multiple addresses Read multiple addresses from the kdcinfo files add call the provided callback with each of them. Related to https://pagure.io/SSSD/sssd/issue/941 --- src/krb5_plugin/sssd_krb5_locator_plugin.c | 342 +++++++++++++++++++++-------- 1 file changed, 245 insertions(+), 97 deletions(-) diff --git a/src/krb5_plugin/sssd_krb5_locator_plugin.c b/src/krb5_plugin/sssd_krb5_locator_plugin.c index 7c17fcb33..82fb5c7b2 100644 --- a/src/krb5_plugin/sssd_krb5_locator_plugin.c +++ b/src/krb5_plugin/sssd_krb5_locator_plugin.c @@ -42,7 +42,7 @@ #define DEFAULT_KADMIN_PORT 749 #define DEFAULT_KPASSWD_PORT 464 -#define BUFSIZE 512 +#define BUFSIZE 4096 #define PORT_STR_SIZE 7 #define SSSD_KRB5_LOCATOR_DEBUG "SSSD_KRB5_LOCATOR_DEBUG" #define SSSD_KRB5_LOCATOR_DISABLE "SSSD_KRB5_LOCATOR_DISABLE" @@ -53,12 +53,15 @@ } \ } while(0) +struct addr_port { + char *addr; + uint16_t port; +}; + struct sssd_ctx { char *sssd_realm; - char *kdc_addr; - uint16_t kdc_port; - char *kpasswd_addr; - uint16_t kpasswd_port; + struct addr_port *kdc_addr; + struct addr_port *kpasswd_addr; bool debug; bool disabled; }; @@ -82,6 +85,186 @@ void plugin_debug_fn(const char *format, ...) free(s); } + +static void free_addr_port_list(struct addr_port **list) +{ + size_t c; + + if (list == NULL || *list == NULL) { + return; + } + + for (c = 0; (*list)[c].addr != NULL; c++) { + free((*list)[c].addr); + } + free(*list); + *list = NULL; +} + +static int copy_addr_port_list(struct addr_port *src, bool clear_port, + struct addr_port **dst) +{ + size_t c; + struct addr_port *d = NULL; + int ret; + + /* only copy if dst is initialized to NULL */ + if (dst == NULL || *dst != NULL) { + return EINVAL; + } + + if (src == NULL) { + return 0; + } + + for (c = 0; src[c].addr != NULL; c++); + + d = calloc((c + 1), sizeof(struct addr_port)); + if (d == NULL) { + return ENOMEM; + } + + for (c = 0; src[c].addr != NULL; c++) { + d[c].addr = strdup(src[c].addr); + if (d[c].addr == NULL) { + ret = ENOMEM; + goto done; + } + if (clear_port) { + d[c].port = 0; + } else { + d[c].port = src[c].port; + } + } + + ret = EOK; + +done: + if (ret != EOK) { + free_addr_port_list(&d); + } else { + *dst = d; + } + + return ret; +} + +static int buf_to_addr_port_list(struct sssd_ctx *ctx, + uint8_t *buf, size_t buf_size, + struct addr_port **list) +{ + struct addr_port *l = NULL; + int ret; + uint8_t *p; + uint8_t *pn; + size_t c; + size_t len; + char *tmp = NULL; + char *port_str; + long port; + char *endptr; + + /* only create if list is initialized to NULL */ + if (buf == NULL || buf_size == 0 || list == NULL || *list != NULL) { + return EINVAL; + } + + c = 1; /* to account for a missing \n at the very end */ + p = buf; + while ((p - buf) < buf_size + && (p = memchr(p, '\n', buf_size - (p - buf))) != NULL) { + p++; + c++; + } + + l = calloc((c + 1), sizeof(struct addr_port)); + if (l == NULL) { + return ENOMEM; + } + + c = 0; + p = buf; + do { + pn = memchr(p, '\n', buf_size - (p - buf)); + if (pn != NULL) { + len = pn - p; + } else { + len = buf_size - (p - buf); + } + if (len == 0) { + /* empty line no more processing */ + break; + } + + free(tmp); + tmp = strndup((char *) p, len); + if (tmp == NULL) { + ret = ENOMEM; + goto done; + } + + port_str = strrchr(tmp, ':'); + if (port_str == NULL) { + port = 0; + } else { + *port_str = '\0'; + ++port_str; + + if (isdigit(*port_str)) { + errno = 0; + port = strtol(port_str, &endptr, 10); + if (errno != 0) { + ret = errno; + PLUGIN_DEBUG(("strtol failed on [%s]: [%d][%s], " + "assuming default.\n", port_str, ret, + strerror(ret))); + port = 0; + } + if (*endptr != '\0') { + PLUGIN_DEBUG(("Found additional characters [%s] in port " + "number [%s], assuming default.\n", endptr, + port_str)); + port = 0; + } + + if (port < 0 || port > 65535) { + PLUGIN_DEBUG(("Illegal port number [%ld], assuming " + "default.\n", port)); + port = 0; + } + } else { + PLUGIN_DEBUG(("Illegal port number [%s], assuming default.\n", + port_str)); + port = 0; + } + } + + PLUGIN_DEBUG(("Found [%s][%d].\n", tmp, port)); + + l[c].addr = strdup(tmp); + if (l[c].addr == NULL) { + ret = ENOMEM; + goto done; + } + l[c].port = port; + + c++; + p = pn == NULL ? NULL : (pn + 1); + } while (p != NULL); + + ret = EOK; + +done: + free(tmp); + if (ret != EOK) { + free_addr_port_list(&l); + } else { + *list = l; + } + + return ret; +} + static int get_krb5info(const char *realm, struct sssd_ctx *ctx, enum locate_service_type svc) { @@ -91,9 +274,6 @@ static int get_krb5info(const char *realm, struct sssd_ctx *ctx, uint8_t buf[BUFSIZE + 1]; int fd = -1; const char *name_tmpl = NULL; - char *port_str; - long port; - char *endptr; switch (svc) { case locate_service_kdc: @@ -148,62 +328,21 @@ static int get_krb5info(const char *realm, struct sssd_ctx *ctx, PLUGIN_DEBUG(("Content of krb5info file [%s] is [%d] or larger.\n", krb5info_name, BUFSIZE)); } - PLUGIN_DEBUG(("Found [%s] in [%s].\n", buf, krb5info_name)); - - port_str = strrchr((char *) buf, ':'); - if (port_str == NULL) { - port = 0; - } else { - *port_str = '\0'; - ++port_str; - - if (isdigit(*port_str)) { - errno = 0; - port = strtol(port_str, &endptr, 10); - if (errno != 0) { - ret = errno; - PLUGIN_DEBUG(("strtol failed on [%s]: [%d][%s], " - "assuming default.\n", port_str, ret, strerror(ret))); - port = 0; - } - if (*endptr != '\0') { - PLUGIN_DEBUG(("Found additional characters [%s] in port number " - "[%s], assuming default.\n", endptr, port_str)); - port = 0; - } - - if (port < 0 || port > 65535) { - PLUGIN_DEBUG(("Illegal port number [%ld], assuming default.\n", - port)); - port = 0; - } - } else { - PLUGIN_DEBUG(("Illegal port number [%s], assuming default.\n", - port_str)); - port = 0; - } - } switch (svc) { case locate_service_kdc: - free(ctx->kdc_addr); - ctx->kdc_addr = strdup((char *) buf); - if (ctx->kdc_addr == NULL) { - PLUGIN_DEBUG(("strdup failed.\n")); - ret = ENOMEM; + free_addr_port_list(&(ctx->kdc_addr)); + ret = buf_to_addr_port_list(ctx, buf, len, &(ctx->kdc_addr)); + if (ret != EOK) { goto done; } - ctx->kdc_port = (uint16_t) port; break; case locate_service_kpasswd: - free(ctx->kpasswd_addr); - ctx->kpasswd_addr = strdup((char *) buf); - if (ctx->kpasswd_addr == NULL) { - PLUGIN_DEBUG(("strdup failed.\n")); - ret = ENOMEM; + free_addr_port_list(&(ctx->kpasswd_addr)); + ret = buf_to_addr_port_list(ctx, buf, len, &(ctx->kpasswd_addr)); + if (ret != EOK) { goto done; } - ctx->kpasswd_port = (uint16_t) port; break; default: PLUGIN_DEBUG(("Unsupported service [%d].\n", svc)); @@ -256,8 +395,8 @@ void sssd_krb5_locator_close(void *private_data) ctx = (struct sssd_ctx *) private_data; PLUGIN_DEBUG(("sssd_krb5_locator_close called\n")); - free(ctx->kdc_addr); - free(ctx->kpasswd_addr); + free_addr_port_list(&(ctx->kdc_addr)); + free_addr_port_list(&(ctx->kpasswd_addr)); free(ctx->sssd_realm); free(ctx); @@ -277,8 +416,10 @@ krb5_error_code sssd_krb5_locator_lookup(void *private_data, struct sssd_ctx *ctx; struct addrinfo ai_hints; uint16_t port = 0; - const char *addr = NULL; + uint16_t default_port = 0; + struct addr_port *addr = NULL; char port_str[PORT_STR_SIZE]; + size_t c; if (private_data == NULL) return KRB5_PLUGIN_NO_HANDLE; ctx = (struct sssd_ctx *) private_data; @@ -308,9 +449,13 @@ krb5_error_code sssd_krb5_locator_lookup(void *private_data, if (ret != EOK) { PLUGIN_DEBUG(("reading kpasswd address failed, " "using kdc address.\n")); - free(ctx->kpasswd_addr); - ctx->kpasswd_addr = strdup(ctx->kdc_addr); - ctx->kpasswd_port = 0; + free_addr_port_list(&(ctx->kpasswd_addr)); + ret = copy_addr_port_list(ctx->kdc_addr, true, + &(ctx->kpasswd_addr)); + if (ret != EOK) { + PLUGIN_DEBUG(("copying address list failed.\n")); + return KRB5_PLUGIN_NO_HANDLE; + } } } } @@ -322,19 +467,19 @@ krb5_error_code sssd_krb5_locator_lookup(void *private_data, switch (svc) { case locate_service_kdc: addr = ctx->kdc_addr; - port = ctx->kdc_port ? ctx->kdc_port : DEFAULT_KERBEROS_PORT; + default_port = DEFAULT_KERBEROS_PORT; break; case locate_service_master_kdc: addr = ctx->kpasswd_addr; - port = DEFAULT_KERBEROS_PORT; + default_port = DEFAULT_KERBEROS_PORT; break; case locate_service_kadmin: addr = ctx->kpasswd_addr; - port = DEFAULT_KADMIN_PORT; + default_port = DEFAULT_KADMIN_PORT; break; case locate_service_kpasswd: addr = ctx->kpasswd_addr; - port = ctx->kpasswd_port ? ctx->kpasswd_port : DEFAULT_KPASSWD_PORT; + default_port = DEFAULT_KPASSWD_PORT; break; case locate_service_krb524: return KRB5_PLUGIN_NO_HANDLE; @@ -362,46 +507,49 @@ krb5_error_code sssd_krb5_locator_lookup(void *private_data, if (strcmp(realm, ctx->sssd_realm) != 0) return KRB5_PLUGIN_NO_HANDLE; - memset(port_str, 0, PORT_STR_SIZE); - ret = snprintf(port_str, PORT_STR_SIZE-1, "%u", port); - if (ret < 0 || ret >= (PORT_STR_SIZE-1)) { - PLUGIN_DEBUG(("snprintf failed.\n")); - return KRB5_PLUGIN_NO_HANDLE; - } + for (c = 0; addr[c].addr != NULL; c++) { + port = (addr[c].port == 0 ? default_port : addr[c].port); + memset(port_str, 0, PORT_STR_SIZE); + ret = snprintf(port_str, PORT_STR_SIZE-1, "%u", port); + if (ret < 0 || ret >= (PORT_STR_SIZE-1)) { + PLUGIN_DEBUG(("snprintf failed.\n")); + return KRB5_PLUGIN_NO_HANDLE; + } - memset(&ai_hints, 0, sizeof(struct addrinfo)); - ai_hints.ai_flags = AI_NUMERICHOST|AI_NUMERICSERV; - ai_hints.ai_socktype = socktype; - - ret = getaddrinfo(addr, port_str, &ai_hints, &ai); - if (ret != 0) { - PLUGIN_DEBUG(("getaddrinfo failed [%d][%s].\n", ret, - gai_strerror(ret))); - if (ret == EAI_SYSTEM) { - PLUGIN_DEBUG(("getaddrinfo failed [%d][%s].\n", errno, - strerror(errno))); + memset(&ai_hints, 0, sizeof(struct addrinfo)); + ai_hints.ai_flags = AI_NUMERICHOST|AI_NUMERICSERV; + ai_hints.ai_socktype = socktype; + + ret = getaddrinfo(addr[c].addr, port_str, &ai_hints, &ai); + if (ret != 0) { + PLUGIN_DEBUG(("getaddrinfo failed [%d][%s].\n", ret, + gai_strerror(ret))); + if (ret == EAI_SYSTEM) { + PLUGIN_DEBUG(("getaddrinfo failed [%d][%s].\n", + errno, strerror(errno))); + } + return KRB5_PLUGIN_NO_HANDLE; } - return KRB5_PLUGIN_NO_HANDLE; - } - PLUGIN_DEBUG(("addr[%s:%s] family[%d] socktype[%d]\n", addr, port_str, - ai->ai_family, ai->ai_socktype)); + PLUGIN_DEBUG(("addr[%s:%s] family[%d] socktype[%d]\n", addr[c].addr, + port_str, ai->ai_family, ai->ai_socktype)); - if ((family == AF_UNSPEC || ai->ai_family == family) && - ai->ai_socktype == socktype) { + if ((family == AF_UNSPEC || ai->ai_family == family) && + ai->ai_socktype == socktype) { - ret = cbfunc(cbdata, socktype, ai->ai_addr); - if (ret != 0) { - PLUGIN_DEBUG(("cbfunc failed\n")); - freeaddrinfo(ai); - return ret; + ret = cbfunc(cbdata, socktype, ai->ai_addr); + if (ret != 0) { + PLUGIN_DEBUG(("cbfunc failed\n")); + freeaddrinfo(ai); + return ret; + } else { + PLUGIN_DEBUG(("[%s] used\n", addr[c].addr)); + } } else { - PLUGIN_DEBUG(("[%s] used\n", addr)); + PLUGIN_DEBUG(("[%s] NOT used\n", addr[c].addr)); } - } else { - PLUGIN_DEBUG(("[%s] NOT used\n", addr)); + freeaddrinfo(ai); } - freeaddrinfo(ai); return 0; } From 77d1b35473fb11e4cdf6e8b7a64dc6fd48f9d42c Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Tue, 22 May 2018 17:59:52 +0200 Subject: [PATCH 2/8] krb5 locator: fix IPv6 support IPv6 addresses are added with surrounding '[' and ']' to the kdcinfo file to be able to specify a port number properly. The Kerberos location plugin didn't handle those entries properly. Related to https://pagure.io/SSSD/sssd/issue/941 --- src/krb5_plugin/sssd_krb5_locator_plugin.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/krb5_plugin/sssd_krb5_locator_plugin.c b/src/krb5_plugin/sssd_krb5_locator_plugin.c index 82fb5c7b2..58cac7f4b 100644 --- a/src/krb5_plugin/sssd_krb5_locator_plugin.c +++ b/src/krb5_plugin/sssd_krb5_locator_plugin.c @@ -159,6 +159,8 @@ static int buf_to_addr_port_list(struct sssd_ctx *ctx, uint8_t *pn; size_t c; size_t len; + size_t addr_len; + char *addr_str = NULL; char *tmp = NULL; char *port_str; long port; @@ -206,6 +208,9 @@ static int buf_to_addr_port_list(struct sssd_ctx *ctx, port_str = strrchr(tmp, ':'); if (port_str == NULL) { port = 0; + } else if (tmp[0] == '[' && *(port_str - 1) != ']') { + /* IPv6 address without port number */ + port = 0; } else { *port_str = '\0'; ++port_str; @@ -239,9 +244,19 @@ static int buf_to_addr_port_list(struct sssd_ctx *ctx, } } - PLUGIN_DEBUG(("Found [%s][%d].\n", tmp, port)); + /* make sure tmp is not modified so that it can be freed later */ + addr_str = tmp; + /* strip leading '[' and trailing ']' from IPv6 addresses */ + if (addr_str[0] == '[' + && (addr_len = strlen(addr_str)) + && addr_str[addr_len - 1] == ']') { + addr_str[addr_len -1] = '\0'; + addr_str++; + } + + PLUGIN_DEBUG(("Found [%s][%d].\n", addr_str, port)); - l[c].addr = strdup(tmp); + l[c].addr = strdup(addr_str); if (l[c].addr == NULL) { ret = ENOMEM; goto done; From 061894eae9e2c7eee7ead88dc907442da955ee8e Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Tue, 22 May 2018 18:03:05 +0200 Subject: [PATCH 3/8] krb5 locator: make plugin more robust Although currently libkrb5 sets all parameters of the locator plugin calls to suitable values we should make sure that provided pointers are not NULL before trying to dereference them. Related to https://pagure.io/SSSD/sssd/issue/941 --- src/krb5_plugin/sssd_krb5_locator_plugin.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/krb5_plugin/sssd_krb5_locator_plugin.c b/src/krb5_plugin/sssd_krb5_locator_plugin.c index 58cac7f4b..9874fd2d1 100644 --- a/src/krb5_plugin/sssd_krb5_locator_plugin.c +++ b/src/krb5_plugin/sssd_krb5_locator_plugin.c @@ -439,6 +439,10 @@ krb5_error_code sssd_krb5_locator_lookup(void *private_data, if (private_data == NULL) return KRB5_PLUGIN_NO_HANDLE; ctx = (struct sssd_ctx *) private_data; + if (realm == NULL || cbfunc == NULL || cbdata == NULL) { + return KRB5_PLUGIN_NO_HANDLE; + } + if (ctx->disabled) { PLUGIN_DEBUG(("Plugin disabled, nothing to do.\n")); return KRB5_PLUGIN_NO_HANDLE; From f43ac7781926082c5787b6129b3b12f10f1d4ce2 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Thu, 24 May 2018 17:14:42 +0200 Subject: [PATCH 4/8] krb5 locator: add unit tests Unit test for existing and new functionality of the Kerberos locator plugin. Related to https://pagure.io/SSSD/sssd/issue/941 --- Makefile.am | 20 + src/krb5_plugin/sssd_krb5_locator_plugin.c | 16 + src/tests/cmocka/test_sssd_krb5_locator_plugin.c | 631 +++++++++++++++++++++++ 3 files changed, 667 insertions(+) create mode 100644 src/tests/cmocka/test_sssd_krb5_locator_plugin.c diff --git a/Makefile.am b/Makefile.am index 9539b3cff..9055130ed 100644 --- a/Makefile.am +++ b/Makefile.am @@ -288,6 +288,7 @@ if HAVE_CMOCKA krb5_common_test \ test_iobuf \ sss_certmap_test \ + test_sssd_krb5_locator_plugin \ $(NULL) @@ -3518,6 +3519,25 @@ sss_certmap_test_LDADD = \ libsss_certmap.la \ $(NULL) +test_sssd_krb5_locator_plugin_SOURCES = \ + src/tests/cmocka/test_sssd_krb5_locator_plugin.c \ + src/krb5_plugin/sssd_krb5_locator_plugin.c \ + $(NULL) +test_sssd_krb5_locator_plugin_CFLAGS = \ + $(AM_CFLAGS) \ + $(POPT_CFLAGS) \ + $(TALLOC_CFLAGS) \ + $(KRB5_CFLAGS) \ + -DTEST_PUBCONF_PATH=\"$(abs_builddir)/src/tests/cmocka/pubconf\" \ + $(NULL) +test_sssd_krb5_locator_plugin_LDADD = \ + $(CMOCKA_LIBS) \ + $(POPT_LIBS) \ + $(TALLOC_LIBS) \ + $(KRB5_LIBS) \ + libsss_test_common.la \ + $(NULL) + if BUILD_KCM test_kcm_json_SOURCES = \ src/tests/cmocka/test_kcm_json_marshalling.c \ diff --git a/src/krb5_plugin/sssd_krb5_locator_plugin.c b/src/krb5_plugin/sssd_krb5_locator_plugin.c index 9874fd2d1..952d487c2 100644 --- a/src/krb5_plugin/sssd_krb5_locator_plugin.c +++ b/src/krb5_plugin/sssd_krb5_locator_plugin.c @@ -38,6 +38,22 @@ #include "providers/krb5/krb5_common.h" +/* The following override of KDCINFO_TMPL and KPASSWDINFO_TMPL is not very + * elegant but since they are defined in krb5_common.h with the help of + * PUBCONF_PATH from config.h and PUBCONF_PATH can by set by a configure + * options I didn't found another way to change the path for a unit test. */ +#ifdef TEST_PUBCONF_PATH +#ifdef KDCINFO_TMPL +#undef KDCINFO_TMPL +#endif +#define KDCINFO_TMPL TEST_PUBCONF_PATH"/kdcinfo.%s" + +#ifdef KPASSWDINFO_TMPL +#undef KPASSWDINFO_TMPL +#endif +#define KPASSWDINFO_TMPL TEST_PUBCONF_PATH"/kpasswdinfo.%s" +#endif /* TEST_PUBCONF_PATH */ + #define DEFAULT_KERBEROS_PORT 88 #define DEFAULT_KADMIN_PORT 749 #define DEFAULT_KPASSWD_PORT 464 diff --git a/src/tests/cmocka/test_sssd_krb5_locator_plugin.c b/src/tests/cmocka/test_sssd_krb5_locator_plugin.c new file mode 100644 index 000000000..3e7d00632 --- /dev/null +++ b/src/tests/cmocka/test_sssd_krb5_locator_plugin.c @@ -0,0 +1,631 @@ +/* + SSSD + + Unit test for SSSD's MIT Kerberos locator plugin + + Authors: + Sumit Bose + + Copyright (C) 2018 Red Hat + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +*/ +#include "config.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "tests/cmocka/common_mock.h" + +#define TEST_REALM "TEST.REALM" +#define TEST_IP_1 "123.231.132.213" +#define TEST_IPV6_1_PURE "7025:4d2d:2b06:e321:d971:16c0:6eeb:cc41" +#define TEST_IPV6_1 "["TEST_IPV6_1_PURE"]" +#define TEST_SERVICE_1 "22334" +#define TEST_SERVICE_2 "54321" +#define TEST_IP_1_WITH_SERVICE TEST_IP_1":"TEST_SERVICE_1 +#define TEST_IPV6_1_WITH_SERVICE TEST_IPV6_1":"TEST_SERVICE_2 + +struct test_state { + void *dummy; +}; + +static int setup(void **state) +{ + struct test_state *ts = NULL; + + assert_true(leak_check_setup()); + + ts = talloc(global_talloc_context, struct test_state); + assert_non_null(ts); + + check_leaks_push(ts); + *state = (void *)ts; + + unlink(TEST_PUBCONF_PATH"/kdcinfo."TEST_REALM); + rmdir(TEST_PUBCONF_PATH); + + return 0; +} + +static int teardown(void **state) +{ + struct test_state *ts = talloc_get_type_abort(*state, struct test_state); + + assert_non_null(ts); + + assert_true(check_leaks_pop(ts)); + talloc_free(ts); + assert_true(leak_check_teardown()); + return 0; +} + +/* Taken from MIT Kerberos src/lib/krb5/os/locate_kdc.c and + * lib/krb5/os/os-proto.h */ + +typedef enum { + TCP_OR_UDP = 0, + TCP, + UDP, + HTTPS, +} k5_transport; + +/* A single server hostname or address. */ +struct server_entry { + char *hostname; /* NULL -> use addrlen/addr instead */ + int port; /* Used only if hostname set */ + k5_transport transport; /* May be 0 for UDP/TCP if hostname set */ + char *uri_path; /* Used only if transport is HTTPS */ + int family; /* May be 0 (aka AF_UNSPEC) if hostname set */ + int master; /* True, false, or -1 for unknown. */ + size_t addrlen; + struct sockaddr_storage addr; +}; + +/* A list of server hostnames/addresses. */ +struct serverlist { + struct server_entry *servers; + size_t nservers; +}; +#define SERVERLIST_INIT { NULL, 0 } + +/* Free up everything pointed to by the serverlist structure, but don't + * * free the structure itself. */ +void +k5_free_serverlist (struct serverlist *list) +{ + size_t i; + + for (i = 0; i < list->nservers; i++) { + free(list->servers[i].hostname); + free(list->servers[i].uri_path); + } + free(list->servers); + list->servers = NULL; + list->nservers = 0; +} + +/* Make room for a new server entry in list and return a pointer to the new + * entry. (Do not increment list->nservers.) */ +static struct server_entry * +new_server_entry(struct serverlist *list) +{ + struct server_entry *newservers, *entry; + size_t newspace = (list->nservers + 1) * sizeof(struct server_entry); + + newservers = realloc(list->servers, newspace); + if (newservers == NULL) + return NULL; + list->servers = newservers; + entry = &newservers[list->nservers]; + memset(entry, 0, sizeof(*entry)); + entry->master = -1; + return entry; +} + +/* Add an address entry to list. */ +static int +add_addr_to_list(struct serverlist *list, k5_transport transport, int family, + size_t addrlen, struct sockaddr *addr) +{ + struct server_entry *entry; + + entry = new_server_entry(list); + if (entry == NULL) + return ENOMEM; + entry->transport = transport; + entry->family = family; + entry->hostname = NULL; + entry->uri_path = NULL; + entry->addrlen = addrlen; + memcpy(&entry->addr, addr, addrlen); + list->nservers++; + return 0; +} + +struct module_callback_data { + int out_of_mem; + struct serverlist *list; +}; + +static int +module_callback(void *cbdata, int socktype, struct sockaddr *sa) +{ + struct module_callback_data *d = cbdata; + size_t addrlen; + k5_transport transport; + + if (socktype != SOCK_STREAM && socktype != SOCK_DGRAM) + return 0; + if (sa->sa_family == AF_INET) + addrlen = sizeof(struct sockaddr_in); + else if (sa->sa_family == AF_INET6) + addrlen = sizeof(struct sockaddr_in6); + else + return 0; + transport = (socktype == SOCK_STREAM) ? TCP : UDP; + if (add_addr_to_list(d->list, transport, sa->sa_family, addrlen, + sa) != 0) { + /* Assumes only error is ENOMEM. */ + d->out_of_mem = 1; + return 1; + } + return 0; +} + +krb5_error_code sssd_krb5_locator_init(krb5_context context, + void **private_data); +void sssd_krb5_locator_close(void *private_data); + +krb5_error_code sssd_krb5_locator_lookup(void *private_data, + enum locate_service_type svc, + const char *realm, + int socktype, + int family, + int (*cbfunc)(void *, int, struct sockaddr *), + void *cbdata); + +void test_init(void **state) +{ + krb5_context ctx; + krb5_error_code kerr; + void *priv; + + kerr = krb5_init_context (&ctx); + assert_int_equal(kerr, 0); + + kerr = sssd_krb5_locator_init(ctx, &priv); + assert_int_equal(kerr, 0); + + sssd_krb5_locator_close(priv); + + krb5_free_context(ctx); +} + +void test_failed_lookup(void **state) +{ + krb5_context ctx; + krb5_error_code kerr; + void *priv; + struct module_callback_data cbdata = { 0 }; + + + kerr = krb5_init_context (&ctx); + assert_int_equal(kerr, 0); + + kerr = sssd_krb5_locator_init(ctx, &priv); + assert_int_equal(kerr, 0); + + kerr = sssd_krb5_locator_lookup(NULL, -1, NULL, -1, -1, NULL, NULL); + assert_int_equal(kerr, KRB5_PLUGIN_NO_HANDLE); + + kerr = sssd_krb5_locator_lookup(priv, -1, NULL, -1, -1, NULL, NULL); + assert_int_equal(kerr, KRB5_PLUGIN_NO_HANDLE); + + kerr = sssd_krb5_locator_lookup(priv, locate_service_kdc , NULL, -1, -1, + NULL, NULL); + assert_int_equal(kerr, KRB5_PLUGIN_NO_HANDLE); + + kerr = sssd_krb5_locator_lookup(priv, locate_service_kdc , TEST_REALM, -1, + -1, NULL, NULL); + assert_int_equal(kerr, KRB5_PLUGIN_NO_HANDLE); + + kerr = sssd_krb5_locator_lookup(priv, locate_service_kdc , TEST_REALM, + SOCK_DGRAM, -1, NULL, NULL); + assert_int_equal(kerr, KRB5_PLUGIN_NO_HANDLE); + + kerr = sssd_krb5_locator_lookup(priv, locate_service_kdc , TEST_REALM, + SOCK_DGRAM, AF_INET6, NULL, NULL); + assert_int_equal(kerr, KRB5_PLUGIN_NO_HANDLE); + + kerr = sssd_krb5_locator_lookup(priv, locate_service_kdc , TEST_REALM, + SOCK_DGRAM, AF_INET6, module_callback, + NULL); + assert_int_equal(kerr, KRB5_PLUGIN_NO_HANDLE); + + kerr = sssd_krb5_locator_lookup(priv, locate_service_kdc , TEST_REALM, + SOCK_DGRAM, AF_INET6, module_callback, + &cbdata); + assert_int_equal(kerr, KRB5_PLUGIN_NO_HANDLE); + + sssd_krb5_locator_close(priv); + + krb5_free_context(ctx); +} + +void test_empty(void **state) +{ + krb5_context ctx; + krb5_error_code kerr; + void *priv; + int fd; + struct module_callback_data cbdata = { 0 }; + + kerr = krb5_init_context (&ctx); + assert_int_equal(kerr, 0); + + kerr = sssd_krb5_locator_init(ctx, &priv); + assert_int_equal(kerr, 0); + + mkdir(TEST_PUBCONF_PATH, 0777); + fd = open(TEST_PUBCONF_PATH"/kdcinfo."TEST_REALM, O_CREAT, 0777); + assert_int_not_equal(fd, -1); + close(fd); + + kerr = sssd_krb5_locator_lookup(priv, locate_service_kdc , TEST_REALM, + SOCK_DGRAM, AF_INET6, module_callback, + &cbdata); + assert_int_equal(kerr, KRB5_PLUGIN_NO_HANDLE); + unlink(TEST_PUBCONF_PATH"/kdcinfo."TEST_REALM); + rmdir(TEST_PUBCONF_PATH); + + sssd_krb5_locator_close(priv); + + krb5_free_context(ctx); +} + +void test_single(void **state) +{ + krb5_context ctx; + krb5_error_code kerr; + void *priv; + int fd; + struct serverlist list = SERVERLIST_INIT; + struct module_callback_data cbdata = { 0 }; + ssize_t s; + int ret; + char host[NI_MAXHOST]; + char service[NI_MAXSERV]; + + cbdata.list = &list; + + kerr = krb5_init_context (&ctx); + assert_int_equal(kerr, 0); + + kerr = sssd_krb5_locator_init(ctx, &priv); + assert_int_equal(kerr, 0); + + mkdir(TEST_PUBCONF_PATH, 0777); + fd = open(TEST_PUBCONF_PATH"/kdcinfo."TEST_REALM, O_CREAT|O_RDWR, 0777); + assert_int_not_equal(fd, -1); + s = write(fd, TEST_IP_1, sizeof(TEST_IP_1)); + assert_int_equal(s, sizeof(TEST_IP_1)); + close(fd); + + kerr = sssd_krb5_locator_lookup(priv, locate_service_kdc , TEST_REALM, + SOCK_DGRAM, AF_INET6, module_callback, + &cbdata); + assert_int_equal(kerr, 0); + + /* We asked for AF_INET6, but TEST_IP_1 is IPv4 */ + assert_int_equal(list.nservers, 0); + assert_null(list.servers); + + kerr = sssd_krb5_locator_lookup(priv, locate_service_kdc , TEST_REALM, + SOCK_DGRAM, AF_INET, module_callback, + &cbdata); + assert_int_equal(kerr, 0); + assert_int_equal(list.nservers, 1); + assert_non_null(list.servers); + assert_int_equal(list.servers[0].addrlen, 16); + ret = getnameinfo((struct sockaddr *) &list.servers[0].addr, + list.servers[0].addrlen, + host, sizeof(host), service, sizeof(service), + NI_NUMERICHOST|NI_NUMERICSERV); + assert_int_equal(ret, 0); + assert_string_equal(TEST_IP_1, host); + assert_string_equal("88", service); + + k5_free_serverlist(&list); + + kerr = sssd_krb5_locator_lookup(priv, locate_service_kdc , TEST_REALM, + SOCK_DGRAM, AF_UNSPEC, module_callback, + &cbdata); + assert_int_equal(kerr, 0); + assert_int_equal(list.nservers, 1); + assert_non_null(list.servers); + assert_int_equal(list.servers[0].addrlen, 16); + ret = getnameinfo((struct sockaddr *) &list.servers[0].addr, + list.servers[0].addrlen, + host, sizeof(host), service, sizeof(service), + NI_NUMERICHOST|NI_NUMERICSERV); + assert_int_equal(ret, 0); + assert_string_equal(TEST_IP_1, host); + assert_string_equal("88", service); + + k5_free_serverlist(&list); + + unlink(TEST_PUBCONF_PATH"/kdcinfo."TEST_REALM); + rmdir(TEST_PUBCONF_PATH); + sssd_krb5_locator_close(priv); + + krb5_free_context(ctx); +} + +struct test_data { + const char *ip; + bool found; +}; + +void test_multi_check_results(struct test_data *test_data, + struct serverlist *list, + const char *exp_service) +{ + int ret; + char host[NI_MAXHOST]; + char service[NI_MAXSERV]; + size_t c; + size_t d; + + /* To make sure each result from list has a matching entry in test_data we + * use a flag to mark found entries, this way we can properly detect is + * the same address is used multiple times. */ + for (d = 0; test_data[d].ip != NULL; d++) { + test_data[d].found = false; + } + + for (c = 0; c < list->nservers; c++) { + ret = getnameinfo((struct sockaddr *) &list->servers[c].addr, + list->servers[c].addrlen, + host, sizeof(host), service, sizeof(service), + NI_NUMERICHOST|NI_NUMERICSERV); + assert_int_equal(ret, 0); + assert_string_equal(exp_service, service); + for (d = 0; test_data[d].ip != NULL; d++) { + /* Compare result with test_data, be aware that the test_data has + * '[]' around IPv& addresses */ + if (strncmp(host, + test_data[d].ip + (test_data[d].ip[0] == '[' ? 1 : 0), + strlen(host)) == 0 && !test_data[d].found) { + test_data[d].found = true; + break; + } + } + /* Make sure we found the result in the list */ + assert_non_null(test_data[d].ip); + } +} + +void test_multi(void **state) +{ + krb5_context ctx; + krb5_error_code kerr; + void *priv; + int fd; + struct serverlist list = SERVERLIST_INIT; + struct module_callback_data cbdata = { 0 }; + ssize_t s; + size_t c; + struct test_data test_data[] = { + {TEST_IP_1, false}, + {TEST_IPV6_1, false}, + {"[c89a:565b:4510:5b9f:41fe:ea81:87a0:f21b]", false}, + {"155.42.66.53", false}, + {"[f812:5941:ba69:2bae:e806:3b68:770d:d75e]", false}, + {"[3ad3:9dda:50e4:3c82:548f:eaa1:e120:6dd]", false}, + {"55.116.79.183", false}, + {"[ce8a:ee99:98cd:d8cd:218d:393e:d5a9:dc52]", false}, + /* the following address is added twice to check if + * an address can be added more than once. */ + {"37.230.88.162", false}, + {"37.230.88.162", false}, + {NULL, false} }; + + cbdata.list = &list; + + kerr = krb5_init_context (&ctx); + assert_int_equal(kerr, 0); + + kerr = sssd_krb5_locator_init(ctx, &priv); + assert_int_equal(kerr, 0); + + mkdir(TEST_PUBCONF_PATH, 0777); + fd = open(TEST_PUBCONF_PATH"/kdcinfo."TEST_REALM, O_CREAT|O_RDWR, 0777); + assert_int_not_equal(fd, -1); + for (c = 0; test_data[c].ip != NULL; c++) { + s = write(fd, test_data[c].ip, strlen(test_data[c].ip)); + assert_int_equal(s, strlen(test_data[c].ip)); + s = write(fd, "\n", 1); + assert_int_equal(s, 1); + } + close(fd); + + kerr = sssd_krb5_locator_lookup(priv, locate_service_kdc , TEST_REALM, + SOCK_DGRAM, AF_INET6, module_callback, + &cbdata); + assert_int_equal(kerr, 0); + + assert_int_equal(list.nservers, 5); + assert_non_null(list.servers); + test_multi_check_results(test_data, &list, "88"); + + k5_free_serverlist(&list); + + kerr = sssd_krb5_locator_lookup(priv, locate_service_kdc , TEST_REALM, + SOCK_DGRAM, AF_INET, module_callback, + &cbdata); + assert_int_equal(kerr, 0); + + assert_int_equal(list.nservers, 5); + assert_non_null(list.servers); + test_multi_check_results(test_data, &list, "88"); + + + k5_free_serverlist(&list); + + kerr = sssd_krb5_locator_lookup(priv, locate_service_kdc , TEST_REALM, + SOCK_DGRAM, AF_UNSPEC, module_callback, + &cbdata); + assert_int_equal(kerr, 0); + + assert_int_equal(list.nservers, 10); + assert_non_null(list.servers); + test_multi_check_results(test_data, &list, "88"); + + k5_free_serverlist(&list); + + unlink(TEST_PUBCONF_PATH"/kdcinfo."TEST_REALM); + rmdir(TEST_PUBCONF_PATH); + sssd_krb5_locator_close(priv); + + krb5_free_context(ctx); +} + +void test_service(void **state) +{ + krb5_context ctx; + krb5_error_code kerr; + void *priv; + int fd; + struct serverlist list = SERVERLIST_INIT; + struct module_callback_data cbdata = { 0 }; + ssize_t s; + int ret; + char host[NI_MAXHOST]; + char service[NI_MAXSERV]; + + cbdata.list = &list; + + kerr = krb5_init_context (&ctx); + assert_int_equal(kerr, 0); + + kerr = sssd_krb5_locator_init(ctx, &priv); + assert_int_equal(kerr, 0); + + mkdir(TEST_PUBCONF_PATH, 0777); + fd = open(TEST_PUBCONF_PATH"/kdcinfo."TEST_REALM, O_CREAT|O_RDWR, 0777); + assert_int_not_equal(fd, -1); + s = write(fd, TEST_IP_1_WITH_SERVICE, sizeof(TEST_IP_1_WITH_SERVICE)); + assert_int_equal(s, sizeof(TEST_IP_1_WITH_SERVICE)); + s = write(fd, "\n", 1); + assert_int_equal(s, 1); + s = write(fd, TEST_IPV6_1_WITH_SERVICE, sizeof(TEST_IPV6_1_WITH_SERVICE)); + assert_int_equal(s, sizeof(TEST_IPV6_1_WITH_SERVICE)); + close(fd); + + kerr = sssd_krb5_locator_lookup(priv, locate_service_kdc , TEST_REALM, + SOCK_DGRAM, AF_INET6, module_callback, + &cbdata); + assert_int_equal(kerr, 0); + + assert_int_equal(list.nservers, 1); + assert_non_null(list.servers); + ret = getnameinfo((struct sockaddr *) &list.servers[0].addr, + list.servers[0].addrlen, + host, sizeof(host), service, sizeof(service), + NI_NUMERICHOST|NI_NUMERICSERV); + assert_int_equal(ret, 0); + assert_string_equal(TEST_IPV6_1_PURE, host); + assert_string_equal(TEST_SERVICE_2, service); + + k5_free_serverlist(&list); + + kerr = sssd_krb5_locator_lookup(priv, locate_service_kdc , TEST_REALM, + SOCK_DGRAM, AF_INET, module_callback, + &cbdata); + assert_int_equal(kerr, 0); + assert_int_equal(list.nservers, 1); + assert_non_null(list.servers); + ret = getnameinfo((struct sockaddr *) &list.servers[0].addr, + list.servers[0].addrlen, + host, sizeof(host), service, sizeof(service), + NI_NUMERICHOST|NI_NUMERICSERV); + assert_int_equal(ret, 0); + assert_string_equal(TEST_IP_1, host); + assert_string_equal(TEST_SERVICE_1, service); + + k5_free_serverlist(&list); + + + unlink(TEST_PUBCONF_PATH"/kdcinfo."TEST_REALM); + rmdir(TEST_PUBCONF_PATH); + sssd_krb5_locator_close(priv); + + krb5_free_context(ctx); +} + +int main(int argc, const char *argv[]) +{ + poptContext pc; + int opt; + int ret; + struct poptOption long_options[] = { + POPT_AUTOHELP + SSSD_DEBUG_OPTS + POPT_TABLEEND + }; + + const struct CMUnitTest tests[] = { + cmocka_unit_test_setup_teardown(test_init, + setup, teardown), + cmocka_unit_test_setup_teardown(test_failed_lookup, + setup, teardown), + cmocka_unit_test_setup_teardown(test_empty, + setup, teardown), + cmocka_unit_test_setup_teardown(test_single, + setup, teardown), + cmocka_unit_test_setup_teardown(test_multi, + setup, teardown), + cmocka_unit_test_setup_teardown(test_service, + setup, teardown), + }; + + /* Set debug level to invalid value so we can decide if -d 0 was used. */ + debug_level = SSSDBG_INVALID; + + pc = poptGetContext(argv[0], argc, argv, long_options, 0); + while((opt = poptGetNextOpt(pc)) != -1) { + switch(opt) { + default: + fprintf(stderr, "\nInvalid option %s: %s\n\n", + poptBadOption(pc, 0), poptStrerror(opt)); + poptPrintUsage(pc, stderr, 0); + return 1; + } + } + poptFreeContext(pc); + + DEBUG_CLI_INIT(debug_level); + + ret = cmocka_run_group_tests(tests, NULL, NULL); + + return ret; +} From dc443c6fe790dba268d26334f3d041915278f07f Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Thu, 24 May 2018 17:15:38 +0200 Subject: [PATCH 5/8] AD/IPA: Create kdcinfo file for sub-domains With this patch kdcinfo files are created for sub-domains by the AD provider and by the IPA provider on the IPA servers (ipa_server_mode=True). Related to https://pagure.io/SSSD/sssd/issue/3652 --- src/providers/ad/ad_common.c | 9 +++++++++ src/providers/ad/ad_common.h | 1 + src/providers/ad/ad_init.c | 1 + src/providers/ad/ad_subdomains.c | 17 ++++++++++++++--- src/providers/ipa/ipa_subdomains_server.c | 16 ++++++++++++++-- 5 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c index be7791e6c..0aea985e0 100644 --- a/src/providers/ad/ad_common.c +++ b/src/providers/ad/ad_common.c @@ -727,6 +727,7 @@ ad_failover_init(TALLOC_CTX *mem_ctx, struct be_ctx *bectx, const char *ad_service, const char *ad_gc_service, const char *ad_domain, + bool use_kdcinfo, struct ad_service **_service) { errno_t ret; @@ -762,6 +763,14 @@ ad_failover_init(TALLOC_CTX *mem_ctx, struct be_ctx *bectx, goto done; } + /* Set flag that controls whether we want to write the + * kdcinfo files at all + */ + service->krb5_service->write_kdcinfo = use_kdcinfo; + DEBUG(SSSDBG_CONF_SETTINGS, "write_kdcinfo for realm %s set to %s\n", + krb5_realm, + service->krb5_service->write_kdcinfo ? "true" : "false"); + ret = be_fo_add_service(bectx, ad_service, ad_user_data_cmp); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "Failed to create failover service!\n"); diff --git a/src/providers/ad/ad_common.h b/src/providers/ad/ad_common.h index 6eb2ba7e9..dd440da33 100644 --- a/src/providers/ad/ad_common.h +++ b/src/providers/ad/ad_common.h @@ -144,6 +144,7 @@ ad_failover_init(TALLOC_CTX *mem_ctx, struct be_ctx *ctx, const char *ad_service, const char *ad_gc_service, const char *ad_domain, + bool use_kdcinfo, struct ad_service **_service); errno_t diff --git a/src/providers/ad/ad_init.c b/src/providers/ad/ad_init.c index b19624782..637efb761 100644 --- a/src/providers/ad/ad_init.c +++ b/src/providers/ad/ad_init.c @@ -159,6 +159,7 @@ static errno_t ad_init_options(TALLOC_CTX *mem_ctx, ret = ad_failover_init(ad_options, be_ctx, ad_servers, ad_backup_servers, ad_realm, AD_SERVICE_NAME, AD_GC_SERVICE_NAME, dp_opt_get_string(ad_options->basic, AD_DOMAIN), + false, /* will be set in ad_get_auth_options() */ &ad_options->service); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "Failed to init AD failover service: " diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c index 74b9f0751..84886e920 100644 --- a/src/providers/ad/ad_subdomains.c +++ b/src/providers/ad/ad_subdomains.c @@ -249,6 +249,7 @@ ad_subdom_ad_ctx_new(struct be_ctx *be_ctx, const char *hostname; const char *keytab; char *subdom_conf_path; + bool use_kdcinfo = false; realm = dp_opt_get_cstring(id_ctx->ad_options->basic, AD_KRB5_REALM); hostname = dp_opt_get_cstring(id_ctx->ad_options->basic, AD_HOSTNAME); @@ -296,9 +297,19 @@ ad_subdom_ad_ctx_new(struct be_ctx *be_ctx, servers = dp_opt_get_string(ad_options->basic, AD_SERVER); backup_servers = dp_opt_get_string(ad_options->basic, AD_BACKUP_SERVER); - ret = ad_failover_init(ad_options, be_ctx, servers, backup_servers, realm, - service_name, gc_service_name, - subdom->name, &ad_options->service); + if (id_ctx->ad_options->auth_ctx != NULL + && id_ctx->ad_options->auth_ctx->opts != NULL) { + use_kdcinfo = dp_opt_get_bool(id_ctx->ad_options->auth_ctx->opts, + KRB5_USE_KDCINFO); + } + + DEBUG(SSSDBG_TRACE_ALL, + "Init failover for [%s][%s] with use_kdcinfo [%s].\n", + subdom->name, subdom->realm, use_kdcinfo ? "true" : "false"); + + ret = ad_failover_init(ad_options, be_ctx, servers, backup_servers, + subdom->realm, service_name, gc_service_name, + subdom->name, use_kdcinfo, &ad_options->service); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Cannot initialize AD failover\n"); talloc_free(ad_options); diff --git a/src/providers/ipa/ipa_subdomains_server.c b/src/providers/ipa/ipa_subdomains_server.c index 1e53e7a95..02577c921 100644 --- a/src/providers/ipa/ipa_subdomains_server.c +++ b/src/providers/ipa/ipa_subdomains_server.c @@ -228,6 +228,7 @@ ipa_ad_ctx_new(struct be_ctx *be_ctx, struct sdap_domain *sdom; errno_t ret; const char *extra_attrs; + bool use_kdcinfo = false; ad_domain = subdom->name; DEBUG(SSSDBG_TRACE_LIBS, "Setting up AD subdomain %s\n", subdom->name); @@ -284,12 +285,23 @@ ipa_ad_ctx_new(struct be_ctx *be_ctx, ad_servers = dp_opt_get_string(ad_options->basic, AD_SERVER); ad_backup_servers = dp_opt_get_string(ad_options->basic, AD_BACKUP_SERVER); + if (id_ctx->ipa_options != NULL && id_ctx->ipa_options->auth != NULL) { + use_kdcinfo = dp_opt_get_bool(id_ctx->ipa_options->auth, + KRB5_USE_KDCINFO); + } + + DEBUG(SSSDBG_TRACE_ALL, + "Init failover for [%s][%s] with use_kdcinfo [%s].\n", + subdom->name, subdom->realm, use_kdcinfo ? "true" : "false"); + /* Set KRB5 realm to same as the one of IPA when IPA * is able to attach PAC. For testing, use hardcoded. */ + /* Why? */ ret = ad_failover_init(ad_options, be_ctx, ad_servers, ad_backup_servers, - id_ctx->server_mode->realm, + subdom->realm, service_name, gc_service_name, - subdom->name, &ad_options->service); + subdom->name, use_kdcinfo, + &ad_options->service); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Cannot initialize AD failover\n"); talloc_free(ad_options); From de4f40b6abd3fe791a957296b8e72516b2b30cf8 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Tue, 5 Jun 2018 17:44:59 +0200 Subject: [PATCH 6/8] krb5: refactor removal of krb5info files Currently a persistent offline callback removes the krb5info files for the configured main domain and those files were removed by a SIGTERM signal handlers as well. This does not scale if krb5info files are created for sub-domains as well. To remove the files automatically the removal is moved into a talloc destructor of an offline callback which is added if the file is created and frees itself when the system goes offline. Due to the talloc memory hierarchy we get removal on shutdown for free. Related to https://pagure.io/SSSD/sssd/issue/3652 --- src/providers/ad/ad_common.c | 7 +- src/providers/ipa/ipa_common.c | 5 +- src/providers/krb5/krb5_common.c | 176 +++++++++++++++++----------------- src/providers/krb5/krb5_common.h | 7 +- src/providers/krb5/krb5_init_shared.c | 6 -- src/providers/ldap/ldap_common.c | 87 ----------------- 6 files changed, 102 insertions(+), 186 deletions(-) diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c index 0aea985e0..8caaba6c0 100644 --- a/src/providers/ad/ad_common.c +++ b/src/providers/ad/ad_common.c @@ -804,6 +804,8 @@ ad_failover_init(TALLOC_CTX *mem_ctx, struct be_ctx *bectx, goto done; } + service->krb5_service->be_ctx = bectx; + if (!primary_servers) { DEBUG(SSSDBG_CONF_SETTINGS, "No primary servers defined, using service discovery\n"); @@ -984,8 +986,9 @@ ad_resolve_callback(void *private_data, struct fo_server *server) goto done; } - ret = write_krb5info_file(service->krb5_service->realm, safe_address, - SSS_KRB5KDC_FO_SRV); + ret = write_krb5info_file(service->krb5_service, + safe_address, + SSS_KRB5KDC_FO_SRV); if (ret != EOK) { DEBUG(SSSDBG_MINOR_FAILURE, "write_krb5info_file failed, authentication might fail.\n"); diff --git a/src/providers/ipa/ipa_common.c b/src/providers/ipa/ipa_common.c index 87ed96767..dcbb54a74 100644 --- a/src/providers/ipa/ipa_common.c +++ b/src/providers/ipa/ipa_common.c @@ -838,7 +838,8 @@ static void ipa_resolve_callback(void *private_data, struct fo_server *server) return; } - ret = write_krb5info_file(service->krb5_service->realm, safe_address, + ret = write_krb5info_file(service->krb5_service, + safe_address, SSS_KRB5KDC_FO_SRV); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, @@ -1012,6 +1013,8 @@ int ipa_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx, goto done; } + service->krb5_service->be_ctx = ctx; + if (!primary_servers) { DEBUG(SSSDBG_CONF_SETTINGS, "No primary servers defined, using service discovery\n"); diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c index 520e7591c..c6896a6cd 100644 --- a/src/providers/krb5/krb5_common.c +++ b/src/providers/krb5/krb5_common.c @@ -389,7 +389,76 @@ errno_t sss_krb5_get_options(TALLOC_CTX *memctx, struct confdb_ctx *cdb, return ret; } -errno_t write_krb5info_file(const char *realm, const char *server, +static int remove_info_files_destructor(void *p) +{ + int ret; + struct remove_info_files_ctx *ctx = talloc_get_type(p, + struct remove_info_files_ctx); + + ret = remove_krb5_info_files(ctx, ctx->realm); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, "remove_krb5_info_files failed.\n"); + } + + return 0; +} + +static errno_t +krb5_add_krb5info_offline_callback(struct krb5_service *krb5_service) +{ + int ret; + struct remove_info_files_ctx *ctx; + + if (krb5_service == NULL || krb5_service->name == NULL + || krb5_service->realm == NULL + || krb5_service->be_ctx == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "Missing KDC service name or realm!\n"); + return EINVAL; + } + + ctx = talloc_zero(krb5_service->be_ctx, struct remove_info_files_ctx); + if (ctx == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zfree failed.\n"); + return ENOMEM; + } + + ctx->realm = talloc_strdup(ctx, krb5_service->realm); + if (ctx->realm == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup failed!\n"); + ret = ENOMEM; + goto done; + } + + ctx->be_ctx = krb5_service->be_ctx; + ctx->kdc_service_name = talloc_strdup(ctx, krb5_service->name); + if (ctx->kdc_service_name == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup failed!\n"); + ret = ENOMEM; + goto done; + } + + ret = be_add_offline_cb(ctx, krb5_service->be_ctx, + remove_krb5_info_files_callback, ctx, NULL); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, "be_add_offline_cb failed.\n"); + goto done; + } + + talloc_set_destructor((TALLOC_CTX *) ctx, remove_info_files_destructor); + + ret = EOK; + +done: + if (ret != EOK) { + talloc_zfree(ctx); + } + + return ret; +} + + +errno_t write_krb5info_file(struct krb5_service *krb5_service, + const char *server, const char *service) { int ret; @@ -401,17 +470,19 @@ errno_t write_krb5info_file(const char *realm, const char *server, size_t server_len; ssize_t written; - if (realm == NULL || *realm == '\0' || server == NULL || *server == '\0' || - service == NULL || *service == '\0') { + if (krb5_service == NULL || krb5_service->realm == NULL + || *krb5_service->realm == '\0' + || server == NULL || *server == '\0' + || service == NULL || *service == '\0') { DEBUG(SSSDBG_CRIT_FAILURE, "Missing or empty realm, server or service.\n"); return EINVAL; } - if (sss_krb5_realm_has_proxy(realm)) { + if (sss_krb5_realm_has_proxy(krb5_service->realm)) { DEBUG(SSSDBG_CONF_SETTINGS, "KDC Proxy available for realm [%s], no kdcinfo file created.\n", - realm); + krb5_service->realm); return EOK; } @@ -439,7 +510,7 @@ errno_t write_krb5info_file(const char *realm, const char *server, goto done; } - krb5info_name = talloc_asprintf(tmp_ctx, name_tmpl, realm); + krb5info_name = talloc_asprintf(tmp_ctx, name_tmpl, krb5_service->realm); if (krb5info_name == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, "talloc_asprintf failed.\n"); ret = ENOMEM; @@ -495,6 +566,12 @@ errno_t write_krb5info_file(const char *realm, const char *server, goto done; } + ret = krb5_add_krb5info_offline_callback(krb5_service); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "Failed to add offline callback, krb5info " + "file might not be removed properly.\n"); + } + ret = EOK; done: if (fd != -1) { @@ -561,7 +638,8 @@ static void krb5_resolve_callback(void *private_data, struct fo_server *server) return; } - ret = write_krb5info_file(krb5_service->realm, safe_address, + ret = write_krb5info_file(krb5_service, + safe_address, krb5_service->name); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, @@ -761,6 +839,7 @@ int krb5_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx, } service->write_kdcinfo = use_kdcinfo; + service->be_ctx = ctx; if (!primary_servers) { DEBUG(SSSDBG_CONF_SETTINGS, @@ -839,7 +918,6 @@ errno_t remove_krb5_info_files(TALLOC_CTX *mem_ctx, const char *realm) void remove_krb5_info_files_callback(void *pvt) { int ret; - TALLOC_CTX *tmp_ctx = NULL; struct remove_info_files_ctx *ctx = talloc_get_type(pvt, struct remove_info_files_ctx); @@ -864,19 +942,10 @@ void remove_krb5_info_files_callback(void *pvt) } } - tmp_ctx = talloc_new(NULL); - if (tmp_ctx == NULL) { - DEBUG(SSSDBG_CRIT_FAILURE, - "talloc_new failed, cannot remove krb5 info files.\n"); - return; - } - - ret = remove_krb5_info_files(tmp_ctx, ctx->realm); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, "remove_krb5_info_files failed.\n"); - } - - talloc_zfree(tmp_ctx); + /* Freeing the remove_info_files_ctx will remove the related krb5info + * file. Additionally the callback from the list of callbacks is removed, + * it will be added again when a new krb5info file is created. */ + talloc_free(ctx); } void krb5_finalize(struct tevent_context *ev, @@ -886,74 +955,9 @@ void krb5_finalize(struct tevent_context *ev, void *siginfo, void *private_data) { - char *realm = (char *)private_data; - int ret; - - ret = remove_krb5_info_files(se, realm); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, "remove_krb5_info_files failed.\n"); - } - orderly_shutdown(0); } -errno_t krb5_install_offline_callback(struct be_ctx *be_ctx, - struct krb5_ctx *krb5_ctx) -{ - int ret; - struct remove_info_files_ctx *ctx; - const char *krb5_realm; - - if (krb5_ctx->service == NULL || krb5_ctx->service->name == NULL) { - DEBUG(SSSDBG_CRIT_FAILURE, "Missing KDC service name!\n"); - return EINVAL; - } - - ctx = talloc_zero(krb5_ctx, struct remove_info_files_ctx); - if (ctx == NULL) { - DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zfree failed.\n"); - return ENOMEM; - } - - krb5_realm = dp_opt_get_cstring(krb5_ctx->opts, KRB5_REALM); - if (krb5_realm == NULL) { - DEBUG(SSSDBG_CRIT_FAILURE, "Missing krb5_realm option!\n"); - ret = EINVAL; - goto done; - } - - ctx->realm = talloc_strdup(ctx, krb5_realm); - if (ctx->realm == NULL) { - DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup failed!\n"); - ret = ENOMEM; - goto done; - } - - ctx->be_ctx = be_ctx; - ctx->kdc_service_name = krb5_ctx->service->name; - if (krb5_ctx->kpasswd_service == NULL) { - ctx->kpasswd_service_name =NULL; - } else { - ctx->kpasswd_service_name = krb5_ctx->kpasswd_service->name; - } - - ret = be_add_offline_cb(ctx, be_ctx, remove_krb5_info_files_callback, ctx, - NULL); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, "be_add_offline_cb failed.\n"); - goto done; - } - - ret = EOK; - -done: - if (ret != EOK) { - talloc_zfree(ctx); - } - - return ret; -} - errno_t krb5_install_sigterm_handler(struct tevent_context *ev, struct krb5_ctx *krb5_ctx) { diff --git a/src/providers/krb5/krb5_common.h b/src/providers/krb5/krb5_common.h index 48368a528..a2e47b060 100644 --- a/src/providers/krb5/krb5_common.h +++ b/src/providers/krb5/krb5_common.h @@ -67,6 +67,7 @@ enum krb5_opts { typedef enum { INIT_PW, INIT_KT, RENEW, VALIDATE } action_type; struct krb5_service { + struct be_ctx *be_ctx; char *name; char *realm; bool write_kdcinfo; @@ -157,7 +158,8 @@ errno_t krb5_try_kdcip(struct confdb_ctx *cdb, const char *conf_path, errno_t sss_krb5_get_options(TALLOC_CTX *memctx, struct confdb_ctx *cdb, const char *conf_path, struct dp_option **_opts); -errno_t write_krb5info_file(const char *realm, const char *kdc, +errno_t write_krb5info_file(struct krb5_service *krb5_service, + const char *server, const char *service); int krb5_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx, @@ -177,9 +179,6 @@ void krb5_finalize(struct tevent_context *ev, void *siginfo, void *private_data); -errno_t krb5_install_offline_callback(struct be_ctx *be_ctx, - struct krb5_ctx *krb_ctx); - errno_t krb5_install_sigterm_handler(struct tevent_context *ev, struct krb5_ctx *krb5_ctx); diff --git a/src/providers/krb5/krb5_init_shared.c b/src/providers/krb5/krb5_init_shared.c index 3901b7272..368d6f7b0 100644 --- a/src/providers/krb5/krb5_init_shared.c +++ b/src/providers/krb5/krb5_init_shared.c @@ -71,12 +71,6 @@ errno_t krb5_child_init(struct krb5_ctx *krb5_auth_ctx, goto done; } - ret = krb5_install_offline_callback(bectx, krb5_auth_ctx); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, "krb5_install_offline_callback failed.\n"); - goto done; - } - ret = krb5_install_sigterm_handler(bectx->ev, krb5_auth_ctx); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "krb5_install_sigterm_handler failed.\n"); diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c index 91e229243..15377ee1f 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -158,14 +158,6 @@ static void sdap_finalize(struct tevent_context *ev, void *siginfo, void *private_data) { - char *realm = (char *) private_data; - int ret; - - ret = remove_krb5_info_files(se, realm); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, "remove_krb5_info_files failed.\n"); - } - orderly_shutdown(0); } @@ -196,78 +188,6 @@ errno_t sdap_install_sigterm_handler(TALLOC_CTX *mem_ctx, return EOK; } -void sdap_remove_kdcinfo_files_callback(void *pvt) -{ - int ret; - TALLOC_CTX *tmp_ctx = NULL; - struct remove_info_files_ctx *ctx = talloc_get_type(pvt, - struct remove_info_files_ctx); - - ret = be_fo_run_callbacks_at_next_request(ctx->be_ctx, - ctx->kdc_service_name); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, - "be_fo_run_callbacks_at_next_request failed, " - "krb5 info files will not be removed, because " - "it is unclear if they will be recreated properly.\n"); - return; - } - - tmp_ctx = talloc_new(NULL); - if (tmp_ctx == NULL) { - DEBUG(SSSDBG_CRIT_FAILURE, - "talloc_new failed, cannot remove krb5 info files.\n"); - return; - } - - ret = remove_krb5_info_files(tmp_ctx, ctx->realm); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, "remove_krb5_info_files failed.\n"); - } - - talloc_zfree(tmp_ctx); -} - - -errno_t sdap_install_offline_callback(TALLOC_CTX *mem_ctx, - struct be_ctx *be_ctx, - const char *realm, - const char *service_name) -{ - int ret; - struct remove_info_files_ctx *ctx; - - ctx = talloc_zero(mem_ctx, struct remove_info_files_ctx); - if (ctx == NULL) { - DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zfree failed.\n"); - return ENOMEM; - } - - ctx->be_ctx = be_ctx; - ctx->realm = talloc_strdup(ctx, realm); - ctx->kdc_service_name = talloc_strdup(ctx, service_name); - if (ctx->realm == NULL || ctx->kdc_service_name == NULL) { - DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup failed!\n"); - ret = ENOMEM; - goto done; - } - - ret = be_add_offline_cb(ctx, be_ctx, - sdap_remove_kdcinfo_files_callback, - ctx, NULL); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, "be_add_offline_cb failed.\n"); - goto done; - } - - ret = EOK; -done: - if (ret != EOK) { - talloc_zfree(ctx); - } - return ret; -} - errno_t sdap_set_sasl_options(struct sdap_options *id_opts, char *default_primary, @@ -458,13 +378,6 @@ int sdap_gssapi_init(TALLOC_CTX *mem_ctx, goto done; } - ret = sdap_install_offline_callback(mem_ctx, bectx, - krb5_realm, SSS_KRB5KDC_FO_SRV); - if (ret != EOK) { - DEBUG(SSSDBG_FATAL_FAILURE, "Failed to install sigterm handler\n"); - goto done; - } - sdap_service->kinit_service_name = talloc_strdup(sdap_service, service->name); if (sdap_service->kinit_service_name == NULL) { From 4dd0885c833a6bf5345130180c076af751f932b2 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Fri, 8 Jun 2018 08:29:04 +0200 Subject: [PATCH 7/8] add callback only once --- src/providers/krb5/krb5_common.c | 12 +++++++++++- src/providers/krb5/krb5_common.h | 2 ++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c index c6896a6cd..d064a09ac 100644 --- a/src/providers/krb5/krb5_common.c +++ b/src/providers/krb5/krb5_common.c @@ -399,6 +399,7 @@ static int remove_info_files_destructor(void *p) if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "remove_krb5_info_files failed.\n"); } + ctx->krb5_service->removal_callback_available = false; return 0; } @@ -407,7 +408,7 @@ static errno_t krb5_add_krb5info_offline_callback(struct krb5_service *krb5_service) { int ret; - struct remove_info_files_ctx *ctx; + struct remove_info_files_ctx *ctx = NULL; if (krb5_service == NULL || krb5_service->name == NULL || krb5_service->realm == NULL @@ -416,6 +417,13 @@ krb5_add_krb5info_offline_callback(struct krb5_service *krb5_service) return EINVAL; } + if (krb5_service->removal_callback_available) { + DEBUG(SSSDBG_TRACE_ALL, + "Removal callback already available for service [%s].\n", + krb5_service->name); + return EOK; + } + ctx = talloc_zero(krb5_service->be_ctx, struct remove_info_files_ctx); if (ctx == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zfree failed.\n"); @@ -430,6 +438,7 @@ krb5_add_krb5info_offline_callback(struct krb5_service *krb5_service) } ctx->be_ctx = krb5_service->be_ctx; + ctx->krb5_service = krb5_service; ctx->kdc_service_name = talloc_strdup(ctx, krb5_service->name); if (ctx->kdc_service_name == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup failed!\n"); @@ -445,6 +454,7 @@ krb5_add_krb5info_offline_callback(struct krb5_service *krb5_service) } talloc_set_destructor((TALLOC_CTX *) ctx, remove_info_files_destructor); + krb5_service->removal_callback_available = true; ret = EOK; diff --git a/src/providers/krb5/krb5_common.h b/src/providers/krb5/krb5_common.h index a2e47b060..3529d740b 100644 --- a/src/providers/krb5/krb5_common.h +++ b/src/providers/krb5/krb5_common.h @@ -71,6 +71,7 @@ struct krb5_service { char *name; char *realm; bool write_kdcinfo; + bool removal_callback_available; }; struct fo_service; @@ -146,6 +147,7 @@ struct remove_info_files_ctx { struct be_ctx *be_ctx; const char *kdc_service_name; const char *kpasswd_service_name; + struct krb5_service *krb5_service; }; errno_t sss_krb5_check_options(struct dp_option *opts, From 2fd67a33863ee8de2b3fd2533ce7e2bea924d8c1 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Fri, 8 Jun 2018 18:42:28 +0200 Subject: [PATCH 8/8] data provider: run offline callbacks only once --- src/providers/backend.h | 1 + src/providers/data_provider_be.c | 1 + src/providers/data_provider_callbacks.c | 36 +++++++++++++++++++++++++-------- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/providers/backend.h b/src/providers/backend.h index 191427403..6a34b91a9 100644 --- a/src/providers/backend.h +++ b/src/providers/backend.h @@ -95,6 +95,7 @@ struct be_ctx { struct be_cb *online_cb_list; bool run_online_cb; struct be_cb *offline_cb_list; + bool run_offline_cb; struct be_cb *reconnect_cb_list; /* In contrast to online_cb_list which are only run if the backend is * offline the unconditional_online_cb_list should be run whenever the diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index e8cddd976..fad6f2801 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -219,6 +219,7 @@ static void be_reset_offline(struct be_ctx *ctx) { ctx->offstat.went_offline = 0; ctx->offstat.offline = false; + ctx->run_offline_cb = true; reactivate_subdoms(ctx->domain); diff --git a/src/providers/data_provider_callbacks.c b/src/providers/data_provider_callbacks.c index 436357e22..24e125ea5 100644 --- a/src/providers/data_provider_callbacks.c +++ b/src/providers/data_provider_callbacks.c @@ -265,22 +265,42 @@ void be_run_unconditional_online_cb(struct be_ctx *be) int be_add_offline_cb(TALLOC_CTX *mem_ctx, struct be_ctx *ctx, be_callback_t cb, void *pvt, struct be_cb **offline_cb) { - return be_add_cb(mem_ctx, ctx, cb, pvt, &ctx->offline_cb_list, offline_cb); + int ret; + + ret = be_add_cb(mem_ctx, ctx, cb, pvt, &ctx->offline_cb_list, offline_cb); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, "be_add_cb failed.\n"); + return ret; + } + + /* Make sure we run the callback when SSSD goes offline */ + ctx->run_offline_cb = true; + + return EOK; } void be_run_offline_cb(struct be_ctx *be) { int ret; - if (be->offline_cb_list) { - DEBUG(SSSDBG_MINOR_FAILURE, "Going offline. Running callbacks.\n"); + if (be->run_offline_cb) { + /* Reset the flag, we only want to run these callbacks once when going + * offline */ + be->run_offline_cb = false; - ret = be_run_cb(be, be->offline_cb_list); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, "be_run_cb failed.\n"); - } + if (be->offline_cb_list) { + DEBUG(SSSDBG_MINOR_FAILURE, "Going offline. Running callbacks.\n"); + ret = be_run_cb(be, be->offline_cb_list); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, "be_run_cb failed.\n"); + } + + } else { + DEBUG(SSSDBG_TRACE_ALL, + "Offline call back list is empty, nothing to do.\n"); + } } else { DEBUG(SSSDBG_TRACE_ALL, - "Offline call back list is empty, nothing to do.\n"); + "Flag indicates that offline callback were already called.\n"); } }