From 8690c09cc914da6ac3a6ba46ab3ba7690a344cf9 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Mon, 20 Aug 2018 18:00:13 -0400 Subject: Sudo.enabledFor: Write to /etc/sudoers.d/000users rather than to /etc/sudoers (Any old lines it wrote to /etc/sudoers will be removed.) This fixes a potential ordering problem; the property used to append the line to /etc/sudoers, but that would override more specific lines in the include directory. By putting it in a file that is included first, it'll come before all includes, without needing to parse the sudoers file in order to put it before the includedir line. Note that, if there is a more specific line for the user in /etc/sudoers before the includedir, it will be overridden by the line in /etc/sudoers.d/000users. But, this is not a behavior change from before, when the line was appended to the end. This commit was sponsored by Jeff Goeke-Smith on Patreon. --- debian/changelog | 5 ++++ src/Propellor/Property/SiteSpecific/JoeySites.hs | 2 +- src/Propellor/Property/Sudo.hs | 29 +++++++++++++++++------- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/debian/changelog b/debian/changelog index f0b8db04..8faca945 100644 --- a/debian/changelog +++ b/debian/changelog @@ -6,6 +6,11 @@ propellor (5.5.0) UNRELEASED; urgency=medium * Added Systemd.escapePath helper function useful when creating mount units. * Added Sudo.sudoersDFile property. + * Sudo.enabledFor: Write to /etc/sudoers.d/000users rather than to + /etc/sudoers. (Any old lines it wrote to /etc/sudoers will be removed.) + This fixes a potential ordering problem; the property used to append + the line to /etc/sudoers, but that would override more specific lines + in the include directory. -- Joey Hess Thu, 09 Aug 2018 10:54:41 -0400 diff --git a/src/Propellor/Property/SiteSpecific/JoeySites.hs b/src/Propellor/Property/SiteSpecific/JoeySites.hs index 909ff929..fa7cb064 100644 --- a/src/Propellor/Property/SiteSpecific/JoeySites.hs +++ b/src/Propellor/Property/SiteSpecific/JoeySites.hs @@ -1285,7 +1285,7 @@ autoMountDrive label (USBHubPort port) malias = propertyList desc $ props & Systemd.enabled automount & Systemd.started automount & Sudo.sudoersDFile ("automount-" ++ label) - [ "%joey ALL= NOPASSWD: " ++ sudocommands + [ "joey ALL= NOPASSWD: " ++ sudocommands ] where mountpoint = "/media/joey/" ++ label diff --git a/src/Propellor/Property/Sudo.hs b/src/Propellor/Property/Sudo.hs index c2f0ac4e..12660aa9 100644 --- a/src/Propellor/Property/Sudo.hs +++ b/src/Propellor/Property/Sudo.hs @@ -7,34 +7,47 @@ import Propellor.Property.File import qualified Propellor.Property.Apt as Apt import Propellor.Property.User --- | Allows a user to sudo. If the user has a password, sudo is configured --- to require it. If not, NOPASSWORD is enabled for the user. +-- | Allows a user to run any command with sudo. +-- If the user has a password, sudo is configured to require it. +-- If not, NOPASSWORD is enabled for the user. +-- +-- Writes to the file /etc/sudoers.d/000users rather than the main sudoers +-- file. This file should come before other include files that may eg, +-- allow running more specific commands without a password, since sudo +-- uses the last matching configuration line. +-- +-- If the main sudoers file contains a conflicting line for +-- the user for ALL commands, the line will be removed. enabledFor :: User -> RevertableProperty DebianLike DebianLike enabledFor user@(User u) = setup `requires` Apt.installed ["sudo"] cleanup where setup :: Property UnixLike setup = property' desc $ \w -> do locked <- liftIO $ isLockedPassword user - ensureProperty w $ - fileProperty desc + ensureProperty w $ combineProperties desc $ props + & fileProperty desc (modify locked . filter (wanted locked)) - sudoers + dfile + & removeconflicting sudoers where desc = u ++ " is sudoer" cleanup :: Property DebianLike - cleanup = tightenTargets $ - fileProperty desc (filter notuserline) sudoers + cleanup = tightenTargets $ combineProperties desc $ props + & removeconflicting sudoers + & removeconflicting dfile where desc = u ++ " is not sudoer" + removeconflicting = fileProperty "remove conflicting" (filter notuserline) + sudoers = "/etc/sudoers" + dfile = "/etc/sudoers.d/000users" sudobaseline = u ++ " ALL=(ALL:ALL)" notuserline l = not (sudobaseline `isPrefixOf` l) sudoline True = sudobaseline ++ " NOPASSWD:ALL" sudoline False = sudobaseline ++ " ALL" wanted locked l - -- TODO: Full sudoers file format parse.. | notuserline l = True | "NOPASSWD" `isInfixOf` l = locked | otherwise = True -- cgit v1.2.3