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