Subject: Re: bug#25975: Use HTTPS in `guix pull`

From: Marius Bakke <[email-masked]>
Date: Wed, 08 Mar 2017 23:49:54 +0100

Ludovic Courtès <[email-masked]> writes:

>> From 6667ea5a2ec3a26dd5c4fb5f792485eeb941a969 Mon Sep 17 00:00:00 2001
>> From: Marius Bakke <[email-masked]>
>> Date: Wed, 1 Mar 2017 22:11:02 +0100
>> Subject: [PATCH] pull: Default to HTTPS.
>>
>> * guix/scripts/pull.scm (%snapshot-url): Use HTTPS.
>> (guix-pull): Add GNUTLS and NSS-CERTS to inputs when appropriate.
>
> [...]
>
>>    (with-error-handling
>>      (let* ((opts  (parse-options))
>>             (store (open-connection))
>>             (url   (assoc-ref opts 'tarball-url)))
>> -      (let ((tarball (download-to-store store url "guix-latest.tar.gz")))
>> +      (let ((tarball
>> +             (if (use-gnutls? url)
>> +                 (begin
>> +                   ;; Add GnuTLS to inputs and load path.
>> +                   (set! %load-path
>> +                     (cons (string-append (package-output store gnutls)
>> +                                          "/share/guile/site/"
>> +                                          (effective-version))
>> +                           %load-path))
>> +                   (if (use-le-certs? url)
>> +                       (parameterize ((%x509-certificate-directory
>> +                                       (string-append (package-output store nss-certs)
>> +                                                      "/etc/ssl/certs")))
>> +                         (fetch-tarball store url))
>> +                       (fetch-tarball store url)))
>> +                 (fetch-tarball store url))))
>
> This doesn’t really work, contrary to what you may experience.  ;-)
>
> Namely, ‘package-output’ is risky because it returns the output file
> name of a package but doesn’t ensure that the store item actually
> exists.  So the above code works as intended when your store already
> contains nss-certs and GnuTLS, but it breaks otherwise.

I suspected as much[0], but when I tested it with Leos "le-certs"
package that was not in my store, it actually got built by this code.
Not sure what's up with that.

[0] https://lists.gnu.org/archive/html/guix-devel/2017-02/msg01161.html

> Instead we need to do something like this, though it’s not great either:
>
>   (let* ((drv (package-derivation store nss-certs))
>          (certs (string-append (derivation->output-path drv) "/etc/…")))
>     (build-derivation store (list drv))  ;ugly: builds something right here
>     …)

I'll give this a go, thanks!

> Another problem is changing ‘%load-path’ for the current process: this
> will fail weirdly if GnuTLS is linked against a different libguile or
> libc than the Guile executing ‘guix pull’.  We should refrain from doing
> that and instead rely on the already install GnuTLS (I think we can
> officially make it a hard requirement).

What is the best way to do this? Simply propagate "gnutls" with "guix"?
The %load-path trick was stolen from (guix download), so I assumed it
was safe ;-)

> The code checks for ‘use-le-certs?’ but then uses all the NSS certs,
> whereas the name implies something LE-specific.  Is that intended?  :-)

That was for easier testing/review while waiting for the "le-certs"
package.

> It’s also a case where I think we might want to use the
> already-installed certificates.

If the URL is not from savannah, the GnuTLS defaults will be used (which
should consult SSL_CERT_DIR). Or did you mean instead of "le-certs"?

Thanks for the feedback!