On 23.7.2012 19:00, Jakub Hrozek wrote:
On Mon, Jul 23, 2012 at 01:09:13PM +0200, Jan Zelený wrote:
Dne pondělí 23 července 2012 09:46:07, Pavel Březina napsal(a):
On 07/20/2012 10:47 AM, Jakub Hrozek wrote:
On Thu, Jul 19, 2012 at 07:28:18PM +0200, Pavel Březina wrote:
On 19.7.2012 13:18, Jan Zelený wrote:
Dne pondělí 16 července 2012 16:01:46, Pavel Březina napsal(a):
Expects that patch from "resolv_gethostbyname_send: talloc_strdup
hostname on state" thread is applied.
Nack,
please don't use talloc_realloc () in sdap_sudo_get_hostnames_send(),
it's
confusing. Allocating an array for three elements with
talloc_zero_array()
will save you couple lines of code (all those reallocs and NULL
assignments).
This is of course based on two observations I've made in the code are
that
before allocation:
- the state->hostnames is NULL when calling
sdap_sudo_get_hostnames_send()
- in there is no error, the state->hostnames will contain two records
and NULL
You are not checking if talloc_strdup() calls returned non-NULL pointer.
Thanks. New patch is attached.
I suspect you should use CONFDB_DOMAIN_RESOLV_OP_TIMEOUT instead of
CONFDB_DOMAIN_RESOLV_TIMEOUT. I'm not sure though, please take a look at
data_provider_fo.c:106 and provide a comment.
Well, I don't know. What's the difference between those two? I was
following ipa_dyndns.c and the shorter version is used there (see line
148).
CONFDB_DOMAIN_RESOLV_OP_TIMEOUT (aka dns_resolver_op_timeout) is a very
low-level setting that fine-tunes how long should c-ares talk to
individual
nameservers before giving up and trying the next DNS server. It is not
documented and left out of configAPI.
CONFDB_DOMAIN_RESOLV_TIMEOUT (aka dns_resolver_timeout) is a fail over
setting that tunes how long should a service resolution take. Keep in mind
that "service resolution" might involve talking to different DNS servers.
Unless you want to control timeouts related to a single DNS server,
CONFDB_DOMAIN_RESOLV_TIMEOUT is what you'd want to use.
https://fedorahosted.org/sssd/ticket/976 has more details.
Thank you for the explanation. Than I believe it is OK to use
CONFDB_DOMAIN_RESOLV_TIMEOUT in this case.
I agree, Ack
Jan
Nack,
+#include <unistd.h>
+#include <bits/local_lim.h> // HOST_NAME_MAX
+#include <string.h>
Please include only <limits.h> here, HOST_NAME_MAX will be included from
local_lim.h (via another header called posix_lim.h or similarly..)
Done.
Also don't use C++ comments.
Removed.
+ char hostname[HOST_NAME_MAX + 1];
+ errno = 0;
+ ret = gethostname(hostname, sizeof(hostname));
+ if (ret != EOK) {
This looks like a potential buffer overrun to me for cases when the host
name is HOST_NAME_MAX+1 long, I think you should use HOST_NAME_MAX as
the size parameter and also unconditionally set hostname[HOST_NAME_MAX]
to '\0' to make sure the buffer is NULL-terminated.
Done.
The rest looks good, I'm just wondering that maybe it would be nice to
create a utility function to initialize resolver with the defaults that
could be reused in data_provider_fo.c and this code.
+1
>From d84484bd85c5b8725862ebd797ad3923bf65bfd8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Thu, 28 Jun 2012 10:15:03 +0200
Subject: [PATCH] sudo ldap provider: support autoconfiguration of hostnames
https://fedorahosted.org/sssd/ticket/1420
sudoHost attribute may contain hostname or fqdn of the machine.
Sudo itself supports only one hostname and its fqdn - the one that
is returned by gethostbyname().
This patch implements autoconfiguration of hostname and fqdn if
it has not been set manually by ldap_sudo_hostnames option.
---
src/providers/ldap/sdap_async_sudo_hostinfo.c | 263 ++++++++++++++++++++++++-
1 files changed, 262 insertions(+), 1 deletions(-)
diff --git a/src/providers/ldap/sdap_async_sudo_hostinfo.c b/src/providers/ldap/sdap_async_sudo_hostinfo.c
index 82b63296139483d0bb48ca0ced60aafdcb0ac245..5e6f1fa7c47c1c55b57a7b191f4cfd2e6b2609dd 100644
--- a/src/providers/ldap/sdap_async_sudo_hostinfo.c
+++ b/src/providers/ldap/sdap_async_sudo_hostinfo.c
@@ -25,11 +25,15 @@
#include <sys/socket.h>
#include <arpa/inet.h>
#include <ifaddrs.h>
+#include <unistd.h>
+#include <bits/local_lim.h> // HOST_NAME_MAX
+#include <string.h>
#include "util/util.h"
#include "providers/ldap/sdap.h"
#include "providers/ldap/sdap_id_op.h"
#include "providers/ldap/sdap_sudo.h"
+#include "resolv/async_resolv.h"
static int sdap_sudo_get_ip_addresses(TALLOC_CTX *mem_ctx, char ***_ip_addr);
@@ -38,15 +42,36 @@ struct sdap_sudo_get_hostinfo_state {
char **ip_addr;
};
+struct sdap_sudo_get_hostnames_state {
+ struct tevent_context *ev;
+ struct resolv_ctx *resolv_ctx;
+ enum host_database *host_db;
+ enum restrict_family family_order;
+ char **hostnames;
+};
+
+static struct tevent_req *sdap_sudo_get_hostnames_send(TALLOC_CTX *mem_ctx,
+ struct be_ctx *be_ctx);
+
+static void sdap_sudo_get_hostnames_done(struct tevent_req *req);
+
+static void sdap_sudo_get_hostnames_resolv_done(struct tevent_req *subreq);
+
+static int sdap_sudo_get_hostnames_recv(TALLOC_CTX *mem_ctx,
+ struct tevent_req *req,
+ char ***hostnames);
+
+
struct tevent_req * sdap_sudo_get_hostinfo_send(TALLOC_CTX *mem_ctx,
struct sdap_options *opts,
struct be_ctx *be_ctx)
{
struct tevent_req *req = NULL;
+ struct tevent_req *subreq = NULL;
struct sdap_sudo_get_hostinfo_state *state = NULL;
char *conf_hostnames = NULL;
char *conf_ip_addr = NULL;
- int ret;
+ int ret = EOK;
/* create request */
req = tevent_req_create(mem_ctx, &state, struct sdap_sudo_get_hostinfo_state);
@@ -90,8 +115,21 @@ struct tevent_req * sdap_sudo_get_hostinfo_send(TALLOC_CTX *mem_ctx,
if (state->ip_addr == NULL) {
ret = sdap_sudo_get_ip_addresses(state, &state->ip_addr);
if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ ("Unable to detect IP addresses [%d]: %s\n", ret, strerror(ret)));
+ }
+ }
+ /* if hostnames are not specified, configure it automatically */
+ if (state->hostnames == NULL) {
+ subreq = sdap_sudo_get_hostnames_send(state, be_ctx);
+ if (subreq == NULL) {
+ ret = ENOMEM;
+ goto done;
}
+
+ tevent_req_set_callback(subreq, sdap_sudo_get_hostnames_done, req);
+ ret = EAGAIN;
}
done:
@@ -292,3 +330,226 @@ done:
return ret;
}
+
+/*
+ * SUDO allows only one hostname that is returned from gethostname()
+ * (and set to "localhost" if the returned value is empty)
+ * and then - if allowed - resolves its fqdn using gethostbyname() or
+ * getaddrinfo() if available.
+ */
+static struct tevent_req *sdap_sudo_get_hostnames_send(TALLOC_CTX *mem_ctx,
+ struct be_ctx *be_ctx)
+{
+ struct tevent_req *req = NULL;
+ struct tevent_req *subreq = NULL;
+ struct sdap_sudo_get_hostnames_state *state = NULL;
+ char *dot = NULL;
+ char hostname[HOST_NAME_MAX + 1];
+ int resolv_timeout;
+ int ret;
+
+ req = tevent_req_create(mem_ctx, &state, struct sdap_sudo_get_hostnames_state);
+ if (req == NULL) {
+ return NULL;
+ }
+
+ state->ev = be_ctx->ev;
+ state->hostnames = NULL;
+
+ /* hostname, fqdn and NULL */
+ state->hostnames = talloc_zero_array(state, char*, 3);
+ if (state->hostnames == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_zero_array() failed\n"));
+ ret = ENOMEM;
+ goto done;
+ }
+
+ /* get hostname */
+
+ errno = 0;
+ ret = gethostname(hostname, sizeof(hostname));
+ if (ret != EOK) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Unable to retrieve machine hostname "
+ "[%d]: %s\n", ret, strerror(ret)));
+ goto done;
+ }
+
+ state->hostnames[0] = talloc_strdup(state->hostnames, hostname);
+ if (state->hostnames[0] == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_strdup() failed\n"));
+ ret = ENOMEM;
+ goto done;
+ }
+
+ dot = strchr(hostname, '.');
+ if (dot != NULL) {
+ /* already a fqdn, determine hostname and finish */
+ DEBUG(SSSDBG_TRACE_INTERNAL, ("Found fqdn: %s\n", hostname));
+
+ *dot = '\0';
+ DEBUG(SSSDBG_TRACE_INTERNAL, ("Found hostname: %s\n", hostname));
+
+ state->hostnames[1] = talloc_strdup(state->hostnames, hostname);
+ if (state->hostnames[1] == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_strdup() failed\n"));
+ ret = ENOMEM;
+ goto done;
+ }
+
+ ret = EOK;
+ goto done;
+ } else {
+ DEBUG(SSSDBG_TRACE_INTERNAL, ("Found hostname: %s\n", hostname));
+ }
+
+ /* initialize resolv ctx */
+
+ ret = confdb_get_int(be_ctx->cdb, be_ctx->conf_path,
+ CONFDB_DOMAIN_RESOLV_TIMEOUT,
+ RESOLV_DEFAULT_TIMEOUT, &resolv_timeout);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ ("Could get the timeout parameter from confdb\n"));
+ goto done;
+ }
+
+ ret = resolv_init(be_ctx, be_ctx->ev, resolv_timeout, &state->resolv_ctx);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Could not set up resolver context\n"));
+ goto done;
+ }
+
+ /* get family order */
+
+ ret = resolv_get_family_order(be_ctx->cdb, be_ctx->conf_path,
+ &state->family_order);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Unable to retrieve family order "
+ "[%d]: %s\n", ret, strerror(ret)));
+ goto done;
+ }
+
+ /* get database order */
+
+ state->host_db = talloc_zero_array(state, enum host_database, 3);
+ state->host_db[0] = DB_FILES;
+ state->host_db[1] = DB_DNS;
+ state->host_db[2] = DB_SENTINEL;
+
+ /* get fqdn */
+
+ subreq = resolv_gethostbyname_send(state, state->ev, state->resolv_ctx,
+ hostname, state->family_order,
+ state->host_db);
+ if (subreq == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ tevent_req_set_callback(subreq, sdap_sudo_get_hostnames_resolv_done, req);
+
+ ret = EAGAIN;
+
+done:
+ if (ret != EAGAIN) {
+ if (ret == EOK) {
+ tevent_req_done(req);
+ } else {
+ tevent_req_error(req, ret);
+ }
+ tevent_req_post(req, be_ctx->ev);
+ }
+
+ return req;
+}
+
+static void sdap_sudo_get_hostnames_resolv_done(struct tevent_req *subreq)
+{
+ struct tevent_req *req = NULL;
+ struct sdap_sudo_get_hostnames_state *state = NULL;
+ struct resolv_hostent *rhostent = NULL;
+ int resolv_status;
+ int ret;
+
+ req = tevent_req_callback_data(subreq, struct tevent_req);
+ state = tevent_req_data(req, struct sdap_sudo_get_hostnames_state);
+
+ ret = resolv_gethostbyname_recv(subreq, state, &resolv_status, NULL,
+ &rhostent);
+ talloc_zfree(subreq);
+ if (ret == ENOENT) {
+ /* Empty result, just quit */
+ DEBUG(SSSDBG_TRACE_INTERNAL, ("No hostent found\n"));
+ goto done;
+ } else if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ ("Could not resolve fqdn for this machine, error [%d]: %s, "
+ "resolver returned: [%d]: %s\n", ret, strerror(ret),
+ resolv_status, resolv_strerror(resolv_status)));
+ tevent_req_error(req, ret);
+ return;
+ }
+
+ /* EOK */
+
+ DEBUG(SSSDBG_TRACE_INTERNAL, ("Found fqdn: %s\n", rhostent->name));
+
+ if (state->hostnames == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("state->hostnames is NULL\n"));
+ ret = EINVAL;
+ goto done;
+ }
+
+ state->hostnames[1] = talloc_strdup(state->hostnames, rhostent->name);
+ if (state->hostnames[1] == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_strdup() failed\n"));
+ ret = ENOMEM;
+ goto done;
+ }
+
+ ret = EOK;
+
+done:
+ if (ret == EOK) {
+ tevent_req_done(req);
+ } else {
+ tevent_req_error(req, ret);
+ }
+}
+
+static void sdap_sudo_get_hostnames_done(struct tevent_req *subreq)
+{
+ struct tevent_req *req = NULL;
+ struct sdap_sudo_get_hostinfo_state *state = NULL;
+ int ret;
+
+ req = tevent_req_callback_data(subreq, struct tevent_req);
+ state = tevent_req_data(req, struct sdap_sudo_get_hostinfo_state);
+
+ ret = sdap_sudo_get_hostnames_recv(state, subreq, &state->hostnames);
+ talloc_zfree(subreq);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Unable to retrieve hostnames [%d]: %s\n",
+ ret, strerror(ret)));
+ tevent_req_error(req, ret);
+ return;
+ }
+
+ tevent_req_done(req);
+}
+
+static int sdap_sudo_get_hostnames_recv(TALLOC_CTX *mem_ctx,
+ struct tevent_req *req,
+ char ***hostnames)
+{
+ struct sdap_sudo_get_hostnames_state *state = NULL;
+
+ state = tevent_req_data(req, struct sdap_sudo_get_hostnames_state);
+
+ TEVENT_REQ_RETURN_ON_ERROR(req);
+
+ *hostnames = talloc_steal(mem_ctx, state->hostnames);
+
+ return EOK;
+}
--
1.7.6.4
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
