[SSSD] [PATCH] SSH: Add dp_get_host_send to common responder code
Jakub Hrozek
jhrozek at redhat.com
Thu May 3 11:24:59 UTC 2012
On Tue, May 01, 2012 at 02:47:34PM -0400, Stephen Gallagher wrote:
> On Wed, 2012-04-25 at 14:52 +0200, Jakub Hrozek wrote:
> > On Tue, Apr 24, 2012 at 11:31:10AM +0200, Jakub Hrozek wrote:
> > > [PATCH] Rename split_service_name_filter
> > > The function was used outside services code which was confusing due to
> > > its name. This patch renames it to sound more netrual.
> > >
>
> Ack.
>
> > > [PATCH] SSH: Add dp_get_host_send to common responder code
> > > Instead of using account_info request, creates a new ssh specific
> > > request. This improves code readability and will make the code more
> > > flexible in the future.
> > >
> > > https://fedorahosted.org/sssd/ticket/1176
> > >
> >
>
> Nack.
>
> I don't like the construct you added to the "Invalid Filter" section of
> be_host_handler(). For one thing, ret is checked without a guarantee it
> has been set (it's set coincidentally further up, but we cannot rely on
> that). For another, it's identical to the handling for when the
> strncmp() fails. Please combine them and make sure that ret is
> initialized properly.
>
> You're not freeing filter in sss_dp_get_ssh_host_msg() if
> dbus_message_new_method_call() returns NULL.
>
Thanks, new patches are attached.
-------------- next part --------------
>From a09c9c1197a4bdaf1a410dde529b52b8fb882707 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 24 Apr 2012 11:15:04 +0200
Subject: [PATCH 1/2] Rename split_service_name_filter
The function was used outside services code which was confusing due to
its name. This patch renames it to sound more netrual.
---
src/providers/data_provider_be.c | 32 ++++++++++++++++----------------
1 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index b9234d573dcdfdafc9716aa9eee6a3833406100f..a343b107eaa21d17173a4a6b7d2e5b17bd2a3444 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -516,10 +516,10 @@ static void acctinfo_callback(struct be_req *req,
}
static errno_t
-split_service_name_filter(TALLOC_CTX *mem_ctx,
- const char *filter,
- char **name,
- char **protocol)
+split_name_extended(TALLOC_CTX *mem_ctx,
+ const char *filter,
+ char **name,
+ char **extended)
{
char *p;
@@ -530,12 +530,12 @@ split_service_name_filter(TALLOC_CTX *mem_ctx,
p = strchr(*name, ':');
if (p) {
- /* Protocol included */
+ /* Extended info included */
*p = '\0';
- *protocol = p + 1;
+ *extended = p + 1;
} else {
- *protocol = NULL;
+ *extended = NULL;
}
return EOK;
@@ -660,14 +660,14 @@ static int be_get_account_info(DBusMessage *message, struct sbus_connection *con
ret = EOK;
if (strncmp(filter, "name=", 5) == 0) {
req->filter_type = BE_FILTER_NAME;
- ret = split_service_name_filter(req, &filter[5],
- &req->filter_value,
- &req->extra_value);
+ ret = split_name_extended(req, &filter[5],
+ &req->filter_value,
+ &req->extra_value);
} else if (strncmp(filter, "idnumber=", 9) == 0) {
req->filter_type = BE_FILTER_IDNUM;
- ret = split_service_name_filter(req, &filter[9],
- &req->filter_value,
- &req->extra_value);
+ ret = split_name_extended(req, &filter[9],
+ &req->filter_value,
+ &req->extra_value);
} else if (strcmp(filter, ENUM_INDICATOR) == 0) {
req->filter_type = BE_FILTER_ENUM;
req->filter_value = NULL;
@@ -1445,9 +1445,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;
- ret = split_service_name_filter(req, &filter[5],
- &req->filter_value,
- &req->extra_value);
+ ret = split_name_extended(req, &filter[5],
+ &req->filter_value,
+ &req->extra_value);
} else {
err_maj = DP_ERR_FATAL;
err_min = EINVAL;
--
1.7.7.6
-------------- next part --------------
>From 6696b09217256222581047dfb6cc27e23b26bb16 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 18 Apr 2012 14:27:44 +0200
Subject: [PATCH 2/2] SSH: Add dp_get_host_send to common responder code
Instead of using account_info request, creates a new ssh specific
request. This improves code readability and will make the code more
flexible in the future.
https://fedorahosted.org/sssd/ticket/1176
---
Makefile.am | 1 +
src/providers/data_provider_be.c | 32 +++-----
src/providers/dp_backend.h | 7 ++
src/providers/ipa/ipa_hostid.c | 15 ++--
src/responder/common/responder.h | 3 +-
src/responder/common/responder_dp.c | 12 +---
src/responder/ssh/sshsrv_cmd.c | 14 ++--
src/responder/ssh/sshsrv_dp.c | 163 +++++++++++++++++++++++++++++++++++
src/responder/ssh/sshsrv_private.h | 16 +++-
9 files changed, 211 insertions(+), 52 deletions(-)
create mode 100644 src/responder/ssh/sshsrv_dp.c
diff --git a/Makefile.am b/Makefile.am
index 985e948ff423b973a230cca95f6592433fbd77a7..f5c1b940c079564e1173cffc8376f3997f33e06c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -570,6 +570,7 @@ endif
if BUILD_SSH
sssd_ssh_SOURCES = \
src/responder/ssh/sshsrv.c \
+ src/responder/ssh/sshsrv_dp.c \
src/responder/ssh/sshsrv_cmd.c \
$(SSSD_RESPONDER_OBJ)
sssd_ssh_LDADD = \
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index a343b107eaa21d17173a4a6b7d2e5b17bd2a3444..0b45baaa7462a72fc7c560ba9aaf45b67a31ca74 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -1336,7 +1336,7 @@ static void be_autofs_handler_callback(struct be_req *req,
static int be_host_handler(DBusMessage *message, struct sbus_connection *conn)
{
- struct be_acct_req *req;
+ struct be_host_req *req;
struct be_req *be_req;
struct be_client *becli;
DBusMessage *reply;
@@ -1345,7 +1345,6 @@ static int be_host_handler(DBusMessage *message, struct sbus_connection *conn)
void *user_data;
uint32_t flags;
char *filter;
- uint32_t attr_type;
int ret;
dbus_uint16_t err_maj;
dbus_uint32_t err_min;
@@ -1362,7 +1361,6 @@ static int be_host_handler(DBusMessage *message, struct sbus_connection *conn)
ret = dbus_message_get_args(message, &dbus_error,
DBUS_TYPE_UINT32, &flags,
- DBUS_TYPE_UINT32, &attr_type,
DBUS_TYPE_STRING, &filter,
DBUS_TYPE_INVALID);
if (!ret) {
@@ -1372,7 +1370,7 @@ static int be_host_handler(DBusMessage *message, struct sbus_connection *conn)
}
DEBUG(SSSDBG_TRACE_LIBS,
- ("Got request for [%u][%d][%s]\n", flags, attr_type, filter));
+ ("Got request for [%u][%s]\n", flags, filter));
reply = dbus_message_new_method_return(message);
if (!reply) return ENOMEM;
@@ -1420,35 +1418,27 @@ static int be_host_handler(DBusMessage *message, struct sbus_connection *conn)
be_req->fn = acctinfo_callback;
be_req->pvt = reply;
- req = talloc(be_req, struct be_acct_req);
+ req = talloc(be_req, struct be_host_req);
if (!req) {
err_maj = DP_ERR_FATAL;
err_min = ENOMEM;
err_msg = "Out of memory";
goto done;
}
- req->entry_type = BE_REQ_HOST | (flags & BE_REQ_FAST);
- req->attr_type = (int)attr_type;
+ req->type = BE_REQ_HOST | (flags & BE_REQ_FAST);
be_req->req_data = req;
- if ((attr_type != BE_ATTR_CORE) &&
- (attr_type != BE_ATTR_MEM) &&
- (attr_type != BE_ATTR_ALL)) {
- /* Unrecognized attr type */
- err_maj = DP_ERR_FATAL;
- err_min = EINVAL;
- err_msg = "Invalid Attrs Parameter";
- goto done;
- }
-
if (filter) {
- if (strncmp(filter, "name=", 5) == 0) {
+ ret = strncmp(filter, "name=", 5);
+ if (ret == 0) {
req->filter_type = BE_FILTER_NAME;
ret = split_name_extended(req, &filter[5],
- &req->filter_value,
- &req->extra_value);
- } else {
+ &req->name,
+ &req->alias);
+ }
+
+ if (ret) {
err_maj = DP_ERR_FATAL;
err_min = EINVAL;
err_msg = "Invalid Filter";
diff --git a/src/providers/dp_backend.h b/src/providers/dp_backend.h
index 8357e85f9b4760a513c53b6cf4023266f4ef46d9..c6bf2d0c6e4642c68f16aba0e60b2e760265a268 100644
--- a/src/providers/dp_backend.h
+++ b/src/providers/dp_backend.h
@@ -173,6 +173,13 @@ struct be_get_subdomains_req {
struct subdomain_info **domain_list;
};
+struct be_host_req {
+ uint32_t type;
+ int filter_type;
+ char *name;
+ char *alias;
+};
+
bool be_is_offline(struct be_ctx *ctx);
void be_mark_offline(struct be_ctx *ctx);
diff --git a/src/providers/ipa/ipa_hostid.c b/src/providers/ipa/ipa_hostid.c
index 8fcc59ed2e367e0214cebdc7f32c298f5d8874c6..c322c61f514b168f198740fdb333f4b47e3ec02f 100644
--- a/src/providers/ipa/ipa_hostid.c
+++ b/src/providers/ipa/ipa_hostid.c
@@ -45,8 +45,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);
+ const char *alias);
static errno_t
hosts_get_recv(struct tevent_req *req,
int *dp_error_out);
@@ -59,7 +58,7 @@ ipa_host_info_handler(struct be_req *breq)
{
struct ipa_hostid_ctx *hostid_ctx;
struct sdap_id_ctx *ctx;
- struct be_acct_req *ar;
+ struct be_host_req *hr;
struct tevent_req *req;
int dp_error = DP_ERR_FATAL;
errno_t ret = EOK;
@@ -75,17 +74,16 @@ ipa_host_info_handler(struct be_req *breq)
goto done;
}
- ar = talloc_get_type(breq->req_data, struct be_acct_req);
+ hr = talloc_get_type(breq->req_data, struct be_host_req);
- if (ar->filter_type != BE_FILTER_NAME) {
+ if (hr->filter_type != BE_FILTER_NAME) {
ret = EINVAL;
err = "Invalid filter type";
goto done;
}
req = hosts_get_send(breq, breq->be_ctx->ev, hostid_ctx,
- ar->filter_value, ar->extra_value,
- ar->attr_type);
+ hr->name, hr->alias);
if (!req) {
ret = ENOMEM;
err = "Out of memory";
@@ -150,8 +148,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)
+ const char *alias)
{
struct tevent_req *req;
struct hosts_get_state *state;
diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h
index 30a7101d4a1277d083bc82f67873c89fd58adc91..27a58eae53e025c2d2a1dc1e1cbfc268e2b8a4f9 100644
--- a/src/responder/common/responder.h
+++ b/src/responder/common/responder.h
@@ -253,8 +253,7 @@ enum sss_dp_acct_type {
SSS_DP_GROUP,
SSS_DP_INITGROUPS,
SSS_DP_NETGR,
- SSS_DP_SERVICES,
- SSS_DP_HOST
+ SSS_DP_SERVICES
};
struct tevent_req *
diff --git a/src/responder/common/responder_dp.c b/src/responder/common/responder_dp.c
index 7362bd74b17ad8e8b212d06bdc8ef2bfa8bb95ce..ca9cb834de7a6c0c3ce89ecc8eb20f61fe6c0428 100644
--- a/src/responder/common/responder_dp.c
+++ b/src/responder/common/responder_dp.c
@@ -490,34 +490,24 @@ sss_dp_get_account_msg(void *pvt)
uint32_t be_type;
uint32_t attrs = BE_ATTR_CORE;
char *filter;
- const char *dp_method;
info = talloc_get_type(pvt, struct sss_dp_account_info);
switch (info->type) {
case SSS_DP_USER:
be_type = BE_REQ_USER;
- dp_method = DP_METHOD_GETACCTINFO;
break;
case SSS_DP_GROUP:
be_type = BE_REQ_GROUP;
- dp_method = DP_METHOD_GETACCTINFO;
break;
case SSS_DP_INITGROUPS:
be_type = BE_REQ_INITGROUPS;
- dp_method = DP_METHOD_GETACCTINFO;
break;
case SSS_DP_NETGR:
be_type = BE_REQ_NETGROUP;
- dp_method = DP_METHOD_GETACCTINFO;
break;
case SSS_DP_SERVICES:
be_type = BE_REQ_SERVICES;
- dp_method = DP_METHOD_GETACCTINFO;
- break;
- case SSS_DP_HOST:
- be_type = 0;
- dp_method = DP_METHOD_HOSTHANDLER;
break;
}
@@ -550,7 +540,7 @@ sss_dp_get_account_msg(void *pvt)
msg = dbus_message_new_method_call(NULL,
DP_PATH,
DP_INTERFACE,
- dp_method);
+ DP_METHOD_GETACCTINFO);
if (msg == NULL) {
talloc_free(filter);
DEBUG(SSSDBG_CRIT_FAILURE, ("Out of memory?!\n"));
diff --git a/src/responder/ssh/sshsrv_cmd.c b/src/responder/ssh/sshsrv_cmd.c
index 0740cd25c2ec054c4d86d17ffaeebd01b65df7c3..fa02025e884ff5ab665e3a457ac00043b49cfb02 100644
--- a/src/responder/ssh/sshsrv_cmd.c
+++ b/src/responder/ssh/sshsrv_cmd.c
@@ -55,7 +55,6 @@ sss_ssh_cmd_get_user_pubkeys(struct cli_ctx *cctx)
return ENOMEM;
}
cmd_ctx->cctx = cctx;
- cmd_ctx->type = SSS_DP_USER;
ret = ssh_cmd_parse_request(cmd_ctx);
if (ret != EOK) {
@@ -98,7 +97,6 @@ sss_ssh_cmd_get_host_pubkeys(struct cli_ctx *cctx)
return ENOMEM;
}
cmd_ctx->cctx = cctx;
- cmd_ctx->type = SSS_DP_HOST;
ret = ssh_cmd_parse_request(cmd_ctx);
if (ret != EOK) {
@@ -139,9 +137,9 @@ ssh_dp_send_req_done(struct tevent_req *req)
dbus_uint32_t err_min;
char *err_msg;
- ret = sss_dp_get_account_recv(cb_ctx->mem_ctx, req,
- &err_maj, &err_min,
- &err_msg);
+ ret = sss_dp_get_ssh_host_recv(cb_ctx->mem_ctx, req,
+ &err_maj, &err_min,
+ &err_msg);
talloc_zfree(req);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
@@ -312,9 +310,9 @@ ssh_host_pubkeys_search(struct ssh_cmd_ctx *cmd_ctx)
/* refresh the host's cache entry */
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, cmd_ctx->alias);
+ req = sss_dp_get_ssh_host_send(cmd_ctx, cmd_ctx->cctx->rctx,
+ cmd_ctx->domain, false,
+ cmd_ctx->name, cmd_ctx->alias);
if (!req) {
DEBUG(SSSDBG_CRIT_FAILURE,
("Out of memory sending data provider request\n"));
diff --git a/src/responder/ssh/sshsrv_dp.c b/src/responder/ssh/sshsrv_dp.c
new file mode 100644
index 0000000000000000000000000000000000000000..dbdcd479ec2c604a0c0d433b4f5b711e0b6e39d8
--- /dev/null
+++ b/src/responder/ssh/sshsrv_dp.c
@@ -0,0 +1,163 @@
+/*
+ Authors:
+ Jakub Hrozek <jhrozek at redhat.com>
+
+ Copyright (C) 2012 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 <http://www.gnu.org/licenses/>.
+*/
+
+#include <talloc.h>
+#include <tevent.h>
+#include <dbus/dbus.h>
+#include "sbus/sssd_dbus.h"
+
+#include "util/util.h"
+#include "sbus/sbus_client.h"
+#include "providers/data_provider.h"
+#include "responder/common/responder.h"
+
+struct sss_dp_get_ssh_host_info {
+ struct sss_domain_info *dom;
+
+ bool fast_reply;
+ const char *name;
+ const char *alias;
+};
+
+static DBusMessage *
+sss_dp_get_ssh_host_msg(void *pvt);
+
+struct tevent_req *
+sss_dp_get_ssh_host_send(TALLOC_CTX *mem_ctx,
+ struct resp_ctx *rctx,
+ struct sss_domain_info *dom,
+ bool fast_reply,
+ const char *name,
+ const char *alias)
+{
+ errno_t ret;
+ struct tevent_req *req;
+ struct sss_dp_get_ssh_host_info *info;
+ struct sss_dp_req_state *state;
+ char *key;
+
+ req = tevent_req_create(mem_ctx, &state, struct sss_dp_req_state);
+ if (!req) {
+ ret = ENOMEM;
+ goto error;
+ }
+
+ if (!dom) {
+ ret = EINVAL;
+ goto error;
+ }
+
+ info = talloc_zero(state, struct sss_dp_get_ssh_host_info);
+ info->fast_reply = fast_reply;
+ info->name = name;
+ info->alias = alias;
+ info->dom = dom;
+
+ if (alias) {
+ key = talloc_asprintf(state, "%s:%s@%s", name, alias, dom->name);
+ } else {
+ key = talloc_asprintf(state, "%s@%s", name, dom->name);
+ }
+ if (!key) {
+ ret = ENOMEM;
+ goto error;
+ }
+
+ ret = sss_dp_issue_request(state, rctx, key, dom, sss_dp_get_ssh_host_msg,
+ info, req);
+ talloc_free(key);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ ("Could not issue DP request [%d]: %s\n",
+ ret, strerror(ret)));
+ goto error;
+ }
+
+ return req;
+
+error:
+ tevent_req_error(req, ret);
+ tevent_req_post(req, rctx->ev);
+ return req;
+}
+
+static DBusMessage *
+sss_dp_get_ssh_host_msg(void *pvt)
+{
+ DBusMessage *msg;
+ dbus_bool_t dbret;
+ struct sss_dp_get_ssh_host_info *info;
+ uint32_t be_type = 0;
+ char *filter;
+
+ info = talloc_get_type(pvt, struct sss_dp_get_ssh_host_info);
+
+ if (info->fast_reply) {
+ be_type |= BE_REQ_FAST;
+ }
+
+ if (info->alias) {
+ filter = talloc_asprintf(info, "name=%s:%s", info->name, info->alias);
+ } else {
+ filter = talloc_asprintf(info, "name=%s", info->name);
+ }
+ if (!filter) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Out of memory?!\n"));
+ return NULL;
+ }
+
+ msg = dbus_message_new_method_call(NULL,
+ DP_PATH,
+ DP_INTERFACE,
+ DP_METHOD_HOSTHANDLER);
+ if (msg == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Out of memory?!\n"));
+ talloc_free(filter);
+ return NULL;
+ }
+
+ /* create the message */
+ DEBUG(SSSDBG_TRACE_FUNC,
+ ("Creating SSH host request for [%s][%u][%s]\n",
+ info->dom->name, be_type, filter));
+
+ dbret = dbus_message_append_args(msg,
+ DBUS_TYPE_UINT32, &be_type,
+ DBUS_TYPE_STRING, &filter,
+ DBUS_TYPE_INVALID);
+ talloc_free(filter);
+ if (!dbret) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to build message\n"));
+ dbus_message_unref(msg);
+ return NULL;
+ }
+
+ return msg;
+}
+
+errno_t
+sss_dp_get_ssh_host_recv(TALLOC_CTX *mem_ctx,
+ struct tevent_req *req,
+ dbus_uint16_t *dp_err,
+ dbus_uint32_t *dp_ret,
+ char **err_msg)
+{
+ return sss_dp_req_recv(mem_ctx, req, dp_err, dp_ret, err_msg);
+}
diff --git a/src/responder/ssh/sshsrv_private.h b/src/responder/ssh/sshsrv_private.h
index d74b492508677ab1d5ac962e15101a41228302aa..e63a3105ca0456d4d3ec79bc2f91915b0facf277 100644
--- a/src/responder/ssh/sshsrv_private.h
+++ b/src/responder/ssh/sshsrv_private.h
@@ -37,7 +37,6 @@ struct ssh_ctx {
struct ssh_cmd_ctx {
struct cli_ctx *cctx;
- enum sss_dp_acct_type type;
char *name;
char *alias;
char *domname;
@@ -51,4 +50,19 @@ struct ssh_cmd_ctx {
struct sss_cmd_table *get_ssh_cmds(void);
+struct tevent_req *
+sss_dp_get_ssh_host_send(TALLOC_CTX *mem_ctx,
+ struct resp_ctx *rctx,
+ struct sss_domain_info *dom,
+ bool fast_reply,
+ const char *name,
+ const char *alias);
+
+errno_t
+sss_dp_get_ssh_host_recv(TALLOC_CTX *mem_ctx,
+ struct tevent_req *req,
+ dbus_uint16_t *dp_err,
+ dbus_uint32_t *dp_ret,
+ char **err_msg);
+
#endif /* _SSHSRV_PRIVATE_H_ */
--
1.7.7.6
More information about the sssd-devel
mailing list