From owner-freebsd-hackers@FreeBSD.ORG Thu Nov 3 13:06:32 2005 Return-Path: X-Original-To: hackers@FreeBSD.org Delivered-To: freebsd-hackers@FreeBSD.ORG Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id B125C16A41F; Thu, 3 Nov 2005 13:06:32 +0000 (GMT) (envelope-from bushman@rsu.ru) Received: from mail.r61.net (mail.r61.net [195.208.245.249]) by mx1.FreeBSD.org (Postfix) with ESMTP id 5EAA743D5D; Thu, 3 Nov 2005 13:06:28 +0000 (GMT) (envelope-from bushman@rsu.ru) Received: from [195.208.252.201] (celsius.cc.rsu.ru [195.208.252.201]) by mail.r61.net (8.13.4/8.13.4) with ESMTP id jA3D6QNu067009; Thu, 3 Nov 2005 16:06:26 +0300 (MSK) (envelope-from bushman@rsu.ru) Message-ID: <436A0C73.3010405@rsu.ru> Date: Thu, 03 Nov 2005 16:11:15 +0300 From: Michael Bushkov User-Agent: Mozilla Thunderbird 1.0.7 (X11/20051018) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Ceri Davies References: <20051102001507.GB14638@odin.ac.hmc.edu> <20051103124027.GE29387@submonkey.net> In-Reply-To: <20051103124027.GE29387@submonkey.net> Content-Type: text/plain; charset=windows-1251; format=flowed Content-Transfer-Encoding: 7bit X-Virus-Scanned: ClamAV version 0.86.2, clamav-milter version 0.86 on asterix.r61.net X-Virus-Status: Clean Cc: hackers@FreeBSD.org, current@FreeBSD.org Subject: Re: nsswitch reviewer wanted X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Nov 2005 13:06:32 -0000 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