Bugzilla Bug 174759

In the ssl-cert.eclass, the docert function generates an SSL key, along with an
SSL certificate.

If the docert function is used inside src_compile or src_install (see
openldap-2.3.3[45]* for example), the SSL key is then included inside the
binpkg, where it is NOT protected - anybody with access to the system can
extract the tarball to recover the key.

Additionally, if the binpkg is used on multiple machines, they will be
configured with the same SSL key and cert, and this may prevent correct
functioning of some SSL applications.

vapier/agriffis please advise.

vapier/agriffis please advise.

vapier/agriffis please advise.

Robin's analysis of the situation is correct ... this is the same reason we
stopped managing things like /etc/shadow in baselayout's src_install function
...

docert() should be rewritten to only allow it to be run inside of pkg_preinst()

ive never touched that eclass before, so ...

i'd say with the recent changes portage has seen, this should be fixed now

spanky: could you clarify which changes you are talking about?

someone just posted on this issue on the -dev mailing-list, maybe we should
make it public now. Any opinions?

As the original reporter, I say open it, so that we can get some traction in
getting it fixed all over the tree.

The only things needing nuking are public tinderboxes.

Opening.

sorry, i was thinking in terms of "binpkgs being world readable" and "portage
doesnt consume /etc by default with quickpkg and friends" ... but the use case
here doesnt fully apply (packages generating unique keys on each host), so the
proposed route of pkg_postinst() is the only one i think

easy enough to add to the top of the docert() function:
case ${EBUILD_PHASE} in
    unpack|compile|test|install)  die "BOO";;
esac

"docert" doesn't work inside pkg_postinst at the moment, probably related to
bug #197942.

Created an attachment (id=137516) [details]
Proposed patch for docert()

- Check for ebuild phase as suggested in comment #10
- Install relative to ${ROOT} instead of ${D}${INSDESTTREE}
- Arguments must be a full path now

Maybe the function should also be renamed, since it's no longer similar to
other do* functions?

Patch looks good to me.

The following packages will have to be changed then. They either call docert in
src_install and suffer from the security issue of this bug, or they call it in
pkg_postinst where it doesn't work at all (that is bug #197942).

src_install usage:
  app-admin/conserver
  mail-mta/postfix
  net-analyzer/sguil-server
  net-firewall/nufw
  net-ftp/netkit-ftpd
  net-im/ejabberd
  net-irc/ptlink-ircd
  net-irc/unrealircd
  net-mail/cyrus-imapd
  net-mail/dovecot
  net-misc/stunnel
  net-nntp/inn

pkg_postinst usage:
  net-mail/cyrus-imspd
  net-nds/openldap
  www-servers/nginx

Ulrich, how do you propose we proceed now? The commit can only be done if all
ebuilds (stable and unstable) are fixed for this?

Just open bugs and bug maintainers? Or would it be advisable to announce this
migration and change the ebuilds by ourselves?

unless I'm mistaken, the ones in pkg_postinst should just start to work after
applying this patch? And all the ebuilds should be converted to use the
functions in either postinst or config phases?

One patch review comment, in the loop commented with "# Check for previous
existence of generated files", we should probably check all 4 file suffixes
(.csr is not checked).

(In reply to comment #15)
> Ulrich, how do you propose we proceed now? The commit can only be done if all
> ebuilds (stable and unstable) are fixed for this?

If we rename the function ("install_cert"?) we could add the new one, fix all
ebuilds and finally replace docert by a dummy (or remove it completely).

> Just open bugs and bug maintainers?

I would say yes.

(In reply to comment #16)
> unless I'm mistaken, the ones in pkg_postinst should just start to work after
> applying this patch?

Unfortunately they must be changed, too, since they rely on INSDESTTREE.

> we should probably check all 4 file suffixes (.csr is not checked).

It did not check for .csr before. I don't see a reason why it shouldn't,
though.

Updated ssl-cert.eclass committed. Function "docert" has been copied to a new
function "install_cert", modified according to comment #12.

Adding package maintainers to CC, please fix your packages mentioned in comment
#14 to use "install_cert" in pkg_postinst or pkg_config. Since it is a security
issue, and we also want to get rid of the duplicate code in the eclass, I will
fix any remaining packages after 14 days.

"docert" will then be replaced by a dummy function that forcibly fails.

net-misc/stunnel fixed, bug #197881.

app-admin/conserver fixed (and bumped to 8.1.16) since it's maintainer-needed.

Creating seperate bugs.

www-servers/nginx fixed.

All packages on all supported arches are done.

Only mail-mta/postfix keywording on mips and stabilisation on arm/s390/sh (bug
201671) are still missing. After that, we can replace the docert function in
the eclass by a dummy.

This was sent as GLSA 200803-30.

I'll leave this bug open until all packages are fixed and the eclass corrected.

Eclass corrected: "docert" replaced by a dummy function.
All ebuilds that were affected by this issue have been removed from the tree.