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

From: Simon Tournier <[email-masked]>
Date: Fri, 20 Sep 2024 17:04:44 +0200

Hi,

On mar., 09 avril 2024 at 15:05, Richard Sent <[email-masked]> 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