unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / Atom feed
* [bug#48923] [PATCH] build: utils: Add ‘call-with-outp
@ 2021-06-08 15:40 Xinglu Chen
  2021-06-08 16:04 ` Maxime Devos
  2021-06-08 18:30 ` [bug#48923] [PATCH v2] activation: Add ‘call-with-output-file*’ procedure Xinglu Chen
  0 siblings, 2 replies; 6+ messages in thread
From: Xinglu Chen @ 2021-06-08 15:40 UTC (permalink / raw)
  To: 48923; +Cc: Maxime Devos

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@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)))
+
 \f
 ;;;
 ;;; Search paths.

base-commit: 503c2039a280dd52a751a6852b4157fccd1b4195
-- 
2.32.0






^ permalink raw reply	[flat|nested] 6+ messages in thread

* [bug#48923] [PATCH] build: utils: Add ‘call-with-outp
  2021-06-08 15:40 [bug#48923] [PATCH] build: utils: Add ‘call-with-outp Xinglu Chen
@ 2021-06-08 16:04 ` Maxime Devos
  2021-06-08 17:36   ` Xinglu Chen
  2021-06-08 18:30 ` [bug#48923] [PATCH v2] activation: Add ‘call-with-output-file*’ procedure Xinglu Chen
  1 sibling, 1 reply; 6+ messages in thread
From: Maxime Devos @ 2021-06-08 16:04 UTC (permalink / raw)
  To: public, 48923

[-- Attachment #1: Type: text/plain, Size: 695 bytes --]

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.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [bug#48923] [PATCH] build: utils: Add ‘call-with-outp
  2021-06-08 16:04 ` Maxime Devos
@ 2021-06-08 17:36   ` Xinglu Chen
  2021-06-08 17:45     ` Maxime Devos
  0 siblings, 1 reply; 6+ messages in thread
From: Xinglu Chen @ 2021-06-08 17:36 UTC (permalink / raw)
  To: Maxime Devos, 48923

[-- Attachment #1: Type: text/plain, Size: 967 bytes --]

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?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [bug#48923] [PATCH] build: utils: Add ‘call-with-outp
  2021-06-08 17:36   ` Xinglu Chen
@ 2021-06-08 17:45     ` Maxime Devos
  2021-06-08 18:04       ` Xinglu Chen
  0 siblings, 1 reply; 6+ messages in thread
From: Maxime Devos @ 2021-06-08 17:45 UTC (permalink / raw)
  To: Xinglu Chen, 48923

[-- Attachment #1: Type: text/plain, Size: 1563 bytes --]

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.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [bug#48923] [PATCH] build: utils: Add ‘call-with-outp
  2021-06-08 17:45     ` Maxime Devos
@ 2021-06-08 18:04       ` Xinglu Chen
  0 siblings, 0 replies; 6+ messages in thread
From: Xinglu Chen @ 2021-06-08 18:04 UTC (permalink / raw)
  To: Maxime Devos, 48923

[-- Attachment #1: Type: text/plain, Size: 1213 bytes --]

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!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [bug#48923] [PATCH v2] activation: Add ‘call-with-output-file*’ procedure.
  2021-06-08 15:40 [bug#48923] [PATCH] build: utils: Add ‘call-with-outp Xinglu Chen
  2021-06-08 16:04 ` Maxime Devos
@ 2021-06-08 18:30 ` Xinglu Chen
  1 sibling, 0 replies; 6+ messages in thread
From: Xinglu Chen @ 2021-06-08 18:30 UTC (permalink / raw)
  To: 48923; +Cc: Maxime Devos

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






^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-06-08 18:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-08 15:40 [bug#48923] [PATCH] build: utils: Add ‘call-with-outp Xinglu Chen
2021-06-08 16:04 ` Maxime Devos
2021-06-08 17:36   ` Xinglu Chen
2021-06-08 17:45     ` Maxime Devos
2021-06-08 18:04       ` Xinglu Chen
2021-06-08 18:30 ` [bug#48923] [PATCH v2] activation: Add ‘call-with-output-file*’ procedure Xinglu Chen

unofficial mirror of guix-patches@gnu.org 

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://yhetil.org/guix-patches/1 guix-patches/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 guix-patches guix-patches/ https://yhetil.org/guix-patches \
		guix-patches@gnu.org
	public-inbox-index guix-patches

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://news.yhetil.org/yhetil.gnu.guix.patches


AGPL code for this site: git clone http://ou63pmih66umazou.onion/public-inbox.git