Package: guix-patches;
Reported by: Richard Sent <richard <at> freakingpenguin.com>
Date: Tue, 28 Jan 2025 21:14:02 UTC
Severity: normal
Tags: patch
Merged with 70314
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Richard Sent <richard <at> freakingpenguin.com> To: guix-patches <at> gnu.org, 70314 <at> debbugs.gnu.org Cc: Richard Sent <richard <at> freakingpenguin.com>, rprior <at> protonmail.com, ludo <at> gnu.org, zimon.toutoune <at> gmail.com Subject: [PATCH v2] guix: scripts: environment: add tls certs to networked containers. 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 <ludo <at> gnu.org>
;;; Copyright © 2018 Mike Gerwitz <mtg <at> gnu.org>
;;; Copyright © 2022, 2023 John Kehayias <john.kehayias <at> protonmail.com>
+;;; Copyright © 2025 Richard Sent <richard <at> freakingpenguin.com>
;;;
;;; 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 <at> gnu.org>
# Copyright © 2022, 2023 John Kehayias <john.kehayias <at> protonmail.com>
# Copyright © 2023 Ludovic Courtès <ludo <at> gnu.org>
+# Copyright © 2025 Richard Sent <richard <at> freakingpenguin.com>
#
# 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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.