Bug 225974

Summary: Merge Review: krb5
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Gwyn Ciesla <gwync>
Status: CLOSED NOTABUG QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: a.badger, gwync, nalin, pmatilai, redhat-bugzilla
Target Milestone: ---Flags: gwync: fedora-review+
a.badger: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-06-17 15:32:37 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 426387    

Description Nobody's working on this, feel free to take it 2007-01-31 19:16:53 UTC
Fedora Merge Review: krb5

http://cvs.fedora.redhat.com/viewcvs/devel/krb5/
Initial Owner: nalin

Comment 1 Robert Scheck 2007-02-18 22:38:07 UTC
/etc/profile.d/krb5-devel.{sh,csh} is 755, but should be 644.

Comment 2 Gwyn Ciesla 2008-09-17 19:48:30 UTC
rpmlint on SRPM:

krb5.src:109: E: prereq-use grep, info, sh-utils, /sbin/install-info
The use of PreReq is deprecated. In the majority of cases, a plain Requires is
enough and the right thing to do. Sometimes Requires(pre), Requires(post),
Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

krb5.src:110: E: buildprereq-use autoconf, bison, e2fsprogs-devel >= 1.35, flex, gawk
The use of BuildPreReq is deprecated, build dependencies are always required
before a package can be built.  Use plain BuildRequires instead.

krb5.src:111: E: buildprereq-use gzip, ncurses-devel, rsh, texinfo, texinfo-tex, tar
The use of BuildPreReq is deprecated, build dependencies are always required
before a package can be built.  Use plain BuildRequires instead.

krb5.src:144: E: prereq-use grep, /sbin/ldconfig, sh-utils
The use of PreReq is deprecated. In the majority of cases, a plain Requires is
enough and the right thing to do. Sometimes Requires(pre), Requires(post),
Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

krb5.src:145: W: unversioned-explicit-obsoletes krb5-configs
The specfile contains an unversioned Obsoletes: token, which will match all
older, equal and newer versions of the obsoleted thing.  This may cause update
problems, restrict future package/provides naming, and may match something it
was originally not inteded to match -- make the Obsoletes versioned if
possible.

krb5.src:156: E: prereq-use grep, /sbin/install-info, /bin/sh, sh-utils, /sbin/chkconfig
The use of PreReq is deprecated. In the majority of cases, a plain Requires is
enough and the right thing to do. Sometimes Requires(pre), Requires(post),
Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

krb5.src:181: E: prereq-use grep, /sbin/install-info, /bin/sh, sh-utils
The use of PreReq is deprecated. In the majority of cases, a plain Requires is
enough and the right thing to do. Sometimes Requires(pre), Requires(post),
Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

krb5.src:196: E: prereq-use grep, /sbin/install-info, /bin/sh, sh-utils
The use of PreReq is deprecated. In the majority of cases, a plain Requires is
enough and the right thing to do. Sometimes Requires(pre), Requires(post),
Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

krb5.src:211: E: prereq-use grep, /sbin/install-info, /bin/sh, sh-utils
The use of PreReq is deprecated. In the majority of cases, a plain Requires is
enough and the right thing to do. Sometimes Requires(pre), Requires(post),
Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

krb5.src:1076: W: macro-in-%changelog _var
Macros are expanded in %changelog too, which can in unfortunate cases lead to
the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally odd
entries eg. in source rpms, which is rarely wanted.  Avoid use of macros in
%changelog altogether, or use two '%'s to escape them, like '%%foo'.

krb5.src:1113: W: macro-in-%changelog post
Macros are expanded in %changelog too, which can in unfortunate cases lead to
the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally odd
entries eg. in source rpms, which is rarely wanted.  Avoid use of macros in
%changelog altogether, or use two '%'s to escape them, like '%%foo'.

krb5.src:1191: W: macro-in-%changelog _var
Macros are expanded in %changelog too, which can in unfortunate cases lead to
the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally odd
entries eg. in source rpms, which is rarely wanted.  Avoid use of macros in
%changelog altogether, or use two '%'s to escape them, like '%%foo'.

krb5.src:1237: W: macro-in-%changelog version
Macros are expanded in %changelog too, which can in unfortunate cases lead to
the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally odd
entries eg. in source rpms, which is rarely wanted.  Avoid use of macros in
%changelog altogether, or use two '%'s to escape them, like '%%foo'.

Fix.

krb5.src:1399: E: use-of-RPM_SOURCE_DIR
You use $RPM_SOURCE_DIR or %{_sourcedir} in your spec file. If you have to use
a directory for building, use $RPM_BUILD_ROOT instead.

Fix or explain need in spec.

krb5.src:1490: W: make-check-outside-check-section : make check TMPDIR=%{_tmppath}
Make check or other automated regression test should be run in %check, as they
can be disabled with a rpm macro for short circuiting purposes.

Fix or explain placement in spec.

krb5.src: W: mixed-use-of-spaces-and-tabs (spaces: line 325, tab: line 1400)
The specfile mixes use of spaces and tabs for indentation, which is a cosmetic
annoyance.  Use either spaces or tabs for indentation, not both.

Fix.

krb5.src: W: patch-not-applied Patch26: krb5-1.3.2-efence.patch
A patch is included in your package but was not applied. Refer to the patches
documentation to see what's wrong.

krb5.src: W: patch-not-applied Patch55: krb5-1.6.1-empty.patch
A patch is included in your package but was not applied. Refer to the patches
documentation to see what's wrong.

krb5.src: W: patch-not-applied Patch57: krb5-1.6.2-login_chdir.patch
A patch is included in your package but was not applied. Refer to the patches
documentation to see what's wrong.

krb5.src: W: patch-not-applied Patch64: krb5-ok-as-delegate.patch
A patch is included in your package but was not applied. Refer to the patches
documentation to see what's wrong.

krb5.src: W: patch-not-applied Patch70: krb5-trunk-kpasswd_tcp2.patch
A patch is included in your package but was not applied. Refer to the patches
documentation to see what's wrong.

Apply, drop or explain in spec.

krb5.src: W: summary-ended-with-dot The Kerberos network authentication system.
Summary ends with a dot.

Fix.

krb5.src: W: strange-permission krb5kdc.init 0755
A file that you listed to include in your package has strange permissions.
Usually, a file should have 0644 permissions.

krb5.src: W: strange-permission krb5.sh 0755
A file that you listed to include in your package has strange permissions.
Usually, a file should have 0644 permissions.

krb5.src: W: strange-permission kpropd.init 0755
A file that you listed to include in your package has strange permissions.
Usually, a file should have 0644 permissions.

krb5.src: W: strange-permission krb524d.init 0755
A file that you listed to include in your package has strange permissions.
Usually, a file should have 0644 permissions.

krb5.src: W: strange-permission kadmind.init 0755
A file that you listed to include in your package has strange permissions.
Usually, a file should have 0644 permissions.

krb5.src: W: strange-permission krb5.csh 0755
A file that you listed to include in your package has strange permissions.
Usually, a file should have 0644 permissions.

krb5.src: W: strange-permission krb5-tex-pdf.sh 0755
A file that you listed to include in your package has strange permissions.
Usually, a file should have 0644 permissions.

Fix or document in spec.

rpmlint on RPMS:

krb5-devel.i386: W: spurious-executable-perm /usr/share/doc/krb5-devel-1.6.3/krb5-protocol/draft-jaganathan-rc4-hmac-03.txt
The file is installed with executable permissions, but was identified as one
that probably should not be executable.  Verify if the executable bits are
desired, and remove if not.

krb5-devel.i386: W: summary-ended-with-dot Development files needed to compile Kerberos 5 programs.
Summary ends with a dot.

Fix.

krb5-devel.i386: W: non-standard-dir-in-usr kerberos
Your package is creating a non-standard subdirectory in /usr. The standard
directories are: X11R6, X386, bin, games, include, lib, lib64, local, sbin,
share, src, spool, tmp.

Probably fine.

krb5-libs.i386: W: hidden-file-or-dir /usr/kerberos/man/man5/.k5login.5.gz
The file or directory is hidden. You should see if this is normal, and delete
it from the package if not.

???????????

krb5-libs.i386: W: summary-ended-with-dot The shared libraries used by Kerberos 5.
Summary ends with a dot.

krb5-libs.i386: W: obsolete-not-provided krb5-configs
If a package is obsoleted by a compatible replacement, the obsoleted package
must also be provided in order to provide clean upgrade paths and not cause
unnecessary dependency breakage.  If the obsoleting package is not a
compatible replacement for the old one, leave out the provides.

Fix.

krb5-libs.i386: W: non-standard-dir-in-usr kerberos
Your package is creating a non-standard subdirectory in /usr. The standard
directories are: X11R6, X386, bin, games, include, lib, lib64, local, sbin,
share, src, spool, tmp.

Fine.

krb5-pkinit-openssl.i386: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

Fix if possible.

krb5-pkinit-openssl.i386: W: summary-ended-with-dot The PKINIT module for Kerberos 5.
Summary ends with a dot.

Fix.

krb5-server.i386: E: executable-marked-as-config-file /etc/rc.d/init.d/krb524
Executables must not be marked as config files because that may prevent
upgrades from working correctly. If you need to be able to customize an
executable, make it for example read a config file in /etc/sysconfig.

krb5-server.i386: E: executable-marked-as-config-file /etc/rc.d/init.d/krb5kdc
Executables must not be marked as config files because that may prevent
upgrades from working correctly. If you need to be able to customize an
executable, make it for example read a config file in /etc/sysconfig.

krb5-server.i386: E: executable-marked-as-config-file /etc/rc.d/init.d/kadmin
Executables must not be marked as config files because that may prevent
upgrades from working correctly. If you need to be able to customize an
executable, make it for example read a config file in /etc/sysconfig.

krb5-server.i386: E: executable-marked-as-config-file /etc/rc.d/init.d/kprop
Executables must not be marked as config files because that may prevent
upgrades from working correctly. If you need to be able to customize an
executable, make it for example read a config file in /etc/sysconfig.

I suspect this is necessary, but you might want to comment on why in the spec.

krb5-server.i386: W: summary-ended-with-dot The KDC and related programs for Kerberos 5.
Summary ends with a dot.

Fix.

krb5-server.i386: W: conffile-without-noreplace-flag /etc/rc.d/init.d/kadmin
A configuration file is stored in your package without the noreplace flag. A
way to resolve this is to put the following in your SPEC file:
%config(noreplace) /etc/your_config_file_here

krb5-server.i386: W: conffile-without-noreplace-flag /etc/rc.d/init.d/kprop
A configuration file is stored in your package without the noreplace flag. A
way to resolve this is to put the following in your SPEC file:
%config(noreplace) /etc/your_config_file_here

krb5-server.i386: W: conffile-without-noreplace-flag /etc/rc.d/init.d/krb524
A configuration file is stored in your package without the noreplace flag. A
way to resolve this is to put the following in your SPEC file:
%config(noreplace) /etc/your_config_file_here

krb5-server.i386: W: conffile-without-noreplace-flag /etc/rc.d/init.d/krb5kdc
A configuration file is stored in your package without the noreplace flag. A
way to resolve this is to put the following in your SPEC file:
%config(noreplace) /etc/your_config_file_here

Fix, unless breakage would ensue.

krb5-server.i386: W: non-standard-dir-in-usr kerberos
Your package is creating a non-standard subdirectory in /usr. The standard
directories are: X11R6, X386, bin, games, include, lib, lib64, local, sbin,
share, src, spool, tmp.

krb5-server.i386: W: non-standard-dir-in-var kerberos
Your package is creating a non-standard subdirectory in /var. The standard
directories are: account, lib, cache, crash, games, lock, log, opt, run,
spool, state, tmp, yp, www, ftp.

Fine.

krb5-server.i386: W: no-reload-entry /etc/rc.d/init.d/krb524
In your init script (/etc/rc.d/init.d/your_file), you don't have a 'reload'
entry, which is necessary for good functionality.

krb5-server.i386: W: no-reload-entry /etc/rc.d/init.d/kprop
In your init script (/etc/rc.d/init.d/your_file), you don't have a 'reload'
entry, which is necessary for good functionality.

Fix.  If the software doesn't support it, just mirror the restart entry.

krb5-server-ldap.i386: W: devel-file-in-non-devel-package /usr/lib/libkdb_ldap.so
A development file (usually source code) is located in a non-devel package. If
you want to include source code in your package, be sure to create a
development package.

Fine.

krb5-server-ldap.i386: E: library-without-ldconfig-postin /usr/lib/libkdb_ldap.so.1.0
This package contains a library and provides no %post scriptlet containing a
call to ldconfig.

krb5-server-ldap.i386: E: library-without-ldconfig-postun /usr/lib/libkdb_ldap.so.1.0
This package contains a library and provides no %postun scriptlet containing a
call to ldconfig.

Probably needs fixing.

krb5-server-ldap.i386: W: summary-ended-with-dot The LDAP storage plugin for the Kerberos 5 KDC.
Summary ends with a dot.

Fix.

krb5-server-ldap.i386: W: non-standard-dir-in-usr kerberos
Your package is creating a non-standard subdirectory in /usr. The standard
directories are: X11R6, X386, bin, games, include, lib, lib64, local, sbin,
share, src, spool, tmp.

Fix.

krb5-workstation.i386: E: setuid-binary /usr/kerberos/bin/ksu root 04755
The file is setuid, this may be dangerous, especially if this  file is setuid
root.

Necessary, I suspect.

krb5-workstation.i386: E: non-standard-executable-perm /usr/kerberos/bin/ksu 04755
A standard executable should have permission set to 0755. If you get this
message, it means that you have a wrong executable permissions in some files
included in your package.

See above.

krb5-workstation.i386: W: summary-ended-with-dot Kerberos 5 programs for use on workstations.
Summary ends with a dot.

Fix.

krb5-workstation.i386: W: unstripped-binary-or-object /usr/kerberos/bin/ksu

Fix if possible.

krb5-workstation.i386: W: non-standard-dir-in-usr kerberos
Your package is creating a non-standard subdirectory in /usr. The standard
directories are: X11R6, X386, bin, games, include, lib, lib64, local, sbin,
share, src, spool, tmp.

Fine.

krb5-workstation-clients.i386: E: info-files-without-install-info-postin /usr/share/info/krb5-user.info.gz
This package contains info files and provides no %post scriptlet containing a
call to install-info.

krb5-workstation-clients.i386: E: info-files-without-install-info-postun /usr/share/info/krb5-user.info.gz
This package contains info files and provides no %postun scriptlet containing
a call to install-info.

Fix.

krb5-workstation-clients.i386: W: summary-ended-with-dot Kerberos 5 clients for use on workstations.
Summary ends with a dot.

Fix.

krb5-workstation-clients.i386: W: unstripped-binary-or-object /usr/kerberos/bin/v4rcp

Fix if possible.

krb5-workstation-clients.i386: W: non-standard-dir-in-usr kerberos
Your package is creating a non-standard subdirectory in /usr. The standard
directories are: X11R6, X386, bin, games, include, lib, lib64, local, sbin,
share, src, spool, tmp.

Fine.

krb5-workstation-servers.i386: E: executable-marked-as-config-file /etc/rc.d/init.d/krb524
Executables must not be marked as config files because that may prevent
upgrades from working correctly. If you need to be able to customize an
executable, make it for example read a config file in /etc/sysconfig.

Probably fine.

krb5-workstation-servers.i386: W: summary-ended-with-dot Kerberos 5 servers for use on workstations.
Summary ends with a dot.

Fix.

krb5-workstation-servers.i386: W: unstripped-binary-or-object /usr/kerberos/bin/v4rcp
krb5-workstation-servers.i386: W: conffile-without-noreplace-flag /etc/rc.d/init.d/krb524
A configuration file is stored in your package without the noreplace flag. A
way to resolve this is to put the following in your SPEC file:
%config(noreplace) /etc/your_config_file_here

Fix if possible.

krb5-workstation-servers.i386: W: non-standard-dir-in-usr kerberos
Your package is creating a non-standard subdirectory in /usr. The standard
directories are: X11R6, X386, bin, games, include, lib, lib64, local, sbin,
share, src, spool, tmp.

Fine.

krb5-workstation-servers.i386: E: postin-without-chkconfig /etc/rc.d/init.d/krb524
The package contains an init script but doesn't call chkconfig in its %post.

krb5-workstation-servers.i386: E: init-script-without-chkconfig-preun /etc/rc.d/init.d/krb524
The package contains an init script but doesn't contain a %preun with a call
to chkconfig.

Probably needs fixing.

krb5-workstation-servers.i386: W: no-reload-entry /etc/rc.d/init.d/krb524
In your init script (/etc/rc.d/init.d/your_file), you don't have a 'reload'
entry, which is necessary for good functionality.

See above.

krb5-workstation-servers.i386: W: incoherent-init-script-name krb524
The init script name should be the same as the package name in lower case, or
one with 'd' appended if it invokes a process by that name.

Fix or explain.

Source: tag lacks a URL, so I can't check the md5sum.

Working on a local mock build to test BuildRequires.  I'll post back when it's done.

Other than the above, full review is OK.

Comment 3 Gwyn Ciesla 2008-09-18 12:18:50 UTC
Mock build good, BuildRequires are fine.

Comment 4 Gwyn Ciesla 2008-12-04 19:34:00 UTC
Ping?

Comment 5 Gwyn Ciesla 2009-03-31 15:16:23 UTC
Ping again?

Comment 6 Nalin Dahyabhai 2009-04-02 03:03:48 UTC
Hi Jon, thanks for your patience with this -- it's a lot to wade through every time I find a few minutes to poke at this one.

I think the thorniest part of it is the part where krb524d is in two subpackages, because figuring out when it's safe/correct to remove the chkconfig symlinks and shut down the daemon isn't lending itself to any elegant solutions.

Since all v4-related items are going to be dropped in the upstream 1.7 release, I figure it can be side-stepped by switching off v4 functionality and just triggering on a package version ugprade.  The downside is that it's way too late to be switching off the v4 bits for F11, particularly as it removes libkrb4.so and the hooks libkrb4.so needs from libkrb5.so.  Needing those hooks to be there for a recent libkrb4.so to work properly makes the odds of adding a compat package highly unlikely.  I'm requesting an early branch so that making this and other review-related changes won't create any unexpected problems as it churns.

Package Change Request
======================
Package Name: krb5
New Branches: F11

Comment 7 Gwyn Ciesla 2009-04-02 12:51:04 UTC
Ok, that sounds reasonable.  I'll await fixes and/or commentary once the new branch is done.

Comment 8 Kevin Fenzi 2009-04-03 20:43:13 UTC
cvs done.

Comment 9 Nalin Dahyabhai 2009-04-08 21:35:29 UTC
(In reply to comment #2)

Okay, finally worked through most of these.  Comments in-line, apologies in advance for the length, I'll try to cut down duplicates.

> rpmlint on SRPM:
> 
> krb5.src:109: E: prereq-use grep, info, sh-utils, /sbin/install-info
> The use of PreReq is deprecated. In the majority of cases, a plain Requires is
> enough and the right thing to do. Sometimes Requires(pre), Requires(post),
> Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

Cleaned these up to Requires(post)/Requires(preun)/Requires(postun) for the subpackages which include scriptlets.

> krb5.src:110: E: buildprereq-use autoconf, bison, e2fsprogs-devel >= 1.35,
> flex, gawk
> The use of BuildPreReq is deprecated, build dependencies are always required
> before a package can be built.  Use plain BuildRequires instead.

Done.

> krb5.src:145: W: unversioned-explicit-obsoletes krb5-configs
> The specfile contains an unversioned Obsoletes: token, which will match all
> older, equal and newer versions of the obsoleted thing.  This may cause update
> problems, restrict future package/provides naming, and may match something it
> was originally not inteded to match -- make the Obsoletes versioned if
> possible.

Removed the obsoletes, as the package being obsoleted last existed ca. RHL 6.2, which was long enough ago that it's not worth keeping in there.
 
> krb5.src:1076: W: macro-in-%changelog _var
> Macros are expanded in %changelog too, which can in unfortunate cases lead to
> the package not building at all, or other subtle unexpected conditions that
> affect the build.  Even when that doesn't happen, the expansion results in
> possibly "rewriting history" on subsequent package revisions and generally odd
> entries eg. in source rpms, which is rarely wanted.  Avoid use of macros in
> %changelog altogether, or use two '%'s to escape them, like '%%foo'.

Cleaned up all instances of unescaped macros in the changelog.

> krb5.src:1399: E: use-of-RPM_SOURCE_DIR
> You use $RPM_SOURCE_DIR or %{_sourcedir} in your spec file. If you have to use
> a directory for building, use $RPM_BUILD_ROOT instead.

The source package includes scripts.  Calling them by name is more readable for me than calling them using macros such as %{sourceNN}, which is the only other alternative I know of.

> krb5.src:1490: W: make-check-outside-check-section : make check
> TMPDIR=%{_tmppath}
> Make check or other automated regression test should be run in %check, as they
> can be disabled with a rpm macro for short circuiting purposes.

Last check, the test suite didn't run properly in buildroots because there's no controlling terminal.  It's effectively commented out, though.
 
> krb5.src: W: mixed-use-of-spaces-and-tabs (spaces: line 325, tab: line 1400)
> The specfile mixes use of spaces and tabs for indentation, which is a cosmetic
> annoyance.  Use either spaces or tabs for indentation, not both.

This feels needlessly pedantic to me.  The scriptlets consistently use tabs for indentation, and the changelog consistently uses spaces.

> krb5.src: W: patch-not-applied Patch26: krb5-1.3.2-efence.patch
> A patch is included in your package but was not applied. Refer to the patches
> documentation to see what's wrong.

Patch includes comment at its top which notes that it's only useful in debugging setups.
 
> krb5.src: W: patch-not-applied Patch55: krb5-1.6.1-empty.patch
> A patch is included in your package but was not applied. Refer to the patches
> documentation to see what's wrong.

Patch includes comment at its top which notes that it's a work-in-progress.

> krb5.src: W: patch-not-applied Patch57: krb5-1.6.2-login_chdir.patch
> A patch is included in your package but was not applied. Refer to the patches
> documentation to see what's wrong.

Patch includes comment at its top which notes that it's a work-in-progress.

> krb5.src: W: patch-not-applied Patch64: krb5-ok-as-delegate.patch
> A patch is included in your package but was not applied. Refer to the patches
> documentation to see what's wrong.

Patch includes comment at its top noting that it's an ABI change and waiting on feedback/approval/motion upstream.

> krb5.src: W: patch-not-applied Patch70: krb5-trunk-kpasswd_tcp2.patch
> A patch is included in your package but was not applied. Refer to the patches
> documentation to see what's wrong.

This is an alternate version of patch59, waiting on feedback/approval/motion upstream.
> krb5.src: W: summary-ended-with-dot The Kerberos network authentication system.
> Summary ends with a dot.

Fixed.

> krb5.src: W: strange-permission krb5kdc.init 0755
> A file that you listed to include in your package has strange permissions.
> Usually, a file should have 0644 permissions.

I'm gonna need help from CVS admins for this one -- the permissions are set at checkout-time.

> krb5.src: W: strange-permission krb5-tex-pdf.sh 0755
> A file that you listed to include in your package has strange permissions.
> Usually, a file should have 0644 permissions.

We run this script during the build.  We could take the execute bit off and run a shell with the script as its argument, I guess.

> krb5-devel.i386: W: spurious-executable-perm
> /usr/share/doc/krb5-devel-1.6.3/krb5-protocol/draft-jaganathan-rc4-hmac-03.txt
> The file is installed with executable permissions, but was identified as one
> that probably should not be executable.  Verify if the executable bits are
> desired, and remove if not.

Fixed.

> krb5-libs.i386: W: hidden-file-or-dir /usr/kerberos/man/man5/.k5login.5.gz
> The file or directory is hidden. You should see if this is normal, and delete
> it from the package if not.
> 
> ???????????

This is the man page for the use of ~/.k5login.  Not really sure what to do with it.

> krb5-pkinit-openssl.i386: W: no-documentation
> The package contains no documentation (README, doc, etc). You have to include
> documentation files.
> 
> Fix if possible.

It doesn't look like there are any docs specific to this module.
 
> krb5-server.i386: E: executable-marked-as-config-file /etc/rc.d/init.d/krb524
> Executables must not be marked as config files because that may prevent
> upgrades from working correctly. If you need to be able to customize an
> executable, make it for example read a config file in /etc/sysconfig.

File dropped (krb4-specific).

> krb5-server.i386: E: executable-marked-as-config-file /etc/rc.d/init.d/krb5kdc
> Executables must not be marked as config files because that may prevent
> upgrades from working correctly. If you need to be able to customize an
> executable, make it for example read a config file in /etc/sysconfig.
> 
> krb5-server.i386: E: executable-marked-as-config-file /etc/rc.d/init.d/kadmin
> Executables must not be marked as config files because that may prevent
> upgrades from working correctly. If you need to be able to customize an
> executable, make it for example read a config file in /etc/sysconfig.
> 
> krb5-server.i386: E: executable-marked-as-config-file /etc/rc.d/init.d/kprop
> Executables must not be marked as config files because that may prevent
> upgrades from working correctly. If you need to be able to customize an
> executable, make it for example read a config file in /etc/sysconfig.

Fixed.

> krb5-server.i386: W: conffile-without-noreplace-flag /etc/rc.d/init.d/kadmin
> A configuration file is stored in your package without the noreplace flag. A
> way to resolve this is to put the following in your SPEC file:
> %config(noreplace) /etc/your_config_file_here
> 
> krb5-server.i386: W: conffile-without-noreplace-flag /etc/rc.d/init.d/kprop
> A configuration file is stored in your package without the noreplace flag. A
> way to resolve this is to put the following in your SPEC file:
> %config(noreplace) /etc/your_config_file_here
> 
> krb5-server.i386: W: conffile-without-noreplace-flag /etc/rc.d/init.d/krb5kdc
> A configuration file is stored in your package without the noreplace flag. A
> way to resolve this is to put the following in your SPEC file:
> %config(noreplace) /etc/your_config_file_here

Fixed by not marking them as configuration files.

> krb5-server.i386: W: no-reload-entry /etc/rc.d/init.d/kprop
> In your init script (/etc/rc.d/init.d/your_file), you don't have a 'reload'
> entry, which is necessary for good functionality.
> 
> Fix.  If the software doesn't support it, just mirror the restart entry.

Done.

> krb5-server-ldap.i386: E: library-without-ldconfig-postin
> /usr/lib/libkdb_ldap.so.1.0
> This package contains a library and provides no %post scriptlet containing a
> call to ldconfig.
> 
> krb5-server-ldap.i386: E: library-without-ldconfig-postun
> /usr/lib/libkdb_ldap.so.1.0
> This package contains a library and provides no %postun scriptlet containing a
> call to ldconfig.
> 
> Probably needs fixing.

Fixed.

> krb5-workstation.i386: E: setuid-binary /usr/kerberos/bin/ksu root 04755
> The file is setuid, this may be dangerous, especially if this  file is setuid
> root.
> 
> Necessary, I suspect.

Yup.  For a while we took the setuid bit off, but it's actually useless without it, and the bug reports were rarely friendly.

> krb5-workstation.i386: W: unstripped-binary-or-object /usr/kerberos/bin/ksu
> 
> Fix if possible.

The buildroot strip script misses setuid applications.  I'm actually not sure if that's intentional or not.
 
> krb5-workstation-clients.i386: E: info-files-without-install-info-postin
> /usr/share/info/krb5-user.info.gz
> This package contains info files and provides no %post scriptlet containing a
> call to install-info.
> 
> krb5-workstation-clients.i386: E: info-files-without-install-info-postun
> /usr/share/info/krb5-user.info.gz
> This package contains info files and provides no %postun scriptlet containing
> a call to install-info.
> 
> Fix.

Done.

> krb5-workstation-clients.i386: W: unstripped-binary-or-object
> /usr/kerberos/bin/v4rcp
> 
> Fix if possible.

krb4-specific, so it's gone.
 
> krb5-workstation-servers.i386: E: postin-without-chkconfig
> /etc/rc.d/init.d/krb524
> The package contains an init script but doesn't call chkconfig in its %post.
> 
> krb5-workstation-servers.i386: E: init-script-without-chkconfig-preun
> /etc/rc.d/init.d/krb524
> The package contains an init script but doesn't contain a %preun with a call
> to chkconfig.
> 
> Probably needs fixing.

The logic for figuring out which subpackage should chkconfig --del the symlinks would have been a nightmare.  But it's krb4-specific, so triggering on removal of the older version of either subpackage isn't that bad.  (Except for the part where it's a trigger, and triggers are best avoided if possible.)
 
...


Okay, I think that's most of it.  Can I trouble you to poke at it again and see what I've missed that still needs to be addressed?

Comment 10 Gwyn Ciesla 2009-04-09 14:09:17 UTC
Ok, let's see.

>> krb5.src: W: strange-permission krb5kdc.init 0755
>> A file that you listed to include in your package has strange permissions.
>> Usually, a file should have 0644 permissions.
>
>I'm gonna need help from CVS admins for this one -- the permissions are set at
>checkout-time.

Yeah, it was probably set this way when the SRPM that was imported was built.  I'll set the CVS flag again for assistance.

>> krb5.src: W: strange-permission krb5-tex-pdf.sh 0755
>> A file that you listed to include in your package has strange permissions.
>> Usually, a file should have 0644 permissions.
>
>We run this script during the build.  We could take the execute bit off and run
>a shell with the script as its argument, I guess.

Probably worth doing.

>> krb5-libs.i386: W: hidden-file-or-dir /usr/kerberos/man/man5/.k5login.5.gz
>> The file or directory is hidden. You should see if this is normal, and delete
>> it from the package if not.
>> 
>> ???????????
>
>This is the man page for the use of ~/.k5login.  Not really sure what to do
>with it.

Since it's a valid manpage with a valid name, file a bug for an rpmlint exception.

>> krb5-workstation.i386: E: setuid-binary /usr/kerberos/bin/ksu root 04755
>> The file is setuid, this may be dangerous, especially if this  file is setuid
>> root.
>> 
>> Necessary, I suspect.
>
>Yup.  For a while we took the setuid bit off, but it's actually useless without
>it, and the bug reports were rarely friendly.

I can imagine. :)  rpmlint exception here, too.

>> krb5-workstation.i386: W: unstripped-binary-or-object /usr/kerberos/bin/ksu
>> 
>> Fix if possible.
>
>The buildroot strip script misses setuid applications.  I'm actually not sure
>if that's intentional or not.

CCing Panu for this one.

>> krb5-workstation-servers.i386: E: postin-without-chkconfig
>> /etc/rc.d/init.d/krb524
>> The package contains an init script but doesn't call chkconfig in its %post.
>> 
>> krb5-workstation-servers.i386: E: init-script-without-chkconfig-preun
>> /etc/rc.d/init.d/krb524
>> The package contains an init script but doesn't contain a %preun with a call
>> to chkconfig.
>> 
>> Probably needs fixing.
>
>The logic for figuring out which subpackage should chkconfig --del the symlinks
>would have been a nightmare.  But it's krb4-specific, so triggering on removal
>of the older version of either subpackage isn't that bad.  (Except for the part
>where it's a trigger, and triggers are best avoided if possible.)

Am I to understand that this will be deprecated upon removal of krb4 stuff?

Comment 11 Toshio Ernie Kuratomi 2009-04-09 21:07:53 UTC
>> krb5.src: W: strange-permission krb5kdc.init 0755
>> A file that you listed to include in your package has strange permissions.
>> Usually, a file should have 0644 permissions.
>
>I'm gonna need help from CVS admins for this one -- the permissions are set at
>checkout-time.

cvs done.  Note that I think this warning is ignorable if there's a reason for the file to be executable.

Comment 12 Nalin Dahyabhai 2009-04-20 21:13:34 UTC
(In reply to comment #10)
> >> krb5.src: W: strange-permission krb5-tex-pdf.sh 0755
> >> A file that you listed to include in your package has strange permissions.
> >> Usually, a file should have 0644 permissions.
> >
> >We run this script during the build.  We could take the execute bit off and run
> >a shell with the script as its argument, I guess.
> 
> Probably worth doing.

Done.

> >> krb5-libs.i386: W: hidden-file-or-dir /usr/kerberos/man/man5/.k5login.5.gz
> >> The file or directory is hidden. You should see if this is normal, and delete
> >> it from the package if not.
> >> 
> >> ???????????
> >
> >This is the man page for the use of ~/.k5login.  Not really sure what to do
> >with it.
> 
> Since it's a valid manpage with a valid name, file a bug for an rpmlint
> exception.

Filed bug #496735.

> >> krb5-workstation.i386: E: setuid-binary /usr/kerberos/bin/ksu root 04755
> >> The file is setuid, this may be dangerous, especially if this  file is setuid
> >> root.
> >> 
> >> Necessary, I suspect.
> >
> >Yup.  For a while we took the setuid bit off, but it's actually useless without
> >it, and the bug reports were rarely friendly.
> 
> I can imagine. :)  rpmlint exception here, too.

Filed, bug #496737.

> ...
> Am I to understand that this will be deprecated upon removal of krb4 stuff?  

Yup.  When the code to handle the krb4 protocol gets turned off and eventually ejected, krb524 (and its client, krb524init) disappear as well.

Comment 13 Gwyn Ciesla 2009-04-21 14:34:17 UTC
>>> krb5-workstation.i386: W: unstripped-binary-or-object /usr/kerberos/bin/ksu
>>> 
>>> Fix if possible.
>>
>>The buildroot strip script misses setuid applications.  I'm actually not sure
>>if that's intentional or not.
>
>CCing Panu for this one.

Great, looks like this is all we have.  Panu?

Comment 14 Gwyn Ciesla 2010-04-29 20:12:31 UTC
Ping?

Comment 15 Gwyn Ciesla 2011-03-31 16:40:09 UTC
Ping?

Comment 16 Panu Matilainen 2011-04-01 06:01:42 UTC
Oh, sorry. This has just gotten buried in bugzilla mail and long forgotten...

It's not that SUID binaries are skipped when stripping, the problem is that ksu gets installed without any executable permissions at all (apparently because the krb5 Makefiles try to do something "clever" about installing suid parts):

[pmatilai@localhost krb5]$ ls -l /home/pmatilai/rpmbuild/BUILDROOT/krb5-1.9-6.fc16.x86_64/usr/bin/
total 532
-rwxr-xr-x. 1 pmatilai pmatilai  22616 Apr  1 08:58 gss-client
-rwxr-xr-x. 1 pmatilai pmatilai   1914 Apr  1 08:58 k5srvutil
-rwxr-xr-x. 1 pmatilai pmatilai  67680 Apr  1 08:58 kadmin
-rwxr-xr-x. 1 pmatilai pmatilai  10256 Apr  1 08:58 kdestroy
-rwxr-xr-x. 1 pmatilai pmatilai  22544 Apr  1 08:58 kinit
-rwxr-xr-x. 1 pmatilai pmatilai  22544 Apr  1 08:58 klist
-rwxr-xr-x. 1 pmatilai pmatilai  10256 Apr  1 08:58 kpasswd
-rwxr-xr-x. 1 pmatilai pmatilai   6161 Apr  1 08:58 krb5-config
-rw-------. 1 pmatilai pmatilai 214741 Apr  1 08:58 ksu
-rwxr-xr-x. 1 pmatilai pmatilai  22544 Apr  1 08:58 ktutil
-rwxr-xr-x. 1 pmatilai pmatilai  14352 Apr  1 08:58 kvno
-rwxr-xr-x. 1 pmatilai pmatilai  14352 Apr  1 08:58 sclient
-rwxr-xr-x. 1 pmatilai pmatilai  14360 Apr  1 08:58 sim_client
-rwxr-xr-x. 1 pmatilai pmatilai  14352 Apr  1 08:58 uuclient

A simple 'chmod u+x $RPM_BUILD_ROOT/%{_bindir}/ksu' at the end of %install will fix that. Or a tweak to Makefile(s).

Comment 17 Nalin Dahyabhai 2011-04-01 19:36:13 UTC
Oh, that's install apparently failing to change the file's ownership to root, and skipping the set-the-requested-permissions step as a result.  I'll work around it soon.

Comment 18 Gwyn Ciesla 2011-06-17 14:58:29 UTC
Any updates?

Comment 19 Nalin Dahyabhai 2011-06-17 15:30:12 UTC
Whoops, yes, that was fixed in 1.9-9, if I'm reading the changelog right.

Comment 20 Gwyn Ciesla 2011-06-17 15:32:37 UTC
Awesome, thanks!  APPROVED.