[bug#70314] [PATCH] guix: scripts: environment: add tls certs to networked containers

[bug#70314] [PATCH] guix: scripts: environment: add tls certs to networked containers

[Richard Sent]

* guix/scripts/environment.scm: Add --no-tls flag. By default when starting a
container with -N, add nss-certs package and set SSL_CERT_DIR and
SSL_CERT_FILE environment variables. When --no-tls is passed, default to old
behavior.
* doc/guix.texi: Document it.

Change-Id: I3d222522fa9785fbf589f15efd14e6d6d072bfa7
---
Hi Guix!

Given the discussion on IRC and guix-devel [1] recently about making
nss-certs easier to use, this patch modifies guix environment (and
thus guix shell) to automatically add nss-certs to the profile when
sharing the network namespace, as well as setting the
mostly-standardized SSL_CERT_DIR and SSL_CERT_FILE environment
variables.

This behavior can be reverted with the --no-tls flag. Since presumably
the majority of shell users want TLS to work out of the box, adding
TLS by default makes sense to me.

Previous workarounds were verbose [2] and prone to failure [3].

[1] https://lists.gnu.org/archive/html/guix-devel/2024-04/msg00020.html

[2] https://lists.gnu.org/archive/html/guix-patches/2020-05/msg00197.html

[3] See tail of https://logs.guix.gnu.org/guix/2024-04-08.log, [2]
works coincidentally since guix system w/ nss-certs happens to have
identical nss-certs hash as the guix building the shell profile.
Otherwise the system version would not be visible inside the
container.

 doc/guix.texi                |  8 ++++++++
 guix/scripts/environment.scm | 28 +++++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 5827e0de14..912ed79ccd 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -6214,6 +6214,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}
@@ -6711,6 +6715,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 1d7a6e198d..b38882a4ca 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -49,6 +49,7 @@ (define-module (guix scripts environment)
   #:autoload   (guix build syscalls) (set-network-interface-up openpty login-tty)
   #:use-module (gnu system file-systems)
   #:autoload   (gnu packages) (specification->package+output)
+  #:autoload   (gnu packages certs) (nss-certs)
   #:autoload   (gnu packages bash) (bash)
   #:autoload   (gnu packages bootstrap) (bootstrap-executable %bootstrap-guile)
   #:autoload   (gnu packages package-management) (guix)
@@ -72,6 +73,9 @@ (define-module (guix scripts environment)
 (define %default-shell
   (or (getenv "SHELL") "/bin/sh"))
 
+(define %default-tls-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 +112,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 +251,9 @@ (define %options
          (option '(#\N "network") #f #f
                  (lambda (opt name arg result)
                    (alist-cons 'network? #t result)))
+         (option '(#\T "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 +369,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)
                        '()
@@ -725,7 +740,7 @@ (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?
+                                       no-tls? map-cwd? emulate-fhs? nesting?
                                        (setup-hook #f)
                                        (symlinks '()) (white-list '()))
   "Run COMMAND within a container that features the software in PROFILE.
@@ -929,6 +944,11 @@ (define* (launch-environment/container #:key command bash user user-mappings
               ;; Allow local AF_INET communications.
               (set-network-interface-up "lo"))
 
+            (unless no-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?
@@ -1078,6 +1098,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?))
@@ -1133,6 +1154,10 @@ (define (guix-environment* opts)
       (when (pair? symlinks)
         (leave (G_ "'--symlink' cannot be used without '--container'~%"))))
 
+    (when (and (not network?)
+               no-tls?)
+      (leave (G_ "'--no-tls' cannot be used without '--networking'~%")))
+
     (with-store/maybe store
       (with-status-verbosity (assoc-ref opts 'verbosity)
         (define manifest-from-opts
@@ -1212,6 +1237,7 @@ (define (guix-environment* opts)
                                                   #:network? network?
                                                   #:map-cwd? (not no-cwd?)
                                                   #:emulate-fhs? emulate-fhs?
+                                                  #:no-tls? no-tls?
                                                   #:nesting? nesting?
                                                   #:symlinks symlinks
                                                   #:setup-hook

base-commit: 35e1d9247e39f3c91512cf3d9ef1467962389e35
-- 
2.41.0

[bug#70314] [PATCH] guix: scripts: environment: add tls certs to networked containers

[Ludovic Courtès]

Hi Richard,

Richard Sent <richard@freakingpenguin.com> skribis:

> * guix/scripts/environment.scm: Add --no-tls flag. By default when starting a
> container with -N, add nss-certs package and set SSL_CERT_DIR and
> SSL_CERT_FILE environment variables. When --no-tls is passed, default to old
> behavior.
> * doc/guix.texi: Document it.
>
> Change-Id: I3d222522fa9785fbf589f15efd14e6d6d072bfa7

Apparently this patch fell through the cracks, despite the long Cc:
list.

> Given the discussion on IRC and guix-devel [1] recently about making
> nss-certs easier to use, this patch modifies guix environment (and
> thus guix shell) to automatically add nss-certs to the profile when
> sharing the network namespace, as well as setting the
> mostly-standardized SSL_CERT_DIR and SSL_CERT_FILE environment
> variables.
>
> This behavior can be reverted with the --no-tls flag. Since presumably
> the majority of shell users want TLS to work out of the box, adding
> TLS by default makes sense to me.

[...]

> +                  (('network? . #t)
> +                   (if (assoc-ref opts 'no-tls?)
> +                       '()
> +                       (manifest-entries
> +                        (packages->manifest %default-tls-certs))))

Instead of adding the ‘nss-certs’ package, I would rather expose
/etc/ssl/certs (when it exists), for two reasons: (1) the system-chosen
certificates will be used, and (2) it’s less expensive than having to
compute the derivation of ‘nss-certs’.

Users who definitely want Guix’s ‘nss-certs’ can always add it to the
shell and it will take precedence over /etc/ssl/certs, assuming
SSL_CERT_{FILE,DIR} is defined.

WDYT?

Thanks,
Ludo’.

[bug#70314] [PATCH] guix: scripts: environment: add tls certs to networked containers

[Richard Sent]

Hi Ludo!

Thanks for the response!

Ludovic Courtès <ludo@gnu.org> writes:

> Instead of adding the ‘nss-certs’ package, I would rather expose
> /etc/ssl/certs (when it exists), for two reasons: (1) the system-chosen
> certificates will be used, and (2) it’s less expensive than having to
> compute the derivation of ‘nss-certs’.

There is an issue with this that's cropped up in the past. The files in
/etc/ssl/certs/* are symlinks to store items. Because containers only
see a subset of store items that are in that container's profile, it
often sees the symlinks to store items but not the target file.

For example:

--8<---------------cut here---------------start------------->8---
$ guix shell -C bash coreutils --expose=/etc/ssl/certs -- bash
[env]$ ls /etc/ssl/certs/ca*
/etc/ssl/certs/ca-certificates.crt  /etc/ssl/certs/ca6e4ad9.0
[env]$ cat /etc/ssl/certs/ca-certificates.crt
cat: /etc/ssl/certs/ca-certificates.crt: No such file or directory
[env]$ ls -l /etc/ssl/certs/ca6e4ad9.0
lrwxrwxrwx 1 65534 overflow 85 Jan  1  1970 /etc/ssl/certs/ca6e4ad9.0 -> /gnu/store/5y39gqnvlfrw9gxyxbqqkdr8cxgp1fa1-nss-certs-3.88.1/etc/ssl/certs/ca6e4ad9.0
[env]$ cat /etc/ssl/certs/ca6e4ad9.0  
cat: /etc/ssl/certs/ca6e4ad9.0: No such file or directory
--8<---------------cut here---------------end--------------->8---

We can /sort of/ solve this by adding nss-certs to the container, but
only when the nss-certs being added has the same hash as the nss-certs
package.

--8<---------------cut here---------------start------------->8---
# nss-certs w/o version adds v3.99 to the profile, which doesn't match
# the system. Ergo it's still unavailable.
~ $ guix shell -C bash coreutils --expose=/etc/ssl/certs -- bash -c 'cat /etc/ssl/certs/ca6e4ad9.0'
cat: /etc/ssl/certs/ca6e4ad9.0: No such file or directory
# 
# If we specify 3.88.1, it does work, but only for various nss-certificates,
# not the ca-certificates.crt bundle file (which isn't a package).
guix shell -C bash coreutils nss-certs@3.88.1 --expose=/etc/ssl/certs -- bash -c 'cat /etc/ssl/certs/ca6e4ad9.0'
# snip, contents of ca6e4ad9.0
# 
~ $ guix shell -C bash coreutils nss-certs@3.88.1 --expose=/etc/ssl/certs -- bash -c 'cat /etc/ssl/certs/ca-certificates.crt'
cat: /etc/ssl/certs/ca-certificates.crt: No such file or directory
--8<---------------cut here---------------end--------------->8---

This problem becomes impossible to solve in situations where the system
Guix and user Guix have different nss-certs hashes.

Be it by adding nss-certs to the container profile or by exposing
/etc/ssl/certs, we still need to calculate the nss-certs derivation.

(Perhaps a alternative solution is making sure symlink targets to store
items visible to a container are persisted. I don't know how complicated
that would be, but I imagine it's nontrivial.)

> Users who definitely want Guix’s ‘nss-certs’ can always add it to the
> shell and it will take precedence over /etc/ssl/certs, assuming
> SSL_CERT_{FILE,DIR} is defined.

True, although at present anyone who wants to use nss-certs must set
SSL_CERT_{FILE,DIR} manually (or coincidentally install a package that
registers the search path).

--8<---------------cut here---------------start------------->8---
# nss-certs alone doesn't set SSL_CERT_DIR
~ $ guix shell -C bash coreutils nss-certs@3.88.1 -- bash -c 'echo $SSL_CERT_DIR'
# blank
#
# curl registers $SSL_CERT_{FILE,DIR}
~ $ guix shell -C bash coreutils nss-certs@3.88.1 curl -- bash -c 'echo $SSL_CERT_DIR'
/gnu/store/hxylrsqs5cy87cgkxi5fmlzxvfhczlzj-profile/etc/ssl/certs
--8<---------------cut here---------------end--------------->8---

This is unintuitive. Many packages that make use of nss-certs don't
register the search path, e.g. rust-cargo [1]. I'd rather avoid a
solution that is "edit every package that may possibly use nss-certs now
and in the future to register the search path".

> WDYT?

My thoughts are if we have to decide between

1. Users who want TLS with standard public endpoints
2. Users who want TLS with custom private endpoints

it's better to prioritize a good experience for 1 and let 2 opt-out of
the "hand holding" defaults. But perhaps it's possible to make everyone
happy.

If desired this patch can be reworked as opt-in.

[1]: https://logs.guix.gnu.org/guix/2024-04-08.log

-- 
Take it easy,
Richard Sent
Making my computer weirder one commit at a time.

Re: [bug#70314] [PATCH] guix: scripts: environment: add tls certs to networked containers

[Ludovic Courtès]

Hi Richard,

Cc: guix-devel to get more feedback: this is about adding ‘nss-certs’ by
default in ‘guix shell -CN’ containers, along with a ‘--no-tls’ option
to opt out:

  https://issues.guix.gnu.org/70314

Richard Sent <richard@freakingpenguin.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Instead of adding the ‘nss-certs’ package, I would rather expose
>> /etc/ssl/certs (when it exists), for two reasons: (1) the system-chosen
>> certificates will be used, and (2) it’s less expensive than having to
>> compute the derivation of ‘nss-certs’.
>
> There is an issue with this that's cropped up in the past. The files in
> /etc/ssl/certs/* are symlinks to store items. Because containers only
> see a subset of store items that are in that container's profile, it
> often sees the symlinks to store items but not the target file.

Oh, indeed.

[...]

>> Users who definitely want Guix’s ‘nss-certs’ can always add it to the
>> shell and it will take precedence over /etc/ssl/certs, assuming
>> SSL_CERT_{FILE,DIR} is defined.
>
> True, although at present anyone who wants to use nss-certs must set
> SSL_CERT_{FILE,DIR} manually (or coincidentally install a package that
> registers the search path).

Right.

[...]

> My thoughts are if we have to decide between
>
> 1. Users who want TLS with standard public endpoints
> 2. Users who want TLS with custom private endpoints
>
> it's better to prioritize a good experience for 1 and let 2 opt-out of
> the "hand holding" defaults. But perhaps it's possible to make everyone
> happy.

You’ve convinced me.

That it’s opt-out sounds reasonable to me.  ‘--no-tls’ sounds reasonable
too as a name (I thought about ‘--no-x509-certificates’ but that’s
actually less accurate since there are the SSL_* variables in addition
to the certificates themselves).

I have some comments about the patch and I’d like others to weigh in too
before we commit this change.

Thank you!

Ludo’.

[bug#70314] [PATCH] guix: scripts: environment: add tls certs to networked containers

[Ludovic Courtès]

Hi,

Richard Sent <richard@freakingpenguin.com> skribis:

> * guix/scripts/environment.scm: Add --no-tls flag. By default when starting a
> container with -N, add nss-certs package and set SSL_CERT_DIR and
> SSL_CERT_FILE environment variables. When --no-tls is passed, default to old
> behavior.
> * doc/guix.texi: Document it.
>
> Change-Id: I3d222522fa9785fbf589f15efd14e6d6d072bfa7

[...]

> +  #:autoload   (gnu packages certs) (nss-certs)
>    #:autoload   (gnu packages bash) (bash)
>    #:autoload   (gnu packages bootstrap) (bootstrap-executable %bootstrap-guile)
>    #:autoload   (gnu packages package-management) (guix)
> @@ -72,6 +73,9 @@ (define-module (guix scripts environment)
>  (define %default-shell
>    (or (getenv "SHELL") "/bin/sh"))
>  
> +(define %default-tls-certs
> +  (list nss-certs))

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.

This matters for startup time.  To see how it affects the command, you
can run:

  strace -c guix shell coreutils -- true

The second run should make as few system calls as possible.

> +                 (lambda (opt name arg result)
> +                   (alist-cons 'no-tls? #t result)))

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

> +                  (('network? . #t)
> +                   (if (assoc-ref opts 'no-tls?)
> +                       '()
> +                       (manifest-entries
> +                        (packages->manifest %default-tls-certs))))

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

That way ‘guix shell -N --no-tls’ does not add ‘nss-certs’ to the
environments.

>  (define* (launch-environment/container #:key command bash user user-mappings
>                                         profile manifest link-profile? network?
> -                                       map-cwd? emulate-fhs? nesting?
> +                                       no-tls? map-cwd? emulate-fhs? nesting?

Same as above: ‘tls?’ rather than ‘no-tls?’.

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

Thanks,
Ludo’.

[bug#70314] [PATCH] guix: scripts: environment: add tls certs to networked containers

[Ryan Prior via Guix-patches via]

On Sunday, September 15th, 2024 at 4:39 PM, Ludovic Courtès <ludo@gnu.org> wrote:

> You’ve convinced me.
> 
> That it’s opt-out sounds reasonable to me. ‘--no-tls’ sounds reasonable
> too as a name


Agreed on all points. Even though I'm aware of the need, I've forgotten to add tls-certs many times. Removing a known footgun for containers is a great plan.

Thanks!
Ryan

[bug#70314] [PATCH] guix: scripts: environment: add tls certs to networked containers

[Richard Sent]

Ludovic Courtès <ludo@gnu.org> writes:

> Can we delay changes to the manifest until after all options have been
> parsed, so we know whether ‘-C’ has been passed?
> 
> That way ‘guix shell -N --no-tls’ does not add ‘nss-certs’ to the
> environments.

Is `$ guix shell -N -- true` valid? I know it works at present, but my
understanding is sharing the network only works with containers. From
the manual:

> ‘--network’
> ‘-N’
>      For containers, share the network namespace with the host system.
>      Containers created without this flag only have access to the
>      loopback device.

Perhaps instead we should error when -N is passed without -C, ala

--8<---------------cut here---------------start------------->8---
modified   guix/scripts/environment.scm
@@ -1153,7 +1153,9 @@ (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?)
--8<---------------cut here---------------end--------------->8---

-- 
Take it easy,
Richard Sent
Making my computer weirder one commit at a time.