summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJoey Hess2016-11-20 20:07:57 -0400
committerJoey Hess2016-11-20 20:07:57 -0400
commitc47474d3a8ea926c185481acf4f0c21006b8d7ef (patch)
tree5865446cbb2a0db104e722a3a3c946ee6c6133b8
parent1178d210043894a87ee4cdb8cda00ca8da5883c5 (diff)
parent42fafdc21313dff0e5d1972b457d5edcc589cfb0 (diff)
Merge branch 'master' into joeyconfig
l---------config.hs2
-rw-r--r--debian/changelog3
-rw-r--r--doc/todo/userScriptProperty_fails_inside_a_debootstrapped_chroot.mdwn25
-rw-r--r--doc/todo/userScriptProperty_fails_inside_a_debootstrapped_chroot/comment_1_75ae52da0638ff6ea1c04820091b89f3._comment23
-rw-r--r--privdata/relocate1
-rw-r--r--src/Propellor/Property/Debootstrap.hs22
6 files changed, 66 insertions, 10 deletions
diff --git a/config.hs b/config.hs
index 97d90636..ec313725 120000
--- a/config.hs
+++ b/config.hs
@@ -1 +1 @@
-joeyconfig.hs \ No newline at end of file
+config-simple.hs \ No newline at end of file
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 <id@joeyh.name> 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
new file mode 100644
index 00000000..c4464d03
--- /dev/null
+++ b/doc/todo/userScriptProperty_fails_inside_a_debootstrapped_chroot.mdwn
@@ -0,0 +1,25 @@
+Config snippet to reproduce:
+
+ & Chroot.provisioned sidChroot
+ where
+ sidChroot = Chroot.debootstrapped mempty "/tmp/sid" $ props
+ & osDebian Unstable X86_64
+ & User.accountFor (User "spwhitton")
+ & userScriptProperty (User "spwhitton")
+ [ "echo hello > /home/spwhitton/greeting" ]
+ `assume` MadeChange
+
+During a spin, I see the error `Cannot execute /bin/sh`.
+
+I can obtain the error manually as follows. My `/tmp` is not mounted `noexec`.
+
+ iris ~ % sudo chroot /tmp/sid /bin/bash
+ [sudo] password for spwhitton:
+ root@iris:/# su --shell /bin/sh -c "echo hello > /home/spwhitton/greeting" spwhitton
+ Cannot execute /bin/sh
+ root@iris:/# su --shell /bin/sh spwhitton
+ 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/privdata/relocate b/privdata/relocate
deleted file mode 100644
index 271692d8..00000000
--- a/privdata/relocate
+++ /dev/null
@@ -1 +0,0 @@
-.joeyconfig
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