nsswitch reviewer wanted
bushman at rsu.ru
Thu Nov 3 05:06:32 PST 2005
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
>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.
More information about the freebsd-hackers