From ebca2f254ca96ad7ad855dca6e76c9d1c792c4a0 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 28 Nov 2015 13:17:57 +0100 Subject: [PATCH] disable privilege-drop verification by default as fakeroot trips over it Dropping privileges is an involved process for code and system alike so ideally we want to verify that all the work wasn't in vain. Stuff designed to sidestep the usual privilege checks like fakeroot (and its many alternatives) have their problem with this through, partly through missing wrapping (#806521), partly as e.g. regaining root from an unprivileged user is in their design. This commit therefore disables most of these checks by default so that apt runs fine again in a fakeroot environment. Closes: 806475 --- apt-pkg/contrib/fileutl.cc | 102 ++++++++++++++++++++++--------------- test/integration/framework | 2 + 2 files changed, 62 insertions(+), 42 deletions(-) diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index f754b313e..014f0ee51 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -2283,6 +2283,12 @@ bool DropPrivileges() /*{{{*/ if (toUser.empty() || toUser == "root") return true; + // a lot can go wrong trying to drop privileges completely, + // so ideally we would like to verify that we have done it – + // but the verify asks for too much in case of fakeroot (and alike) + // [Specific checks can be overridden with dedicated options] + bool const VerifySandboxing = _config->FindB("APT::Sandbox::Verify", false); + // uid will be 0 in the end, but gid might be different anyway uid_t const old_uid = getuid(); gid_t const old_gid = getgid(); @@ -2322,56 +2328,68 @@ bool DropPrivileges() /*{{{*/ return _error->Errno("seteuid", "Failed to seteuid"); #endif - // Verify that the user isn't still in any supplementary groups - long const ngroups_max = sysconf(_SC_NGROUPS_MAX); - std::unique_ptr gidlist(new gid_t[ngroups_max]); - if (unlikely(gidlist == NULL)) - return _error->Error("Allocation of a list of size %lu for getgroups failed", ngroups_max); - ssize_t gidlist_nr; - if ((gidlist_nr = getgroups(ngroups_max, gidlist.get())) < 0) - return _error->Errno("getgroups", "Could not get new groups (%lu)", ngroups_max); - for (ssize_t i = 0; i < gidlist_nr; ++i) - if (gidlist[i] != pw->pw_gid) - return _error->Error("Could not switch group, user %s is still in group %d", toUser.c_str(), gidlist[i]); - - // Verify that gid, egid, uid, and euid changed - if (getgid() != pw->pw_gid) - return _error->Error("Could not switch group"); - if (getegid() != pw->pw_gid) - return _error->Error("Could not switch effective group"); - if (getuid() != pw->pw_uid) - return _error->Error("Could not switch user"); - if (geteuid() != pw->pw_uid) - return _error->Error("Could not switch effective user"); + // disabled by default as fakeroot doesn't implement getgroups currently (#806521) + if (VerifySandboxing == true || _config->FindB("APT::Sandbox::Verify::Groups", false) == true) + { + // Verify that the user isn't still in any supplementary groups + long const ngroups_max = sysconf(_SC_NGROUPS_MAX); + std::unique_ptr gidlist(new gid_t[ngroups_max]); + if (unlikely(gidlist == NULL)) + return _error->Error("Allocation of a list of size %lu for getgroups failed", ngroups_max); + ssize_t gidlist_nr; + if ((gidlist_nr = getgroups(ngroups_max, gidlist.get())) < 0) + return _error->Errno("getgroups", "Could not get new groups (%lu)", ngroups_max); + for (ssize_t i = 0; i < gidlist_nr; ++i) + if (gidlist[i] != pw->pw_gid) + return _error->Error("Could not switch group, user %s is still in group %d", toUser.c_str(), gidlist[i]); + } + + // enabled by default as all fakeroot-lookalikes should fake that accordingly + if (VerifySandboxing == true || _config->FindB("APT::Sandbox::Verify::IDs", true) == true) + { + // Verify that gid, egid, uid, and euid changed + if (getgid() != pw->pw_gid) + return _error->Error("Could not switch group"); + if (getegid() != pw->pw_gid) + return _error->Error("Could not switch effective group"); + if (getuid() != pw->pw_uid) + return _error->Error("Could not switch user"); + if (geteuid() != pw->pw_uid) + return _error->Error("Could not switch effective user"); #ifdef HAVE_GETRESUID - // verify that the saved set-user-id was changed as well - uid_t ruid = 0; - uid_t euid = 0; - uid_t suid = 0; - if (getresuid(&ruid, &euid, &suid)) - return _error->Errno("getresuid", "Could not get saved set-user-ID"); - if (suid != pw->pw_uid) - return _error->Error("Could not switch saved set-user-ID"); + // verify that the saved set-user-id was changed as well + uid_t ruid = 0; + uid_t euid = 0; + uid_t suid = 0; + if (getresuid(&ruid, &euid, &suid)) + return _error->Errno("getresuid", "Could not get saved set-user-ID"); + if (suid != pw->pw_uid) + return _error->Error("Could not switch saved set-user-ID"); #endif #ifdef HAVE_GETRESGID - // verify that the saved set-group-id was changed as well - gid_t rgid = 0; - gid_t egid = 0; - gid_t sgid = 0; - if (getresgid(&rgid, &egid, &sgid)) - return _error->Errno("getresuid", "Could not get saved set-group-ID"); - if (sgid != pw->pw_gid) - return _error->Error("Could not switch saved set-group-ID"); + // verify that the saved set-group-id was changed as well + gid_t rgid = 0; + gid_t egid = 0; + gid_t sgid = 0; + if (getresgid(&rgid, &egid, &sgid)) + return _error->Errno("getresuid", "Could not get saved set-group-ID"); + if (sgid != pw->pw_gid) + return _error->Error("Could not switch saved set-group-ID"); #endif + } - // Check that uid and gid changes do not work anymore - if (pw->pw_gid != old_gid && (setgid(old_gid) != -1 || setegid(old_gid) != -1)) - return _error->Error("Could restore a gid to root, privilege dropping did not work"); + // disabled as fakeroot doesn't forbid (by design) (re)gaining root from unprivileged + if (VerifySandboxing == true || _config->FindB("APT::Sandbox::Verify::Regain", false) == true) + { + // Check that uid and gid changes do not work anymore + if (pw->pw_gid != old_gid && (setgid(old_gid) != -1 || setegid(old_gid) != -1)) + return _error->Error("Could restore a gid to root, privilege dropping did not work"); - if (pw->pw_uid != old_uid && (setuid(old_uid) != -1 || seteuid(old_uid) != -1)) - return _error->Error("Could restore a uid to root, privilege dropping did not work"); + if (pw->pw_uid != old_uid && (setuid(old_uid) != -1 || seteuid(old_uid) != -1)) + return _error->Error("Could restore a uid to root, privilege dropping did not work"); + } return true; } diff --git a/test/integration/framework b/test/integration/framework index 292a7b958..ce10002ec 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -370,6 +370,8 @@ EOF # in testcases, it can appear as if localhost has a rotation setup, # hide this as we can't really deal with it properly echo 'Acquire::Failure::ShowIP "false";' + # fakeroot can't fake everything, so disabled in production but good for tests + echo 'APT::Sandbox::Verify "true";' } >> aptconfig.conf cp "${TESTDIRECTORY}/apt.pem" "${TMPWORKINGDIRECTORY}/rootdir/etc/webserver.pem" -- 2.45.2