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.
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 attachment 137516 [details, diff] 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.