unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#70933] [PATCH] system: Do not add "--disable-chroot" to containers.
@ 2024-05-14 11:50 Andreas Enge
  2024-05-31 12:01 ` Ludovic Courtès
  2024-07-05 14:24 ` [bug#70933] Patch Andreas Enge
  0 siblings, 2 replies; 6+ messages in thread
From: Andreas Enge @ 2024-05-14 11:50 UTC (permalink / raw)
  To: 70933; +Cc: Andreas Enge

The rationale for these lines is that they enable non-privileged docker
containers. But I would like to create a privileged container with
chroot (in an openshift environment, where I suppose this environment
does additional encapsulation to enforce security), which these lines
prevent.

Users can still add the option. Alternatively, we could add an additional
field "chroot? (default: #t)" to guix-configuration.

Andreas



* gnu/system/linux-container.scm (containerized-operating-system): Do not
add "--disable-chroot".

Change-Id: I1eff9aa0d02d6e53bd4e42f3aeb07d0ab42616a8
---
 gnu/system/linux-container.scm | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/gnu/system/linux-container.scm b/gnu/system/linux-container.scm
index c780b68fba..2fc54a8121 100644
--- a/gnu/system/linux-container.scm
+++ b/gnu/system/linux-container.scm
@@ -159,17 +159,6 @@ (define* (containerized-operating-system os mappings
                                              (nscd-configuration
                                               (inherit (service-value s))
                                               (caches %nscd-container-caches))))
-                                   ((eq? guix-service-type (service-kind s))
-                                    ;; Pass '--disable-chroot' so that
-                                    ;; guix-daemon can build thing even in
-                                    ;; Docker without '--privileged'.
-                                    (service guix-service-type
-                                             (guix-configuration
-                                              (inherit (service-value s))
-                                              (extra-options
-                                               (cons "--disable-chroot"
-                                                     (guix-configuration-extra-options
-                                                      (service-value s)))))))
                                    (else s)))
                            (operating-system-user-services os))))
       (file-systems (append (map mapping->fs

base-commit: a682ddd70846d488cfbd82d65e8566ec6739813c
-- 
2.41.0





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

* [bug#70933] [PATCH] system: Do not add "--disable-chroot" to containers.
  2024-05-14 11:50 [bug#70933] [PATCH] system: Do not add "--disable-chroot" to containers Andreas Enge
@ 2024-05-31 12:01 ` Ludovic Courtès
  2024-05-31 14:26   ` Andreas Enge
  2024-07-05 14:24 ` [bug#70933] Patch Andreas Enge
  1 sibling, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2024-05-31 12:01 UTC (permalink / raw)
  To: Andreas Enge; +Cc: 70933

Hi,

Andreas Enge <andreas@enge.fr> skribis:

> The rationale for these lines is that they enable non-privileged docker
> containers. But I would like to create a privileged container with
> chroot (in an openshift environment, where I suppose this environment
> does additional encapsulation to enforce security), which these lines
> prevent.
>
> Users can still add the option. Alternatively, we could add an additional
> field "chroot? (default: #t)" to guix-configuration.

[...]

> -                                   ((eq? guix-service-type (service-kind s))
> -                                    ;; Pass '--disable-chroot' so that
> -                                    ;; guix-daemon can build thing even in
> -                                    ;; Docker without '--privileged'.

This is tricky, I’m not sure how to provide defaults that works in most
common setups while still allowing the use of privileged Docker
containers as in your case.

I think the current default is good because it’s the common case, but I
agree that we need to find a way to override it.

Thoughts?

Ludo’.




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

* [bug#70933] [PATCH] system: Do not add "--disable-chroot" to containers.
  2024-05-31 12:01 ` Ludovic Courtès
@ 2024-05-31 14:26   ` Andreas Enge
  2024-06-25 15:30     ` Ludovic Courtès
  2024-09-19  8:01     ` Andreas Enge
  0 siblings, 2 replies; 6+ messages in thread
From: Andreas Enge @ 2024-05-31 14:26 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 70933

Am Fri, May 31, 2024 at 02:01:36PM +0200 schrieb Ludovic Courtès:
> Andreas Enge <andreas@enge.fr> skribis:
> > The rationale for these lines is that they enable non-privileged docker
> > containers. But I would like to create a privileged container with
> > chroot (in an openshift environment, where I suppose this environment
> > does additional encapsulation to enforce security), which these lines
> > prevent.
> > Users can still add the option. Alternatively, we could add an additional
> > field "chroot? (default: #t)" to guix-configuration.
> This is tricky, I’m not sure how to provide defaults that works in most
> common setups while still allowing the use of privileged Docker
> containers as in your case.

The problem with a default is that apparently, for containers we want #f,
for real machines we want #t as the default; and then it should be
overridable. The only solution I see is to use a ternary value,
allowing chroot? to be #f, #t or 'default, with the last one, you guess it,
being the default. It would be replaced by #f or #t depending on whether
we are in a container or not.

I had considered it when suggesting the patch, but found it a bit too much
shepherding; I still think that "chroot? (default: #t)" would be enough.

Andreas





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

* [bug#70933] [PATCH] system: Do not add "--disable-chroot" to containers.
  2024-05-31 14:26   ` Andreas Enge
@ 2024-06-25 15:30     ` Ludovic Courtès
  2024-09-19  8:01     ` Andreas Enge
  1 sibling, 0 replies; 6+ messages in thread
From: Ludovic Courtès @ 2024-06-25 15:30 UTC (permalink / raw)
  To: Andreas Enge; +Cc: 70933

Hi!

Andreas Enge <andreas@enge.fr> skribis:

> Am Fri, May 31, 2024 at 02:01:36PM +0200 schrieb Ludovic Courtès:
>> Andreas Enge <andreas@enge.fr> skribis:
>> > The rationale for these lines is that they enable non-privileged docker
>> > containers. But I would like to create a privileged container with
>> > chroot (in an openshift environment, where I suppose this environment
>> > does additional encapsulation to enforce security), which these lines
>> > prevent.
>> > Users can still add the option. Alternatively, we could add an additional
>> > field "chroot? (default: #t)" to guix-configuration.
>> This is tricky, I’m not sure how to provide defaults that works in most
>> common setups while still allowing the use of privileged Docker
>> containers as in your case.
>
> The problem with a default is that apparently, for containers we want #f,
> for real machines we want #t as the default; and then it should be
> overridable. The only solution I see is to use a ternary value,
> allowing chroot? to be #f, #t or 'default, with the last one, you guess it,
> being the default. It would be replaced by #f or #t depending on whether
> we are in a container or not.

Making it a ternary value sounds like a good idea, indeed.  #t, #f, and
'default sounds like a good choice to me.

Thanks!

Ludo’.




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

* [bug#70933] Patch
  2024-05-14 11:50 [bug#70933] [PATCH] system: Do not add "--disable-chroot" to containers Andreas Enge
  2024-05-31 12:01 ` Ludovic Courtès
@ 2024-07-05 14:24 ` Andreas Enge
  1 sibling, 0 replies; 6+ messages in thread
From: Andreas Enge @ 2024-07-05 14:24 UTC (permalink / raw)
  To: 70933

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

Here is a suggestion for a patch implementing the chroot? parameter.
I have tested it by reconfiguring a real machine and still need to
test it in containers.

Andreas


[-- Attachment #2: 0001-gnu-guix-configuration-Add-a-chroot-parameter.patch --]
[-- Type: text/plain, Size: 7551 bytes --]

From 8629decf975576447c8662355a35ec0f20e892cf Mon Sep 17 00:00:00 2001
Message-ID: <8629decf975576447c8662355a35ec0f20e892cf.1720189422.git.andreas@enge.fr>
From: Andreas Enge <andreas@enge.fr>
Date: Fri, 5 Jul 2024 15:47:13 +0200
Subject: [PATCH] gnu: guix-configuration: Add a chroot? parameter.

The parameter should take the values #t, #f or 'default.
In a container environment, 'default amounts to #f, otherwise it
amounts to #t.

* gnu/services/base.scm (guix-configuration)<chroot?>: New field.
(guix-shepherd-service): If chroot? is #f, add "--disable-chroot".
If it is #t or 'default, do nothing.
* gnu/system/linux-container.scm (containerized-operating-system):
If chroot? is 'default, replace it by #f.
* doc/guix.texi: Document the parameter.

Change-Id: I8b9c3f46ad8650fa6ed4acee947b4ae5d002d03d
---
 doc/guix.texi                  |  7 ++++++
 gnu/services/base.scm          |  9 ++++++-
 gnu/system/linux-container.scm | 43 ++++++++++++++++++----------------
 3 files changed, 38 insertions(+), 21 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 8bfb342253..92db7ddcf5 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19609,6 +19609,13 @@ Base Services
 @item @code{build-accounts} (default: @code{10})
 Number of build user accounts to create.
 
+@item @code{chroot?} (default: @code{'default})
+The value should be one of @code{#t} or @code{#f}, in which
+case chroot is enabled or disabled, respectively;
+or it should be @code{'default}, which amounts to @code{#f} in
+Docker containers (so that they can be run in non-privileged mode)
+or @code{#t} otherwise.
+
 @item @code{authorize-key?} (default: @code{#t})
 @cindex substitutes, authorization thereof
 Whether to authorize the substitute keys listed in
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 4b5b103cc3..a3e23035ca 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -21,6 +21,7 @@
 ;;; Copyright © 2022 Justin Veilleux <terramorpha@cock.li>
 ;;; Copyright © 2022 ( <paren@disroot.org>
 ;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
+;;; Copyright © 2024 Andreas Enge <andreas@enge.fr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -213,6 +214,7 @@ (define-module (gnu services base)
             guix-configuration-build-group
             guix-configuration-build-accounts
             guix-configuration-build-machines
+            guix-configuration-chroot?
             guix-configuration-authorize-key?
             guix-configuration-authorized-keys
             guix-configuration-use-substitutes?
@@ -1848,6 +1850,8 @@ (define-record-type* <guix-configuration>
                     (default "guixbuild"))
   (build-accounts   guix-configuration-build-accounts ;integer
                     (default 10))
+  (chroot?          guix-configuration-chroot? ;Boolean | 'default
+                    (default 'default))
   (authorize-key?   guix-configuration-authorize-key? ;Boolean
                     (default #t))
   (authorized-keys  guix-configuration-authorized-keys ;list of gexps
@@ -1942,7 +1946,7 @@ (define (guix-shepherd-service config)
           glibc-utf8-locales)))
 
   (match-record config <guix-configuration>
-    (guix build-group build-accounts authorize-key? authorized-keys
+    (guix build-group build-accounts chroot? authorize-key? authorized-keys
           use-substitutes? substitute-urls max-silent-time timeout
           log-compression discover? extra-options log-file
           http-proxy tmpdir chroot-directories environment)
@@ -1989,6 +1993,9 @@ (define (guix-shepherd-service config)
                           "--substitute-urls" #$(string-join substitute-urls)
                           #$@extra-options
 
+                          #$@(if chroot?
+                                 '()
+                                 '("--disable-chroot"))
                           ;; Add CHROOT-DIRECTORIES and all their dependencies
                           ;; (if these are store items) to the chroot.
                           (append-map
diff --git a/gnu/system/linux-container.scm b/gnu/system/linux-container.scm
index c780b68fba..c1705f491c 100644
--- a/gnu/system/linux-container.scm
+++ b/gnu/system/linux-container.scm
@@ -7,6 +7,7 @@
 ;;; Copyright © 2022 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2023 Pierre Langlois <pierre.langlois@gmx.com>
 ;;; Copyright © 2024 Leo Nikkilä <hello@lnikki.la>
+;;; Copyright © 2024 Andreas Enge <andreas@enge.fr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -151,26 +152,28 @@ (define* (containerized-operating-system os mappings
       (swap-devices '()) ; disable swap
       (services
        (append services-to-add
-               (filter-map (lambda (s)
-                             (cond ((memq (service-kind s) services-to-drop)
-                                    #f)
-                                   ((eq? nscd-service-type (service-kind s))
-                                    (service nscd-service-type
-                                             (nscd-configuration
-                                              (inherit (service-value s))
-                                              (caches %nscd-container-caches))))
-                                   ((eq? guix-service-type (service-kind s))
-                                    ;; Pass '--disable-chroot' so that
-                                    ;; guix-daemon can build thing even in
-                                    ;; Docker without '--privileged'.
-                                    (service guix-service-type
-                                             (guix-configuration
-                                              (inherit (service-value s))
-                                              (extra-options
-                                               (cons "--disable-chroot"
-                                                     (guix-configuration-extra-options
-                                                      (service-value s)))))))
-                                   (else s)))
+               (filter-map
+                 (lambda (s)
+                   (let ((kind (service-kind s))
+                         (value (service-value s)))
+                        (cond ((memq kind services-to-drop)
+                               #f)
+                              ((eq? nscd-service-type kind)
+                               (service nscd-service-type
+                                        (nscd-configuration
+                                         (inherit value)
+                                         (caches %nscd-container-caches))))
+                              ((and (eq? guix-service-type kind)
+                                    (eq? (guix-configuration-chroot? value)
+                                         'default))
+                               ;; If chroot? is 'default, it should become #f
+                               ;; so that guix-daemon can build things even in
+                               ;; Docker without '--privileged'.
+                               (service guix-service-type
+                                        (guix-configuration
+                                         (inherit value)
+                                         (chroot? #f))))
+                              (else s))))
                            (operating-system-user-services os))))
       (file-systems (append (map mapping->fs
                                  (if shared-network?

base-commit: b91685abdecdb42c0ac2e30e5cb5032b11b5d269
-- 
2.45.2


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

* [bug#70933] [PATCH] system: Do not add "--disable-chroot" to containers.
  2024-05-31 14:26   ` Andreas Enge
  2024-06-25 15:30     ` Ludovic Courtès
@ 2024-09-19  8:01     ` Andreas Enge
  1 sibling, 0 replies; 6+ messages in thread
From: Andreas Enge @ 2024-09-19  8:01 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 70933

Am Fri, May 31, 2024 at 04:26:58PM +0200 schrieb Andreas Enge:
> The problem with a default is that apparently, for containers we want #f,
> for real machines we want #t as the default; and then it should be
> overridable. The only solution I see is to use a ternary value,
> allowing chroot? to be #f, #t or 'default, with the last one, you guess it,
> being the default. It would be replaced by #f or #t depending on whether
> we are in a container or not.

The patch works in our kubernetes environment (where we create docker
containers with 'chroot? #t'). If there is agreement, I am happy to adapt
the documentation and to push.

Andreas





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

end of thread, other threads:[~2024-09-19  8:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-14 11:50 [bug#70933] [PATCH] system: Do not add "--disable-chroot" to containers Andreas Enge
2024-05-31 12:01 ` Ludovic Courtès
2024-05-31 14:26   ` Andreas Enge
2024-06-25 15:30     ` Ludovic Courtès
2024-09-19  8:01     ` Andreas Enge
2024-07-05 14:24 ` [bug#70933] Patch Andreas Enge

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).