On 08/25/2014 02:05 PM, Jan Cholasta wrote:
Dne 4.8.2014 v 19:34 Pavel Reichl napsal(a):
>
> On 07/21/2014 02:08 PM, Jakub Hrozek wrote:
>> On Mon, Jul 21, 2014 at 01:55:20PM +0200, Jan Cholasta wrote:
>>> On 18.7.2014 06:41, William wrote:
>>>>>> Ignore that last patch, I messed up and didn't include a .h
file.
>>>>>> Here
>>>>>> is the fixed patch.
>>>>> This new patch seems to be based on some older version of the
>>>>> patch, it
>>>>> uses wrong option name, sysdb_update_ssh_host still has the confdb
>>>>> argument, etc.
>>>>>
>>>> Fixed, and fixed the documentation into the xml file, removed the
>>>> references to the .pot file.
>>> I don't see any of the requested fixes in the patch.
>>>
>>> See attachment for fixed patch. I also did a few additional tweaks to
>>> make
>>> the patch work when SSSD is updated from a previous version and when
>>> building without SSH bits.
>> Awesome thank you, I guess some of us need to review the patch now
>> you're also an author.
> While Jan was so kind to give me some hints on how to test patch
> properly, we found out a little bug. So Jan is going to prepare an
> updated patch.
>
> I'll do the review then.
I can't reproduce the issue, maybe something else was wrong in your
setup?
For the record, I installed IPA in a VM and then ran these commands on
it:
$ sss_ssh_knownhostsproxy --debug 10 $HOSTNAME
[no errors]
$ cat /var/lib/sss/pubconf/known_hosts
[not empty]
# systemctl stop ipa
$ sss_ssh_knownhostsproxy --debug 10 $HOSTNAME
[no errors]
$ cat /var/lib/sss/pubconf/known_hosts
[not empty]
# sss_cache -H
$ sss_ssh_knownhostsproxy --debug 10 $HOSTNAME
[no errors]
$ cat /var/lib/sss/pubconf/known_hosts
[empty]
# systemctl start ipa
$ sss_ssh_knownhostsproxy --debug 10 $HOSTNAME
[no errors]
$ cat /var/lib/sss/pubconf/known_hosts
[not empty]
Pavel had an empty /var/lib/sss/pubconf/known_hosts in the last step,
I have it non-empty as expected.
(sorry for the delay, I was on leave)
This test ran OK this time, so I think you are right.
I have some comments to the patch itself.
I'm not personally fan of big patches like this one. Could you at least
separate the man page changes to separate patch - so we can ask some
native speaker to review it.
Also new options in sss_cache -h and -H are not documented in man pages,
could you document them?
Please see some minor comments inline:
From 31312ab877da0b7fe8da9ddd808345e3096dbcd0 Mon Sep 17 00:00:00
2001
From: William B <william(a)adelaide.edu.au>
Date: Mon, 21 Jul 2014 11:13:25 +0200
Subject: [PATCH] Allow sss_cache tool to flush SSH hosts cache
---
src/confdb/confdb.c | 11 ++++++
src/confdb/confdb.h | 2 +
src/config/SSSDConfigTest.py | 2 +
src/config/etc/sssd.api.conf | 1 +
src/db/sysdb_ssh.c | 65 +++++++++++++++++++++++++++----
src/db/sysdb_ssh.h | 15 ++++++++
src/man/sssd.conf.5.xml | 14 +++++++
src/providers/ipa/ipa_hostid.c | 4 +-
src/tests/sysdb_ssh-tests.c | 1 +
src/tools/sss_cache.c | 86
+++++++++++++++++++++++++++++++++++++-----
10 files changed, 183 insertions(+), 18 deletions(-)
diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index 15de961..e2d7e8d 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -1040,6 +1040,17 @@ static int confdb_get_domain_internal(struct
confdb_ctx *cdb,
goto done;
}
+ /* Override the ssh known hosts timeout, if specified */
+ ret = get_entry_as_uint32(res->msgs[0], &domain->ssh_host_timeout,
+ CONFDB_DOMAIN_SSH_HOST_CACHE_TIMEOUT,
+ entry_cache_timeout);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_FATAL_FAILURE,
+ "Invalid value for [%s]\n",
+ CONFDB_DOMAIN_SSH_HOST_CACHE_TIMEOUT);
+ goto done;
+ }
+
/* Set refresh_expired_interval, if specified */
ret = get_entry_as_uint32(res->msgs[0],
&domain->refresh_expired_interval,
CONFDB_DOMAIN_REFRESH_EXPIRED_INTERVAL,
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index ba33ea5..e6c5c2d 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -175,6 +175,7 @@
#define CONFDB_DOMAIN_SERVICE_CACHE_TIMEOUT "entry_cache_service_timeout"
#define CONFDB_DOMAIN_AUTOFS_CACHE_TIMEOUT "entry_cache_autofs_timeout"
#define CONFDB_DOMAIN_SUDO_CACHE_TIMEOUT "entry_cache_sudo_timeout"
+#define CONFDB_DOMAIN_SSH_HOST_CACHE_TIMEOUT
"entry_cache_ssh_host_timeout"
#define CONFDB_DOMAIN_PWD_EXPIRATION_WARNING "pwd_expiration_warning"
#define CONFDB_DOMAIN_REFRESH_EXPIRED_INTERVAL "refresh_expired_interval"
#define CONFDB_DOMAIN_OFFLINE_TIMEOUT "offline_timeout"
@@ -232,6 +233,7 @@ struct sss_domain_info {
uint32_t service_timeout;
uint32_t autofsmap_timeout;
uint32_t sudo_timeout;
+ uint32_t ssh_host_timeout;
uint32_t refresh_expired_interval;
uint32_t subdomain_refresh_interval;
diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py
index 1450003..2d12bc0 100755
--- a/src/config/SSSDConfigTest.py
+++ b/src/config/SSSDConfigTest.py
@@ -505,6 +505,7 @@ class SSSDConfigTestSSSDDomain(unittest.TestCase):
'entry_cache_service_timeout',
'entry_cache_autofs_timeout',
'entry_cache_sudo_timeout',
+ 'entry_cache_ssh_host_timeout',
'refresh_expired_interval',
'lookup_family_order',
'account_cache_expiration',
@@ -863,6 +864,7 @@ class SSSDConfigTestSSSDDomain(unittest.TestCase):
'entry_cache_service_timeout',
'entry_cache_autofs_timeout',
'entry_cache_sudo_timeout',
+ 'entry_cache_ssh_host_timeout',
'refresh_expired_interval',
'account_cache_expiration',
'lookup_family_order',
diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf
index 5e5a928..5f995af 100644
--- a/src/config/etc/sssd.api.conf
+++ b/src/config/etc/sssd.api.conf
@@ -133,6 +133,7 @@ entry_cache_netgroup_timeout = int, None, false
entry_cache_service_timeout = int, None, false
entry_cache_autofs_timeout = int, None, false
entry_cache_sudo_timeout = int, None, false
+entry_cache_ssh_host_timeout = int, None, false
refresh_expired_interval = int, None, false
# Dynamic DNS updates
diff --git a/src/db/sysdb_ssh.c b/src/db/sysdb_ssh.c
index 7dd98cf..bc8b1f6 100644
--- a/src/db/sysdb_ssh.c
+++ b/src/db/sysdb_ssh.c
@@ -23,6 +23,14 @@
#include "db/sysdb_ssh.h"
#include "db/sysdb_private.h"
+static struct ldb_dn *
+sysdb_ssh_host_dn(TALLOC_CTX *mem_ctx,
+ struct sss_domain_info *domain,
+ const char *name)
+{
+ return sysdb_custom_dn(mem_ctx, domain, name, SSH_HOSTS_SUBDIR);
+}
+
static errno_t
sysdb_update_ssh_host(struct sss_domain_info *domain,
const char *name,
@@ -45,6 +53,7 @@ errno_t
sysdb_store_ssh_host(struct sss_domain_info *domain,
const char *name,
const char *alias,
+ int cache_timeout,
time_t now,
struct sysdb_attrs *attrs)
{
@@ -147,6 +156,14 @@ sysdb_store_ssh_host(struct sss_domain_info *domain,
goto done;
}
+ ret = sysdb_attrs_add_time_t(attrs, SYSDB_CACHE_EXPIRE,
+ cache_timeout ? (now +
cache_timeout) : 0);
+ if (ret) {
Please use form:
ret != EOK
as it is used in the rest of the module
+ DEBUG(SSSDBG_OP_FAILURE, "Could not set sysdb cache
expire
[%d]: %s\n",
+ ret, strerror(ret));
+ goto done;
+ }
+
ret = sysdb_update_ssh_host(domain, name, attrs);
if (ret != EOK) {
goto done;
@@ -175,6 +192,36 @@ done:
return ret;
}
+
One too many empty lines.
+errno_t
+sysdb_set_ssh_host_attr(struct sss_domain_info *domain,
+ const char *name,
+ struct sysdb_attrs *attrs,
+ int mod_op)
+{
+
Is the empty line here intentional?
+ errno_t ret;
+ struct ldb_dn *dn;
+ TALLOC_CTX *tmp_ctx;
+
+ tmp_ctx = talloc_new(NULL);
+ if (!tmp_ctx) {
+ return ENOMEM;
+ }
+
+ dn = sysdb_ssh_host_dn(tmp_ctx, domain, name);
+ if (!dn) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ ret = sysdb_set_entry_attr(domain->sysdb, dn, attrs, mod_op);
We should check return value here, right?
+
+done:
+ talloc_free(tmp_ctx);
+ return ret;
+}
+
errno_t
sysdb_update_ssh_known_host_expire(struct sss_domain_info *domain,
const char *name,
@@ -229,13 +276,13 @@ sysdb_delete_ssh_host(struct sss_domain_info
*domain,
return sysdb_delete_custom(domain, name, SSH_HOSTS_SUBDIR);
}
-static errno_t
+errno_t
sysdb_search_ssh_hosts(TALLOC_CTX *mem_ctx,
struct sss_domain_info *domain,
const char *filter,
const char **attrs,
- struct ldb_message ***hosts,
- size_t *num_hosts)
+ size_t *num_hosts,
+ struct ldb_message ***hosts)
{
errno_t ret;
TALLOC_CTX *tmp_ctx;
@@ -297,7 +344,7 @@ sysdb_get_ssh_host(TALLOC_CTX *mem_ctx,
}
ret = sysdb_search_ssh_hosts(tmp_ctx, domain, filter, attrs,
- &hosts, &num_hosts);
+ &num_hosts, &hosts);
if (ret != EOK) {
goto done;
}
@@ -335,15 +382,19 @@ sysdb_get_ssh_known_hosts(TALLOC_CTX *mem_ctx,
return ENOMEM;
}
- filter = talloc_asprintf(tmp_ctx, "(%s>=%ld)",
- SYSDB_SSH_KNOWN_HOSTS_EXPIRE, (long)now);
+ filter = talloc_asprintf(tmp_ctx,
+ "(&(|(!(%s=*))(%s=0)(%s>=%lld))(%s>=%lld))",
+ SYSDB_CACHE_EXPIRE,
+ SYSDB_CACHE_EXPIRE,
+ SYSDB_CACHE_EXPIRE, (long long)now + 1,
+ SYSDB_SSH_KNOWN_HOSTS_EXPIRE, (long
long)now + 1);
if (!filter) {
ret = ENOMEM;
goto done;
}
ret = sysdb_search_ssh_hosts(mem_ctx, domain, filter, attrs,
- hosts, num_hosts);
+ num_hosts, hosts);
done:
talloc_free(tmp_ctx);
diff --git a/src/db/sysdb_ssh.h b/src/db/sysdb_ssh.h
index e8aca77..960a13f 100644
--- a/src/db/sysdb_ssh.h
+++ b/src/db/sysdb_ssh.h
@@ -33,6 +33,7 @@ errno_t
sysdb_store_ssh_host(struct sss_domain_info *domain,
const char *name,
const char *alias,
+ int cache_timeout,
time_t now,
struct sysdb_attrs *attrs);
@@ -42,11 +43,25 @@ sysdb_update_ssh_known_host_expire(struct
sss_domain_info *domain,
time_t now,
int known_hosts_timeout);
+int
+sysdb_set_ssh_host_attr(struct sss_domain_info *domain,
+ const char *name,
+ struct sysdb_attrs *attrs,
+ int mod_op);
+
errno_t
sysdb_delete_ssh_host(struct sss_domain_info *domain,
const char *name);
errno_t
+sysdb_search_ssh_hosts(TALLOC_CTX *mem_ctx,
+ struct sss_domain_info *domain,
+ const char *filter,
+ const char **attrs,
+ size_t *num_hosts,
+ struct ldb_message ***hosts);
+
+errno_t
sysdb_get_ssh_host(TALLOC_CTX *mem_ctx,
struct sss_domain_info *domain,
const char *name,
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 27d22f4..bb631c3 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -1175,6 +1175,20 @@ fallback_homedir = /home/%u
</listitem>
</varlistentry>
+ <varlistentry condition="with_ssh">
+ <term>entry_cache_ssh_host_timeout (integer)</term>
+ <listitem>
+ <para>
+ How many seconds to keep a host ssh key after
+ refresh. IE how long to cache the host key
+ for.
+ </para>
+ <para>
+ Default: entry_cache_timeout
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term>refresh_expired_interval (integer)</term>
<listitem>
diff --git a/src/providers/ipa/ipa_hostid.c
b/src/providers/ipa/ipa_hostid.c
index 905454d..6b82335 100644
--- a/src/providers/ipa/ipa_hostid.c
+++ b/src/providers/ipa/ipa_hostid.c
@@ -300,8 +300,8 @@ hosts_get_done(struct tevent_req *subreq)
goto done;
}
- ret = sysdb_store_ssh_host(state->domain, state->name,
state->alias, now,
- attrs);
+ ret = sysdb_store_ssh_host(state->domain, state->name, state->alias,
+ state->domain->ssh_host_timeout, now, attrs);
if (ret != EOK) {
goto done;
}
diff --git a/src/tests/sysdb_ssh-tests.c b/src/tests/sysdb_ssh-tests.c
index 6a4c219..46f5c47 100644
--- a/src/tests/sysdb_ssh-tests.c
+++ b/src/tests/sysdb_ssh-tests.c
@@ -175,6 +175,7 @@ static int test_sysdb_store_ssh_host(struct
test_data *data)
ret = sysdb_store_ssh_host(data->ctx->domain,
data->hostname,
data->alias,
+ data->ctx->domain->ssh_host_timeout,
now,
data->attrs);
return ret;
diff --git a/src/tools/sss_cache.c b/src/tools/sss_cache.c
index 7cd5852..c72af4b 100644
--- a/src/tools/sss_cache.c
+++ b/src/tools/sss_cache.c
@@ -30,6 +30,7 @@
#include "db/sysdb.h"
#include "db/sysdb_services.h"
#include "db/sysdb_autofs.h"
+#include "db/sysdb_ssh.h"
#define INVALIDATE_NONE 0
#define INVALIDATE_USERS 1
@@ -37,22 +38,36 @@
#define INVALIDATE_NETGROUPS 4
#define INVALIDATE_SERVICES 8
#define INVALIDATE_AUTOFSMAPS 16
+#define INVALIDATE_SSH_HOSTS 32
#ifdef BUILD_AUTOFS
+#ifdef BUILD_SSH
#define INVALIDATE_EVERYTHING (INVALIDATE_USERS | INVALIDATE_GROUPS | \
INVALIDATE_NETGROUPS |
INVALIDATE_SERVICES | \
- INVALIDATE_AUTOFSMAPS)
-#else
+ INVALIDATE_AUTOFSMAPS |
INVALIDATE_SSH_HOSTS )
+#else /* BUILD_SSH */
+#define INVALIDATE_EVERYTHING (INVALIDATE_USERS | INVALIDATE_GROUPS | \
+ INVALIDATE_NETGROUPS |
INVALIDATE_SERVICES | \
+ INVALIDATE_AUTOFSMAPS )
+#endif /* BUILD_SSH */
+#else /* BUILD_AUTOFS */
+#ifdef BUILD_SSH
+#define INVALIDATE_EVERYTHING (INVALIDATE_USERS | INVALIDATE_GROUPS | \
+ INVALIDATE_NETGROUPS |
INVALIDATE_SERVICES | \
+ INVALIDATE_SSH_HOSTS )
+#else /* BUILD_SSH */
#define INVALIDATE_EVERYTHING (INVALIDATE_USERS | INVALIDATE_GROUPS | \
- INVALIDATE_NETGROUPS |
INVALIDATE_SERVICES)
-#endif
+ INVALIDATE_NETGROUPS |
INVALIDATE_SERVICES )
+#endif /* BUILD_SSH */
+#endif /* BUILD_AUTOFS */
enum sss_cache_entry {
TYPE_USER=0,
TYPE_GROUP,
TYPE_NETGROUP,
TYPE_SERVICE,
- TYPE_AUTOFSMAP
+ TYPE_AUTOFSMAP,
+ TYPE_SSH_HOST
};
static errno_t search_autofsmaps(TALLOC_CTX *mem_ctx,
@@ -69,18 +84,21 @@ struct cache_tool_ctx {
char *netgroup_filter;
char *service_filter;
char *autofs_filter;
+ char *ssh_host_filter;
char *user_name;
char *group_name;
char *netgroup_name;
char *service_name;
char *autofs_name;
+ char *ssh_host_name;
bool update_user_filter;
bool update_group_filter;
bool update_netgroup_filter;
bool update_service_filter;
bool update_autofs_filter;
+ bool update_ssh_host_filter;
};
errno_t init_domains(struct cache_tool_ctx *ctx, const char *domain);
@@ -152,6 +170,9 @@ int main(int argc, const char *argv[])
skipped &= !invalidate_entries(tctx, dinfo, TYPE_AUTOFSMAP,
tctx->autofs_filter,
tctx->autofs_name);
+ skipped &= !invalidate_entries(tctx, dinfo, TYPE_SSH_HOST,
+ tctx->ssh_host_filter,
+ tctx->ssh_host_name);
ret = sysdb_transaction_commit(sysdb);
if (ret != EOK) {
@@ -328,6 +349,14 @@ static errno_t update_all_filters(struct
cache_tool_ctx *tctx,
return ret;
}
+ /* Update ssh host filter */
+ ret = update_filter(tctx, dinfo, tctx->ssh_host_name,
+ tctx->update_ssh_host_filter, "(%s=%s)", false,
+ &tctx->ssh_host_filter);
+ if (ret != EOK) {
+ return ret;
+ }
+
return EOK;
}
@@ -371,6 +400,15 @@ static bool invalidate_entries(TALLOC_CTX *ctx,
type_string = "autofs map";
ret = search_autofsmaps(ctx, dinfo, filter, attrs,
&msg_count, &msgs);
break;
+ case TYPE_SSH_HOST:
+ type_string = "ssh_host";
+#ifdef BUILD_SSH
+ ret = sysdb_search_ssh_hosts(ctx, dinfo,
+ filter, attrs, &msg_count, &msgs);
+#else /* BUILD_SSH */
+ ret = ENOSYS;
+#endif /* BUILD_SSH */
+ break;
}
if (ret != EOK) {
@@ -446,6 +484,14 @@ static errno_t invalidate_entry(TALLOC_CTX *ctx,
ret = sysdb_set_autofsmap_attr(domain, name,
sys_attrs,
SYSDB_MOD_REP);
break;
+ case TYPE_SSH_HOST:
+#ifdef BUILD_SSH
+ ret = sysdb_set_ssh_host_attr(domain, name,
+ sys_attrs,
SYSDB_MOD_REP);
+#else /* BUILD_SSH */
+ ret = ENOSYS;
+#endif /* BUILD_SSH */
+ break;
default:
return EINVAL;
}
@@ -530,6 +576,7 @@ errno_t init_context(int argc, const char *argv[],
struct cache_tool_ctx **tctx)
char *netgroup = NULL;
char *service = NULL;
char *map = NULL;
+ char *ssh_host = NULL;
char *domain = NULL;
int debug = SSSDBG_DEFAULT;
errno_t ret = EOK;
@@ -563,6 +610,12 @@ errno_t init_context(int argc, const char
*argv[], struct cache_tool_ctx **tctx)
{ "autofs-maps", 'A', POPT_ARG_NONE, NULL, 'a',
_("Invalidate all autofs maps"), NULL },
#endif /* BUILD_AUTOFS */
+#ifdef BUILD_SSH
+ { "ssh_host", 'h', POPT_ARG_STRING, &ssh_host, 0,
+ _("Invalidate particular SSH host"), NULL },
+ { "ssh_hosts", 'H', POPT_ARG_NONE, NULL, 'h',
+ _("Invalidate all SSH hosts"), NULL },
+#endif /* BUILD_SSH */
{ "domain", 'd', POPT_ARG_STRING, &domain, 0,
_("Only invalidate entries from a particular domain"),
NULL },
POPT_TABLEEND
@@ -594,6 +647,9 @@ errno_t init_context(int argc, const char *argv[],
struct cache_tool_ctx **tctx)
case 'a':
idb |= INVALIDATE_AUTOFSMAPS;
break;
+ case 'h':
+ idb |= INVALIDATE_SSH_HOSTS;
+ break;
case 'e':
idb = INVALIDATE_EVERYTHING;
break;
@@ -608,7 +664,7 @@ errno_t init_context(int argc, const char *argv[],
struct cache_tool_ctx **tctx)
}
if (idb == INVALIDATE_NONE && !user && !group &&
- !netgroup && !service && !map) {
+ !netgroup && !service && !map && !ssh_host) {
BAD_POPT_PARAMS(pc,
_("Please select at least one object to invalidate\n"),
ret, fini);
@@ -665,14 +721,26 @@ errno_t init_context(int argc, const char
*argv[], struct cache_tool_ctx **tctx)
ctx->update_autofs_filter = true;
}
+ if (idb & INVALIDATE_SSH_HOSTS) {
+ ctx->ssh_host_filter = talloc_asprintf(ctx, "(%s=*)",
SYSDB_NAME);
+ ctx->update_ssh_host_filter = false;
+ } else if (ssh_host) {
+ ctx->ssh_host_name = talloc_strdup(ctx, ssh_host);
+ ctx->update_ssh_host_filter = true;
+ }
+
if (((idb & INVALIDATE_USERS) && !ctx->user_filter) ||
((idb & INVALIDATE_GROUPS) && !ctx->group_filter) ||
((idb & INVALIDATE_NETGROUPS) && !ctx->netgroup_filter) ||
((idb & INVALIDATE_SERVICES) && !ctx->service_filter) ||
((idb & INVALIDATE_AUTOFSMAPS) && !ctx->autofs_filter) ||
- (user && !ctx->user_name) || (group && !ctx->group_name)
||
- (netgroup && !ctx->netgroup_name) || (map &&
!ctx->autofs_name) ||
- (service && !ctx->service_name)) {
+ ((idb & INVALIDATE_SSH_HOSTS) && !ctx->ssh_host_filter) ||
+ (user && !ctx->user_name) ||
+ (group && !ctx->group_name) ||
+ (netgroup && !ctx->netgroup_name) ||
+ (service && !ctx->service_name) ||
+ (map && !ctx->autofs_name) ||
+ (ssh_host && !ctx->ssh_host_name)) {
DEBUG(SSSDBG_CRIT_FAILURE, "Construction of filters failed\n");
ret = ENOMEM;
goto fini;
--