diff mbox series

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

Message ID 23ac66d29119c5395fee0e993ea0fe811beefd91.1623166798.git.public@yoctocell.xyz
State New
Headers show
Series [bug#48923] build: utils: Add ‘call-with-outp | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Xinglu Chen June 8, 2021, 3:40 p.m. UTC
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(+)


base-commit: 503c2039a280dd52a751a6852b4157fccd1b4195

Comments

Maxime Devos June 8, 2021, 4:04 p.m. UTC | #1
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.
Xinglu Chen June 8, 2021, 5:36 p.m. UTC | #2
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?
Maxime Devos June 8, 2021, 5:45 p.m. UTC | #3
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.
Xinglu Chen June 8, 2021, 6:04 p.m. UTC | #4
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!
diff mbox series

Patch

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.