GNU bug report logs - #75917
[PATCH v2] guix: scripts: environment: add tls certs to networked containers.

Message received at submit <at> debbugs.gnu.org:

Received: (at submit) by debbugs.gnu.org; 28 Jan 2025 21:13:29 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Tue Jan 28 16:13:29 2025
Received: from localhost ([127.0.0.1]:38891 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1tcstk-0001Qi-GT
	for submit <at> debbugs.gnu.org; Tue, 28 Jan 2025 16:13:29 -0500
Received: from lists.gnu.org ([2001:470:142::17]:56018)
 by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.84_2) (envelope-from <richard@HIDDEN>)
 id 1tcsti-0001QP-7i
 for submit <at> debbugs.gnu.org; Tue, 28 Jan 2025 16:13:27 -0500
Received: from eggs.gnu.org ([2001:470:142:3::10])
 by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <richard@HIDDEN>)
 id 1tcstX-0002yN-T5
 for guix-patches@HIDDEN; Tue, 28 Jan 2025 16:13:17 -0500
Received: from mail-108-mta179.mxroute.com ([136.175.108.179])
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128)
 (Exim 4.90_1) (envelope-from <richard@HIDDEN>)
 id 1tcstV-0003lw-MU
 for guix-patches@HIDDEN; Tue, 28 Jan 2025 16:13:15 -0500
Received: from filter006.mxroute.com ([136.175.111.3] filter006.mxroute.com)
 (Authenticated sender: mN4UYu2MZsgR)
 by mail-108-mta179.mxroute.com (ZoneMTA) with ESMTPSA id
 194aec2c521000310e.003 for <guix-patches@HIDDEN>
 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384);
 Tue, 28 Jan 2025 21:13:07 +0000
X-Zone-Loop: d9bbbcf7fbc3a3b31eefa8c6d6e973d64e9be9bae1d8
X-Originating-IP: [136.175.111.3]
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;
 d=freakingpenguin.com; s=x; h=Content-Transfer-Encoding:Content-Type:
 MIME-Version:Message-ID:Date:Subject:Cc:To:From:Sender:Reply-To:Content-ID:
 Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc
 :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe:
 List-Subscribe:List-Post:List-Owner:List-Archive;
 bh=EVnT23MAOto7dGaHWHfL/EEKWFMc0M5vmna0+fAIi90=; b=AHZ02nUfrrzLYjKvBtXvuTMHlY
 6XNH+/2D3MtT7+pjNMg8B2PY8oZIiGgdEDn49oB7oFIt5HCAh/Owz52BrG3WRN3EY7cLx7WujiAHu
 4KY3W9fIUk5/7+J4bmI6ZCZVckStCCVExHgstM+uMQzxVSBW2tbDMMBfPnloRY8sOQY8cwMIChgEF
 TCowDn+QdJ864LxVAinE7Va4/1MjGM8fpjoMw7e4v5zTe70Rb9v7ueqkFSRNj+roAGUYSwMHUTEIH
 fq9HpSsR0A+v+gDU7vXsxLQjH+rrh91ugYYVti2zoO1JhZHk08bkRSaBX2X5CF5FRYZS4rYZwKohA
 3uGM7xSg==;
From: Richard Sent <richard@HIDDEN>
To: guix-patches@HIDDEN,
	70314 <at> debbugs.gnu.org
Subject: [PATCH v2] guix: scripts: environment: add tls certs to networked
 containers.
Date: Tue, 28 Jan 2025 16:11:28 -0500
Message-ID: <4e1a986e224083876c8386e6efe45512251dd3af.1738098141.git.richard@HIDDEN>
MIME-Version: 1.0
X-Debbugs-Cc: Christopher Baines <guix@HIDDEN>, Josselin Poiret <dev@HIDDEN>, Ludovic Courtès <ludo@HIDDEN>, Mathieu Othacehe <othacehe@HIDDEN>, Maxim Cournoyer <maxim.cournoyer@HIDDEN>, Simon Tournier <zimon.toutoune@HIDDEN>, Tobias Geerinckx-Rice <me@HIDDEN>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
X-Authenticated-Id: richard@HIDDEN
Received-SPF: pass client-ip=136.175.108.179;
 envelope-from=richard@HIDDEN; helo=mail-108-mta179.mxroute.com
X-Spam_score_int: -20
X-Spam_score: -2.1
X-Spam_bar: --
X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,
 DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1,
 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001,
 SPF_HELO_NONE=0.001, SPF_PASS=-0.001,
 T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no
X-Spam_action: no action
X-Spam-Score: 0.9 (/)
X-Debbugs-Envelope-To: submit
Cc: Richard Sent <richard@HIDDEN>, rprior@HIDDEN,
 ludo@HIDDEN, zimon.toutoune@HIDDEN
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -0.1 (/)

Add the --no-tls flag. By default when starting a container with -N, add the
nss-certs package to the profile and set the SSL_CERT_DIR and SSL_CERT_FILE
environment variables. When --no-tls is passed, default to the old behavior.

* guix/scripts/environment.scm (%default-tls-certs): New function.
(show-environment-options-help): Add help for --no-tls.
(%options): Add --no-tls option.
(options/resolve-packages): Add %default-tls-certs to profile when network is
true and no-tls is false.
(launch-environment/container): Add set-tls? argument and set
SSL_CERT_DIR/FILE if #t.
(guix-environment*): Sanity check no-tls? and pass the negated version to
launch-environment/container.
* doc/guix.texi (Invoking guix shell): Document it.
(Invoking guix environment): Ditto.
* tests/guix-environment-container.sh: Add tests for behavior with and without
no-tls flag.

Change-Id: I04735024df025740cb7a0e28f39a3bb5c7fcf895
---
Hi all. Been a while but I figured I'd take another crack at this.

> Ludo:
> This would force all the package modules to be loaded upfront.  Instead you
> should arrange to not refer to ‘nss-certs’ until it’s needed.

Understood, %default-tls-certs is thunked in V2. To my understanding this
should achieve what we want.

> Ludo:
> Internally, I would reverse the logic to have ‘tls?’ instead (as a rule
> of thumb, I always avoid negating Booleans in code).

I choose no-tls? to be consistent with the no-cwd? option,. V2 now uses the
set-tls? option in launch-environment/container and no-tls? everywhere else,
which I think fits better because outside l-e/c, no-tls? is more of a flag for
if the --no-tls option was passed than a control boolean.

> Ludo:
> Can we delay changes to the manifest until after all options have been
> parsed, so we know whether ‘-C’ has been passed?

Possibly, but it would be inconsistent with how nesting? works at present,
which also requires -C. I believe emulate-fhs? also adds packages to the
profile immediately, see parse-args in guix/scripts/shell.scm, which AFAICT
splices a '-e (@@ (gnu packages base) glibc-for-fhs)' in the options.

One way this could be resolved is by creating a internal manifest, then
concatenating it with manifest-from-opts. i.e. have a user manifest containing
explicitly provided packages and an internal manifest containing
glibc-for-fhs, nss-certs and guix depending on emulate-fhs?, no-tls?, and
nesting?. That's probably outside the scope of this patch.

> Ludo:
> Please make sure tests/guix-shell*.sh and tests/guix-environment*.sh
> pass.

For the low low price of free, not only do you get passing tests but now you
get more tests! What a steal.

> Simon:
> I would prefer to not have any short option at all.

Agreed and changed.

> Simon:
> Why not a warning instead of leaving with an error?

I elected to go with an error to be consistent with the sanity checking around
container?. In my opinion, warnings are best for when a user is doing
something technically valid but likely unintended, errors are for the
"technically makes no sense".

 doc/guix.texi                       |  8 +++++++
 guix/scripts/environment.scm        | 35 +++++++++++++++++++++++++++--
 tests/guix-environment-container.sh | 11 +++++++++
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index b1b6d98e74..d291c15759 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -6289,6 +6289,10 @@ Invoking guix shell
 Containers created without this flag only have access to the loopback
 device.
 
+@item --no-tls
+For containers that share the network namespace, disable automatically
+adding TLS/SSL certificates.
+
 @item --link-profile
 @itemx -P
 For containers, link the environment profile to @file{~/.guix-profile}
@@ -6786,6 +6790,10 @@ Invoking guix environment
 Containers created without this flag only have access to the loopback
 device.
 
+@item --no-tls
+For containers that share the network namespace, disable automatically
+adding TLS/SSL certificates.
+
 @item --link-profile
 @itemx -P
 For containers, link the environment profile to @file{~/.guix-profile}
diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index 648a497743..174d446635 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2015-2024 Ludovic Courtès <ludo@HIDDEN>
 ;;; Copyright © 2018 Mike Gerwitz <mtg@HIDDEN>
 ;;; Copyright © 2022, 2023 John Kehayias <john.kehayias@HIDDEN>
+;;; Copyright © 2025 Richard Sent <richard@HIDDEN>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -50,6 +51,7 @@ (define-module (guix scripts environment)
   #:use-module (gnu system file-systems)
   #:autoload   (gnu packages) (specification->package+output)
   #:autoload   (gnu packages bash) (bash)
+  #:autoload   (gnu packages certs) (nss-certs)
   #:autoload   (gnu packages bootstrap) (bootstrap-executable %bootstrap-guile)
   #:autoload   (gnu packages package-management) (guix)
   #:use-module (ice-9 match)
@@ -72,6 +74,10 @@ (define-module (guix scripts environment)
 (define %default-shell
   (or (getenv "SHELL") "/bin/sh"))
 
+(define (%default-tls-certs)
+  ;; Thunk to defer loading (gnu packages certs)
+  (list nss-certs))
+
 (define* (show-search-paths profile manifest #:key pure?)
   "Display the search paths of MANIFEST applied to PROFILE.  When PURE? is #t,
 do not augment existing environment variables with additional search paths."
@@ -108,6 +114,9 @@ (define (show-environment-options-help)
   -C, --container        run command within an isolated container"))
   (display (G_ "
   -N, --network          allow containers to access the network"))
+  (display (G_ "
+      --no-tls            do not add SSL/TLS certificates or set environment
+                          variables for a networked container"))
   (display (G_ "
   -P, --link-profile     link environment profile to ~/.guix-profile within
                          an isolated container"))
@@ -244,6 +253,9 @@ (define %options
          (option '(#\N "network") #f #f
                  (lambda (opt name arg result)
                    (alist-cons 'network? #t result)))
+         (option '("no-tls") #f #f
+                 (lambda (opt name arg result)
+                   (alist-cons 'no-tls? #t result)))
          (option '(#\W "nesting") #f #f
                  (lambda (opt name arg result)
                    (alist-cons 'nesting? #t result)))
@@ -359,6 +371,11 @@ (define (options/resolve-packages store opts)
                      (packages->outputs (load* file module) mode)))
                   (('manifest . file)
                    (manifest-entries (load-manifest file)))
+                  (('network? . #t)
+                   (if (assoc-ref opts 'no-tls?)
+                       '()
+                       (manifest-entries
+                        (packages->manifest (%default-tls-certs)))))
                   (('nesting? . #t)
                    (if (assoc-ref opts 'profile)
                        '()
@@ -732,7 +749,8 @@ (define* (launch-environment/fork command profile manifest
 
 (define* (launch-environment/container #:key command bash user user-mappings
                                        profile manifest link-profile? network?
-                                       map-cwd? emulate-fhs? nesting?
+                                       set-tls? map-cwd? emulate-fhs?
+                                       nesting?
                                        (setup-hook #f)
                                        (symlinks '()) (white-list '()))
   "Run COMMAND within a container that features the software in PROFILE.
@@ -936,6 +954,11 @@ (define* (launch-environment/container #:key command bash user user-mappings
               ;; Allow local AF_INET communications.
               (set-network-interface-up "lo"))
 
+            (when set-tls?
+              (setenv "SSL_CERT_DIR" (string-append profile "/etc/ssl/certs"))
+              (setenv "SSL_CERT_FILE" (string-append (getenv "SSL_CERT_DIR")
+                                                     "/ca-certificates.crt")))
+
             ;; For convenience, start in the user's current working
             ;; directory or, if unmapped, the home directory.
             (chdir (if map-cwd?
@@ -1085,6 +1108,7 @@ (define (guix-environment* opts)
          (link-prof?   (assoc-ref opts 'link-profile?))
          (symlinks     (assoc-ref opts 'symlinks))
          (network?     (assoc-ref opts 'network?))
+         (no-tls?      (assoc-ref opts 'no-tls?))
          (no-cwd?      (assoc-ref opts 'no-cwd?))
          (emulate-fhs? (assoc-ref opts 'emulate-fhs?))
          (nesting?     (assoc-ref opts 'nesting?))
@@ -1138,7 +1162,13 @@ (define (guix-environment* opts)
       (when nesting?
         (leave (G_ "'--nesting' cannot be used without '--container'~%")))
       (when (pair? symlinks)
-        (leave (G_ "'--symlink' cannot be used without '--container'~%"))))
+        (leave (G_ "'--symlink' cannot be used without '--container'~%")))
+      (when network?
+        (leave (G_ "'--network cannot be used without '--container'~%"))))
+
+    (when (and (not network?)
+               no-tls?)
+      (leave (G_ "'--no-tls' cannot be used without '--networking'~%")))
 
     (with-status-verbosity (assoc-ref opts 'verbosity)
       (with-store/maybe store
@@ -1217,6 +1247,7 @@ (define (guix-environment* opts)
                                                   #:white-list white-list
                                                   #:link-profile? link-prof?
                                                   #:network? network?
+                                                  #:set-tls? (not no-tls?)
                                                   #:map-cwd? (not no-cwd?)
                                                   #:emulate-fhs? emulate-fhs?
                                                   #:nesting? nesting?
diff --git a/tests/guix-environment-container.sh b/tests/guix-environment-container.sh
index 09704f751c..7ffc7f8c9f 100644
--- a/tests/guix-environment-container.sh
+++ b/tests/guix-environment-container.sh
@@ -2,6 +2,7 @@
 # Copyright © 2015 David Thompson <davet@HIDDEN>
 # Copyright © 2022, 2023 John Kehayias <john.kehayias@HIDDEN>
 # Copyright © 2023 Ludovic Courtès <ludo@HIDDEN>
+# Copyright © 2025 Richard Sent <richard@HIDDEN>
 #
 # This file is part of GNU Guix.
 #
@@ -272,3 +273,13 @@ guix shell -C -D guix -- "$env" guix build hello -d && false # cannot work
 hello_drv="$(guix build hello -d)"
 hello_drv_nested="$(cd "$(dirname env)" && guix shell --bootstrap -E GUIX_BUILD_OPTIONS -CW -D guix -- "$env" guix build hello -d)"
 test "$hello_drv" = "$hello_drv_nested"
+
+# Test if SSL_CERT_{DIR,FILE} are set and readable in the container.
+#
+# -f does cover the case of a symlink to a file inaccessible within the
+# -container.
+guix shell -CN -- /bin/sh -c 'test -d $SSL_CERT_DIR'
+guix shell -CN -- /bin/sh -c 'test -f $SSL_CERT_FILE'
+# Confirm --no-tls causes SSL_CERT_{DIR,FILE} to be unset.
+guix shell -CN --no-tls -- /bin/sh -c 'test -z $SSL_CERT_DIR'
+guix shell -CN --no-tls -- /bin/sh -c 'test -z $SSL_CERT_FILE'

base-commit: 97fb1887ad10000c067168176c504274e29e4430
-- 
2.47.1