On Fri, 2012-02-24 at 19:10 +0100, Jan Cholasta wrote: > Hi, > > this patchset contains these patches: > > [PATCH 1/3] SSH: Save SSH host name aliases > > This is needed in order to properly generate the known_hosts file when > user uses ssh with non-FQDN host name. >
I fixed up a few minor issues with this patch. The check for in_transaction should not have been conditional on ret (in the current code it was fine, but defensively it only needs to check for in_transaction). It's against our coding policy to mix pointers to structs and plain structs in the same variable declaration, so I split + struct addrinfo ai_hint; + struct addrinfo *ai = NULL; for clarity. You forgot to check for memory allocation error in sss_ssh_cmd_get_host_pubkeys() when creating the new primary name. I added it. And I moved the SSS_SSH_KNOWN_HOSTS_PATH and SSS_SSH_KNOWN_HOSTS_TEMP_TMPL define to patch 0003 where it belongs. Functionally, this is fine, so ack with my changes (attached). > > [PATCH 2/3] SSH: Move responder and client common code to util > > Don't manage user-specific known_hosts file in sss_ssh_knownhostsproxy. > > Continue connecting to SSH server even when SSSD is not running in > sss_ssh_knownhostsproxy and don't drop the connection when the > sss_ssh_knownhostsproxy process receives a signal. > > https://fedorahosted.org/sssd/ticket/1179 > https://fedorahosted.org/sssd/ticket/1184 > I'm not comfortable enough in this code to review it. I'd prefer to have Jakub's sign-off here. It would also be really nice if you could split this into multiple logical patches, because it's really overwhelming. > > [PATCH 3/3] SSH: Manage global known_hosts file in the responder > > https://fedorahosted.org/sssd/ticket/1193 > > The known_hosts file is stored in /var/lib/sss/pubconf/known_hosts. Nack You need to wrap the write() in a loop to make sure it writes completely. It also needs to loop on EINTR.
From 6aa130290320bb52ed39cb07225a710e704444dd Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Fri, 24 Feb 2012 12:48:08 -0500
Subject: [PATCH 1/3] SSH: Save SSH host name aliases
---
src/db/sysdb_ssh.c | 85 +++++++++++++++++++++++++++++++++---
src/db/sysdb_ssh.h | 9 ++--
src/providers/data_provider_be.c | 4 +-
src/providers/ipa/ipa_hostid.c | 34 +++-----------
src/responder/ssh/sshsrv_cmd.c | 26 +++++++++++-
src/responder/ssh/sshsrv_private.h | 1 +
6 files changed, 119 insertions(+), 40 deletions(-)
diff --git a/src/db/sysdb_ssh.c b/src/db/sysdb_ssh.c
index c2ea35b03869cdd49fa46464a22f224873a1aed4..d83ad96693b50cd6fe29b294e4f7cbff8448b573 100644
--- a/src/db/sysdb_ssh.c
+++ b/src/db/sysdb_ssh.c
@@ -24,12 +24,19 @@
#include "db/sysdb_private.h"
errno_t
-sysdb_save_ssh_host(struct sysdb_ctx *sysdb_ctx,
- const char *name,
- struct sysdb_attrs *attrs)
+sysdb_store_ssh_host(struct sysdb_ctx *sysdb,
+ const char *name,
+ const char *alias,
+ struct sysdb_attrs *attrs)
{
- errno_t ret;
TALLOC_CTX *tmp_ctx;
+ errno_t ret;
+ struct ldb_message **hosts;
+ size_t num_hosts;
+ struct ldb_message_element *el;
+ unsigned int i;
+ const char *search_attrs[] = { SYSDB_NAME_ALIAS, NULL };
+ bool in_transaction = false;
DEBUG(SSSDBG_TRACE_FUNC, ("Adding host %s\n", name));
@@ -46,25 +53,89 @@ sysdb_save_ssh_host(struct sysdb_ctx *sysdb_ctx,
}
}
- ret = sysdb_store_custom(sysdb_ctx, name, SSH_HOSTS_SUBDIR, attrs);
+ ret = sysdb_transaction_start(sysdb);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ ("Failed to start update transaction\n"));
+ goto done;
+ }
+
+ in_transaction = true;
+
+ ret = sysdb_search_ssh_hosts(tmp_ctx, sysdb, name, search_attrs,
+ &hosts, &num_hosts);
+ if (ret != EOK && ret != ENOENT) {
+ goto done;
+ }
+
+ if (num_hosts > 1) {
+ ret = EINVAL;
+ goto done;
+ }
+
+ ret = sysdb_delete_ssh_host(sysdb, name);
+ if (ret != EOK && ret != ENOENT) {
+ goto done;
+ }
+
+ if (num_hosts == 1) {
+ el = ldb_msg_find_element(hosts[0], SYSDB_NAME_ALIAS);
+
+ if (el) {
+ for (i = 0; i < el->num_values; i++) {
+ if (alias && strcmp((char *)el->values[i].data, alias) == 0) {
+ alias = NULL;
+ }
+
+ ret = sysdb_attrs_add_val(attrs,
+ SYSDB_NAME_ALIAS, &el->values[i]);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, ("Could not add name alias\n"));
+ goto done;
+ }
+ }
+ }
+ }
+
+ if (alias) {
+ ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, alias);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, ("Could not add name alias\n"));
+ goto done;
+ }
+ }
+
+ ret = sysdb_store_custom(sysdb, name, SSH_HOSTS_SUBDIR, attrs);
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, ("sysdb_store_custom failed [%d]: %s\n",
ret, strerror(ret)));
goto done;
}
+ ret = sysdb_transaction_commit(sysdb);
+ if (ret != EOK) {
+ goto done;
+ }
+
+ in_transaction = false;
ret = EOK;
+
done:
+ if (in_transaction) {
+ sysdb_transaction_cancel(sysdb);
+ }
+
talloc_free(tmp_ctx);
+
return ret;
}
errno_t
-sysdb_delete_ssh_host(struct sysdb_ctx *sysdb_ctx,
+sysdb_delete_ssh_host(struct sysdb_ctx *sysdb,
const char *name)
{
DEBUG(SSSDBG_TRACE_FUNC, ("Deleting host %s\n", name));
- return sysdb_delete_custom(sysdb_ctx, name, SSH_HOSTS_SUBDIR);
+ return sysdb_delete_custom(sysdb, name, SSH_HOSTS_SUBDIR);
}
errno_t
diff --git a/src/db/sysdb_ssh.h b/src/db/sysdb_ssh.h
index f9d8d6ecaf73ceb72015f93a1d65ba81398e8bf9..3136dcc07cca27d30ed70f9e61ed5077ba074dbf 100644
--- a/src/db/sysdb_ssh.h
+++ b/src/db/sysdb_ssh.h
@@ -26,12 +26,13 @@
#define SSH_HOSTS_SUBDIR "ssh_hosts"
errno_t
-sysdb_save_ssh_host(struct sysdb_ctx *sysdb_ctx,
- const char *name,
- struct sysdb_attrs *attrs);
+sysdb_store_ssh_host(struct sysdb_ctx *sysdb,
+ const char *name,
+ const char *alias,
+ struct sysdb_attrs *attrs);
errno_t
-sysdb_delete_ssh_host(struct sysdb_ctx *sysdb_ctx,
+sysdb_delete_ssh_host(struct sysdb_ctx *sysdb,
const char *name);
errno_t
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index 2cea1933c0af83573bd5a703ab2f0630818d9fe5..7e27ceacdb961cb9c5fd4e6874e1fdd31ca6d556 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -1182,7 +1182,9 @@ static int be_host_handler(DBusMessage *message, struct sbus_connection *conn)
if (filter) {
if (strncmp(filter, "name=", 5) == 0) {
req->filter_type = BE_FILTER_NAME;
- req->filter_value = &filter[5];
+ ret = split_service_name_filter(req, &filter[5],
+ &req->filter_value,
+ &req->extra_value);
} else {
err_maj = DP_ERR_FATAL;
err_min = EINVAL;
diff --git a/src/providers/ipa/ipa_hostid.c b/src/providers/ipa/ipa_hostid.c
index 873cc529668d45c8cd76cc12cfbe8f6cfcf6b37e..8ab24fe5aac839b6df88133e04229156c9b9148b 100644
--- a/src/providers/ipa/ipa_hostid.c
+++ b/src/providers/ipa/ipa_hostid.c
@@ -33,6 +33,7 @@ struct hosts_get_state {
struct sysdb_ctx *sysdb;
struct sss_domain_info *domain;
const char *name;
+ const char *alias;
const char **attrs;
size_t count;
@@ -45,6 +46,7 @@ hosts_get_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct ipa_hostid_ctx *hostid_ctx,
const char *name,
+ const char *alias,
int attrs_type);
static errno_t
hosts_get_recv(struct tevent_req *req,
@@ -83,7 +85,7 @@ ipa_host_info_handler(struct be_req *breq)
}
req = hosts_get_send(breq, breq->be_ctx->ev, hostid_ctx,
- ar->filter_value,
+ ar->filter_value, ar->extra_value,
ar->attr_type);
if (!req) {
ret = ENOMEM;
@@ -149,6 +151,7 @@ hosts_get_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct ipa_hostid_ctx *hostid_ctx,
const char *name,
+ const char *alias,
int attrs_type)
{
struct tevent_req *req;
@@ -175,6 +178,7 @@ hosts_get_send(TALLOC_CTX *memctx,
state->sysdb = ctx->be->sysdb;
state->domain = ctx->be->domain;
state->name = name;
+ state->alias = alias;
/* TODO: handle attrs_type */
ret = build_attrs_from_map(state, ctx->opts->host_map,
@@ -253,7 +257,6 @@ hosts_get_done(struct tevent_req *subreq)
struct hosts_get_state);
int dp_error = DP_ERR_FATAL;
errno_t ret;
- bool in_transaction = false;
ret = ipa_host_info_recv(subreq, state,
&state->count, &state->hosts,
@@ -274,20 +277,6 @@ hosts_get_done(struct tevent_req *subreq)
goto done;
}
- ret = sysdb_transaction_start(state->sysdb);
- if (ret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE,
- ("Failed to start update transaction\n"));
- goto done;
- }
-
- in_transaction = true;
-
- ret = sysdb_delete_ssh_host(state->sysdb, state->name);
- if (ret != EOK && ret != ENOENT) {
- goto done;
- }
-
if (state->count == 0) {
DEBUG(SSSDBG_CRIT_FAILURE,
("No host with name [%s] found.\n", state->name));
@@ -302,18 +291,12 @@ hosts_get_done(struct tevent_req *subreq)
goto done;
}
- ret = sysdb_save_ssh_host(state->sysdb,
- state->name, state->hosts[0]);
+ ret = sysdb_store_ssh_host(state->sysdb, state->name, state->alias,
+ state->hosts[0]);
if (ret != EOK) {
goto done;
}
- ret = sysdb_transaction_commit(state->sysdb);
- if (ret != EOK) {
- goto done;
- }
-
- in_transaction = false;
dp_error = DP_ERR_OK;
done:
@@ -321,9 +304,6 @@ done:
if (ret == EOK) {
tevent_req_done(req);
} else {
- if (in_transaction) {
- sysdb_transaction_cancel(state->sysdb);
- }
tevent_req_error(req, ret);
}
}
diff --git a/src/responder/ssh/sshsrv_cmd.c b/src/responder/ssh/sshsrv_cmd.c
index 0a182f3109ce53f0975497f283be0bac55160ea8..da6a4540ba96cbb5ac0e2226966cce8aafb16de5 100644
--- a/src/responder/ssh/sshsrv_cmd.c
+++ b/src/responder/ssh/sshsrv_cmd.c
@@ -22,6 +22,7 @@
#include <talloc.h>
#include <string.h>
+#include <netdb.h>
#include "util/util.h"
#include "util/crypto/sss_crypto.h"
@@ -90,6 +91,8 @@ sss_ssh_cmd_get_host_pubkeys(struct cli_ctx *cctx)
{
struct ssh_cmd_ctx *cmd_ctx;
errno_t ret;
+ struct addrinfo ai_hint;
+ struct addrinfo *ai = NULL;
cmd_ctx = talloc_zero(cctx, struct ssh_cmd_ctx);
if (!cmd_ctx) {
@@ -107,6 +110,27 @@ sss_ssh_cmd_get_host_pubkeys(struct cli_ctx *cctx)
("Requesting SSH host public keys for [%s] from [%s]\n",
cmd_ctx->name, cmd_ctx->domname ? cmd_ctx->domname : "<ALL>"));
+ /* canonicalize host name */
+ memset(&ai_hint, 0, sizeof(struct addrinfo));
+ ai_hint.ai_flags = AI_CANONNAME;
+
+ ret = getaddrinfo(cmd_ctx->name, NULL, &ai_hint, &ai);
+ if (!ret) {
+ if (strcmp(cmd_ctx->name, ai[0].ai_canonname) != 0) {
+ cmd_ctx->alias = cmd_ctx->name;
+ cmd_ctx->name = talloc_strdup(cmd_ctx, ai[0].ai_canonname);
+ if (!cmd_ctx->name) {
+ ret = ENOMEM;
+ goto done;
+ }
+ }
+ } else {
+ DEBUG(SSSDBG_OP_FAILURE,
+ ("getaddrinfo() failed (%d): %s\n", ret, gai_strerror(ret)));
+ }
+
+ freeaddrinfo(ai);
+
if (cmd_ctx->domname) {
cmd_ctx->domain = responder_get_domain(cctx->rctx->domains,
cmd_ctx->domname);
@@ -312,7 +336,7 @@ ssh_host_pubkeys_search(struct ssh_cmd_ctx *cmd_ctx)
if (NEED_CHECK_PROVIDER(cmd_ctx->domain->provider)) {
req = sss_dp_get_account_send(cmd_ctx, cmd_ctx->cctx->rctx,
cmd_ctx->domain, false, SSS_DP_HOST,
- cmd_ctx->name, 0, NULL);
+ cmd_ctx->name, 0, cmd_ctx->alias);
if (!req) {
DEBUG(SSSDBG_CRIT_FAILURE,
("Out of memory sending data provider request\n"));
diff --git a/src/responder/ssh/sshsrv_private.h b/src/responder/ssh/sshsrv_private.h
index 26ed6b92eb8836a611a8add7657f71e30df0e816..ffe88eb35fa7d86b7adab5e3b154aa430188e874 100644
--- a/src/responder/ssh/sshsrv_private.h
+++ b/src/responder/ssh/sshsrv_private.h
@@ -34,6 +34,7 @@ struct ssh_cmd_ctx {
struct cli_ctx *cctx;
enum sss_dp_acct_type type;
char *name;
+ char *alias;
char *domname;
struct sss_domain_info *domain;
--
1.7.7.6
signature.asc
Description: This is a digitally signed message part
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
