Request for review: keychain

[Chris Grau]

On Mon, Jul 11, 2005 at 10:10:35PM -0400, Ed Hill wrote:
> On Tue, 2005-07-12 at 01:52 +0200, Alexander Dalloz wrote:
> > keychain: agent manager for OpenSSH, ssh.com, Sun SSH, and GnuPG
> 
> > http://www.uni-x.org/keychain.spec
> > http://www.uni-x.org/keychain-2.5.4.1-1.src.rpm
> 
> 
> Hi Alexander,
> 
> Hey, thats funny!  I've been referring people to those well-written IBM
> developerWorks articles on SSH:
> 
>   http://www-106.ibm.com/developerworks/linux/library/l-keyc2/
> 
> that are referenced from the main keychain web site but I never actually
> took the time to learn about or use keychain itself.  I just use ssh-add
> and ssh-agent.  Silly me!
> 
> So heres a quick review of keychain which mostly looks good:
> 
> please fix:
>  - rpmlint complains:
>      W: keychain summary-not-capitalized agent manager ...
>  - BuildRoot should be:
>      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
>  - please add dist to version:  2.5.4.1%{?dist}

That shouldn't be added to revision rather than version?

> good:
>  - names OK
>  - spec mostly OK (except above) -- *very* simple!
>  - src matches upstream
>  - license OK
>  - seems to build, install, and work on FC3
>  - cleans OK
>  - perms OK
>  - code not content OK
> 
> And, if possible, it would be nice if we could get a second opinion on
> the security implications of the actual "keychain" shell script.  I'm
> *no* security guru and maybe someone more knowledgeable could step in
> and say the script looks OK.  Or that its probably OK since its widely
> used and widely reviewed...?
> 
> Or, is that just too much to ask for as part of the package review
> process?

The script is pretty long, but I've skimmed it in the past.  I'm not
really an expert, but it's just a fancy wrapper around ssh-agent and
ssh-add, so the security implications are more or less passed on to
those programs.

> Ed -- who doesn't want to become known as "that moron who said it was 
>       just dandy to include a security nightmare in Extras"  ;-)

-chris -- who will probably become known as "that moron who seconded
          Ed's dandy opinion"