[bug#30165] [PATCH] gnu: gnurl: Add '--with-ca-bundle' path to configure-flags.

[bug#30165] [PATCH] gnu: gnurl: Add '--with-ca-bundle' path to configure-flags.

[ng0+guixpatches]

From: ng0 <ng0@n0.is>

As a fork of cURL, gnURL on Guix needs to be made aware of the certificate store
to prevent the well-known issue we have.

* gnu/packages/gnunet.scm (gnurl)[configure-flags]: Add '--with-ca-bundle=/etc/ssl/certs/ca-certificates.crt'.
---
 gnu/packages/gnunet.scm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/gnunet.scm b/gnu/packages/gnunet.scm
index 2d238febc..0ebfc9116 100644
--- a/gnu/packages/gnunet.scm
+++ b/gnu/packages/gnunet.scm
@@ -5,7 +5,7 @@
 ;;; Copyright © 2015, 2017 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2016 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2016 Mark H Weaver <mhw@netris.org>
-;;; Copyright © 2016, 2017 ng0 <ng0@n0.is>
+;;; Copyright © 2016, 2017, 2018 ng0 <ng0@n0.is>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -205,7 +205,8 @@ authentication and support for SSL3 and TLS.")
       ("pkg-config" ,pkg-config)
       ("python" ,python-2)))
    (arguments
-    `(#:configure-flags (list "--disable-ntlm-wb")
+    `(#:configure-flags (list "--disable-ntlm-wb"
+                              "--with-ca-bundle=/etc/ssl/certs/ca-certificates.crt")
       #:test-target "test"
       #:parallel-tests? #f
       #:phases
-- 
2.15.1

Re: [bug#30165] [PATCH] gnu: gnurl: Add '--with-ca-bundle' path to configure-flags.

[Adam Van Ymeren]

Regarding https://debbugs.gnu.org/30165

gnurl is failing to build on my system and I think this patch is to
blame.  Why is gnurl referencing something under the root filesystem
rather something provided by an input?  Shouldn't we provided
ca-certificates.crt from an input and reference that?

I guess we want to be able to to change what certificates that gnurl
accepts without rebulding the package, but I think we need something to
provide that file when building the package in the first place, or
disable the relelvant tests.

For reference the build error I'm seeing is
==========================
test 0324...[TLS-SRP with server cert checking]

curl returned 77, when expecting 51
 exit FAILED

 - abort tests

=========================

From the curl man page, exit code 77 means "Problem with reading the SSL
CA cert (path? access rights?)."

Re: [bug#30165] [PATCH] gnu: gnurl: Add '--with-ca-bundle' path to configure-flags.

[ng0+guixpatches]

Sorry, my email went out-of-order for the past 7 days.
I would've sent an explanation to the patch otherwise.
Let me comment inline.

adam@vany.ca transcribed Tue 23 Jan 2018 08:55:35 PM UTC bytes:
> Regarding https://debbugs.gnu.org/30165
>
> gnurl is failing to build on my system and I think this patch is to
> blame.  Why is gnurl referencing something under the root filesystem
> rather something provided by an input?  Shouldn't we provided
> ca-certificates.crt from an input and reference that?

The problem I'm trying to address is the same horror story we
have with cURL: We need to be able to reference a certificate
store.
So far no one in 2+ years fixed this in our cURL to my best
knowledge, so my idea as a maintainer of gnURL was to simply
apply this to gnURL because someone in GNUnet reported errors
with regards to gnURL not finding the certificates with a recent
build of gnURL. I though I had this fixed a while ago, but
apparently I didn't.
I'm more than open to better fixes (we could also set the
expected environment variable).

> I guess we want to be able to to change what certificates that gnurl
> accepts without rebulding the package, but I think we need something to
> provide that file when building the package in the first place, or

What you seem to want is the env. variable solution.

> disable the relelvant tests.
>
> For reference the build error I'm seeing is
> ==========================
> test 0324...[TLS-SRP with server cert checking]
>
> curl returned 77, when expecting 51
>  exit FAILED
>
>  - abort tests
>
> =========================

Wild. For what it's worth, I built this before I've sent a patch
and it passed all of its relevant testsuites. I didn't get this
error.

> From the curl man page, exit code 77 means "Problem with reading the SSL
> CA cert (path? access rights?)."

May I ask what system you are building on? I have a GuixSD-only
setup here. Next time I'll wait for the CI to finish building
(Debian based). I'm in the middle of releasing gnURL 7.58.0 and
preparing for a test that I have tomorrow, followed by some
social appointments afterwards, so I'll be able to start working
on a real fix on the weekend.

In the meantime you could send a patch to revert my commit.
-- 
ng0 :: https://ea.n0.is
A88C8ADD129828D7EAC02E52E22F9BBFEE348588 :: https://ea.n0.is/keys/

Re: [bug#30165] [PATCH] gnu: gnurl: Add '--with-ca-bundle' path to configure-flags.

[ng0]

> Sorry, my email went out-of-order for the past 7 days.
> I would've sent an explanation to the patch otherwise.
> Let me comment inline.
>
> adam@vany.ca transcribed Tue 23 Jan 2018 08:55:35 PM UTC bytes:
>> Regarding https://debbugs.gnu.org/30165
>>
>> gnurl is failing to build on my system and I think this patch is to
>> blame.  Why is gnurl referencing something under the root filesystem
>> rather something provided by an input?  Shouldn't we provided
>> ca-certificates.crt from an input and reference that?
>
> The problem I'm trying to address is the same horror story we
> have with cURL: We need to be able to reference a certificate
> store.
> So far no one in 2+ years fixed this in our cURL to my best
> knowledge, so my idea as a maintainer of gnURL was to simply
> apply this to gnURL because someone in GNUnet reported errors
> with regards to gnURL not finding the certificates with a recent
> build of gnURL. I though I had this fixed a while ago, but
> apparently I didn't.
> I'm more than open to better fixes (we could also set the
> expected environment variable).

Another path forward I see is that I recommend every distro
to set this path for themselves. We haven't fixed the Guix
and GuixSD issue with this, but that's something I need to
adjust for the upcoming release.


>> I guess we want to be able to to change what certificates that gnurl
>> accepts without rebulding the package, but I think we need something to
>> provide that file when building the package in the first place, or
>
> What you seem to want is the env. variable solution.

I'll try that on the weekend at the CI and at home.
Alternative solutions with reference to the original cURL issue
bug in Guix are still welcome.

If we haven't fixed that for cURL in core-updates, as I thought
we had when I read the thread that inspired me for this fix:
Why? Why are we waiting for months and months and months to
patch every single application instead of simply fixing cURL
or our build of cURL and close this case?

On the plus side, once we know which configuration this bug
occurred with, we know that cURL could fail in a similar way ;)

>> disable the relelvant tests.
>>
>> For reference the build error I'm seeing is
>> ==========================
>> test 0324...[TLS-SRP with server cert checking]
>>
>> curl returned 77, when expecting 51
>>  exit FAILED
>>
>>  - abort tests
>>
>> =========================
>
> Wild. For what it's worth, I built this before I've sent a patch
> and it passed all of its relevant testsuites. I didn't get this
> error.
>
>> From the curl man page, exit code 77 means "Problem with reading the SSL
>> CA cert (path? access rights?)."
>
> May I ask what system you are building on? I have a GuixSD-only
> setup here. Next time I'll wait for the CI to finish building
> (Debian based). I'm in the middle of releasing gnURL 7.58.0 and
> preparing for a test that I have tomorrow, followed by some
> social appointments afterwards, so I'll be able to start working
> on a real fix on the weekend.
>
> In the meantime you could send a patch to revert my commit.

-- 
ng0 :: https://ea.n0.is
A88C8ADD129828D7EAC02E52E22F9BBFEE348588 :: https://ea.n0.is/keys/

Re: [bug#30165] [PATCH] gnu: gnurl: Add '--with-ca-bundle' path to configure-flags.

[Ricardo Wurmus]

ng0@n0.is writes:

> Why? Why are we waiting for months and months and months to
> patch every single application instead of simply fixing cURL
> or our build of cURL and close this case?

This is only about libcurl.  Libcurl expects that a user (i.e. a
programme linking with libcurl) sets the certificate bundle.  This
happens in different ways in different applications.  Libcurl
purposefully ignores the usual environment variables, because those are
meant for the “curl” command line tool only.

There also is no bug report in the bug tracker.  Instead of demanding an
answer to why this hasn’t “simply” been fixed, I encourage you to
(re-)read the previous discussions about this:

https://lists.gnu.org/archive/html/guix-devel/2017-01/msg00516.html
https://lists.gnu.org/archive/html/guix-devel/2014-02/msg00245.html


--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

Re: [bug#30165] [PATCH] gnu: gnurl: Add '--with-ca-bundle' path to configure-flags.

[Adam Van Ymeren]

ng0@n0.is writes:

>> The problem I'm trying to address is the same horror story we
>> have with cURL: We need to be able to reference a certificate
>> store.
>> So far no one in 2+ years fixed this in our cURL to my best
>> knowledge, so my idea as a maintainer of gnURL was to simply
>> apply this to gnURL because someone in GNUnet reported errors
>> with regards to gnURL not finding the certificates with a recent
>> build of gnURL. I though I had this fixed a while ago, but
>> apparently I didn't.
>> I'm more than open to better fixes (we could also set the
>> expected environment variable).
>
> Another path forward I see is that I recommend every distro
> to set this path for themselves. We haven't fixed the Guix
> and GuixSD issue with this, but that's something I need to
> adjust for the upcoming release.
>
>
>>> I guess we want to be able to to change what certificates that gnurl
>>> accepts without rebulding the package, but I think we need something to
>>> provide that file when building the package in the first place, or
>>
>> What you seem to want is the env. variable solution.

All I really want is to be able to update my system ;).

I think an env. variable solution makes sense.  We already have
machinery for this in place with how packages can provide "search paths"
for other environment variables.  Perhaps the same should be applied to
SSL certificates.

Referencing some file on the root filesystem does not feel very
functional to me.  That becomes system specific mutable state, which
certainly shouldn't be an input to the build/test process.  For the
purposes of the test, gnurl should be providing an embedded certificate
store inside its own package that it runs tests against, otherwise the
tests are going to be prone to flaky failure like this.

If we want to refernce /etc/ssl/certs for use on non GuixSD distros,
then I think we should make sure that that directory is populated on
startup from the operating system profile on activation.

>> May I ask what system you are building on? I have a GuixSD-only
>> setup here. Next time I'll wait for the CI to finish building
>> (Debian based). I'm in the middle of releasing gnURL 7.58.0 and
>> preparing for a test that I have tomorrow, followed by some
>> social appointments afterwards, so I'll be able to start working
>> on a real fix on the weekend.

I'm building on GuixSD only as well.  I'm building from a git checkouts
with a few local changes to add/update packages I'm working on
(strongswan/python-axolotl) neither of which I believe are inputs to
gnurl.

It's pretty concerning that we're getting different results for building
gnurl, such a thing should be impossible or at least incredibly rare
with a purely function package manager.

Checking hydra.gnu.org, gnurl is failing there as well for i686 and
x86_64 but for different reasons.  It's failing because unbound is
failing which is an input to gnutls-dane which is an input to gnurl.

https://hydra.gnu.org/build/2453496



>>
>> In the meantime you could send a patch to revert my commit.
>
> -- 
> ng0 :: https://ea.n0.is
> A88C8ADD129828D7EAC02E52E22F9BBFEE348588 :: https://ea.n0.is/keys/

Re: [bug#30165] [PATCH] gnu: gnurl: Add '--with-ca-bundle' path to configure-flags.

[Adam Van Ymeren]

ng0@n0.is writes:

>> The problem I'm trying to address is the same horror story we
>> have with cURL: We need to be able to reference a certificate
>> store.
>> So far no one in 2+ years fixed this in our cURL to my best
>> knowledge, so my idea as a maintainer of gnURL was to simply
>> apply this to gnURL because someone in GNUnet reported errors
>> with regards to gnURL not finding the certificates with a recent
>> build of gnURL. I though I had this fixed a while ago, but
>> apparently I didn't.
>> I'm more than open to better fixes (we could also set the
>> expected environment variable).
>
> Another path forward I see is that I recommend every distro
> to set this path for themselves. We haven't fixed the Guix
> and GuixSD issue with this, but that's something I need to
> adjust for the upcoming release.
>
>
>>> I guess we want to be able to to change what certificates that gnurl
>>> accepts without rebulding the package, but I think we need something to
>>> provide that file when building the package in the first place, or
>>
>> What you seem to want is the env. variable solution.

All I really want is to be able to update my system ;).

I think an env. variable solution makes sense.  We already have
machinery for this in place with how packages can provide "search paths"
for other environment variables.  Perhaps the same should be applied to
SSL certificates.

Referencing some file on the root filesystem does not feel very
functional to me.  That becomes system specific mutable state, which
certainly shouldn't be an input to the build/test process.  For the
purposes of the test, gnurl should be providing an embedded certificate
store inside its own package that it runs tests against, otherwise the
tests are going to be prone to flaky failure like this.

If we want to refernce /etc/ssl/certs for use on non GuixSD distros,
then I think we should make sure that that directory is populated on
startup from the operating system profile on activation.

>> May I ask what system you are building on? I have a GuixSD-only
>> setup here. Next time I'll wait for the CI to finish building
>> (Debian based). I'm in the middle of releasing gnURL 7.58.0 and
>> preparing for a test that I have tomorrow, followed by some
>> social appointments afterwards, so I'll be able to start working
>> on a real fix on the weekend.

I'm building on GuixSD only as well.  I'm building from a git checkouts
with a few local changes to add/update packages I'm working on
(strongswan/python-axolotl) neither of which I believe are inputs to
gnurl.

It's pretty concerning that we're getting different results for building
gnurl, such a thing should be impossible or at least incredibly rare
with a purely function package manager.

Checking hydra.gnu.org, gnurl is failing there as well for i686 and
x86_64 but for different reasons.  It's failing because unbound is
failing which is an input to gnutls-dane which is an input to gnurl.

https://hydra.gnu.org/build/2453496



>>
>> In the meantime you could send a patch to revert my commit.
>
> -- 
> ng0 :: https://ea.n0.is
> A88C8ADD129828D7EAC02E52E22F9BBFEE348588 :: https://ea.n0.is/keys/

Re: [bug#30165] [PATCH] gnu: gnurl: Add '--with-ca-bundle' path to configure-flags.

[ng0]

On Wed, 24 Jan 2018, Adam Van Ymeren <adam@vany.ca> wrote:
> ng0@n0.is writes:
>
>>> The problem I'm trying to address is the same horror story we
>>> have with cURL: We need to be able to reference a certificate
>>> store.
>>> So far no one in 2+ years fixed this in our cURL to my best
>>> knowledge, so my idea as a maintainer of gnURL was to simply
>>> apply this to gnURL because someone in GNUnet reported errors
>>> with regards to gnURL not finding the certificates with a recent
>>> build of gnURL. I though I had this fixed a while ago, but
>>> apparently I didn't.
>>> I'm more than open to better fixes (we could also set the
>>> expected environment variable).
>>
>> Another path forward I see is that I recommend every distro
>> to set this path for themselves. We haven't fixed the Guix
>> and GuixSD issue with this, but that's something I need to
>> adjust for the upcoming release.
>>
>>
>>>> I guess we want to be able to to change what certificates that gnurl
>>>> accepts without rebulding the package, but I think we need something to
>>>> provide that file when building the package in the first place, or
>>>
>>> What you seem to want is the env. variable solution.
>
> All I really want is to be able to update my system ;).

Out of curiosity: Where do you use gnURL in the system? For
normal user experience it was never intended (though it would
work). It would be nice to know if another project depends on it,
as we've started looking into wget2 as a successor to cURL for
GNUnet.

Not that we make it obsolete and people really use it. I have no
problem with long-term maintenance.

> I think an env. variable solution makes sense.  We already have
> machinery for this in place with how packages can provide "search paths"
> for other environment variables.  Perhaps the same should be applied to
> SSL certificates.

https://curl.haxx.se/docs/sslcerts.html what applies is this +
some grepping through the cURL source and reading it.

probably useful TIL of this whole issue: cURL could suffer the
same issue if we try this, and I need to test all the options.

> Referencing some file on the root filesystem does not feel very
> functional to me.  That becomes system specific mutable state, which
> certainly shouldn't be an input to the build/test process.  For the
> purposes of the test, gnurl should be providing an embedded certificate
> store inside its own package that it runs tests against, otherwise the
> tests are going to be prone to flaky failure like this.

That really is a cURL specific bug though. I wouldn't mind
extending gnURL where it makes sense, but due to the high speed
chase with cURL it is difficult.

> If we want to refernce /etc/ssl/certs for use on non GuixSD distros,
> then I think we should make sure that that directory is populated on
> startup from the operating system profile on activation.

... with the only problem that operating-system profile
activation does not exist (yet ;)?) for other systems, iirc.

>>> May I ask what system you are building on? I have a GuixSD-only
>>> setup here. Next time I'll wait for the CI to finish building
>>> (Debian based). I'm in the middle of releasing gnURL 7.58.0 and
>>> preparing for a test that I have tomorrow, followed by some
>>> social appointments afterwards, so I'll be able to start working
>>> on a real fix on the weekend.
>
> I'm building on GuixSD only as well.  I'm building from a git checkouts
> with a few local changes to add/update packages I'm working on
> (strongswan/python-axolotl) neither of which I believe are inputs to
> gnurl.

Nope, they aren't. Hm. Curious.

> It's pretty concerning that we're getting different results for building
> gnurl, such a thing should be impossible or at least incredibly rare
> with a purely function package manager.
>
> Checking hydra.gnu.org, gnurl is failing there as well for i686 and
> x86_64 but for different reasons.  It's failing because unbound is
> failing which is an input to gnutls-dane which is an input to gnurl.
>
> https://hydra.gnu.org/build/2453496

Damn. I'll look into it as soon (or slow) as I can. Time is
limited atm.

Thanks for your reply so far. I'll also look into spinning up
some addition gitlab CI jobs to see if I can replicate what we
encountered.
We don't have GuixSD runners yet, but some people on our side are
working on it.

If you have more, like build logs etc would you like to send them
in here, open a bug on our (GNUnet) Mantis instance or send them
to the gnunet bugs list?

Thanks!
>>>
>>> In the meantime you could send a patch to revert my commit.

-- 
ng0 :: https://ea.n0.is
A88C8ADD129828D7EAC02E52E22F9BBFEE348588 :: https://ea.n0.is/keys/

Re: [bug#30165] [PATCH] gnu: gnurl: Add '--with-ca-bundle' path to configure-flags.

[Adam Van Ymeren]

[-- Attachment #1: Type: text/plain, Size: 3834 bytes --]

ng0@n0.is writes:

>
> Out of curiosity: Where do you use gnURL in the system? For
> normal user experience it was never intended (though it would
> work). It would be nice to know if another project depends on it,
> as we've started looking into wget2 as a successor to cURL for
> GNUnet.

I don't actually use it directly, but it's an input for gnunet which I
like to play with from time to time, so I need gnurl to build for gnunet
to build.

>
>> I think an env. variable solution makes sense.  We already have
>> machinery for this in place with how packages can provide "search paths"
>> for other environment variables.  Perhaps the same should be applied to
>> SSL certificates.
>
> https://curl.haxx.se/docs/sslcerts.html what applies is this +
> some grepping through the cURL source and reading it.
>
> probably useful TIL of this whole issue: cURL could suffer the
> same issue if we try this, and I need to test all the options.

I think the patch you did of setting
--with-ca-bundle=/etc/ssl/certs/ca-certificates.crt is reasonable as it
lets gnURL use the operating system level cert store..

I think the root problem is that the gnURL tests the gnURL/cURL tests
are not self contained and depend upon this setting rather for tests
rather than using an embedded cert store during testing.

>
>> Referencing some file on the root filesystem does not feel very
>> functional to me.  That becomes system specific mutable state, which
>> certainly shouldn't be an input to the build/test process.  For the
>> purposes of the test, gnurl should be providing an embedded certificate
>> store inside its own package that it runs tests against, otherwise the
>> tests are going to be prone to flaky failure like this.
>
> That really is a cURL specific bug though. I wouldn't mind
> extending gnURL where it makes sense, but due to the high speed
> chase with cURL it is difficult.

Yes this is definitely a bug with upstream cURL tests, and a fix for
that should be submitted upstream if anyone makes one.

>
>> If we want to refernce /etc/ssl/certs for use on non GuixSD distros,
>> then I think we should make sure that that directory is populated on
>> startup from the operating system profile on activation.
>
> ... with the only problem that operating-system profile
> activation does not exist (yet ;)?) for other systems, iirc.

I may not have been clear.  I meant if we want to have gnURL reference
this directory so that it works nicely on non GuixSD systems, then we
should also make sure that it _is_ provided on GuixSD systems.  And I
think it should be provided on GuixSD at operating system activation
time, so it remains a part of the purely functional system rather than
some mutable state that could get out of date or corrupted.

>> It's pretty concerning that we're getting different results for building
>> gnurl, such a thing should be impossible or at least incredibly rare
>> with a purely function package manager.
>>
>> Checking hydra.gnu.org, gnurl is failing there as well for i686 and
>> x86_64 but for different reasons.  It's failing because unbound is
>> failing which is an input to gnutls-dane which is an input to gnurl.
>>
>> https://hydra.gnu.org/build/2453496
>
> Damn. I'll look into it as soon (or slow) as I can. Time is
> limited atm.

No rush, I can work around it for now :).  Thanks for your hard work
maintaining this.

>
> Thanks for your reply so far. I'll also look into spinning up
> some addition gitlab CI jobs to see if I can replicate what we
> encountered.
> We don't have GuixSD runners yet, but some people on our side are
> working on it.
>
> If you have more, like build logs etc would you like to send them
> in here, open a bug on our (GNUnet) Mantis instance or send them
> to the gnunet bugs list?

I've attached a build log to this message in case it helps :).


[-- Attachment #2: log of guix build gnurl --]
[-- Type: application/octet-stream, Size: 27393 bytes --]