bug#47584: Race condition in ‘copy-account-skeletons’: possible privilege escalation.

[Maxime Devos]

Now openat etc is in Guile, I've looked into adjusting mkdir-p/perms 
appropriately.  TODO: change the Guile used for activation to some 
commit that has openat etc, adjust patch according to test failures. 
(Not tested yet)


Greetings,
Maxime.

diff --git a/gnu/build/activation.scm b/gnu/build/activation.scm
index 10c9045740..ee52bb1979 100644
--- a/gnu/build/activation.scm
+++ b/gnu/build/activation.scm
@@ -5,7 +5,7 @@
 ;;; Copyright © 2015, 2018 Mark H Weaver 
 ;;; Copyright © 2018 Arun Isaac 
 ;;; Copyright © 2018, 2019 Ricardo Wurmus 
-;;; Copyright © 2021 Maxime Devos 
+;;; Copyright © 2021, 2022 Maxime Devos 
 ;;; Copyright © 2020 Christine Lemmer-Webber 
 ;;; Copyright © 2021 Brice Waegeneire 
 ;;;
@@ -65,45 +65,61 @@ (define (dot-or-dot-dot? file)
   (member file '("." "..")))
 
 ;; Based upon mkdir-p from (guix build utils)
-(define (verify-not-symbolic dir)
-  "Verify DIR or its ancestors aren't symbolic links."
+(define (mkdir-p/perms directory owner bits)
+  "Create directory DIRECTORY and all its ancestors.
+
+Additionally, verify no component of DIRECTORY is a symbolic link,
+without TOCTTOU races.  However, if OWNER differs from the the current
+(process) uid/gid, there is a small window in which DIRECTORY is set to the
+current (process) uid/gid instead of OWNER.  This is not expected to be
+a problem in practice.
+
+The permission bits and owner of DIRECTORY are set to BITS and OWNER.
+Anything above DIRECTORY that already exists keeps
+its old owner and bits.  For components that do not exist yet, the owner
+and bits are set according to the default behaviour of 'mkdir'."
   (define absolute?
-(string-prefix? "/" dir))
+(string-prefix? "/" directory))
 
   (define not-slash
 (char-set-complement (char-set #\/)))
 
-  (define (verify-component file)
-(unless (eq? 'directory (stat:type (lstat file)))
-  (error "file name component is not a directory" dir)))
-
-  (let loop ((components (string-tokenize dir not-slash))
- (root   (if absolute?
- ""
- ".")))
+  ;; By combining O_NOFOLLOW and O_DIRECTORY, this procedure automatically
+  ;; verifies that no components are symlinks.
+  (define open-flags (logior O_CLOEXEC ; don't pass the port on to subprocesses
+ O_NOFOLLOW ; don't follow symlinks
+ O_DIRECTORY ; reject anything not a directory
+ O_PATH)) ; TODO: Does Hurd have O_PATH?
+  
+  (let loop ((components (string-tokenize directory not-slash))
+ (root (open (if absolute? "/" ".") open-flags)))
 (match components
   ((head tail ...)
-   (let ((file (string-append root "/" head)))
- (catch 'system-error
-   (lambda ()
- (verify-component file)
- (loop tail file))
-   (lambda args
- (if (= ENOENT (system-error-errno args))
- #t
- (apply throw args))
-  (() #t
-
-;; TODO: the TOCTTOU race can be addressed once guile has bindings
-;; for fstatat, openat and friends.
-(define (mkdir-p/perms directory owner bits)
-  "Create the directory DIRECTORY and all its ancestors.
-Verify no component of DIRECTORY is a symbolic link.
-Warning: this is currently suspect to a TOCTTOU race!"
-  (verify-not-symbolic directory)
-  (mkdir-p directory)
-  (chown directory (passwd:uid owner) (passwd:gid owner))
-  (chmod directory bits))
+   (let retry ()
+ ;; In the usual case, we expect HEAD to already exist.
+ (match (catch 'system-error
+  (lambda ()
+(openat root head open-flags))
+  (lambda args
+(if (= ENOENT (system-error-errno args))
+#false
+(apply throw args
+   ((? port? new-root)
+(close root)
+(loop tail new-root))
+   (#false
+;; If not, create it.
+(catch 'system-error
+  (lambda _
+(mkdirat root head))
+  (lambda args
+;; Someone else created the directory.  Unexpected but fine.
+(unless (= EEXIST (system-error-errno args))
+  (apply throw args
+(retry)
+  (()
+   (chown directory (passwd:uid owner) (passwd:gid owner))
+   (chmod directory bits)
 
 (define* (copy-account-skeletons home
  #:key


OpenPGP_0x49E3EE22191725EE.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

bug#47584: Race condition in ‘copy-account-skeletons’: possible privilege escalation.

[Maxime Devos]

On Tue, 2021-04-06 at 13:57 +0200, Ludovic Courtès wrote:
> [...]
> 
> The blog post and info-guix messages are the highest levels of
> visibility we can give, roughly.  So I think we have to think twice
> before doing that or truly important issues will eventually go
> unnoticed.
> 
> The risk with this issue seems much lower than that of the keep-failed
> issue, it even looks super low.
> 
> WDYT?

That is a good point, but I still wonder if there's *somewhere* this
can be posted.

I was going to start a thread at guix-devel about
blog posts in general (categories, what can be posted as a ‘official’
blog post on guix.gnu.org, any maximal frequencies ...) but I ended up
being busy with other things.

Greetings,
Maxime.


signature.asc
Description: This is a digitally signed message part

bug#47584: Race condition in ‘copy-account-skeletons’: possible privilege escalation.

[Ludovic Courtès]

Hi Maxime,

Maxime Devos  skribis:

> On Mon, 2021-04-05 at 21:54 +0200, Ludovic Courtès wrote:
>> [...]
>> 
>> OK.  It does mean that the bug is hardly exploitable in practice: you
>> have to be able to log in at all,
> Yes.
>
>>  and if you’re able to log in, you have
>> to log in precisely within the 1s (or less) that follows account
>> creation, which sounds challenging (TCP + SSH connection establishment
>> is likely to take as much time or more,
>
> Is logging in possible when the home directory doesn't exist?

I think so.

> An attacker could copy and paste, or have used a single-character password,
> to save some time.

Hmm yes.  It’s a bit a far-fetched though: the attacker would have
passed the sysadmin the output of the ‘crypt’ procedure, such that the
sysadmin cannot know the password length.

>> Does it warrant as strong messaging as for the recent daemon
>> ‘--keep-failed’ vulnerability?
>
> As it is a one-time chance, with a limited window, and only under specific
> circumstances (creating a new user account), I don't think so.  But I would
> still recommend to upgrade.  Does the blog post have ‘too strong messaging’? 

The blog post and info-guix messages are the highest levels of
visibility we can give, roughly.  So I think we have to think twice
before doing that or truly important issues will eventually go
unnoticed.

The risk with this issue seems much lower than that of the keep-failed
issue, it even looks super low.

WDYT?

Ludo’.

bug#47584: Race condition in ‘copy-account-skeletons’: possible privilege escalation.

[Maxime Devos]

On Mon, 2021-04-05 at 21:54 +0200, Ludovic Courtès wrote:
> [...]
> 
> OK.  It does mean that the bug is hardly exploitable in practice: you
> have to be able to log in at all,
Yes.

>  and if you’re able to log in, you have
> to log in precisely within the 1s (or less) that follows account
> creation, which sounds challenging (TCP + SSH connection establishment
> is likely to take as much time or more,

Is logging in possible when the home directory doesn't exist?
It isn't possible from the console.  I guess it isn't possible from SSH
either.

If it is possible,
then the window would be somewhat larger I think.  Account creation is done
at activation time, while creating home directories is done as a shepherd
service (see account-service-type in gnu/system/shadow.scm).

>  likewise for typing in your password.)
An attacker could copy and paste, or have used a single-character password,
to save some time.

>   It’s also one-time chance.

Yes.

> Do I get it right?

I think so, except the window might be larger (but still a one-time chance).

> Does it warrant as strong messaging as for the recent daemon
> ‘--keep-failed’ vulnerability?

As it is a one-time chance, with a limited window, and only under specific
circumstances (creating a new user account), I don't think so.  But I would
still recommend to upgrade.  Does the blog post have ‘too strong messaging’? 

Greetings,
Maxime


signature.asc
Description: This is a digitally signed message part

bug#47584: Race condition in ‘copy-account-skeletons’: possible privilege escalation.

[Ludovic Courtès]

Hi Maxime,

Maxime Devos  skribis:

> On Sat, 2021-04-03 at 22:33 +0200, Ludovic Courtès wrote:
>> Maxime Devos  skribis:
>> 
>> > The attack consists of the user being logged in after the account
>> > skeletons have been copied to the home directory, but before the
>> > owner of the account skeletons have been set.  The user then deletes
>> > a copied account skeleton (e.g. @file{$HOME/.gdbinit}) and replaces
>> > it with a symbolic link to a file not owned by the user, such as
>> > @file{/etc/shadow}.
>> > 
>> > The activation code then changes the ownership
>> > of the file the symbolic link points to instead of the symbolic
>> > link itself.  At that point, the user has read-write access
>> > to the target file.
>> 
>> In the draft blog post, you mention that the attack cannot be carried
>> out when protected symlinks are enabled.
>
> In the blog post, I thought I wrote the attack can be carried out
> *even if* protected symlinks are enabled.  Looking at
>
> https://sysctl-explorer.net/fs/protected_symlinks/,
>
> I don't think the Linux protected symlink feature helps, as home
> directories are never sticky and word-writable.

Oh right, my bad, I overlooked this.

> Perhaps I should have written ‘possible’ instead of ‘not impossible’
> in the blog post.

Dunno, maybe it’s just me not paying enough attention.

> I agree with all other comments on this bug report.

OK.  It does mean that the bug is hardly exploitable in practice: you
have to be able to log in at all, and if you’re able to log in, you have
to log in precisely within the 1s (or less) that follows account
creation, which sounds challenging (TCP + SSH connection establishment
is likely to take as much time or more, likewise for typing in your
password.)  It’s also one-time chance.

Do I get it right?

Does it warrant as strong messaging as for the recent daemon
‘--keep-failed’ vulnerability?

Thanks,
Ludo’.

bug#47584: Race condition in ‘copy-account-skeletons’: possible privilege escalation.

[Maxime Devos]

On Sat, 2021-04-03 at 18:26 +0200, Maxime Devos wrote:
> A suggested blog post is attached.
A revised blog post is attached.

The following points are currently _not_ addressed:

Ludovic Courtès wrote:
> Also…  in this paragraph, it’s not entirely clear which user we’re
> talking about it.  In news.scm, I reworded it like so:
>  The attack can happen when @command{guix system reconfigure} is running.
>  Running @command{guix system reconfigure} can trigger the creation of new 
> user
>  accounts if the configuration specifies new accounts.  If a user whose 
> account
>  is being created manages to log in after the account has been created but
>  before ``skeleton files'' copied to its home directory have the right
>  ownership, they may, by creating an appropriately-named symbolic link in the
>  home directory pointing to a sensitive file, such as @file{/etc/shadow}, get
>  root privileges.
>
> It may also be worth mentioning that the user is likely unable to log in
> at all at that point, as I wrote here:

I can't think of something along these lines to write at the moment ...

Greetings,
Maxime.
From 10b4528aac6cd9c0c341634b9f163f0a38ec4c6b Mon Sep 17 00:00:00 2001
From: Maxime Devos 
Date: Sat, 3 Apr 2021 18:02:05 +0200
Subject: [PATCH] =?UTF-8?q?website:=20Add=20post=20about=20vulnerability?=
 =?UTF-8?q?=20in=20=E2=80=98copy-account-skeletons=E2=80=99.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* website/posts/home-symlink.md: New post.

Co-authored-by: Ludovic Courtès 
---
 website/posts/home-symlink.md | 86 +++
 1 file changed, 86 insertions(+)
 create mode 100644 website/posts/home-symlink.md

diff --git a/website/posts/home-symlink.md b/website/posts/home-symlink.md
new file mode 100644
index 000..67f3053
--- /dev/null
+++ b/website/posts/home-symlink.md
@@ -0,0 +1,86 @@
+title: Risk of local privilege escalation in account creation
+date: 2021-04-04 15:30
+author: Maxime Devos, Ludovic Courtès
+tags: Security Advisory
+---
+
+A security vulnerability that can lead to local privilege
+escalation has been found in the code that creates user accounts on Guix
+System—Guix on other distros is unaffected.  The system is only vulnerable
+during the activation of non-system user accounts that do not already exist.
+
+This exploit is _not_ prevented by the Linux [protected
+symlinks](https://sysctl-explorer.net/fs/protected_symlinks/) feature.
+
+# Vulnerability
+
+The attack consists of the user being logged in after the account
+skeletons have been copied to the home directory, but before the
+owner of the account skeletons have been set.  The user then deletes
+a copied account skeleton (e.g. `$HOME/.gdbinit`) and replaces
+it with a symbolic link to a file not owned by the user, such as
+`/etc/shadow`.
+
+The activation code then changes the ownership of the file the symbolic
+link points to instead of the symbolic link itself.  At that point, the
+user has read-write access to the target file.
+
+# Fix
+
+This [bug](https://issues.guix.gnu.org/47584) has been
+[fixed](https://git.savannah.gnu.org/cgit/guix.git/commit/?id=2161820ebbbab62a5ce76c9101ebaec54dc61586).
+See below for upgrade instructions.
+
+The fix consist of initially creating the home directory root-owned and only
+changing the owner of the home directory once all skeletons have been copied
+and their owner has been set.
+
+# Upgrading
+
+To upgrade the Guix System, run something like:
+
+```
+guix pull
+sudo guix system reconfigure /run/current-system/configuration.scm
+sudo reboot
+```
+
+As the user account activation code is run as a shepherd service,
+the last step is required to make sure the fixed activation code
+is run in the future.
+
+To avoid the vulnerability while upgrading the system, only declare
+new user accounts in the configuration file after the Guix System
+has been upgraded.
+
+# Conclusions
+
+Work is ongoing to support the `openat` family of POSIX functions in
+Guile, which, when used, help address this class of vulnerabilities.
+
+This issue is tracked as
+[bug #47584](https://issues.guix.gnu.org/47584); you can read the thread
+for more information.
+
+Please report any issues you may have to
+[`guix-de...@gnu.org`](https://guix.gnu.org/en/contact/).  See the
+[security web page](https://guix.gnu.org/en/security/) for information
+on how to report security issues.
+
+ About GNU Guix
+
+[GNU Guix](https://guix.gnu.org) is a transactional package manager and
+an advanced distribution of the GNU system that [respects user
+freedom](https://www.gnu.org/distros/free-system-distribution-guidelines.html).
+Guix can be used on top of any system running the Hurd or the Linux
+kernel, or it can be used as a standalone operating system distribution
+for i686, x86_64, ARMv7, and AArch64 machines.
+
+In addition to standard package management features, Guix supports
+transactional upgrades and roll-backs, unprivileged package 

bug#47584: Race condition in ‘copy-account-skeletons’: possible privilege escalation.

[Maxime Devos]

On Sat, 2021-04-03 at 22:33 +0200, Ludovic Courtès wrote:
> Maxime Devos  skribis:
> 
> > The attack consists of the user being logged in after the account
> > skeletons have been copied to the home directory, but before the
> > owner of the account skeletons have been set.  The user then deletes
> > a copied account skeleton (e.g. @file{$HOME/.gdbinit}) and replaces
> > it with a symbolic link to a file not owned by the user, such as
> > @file{/etc/shadow}.
> > 
> > The activation code then changes the ownership
> > of the file the symbolic link points to instead of the symbolic
> > link itself.  At that point, the user has read-write access
> > to the target file.
> 
> In the draft blog post, you mention that the attack cannot be carried
> out when protected symlinks are enabled.

In the blog post, I thought I wrote the attack can be carried out
*even if* protected symlinks are enabled.  Looking at

https://sysctl-explorer.net/fs/protected_symlinks/,

I don't think the Linux protected symlink feature helps, as home
directories are never sticky and word-writable.

Perhaps I should have written ‘possible’ instead of ‘not impossible’
in the blog post.

> Please mention that protected symlinks are enabled by default on Guix
> System since a March 16th commit, with a link to [...]

See my response above.

I agree with all other comments on this bug report.

Greetings,
Maxime.


signature.asc
Description: This is a digitally signed message part

bug#47584: Race condition in ‘copy-account-skeletons’: possible privilege escalation.

[Ludovic Courtès]

Maxime Devos  skribis:

> +The attack consists of the user being logged in after the account
> +skeletons have been copied to the home directory, but before the
> +owner of the account skeletons have been set.  The user then deletes
> +a copied account skeleton (e.g. `$HOME/.gdbinit`) and replaces
> +it with a symbolic link to a file not owned by the user, such as
> +`/etc/shadow`.

Also…  in this paragraph, it’s not entirely clear which user we’re
talking about it.  In news.scm, I reworded it like so:

  The attack can happen when @command{guix system reconfigure} is running.
  Running @command{guix system reconfigure} can trigger the creation of new user
  accounts if the configuration specifies new accounts.  If a user whose account
  is being created manages to log in after the account has been created but
  before ``skeleton files'' copied to its home directory have the right
  ownership, they may, by creating an appropriately-named symbolic link in the
  home directory pointing to a sensitive file, such as @file{/etc/shadow}, get
  root privileges.

It may also be worth mentioning that the user is likely unable to log in
at all at that point, as I wrote here:

  https://issues.guix.gnu.org/47584#6

WDYT?

Ludo’.

bug#47584: Race condition in ‘copy-account-skeletons’: possible privilege escalation.

[Ludovic Courtès]

Maxime Devos  skribis:

> From 7937b9f18085569e5d7cb8a3c4dc08e1088a94a9 Mon Sep 17 00:00:00 2001
> From: Maxime Devos 
> Date: Sat, 3 Apr 2021 18:02:05 +0200
> Subject: [PATCH] =?UTF-8?q?website:=20Add=20post=20about=20vulnerability?=
>  =?UTF-8?q?=20in=20=E2=80=98copy-account-skeletons=E2=80=99.?=
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> * website/posts/home-symlink.md: New post.

It’s unfortunate that this is going out during a week-end, and a
three-day week-end on top of that in some regions of the world, with
many people not seeing the message and not being able to act upon it for
three days.

> +title: Risk of local privilege escalation in account creation
> +date: 2021-04-03 17:30
> +author: Maxime Devos
> +tags: Security Advisory
> +---
> +
> +A security vulnerability that can lead to local privilege escalation
> +has been found in the activation code of user accounts (excluding
> +system accounts).  It does not affect users on foreign distros
> +and is only exploitable during system reconfiguration.

How about this, taken from the news.scm entry I tweaked:

  A security vulnerability that can lead to local privilege
  escalation has been found in the code that creates user accounts on Guix
  System—Guix on other distros is unaffected.  The system is only vulnerable
  during the activation of non-system user accounts that do not already exist.

(This is more upfront about who’s affected and avoids the technical term
“activation code” which makes no sense outside the circle of Guix System
and NixOS hackers.)

> +This exploit is _not_ impossible on machines where the Linux [protected
> +symlinks](https://sysctl-explorer.net/fs/protected_symlinks/) feature
> +is enabled.  It is believed the attack can also be performed using hard
> +links.

Please mention that protected symlinks are enabled by default on Guix
System since a March 16th commit, with a link to
.

> +# Conclusions
> +
> +The activation code in Guix System originally was written with the
> +assumption that no other code was running at the same time in mind.
> +However, this is not a reasonable assumption in practice, as this
> +vulnerability demonstrates.  Thus, it may be worthwhile to look
> +over other activation code for similar issues.

That’s an interesting conclusion for us developers, but not necessarily
for the users this is targeting.  It also sounds unnecessarily scary and
casual.

> +While investigating how to fix the issue, it became apparent GNU Guile,
> +the implementation of the Algorithmic Language Scheme GNU Guix is
> +written in, is lacking in primitives that usually are used to avoid
> +these kind of issues, such `openat` and `O_NOFOLLOW`.
> +
> +While these primitives turned out not to be necessary to fix the
> +issue and a [patch 
> series]()
> +to GNU Guile has been submitted that adds these primitives, this does
> +serve as a remainder that GNU Guile is a critical component of
> +Guix System and working around missing primitives will not always be 
> possible.

All this is true but also probably too detailed (or not enough,
depending on the reader).  How about just mentioning that work is
ongoing to support the `openat` family of POSIX functions in Guile,
which, when used, while help address this class of vulnerability?

Otherwise LGTM, thanks!

Ludo’.

bug#47584: Race condition in ‘copy-account-skeletons’: possible privilege escalation.

[Ludovic Courtès]

Maxime Devos  skribis:

> The attack consists of the user being logged in after the account
> skeletons have been copied to the home directory, but before the
> owner of the account skeletons have been set.  The user then deletes
> a copied account skeleton (e.g. @file{$HOME/.gdbinit}) and replaces
> it with a symbolic link to a file not owned by the user, such as
> @file{/etc/shadow}.
>
> The activation code then changes the ownership
> of the file the symbolic link points to instead of the symbolic
> link itself.  At that point, the user has read-write access
> to the target file.

In the draft blog post, you mention that the attack cannot be carried
out when protected symlinks are enabled.  This is now the case by
default on Guix System¹, so in that case, a system upgraded from a
commit after March 16th is unaffected.

Ludo’.

¹ https://issues.guix.gnu.org/47013#13

bug#47584: Race condition in ‘copy-account-skeletons’: possible privilege escalation.

[Ludovic Courtès]

Note that this issue is about Guix System; users of Guix on other
distros are unaffected.

Maxime Devos  skribis:

> The attack consists of the user being logged in after the account
> skeletons have been copied to the home directory, but before the
> owner of the account skeletons have been set.  The user then deletes
> a copied account skeleton (e.g. @file{$HOME/.gdbinit}) and replaces
> it with a symbolic link to a file not owned by the user, such as
> @file{/etc/shadow}.
>
> The activation code then changes the ownership
> of the file the symbolic link points to instead of the symbolic
> link itself.  At that point, the user has read-write access
> to the target file.

To give a bit more context, account creation on Guix System happens
while ‘guix system reconfigure’ is running.

The user whose account is being created thus needs to be able to log in
right during the time window described above.

Users whose password is uninitialized (i.e., the ‘password’ field of
 is left unspecified¹) cannot log in at that point, unless
possibly if the OpenSSH configuration specifies an authorized key for
the user account.

Ludo’.

¹ https://guix.gnu.org/manual/en/html_node/User-Accounts.html
² 
https://guix.gnu.org/manual/en/html_node/Networking-Services.html#index-openssh_002dservice_002dtype

bug#47584: Race condition in ‘copy-account-skeletons’: possible privilege escalation.

[Ludovic Courtès]

Hi Maxime,

Maxime Devos  skribis:

> From 9672bd37bf50db1e0989d0b84035c4788422bd31 Mon Sep 17 00:00:00 2001
> From: Maxime Devos 
> Date: Tue, 30 Mar 2021 22:36:14 +0200
> Subject: [PATCH 1/2] activation: Do not dereference symlinks in home directory
>  creation.
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Fixes .
>
> * gnu/build/activation.scm
>   (copy-account-skeletons): Do not chown the home directory; leave this
>   to 'activate-user-home'.
>   (activate-user-home): Only chown the home directory after the account
>   skeletons have been copied.
>
> Co-authored-by: Ludovic Courtès .

Pushed:

  
https://git.savannah.gnu.org/cgit/guix.git/commit/?id=2161820ebbbab62a5ce76c9101ebaec54dc61586

> From d071ee3aff5be1a6d7876d7411e70f7283dce1fb Mon Sep 17 00:00:00 2001
> From: Maxime Devos 
> Date: Sat, 3 Apr 2021 12:19:10 +0200
> Subject: [PATCH 2/2] news: Add entry for user account activation
>  vulnerability.
>
> TODO for guix committer: correct the commit id appropriately.
>
> * etc/news.scm: Add entry.

I tweaked it to (1) make it clear upfront that only Guix System is
affected, (2) to explicitly recommend an upgrade on Guix System, and (3)
to clarify when the attack can happen.

Thanks for finding the issue, for reporting it at guix-security, and for
preparing these patches!

Ludo’.

bug#47584: Race condition in ‘copy-account-skeletons’: possible privilege escalation.

[Maxime Devos]

On Sat, 2021-04-03 at 18:22 +0200, Maxime Devos wrote:
> +;; It is important 'chown' is called after 
> 'copy-account-skeletons'
> +;; Otherwise, a malicious user with good timing could
> +;; create a symlink in HOME that would be dereferenced by
> +;; 'copy-account-skeletons'.

Oops please add a period after 'copy-account-skeletons';

bug#47584: Race condition in ‘copy-account-skeletons’: possible privilege escalation.

[Maxime Devos]

A suggested blog post is attached.
From 7937b9f18085569e5d7cb8a3c4dc08e1088a94a9 Mon Sep 17 00:00:00 2001
From: Maxime Devos 
Date: Sat, 3 Apr 2021 18:02:05 +0200
Subject: [PATCH] =?UTF-8?q?website:=20Add=20post=20about=20vulnerability?=
 =?UTF-8?q?=20in=20=E2=80=98copy-account-skeletons=E2=80=99.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* website/posts/home-symlink.md: New post.
---
 website/posts/home-symlink.md | 103 ++
 1 file changed, 103 insertions(+)
 create mode 100644 website/posts/home-symlink.md

diff --git a/website/posts/home-symlink.md b/website/posts/home-symlink.md
new file mode 100644
index 000..9289870
--- /dev/null
+++ b/website/posts/home-symlink.md
@@ -0,0 +1,103 @@
+title: Risk of local privilege escalation in account creation
+date: 2021-04-03 17:30
+author: Maxime Devos
+tags: Security Advisory
+---
+
+A security vulnerability that can lead to local privilege escalation
+has been found in the activation code of user accounts (excluding
+system accounts).  It does not affect users on foreign distros
+and is only exploitable during system reconfiguration.
+
+This exploit is _not_ impossible on machines where the Linux [protected
+symlinks](https://sysctl-explorer.net/fs/protected_symlinks/) feature
+is enabled.  It is believed the attack can also be performed using hard
+links.
+
+# Vulnerability
+
+The attack consists of the user being logged in after the account
+skeletons have been copied to the home directory, but before the
+owner of the account skeletons have been set.  The user then deletes
+a copied account skeleton (e.g. `$HOME/.gdbinit`) and replaces
+it with a symbolic link to a file not owned by the user, such as
+`/etc/shadow`.
+
+The activation code then changes the ownership of the file the symbolic
+link points to instead of the symbolic link itself.  At that point, the
+user has read-write access to the target file.
+
+# Fix
+
+This [bug](https://issues.guix.gnu.org/47584) has been
+
+[fixed](https://git.savannah.gnu.org/cgit/guix.git/commit/?id= XXX).
+See below for upgrade instructions.
+
+The fix consist of initially creating the home directory root-owned and only
+changing the owner of the home directory once all skeletons have been copied
+and their owner has been set.
+
+# Upgrading
+
+To upgrade the Guix System, run something like:
+
+```
+guix pull
+sudo guix system reconfigure /run/current-system/configuration.scm
+sudo reboot
+```
+
+As the user account activation code is run as a shepherd service,
+the last step is required to make sure the fixed activation code
+is run in the future.
+
+To avoid the vulnerability while upgrading the system, only declare
+new user accounts in the configuration file after the Guix System
+has been upgraded.
+
+# Conclusions
+
+The activation code in Guix System originally was written with the
+assumption that no other code was running at the same time in mind.
+However, this is not a reasonable assumption in practice, as this
+vulnerability demonstrates.  Thus, it may be worthwhile to look
+over other activation code for similar issues.
+
+While investigating how to fix the issue, it became apparent GNU Guile,
+the implementation of the Algorithmic Language Scheme GNU Guix is
+written in, is lacking in primitives that usually are used to avoid
+these kind of issues, such `openat` and `O_NOFOLLOW`.
+
+While these primitives turned out not to be necessary to fix the
+issue and a [patch series]()
+to GNU Guile has been submitted that adds these primitives, this does
+serve as a remainder that GNU Guile is a critical component of
+Guix System and working around missing primitives will not always be possible.
+
+This issue is tracked as
+[bug #47584](https://issues.guix.gnu.org/47584); you can read the thread
+for more information.
+
+Please report any issues you may have to
+[`guix-de...@gnu.org`](https://guix.gnu.org/en/contact/).  See the
+[security web page](https://guix.gnu.org/en/security/) for information
+on how to report security issues.
+
+ About GNU Guix
+
+[GNU Guix](https://guix.gnu.org) is a transactional package manager and
+an advanced distribution of the GNU system that [respects user
+freedom](https://www.gnu.org/distros/free-system-distribution-guidelines.html).
+Guix can be used on top of any system running the Hurd or the Linux
+kernel, or it can be used as a standalone operating system distribution
+for i686, x86_64, ARMv7, and AArch64 machines.
+
+In addition to standard package management features, Guix supports
+transactional upgrades and roll-backs, unprivileged package management,
+per-user profiles, and garbage collection.  When used as a standalone
+GNU/Linux distribution, Guix offers a declarative, stateless approach to
+operating system configuration management.  Guix is highly customizable
+and hackable through 

bug#47584: Race condition in ‘copy-account-skeletons’: possible privilege escalation.

[Maxime Devos]

Patch is attached.
The committer will need to change the commit id appropriately.
From 9672bd37bf50db1e0989d0b84035c4788422bd31 Mon Sep 17 00:00:00 2001
From: Maxime Devos 
Date: Tue, 30 Mar 2021 22:36:14 +0200
Subject: [PATCH 1/2] activation: Do not dereference symlinks in home directory
 creation.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes .

* gnu/build/activation.scm
  (copy-account-skeletons): Do not chown the home directory; leave this
  to 'activate-user-home'.
  (activate-user-home): Only chown the home directory after the account
  skeletons have been copied.

Co-authored-by: Ludovic Courtès .
---
 gnu/build/activation.scm | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/gnu/build/activation.scm b/gnu/build/activation.scm
index 6cb6f8819b..43d973d3da 100644
--- a/gnu/build/activation.scm
+++ b/gnu/build/activation.scm
@@ -107,7 +107,8 @@ Warning: this is currently suspect to a TOCTTOU race!"
  (directory %skeleton-directory)
  uid gid)
   "Copy the account skeletons from DIRECTORY to HOME.  When UID is an integer,
-make it the owner of all the files created; likewise for GID."
+make it the owner of all the files created except the home directory; likewise
+for GID."
   (define (set-owner file)
 (when (or uid gid)
   (chown file (or uid -1) (or gid -1
@@ -115,7 +116,6 @@ make it the owner of all the files created; likewise for GID."
   (let ((files (scandir directory (negate dot-or-dot-dot?)
 stringFrom d071ee3aff5be1a6d7876d7411e70f7283dce1fb Mon Sep 17 00:00:00 2001
From: Maxime Devos 
Date: Sat, 3 Apr 2021 12:19:10 +0200
Subject: [PATCH 2/2] news: Add entry for user account activation
 vulnerability.

TODO for guix committer: correct the commit id appropriately.

* etc/news.scm: Add entry.
---
 etc/news.scm | 17 +
 1 file changed, 17 insertions(+)

diff --git a/etc/news.scm b/etc/news.scm
index deedc69f6e..0cc9c183a0 100644
--- a/etc/news.scm
+++ b/etc/news.scm
@@ -12,6 +12,7 @@
 ;; Copyright © 2020, 2021 Maxim Cournoyer 
 ;; Copyright © 2021 Leo Famulari 
 ;; Copyright © 2021 Zhu Zihao 
+;; Copyright © 2021 Maxime Devos 
 ;;
 ;; Copying and distribution of this file, with or without modification, are
 ;; permitted in any medium without royalty provided the copyright notice and
@@ -20,6 +21,22 @@
 (channel-news
  (version 0)
 
+ ;; XXX to guix committers: this commit likely needs to be changed.
+ (entry (commit "9672bd37bf50db1e0989d0b84035c4788422bd31")
+(title
+ (en "Risk of local privilege escalation by creation of new user accounts"))
+(body
+ (en "A security vulnerability that can lead to local privilege
+escalation has been found in the activation code of user accounts.  The
+system is only vulnerable during the activation of user accounts (including
+system accounts) that do not already exist.
+
+The attack consists of the user logging in after the user's home directory
+has been created, but before the activation of the user has been completed,
+by creating an appropriately named symbolic link in the home directory
+pointing to a sensitive file, such as @file{/etc/shadow}.
+
+See @uref{https://issues.guix.gnu.org/47584} for more information on this bug.")))
  (entry (commit "9ade2b720af91acecf76278b4d9b99ace406781e")
 (title
  (en "Update on previous @command{guix-daemon} local privilege escalation")
-- 
2.31.1



signature.asc
Description: This is a digitally signed message part

bug#47584: Race condition in ‘copy-account-skeletons’: possible privilege escalation.

[Maxime Devos]

A TOCTTOU (time-of-check to time-of-use) vulnerability has been found
in the activation code of user accounts, more specifically in the
code that copies the account skeletons.

* Vulnerability

The attack consists of the user being logged in after the account
skeletons have been copied to the home directory, but before the
owner of the account skeletons have been set.  The user then deletes
a copied account skeleton (e.g. @file{$HOME/.gdbinit}) and replaces
it with a symbolic link to a file not owned by the user, such as
@file{/etc/shadow}.

The activation code then changes the ownership
of the file the symbolic link points to instead of the symbolic
link itself.  At that point, the user has read-write access
to the target file.

* Where in the code does this happen?

Module: (gnu build activation).
Procedures: 'copy-account-skeletons' and 'activate-user-home'.

'copy-account-skeletons' creates the home directory, sets it
owner, copies the account skeletons, and chowns the copied skeletons,
in that order.   The bug is that it dereferences symbolic links.

It is called from 'activate-user-home' if the home directory does
not already exist.

* Fix

The fix consist of initially creating the home directory root-owned and only
changing the owner of the home directory once all skeletons have been copied
and their owner has been set.

* Extra notes

A blog post, a news entry and a fix have been prepared and will be posted
and hopefully merged soon.  The following tests succeeded:

$ make check-system TESTS='switch-to-system upgrade-services install-bootloader 
basic'
$ make check


signature.asc
Description: This is a digitally signed message part