Re: TOCTTOU race (was: Potential security weakness in Guix services)

[Bengt Richter]

Hi,

On +2021-02-14 13:29:29 +0100, Maxime Devos wrote:
> On Sat, 2021-02-06 at 22:26 +0100, Ludovic Courtès wrote:
> > 
> > [...]
> > I understand the TOCTTOU race.  However, activation code runs in two
> > situations: when booting the system (before shepherd takes over), and 
> > upon ‘guix system reconfigure’ completion.
> >

Until we have a guix jargon file and a
guix gloss SEARCHARGS ...
convenience command, it is nice towards noobs to spell out
an abbreviation or acronym on first use ;-)

--8<---cut here---start->8---
Time-of-check to time-of-use

   From Wikipedia, the free encyclopedia
 (Redirected from TOCTTOU)
   Jump to navigation Jump to search

   In software development, time-of-check to time-of-use (TOCTOU, TOCTTOU
   or TOC/TOU) is a class of software bugs caused by a race condition
   involving the checking of the state of a part of a system (such as a
   security credential) and the use of the results of that check.

   TOCTOU race conditions are common in Unix between operations on the
   file system,^[1] but can occur in other contexts, including local
   sockets and improper use of database transactions. In the early 1990s,
   the mail utility of BSD 4.3 UNIX had an exploitable race condition for
   temporary files because it used the mktemp()^[2] function.^[3] Early
   versions of OpenSSH had an exploitable race condition for Unix domain
   sockets.^[4] They remain a problem in modern systems; as of 2019, a
   TOCTOU race condition in Docker allows root access to the filesystem of
   the host platform.^[5]
   [ ]
--8<---cut here---end--->8---

[...snip...]
-- 
Regards,
Bengt Richter

TOCTTOU race (was: Potential security weakness in Guix services)

[Maxime Devos]

On Sat, 2021-02-06 at 22:26 +0100, Ludovic Courtès wrote:
> 
> [...]
> I understand the TOCTTOU race.  However, activation code runs in two
> situations: when booting the system (before shepherd takes over), and
> upon ‘guix system reconfigure’ completion.
> 
> When booting the system, there’s just no process out there to take
> advantage of the race condition.
> 
> In the second case, presumably all the file name components already
> exist.

In the second situation, a compromised service could quickly rename
a component to something else and create a symlink in place, and after
the activation code has changed permissions and owner remove the symlink
and rename the component back to avoid suspicion. 

(The old component could be removed entirely and replaced with a symlink,
but that will likely break something, which may lead to the sysadmin
investigating.)

(The attack method I'm describing here of course only works if the
compromised service has control over both the component and the parent
directory.)

> Does that make sense?

Maybe?  While I would prefer there would *not* be a TOCTTOU race,
we may have to live with that for the moment (and even with a TOCTTOU
race, at least an attacker only has a narrow window).

I'll submit a new patch *without* a TOCTTOU race once openat,
fstatat, ... bindings make it into guile, but for the mean time, I've
attached a patch with the TOCTTOU race.

I've tested with 'make check-system TESTS="basic cups"'.
I couldn't test all affected services, unfortunately,
due to lack of system tests.

Thoughts?

Greetings,
Maxime.
From ad10c577eb1f13b9b66ea387648671df33b869d7 Mon Sep 17 00:00:00 2001
From: Maxime Devos 
Date: Sun, 14 Feb 2021 12:57:32 +0100
Subject: [PATCH] services: prevent following symlinks during activation

Currently, there's a TOCTTOU race.  This can be addressed
once guile has bindings for fstatat, openat and friends.

XXX I'm horrible at naming exceptions:
(throw 'XXX-TODO-does-someone-have-an-idea? path)

* guix/build/service-utils.scm: new module
  with new procedure 'mkdir-p/perms'.
* Makefile.am (MODULES): compile new module.
* gnu/services/authentication.scm
  (%nslcd-activation, nslcd-service-type): use new procedure.
* gnu/services/cups.scm (%cups-activation): likewise.
* gnu/services/dbus.scm (dbus-activation): likewise.
* gnu/services/dns.scm (knot-activation): likewise.
---
 Makefile.am |  1 +
 gnu/services/authentication.scm | 22 ++-
 gnu/services/cups.scm   | 12 +++---
 gnu/services/dbus.scm   | 36 +-
 gnu/services/dns.scm| 20 +-
 guix/build/service-utils.scm| 66 +
 6 files changed, 113 insertions(+), 44 deletions(-)
 create mode 100644 guix/build/service-utils.scm

diff --git a/Makefile.am b/Makefile.am
index 798808bde6..c82922fc87 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -239,6 +239,7 @@ MODULES =	\
   guix/build/bournish.scm			\
   guix/build/qt-utils.scm			\
   guix/build/make-bootstrap.scm			\
+  guix/build/service-utils.scm  \
   guix/search-paths.scm\
   guix/packages.scm\
   guix/import/cabal.scm\
diff --git a/gnu/services/authentication.scm b/gnu/services/authentication.scm
index 73969a5a6d..aad02d3eab 100644
--- a/gnu/services/authentication.scm
+++ b/gnu/services/authentication.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2018 Danny Milosavljevic 
 ;;; Copyright © 2018, 2019 Ricardo Wurmus 
+;;; Copyright © 2021 Maxime Devos 
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -31,6 +32,7 @@
   #:use-module (guix gexp)
   #:use-module (guix records)
   #:use-module (guix packages)
+  #:use-module (guix modules)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
@@ -521,6 +523,16 @@ password.")
 (define (pam-ldap-pam-services config)
   (list (pam-ldap-pam-service config)))
 
+(define nslcd-activation
+  (with-imported-modules (source-module-closure '((guix build service-utils)))
+#~(begin
+(use-modules (guix build service-utils))
+(let ((rundir "/var/run/nslcd")
+  (user (getpwnam "nslcd")))
+  (mkdir-p/perms rundir user #o755)
+  (when (file-exists? "/etc/nslcd.conf")
+(chmod "/etc/nslcd.conf" #o400))
+
 (define nslcd-service-type
   (service-type
(name 'nslcd)
@@ -531,15 +543,7 @@ password.")
   (service-extension etc-service-type
  nslcd-etc-service)
   (service-extension activation-service-type
- (const #~(begin
-(use-modules (guix build utils))
-(let ((rundir "/var/run/nslcd")
-  (user (getpwnam "nslcd")))
-  (mkdir-p rundir)
-  (chown rundir (passwd:uid user