From 42fafdc21313dff0e5d1972b457d5edcc589cfb0 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Sun, 20 Nov 2016 13:22:53 -0400 Subject: Debootstap: Fix too tight permissions lock down of debootstrapped chroots, which prevented non-root users from doing anything in the chroot. --- debian/changelog | 3 +++ ...perty_fails_inside_a_debootstrapped_chroot.mdwn | 2 ++ ...ent_1_75ae52da0638ff6ea1c04820091b89f3._comment | 23 ++++++++++++++++++++++ src/Propellor/Property/Debootstrap.hs | 22 +++++++++++++-------- 4 files changed, 42 insertions(+), 8 deletions(-) create mode 100644 doc/todo/userScriptProperty_fails_inside_a_debootstrapped_chroot/comment_1_75ae52da0638ff6ea1c04820091b89f3._comment diff --git a/debian/changelog b/debian/changelog index f3442116..efbde34e 100644 --- a/debian/changelog +++ b/debian/changelog @@ -4,6 +4,9 @@ propellor (3.2.3) UNRELEASED; urgency=medium * The propellor wrapper checks if ./config.hs exists; if so it runs using the configuration in the current directory, rather than ~/.propellor/config.hs + * Debootstap: Fix too tight permissions lock down of debootstrapped + chroots, which prevented non-root users from doing anything in the + chroot. -- Joey Hess Fri, 11 Nov 2016 19:32:54 -0400 diff --git a/doc/todo/userScriptProperty_fails_inside_a_debootstrapped_chroot.mdwn b/doc/todo/userScriptProperty_fails_inside_a_debootstrapped_chroot.mdwn index d42d4f79..c4464d03 100644 --- a/doc/todo/userScriptProperty_fails_inside_a_debootstrapped_chroot.mdwn +++ b/doc/todo/userScriptProperty_fails_inside_a_debootstrapped_chroot.mdwn @@ -21,3 +21,5 @@ I can obtain the error manually as follows. My `/tmp` is not mounted `noexec`. Cannot execute /bin/sh: Permission denied --spwhitton + +> [[fixed|done]] --[[Joey]] diff --git a/doc/todo/userScriptProperty_fails_inside_a_debootstrapped_chroot/comment_1_75ae52da0638ff6ea1c04820091b89f3._comment b/doc/todo/userScriptProperty_fails_inside_a_debootstrapped_chroot/comment_1_75ae52da0638ff6ea1c04820091b89f3._comment new file mode 100644 index 00000000..89bb17f1 --- /dev/null +++ b/doc/todo/userScriptProperty_fails_inside_a_debootstrapped_chroot/comment_1_75ae52da0638ff6ea1c04820091b89f3._comment @@ -0,0 +1,23 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 1""" + date="2016-11-20T16:55:25Z" + content=""" +This is due to `Debootstrap.built'` removing world read access from the +chroot it creates. + +So, /tmp/sid/ is not accessible by spwhitton, and when su +has switched id to spwhitton, it can't access anything inside the chroot. + +See commit f6afeb889f4b11418daac7825c1adb1df4ff145c for when this was +added. I think that the risk of farming old security vulnerabilities from +chroots is real, but this is not a good approach for a fix. + +(It would work to put the chroot in a parent +directory that is itself not world readable, then the root directory inside the +chroot would be world readable. But this would require relocating existing +chroots. At least when chroots are used for systemd containers, +/var/lib/container has appropriately locked down permissions anyway.) + +I'm reverting that commit, and adding some permissions fixup code. +"""]] diff --git a/src/Propellor/Property/Debootstrap.hs b/src/Propellor/Property/Debootstrap.hs index f9737cac..f8cb6e0e 100644 --- a/src/Propellor/Property/Debootstrap.hs +++ b/src/Propellor/Property/Debootstrap.hs @@ -51,18 +51,15 @@ built :: FilePath -> System -> DebootstrapConfig -> Property Linux built target system config = built' (setupRevertableProperty installed) target system config built' :: Property Linux -> FilePath -> System -> DebootstrapConfig -> Property Linux -built' installprop target system@(System _ arch) config = - check (unpopulated target <||> ispartial) setupprop - `requires` installprop +built' installprop target system@(System _ arch) config = + go `before` oldpermfix where + go = check (unpopulated target <||> ispartial) setupprop + `requires` installprop + setupprop :: Property Linux setupprop = property ("debootstrapped " ++ target) $ liftIO $ do createDirectoryIfMissing True target - -- Don't allow non-root users to see inside the chroot, - -- since doing so can allow them to do various attacks - -- including hard link farming suid programs for later - -- exploitation. - modifyFileMode target (removeModes [otherReadMode, otherExecuteMode, otherWriteMode]) suite <- case extractSuite system of Nothing -> errorMessage $ "don't know how to debootstrap " ++ show system Just s -> pure s @@ -86,6 +83,15 @@ built' installprop target system@(System _ arch) config = return True , return False ) + + -- May want to remove this after some appropriate length of time, + -- as it's a workaround for chroots set up with too tight + -- permissions. + oldpermfix :: Property Linux + oldpermfix = property ("fixed old chroot file mode") $ do + liftIO $ modifyFileMode target $ + addModes [otherReadMode, otherExecuteMode] + return NoChange extractSuite :: System -> Maybe String extractSuite (System (Debian _ s) _) = Just $ Apt.showSuite s -- cgit v1.2.3