Hello, Ceri!
Ceri Davies wrote:
I realise that you weren't asking for comments, but I took a quick look
at http://www.rsu.ru/~bushman/nsswitch_cached/nss_cached.patch and have
some. I'll send this to the original lists too.
Thanks for comments! I need them much.
o There aren't nearly enough comments. Granted, I'm not a C aficionado,
but it's ~ line 3200 of that patch before we come to a new non-trivial
comment, and there aren't many after that.
Ok - I'll comment the code.
o cached/config.c has magic numbers in create_def_configuration_entry(),
which probably belong as #defines in config.h instead. I'm not sure
what style(9) says about that though, so am happy to be ignored.
Yes, that's right - I guess, the most correct way is to move them to
config.h
o There is a single mention of a ssh_hostkeys cache in
include/nsswitch.h, and no code to implement it.
I've implemented the patch for OpenSSH, which allows it to use nsswitch
for the hostkeys. It should be committed by the OpenSSH team. That's why
the ssh_hostkeys cache is mentioned in the nsswitch.h. This line can
easialy be removed - as it doesn't affect anything.
o On line 15448, there is a whitespace nit. Also, in this area, are we
sure that there is no benefit in continuing to key by euid/egid if
perform_actual_lookup is enabled; can we send the euid/egid with the
lookup request?
You are talking about passing euid/egid to the underlying nsswitch
modules, right? This will require significant changes in these modules,
and, as far as I'm converned, won't gain us any benefits.
I can't see any benefit of keying by euid/egud when the
'perform_actual_lookups' mode is enabled. By ignoring them, we make the
cache common for all users, and no user can poison it - because all
requests are made solely by ourselves. If we won't ignore the euid/egid,
then for every user, we'll have to make similar requests, this will also
affect the cache size.
When perform_actual_lookups mode is off, cache should be certainly
separated by eud/egid for (basically) security reasons.
o A user should be able to invalidate one of their caches (e.g.,
"cached -i hosts" to flush their hosts cache). Root should be able
to do it for all users with a single command (e.g., "cached -I hosts"
to flush all hosts caches).
Ok - I'll do that.
o The manual for cached.conf is unclear over whether it's OK to name
an "unknown" cache in cached.conf.
I'll corect this. In fact, it's ok to do that.
o The location of cached.conf is defaulted to /usr/local/etc in
cached/cached.c and should be changed on commit.
Will be corrected.
Ceri
--
Michael
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
