[PATCH] Avoid getaddrinfo in winbind krb5 locator
Volker Lendecke
Volker.Lendecke at SerNet.DE
Fri Nov 2 15:12:19 UTC 2018
Hi!
Attached find an experimental patch for the krb5 locator plugin. I
don't really have a good idea how to write an autotest for it. In my
tests of winbind and kinit with system MIT krb5 libraries it did work.
Comments?
Thanks, Volker
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From 719f27880d6a28a171ab2812563a5b37805f9a6e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 2 Nov 2018 13:16:04 +0100
Subject: [PATCH] nsswitch: Don't ask getaddrinfo in the krb5 locator plugin
wbcDomainControllerInfoEx has an address, and so does winbind when
filling the WINBINDD_LOCATOR_KDC_ADDRESS environment variable. There's
no point in going through getaddrinfo again.
This silences "kinit Administrator"'s dns queries to just the srv
queries by winbind which already get the IP address in the response.
This is almost a rewrite of the locator plugin, looking at the patch
is not really helpful. The final code is clearer, at least to me.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
nsswitch/krb5_plugin/winbind_krb5_locator.c | 270 ++++++++++++++--------------
nsswitch/wscript_build | 2 +-
source3/winbindd/winbindd_util.c | 2 +-
3 files changed, 132 insertions(+), 142 deletions(-)
diff --git a/nsswitch/krb5_plugin/winbind_krb5_locator.c b/nsswitch/krb5_plugin/winbind_krb5_locator.c
index 91a2d64d84c..a027589f603 100644
--- a/nsswitch/krb5_plugin/winbind_krb5_locator.c
+++ b/nsswitch/krb5_plugin/winbind_krb5_locator.c
@@ -37,23 +37,18 @@
#define KRB5_PLUGIN_NO_HANDLE KRB5_KDC_UNREACH /* Heimdal */
#endif
-static const char *get_service_from_locate_service_type(enum locate_service_type svc)
+static uint16_t get_port_from_locate_service_type(enum locate_service_type svc)
{
switch (svc) {
case locate_service_kdc:
case locate_service_master_kdc:
- return "88";
- case locate_service_kadmin:
- case locate_service_krb524:
- /* not supported */
- return NULL;
+ return 88;
case locate_service_kpasswd:
- return "464";
+ return 464;
default:
break;
}
- return NULL;
-
+ return 0;
}
#ifdef DEBUG_KRB5
@@ -163,72 +158,6 @@ static int smb_krb5_locator_lookup_sanity_check(enum locate_service_type svc,
}
/**
- * Try to get addrinfo for a given host and call the krb5 callback
- *
- * @param name string
- * @param service string
- * @param in struct addrinfo hint
- * @param cbfunc krb5 callback function
- * @param cbdata void pointer cbdata
- *
- * @return krb5_error_code.
- */
-
-static krb5_error_code smb_krb5_locator_call_cbfunc(const char *name,
- const char *service,
- struct addrinfo *in,
- int (*cbfunc)(void *, int, struct sockaddr *),
- void *cbdata)
-{
- struct addrinfo *out = NULL;
- int ret = 0;
- struct addrinfo *res = NULL;
- int count = 3;
-
- while (count) {
-
- ret = getaddrinfo(name, service, in, &out);
- if (ret == 0) {
- break;
- }
-
- if ((ret == EAI_AGAIN) && (count > 1)) {
- count--;
- continue;
- }
-
-#ifdef DEBUG_KRB5
- fprintf(stderr, "[%5u]: smb_krb5_locator_lookup: "
- "getaddrinfo failed: %s (%d)\n",
- (unsigned int)getpid(), gai_strerror(ret), ret);
-#endif
-
- return KRB5_PLUGIN_NO_HANDLE;
- }
-
- for (res = out; res; res = res->ai_next) {
- if (!res->ai_addr || res->ai_addrlen == 0) {
- continue;
- }
-
- ret = cbfunc(cbdata, res->ai_socktype, res->ai_addr);
- if (ret) {
-#ifdef DEBUG_KRB5
- fprintf(stderr, "[%5u]: smb_krb5_locator_lookup: "
- "failed to call callback: %s (%d)\n",
- (unsigned int)getpid(), error_message(ret), ret);
-#endif
- break;
- }
- }
-
- if (out) {
- freeaddrinfo(out);
- }
- return ret;
-}
-
-/**
* PUBLIC INTERFACE: locate init
*
* @param context krb5_context
@@ -256,13 +185,49 @@ static void smb_krb5_locator_close(void *private_data)
return;
}
+static bool string2addr(const char *addrstr,
+ uint16_t port,
+ struct sockaddr_storage *addr)
+{
+ struct sockaddr_in *addr4 = (struct sockaddr_in *)addr;
+ struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)addr;
+ int ret;
+
+ ret = inet_pton(AF_INET, addrstr, &addr4->sin_addr);
+ if (ret == -1) {
+ /* complete fail */
+ return false;
+ }
+ if (ret == 1) {
+ /* success, found ipv4 */
+ addr4->sin_port = htons(port);
+ addr->ss_family = AF_INET;
+ return true;
+ }
+
+#if defined(HAVE_IPV6)
+ /* try ipv6 */
+ ret = inet_pton(AF_INET6, addrstr, &addr6->sin6_addr);
+ if (ret == 1) {
+ /* success, found ipv6 */
+ addr6->sin6_port = htons(port);
+ addr->ss_family = AF_INET6;
+ return true;
+ }
+#endif
+
+ return false;
+}
-static bool ask_winbind(const char *realm, char **dcname)
+static bool ask_winbind(const char *realm,
+ uint16_t port,
+ struct sockaddr_storage *addr)
{
wbcErr wbc_status;
- const char *dc = NULL;
struct wbcDomainControllerInfoEx *dc_info = NULL;
uint32_t flags;
+ const char *addrstr;
+ bool ok;
flags = WBC_LOOKUP_DC_KDC_REQUIRED |
WBC_LOOKUP_DC_IS_DNS_NAME |
@@ -278,25 +243,74 @@ static bool ask_winbind(const char *realm, char **dcname)
return false;
}
- if (!dc && dc_info->dc_unc) {
- dc = dc_info->dc_unc;
- if (dc[0] == '\\') dc++;
- if (dc[0] == '\\') dc++;
+ if ((dc_info == NULL) || (dc_info->dc_address == NULL)) {
+#ifdef DEBUG_KRB5
+ fprintf(stderr,
+ "[%5u]: %s: Did not get a DC address\n",
+ (unsigned)getpid(),
+ __FUNCTION__);
+#endif
+ goto fail;
}
- if (!dc) {
- wbcFreeMemory(dc_info);
- return false;
+ if (dc_info->dc_address_type != 1) {
+#ifdef DEBUG_KRB5
+ fprintf(stderr,
+ "[%5u]: %s: Did not get an INET address: %"PRIu16"\n",
+ (unsigned)getpid(),
+ __FUNCTION__,
+ dc_info->dc_address_type);
+#endif
+ goto fail;
}
- *dcname = strdup(dc);
- if (!*dcname) {
- wbcFreeMemory(dc_info);
- return false;
+ addrstr = dc_info->dc_address;
+ if (addrstr[0] == '\\') {
+ addrstr += 1;
+ }
+ if (addrstr[0] == '\\') {
+ addrstr += 1;
}
+ ok = string2addr(addrstr, port, addr);
+fail:
wbcFreeMemory(dc_info);
- return true;
+ return ok;
+}
+
+static bool ask_environment(const char *realm,
+ uint16_t port,
+ struct sockaddr_storage *addr)
+{
+ const char *prefix = WINBINDD_LOCATOR_KDC_ADDRESS;
+ int len = snprintf(NULL, 0, "%s_%s", prefix, realm);
+ const char *addrstr;
+ bool ok;
+
+ if (len == -1) {
+ return false;
+ }
+
+ {
+ char var[len+1];
+
+ snprintf(var, sizeof(var), "%s_%s", prefix, realm);
+
+ addrstr = getenv(var);
+ if (addrstr == NULL) {
+#ifdef DEBUG_KRB5
+ fprintf(stderr,
+ "[%5u]: smb_krb5_locator_lookup: "
+ "failed to get kdc from env %s\n",
+ (unsigned int)getpid(),
+ var);
+#endif
+ return false;
+ }
+ }
+
+ ok = string2addr(addrstr, port, addr);
+ return ok;
}
/**
@@ -322,11 +336,9 @@ static krb5_error_code smb_krb5_locator_lookup(void *private_data,
void *cbdata)
{
krb5_error_code ret;
- struct addrinfo aihints;
- char *kdc_name = NULL;
- const char *service = get_service_from_locate_service_type(svc);
-
- ZERO_STRUCT(aihints);
+ uint16_t port = get_port_from_locate_service_type(svc);
+ struct sockaddr_storage addr;
+ bool ok;
#ifdef DEBUG_KRB5
fprintf(stderr,"[%5u]: smb_krb5_locator_lookup: called for '%s' "
@@ -342,64 +354,42 @@ static krb5_error_code smb_krb5_locator_lookup(void *private_data,
if (ret) {
#ifdef DEBUG_KRB5
fprintf(stderr, "[%5u]: smb_krb5_locator_lookup: "
- "returning ret: %s (%d)\n",
- (unsigned int)getpid(), error_message(ret), ret);
+ "returning ret: %d\n",
+ (unsigned int)getpid(),
+ ret);
#endif
return ret;
}
if (!winbind_env_set()) {
- if (!ask_winbind(realm, &kdc_name)) {
-#ifdef DEBUG_KRB5
- fprintf(stderr, "[%5u]: smb_krb5_locator_lookup: "
- "failed to query winbindd\n",
- (unsigned int)getpid());
-#endif
- goto failed;
- }
+ ok = ask_winbind(realm, port, &addr);
} else {
- const char *env = NULL;
- char *var = NULL;
- if (asprintf(&var, "%s_%s",
- WINBINDD_LOCATOR_KDC_ADDRESS, realm) == -1) {
- goto failed;
- }
- env = getenv(var);
- if (!env) {
+ ok = ask_environment(realm, port, &addr);
+ }
+
+ if (!ok) {
#ifdef DEBUG_KRB5
- fprintf(stderr, "[%5u]: smb_krb5_locator_lookup: "
- "failed to get kdc from env %s\n",
- (unsigned int)getpid(), var);
+ fprintf(stderr,
+ "[%5u]: smb_krb5_locator_lookup: "
+ "failed to get kdc address for %s\n",
+ (unsigned)getpid(),
+ realm);
#endif
- free(var);
- goto failed;
- }
- free(var);
-
- kdc_name = strdup(env);
- if (!kdc_name) {
- goto failed;
- }
+ return KRB5_PLUGIN_NO_HANDLE;
}
+
+ ret = cbfunc(cbdata, socktype, (struct sockaddr *)&addr);
+ if (ret != 0) {
#ifdef DEBUG_KRB5
- fprintf(stderr, "[%5u]: smb_krb5_locator_lookup: "
- "got '%s' for '%s' from winbindd\n", (unsigned int)getpid(),
- kdc_name, realm);
+ fprintf(stderr,
+ "[%5u]: smb_krb5_locator_lookup: "
+ "failed to call callback: %d\n",
+ (unsigned int)getpid(),
+ ret);
#endif
+ }
- aihints.ai_family = family;
- aihints.ai_socktype = socktype;
-
- ret = smb_krb5_locator_call_cbfunc(kdc_name,
- service,
- &aihints,
- cbfunc, cbdata);
- SAFE_FREE(kdc_name);
-
- return ret;
-
- failed:
- return KRB5_PLUGIN_NO_HANDLE;
+ return 0;
}
#ifdef HEIMDAL_KRB5_LOCATE_PLUGIN_H
diff --git a/nsswitch/wscript_build b/nsswitch/wscript_build
index 6acc4a19b9b..a0cfe237367 100644
--- a/nsswitch/wscript_build
+++ b/nsswitch/wscript_build
@@ -108,7 +108,7 @@ if bld.CONFIG_SET('WITH_PAM_MODULES') and bld.CONFIG_SET('HAVE_PAM_START'):
if bld.CONFIG_SET('HAVE_KRB5_LOCATE_PLUGIN_H'):
bld.SAMBA_LIBRARY('winbind_krb5_locator',
source='krb5_plugin/winbind_krb5_locator.c',
- deps='wbclient krb5 com_err',
+ deps='wbclient',
realname='winbind_krb5_locator.so',
install_path='${MODULESDIR}/krb5')
diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index 9bc25d98c4e..a178f62c54e 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -2008,7 +2008,7 @@ static void winbindd_set_locator_kdc_env(const struct winbindd_domain *domain)
DEBUG(lvl,("winbindd_set_locator_kdc_env: setting var: %s to: %s\n",
var, kdc));
- setenv(var, kdc, 1);
+ setenv(var, addr, 1);
free(var);
}
--
2.11.0
More information about the samba-technical
mailing list