On 10/01/2012 10:23 AM, Jan Cholasta wrote:
Hi,
Dne 24.9.2012 15:56, Jan Cholasta napsal(a):
Dne 24.9.2012 15:03, Pavel Březina napsal(a):
On 09/19/2012 12:09 PM, Jan Cholasta wrote:
Hi,
this patch set changes the way the known_hosts file is updated so that
only entries for hosts that were requested recently (in the last 5
minutes) are written to the file. There is no need to keep older
entries
in the file, as host keys are needed only before the connection to the
host is established (which usually takes just a few seconds).
The individual patches are:
[PATCH 1/3] DB: Add function for deleting values from sysdb_attrs
Nack.
Can you make new copy of attrs without the deleted attributes instead of
touching the original attrs?
Sure.
It turns out I don't need this function anymore, so this patch is removed.
[PATCH 2/3] SSH: Refactor sysdb code
Nack.
sysdb_update_ssh_host(), sysdb_store_ssh_host():
Is there any advantage to allow the input attrs parameter to be NULL?
Currently there is no code flow how this could happened (except sysdb
test) so I would simply prohibit it and make the code simpler.
I believe that was a copy & paste from some other sysdb function, you
are probably right that it can be safely removed.
Done.
sysdb_update_ssh_host():
What is the purpose of deleting OC and NAME attributes and then adding
them again?
ldb complains when the same value is present more than once in a single
attribute (such as name) and refuses to store the entry.
I have taken a slightly different approach here: instead of deleting
unwanted attributed in sysdb functions, I copy only host keys from the
LDAP result in hostid provider (the rest of attributes is not
interesting from SSH perspective anyway).
[PATCH 3/3] SSH: Expire hosts in known_hosts
Ack.
Added a new configuration option ssh_known_hosts_timeout, which can be
used to specify the timeout of known_hosts entries in seconds. Also
added a sysdb upgrade routine so that this patch works for existing
installs as well.
Nack (nitpick).
sysdb_ssh.c:125 - you have "if (alias)" inside "if (alias)". Can you put
a comment there that would say that the value may change during
execution? Either that or you can use a boolean variable that would
indicate present of the alias in attrs. Just make sure that it is clear
from the first sight...
Otherwise ack. Good job!
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
