Spec URL: http://www.benhur.prf.cuni.cz/medved-7/wydobitki/fedora/iwak/iwak.spec SRPM URL: http://www.benhur.prf.cuni.cz/medved-7/wydobitki/fedora/iwak/iwak-2.2-1.fc12.src.rpm Description: Detect the openssh keys affected by CVE-2008-0166 among authorized_keys. This is done by computing the fingerprints from each authorized key and then comparing against the databaze of blacklisted fingerprints.
Few thoughts from a quick look at the iwak script itself, rather than RPM package: - Any specific reason to avoid getopt to parse command line args? /usr/share/doc/util-linux-ng-* has some easy to use examples. - You seem to create TMPFILE rather early and later have to delete it if one of the error conditions are met. At least one rm seem to be missing there, so may be easier to create it later when you're really going to need it. - You can save few extra commands / redirections by doing changes like: grep something file > /dev/null --> grep -q something file cat file | wc -l --> wc -l file - This should have a README with big warning sysadmins should not to be tempted to use this as root as: for u in /home/* ; do [ -f $u/.ssh/authorized_keys ] && iwak $u/.ssh/authorized_keys ; done Just my 2c...
(In reply to comment #1) > iwak $u/.ssh/authorized_keys iwak -d, of course...
> > - You seem to create TMPFILE rather early and later have to delete it if one of > the error conditions are met. At least one rm seem to be missing there, so may > be easier to create it later when you're really going to need it. > done > - You can save few extra commands / redirections by doing changes like: > > grep something file > /dev/null --> grep -q something file I do not like extra switches if not necessary > cat file | wc -l --> wc -l file This is not the same (try it :) > > - This should have a README with big warning sysadmins should not to be tempted > to use this as root as: > May be, but the sysadmins will do it anyhow.
New version: Spec URL: http://www.benhur.prf.cuni.cz/medved-7/wydobitki/fedora/iwak/iwak.spec SRPM URL: http://www.benhur.prf.cuni.cz/medved-7/wydobitki/fedora/iwak/iwak-2.4-1.fc12.src.rpm
+ FAIL: rpmlint is silent on both source and binary package. bradford:rpmbuild$ rpmlint iwak-2.4-1.fc11.src.rpm iwak.src: E: description-line-too-long Detect the openssh keys affected by CVE-2008-0166 among authorized_keys. This is done by computing the fingerprints from iwak.src: E: description-line-too-long each authorized key and then comparing against the databaze of blacklisted fingerprints. 1 packages and 0 specfiles checked; 2 errors, 0 warnings. bradford:rpmbuild$ + GOOD: The package is named according to the Package Naming Guidelines . + GOOD: The spec file name matches the base package %{name}, in the format %{name}.spec. + GOOD: The package meets the Packaging Guidelines. + GOOD: The package is licensed with a Fedora approved license and meet the Licensing Guidelines. + GOOD: The License field in the package spec file matches the actual license. Well, it would be better if this GPL blurb was in the top of the script itself: iawk - detects the openssh authorized_keys affected by CVE-2008-0166 Copyright (C) 2009 Jan F. Chadima Copyright (C) 2009 Red Hat Inc. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 2 of the License. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with this program; if not, write to the Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + GOOD: The spec file is written in American English. + GOOD: The spec file for the package is legible. + GOOD: The sources used to build the package matches the upstream source, as provided in the spec URL. SHA1: c9567e1590d75afa080102096b0ba19a49834f3a + GOOD: The package successfully compiles and build into binary rpms on at least one supported architecture. It is noarch -- koji scratch build is http://koji.fedoraproject.org/koji/taskinfo?taskID=1494766 + GOOD: builds on all architectures noarch + GOOD: All build dependencies are listed in BuildRequires. (builds in koji) + GOOD: The spec file MUST handle locales properly. No locale support. + GOOD: no libraries + GOOD: not relocatable + GOOD: A package owns all directories that it creates. + GOOD: A package must not contain any duplicate files in the %files listing. + GOOD: Permissions on files must be set properly. + GOOD: Each package have a %clean section. + GOOD: Each package consistently use macros. + GOOD: The package contains code, or permissable content. + GOOD: No large documentation files, so no a -doc subpackage. + GOOD: Files registered in %doc does not affect the runtime of the application. + GOOD: No header files. + GOOD: No static libraries. + GOOD: No pkgconfig(.pc) files. + GOOD: The package does not contain library files with a suffix. + GOOD: No devel packages. + GOOD: No .la libtool archives. + GOOD: Packages does not contain GUI applications. + GOOD: Packages does not own files or directories owned by other packages. + GOOD: Runs rm -rf $RPM_BUILD_ROOT in %install + GOOD: All filenames in rpm packages are valid UTF-8. + GOOD: Includes license text. Please fix or explain above show issues.
New version: Spec URL: http://www.benhur.prf.cuni.cz/medved-7/wydobitki/fedora/iwak/iwak.spec SRPM URL: http://www.benhur.prf.cuni.cz/medved-7/wydobitki/fedora/iwak/iwak-2.4-2.fc12.src.rpm
Still no copyright statement in the script, but that's probably not enough to stop this package from being APPROVED.
New Package CVS Request ======================= Package Name: iwak Short Description: Detect the openssh keys affected by CVE-2008-0166 among authorized_keys Owners: jfch2222 Branches: F-10 F-11 InitialCC:
How did this ticket get reviewed without being assigned to anyone? I fixed that up. CVS done.
(In reply to comment #9) > How did this ticket get reviewed without being assigned to anyone? I fixed > that up. Sorry, lazy @redhat.com bastards just did review over IRC and reviewer was lazy to be a good bureaucrat.
(In reply to comment #3) > > cat file | wc -l --> wc -l file > This is not the same (try it :) Ah, right, more chopping needed, probably not worth it. > > - This should have a README with big warning sysadmins should not to be > > tempted to use this as root as: > > May be, but the sysadmins will do it anyhow. Probably worth a safety-net check in some future revisions? Test checking if euid is the same as file's and file's parent directory owner are the same and refusing to delete if not should catch most obvious mistakes (well, it'll turn it into race), or mktemp-created file instead of $FILE.tmp? Really just some quick thoughts...
ping? what's going on with this package. Has it been already built in Rawhide? Why it hasn't been closed?