GNU bug report logs - #48923
[PATCH] build: utils: Add ‘call-with-outp

Previous Next

To reply to this bug, email your comments to 48923 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Xinglu Chen <public <at> yoctocell.xyz>
To: guix-patches <at> gnu.org
Cc: Maxime Devos <maximedevos <at> telenet.be>
Subject: [PATCH] build: utils: Add ‘call-with-outp
Date: Tue, 08 Jun 2021 17:40:52 +0200

Using ‘call-with-output-file*’ instead of ‘call-with-output-file’ and ‘chmod’
will prevent secrets from being leaked.  See
<https://issues.guix.gnu.org/48872>.

* guix/build/utils.scm (call-with-output-file*): New procedure.
* doc/guix.texi (Build Utilities): Document it.
---
 doc/guix.texi        | 19 +++++++++++++++++++
 guix/build/utils.scm | 10 ++++++++++
 2 files changed, 29 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index 59b4ac11b4..7e15cd9e92 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -8612,6 +8612,25 @@ Be careful about using @code{$} to match the end of a line; by itself it
 won't match the terminating newline of a line.
 @end deffn
 
+@deffn {Scheme Procedure} call-with-output-file* @var{file} @var{proc} @
+  [#:perms #o666]
+Open FILE for output, set the file permission bits to @var{perms}, and
+call @code{(PROC port)} with the resulting port.
+
+The advantage of using this procedure compared to something like this
+
+@lisp
+(call-with-output-file "FILE"
+  (lambda (port)
+    (display "top secret" port)))
+(chmod "FILE" #o400)
+@end lisp
+
+is that, with the latter, an unpriviliged user could open @var{file}
+before the permission was changed to @code{#o400}, thus making it
+possible to leak sensitive information.
+@end deffn
+
 @subsection File Search
 
 @cindex file, searching
diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index 419c10195b..df960eee84 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2015, 2018 Mark H Weaver <mhw <at> netris.org>
 ;;; Copyright © 2018 Arun Isaac <arunisaac <at> systemreboot.net>
 ;;; Copyright © 2018, 2019 Ricardo Wurmus <rekado <at> elephly.net>
+;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -66,6 +67,7 @@
             file-name-predicate
             find-files
             false-if-file-not-found
+            call-with-output-file*
 
             search-path-as-list
             set-path-environment-variable
@@ -448,6 +450,14 @@ also be included.  If FAIL-ON-ERROR? is true, raise an exception upon error."
           #f
           (apply throw args)))))
 
+;; Prevent secrets from leaking, see <https://issues.guix.gnu.org/48872>
+(define* (call-with-output-file* file proc #:key (perms #o666))
+  "FILE should be string containg the path to a file, PROC should be a procedure
+that accepts the port as an argument, and PERMS should be the permission bits
+of the file, the default is 666."
+  (let ((port (open file (bitwise-ior O_WRONLY O_CREAT) perms)))
+    (call-with-port port proc)))
+
 
 ;;;
 ;;; Search paths.

base-commit: 503c2039a280dd52a751a6852b4157fccd1b4195
-- 
2.32.0

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Xinglu Chen <public <at> yoctocell.xyz>, guix-patches <at> gnu.org
Subject: Re: [PATCH] build: utils: Add ‘call-with-outp
Date: Tue, 08 Jun 2021 18:04:40 +0200

[Message part 1 (text/plain, inline)]

Xinglu Chen schreef op di 08-06-2021 om 17:40 [+0200]:
> Using ‘call-with-output-file*’ instead of ‘call-with-output-file’ and ‘chmod’
> will prevent secrets from being leaked.  See
> <https://issues.guix.gnu.org/48872>;.

This procedure LGTM (but I didn't test).
However,

> diff --git a/guix/build/utils.scm b/guix/build/utils.scm
> index 419c10195b..df960eee84 100644
> --- a/guix/build/utils.scm
> +++ b/guix/build/utils.scm
> @@ -5,6 +5,7 @@

Modifying (guix build utils) entails a world-rebuild, as
(guix build utils) is used by the build code of practically
every package. I would suggest placing it in (gnu build activation)
instead.

Greetings,
Maxime.

[signature.asc (application/pgp-signature, inline)]

Message #11 received at 48923 <at> debbugs.gnu.org (full text, mbox):

From: Xinglu Chen <public <at> yoctocell.xyz>
To: Maxime Devos <maximedevos <at> telenet.be>, 48923 <at> debbugs.gnu.org
Subject: Re: [bug#48923] [PATCH] build: utils: Add ‘call-with-outp
Date: Tue, 08 Jun 2021 19:36:07 +0200

[Message part 1 (text/plain, inline)]

On Tue, Jun 08 2021, Maxime Devos wrote:

> Xinglu Chen schreef op di 08-06-2021 om 17:40 [+0200]:
>> Using ‘call-with-output-file*’ instead of ‘call-with-output-file’ and ‘chmod’
>> will prevent secrets from being leaked.  See
>> <https://issues.guix.gnu.org/48872>;.
>
> This procedure LGTM (but I didn't test).
> However,
>
>> diff --git a/guix/build/utils.scm b/guix/build/utils.scm
>> index 419c10195b..df960eee84 100644
>> --- a/guix/build/utils.scm
>> +++ b/guix/build/utils.scm
>> @@ -5,6 +5,7 @@
>
> Modifying (guix build utils) entails a world-rebuild, as
> (guix build utils) is used by the build code of practically
> every package. I would suggest placing it in (gnu build activation)
> instead.

Oh, I didn’t think about that.  Moving it to (gnu build activation)
seems like a good option.

Should I create a new “Activation” section in the manual, or should I
keep it in the “Build Utilities” section?

[signature.asc (application/pgp-signature, inline)]

Message #14 received at 48923 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Xinglu Chen <public <at> yoctocell.xyz>, 48923 <at> debbugs.gnu.org
Subject: Re: [bug#48923] [PATCH] build: utils: Add
 ‘call-with-outp
Date: Tue, 08 Jun 2021 19:45:49 +0200

[Message part 1 (text/plain, inline)]

Xinglu Chen schreef op di 08-06-2021 om 19:36 [+0200]:
> On Tue, Jun 08 2021, Maxime Devos wrote:
> 
> > Xinglu Chen schreef op di 08-06-2021 om 17:40 [+0200]:
> > > Using ‘call-with-output-file*’ instead of ‘call-with-output-file’ and ‘chmod’
> > > will prevent secrets from being leaked.  See
> > > <https://issues.guix.gnu.org/48872>;;.
> > 
> > This procedure LGTM (but I didn't test).
> > However,
> > 
> > > diff --git a/guix/build/utils.scm b/guix/build/utils.scm
> > > index 419c10195b..df960eee84 100644
> > > --- a/guix/build/utils.scm
> > > +++ b/guix/build/utils.scm
> > > @@ -5,6 +5,7 @@
> > 
> > Modifying (guix build utils) entails a world-rebuild, as
> > (guix build utils) is used by the build code of practically
> > every package. I would suggest placing it in (gnu build activation)
> > instead.
> 
> Oh, I didn’t think about that.  Moving it to (gnu build activation)
> seems like a good option.
> 
> Should I create a new “Activation” section in the manual, or should I
> keep it in the “Build Utilities” section?

The procedure isn't available during package building
(well, (gnu build activation) _could_ be imported in a package definition
using #:imported-modules & #:modules but it is not supposed to be used like
that), so ‘Build Utilities’ doesn't seem appropriate, thus I'd suggest creating
an "Activation" section in the manual.

Maybe under ‘Programming Reference’, or after ‘Defining Services’ in
the ‘System configuration’ chapter?

Greetings,
Maxime.

[signature.asc (application/pgp-signature, inline)]

Message #17 received at 48923 <at> debbugs.gnu.org (full text, mbox):

From: Xinglu Chen <public <at> yoctocell.xyz>
To: Maxime Devos <maximedevos <at> telenet.be>, 48923 <at> debbugs.gnu.org
Subject: Re: [bug#48923] [PATCH] build: utils: Add ‘call-with-outp
Date: Tue, 08 Jun 2021 20:04:57 +0200

[Message part 1 (text/plain, inline)]

On Tue, Jun 08 2021, Maxime Devos wrote:

>> > > diff --git a/guix/build/utils.scm b/guix/build/utils.scm
>> > > index 419c10195b..df960eee84 100644
>> > > --- a/guix/build/utils.scm
>> > > +++ b/guix/build/utils.scm
>> > > @@ -5,6 +5,7 @@
>> > 
>> > Modifying (guix build utils) entails a world-rebuild, as
>> > (guix build utils) is used by the build code of practically
>> > every package. I would suggest placing it in (gnu build activation)
>> > instead.
>> 
>> Oh, I didn’t think about that.  Moving it to (gnu build activation)
>> seems like a good option.
>> 
>> Should I create a new “Activation” section in the manual, or should I
>> keep it in the “Build Utilities” section?
>
> The procedure isn't available during package building
> (well, (gnu build activation) _could_ be imported in a package definition
> using #:imported-modules & #:modules but it is not supposed to be used like
> that), so ‘Build Utilities’ doesn't seem appropriate, thus I'd suggest creating
> an "Activation" section in the manual.
>
> Maybe under ‘Programming Reference’, or after ‘Defining Services’ in
> the ‘System configuration’ chapter?

OK, sounds good to me!

[signature.asc (application/pgp-signature, inline)]

Message #20 received at 48923 <at> debbugs.gnu.org (full text, mbox):

From: Xinglu Chen <public <at> yoctocell.xyz>
To: 48923 <at> debbugs.gnu.org
Cc: Maxime Devos <maximedevos <at> telenet.be>
Subject: [PATCH v2] activation: Add ‘call-with-output-file*’ procedure.
Date: Tue, 08 Jun 2021 20:30:13 +0200

Using ‘call-with-output-file*’ instead of ‘call-with-output-file’ and ‘chmod’
will prevent secrets from being leaked.  See
<https://issues.guix.gnu.org/48872>.

* guix/build/activation.scm (call-with-output-file*): New procedure.
* doc/guix.texi (Activation): New section; document ‘call-with-output-file*’.
---
Changes since v1:

* Moved ‘call-with-output-file*’ from (gnu build utils) to (gnu build
  activation).

* Added a “Activation” section in the manual to document the new
  procedure.

 doc/guix.texi            | 31 +++++++++++++++++++++++++++++++
 gnu/build/activation.scm | 13 ++++++++++++-
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 59b4ac11b4..643c7ff126 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -321,6 +321,7 @@ System Configuration
 * Invoking guix deploy::        Deploying a system configuration to a remote host.
 * Running Guix in a VM::        How to run Guix System in a virtual machine.
 * Defining Services::           Adding new service definitions.
+* Activation::                  Setting up system-wide files and directories.
 
 Services
 
@@ -13386,6 +13387,7 @@ instance to support new system services.
 * Invoking guix deploy::        Deploying a system configuration to a remote host.
 * Running Guix in a VM::        How to run Guix System in a virtual machine.
 * Defining Services::           Adding new service definitions.
+* Activation::                  Setting up system-wide files and directories.
 @end menu
 
 @node Using the Configuration System
@@ -34633,6 +34635,35 @@ system:
 This service represents PID <at> tie{}1.
 @end defvr
 
+@node Activation
+@section Activation
+
+@dfn{Activation} is the process that sets up system-wide files and
+directories so that an @code{operating-system} (@pxref{operating-system
+Reference}) configuration becomes active.  This will happen when
+invoking commands like @command{guix system reconfigure} or
+@command{guix system switch-generation}, but not when invoking
+@command{guix system build} (@pxref{Invoking guix system}).
+
+@deffn {Scheme Procedure} call-with-output-file* @var{file} @var{proc} @
+  [#:perms #o666]
+Open FILE for output, set the file permission bits to @var{perms}, and
+call @code{(PROC port)} with the resulting port.
+
+The advantage of using this procedure compared to something like this
+
+@lisp
+(call-with-output-file "FILE"
+  (lambda (port)
+    (display "top secret" port)))
+(chmod "FILE" #o400)
+@end lisp
+
+is that, with the latter, an unpriviliged user could open @var{file}
+before the permission was changed to @code{#o400}, thus making it
+possible to leak sensitive information.
+@end deffn
+
 
 @node Documentation
 @chapter Documentation
diff --git a/gnu/build/activation.scm b/gnu/build/activation.scm
index 2af1d44b5f..0054079cb6 100644
--- a/gnu/build/activation.scm
+++ b/gnu/build/activation.scm
@@ -6,6 +6,7 @@
 ;;; Copyright © 2018 Arun Isaac <arunisaac <at> systemreboot.net>
 ;;; Copyright © 2018, 2019 Ricardo Wurmus <rekado <at> elephly.net>
 ;;; Copyright © 2021 Maxime Devos <maximedevos <at> telenet.be>
+;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -34,6 +35,7 @@
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-60)
   #:export (activate-users+groups
             activate-user-home
             activate-etc
@@ -43,7 +45,8 @@
             activate-firmware
             activate-ptrace-attach
             activate-current-system
-            mkdir-p/perms))
+            mkdir-p/perms
+            call-with-output-file*))
 
 ;;; Commentary:
 ;;;
@@ -102,6 +105,14 @@ Warning: this is currently suspect to a TOCTTOU race!"
   (chown directory (passwd:uid owner) (passwd:gid owner))
   (chmod directory bits))
 
+;; Prevent secrets from leaking, see <https://issues.guix.gnu.org/48872>
+(define* (call-with-output-file* file proc #:key (perms #o666))
+  "FILE should be string containg the path to a file, PROC should be a procedure
+that accepts the port as an argument, and PERMS should be the permission bits
+of the file, the default is 666."
+  (let ((port (open file (bitwise-ior O_WRONLY O_CREAT) perms)))
+    (call-with-port port proc)))
+
 (define* (copy-account-skeletons home
                                  #:key
                                  (directory %skeleton-directory)

base-commit: 503c2039a280dd52a751a6852b4157fccd1b4195
-- 
2.32.0

Message #23 received at 48923 <at> debbugs.gnu.org (full text, mbox):

From: Xinglu Chen <public <at> yoctocell.xyz>
To: 48923 <at> debbugs.gnu.org
Cc: Maxime Devos <maximedevos <at> telenet.be>
Subject: Re: [bug#48923] [PATCH v2] activation: Add ‘call-with-output-file*’ procedure.
Date: Wed, 04 Aug 2021 10:25:30 +0200

[Message part 1 (text/plain, inline)]

Ping!  :)

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.