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

From: [email-masked] (Ludovic Courtès)
Date: Thu, 09 Mar 2017 11:48:15 +0100

Hi Marius,

Marius Bakke <[email-masked]> skribis:

> 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

Weird.  ‘package-output’ definitely doesn’t build the thing.

>> 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 assume that GnuTLS is already available and thus do nothing.  :-)

> Simply propagate "gnutls" with "guix"?  The %load-path trick was
> stolen from (guix download), so I assumed it was safe ;-)

The gexp in (guix download) that does that is a different story: it’s a
situation where we spawn a new process and we know which Guile and which
GnuTLS package is being used, so it’s completely 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.

OK.

>> 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"?

Yes, I was talking about le-certs.

Thanks!

Ludo’.