Subject: [PATCH v2] guix: scripts: environment: add tls certs to networked containers.

From: Richard Sent <[email-masked]>
Date: Tue, 28 Jan 2025 16:11:28 -0500

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 <[email-masked]>
 ;;; Copyright © 2018 Mike Gerwitz <[email-masked]>
 ;;; Copyright © 2022, 2023 John Kehayias <[email-masked]>
+;;; Copyright © 2025 Richard Sent <[email-masked]>
 ;;;
 ;;; 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 <[email-masked]>
 # Copyright © 2022, 2023 John Kehayias <[email-masked]>
 # Copyright © 2023 Ludovic Courtès <[email-masked]>
+# Copyright © 2025 Richard Sent <[email-masked]>
 #
 # 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