[PATCH] build: utils: Add ‘call-with-outp

Xinglu Chen wrote on 8 Jun 2021 17:40

Recipients:(address . guix-patches@gnu.org)(name . Maxime Devos)(address . maximedevos@telenet.be)

Message-ID:23ac66d29119c5395fee0e993ea0fe811beefd91.1623166798.git.public@yoctocell.xyz

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(+)

Toggle diff (69 lines)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@netris.org>
 ;;; Copyright © 2018 Arun Isaac <arunisaac@systemreboot.net>
 ;;; Copyright © 2018, 2019 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2021 Xinglu Chen <public@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

Maxime Devos wrote on 8 Jun 2021 18:04

Recipients:

Message-ID:a0972b8b687b465ceb341454a6a01a16bf4a444a.camel@telenet.be

Xinglu Chen schreef op di 08-06-2021 om 17:40 [+0200]:

Toggle quote (4 lines)

> 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,

Toggle quote (6 lines)

> 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.

-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYL+VGBccbWF4aW1lZGV2

b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7sn3AP9Q6pKw5YWetMIVgjeWgCN42uaU

scRe8qFqFsUznGaPIAD9HpI3QNX1R1oQDIRQD0GWztPZFSDdyo2Dv2phcVrsLAM=

=2xze

-----END PGP SIGNATURE-----

Xinglu Chen wrote on 8 Jun 2021 19:36

Re: [bug#48923] [PATCH] build: utils: Add ‘call -with-outp

Recipients:

Message-ID:871r9cgsxk.fsf@yoctocell.xyz

On Tue, Jun 08 2021, Maxime Devos wrote:

Toggle quote (19 lines)> 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?

Maxime Devos wrote on 8 Jun 2021 19:45

Re: [bug#48923] [PATCH] build: utils: Add ‘call-with-outp

Recipients:

Message-ID:38b69976891db7870992091d9eb3aa7aeb20e471.camel@telenet.be

Xinglu Chen schreef op di 08-06-2021 om 19:36 [+0200]:

Toggle quote (27 lines)> 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.

-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYL+szRccbWF4aW1lZGV2

b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7pQ5AQCEOJH3eY571Igmx7W/uXxmVMYE

SahmUcwWkpIyGdcXDgEAhwmsk2gyHJ3JQItn0atQA8r2Mq/zslgVvFp6L1q4GAI=

=AWlF

-----END PGP SIGNATURE-----

Xinglu Chen wrote on 8 Jun 2021 20:04

Re: [bug#48923] [PATCH] build: utils: Add ‘call -with-outp

Recipients:

Message-ID:87wnr4fd12.fsf@yoctocell.xyz

On Tue, Jun 08 2021, Maxime Devos wrote:

Toggle quote (26 lines)>> > > 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!

Xinglu Chen wrote on 8 Jun 2021 20:30

[PATCH v2] activation: Add ‘call-with-output-fi le*’ procedure.

Recipients:(address . 48923@debbugs.gnu.org)(name . Maxime Devos)(address . maximedevos@telenet.be)

Message-ID:7500e1ba2d55d397d4c105ca189746f78de02d35.1623176839.git.public@yoctocell.xyz

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(-)

Toggle diff (105 lines)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@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@systemreboot.net>
 ;;; Copyright © 2018, 2019 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2021 Maxime Devos <maximedevos@telenet.be>
+;;; Copyright © 2021 Xinglu Chen <public@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

Xinglu Chen wrote on 4 Aug 2021 10:25

Re: [bug#48923] [PATCH v2] activation: Add ‘cal l-with-output-file*’ procedure.

Recipients:(address . 48923@debbugs.gnu.org)(name . Maxime Devos)(address . maximedevos@telenet.be)

Message-ID:878s1h4nwl.fsf@yoctocell.xyz

Ping! :)

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date