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

OpenSubmitted by Xinglu Chen.
Details
2 participants
  • Maxime Devos
  • Xinglu Chen
Owner
unassigned
Severity
normal
X
X
Xinglu Chen wrote on 8 Jun 2021 17:40
(address . guix-patches@gnu.org)(name . Maxime Devos)(address . maximedevos@telenet.be)
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

* 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
M
M
Maxime Devos wrote on 8 Jun 2021 18:04
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-----


X
X
Xinglu Chen wrote on 8 Jun 2021 19:36
Re: [bug#48923] [PATCH] build: utils: Add ‘call -with-outp
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?
-----BEGIN PGP SIGNATURE-----

iQJJBAEBCAAzFiEEAVhh4yyK5+SEykIzrPUJmaL7XHkFAmC/qocVHHB1YmxpY0B5
b2N0b2NlbGwueHl6AAoJEKz1CZmi+1x5TC4P/j1np4RF2NXviS/KT3BtcK8dyZkX
qUM/pKqCQECwde6hdl1WqDBbMTGdIN0Lrm6YeAcS7OQ7cvzmXyTz7zR/PPd9C3sc
/Z9hRzWDa/7WvecgzRJpd/UDwN3bY3Z0OxDDVnKJd0fIGO4fahQgoTzyumEN/DbP
5QTKIVoSFt4jcxj6yDU97Juz2hx4C8+6puqe+d3taNebMgv+hPcg+4dvjuyLCRMd
vmBHZJR+vsNKraqr5jA8TZc/i8wHczM6UqxDRhzjwGZz+qGkwFq81BjmDrH/3OSK
MD5LM+QgTVuK9+WmkmB+pF0+256+Z6B8pq6NL7u3LT+jZIP5flQWZe4jM4zWzcR8
XK2FbWJQNzbXeXYUNLSVp5QoC3oNjil8f4kZg5RC3zqozdoM2KwPmbN9vdfvu97t
XOpVPwm0iF/2FfXbeHLUX2XyJ1gqlOElqfNf29MxOl95n/4s0QggPVmVgY5aUfDQ
M4FlNUuxZVf9N2CpJZSC1XKpd8dE0Tv/+p9wE97bVeQzGxYUsGkVpHYZ/LNH6Qqj
ENlmVCJMh17hReLj540emht/fZeEhqkBO56WRS0lDb1kNXNz+TQTlCzBaH0dx968
A8y7COq9+XZOUZZI5z/8b4bxyYZF+80p+Sca6wKSHz6175UowKq5GFfnSaxQvWud
TRhzOF7zmKV6jMW9
=OrPB
-----END PGP SIGNATURE-----

M
M
Maxime Devos wrote on 8 Jun 2021 19:45
Re: [bug#48923] [PATCH] build: utils: Add ‘call-with-outp
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-----


X
X
Xinglu Chen wrote on 8 Jun 2021 20:04
Re: [bug#48923] [PATCH] build: utils: Add ‘call -with-outp
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!
-----BEGIN PGP SIGNATURE-----

iQJJBAEBCAAzFiEEAVhh4yyK5+SEykIzrPUJmaL7XHkFAmC/sUkVHHB1YmxpY0B5
b2N0b2NlbGwueHl6AAoJEKz1CZmi+1x5EKoP/jHfDpk2/kjtkbb6LnheSR3cXudo
oyL2Z07XYZAUWZZ/s/Qx8xixf/o2mluedFofoOZqOHP4997+AUgRVi7FGOTDJU4h
qWfx8BOrAHCtuRQ9dNcJgLpudKxQM50hWyy0Y3EywKWGTMTFOvrKGbDHihRXw8ll
twT7pZuUVyuhnz4ohhUMzDGtLyqFN5/d61SZOuDOc7ir6PPwvfAkkp09KIhVnuUW
mDpyCyYvDnqeEbGNxMOiEyZEydi7/7Ry8jv4NR5YmvTRRgb/7c2txrd7dPKU/3fN
Qdr986F6qbuIBZp6YmvFZufmR6MDuorYRhreR7xke8sLwQ3K88dKjICxnD/FKSht
lSjB7Yif/KtIOT0Fh4Q8OELT+xHwMpqiUmUIF/BHuOKazZ+jFSVZAPQnQ94haag6
sO4OZknEgnbQZ5kWVhAsAUCkPQVgdUAY5idgDmPHnCv1nUDqjBJSPjF9Mpf0ANS9
qO4Abl/qs2ndyqzapx4SilIsOy1k2MerMWnsiSGAldHePaZF9ZPktjXd9I0CYFm1
8uQiIwnPh4dr6g0/hn7hp6QviR93FrZHiPrGXVNjNctmArh3/+MYCz4z9iXmXupp
8UvB7AKfDvbYBkmOwps7bdeTicAZL6e40In+QJOfVw/uRs2MeeXTvzhRrODXDlBF
z4cjnvNh0PeSLQeX
=DeU2
-----END PGP SIGNATURE-----

X
X
Xinglu Chen wrote on 8 Jun 2021 20:30
[PATCH v2] activation: Add ‘call-with-output-fi le*’ procedure.
(address . 48923@debbugs.gnu.org)(name . Maxime Devos)(address . maximedevos@telenet.be)
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

* 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
X
X
Xinglu Chen wrote on 4 Aug 2021 10:25
Re: [bug#48923] [PATCH v2] activation: Add ‘cal l-with-output-file*’ procedure.
(address . 48923@debbugs.gnu.org)(name . Maxime Devos)(address . maximedevos@telenet.be)
878s1h4nwl.fsf@yoctocell.xyz
Ping! :)
-----BEGIN PGP SIGNATURE-----

iQJJBAEBCAAzFiEEAVhh4yyK5+SEykIzrPUJmaL7XHkFAmEKTvoVHHB1YmxpY0B5
b2N0b2NlbGwueHl6AAoJEKz1CZmi+1x5TlgP/Rbng1BbATQhbogzK/INHNOQjtSg
bbKCa30rFJvltVmEmNsKNemXfDrS7gndzHl8nZ8QqW8I6PeGSCdg6gWNsvp0SuD7
8ufWuegDBBR/dGLVU7x7nKBfRk/JcXDMRjMepT4aGPzUFQiNzRHzcKzfARRxr+EV
i/If+LOazev18vB8C8v8abtJtujHJaF0KxOXpseqpGjuSp9ergvniBWaVvtX6zrw
+PSEA6jHvE+e8UihBzCo/2B/DUXvSG9hdnOZ7wKlfwsj51r6U9uemlhykTCHkcOQ
3K9n9PAc7sqVi18OehqOA+KI9EnVm8Qf2CZlLfIsn8ngLBzwwYdcLAIkG6fysnmy
FQURVIxwYYHB9oZLW4Mcrlo/MTO52y1rkgq1IfOumDJ8iJeBYj1ziulrsvO3/L5G
pxqK8eKG8XoiP+smgxcIPkmXFxjt1Vj/Bol8R3Gz94LZAoE494AxOl0ClV/yvCow
pC8x1Hc2CgOj9KyL8xEUzO8eNVgPzHEFLiRsr3nZQ/clEpjKsfWNn5uOL/fbo+/d
02BjAiifpSrEJpHYA4ovPP/vTr03wRZydcTvtr5pCl4amxDNF/mzfIZOf2dMiUqJ
I8GZFRp2+Thb3cqiQMNUWQjT71v8yyx1xyd0YZDVss0WD/aBXyZ1+dFUbWlVA1TD
n3RLQKeD84Lu7mv8
=1JjR
-----END PGP SIGNATURE-----

?