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

Previous Next

Package: guix-patches;

Reported by: Richard Sent <richard <at> freakingpenguin.com>

Date: Tue, 9 Apr 2024 19:15:01 UTC

Severity: normal

Tags: patch

To reply to this bug, email your comments to 70314 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix <at> cbaines.net, dev <at> jpoiret.xyz, ludo <at> gnu.org, othacehe <at> gnu.org, rekado <at> elephly.net, zimon.toutoune <at> gmail.com, me <at> tobias.gr, guix-patches <at> gnu.org:
bug#70314; Package guix-patches. (Tue, 09 Apr 2024 19:15:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Richard Sent <richard <at> freakingpenguin.com>:
New bug report received and forwarded. Copy sent to guix <at> cbaines.net, dev <at> jpoiret.xyz, ludo <at> gnu.org, othacehe <at> gnu.org, rekado <at> elephly.net, zimon.toutoune <at> gmail.com, me <at> tobias.gr, guix-patches <at> gnu.org. (Tue, 09 Apr 2024 19:15:02 GMT) Full text and rfc822 format available.

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
Cc: Richard Sent <richard <at> freakingpenguin.com>
Subject: [PATCH] guix: scripts: environment: add tls certs to networked
 containers
Date: Tue,  9 Apr 2024 15:05:29 -0400
* 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





Information forwarded to guix-patches <at> gnu.org:
bug#70314; Package guix-patches. (Wed, 04 Sep 2024 13:35:01 GMT) Full text and rfc822 format available.

Message #8 received at 70314 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Richard Sent <richard <at> freakingpenguin.com>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 Christopher Baines <guix <at> cbaines.net>, 70314 <at> debbugs.gnu.org
Subject: Re: [bug#70314] [PATCH] guix: scripts: environment: add tls certs
 to networked containers
Date: Wed, 04 Sep 2024 15:33:30 +0200
Hi Richard,

Richard Sent <richard <at> 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’.




Information forwarded to guix-patches <at> gnu.org:
bug#70314; Package guix-patches. (Wed, 04 Sep 2024 15:04:02 GMT) Full text and rfc822 format available.

Message #11 received at 70314 <at> debbugs.gnu.org (full text, mbox):

From: Richard Sent <richard <at> freakingpenguin.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 Christopher Baines <guix <at> cbaines.net>, 70314 <at> debbugs.gnu.org
Subject: Re: [bug#70314] [PATCH] guix: scripts: environment: add tls certs
 to networked containers
Date: Wed, 04 Sep 2024 11:01:53 -0400
Hi Ludo!

Thanks for the response!

Ludovic Courtès <ludo <at> 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 <at> 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 <at> 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 <at> 3.88.1 -- bash -c 'echo $SSL_CERT_DIR'
# blank
#
# curl registers $SSL_CERT_{FILE,DIR}
~ $ guix shell -C bash coreutils nss-certs <at> 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.




Information forwarded to guix-patches <at> gnu.org:
bug#70314; Package guix-patches. (Sun, 15 Sep 2024 21:41:02 GMT) Full text and rfc822 format available.

Message #14 received at 70314 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Richard Sent <richard <at> freakingpenguin.com>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 guix-devel <guix-devel <at> gnu.org>, Christopher Baines <guix <at> cbaines.net>,
 70314 <at> debbugs.gnu.org
Subject: Re: [bug#70314] [PATCH] guix: scripts: environment: add tls certs
 to networked containers
Date: Sun, 15 Sep 2024 23:39:39 +0200
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 <at> freakingpenguin.com> skribis:

> Ludovic Courtès <ludo <at> 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’.




Information forwarded to guix-patches <at> gnu.org:
bug#70314; Package guix-patches. (Sun, 15 Sep 2024 21:50:02 GMT) Full text and rfc822 format available.

Message #17 received at 70314 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Richard Sent <richard <at> freakingpenguin.com>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 Christopher Baines <guix <at> cbaines.net>, 70314 <at> debbugs.gnu.org
Subject: Re: [bug#70314] [PATCH] guix: scripts: environment: add tls certs
 to networked containers
Date: Sun, 15 Sep 2024 23:49:27 +0200
Hi,

Richard Sent <richard <at> 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’.




Information forwarded to guix-patches <at> gnu.org:
bug#70314; Package guix-patches. (Mon, 16 Sep 2024 00:05:02 GMT) Full text and rfc822 format available.

Message #20 received at 70314 <at> debbugs.gnu.org (full text, mbox):

From: Ryan Prior <rprior <at> protonmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Richard Sent <richard <at> freakingpenguin.com>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 guix-devel <guix-devel <at> gnu.org>, Christopher Baines <guix <at> cbaines.net>,
 70314 <at> debbugs.gnu.org
Subject: Re: [bug#70314] [PATCH] guix: scripts: environment: add tls certs to
 networked containers
Date: Mon, 16 Sep 2024 00:04:23 +0000
On Sunday, September 15th, 2024 at 4:39 PM, Ludovic Courtès <ludo <at> 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




Information forwarded to guix-patches <at> gnu.org:
bug#70314; Package guix-patches. (Mon, 16 Sep 2024 15:23:02 GMT) Full text and rfc822 format available.

Message #23 received at 70314 <at> debbugs.gnu.org (full text, mbox):

From: Richard Sent <richard <at> freakingpenguin.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 Christopher Baines <guix <at> cbaines.net>, 70314 <at> debbugs.gnu.org
Subject: Re: [bug#70314] [PATCH] guix: scripts: environment: add tls certs
 to networked containers
Date: Mon, 16 Sep 2024 11:22:03 -0400
Ludovic Courtès <ludo <at> 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.




Information forwarded to guix-patches <at> gnu.org:
bug#70314; Package guix-patches. (Fri, 20 Sep 2024 16:12:02 GMT) Full text and rfc822 format available.

Message #26 received at 70314 <at> debbugs.gnu.org (full text, mbox):

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Richard Sent <richard <at> freakingpenguin.com>, 70314 <at> debbugs.gnu.org,
 Ludovic Courtès <ludo <at> gnu.org>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Richard Sent <richard <at> freakingpenguin.com>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 Christopher Baines <guix <at> cbaines.net>,
 Ludovic Courtès <ludo <at> gnu.org>
Subject: Re: [bug#70314] [PATCH] guix: scripts: environment: add tls certs
 to networked containers
Date: Fri, 20 Sep 2024 17:04:44 +0200
Hi,

On mar., 09 avril 2024 at 15:05, Richard Sent <richard <at> freakingpenguin.com> wrote:

> 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.

I agree.  I have been annoyed more than once with this.  Then it becomes
something odd that I have forgotten it’s odd. :-)


> +  (display (G_ "
> +      --no-tls           do not add SSL/TLS certificates or set environment
> +                         variables for a networked container"))

[...]

> +         (option '(#\T "no-tls") #f #f
> +                 (lambda (opt name arg result)
> +                   (alist-cons 'no-tls? #t result)))

There is a discrepancy, no?  Missing the short ’-T’ option in the help?

Well, that’s said, I would prefer to not have any short option at all.
Because I think that option would be a rare option.  And if it is not
and many people use “guix shell” without the package ’nss-tls’, then we
will still be able to add the short option.  The converse is not true


>           (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'~%")))
> +

Why not a warning instead of leaving with an error?


Cheers,
simon




Information forwarded to guix-patches <at> gnu.org:
bug#70314; Package guix-patches. (Fri, 20 Sep 2024 16:12:02 GMT) Full text and rfc822 format available.

Message #29 received at 70314 <at> debbugs.gnu.org (full text, mbox):

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>, Richard Sent
 <richard <at> freakingpenguin.com>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 Christopher Baines <guix <at> cbaines.net>, 70314 <at> debbugs.gnu.org
Subject: Re: [bug#70314] [PATCH] guix: scripts: environment: add tls certs
 to networked containers
Date: Fri, 20 Sep 2024 17:19:59 +0200
Hi Ludo,

On dim., 15 sept. 2024 at 23:49, Ludovic Courtès <ludo <at> gnu.org> wrote:

>> +  #:autoload   (gnu packages certs) (nss-certs)

[...]

>> +(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 is a question I had but not reported when commenting elsewhere this
patch.  I was thinking to suggest:

    (module-ref (resolve-interface '(gnu packages certs)) 'nss-certs)

which lazily loads, IIUC.  Then I gave a look to Guile manual which
mentions:

     ‘#:autoload MODULE SYMBOL-LIST’
[...]
          An autoload is a good way to put off loading a big module
          until it’s really needed, for instance for faster startup or
          if it will only be needed in certain circumstances.

Therefore, could you explain the difference if there is one?



Cheers,
simon




This bug report was last modified 59 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.