From b1aca931e9c9cfb8d40b00b0bcaac657caa81d68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Mon, 25 Jun 2018 09:36:46 +0200 Subject: [PATCH] Resolves: upstream#3766 - CVE-2018-10852: information leak from the sssd-sudo responder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit And also ... - Related: upstream#941 - return multiple server addresses to the Kerberos locator plugin - Related: upstream#3652 - kdcinfo doesn't get populated for other domains - Resolves: upstream#3747 - sss_ssh_authorizedkeys exits abruptly if SSHD closes its end of the pipe before reading all the SSH keys - Resolves: upstream#3607 - Handle conflicting e-mail addresses more gracefully - Resolves: upstream#3754 - SSSD AD uses LDAP filter to detect POSIX attributes stored in AD GC also for regular AD DC queries - Related: upstream#3219 - [RFE] Regular expression used in sssd.conf not being able to consume an @-sign in the user/group name. Signed-off-by: Fabiano Fidêncio (cherry picked from commit 68ef824a5fc441c038a6791b7ef39941357a548b) (cherry picked from commit f311832a06a201ae94d57d80b9e1e84231208cd7) --- ...r-add-support-for-multiple-addresses.patch | 468 +++++++++++ 0002-krb5-locator-fix-IPv6-support.patch | 65 ++ ...krb5-locator-make-plugin-more-robust.patch | 35 + 0004-krb5-locator-add-unit-tests.patch | 724 ++++++++++++++++++ ...-Create-kdcinfo-file-for-sub-domains.patch | 148 ++++ ...5-refactor-removal-of-krb5info-files.patch | 493 ++++++++++++ 0007-krb5_common-add-callback-only-once.patch | 86 +++ ...ider-run-offline-callbacks-only-once.patch | 95 +++ ...e-schema-with-sshPublicKey-attribute.patch | 62 ++ ...-Allow-adding-sshPublicKey-for-users.patch | 78 ++ ...TESTS-Add-a-basic-SSH-responder-test.patch | 276 +++++++ ...abruptly-if-SSHD-closes-its-end-of-t.patch | 88 +++ ...er-binary-that-can-trigger-the-SIGPI.patch | 213 ++++++ ...ession-test-for-SIGHUP-handling-in-s.patch | 94 +++ ...A-add-local-email-address-to-aliases.patch | 141 ++++ ...unused-function-is_email_from_domain.patch | 117 +++ ...low-storing-e-mail-address-for-users.patch | 65 ++ ...sion-test-for-looking-up-users-with-.patch | 93 +++ ...ted-notes-from-the-re_expression-des.patch | 47 ++ ...the-socket-with-stricter-permissions.patch | 55 ++ sssd.spec | 39 +- 21 files changed, 3480 insertions(+), 2 deletions(-) create mode 100644 0001-krb5-locator-add-support-for-multiple-addresses.patch create mode 100644 0002-krb5-locator-fix-IPv6-support.patch create mode 100644 0003-krb5-locator-make-plugin-more-robust.patch create mode 100644 0004-krb5-locator-add-unit-tests.patch create mode 100644 0005-AD-IPA-Create-kdcinfo-file-for-sub-domains.patch create mode 100644 0006-krb5-refactor-removal-of-krb5info-files.patch create mode 100644 0007-krb5_common-add-callback-only-once.patch create mode 100644 0008-data-provider-run-offline-callbacks-only-once.patch create mode 100644 0009-TESTS-Extend-the-schema-with-sshPublicKey-attribute.patch create mode 100644 0010-TESTS-Allow-adding-sshPublicKey-for-users.patch create mode 100644 0011-TESTS-Add-a-basic-SSH-responder-test.patch create mode 100644 0012-SSH-Do-not-exit-abruptly-if-SSHD-closes-its-end-of-t.patch create mode 100644 0013-TESTS-Add-a-helper-binary-that-can-trigger-the-SIGPI.patch create mode 100644 0014-TESTS-Add-a-regression-test-for-SIGHUP-handling-in-s.patch create mode 100644 0015-Revert-LDAP-IPA-add-local-email-address-to-aliases.patch create mode 100644 0016-util-Remove-the-unused-function-is_email_from_domain.patch create mode 100644 0017-TESTS-Allow-storing-e-mail-address-for-users.patch create mode 100644 0018-TESTS-Add-regression-test-for-looking-up-users-with-.patch create mode 100644 0019-MAN-Remove-outdated-notes-from-the-re_expression-des.patch create mode 100644 0020-SUDO-Create-the-socket-with-stricter-permissions.patch diff --git a/0001-krb5-locator-add-support-for-multiple-addresses.patch b/0001-krb5-locator-add-support-for-multiple-addresses.patch new file mode 100644 index 0000000..747c2f2 --- /dev/null +++ b/0001-krb5-locator-add-support-for-multiple-addresses.patch @@ -0,0 +1,468 @@ +From 4b1137562c3446e85a6383010702850f9532a4f2 Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Fri, 24 Feb 2017 13:55:47 +0100 +Subject: [PATCH] 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 +Reviewed-by: Jakub Hrozek + +(cherry picked from commit efae9509cb05648357e9b4c10a93c0d38558bed4) + +DOWNSTREAM: +Resolves: rhbz#1494690 - kdcinfo files are not created for subdomains of a directly joined AD client +--- + src/krb5_plugin/sssd_krb5_locator_plugin.c | 344 +++++++++++++++------ + 1 file changed, 246 insertions(+), 98 deletions(-) + +diff --git a/src/krb5_plugin/sssd_krb5_locator_plugin.c b/src/krb5_plugin/sssd_krb5_locator_plugin.c +index 7c17fcb33373293fbbbe2be967dca57b31ef13de..82fb5c7b2ffa319ed250e54cdf9a0b6798d4ff51 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; +- } +- +- 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))); ++ 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; + } +- return KRB5_PLUGIN_NO_HANDLE; +- } + +- PLUGIN_DEBUG(("addr[%s:%s] family[%d] socktype[%d]\n", addr, port_str, +- ai->ai_family, ai->ai_socktype)); ++ memset(&ai_hints, 0, sizeof(struct addrinfo)); ++ ai_hints.ai_flags = AI_NUMERICHOST|AI_NUMERICSERV; ++ ai_hints.ai_socktype = socktype; + +- if ((family == AF_UNSPEC || ai->ai_family == family) && +- ai->ai_socktype == socktype) { +- +- ret = cbfunc(cbdata, socktype, ai->ai_addr); ++ ret = getaddrinfo(addr[c].addr, port_str, &ai_hints, &ai); + if (ret != 0) { +- PLUGIN_DEBUG(("cbfunc failed\n")); +- freeaddrinfo(ai); +- return ret; ++ 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; ++ } ++ ++ 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) { ++ ++ 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; + } +-- +2.17.1 + diff --git a/0002-krb5-locator-fix-IPv6-support.patch b/0002-krb5-locator-fix-IPv6-support.patch new file mode 100644 index 0000000..f602116 --- /dev/null +++ b/0002-krb5-locator-fix-IPv6-support.patch @@ -0,0 +1,65 @@ +From 45a48b9a73f39e9ef9e622dbcf87cc05a2a54e40 Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Tue, 22 May 2018 17:59:52 +0200 +Subject: [PATCH] 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 +Reviewed-by: Jakub Hrozek + +(cherry picked from commit 9f683246228848173c57ad02bde241bd761481ea) +--- + 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 82fb5c7b2ffa319ed250e54cdf9a0b6798d4ff51..58cac7f4b244903347e6f1811cd8de2d61281c4f 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++; ++ } + +- l[c].addr = strdup(tmp); ++ PLUGIN_DEBUG(("Found [%s][%d].\n", addr_str, port)); ++ ++ l[c].addr = strdup(addr_str); + if (l[c].addr == NULL) { + ret = ENOMEM; + goto done; +-- +2.17.1 + diff --git a/0003-krb5-locator-make-plugin-more-robust.patch b/0003-krb5-locator-make-plugin-more-robust.patch new file mode 100644 index 0000000..fb596fb --- /dev/null +++ b/0003-krb5-locator-make-plugin-more-robust.patch @@ -0,0 +1,35 @@ +From 4e851d1391f56c632c271fd21dd96f29565cadfe Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Tue, 22 May 2018 18:03:05 +0200 +Subject: [PATCH] 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 +Reviewed-by: Jakub Hrozek + +(cherry picked from commit c1fbc6b64ecaf51efc4379c4c8a4960de095abf0) +--- + 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 58cac7f4b244903347e6f1811cd8de2d61281c4f..9874fd2d1ce63b69099f057dd05f6e353a12ce75 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; +-- +2.17.1 + diff --git a/0004-krb5-locator-add-unit-tests.patch b/0004-krb5-locator-add-unit-tests.patch new file mode 100644 index 0000000..28fe091 --- /dev/null +++ b/0004-krb5-locator-add-unit-tests.patch @@ -0,0 +1,724 @@ +From 3d6b8b306cdbd4ec15b36a1e7936d219204e08dc Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Thu, 24 May 2018 17:14:42 +0200 +Subject: [PATCH] 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 +Reviewed-by: Jakub Hrozek + +(cherry picked from commit 2124275fe494a0241a552538c70f40c2291f3795) +--- + Makefile.am | 20 + + src/krb5_plugin/sssd_krb5_locator_plugin.c | 16 + + .../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 9539b3cff8544cf406e3e19ab23e76e9cc8234ee..9055130ed74057987795285c243ff47584cf8316 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 9874fd2d1ce63b69099f057dd05f6e353a12ce75..952d487c276ed51e0c3a018b0d0af59ca214525f 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 0000000000000000000000000000000000000000..3e7d00632ddb59da5474c0544eee6fc67edc5570 +--- /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; ++} +-- +2.17.1 + diff --git a/0005-AD-IPA-Create-kdcinfo-file-for-sub-domains.patch b/0005-AD-IPA-Create-kdcinfo-file-for-sub-domains.patch new file mode 100644 index 0000000..f6d7cd8 --- /dev/null +++ b/0005-AD-IPA-Create-kdcinfo-file-for-sub-domains.patch @@ -0,0 +1,148 @@ +From 660ef95e36ad73b4715656a4207aeb499ac96d16 Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Thu, 24 May 2018 17:15:38 +0200 +Subject: [PATCH] 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 +Reviewed-by: Jakub Hrozek + +(cherry picked from commit cc7922755dac53c69558ba060b309ac48ae82783) +--- + 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 be7791e6cc2527d45d3e2ff50294f9b98106ffae..0aea985e00faa996643fd7e7630d4264fb6cf233 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 6eb2ba7e9a7350d1924c45d33d8c332073767a34..dd440da33d48a5820c665f43908d1e1fb18171a6 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 b19624782000c5c7c65e766e3e01ff6ac3ab7adb..637efb761c1cf87b0a2c2b1c19b00ea0bbbe161f 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 74b9f075174b1eaa6c5b5dcbaf609600ef197b52..84886e920b37f8803d85ce0903b74e6c809a8904 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 1e53e7a951189120fcf3f438362e902a5a8f6d97..02577c92159d099a04cbd5cee80064309466db93 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); +-- +2.17.1 + diff --git a/0006-krb5-refactor-removal-of-krb5info-files.patch b/0006-krb5-refactor-removal-of-krb5info-files.patch new file mode 100644 index 0000000..9787c85 --- /dev/null +++ b/0006-krb5-refactor-removal-of-krb5info-files.patch @@ -0,0 +1,493 @@ +From 713bc782502163251ef22eb81b09eed61a8407f7 Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Tue, 5 Jun 2018 17:44:59 +0200 +Subject: [PATCH] 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 +Reviewed-by: Jakub Hrozek + +(cherry picked from commit d91661e295c8e878f1bbf34e6f65f61e8301bf0e) +--- + 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 0aea985e00faa996643fd7e7630d4264fb6cf233..8caaba6c0d06cfe83d9741536192d662fc936273 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 87ed967673358bf833dae13c29b1f6a17b0fc19c..dcbb54a744358718e444972b9827ee64887e5e33 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 520e7591ce1b37b4a8dea357b6dd0ec7afd76f58..c6896a6cd663da896075e72aa0a0602c198b45e8 100644 +--- a/src/providers/krb5/krb5_common.c ++++ b/src/providers/krb5/krb5_common.c +@@ -389,7 +389,76 @@ done: + 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 48368a528e75947102c74cb75bf7a74ec0dd258f..a2e47b0605debdffa28305dab4f7674707f713ac 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 3901b7272119c32930c2b6b47279a2c685bf3cfb..368d6f7b0f2bc038e4cc4aa8f0970cd0e81d7b6b 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 91e229243b9a1b43e7a57704824f5db0341f4ee9..15377ee1f062c0167aabee30ef0757ebe7271682 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) { +-- +2.17.1 + diff --git a/0007-krb5_common-add-callback-only-once.patch b/0007-krb5_common-add-callback-only-once.patch new file mode 100644 index 0000000..2b36703 --- /dev/null +++ b/0007-krb5_common-add-callback-only-once.patch @@ -0,0 +1,86 @@ +From 54ea4576ba8cb8dfbefdd3ced29fc35f836afc61 Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Fri, 8 Jun 2018 08:29:04 +0200 +Subject: [PATCH] krb5_common: add callback only once + +Reviewed-by: Jakub Hrozek +(cherry picked from commit 4759a482781bcecdb0ad1119e74dcefa1fe94337) +--- + 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 c6896a6cd663da896075e72aa0a0602c198b45e8..d064a09ac3726c4185c2fa1eeac76ef6c261d33b 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 a2e47b0605debdffa28305dab4f7674707f713ac..3529d740b89fee91281f936fdafd1bdb99e95bd7 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, +-- +2.17.1 + diff --git a/0008-data-provider-run-offline-callbacks-only-once.patch b/0008-data-provider-run-offline-callbacks-only-once.patch new file mode 100644 index 0000000..fd7a973 --- /dev/null +++ b/0008-data-provider-run-offline-callbacks-only-once.patch @@ -0,0 +1,95 @@ +From 2d350235bc960a91233d29b97c3a205bd2e04c08 Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Fri, 8 Jun 2018 18:42:28 +0200 +Subject: [PATCH] data provider: run offline callbacks only once + +Reviewed-by: Jakub Hrozek +(cherry picked from commit f28d995719db632130e9e063cb1ab7cb4e0fc8d8) +--- + 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 1914274037ce7f7ff4b6d8486b041789a865fd59..6a34b91a911fc12163fa9448ea82ff93f5bf3849 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 e8cddd976bb164dc6d4655bf2ebe9a03c3d9d26a..fad6f280195b615d1de45afaf0c459bdf78c8c0a 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 436357e228c0e1a689aa18b8ef41a82f63774d3a..24e125ea5be70208d7cf2cb06a80c39207e29db4 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"); + } + } +-- +2.17.1 + diff --git a/0009-TESTS-Extend-the-schema-with-sshPublicKey-attribute.patch b/0009-TESTS-Extend-the-schema-with-sshPublicKey-attribute.patch new file mode 100644 index 0000000..3cb7eda --- /dev/null +++ b/0009-TESTS-Extend-the-schema-with-sshPublicKey-attribute.patch @@ -0,0 +1,62 @@ +From 2b210b10ce54f6f2595f6ab181a51bce367d43a9 Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Sun, 17 Jun 2018 21:48:36 +0200 +Subject: [PATCH] TESTS: Extend the schema with sshPublicKey attribute +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +This will allow to store the users with a sshPublicKey attribute +provided that they have the right objectclass as well. + +Related to: +https://pagure.io/SSSD/sssd/issue/3747 + +Reviewed-by: Fabiano Fidêncio +(cherry picked from commit 1575ec97e080656f69b3f93e641c76e74ffb8182) + +DOWNSTREAM: +Resolves: rhbz#1583343 - Login with sshkeys stored in ipa not working after update to RHEL-7.5 +--- + src/tests/intg/data/ssh_schema.ldif | 11 +++++++++++ + src/tests/intg/ds_openldap.py | 6 ++++++ + 2 files changed, 17 insertions(+) + create mode 100644 src/tests/intg/data/ssh_schema.ldif + +diff --git a/src/tests/intg/data/ssh_schema.ldif b/src/tests/intg/data/ssh_schema.ldif +new file mode 100644 +index 0000000000000000000000000000000000000000..efe05706b9ded5614a7f3f5e0bab28a7eb869daa +--- /dev/null ++++ b/src/tests/intg/data/ssh_schema.ldif +@@ -0,0 +1,11 @@ ++dn: cn=openssh-lpk,cn=schema,cn=config ++objectClass: olcSchemaConfig ++cn: openssh-lpk ++olcAttributeTypes: ( 1.3.6.1.4.1.24552.500.1.1.1.13 NAME 'sshPublicKey' ++ DESC 'MANDATORY: OpenSSH Public key' ++ EQUALITY octetStringMatch ++ SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 ) ++olcObjectClasses: ( 1.3.6.1.4.1.24552.500.1.1.2.0 NAME 'ldapPublicKey' SUP top AUXILIARY ++ DESC 'MANDATORY: OpenSSH LPK objectclass' ++ MAY ( sshPublicKey $ uid ) ++ ) +diff --git a/src/tests/intg/ds_openldap.py b/src/tests/intg/ds_openldap.py +index 842ff910803658834841c8f9181f3c4af29b955a..c9a4b6de8c53c6644b3de9047d657ee35ce06512 100644 +--- a/src/tests/intg/ds_openldap.py ++++ b/src/tests/intg/ds_openldap.py +@@ -186,6 +186,12 @@ class DSOpenLDAP(DS): + db_config_file.write(db_config) + db_config_file.close() + ++ # Import ad schema ++ subprocess.check_call( ++ ["slapadd", "-F", self.conf_slapd_d_dir, "-b", "cn=config", ++ "-l", "data/ssh_schema.ldif"], ++ ) ++ + def _start_daemon(self): + """Start the instance.""" + if subprocess.call(["slapd", "-F", self.conf_slapd_d_dir, +-- +2.17.1 + diff --git a/0010-TESTS-Allow-adding-sshPublicKey-for-users.patch b/0010-TESTS-Allow-adding-sshPublicKey-for-users.patch new file mode 100644 index 0000000..ed461cd --- /dev/null +++ b/0010-TESTS-Allow-adding-sshPublicKey-for-users.patch @@ -0,0 +1,78 @@ +From 4bff9d92a51bff2fabb6168f8ae69c8a8d17ba2a Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Sun, 17 Jun 2018 22:06:22 +0200 +Subject: [PATCH] TESTS: Allow adding sshPublicKey for users +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Adds the objectclass and allows storing a list of sshPublicKey +attributes for users. Since there is no harm in adding the extra +objectclass, we can do it unconditionally. + +Related to: +https://pagure.io/SSSD/sssd/issue/3747 + +Reviewed-by: Fabiano Fidêncio +(cherry picked from commit 56cda832e9f61c52e9cfde1f0864507de718ffbb) +--- + src/tests/intg/ldap_ent.py | 15 +++++++++++---- + 1 file changed, 11 insertions(+), 4 deletions(-) + +diff --git a/src/tests/intg/ldap_ent.py b/src/tests/intg/ldap_ent.py +index 6b6d8f903cbcc277d892c3212ca382f4aaadc671..a4c987969d3dcefba2af69e095b220180e0fa54c 100644 +--- a/src/tests/intg/ldap_ent.py ++++ b/src/tests/intg/ldap_ent.py +@@ -24,7 +24,8 @@ def user(base_dn, uid, uidNumber, gidNumber, + homeDirectory=None, + loginShell=None, + cn=None, +- sn=None): ++ sn=None, ++ sshPubKey=()): + """ + Generate an RFC2307(bis) user add-modlist for passing to ldap.add* + """ +@@ -33,7 +34,8 @@ def user(base_dn, uid, uidNumber, gidNumber, + user = ( + "uid=" + uid + ",ou=Users," + base_dn, + [ +- ('objectClass', [b'top', b'inetOrgPerson', b'posixAccount']), ++ ('objectClass', [b'top', b'inetOrgPerson', ++ b'posixAccount', b'ldapPublicKey']), + ('cn', [uidNumber if cn is None else cn.encode('utf-8')]), + ('sn', [b'User' if sn is None else sn.encode('utf-8')]), + ('uidNumber', [uidNumber]), +@@ -51,6 +53,9 @@ def user(base_dn, uid, uidNumber, gidNumber, + ) + if gecos is not None: + user[1].append(('gecos', [gecos.encode('utf-8')])) ++ if len(sshPubKey) > 0: ++ pubkeys = [key.encode('utf-8') for key in sshPubKey] ++ user[1].append(('sshPublicKey', pubkeys)) + return user + + +@@ -118,7 +123,8 @@ class List(list): + homeDirectory=None, + loginShell=None, + cn=None, +- sn=None): ++ sn=None, ++ sshPubKey=()): + """Add an RFC2307(bis) user add-modlist.""" + self.append(user(base_dn or self.base_dn, + uid, uidNumber, gidNumber, +@@ -127,7 +133,8 @@ class List(list): + homeDirectory=homeDirectory, + loginShell=loginShell, + cn=cn, +- sn=sn)) ++ sn=sn, ++ sshPubKey=sshPubKey)) + + def add_group(self, cn, gidNumber, member_uids=[], + base_dn=None): +-- +2.17.1 + diff --git a/0011-TESTS-Add-a-basic-SSH-responder-test.patch b/0011-TESTS-Add-a-basic-SSH-responder-test.patch new file mode 100644 index 0000000..0760c43 --- /dev/null +++ b/0011-TESTS-Add-a-basic-SSH-responder-test.patch @@ -0,0 +1,276 @@ +From 079464a0fd417f09cfafa2bda9ff1a4e1afdbe8a Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Mon, 18 Jun 2018 09:12:13 +0200 +Subject: [PATCH] TESTS: Add a basic SSH responder test +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Adds a basic test that makes sure that a list of SSH public keys can be +retrieved. This is to make sure we don't break the SSH integration later +on. + +Related: +https://pagure.io/SSSD/sssd/issue/3747 + +Reviewed-by: Fabiano Fidêncio +(cherry picked from commit 804c5b538ad89a1a3897b93f39d716fa50530842) +--- + src/tests/intg/Makefile.am | 1 + + src/tests/intg/test_ssh_pubkey.py | 232 ++++++++++++++++++++++++++++++ + 2 files changed, 233 insertions(+) + create mode 100644 src/tests/intg/test_ssh_pubkey.py + +diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am +index 9c5338261353f473d9051c0512c15a54ec38e1ec..a15022eb578394313155538898fe7cd7407eb9c0 100644 +--- a/src/tests/intg/Makefile.am ++++ b/src/tests/intg/Makefile.am +@@ -36,6 +36,7 @@ dist_noinst_DATA = \ + data/ad_schema.ldif \ + test_pysss_nss_idmap.py \ + test_infopipe.py \ ++ test_ssh_pubkey.py \ + $(NULL) + + EXTRA_DIST = data/cwrap-dbus-system.conf.in +diff --git a/src/tests/intg/test_ssh_pubkey.py b/src/tests/intg/test_ssh_pubkey.py +new file mode 100644 +index 0000000000000000000000000000000000000000..fbf55566e341373873057ec4e3af1d7f83202aa7 +--- /dev/null ++++ b/src/tests/intg/test_ssh_pubkey.py +@@ -0,0 +1,232 @@ ++# ++# ssh public key integration test ++# ++# Copyright (c) 2018 Red Hat, Inc. ++# ++# This 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; version 2 only ++# ++# 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 . ++# ++ ++import os ++import stat ++import signal ++import subprocess ++import time ++import ldap ++import ldap.modlist ++import pytest ++ ++import config ++import ds_openldap ++import ent ++import ldap_ent ++from util import unindent, get_call_output ++ ++LDAP_BASE_DN = "dc=example,dc=com" ++ ++USER1_PUBKEY1 = "ssh-dss AAAAB3NzaC1kc3MAAACBAPMkvcU53RVhBtjwiC3IqeRIWR9Qwdv8\ ++DmZzEsDD3Csd6jYxMsPZoXcPrHqwYcEj1s5MVqhdSFS0Cjz13e7gO6OMLInO3xMBSSFHjfp9RE1H\ ++pgc4WisazzyJaW9EMkQo/DqvkFkKh31oqAmxcSbLAFJRg4TTIqm18qu8IRKS6m/RAAAAFQC97TA5\ ++JSsMsaX1bRszC7y4PhMBvQAAAIEAt9Yo9v/h9W4nDbzUdkGwNRszlPEK+T12bJv0O9Fk6subD3Do\ ++6A4Qru/Nr6voXoq8b018Wb7iFWvKOoz5uT/plWBKLXL2NN7ovTR+dUJIzvwurQZroukmU1EghNey\ ++lkSHmDlxSoMK6Nh21uGu6l+b6x5pXNaZHMpsywG4kY8SoC0AAACAAWLHneEGvqkYA8La4Eob+Hjj\ ++mAKilx8byxm3Kfb1XO+ZrR6XxadofZOaUYRMpPKgFjKAKPxJftPLiDjWM7lSe6h8df0dUMLVXt6m\ ++eA83kE0uK5JOOGJfJDqmRed2YnfxUDNNFQGT4xFWGrNtYNbGyw9BWKbkooAsLqaO04zP3Rs= \ ++user1@LDAP" ++ ++USER1_PUBKEY2 = "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAwHUUF3HPH+DkU6j8k7Q1wHG\ ++RJY9NeLqSav3h95mTSCQYPSC7I9RTJ4OORgqCbEzrP/DYrrn4TtQ9dhRJar3ZY+F36SH5yFIXORb\ ++lAIbFU+/anahBuFS9vHi1MqFPckGmwJ4QCpjQhdYxo1ro0e1RuGSaQNp/w9N6S/fDz4Cj4I99xDz\ ++SeQeGHxYv0e60plQ8dUajmnaGmYRJHF9a6Ban7IWySActCja7eQP2zIRXEZMpuhl1E0U4y+gHTFI\ ++gD3zQai3QrXm8RUrQURIJ0u6BlGS910OPbHqLpLTFWG08L8sNUcYzC+DY6yoCSO0n/Df3pVRS4C9\ ++5Krf3FqppMTjdfQ== user1@LDAP" ++ ++ ++@pytest.fixture(scope="module") ++def ds_inst(request): ++ """LDAP server instance fixture""" ++ ds_inst = ds_openldap.DSOpenLDAP( ++ config.PREFIX, 10389, LDAP_BASE_DN, ++ "cn=admin", "Secret123" ++ ) ++ ++ try: ++ ds_inst.setup() ++ except: ++ ds_inst.teardown() ++ raise ++ request.addfinalizer(ds_inst.teardown) ++ return ds_inst ++ ++ ++@pytest.fixture(scope="module") ++def ldap_conn(request, ds_inst): ++ """LDAP server connection fixture""" ++ ldap_conn = ds_inst.bind() ++ ldap_conn.ds_inst = ds_inst ++ request.addfinalizer(ldap_conn.unbind_s) ++ return ldap_conn ++ ++ ++def create_ldap_entries(ldap_conn, ent_list=None): ++ """Add LDAP entries from ent_list""" ++ if ent_list is not None: ++ for entry in ent_list: ++ ldap_conn.add_s(entry[0], entry[1]) ++ ++ ++def cleanup_ldap_entries(ldap_conn, ent_list=None): ++ """Remove LDAP entries added by create_ldap_entries""" ++ if ent_list is None: ++ for ou in ("Users", "Groups", "Netgroups", "Services", "Policies"): ++ for entry in ldap_conn.search_s("ou=" + ou + "," + ++ ldap_conn.ds_inst.base_dn, ++ ldap.SCOPE_ONELEVEL, ++ attrlist=[]): ++ ldap_conn.delete_s(entry[0]) ++ else: ++ for entry in ent_list: ++ ldap_conn.delete_s(entry[0]) ++ ++ ++def create_ldap_cleanup(request, ldap_conn, ent_list=None): ++ """Add teardown for removing all user/group LDAP entries""" ++ request.addfinalizer(lambda: cleanup_ldap_entries(ldap_conn, ent_list)) ++ ++ ++def create_ldap_fixture(request, ldap_conn, ent_list=None): ++ """Add LDAP entries and add teardown for removing them""" ++ create_ldap_entries(ldap_conn, ent_list) ++ create_ldap_cleanup(request, ldap_conn, ent_list) ++ ++ ++SCHEMA_RFC2307_BIS = "rfc2307bis" ++ ++ ++def format_basic_conf(ldap_conn, schema): ++ """Format a basic SSSD configuration""" ++ schema_conf = "ldap_schema = " + schema + "\n" ++ schema_conf += "ldap_group_object_class = groupOfNames\n" ++ return unindent("""\ ++ [sssd] ++ domains = LDAP ++ services = nss, ssh ++ ++ [nss] ++ ++ [ssh] ++ debug_level=10 ++ ++ [domain/LDAP] ++ {schema_conf} ++ id_provider = ldap ++ auth_provider = ldap ++ ldap_uri = {ldap_conn.ds_inst.ldap_url} ++ ldap_search_base = {ldap_conn.ds_inst.base_dn} ++ ldap_sudo_use_host_filter = false ++ debug_level=10 ++ """).format(**locals()) ++ ++ ++def create_conf_file(contents): ++ """Create sssd.conf with specified contents""" ++ conf = open(config.CONF_PATH, "w") ++ conf.write(contents) ++ conf.close() ++ os.chmod(config.CONF_PATH, stat.S_IRUSR | stat.S_IWUSR) ++ ++ ++def cleanup_conf_file(): ++ """Remove sssd.conf, if it exists""" ++ if os.path.lexists(config.CONF_PATH): ++ os.unlink(config.CONF_PATH) ++ ++ ++def create_conf_cleanup(request): ++ """Add teardown for removing sssd.conf""" ++ request.addfinalizer(cleanup_conf_file) ++ ++ ++def create_conf_fixture(request, contents): ++ """ ++ Create sssd.conf with specified contents and add teardown for removing it ++ """ ++ create_conf_file(contents) ++ create_conf_cleanup(request) ++ ++ ++def create_sssd_process(): ++ """Start the SSSD process""" ++ if subprocess.call(["sssd", "-D", "-f"]) != 0: ++ raise Exception("sssd start failed") ++ ++ ++def get_sssd_pid(): ++ pid_file = open(config.PIDFILE_PATH, "r") ++ pid = int(pid_file.read()) ++ return pid ++ ++ ++def cleanup_sssd_process(): ++ """Stop the SSSD process and remove its state""" ++ try: ++ pid = get_sssd_pid() ++ os.kill(pid, signal.SIGTERM) ++ while True: ++ try: ++ os.kill(pid, signal.SIGCONT) ++ except: ++ break ++ time.sleep(1) ++ except: ++ pass ++ for path in os.listdir(config.DB_PATH): ++ os.unlink(config.DB_PATH + "/" + path) ++ for path in os.listdir(config.MCACHE_PATH): ++ os.unlink(config.MCACHE_PATH + "/" + path) ++ ++ ++def create_sssd_fixture(request): ++ """Start SSSD and add teardown for stopping it and removing its state""" ++ create_sssd_process() ++ create_sssd_cleanup(request) ++ ++ ++def create_sssd_cleanup(request): ++ """Add teardown for stopping SSSD and removing its state""" ++ request.addfinalizer(cleanup_sssd_process) ++ ++ ++@pytest.fixture ++def add_user_with_ssh_key(request, ldap_conn): ++ ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) ++ ent_list.add_user("user1", 1001, 2001, ++ sshPubKey=(USER1_PUBKEY1, USER1_PUBKEY2)) ++ ent_list.add_user("user2", 1002, 2001) ++ create_ldap_fixture(request, ldap_conn, ent_list) ++ ++ conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS) ++ create_conf_fixture(request, conf) ++ create_sssd_fixture(request) ++ return None ++ ++ ++def test_ssh_pubkey_retrieve(add_user_with_ssh_key): ++ """ ++ Test that we can retrieve an SSH public key for a user who has one ++ and can't retrieve a key for a user who does not have one. ++ """ ++ sshpubkey = get_call_output(["sss_ssh_authorizedkeys", "user1"]) ++ assert sshpubkey == USER1_PUBKEY1 + '\n' + USER1_PUBKEY2 + '\n' ++ ++ sshpubkey = get_call_output(["sss_ssh_authorizedkeys", "user2"]) ++ assert len(sshpubkey) == 0 +-- +2.17.1 + diff --git a/0012-SSH-Do-not-exit-abruptly-if-SSHD-closes-its-end-of-t.patch b/0012-SSH-Do-not-exit-abruptly-if-SSHD-closes-its-end-of-t.patch new file mode 100644 index 0000000..b212cbc --- /dev/null +++ b/0012-SSH-Do-not-exit-abruptly-if-SSHD-closes-its-end-of-t.patch @@ -0,0 +1,88 @@ +From 60f868ac16c4678d60f17d62fa3b47534d4d07cb Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Mon, 28 May 2018 21:41:49 +0200 +Subject: [PATCH] SSH: Do not exit abruptly if SSHD closes its end of the pipe + before reading all the SSH keys +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Resolves: +https://pagure.io/SSSD/sssd/issue/3747 + +Before writing the keys to sshd, ignore SIGPIPE so that if the pipe +towards the authorizedkeys helper is closed, the sss_ssh_authorizedkeys +helper is not terminated with SIGPIPE, but instead proceeds and then the +write(2) calls would non-terminally fail with EPIPE. + +Reviewed-by: Fabiano Fidêncio +(cherry picked from commit cb138d7d060611e891d341db08477e41f9a3d17d) +--- + src/sss_client/ssh/sss_ssh_authorizedkeys.c | 35 ++++++++++++++++++++- + 1 file changed, 34 insertions(+), 1 deletion(-) + +diff --git a/src/sss_client/ssh/sss_ssh_authorizedkeys.c b/src/sss_client/ssh/sss_ssh_authorizedkeys.c +index 782a9f44379bff5346c896b3e03570720632c0be..b0280fbf8b0ed0501d792973241b826fc4a7a04d 100644 +--- a/src/sss_client/ssh/sss_ssh_authorizedkeys.c ++++ b/src/sss_client/ssh/sss_ssh_authorizedkeys.c +@@ -21,6 +21,7 @@ + #include + #include + #include ++#include + + #include "util/util.h" + #include "util/crypto/sss_crypto.h" +@@ -99,8 +100,16 @@ int main(int argc, const char **argv) + goto fini; + } + ++ /* if sshd closes its end of the pipe, we don't want sss_ssh_authorizedkeys ++ * to exit abruptly, but to finish gracefully instead because the valid ++ * key can be present in the data already written ++ */ ++ signal(SIGPIPE, SIG_IGN); ++ + /* print results */ + for (i = 0; i < ent->num_pubkeys; i++) { ++ char *repr_break = NULL; ++ + ret = sss_ssh_format_pubkey(mem_ctx, &ent->pubkeys[i], &repr); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, +@@ -109,7 +118,31 @@ int main(int argc, const char **argv) + continue; + } + +- printf("%s\n", repr); ++ /* OpenSSH expects a linebreak after each key */ ++ repr_break = talloc_asprintf(mem_ctx, "%s\n", repr); ++ talloc_zfree(repr); ++ if (repr_break == NULL) { ++ ret = ENOMEM; ++ goto fini; ++ } ++ ++ ret = sss_atomic_write_s(STDOUT_FILENO, repr_break, strlen(repr_break)); ++ /* Avoid spiking memory with too many large keys */ ++ talloc_zfree(repr_break); ++ if (ret < 0) { ++ ret = errno; ++ if (ret == EPIPE) { ++ DEBUG(SSSDBG_MINOR_FAILURE, ++ "SSHD closed the pipe before all keys could be written\n"); ++ /* Return 0 so that openssh doesn't abort pubkey auth */ ++ ret = 0; ++ goto fini; ++ } ++ DEBUG(SSSDBG_CRIT_FAILURE, ++ "sss_atomic_write_s() failed (%d): %s\n", ++ ret, strerror(ret)); ++ goto fini; ++ } + } + + ret = EXIT_SUCCESS; +-- +2.17.1 + diff --git a/0013-TESTS-Add-a-helper-binary-that-can-trigger-the-SIGPI.patch b/0013-TESTS-Add-a-helper-binary-that-can-trigger-the-SIGPI.patch new file mode 100644 index 0000000..a9da871 --- /dev/null +++ b/0013-TESTS-Add-a-helper-binary-that-can-trigger-the-SIGPI.patch @@ -0,0 +1,213 @@ +From ef28a3bdc50d0da6fab86b0d27e4c548ac61a749 Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Mon, 28 May 2018 21:49:41 +0200 +Subject: [PATCH] TESTS: Add a helper binary that can trigger the SIGPIPE to + authorizedkeys +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Adds a test tool that simulates the behaviour of OpenSSH in the sense +that it starts to read the output from the sss_ssh_authorizedkeys tool, +but then closes the pipe before reading the whole output. + +Related: +https://pagure.io/SSSD/sssd/issue/3747 + +Reviewed-by: Fabiano Fidêncio +(cherry picked from commit 909c16edb26a3c48b10a49e7919a35d13d31c52e) +--- + Makefile.am | 15 +++- + src/tests/test_ssh_client.c | 133 ++++++++++++++++++++++++++++++++++++ + 2 files changed, 147 insertions(+), 1 deletion(-) + create mode 100644 src/tests/test_ssh_client.c + +diff --git a/Makefile.am b/Makefile.am +index 9055130ed74057987795285c243ff47584cf8316..99974cf0e94e1ec6086a53585042653ec5966c2c 100644 +--- a/Makefile.am ++++ b/Makefile.am +@@ -331,6 +331,7 @@ endif # HAVE_CMOCKA + check_PROGRAMS = \ + stress-tests \ + krb5-child-test \ ++ test_ssh_client \ + $(non_interactive_cmocka_based_tests) \ + $(non_interactive_check_based_tests) + +@@ -2291,6 +2292,18 @@ krb5_child_test_LDADD = \ + $(SSSD_INTERNAL_LTLIBS) \ + libsss_test_common.la + ++test_ssh_client_SOURCES = \ ++ src/tests/test_ssh_client.c \ ++ $(NULL) ++test_ssh_client_CFLAGS = \ ++ $(AM_CFLAGS) \ ++ -DSSH_CLIENT_DIR=\"$(abs_top_builddir)\" \ ++ $(NULL) ++test_ssh_client_LDADD = \ ++ $(SSSD_INTERNAL_LTLIBS) \ ++ $(SSSD_LIBS) \ ++ $(NULL) ++ + if BUILD_DBUS_TESTS + + sbus_tests_SOURCES = \ +@@ -3446,7 +3459,6 @@ test_iobuf_LDADD = \ + $(SSSD_LIBS) \ + $(NULL) + +- + EXTRA_simple_access_tests_DEPENDENCIES = \ + $(ldblib_LTLIBRARIES) + simple_access_tests_SOURCES = \ +@@ -3655,6 +3667,7 @@ intgcheck-prepare: + $(INTGCHECK_CONFIGURE_FLAGS) \ + CFLAGS="-O2 -g $$CFLAGS -DKCM_PEER_UID=$$(id -u)"; \ + $(MAKE) $(AM_MAKEFLAGS) ; \ ++ $(MAKE) $(AM_MAKEFLAGS) test_ssh_client; \ + : Force single-thread install to workaround concurrency issues; \ + $(MAKE) $(AM_MAKEFLAGS) -j1 install; \ + : Remove .la files from LDB module directory to avoid loader warnings; \ +diff --git a/src/tests/test_ssh_client.c b/src/tests/test_ssh_client.c +new file mode 100644 +index 0000000000000000000000000000000000000000..8f963941f3249561178436d6f6dfc376780a4cda +--- /dev/null ++++ b/src/tests/test_ssh_client.c +@@ -0,0 +1,133 @@ ++/* ++ 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 ++#include ++#include "util/util.h" ++ ++#ifdef SSH_CLIENT_DIR ++#define SSH_AK_CLIENT_PATH SSH_CLIENT_DIR"/sss_ssh_authorizedkeys" ++#else ++#error "The path to the ssh authorizedkeys helper is not defined" ++#endif /* SSH_CLIENT_DIR */ ++ ++int main(int argc, const char *argv[]) ++{ ++ poptContext pc; ++ int opt; ++ struct poptOption long_options[] = { ++ POPT_AUTOHELP ++ SSSD_DEBUG_OPTS ++ POPT_TABLEEND ++ }; ++ struct stat sb; ++ int ret; ++ int status; ++ int p[2]; ++ pid_t pid; ++ const char *pc_user = NULL; ++ char *av[3]; ++ char buf[5]; /* Ridiculously small buffer by design */ ++ ++ /* 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); ++ poptSetOtherOptionHelp(pc, "USER"); ++ 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 3; ++ } ++ } ++ ++ pc_user = poptGetArg(pc); ++ if (pc_user == NULL) { ++ fprintf(stderr, "No user specified\n"); ++ return 3; ++ } ++ ++ poptFreeContext(pc); ++ ++ DEBUG_CLI_INIT(debug_level); ++ ++ ret = stat(SSH_AK_CLIENT_PATH, &sb); ++ if (ret != 0) { ++ ret = errno; ++ DEBUG(SSSDBG_CRIT_FAILURE, ++ "Could not stat %s [%d]: %s\n", ++ SSH_AK_CLIENT_PATH, ret, strerror(ret)); ++ return 3; ++ } ++ ++ ret = pipe(p); ++ if (ret != 0) { ++ perror("pipe"); ++ return 3; ++ } ++ ++ switch (pid = fork()) { ++ case -1: ++ ret = errno; ++ close(p[0]); ++ close(p[1]); ++ DEBUG(SSSDBG_CRIT_FAILURE, "fork failed: %d\n", ret); ++ return 3; ++ case 0: ++ /* child */ ++ av[0] = discard_const(SSH_AK_CLIENT_PATH); ++ av[1] = discard_const(pc_user); ++ av[2] = NULL; ++ ++ close(p[0]); ++ ret = dup2(p[1], STDOUT_FILENO); ++ if (ret == -1) { ++ perror("dup2"); ++ return 3; ++ } ++ ++ execv(av[0], av); ++ return 3; ++ default: ++ /* parent */ ++ break; ++ } ++ ++ close(p[1]); ++ read(p[0], buf, sizeof(buf)); ++ close(p[0]); ++ ++ pid = waitpid(pid, &status, 0); ++ if (pid == -1) { ++ perror("waitpid"); ++ return 3; ++ } ++ ++ if (WIFEXITED(status)) { ++ printf("sss_ssh_authorizedkeys exited with return code %d\n", WEXITSTATUS(status)); ++ return 0; ++ } else if (WIFSIGNALED(status)) { ++ printf("sss_ssh_authorizedkeys exited with signal %d\n", WTERMSIG(status)); ++ return 1; ++ } ++ ++ printf("sss_ssh_authorizedkeys exited for another reason\n"); ++ return 2; ++} +-- +2.17.1 + diff --git a/0014-TESTS-Add-a-regression-test-for-SIGHUP-handling-in-s.patch b/0014-TESTS-Add-a-regression-test-for-SIGHUP-handling-in-s.patch new file mode 100644 index 0000000..ccb7133 --- /dev/null +++ b/0014-TESTS-Add-a-regression-test-for-SIGHUP-handling-in-s.patch @@ -0,0 +1,94 @@ +From 0adf4f50e9773afda2dc422b04163f19d946c150 Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Tue, 19 Jun 2018 11:39:02 +0200 +Subject: [PATCH] TESTS: Add a regression test for SIGHUP handling in + sss_ssh_authorizedkeys +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +A regression test for: +https://pagure.io/SSSD/sssd/issue/3747 + +Reviewed-by: Fabiano Fidêncio +(cherry picked from commit 4cc3c1a1b1070c12bcc4351880d8207e47b37496) +--- + src/tests/intg/test_ssh_pubkey.py | 58 +++++++++++++++++++++++++++++++ + 1 file changed, 58 insertions(+) + +diff --git a/src/tests/intg/test_ssh_pubkey.py b/src/tests/intg/test_ssh_pubkey.py +index fbf55566e341373873057ec4e3af1d7f83202aa7..8fb41c62d87ec210c9aad8582023fe1cb00f2b4e 100644 +--- a/src/tests/intg/test_ssh_pubkey.py ++++ b/src/tests/intg/test_ssh_pubkey.py +@@ -24,6 +24,8 @@ import time + import ldap + import ldap.modlist + import pytest ++import string ++import random + + import config + import ds_openldap +@@ -230,3 +232,59 @@ def test_ssh_pubkey_retrieve(add_user_with_ssh_key): + + sshpubkey = get_call_output(["sss_ssh_authorizedkeys", "user2"]) + assert len(sshpubkey) == 0 ++ ++ ++@pytest.fixture() ++def sighup_client(request): ++ test_ssh_cli_path = os.path.join(config.ABS_BUILDDIR, ++ "..", "..", "..", "test_ssh_client") ++ assert os.access(test_ssh_cli_path, os.X_OK) ++ return test_ssh_cli_path ++ ++ ++@pytest.fixture ++def add_user_with_many_keys(request, ldap_conn): ++ # Generate a large list of unique ssh pubkeys ++ pubkey_list = [] ++ while len(pubkey_list) < 50: ++ new_pubkey = list(USER1_PUBKEY1) ++ new_pubkey[10] = random.choice(string.ascii_uppercase) ++ new_pubkey[11] = random.choice(string.ascii_uppercase) ++ new_pubkey[12] = random.choice(string.ascii_uppercase) ++ str_new_pubkey = ''.join(c for c in new_pubkey) ++ if str_new_pubkey in pubkey_list: ++ continue ++ pubkey_list.append(str_new_pubkey) ++ ++ ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) ++ ent_list.add_user("user1", 1001, 2001, sshPubKey=pubkey_list) ++ create_ldap_fixture(request, ldap_conn, ent_list) ++ ++ conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS) ++ create_conf_fixture(request, conf) ++ create_sssd_fixture(request) ++ return None ++ ++ ++def test_ssh_sighup(add_user_with_many_keys, sighup_client): ++ """ ++ A regression test for https://pagure.io/SSSD/sssd/issue/3747 ++ ++ OpenSSH can close its end of the pipe towards sss_ssh_authorizedkeys ++ before all of the output is read. In that case, older versions ++ of sss_ssh_authorizedkeys were receiving a SIGPIPE ++ """ ++ cli_path = sighup_client ++ ++ # python actually does the sensible, but unexpected (for a C programmer) ++ # thing and handles SIGPIPE. In order to reproduce the bug, we need ++ # to unset the SIGPIPE handler ++ signal.signal(signal.SIGPIPE, signal.SIG_DFL) ++ ++ process = subprocess.Popen([cli_path, "user1"], ++ stdout=subprocess.PIPE, ++ stderr=subprocess.PIPE) ++ _, _ = process.communicate() ++ # If the test tool detects that sss_ssh_authorizedkeys was killed with a ++ # signal, it would have returned 1 ++ assert process.returncode == 0 +-- +2.17.1 + diff --git a/0015-Revert-LDAP-IPA-add-local-email-address-to-aliases.patch b/0015-Revert-LDAP-IPA-add-local-email-address-to-aliases.patch new file mode 100644 index 0000000..fb6b1da --- /dev/null +++ b/0015-Revert-LDAP-IPA-add-local-email-address-to-aliases.patch @@ -0,0 +1,141 @@ +From 9efaade255e59b4a2f5cff2ab78c1db61132a40a Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Thu, 21 Jun 2018 12:27:32 +0200 +Subject: [PATCH] Revert "LDAP/IPA: add local email address to aliases" +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +This reverts commit 9a310913d696d190db14c625080678db853a33fd. + +Storing the e-mail address as a nameAlias was a performance optimization +to avoid having to fall back to the UPN lookup, but had the disadvantage +of returning multiple results for cases where an e-mail address is the +same as a user's fully qualified name. + +Since the e-mail lookups would still work without this optimization, +just after one more lookup, let's revert the patch. + +Resolves: +https://pagure.io/SSSD/sssd/issue/3607 + +Reviewed-by: Fabiano Fidêncio +(cherry picked from commit b0ec3875da281a9c29eda2cb19c1026510866d5b) + +DOWNSTREAM: +Resolves: rhbz#1527662 - Handle conflicting e-mail addresses more gracefully +--- + src/providers/ipa/ipa_s2n_exop.c | 49 -------------------------------- + src/providers/ldap/sdap_utils.c | 22 -------------- + 2 files changed, 71 deletions(-) + +diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c +index 9cb735526293ff5a209d732366b86fdb95dc8679..6f3974637a08b9d70e32fb6d79724be4f6e8dbde 100644 +--- a/src/providers/ipa/ipa_s2n_exop.c ++++ b/src/providers/ipa/ipa_s2n_exop.c +@@ -2118,49 +2118,6 @@ done: + return ret; + } + +-static errno_t add_emails_to_aliases(struct sysdb_attrs *attrs, +- struct sss_domain_info *dom) +-{ +- int ret; +- const char **emails; +- size_t c; +- TALLOC_CTX *tmp_ctx; +- +- tmp_ctx = talloc_new(NULL); +- if (tmp_ctx == NULL) { +- DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n"); +- return ENOMEM; +- } +- +- ret = sysdb_attrs_get_string_array(attrs, SYSDB_USER_EMAIL, tmp_ctx, +- &emails); +- if (ret == EOK) { +- for (c = 0; emails[c] != NULL; c++) { +- if (is_email_from_domain(emails[c], dom)) { +- ret = sysdb_attrs_add_lc_name_alias_safe(attrs, emails[c]); +- if (ret != EOK) { +- DEBUG(SSSDBG_OP_FAILURE, +- "Failed to add lower-cased version of email [%s] " +- "into the alias list\n", emails[c]); +- goto done; +- } +- } +- } +- } else if (ret == ENOENT) { +- DEBUG(SSSDBG_TRACE_ALL, "No email addresses available.\n"); +- } else { +- DEBUG(SSSDBG_OP_FAILURE, +- "sysdb_attrs_get_string_array failed, skipping ...\n"); +- } +- +- ret = EOK; +- +-done: +- talloc_free(tmp_ctx); +- +- return ret; +-} +- + static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom, + struct req_input *req_input, + struct resp_attrs *attrs, +@@ -2314,12 +2271,6 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom, + goto done; + } + +- ret = add_emails_to_aliases(attrs->sysdb_attrs, dom); +- if (ret != EOK) { +- DEBUG(SSSDBG_OP_FAILURE, +- "add_emails_to_aliases failed, skipping ...\n"); +- } +- + if (upn == NULL) { + /* We also have to store a fake UPN here, because otherwise the + * krb5 child later won't be able to properly construct one as +diff --git a/src/providers/ldap/sdap_utils.c b/src/providers/ldap/sdap_utils.c +index 0ac3ab2e416d887d00480b5123859c611f514274..6d543101f06ce3cd3925a675af6cabdacb8ebcaa 100644 +--- a/src/providers/ldap/sdap_utils.c ++++ b/src/providers/ldap/sdap_utils.c +@@ -87,7 +87,6 @@ sdap_save_all_names(const char *name, + int i; + bool lowercase = !dom->case_sensitive; + bool store_as_fqdn; +- const char **emails; + + switch (entry_type) { + case SYSDB_MEMBER_USER: +@@ -144,27 +143,6 @@ sdap_save_all_names(const char *name, + + } + +- ret = sysdb_attrs_get_string_array(ldap_attrs, SYSDB_USER_EMAIL, tmp_ctx, +- &emails); +- if (ret == EOK) { +- for (i = 0; emails[i] != NULL; i++) { +- if (is_email_from_domain(emails[i], dom)) { +- ret = sysdb_attrs_add_lc_name_alias_safe(attrs, emails[i]); +- if (ret) { +- DEBUG(SSSDBG_OP_FAILURE, +- "Failed to add lower-cased version of email [%s] " +- "into the alias list\n", emails[i]); +- goto done; +- } +- } +- } +- } else if (ret == ENOENT) { +- DEBUG(SSSDBG_TRACE_ALL, "No email addresses available.\n"); +- } else { +- DEBUG(SSSDBG_OP_FAILURE, +- "sysdb_attrs_get_string_array failed, skipping ...\n"); +- } +- + ret = EOK; + done: + talloc_free(tmp_ctx); +-- +2.17.1 + diff --git a/0016-util-Remove-the-unused-function-is_email_from_domain.patch b/0016-util-Remove-the-unused-function-is_email_from_domain.patch new file mode 100644 index 0000000..ba26190 --- /dev/null +++ b/0016-util-Remove-the-unused-function-is_email_from_domain.patch @@ -0,0 +1,117 @@ +From 5651a893f7dddb13fa9edc94e96d7bc95ec13f8b Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Thu, 21 Jun 2018 12:40:44 +0200 +Subject: [PATCH] util: Remove the unused function is_email_from_domain +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +This commit pretty much reverts commit +04d4c4d45f3942a813b7f772737f801f877f4e64, it's just coded manually, +because "git revert 04d4c4d45f3942a813b7f772737f801f877f4e64" +resulted in conflicts. It's easier to just remove the single +function. + +Related: +https://pagure.io/SSSD/sssd/issue/3607 + +Reviewed-by: Fabiano Fidêncio +(cherry picked from commit 58f60a0949f5d84b1fe5d15e52adfceb84053569) +--- + src/tests/cmocka/test_utils.c | 21 --------------------- + src/util/domain_info_utils.c | 27 --------------------------- + src/util/util.h | 1 - + 3 files changed, 49 deletions(-) + +diff --git a/src/tests/cmocka/test_utils.c b/src/tests/cmocka/test_utils.c +index cf314abe2db4056fe92c167454a4ddc31be98a51..1a8699a2a87d57ab43c70ceebf9bc71da4def4d4 100644 +--- a/src/tests/cmocka/test_utils.c ++++ b/src/tests/cmocka/test_utils.c +@@ -1849,25 +1849,6 @@ static void test_sss_get_domain_mappings_content(void **state) + * capaths might not be as expected. */ + } + +-static void test_is_email_from_domain(void **state) +-{ +- struct dom_list_test_ctx *test_ctx = talloc_get_type(*state, +- struct dom_list_test_ctx); +- struct sss_domain_info *d; +- +- d = find_domain_by_name(test_ctx->dom_list, "name_0.dom", false); +- assert_non_null(d); +- +- assert_false(is_email_from_domain(NULL, NULL)); +- assert_false(is_email_from_domain("hello", NULL)); +- assert_false(is_email_from_domain(NULL, d)); +- assert_false(is_email_from_domain("hello", d)); +- assert_false(is_email_from_domain("hello@hello", d)); +- +- assert_true(is_email_from_domain("hello@name_0.dom", d)); +- assert_true(is_email_from_domain("hello@NaMe_0.DoM", d)); +-} +- + int main(int argc, const char *argv[]) + { + poptContext pc; +@@ -1896,8 +1877,6 @@ int main(int argc, const char *argv[]) + setup_dom_list, teardown_dom_list), + cmocka_unit_test_setup_teardown(test_find_domain_by_name_disabled, + setup_dom_list, teardown_dom_list), +- cmocka_unit_test_setup_teardown(test_is_email_from_domain, +- setup_dom_list, teardown_dom_list), + + cmocka_unit_test_setup_teardown(test_sss_names_init, + confdb_test_setup, +diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c +index 66077092a40111967a98b0937506d9e4472f50d5..9d608ef2079cadbf3c66187e3bb8c81d2d7b4604 100644 +--- a/src/util/domain_info_utils.c ++++ b/src/util/domain_info_utils.c +@@ -889,33 +889,6 @@ bool sss_domain_is_forest_root(struct sss_domain_info *dom) + return (dom->forest_root == dom); + } + +-bool is_email_from_domain(const char *email, struct sss_domain_info *dom) +-{ +- const char *p; +- +- if (email == NULL || dom == NULL) { +- return false; +- } +- +- p = strchr(email, '@'); +- if (p == NULL) { +- DEBUG(SSSDBG_TRACE_ALL, +- "Input [%s] does not look like an email address.\n", email); +- return false; +- } +- +- if (strcasecmp(p+1, dom->name) == 0) { +- DEBUG(SSSDBG_TRACE_ALL, "Email [%s] is from domain [%s].\n", email, +- dom->name); +- return true; +- } +- +- DEBUG(SSSDBG_TRACE_ALL, "Email [%s] is not from domain [%s].\n", email, +- dom->name); +- +- return false; +-} +- + char *subdomain_create_conf_path(TALLOC_CTX *mem_ctx, + struct sss_domain_info *subdomain) + { +diff --git a/src/util/util.h b/src/util/util.h +index 4657ab0c691e3e0442f340b94ae149e9d6602bb5..2785ac2e285cfb4dd6a309fe5d73dd755e07b8ad 100644 +--- a/src/util/util.h ++++ b/src/util/util.h +@@ -539,7 +539,6 @@ struct sss_domain_info *find_domain_by_sid(struct sss_domain_info *domain, + enum sss_domain_state sss_domain_get_state(struct sss_domain_info *dom); + void sss_domain_set_state(struct sss_domain_info *dom, + enum sss_domain_state state); +-bool is_email_from_domain(const char *email, struct sss_domain_info *dom); + bool sss_domain_is_forest_root(struct sss_domain_info *dom); + const char *sss_domain_type_str(struct sss_domain_info *dom); + +-- +2.17.1 + diff --git a/0017-TESTS-Allow-storing-e-mail-address-for-users.patch b/0017-TESTS-Allow-storing-e-mail-address-for-users.patch new file mode 100644 index 0000000..85bee19 --- /dev/null +++ b/0017-TESTS-Allow-storing-e-mail-address-for-users.patch @@ -0,0 +1,65 @@ +From 75710952e74ea6070a53baaf5ea4e80507cdc26c Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Thu, 21 Jun 2018 12:37:42 +0200 +Subject: [PATCH] TESTS: Allow storing e-mail address for users +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +This would allow adding tests for by-e-mail lookups later + +Related: +https://pagure.io/SSSD/sssd/issue/3607 + +Reviewed-by: Fabiano Fidêncio +(cherry picked from commit d057eb2e20a19ce975dc2202f7c0e9f204eb9510) +--- + src/tests/intg/ldap_ent.py | 11 ++++++++--- + 1 file changed, 8 insertions(+), 3 deletions(-) + +diff --git a/src/tests/intg/ldap_ent.py b/src/tests/intg/ldap_ent.py +index a4c987969d3dcefba2af69e095b220180e0fa54c..1f23e3ab7a7ee62909babb8338379a5f2d4e37f2 100644 +--- a/src/tests/intg/ldap_ent.py ++++ b/src/tests/intg/ldap_ent.py +@@ -25,7 +25,8 @@ def user(base_dn, uid, uidNumber, gidNumber, + loginShell=None, + cn=None, + sn=None, +- sshPubKey=()): ++ sshPubKey=(), ++ mail=None): + """ + Generate an RFC2307(bis) user add-modlist for passing to ldap.add* + """ +@@ -56,6 +57,8 @@ def user(base_dn, uid, uidNumber, gidNumber, + if len(sshPubKey) > 0: + pubkeys = [key.encode('utf-8') for key in sshPubKey] + user[1].append(('sshPublicKey', pubkeys)) ++ if mail is not None: ++ user[1].append(('mail', [mail.encode('utf-8')])) + return user + + +@@ -124,7 +127,8 @@ class List(list): + loginShell=None, + cn=None, + sn=None, +- sshPubKey=()): ++ sshPubKey=(), ++ mail=None): + """Add an RFC2307(bis) user add-modlist.""" + self.append(user(base_dn or self.base_dn, + uid, uidNumber, gidNumber, +@@ -134,7 +138,8 @@ class List(list): + loginShell=loginShell, + cn=cn, + sn=sn, +- sshPubKey=sshPubKey)) ++ sshPubKey=sshPubKey, ++ mail=mail)) + + def add_group(self, cn, gidNumber, member_uids=[], + base_dn=None): +-- +2.17.1 + diff --git a/0018-TESTS-Add-regression-test-for-looking-up-users-with-.patch b/0018-TESTS-Add-regression-test-for-looking-up-users-with-.patch new file mode 100644 index 0000000..109d6c0 --- /dev/null +++ b/0018-TESTS-Add-regression-test-for-looking-up-users-with-.patch @@ -0,0 +1,93 @@ +From 0cbf6070bccb6c1f904cea596f00af0cc6328bae Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Thu, 21 Jun 2018 12:37:57 +0200 +Subject: [PATCH] TESTS: Add regression test for looking up users with + conflicting e-mail addresses +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Related: +https://pagure.io/SSSD/sssd/issue/3607 + +Reviewed-by: Fabiano Fidêncio +(cherry picked from commit 76ce965fc3abfdcf3a4a9518e57545ea060033d6) +--- + src/tests/intg/test_ldap.py | 64 +++++++++++++++++++++++++++++++++++++ + 1 file changed, 64 insertions(+) + +diff --git a/src/tests/intg/test_ldap.py b/src/tests/intg/test_ldap.py +index f71915a8086d395e9971a7c82e2744bdd7b931b6..d70ae39841f111fdf2d6c00c9acca8073725c5c5 100644 +--- a/src/tests/intg/test_ldap.py ++++ b/src/tests/intg/test_ldap.py +@@ -1736,3 +1736,67 @@ def test_local_negative_timeout_disabled(ldap_conn, + assert res == NssReturnCode.SUCCESS + + cleanup_ldap_entries(ldap_conn, ent_list) ++ ++ ++@pytest.fixture ++def users_with_email_setup(request, ldap_conn): ++ ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) ++ ent_list.add_user("user1", 1001, 2001, mail="user1.email@LDAP") ++ ++ ent_list.add_user("emailuser", 1002, 2002) ++ ent_list.add_user("emailuser2", 1003, 2003, mail="emailuser@LDAP") ++ ++ ent_list.add_user("userx", 1004, 2004, mail="userxy@LDAP") ++ ent_list.add_user("usery", 1005, 2005, mail="userxy@LDAP") ++ ++ create_ldap_fixture(request, ldap_conn, ent_list) ++ ++ conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS) ++ create_conf_fixture(request, conf) ++ create_sssd_fixture(request) ++ return None ++ ++ ++def test_lookup_by_email(ldap_conn, users_with_email_setup): ++ """ ++ Test the simple case of looking up a user by e-mail ++ """ ++ ent.assert_passwd_by_name("user1.email@LDAP", ++ dict(name="user1", uid=1001, gid=2001)) ++ ++ ++def test_conflicting_mail_addresses_and_fqdn(ldap_conn, ++ users_with_email_setup): ++ """ ++ Test that we handle the case where one user's mail address is the ++ same as another user's FQDN ++ ++ This is a regression test for https://pagure.io/SSSD/sssd/issue/3607 ++ """ ++ # With #3607 unfixed, these two lookups would prime the cache with ++ # nameAlias: emailuser@LDAP for both entries.. ++ ent.assert_passwd_by_name("emailuser@LDAP", ++ dict(name="emailuser", uid=1002, gid=2002)) ++ ent.assert_passwd_by_name("emailuser2@LDAP", ++ dict(name="emailuser2", uid=1003, gid=2003)) ++ ++ # ..and subsequently, emailuser would not be returned because the cache ++ # lookup would have had returned two entries which is an error ++ ent.assert_passwd_by_name("emailuser@LDAP", ++ dict(name="emailuser", uid=1002, gid=2002)) ++ ent.assert_passwd_by_name("emailuser2@LDAP", ++ dict(name="emailuser2", uid=1003, gid=2003)) ++ ++ ++def test_conflicting_mail_addresses(ldap_conn, ++ users_with_email_setup): ++ """ ++ Negative test: looking up a user by e-mail which belongs to more than ++ one account fails in the back end. ++ """ ++ with pytest.raises(KeyError): ++ pwd.getpwnam("userxy@LDAP") ++ ++ # However resolving the users on their own must work ++ ent.assert_passwd_by_name("userx", dict(name="userx", uid=1004, gid=2004)) ++ ent.assert_passwd_by_name("usery", dict(name="usery", uid=1005, gid=2005)) +-- +2.17.1 + diff --git a/0019-MAN-Remove-outdated-notes-from-the-re_expression-des.patch b/0019-MAN-Remove-outdated-notes-from-the-re_expression-des.patch new file mode 100644 index 0000000..4ce9fed --- /dev/null +++ b/0019-MAN-Remove-outdated-notes-from-the-re_expression-des.patch @@ -0,0 +1,47 @@ +From 8d5241404e8eb2388a9fc45115f5210e3ada1f1b Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Fri, 22 Jun 2018 10:40:29 +0200 +Subject: [PATCH] MAN: Remove outdated notes from the re_expression description +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +These notes are only valid for very old pcre releases which hopefully +nobody is using anymore. + +Reviewed-by: Fabiano Fidêncio +Reviewed-by: Sumit Bose +(cherry picked from commit 4c79db69cbad88ed56e87e8fe61f697f72d7408d) + +DOWNSTREAM: +Resolves: rhbz#1509691 - Document how to change the regular expression for SSSD so that group names with an @-sign can be parsed +--- + src/man/sssd.conf.5.xml | 12 ------------ + 1 file changed, 12 deletions(-) + +diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml +index beed677072c9bbfb9e7749f25c2a90d743f9328d..558c97e834d7aaf46adb31b1573eb59705569782 100644 +--- a/src/man/sssd.conf.5.xml ++++ b/src/man/sssd.conf.5.xml +@@ -2630,18 +2630,6 @@ pam_account_locked_message = Account locked, please contact help desk. + the @ sign, the domain everything + after that" + +- +- PLEASE NOTE: the support for non-unique named +- subpatterns is not available on all platforms +- (e.g. RHEL5 and SLES10). Only platforms with +- libpcre version 7 or higher can support non-unique +- named subpatterns. +- +- +- PLEASE NOTE ALSO: older version of libpcre only +- support the Python syntax (?P<name>) to label +- subpatterns. +- + + + +-- +2.17.1 + diff --git a/0020-SUDO-Create-the-socket-with-stricter-permissions.patch b/0020-SUDO-Create-the-socket-with-stricter-permissions.patch new file mode 100644 index 0000000..acab6dd --- /dev/null +++ b/0020-SUDO-Create-the-socket-with-stricter-permissions.patch @@ -0,0 +1,55 @@ +From 69eedc59283888a1d7d5f59284e032f9cad89b73 Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Fri, 15 Jun 2018 22:29:34 +0200 +Subject: [PATCH] SUDO: Create the socket with stricter permissions +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +This patch switches the sudo responder from being created as a public +responder where the permissions are open and not checked by the sssd +deaamon to a private socket. In this case, sssd creates the pipes with +strict permissions (see the umask in the call to create_pipe_fd() in +set_unix_socket()) and additionaly checks the permissions with every read +via the tevent integrations (see accept_fd_handler()). + +Resolves: +https://pagure.io/SSSD/sssd/issue/3766 (CVE-2018-10852) + +Reviewed-by: Sumit Bose +Reviewed-by: Pavel Březina +(cherry picked from commit ed90a20a0f0e936eb00d268080716c0384ffb01d) +--- + src/responder/sudo/sudosrv.c | 3 ++- + src/sysv/systemd/sssd-sudo.socket.in | 1 + + 2 files changed, 3 insertions(+), 1 deletion(-) + +diff --git a/src/responder/sudo/sudosrv.c b/src/responder/sudo/sudosrv.c +index ac4258710d3a9b48285522abd23bdd59ba42ad4e..e87a24499c2d82fafaa8e1f9b386e44332394266 100644 +--- a/src/responder/sudo/sudosrv.c ++++ b/src/responder/sudo/sudosrv.c +@@ -79,7 +79,8 @@ int sudo_process_init(TALLOC_CTX *mem_ctx, + sudo_cmds = get_sudo_cmds(); + ret = sss_process_init(mem_ctx, ev, cdb, + sudo_cmds, +- SSS_SUDO_SOCKET_NAME, -1, NULL, -1, ++ NULL, -1, /* No public socket */ ++ SSS_SUDO_SOCKET_NAME, -1, /* Private socket only */ + CONFDB_SUDO_CONF_ENTRY, + SSS_SUDO_SBUS_SERVICE_NAME, + SSS_SUDO_SBUS_SERVICE_VERSION, +diff --git a/src/sysv/systemd/sssd-sudo.socket.in b/src/sysv/systemd/sssd-sudo.socket.in +index c9abb875f0accbaf58d78846020fef74c7473528..96a8b0327ddb4d331c9b2e97ece3453f8f76872d 100644 +--- a/src/sysv/systemd/sssd-sudo.socket.in ++++ b/src/sysv/systemd/sssd-sudo.socket.in +@@ -11,6 +11,7 @@ ExecStartPre=@libexecdir@/sssd/sssd_check_socket_activated_responders -r sudo + ListenStream=@pipepath@/sudo + SocketUser=@SSSD_USER@ + SocketGroup=@SSSD_USER@ ++SocketMode=0600 + + [Install] + WantedBy=sssd.service +-- +2.17.1 + diff --git a/sssd.spec b/sssd.spec index ed1ce6a..3127635 100644 --- a/sssd.spec +++ b/sssd.spec @@ -38,7 +38,7 @@ Name: sssd Version: 1.16.2 -Release: 3%{?dist} +Release: 4%{?dist} Group: Applications/System Summary: System Security Services Daemon License: GPLv3+ @@ -47,6 +47,26 @@ Source0: https://releases.pagure.org/SSSD/sssd/%{name}-%{version}.tar.gz BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) ### Patches ### +Patch0001: 0001-krb5-locator-add-support-for-multiple-addresses.patch +Patch0002: 0002-krb5-locator-fix-IPv6-support.patch +Patch0003: 0003-krb5-locator-make-plugin-more-robust.patch +Patch0004: 0004-krb5-locator-add-unit-tests.patch +Patch0005: 0005-AD-IPA-Create-kdcinfo-file-for-sub-domains.patch +Patch0006: 0006-krb5-refactor-removal-of-krb5info-files.patch +Patch0007: 0007-krb5_common-add-callback-only-once.patch +Patch0008: 0008-data-provider-run-offline-callbacks-only-once.patch +Patch0009: 0009-TESTS-Extend-the-schema-with-sshPublicKey-attribute.patch +Patch0010: 0010-TESTS-Allow-adding-sshPublicKey-for-users.patch +Patch0011: 0011-TESTS-Add-a-basic-SSH-responder-test.patch +Patch0012: 0012-SSH-Do-not-exit-abruptly-if-SSHD-closes-its-end-of-t.patch +Patch0013: 0013-TESTS-Add-a-helper-binary-that-can-trigger-the-SIGPI.patch +Patch0014: 0014-TESTS-Add-a-regression-test-for-SIGHUP-handling-in-s.patch +Patch0015: 0015-Revert-LDAP-IPA-add-local-email-address-to-aliases.patch +Patch0016: 0016-util-Remove-the-unused-function-is_email_from_domain.patch +Patch0017: 0017-TESTS-Allow-storing-e-mail-address-for-users.patch +Patch0018: 0018-TESTS-Add-regression-test-for-looking-up-users-with-.patch +Patch0019: 0019-MAN-Remove-outdated-notes-from-the-re_expression-des.patch +Patch0020: 0020-SUDO-Create-the-socket-with-stricter-permissions.patch Patch0502: 0502-SYSTEMD-Use-capabilities.patch Patch0503: 0503-Disable-stopping-idle-socket-activated-responders.patch @@ -1261,7 +1281,22 @@ fi %{_libdir}/%{name}/modules/libwbclient.so %changelog -* Thu Jun 21 2018 Fabiano fidêncio - 1.16.2-3 +* Mon Jun 25 2018 Fabiano Fidêncio - 1.16.2-4 +- Related: upstream#941 - return multiple server addresses to the Kerberos + locator plugin +- Related: upstream#3652 - kdcinfo doesn't get populated for other domains +- Resolves: upstream#3747 - sss_ssh_authorizedkeys exits abruptly if SSHD + closes its end of the pipe before reading all the + SSH keys +- Resolves: upstream#3607 - Handle conflicting e-mail addresses more gracefully +- Resolves: upstream#3754 - SSSD AD uses LDAP filter to detect POSIX attributes + stored in AD GC also for regular AD DC queries +- Related: upstream#3219 - [RFE] Regular expression used in sssd.conf not being + able to consume an @-sign in the user/group name. +- Resolves: upstream#3766 - CVE-2018-10852: information leak from the sssd-sudo + responder + +* Thu Jun 21 2018 Fabiano Fidêncio - 1.16.2-3 - Resolves: rhbz#1591804 - something keeps /lib/libnss_systemd.so.2 open on minimal appliance image, breaking composes