Patchwork gnu: r-curl: Respect CURL_CA_BUNDLE variable.

login
register
mail settings
Submitter Ricardo Wurmus
Date Sept. 7, 2016, 2:56 p.m.
Message ID <20160907145659.7543-1-ricardo.wurmus@mdc-berlin.de>
Download mbox | patch
Permalink /patch/15374/
State New
Headers show

Comments

Ricardo Wurmus - Sept. 7, 2016, 2:56 p.m.
* gnu/packages/web.scm (r-curl)[arguments]: Add phase
"allow-CURL_CA_BUNDLE".
---
 gnu/packages/web.scm | 13 +++++++++++++
 1 file changed, 13 insertions(+)
Leo Famulari - Sept. 13, 2016, 5:45 a.m.
On Wed, Sep 07, 2016 at 04:56:59PM +0200, Ricardo Wurmus wrote:
> * gnu/packages/web.scm (r-curl)[arguments]: Add phase
> "allow-CURL_CA_BUNDLE".
> ---
>  gnu/packages/web.scm | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/gnu/packages/web.scm b/gnu/packages/web.scm
> index 87bc3e2..321a250 100644
> --- a/gnu/packages/web.scm
> +++ b/gnu/packages/web.scm
> @@ -3168,6 +3168,19 @@ applications.")
>                 (base32
>                  "1p24bcaf1wbfdi1r9ibyyp0l0zp4kzs4g3srv8vikz93hycm1qa6"))))
>      (build-system r-build-system)
> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         ;; The environment variable CURL_CA_BUNDLE is only respected when
> +         ;; running Windows, so we disable the platform checks.

It seems like a weird restriction. Could you see what the upstream
maintainers think about making this change?
Roel Janssen - Sept. 13, 2016, 9:53 p.m.
Ricardo Wurmus writes:

> * gnu/packages/web.scm (r-curl)[arguments]: Add phase
> "allow-CURL_CA_BUNDLE".
> ---
>  gnu/packages/web.scm | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/gnu/packages/web.scm b/gnu/packages/web.scm
> index 87bc3e2..321a250 100644
> --- a/gnu/packages/web.scm
> +++ b/gnu/packages/web.scm
> @@ -3168,6 +3168,19 @@ applications.")
>                 (base32
>                  "1p24bcaf1wbfdi1r9ibyyp0l0zp4kzs4g3srv8vikz93hycm1qa6"))))
>      (build-system r-build-system)
> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         ;; The environment variable CURL_CA_BUNDLE is only respected when
> +         ;; running Windows, so we disable the platform checks.
> +         (add-after 'unpack 'allow-CURL_CA_BUNDLE
> +           (lambda _
> +             (substitute* "R/onload.R"
> +               (("if \\(!grepl\\(\"mingw\".*")
> +                "if (FALSE)\n"))
> +             (substitute* "src/handle.c"
> +               (("#ifdef _WIN32") "#if 1"))
> +             #t)))))
>      (inputs
>       `(("libcurl" ,curl)))
>      (home-page "https://github.com/jeroenooms/curl")

This patch was essential to me being able to interact with HTTPS urls in
R.  As far as I understand, by default, R only looks for CURL_CA_BUNDLE
on Windows, but with this patch it looks for CURL_CA_BUNDLE on GNU/Linux
as well.  Is this correct?

I can confirm it works for me, so I'd like to see this patch pushed.

Kind regards,
Roel Janssen
Leo Famulari - Sept. 14, 2016, 1:17 a.m.
On Tue, Sep 13, 2016 at 11:53:33PM +0200, Roel Janssen wrote:
> This patch was essential to me being able to interact with HTTPS urls in
> R.  As far as I understand, by default, R only looks for CURL_CA_BUNDLE
> on Windows, but with this patch it looks for CURL_CA_BUNDLE on GNU/Linux
> as well.  Is this correct?
> 
> I can confirm it works for me, so I'd like to see this patch pushed.

It's good to hear that it works, but I still think we should run it by
the upstream maintainers. We are activating C code that they
specifically decided not to use on GNU / Linux. Why did they do that?
Ricardo Wurmus - Sept. 21, 2016, 7:24 p.m.
Leo Famulari <leo@famulari.name> writes:

> On Tue, Sep 13, 2016 at 11:53:33PM +0200, Roel Janssen wrote:
>> This patch was essential to me being able to interact with HTTPS urls in
>> R.  As far as I understand, by default, R only looks for CURL_CA_BUNDLE
>> on Windows, but with this patch it looks for CURL_CA_BUNDLE on GNU/Linux
>> as well.  Is this correct?
>> 
>> I can confirm it works for me, so I'd like to see this patch pushed.
>
> It's good to hear that it works, but I still think we should run it by
> the upstream maintainers. We are activating C code that they
> specifically decided not to use on GNU / Linux. Why did they do that?

The comments in the code indicate that on Windows they try to load the
certs bundle that comes with R for Windows, i.e. in the R HOME’s “etc”
directory.  There is no such file on GNU, so no special handling is
needed.

On GNU this is taken care of by libcurl.  It comes with a default path
to the certs bundle, which can be overridden with configure flags
(“--with-ca-bundle” or “--with-ca-path”).  In our Guix package we don’t
do this (yet?), so by default SSL cert validation is broken.

libcurl does not respect CURL_CA_BUNDLE; it assumes that the application
will override the CA bundle path if it needs a special path, otherwise
it assumes that the default path is fine (using Guix this is not the
case).

The maintainers of the R curl package made the special case for Windows
because it is not needed on GNU systems following the FHS.  The best fix
here would be to patch libcurl such that it checks the CURL_CA_BUNDLE
environment variable invariably, just like the curl command line tool
does.  Until this is done I think we should path packages such as
r-curl to make them usable.  Once we have agreed on a fix to libcurl we
can remove all patches to individual packages using libcurl.

~~ Ricardo
Leo Famulari - Sept. 23, 2016, 5:26 a.m.
On Wed, Sep 21, 2016 at 09:24:10PM +0200, Ricardo Wurmus wrote:
> Leo Famulari <leo@famulari.name> writes:
> 
> > On Tue, Sep 13, 2016 at 11:53:33PM +0200, Roel Janssen wrote:
> >> This patch was essential to me being able to interact with HTTPS urls in
> >> R.  As far as I understand, by default, R only looks for CURL_CA_BUNDLE
> >> on Windows, but with this patch it looks for CURL_CA_BUNDLE on GNU/Linux
> >> as well.  Is this correct?
> >> 
> >> I can confirm it works for me, so I'd like to see this patch pushed.
> >
> > It's good to hear that it works, but I still think we should run it by
> > the upstream maintainers. We are activating C code that they
> > specifically decided not to use on GNU / Linux. Why did they do that?
> 
> The comments in the code indicate that on Windows they try to load the
> certs bundle that comes with R for Windows, i.e. in the R HOME’s “etc”
> directory.  There is no such file on GNU, so no special handling is
> needed.
> 
> On GNU this is taken care of by libcurl.  It comes with a default path
> to the certs bundle, which can be overridden with configure flags
> (“--with-ca-bundle” or “--with-ca-path”).  In our Guix package we don’t
> do this (yet?), so by default SSL cert validation is broken.
> 
> libcurl does not respect CURL_CA_BUNDLE; it assumes that the application
> will override the CA bundle path if it needs a special path, otherwise
> it assumes that the default path is fine (using Guix this is not the
> case).
> 
> The maintainers of the R curl package made the special case for Windows
> because it is not needed on GNU systems following the FHS.  The best fix
> here would be to patch libcurl such that it checks the CURL_CA_BUNDLE
> environment variable invariably, just like the curl command line tool
> does.  Until this is done I think we should path packages such as
> r-curl to make them usable.  Once we have agreed on a fix to libcurl we
> can remove all patches to individual packages using libcurl.

That makes sense. Thank you for taking the time to explain.
Ricardo Wurmus - Sept. 24, 2016, 7:46 a.m.
Leo Famulari <leo@famulari.name> writes:

> On Wed, Sep 21, 2016 at 09:24:10PM +0200, Ricardo Wurmus wrote:
>> Leo Famulari <leo@famulari.name> writes:
>> 
>> > On Tue, Sep 13, 2016 at 11:53:33PM +0200, Roel Janssen wrote:
>> >> This patch was essential to me being able to interact with HTTPS urls in
>> >> R.  As far as I understand, by default, R only looks for CURL_CA_BUNDLE
>> >> on Windows, but with this patch it looks for CURL_CA_BUNDLE on GNU/Linux
>> >> as well.  Is this correct?
>> >> 
>> >> I can confirm it works for me, so I'd like to see this patch pushed.
>> >
>> > It's good to hear that it works, but I still think we should run it by
>> > the upstream maintainers. We are activating C code that they
>> > specifically decided not to use on GNU / Linux. Why did they do that?
>> 
>> The comments in the code indicate that on Windows they try to load the
>> certs bundle that comes with R for Windows, i.e. in the R HOME’s “etc”
>> directory.  There is no such file on GNU, so no special handling is
>> needed.
>> 
>> On GNU this is taken care of by libcurl.  It comes with a default path
>> to the certs bundle, which can be overridden with configure flags
>> (“--with-ca-bundle” or “--with-ca-path”).  In our Guix package we don’t
>> do this (yet?), so by default SSL cert validation is broken.
>> 
>> libcurl does not respect CURL_CA_BUNDLE; it assumes that the application
>> will override the CA bundle path if it needs a special path, otherwise
>> it assumes that the default path is fine (using Guix this is not the
>> case).
>> 
>> The maintainers of the R curl package made the special case for Windows
>> because it is not needed on GNU systems following the FHS.  The best fix
>> here would be to patch libcurl such that it checks the CURL_CA_BUNDLE
>> environment variable invariably, just like the curl command line tool
>> does.  Until this is done I think we should path packages such as
>> r-curl to make them usable.  Once we have agreed on a fix to libcurl we
>> can remove all patches to individual packages using libcurl.
>
> That makes sense. Thank you for taking the time to explain.

Sure!  Pushed to master as 8f309571d3847d4bca331061e881fa01d9badb77.

~~ Ricardo
Roel Janssen - Sept. 24, 2016, 9:39 p.m.
Ricardo Wurmus writes:

> Leo Famulari <leo@famulari.name> writes:
>
>> On Wed, Sep 21, 2016 at 09:24:10PM +0200, Ricardo Wurmus wrote:
>>> Leo Famulari <leo@famulari.name> writes:
>>> 
>>> > On Tue, Sep 13, 2016 at 11:53:33PM +0200, Roel Janssen wrote:
>>> >> This patch was essential to me being able to interact with HTTPS urls in
>>> >> R.  As far as I understand, by default, R only looks for CURL_CA_BUNDLE
>>> >> on Windows, but with this patch it looks for CURL_CA_BUNDLE on GNU/Linux
>>> >> as well.  Is this correct?
>>> >> 
>>> >> I can confirm it works for me, so I'd like to see this patch pushed.
>>> >
>>> > It's good to hear that it works, but I still think we should run it by
>>> > the upstream maintainers. We are activating C code that they
>>> > specifically decided not to use on GNU / Linux. Why did they do that?
>>> 
>>> The comments in the code indicate that on Windows they try to load the
>>> certs bundle that comes with R for Windows, i.e. in the R HOME’s “etc”
>>> directory.  There is no such file on GNU, so no special handling is
>>> needed.
>>> 
>>> On GNU this is taken care of by libcurl.  It comes with a default path
>>> to the certs bundle, which can be overridden with configure flags
>>> (“--with-ca-bundle” or “--with-ca-path”).  In our Guix package we don’t
>>> do this (yet?), so by default SSL cert validation is broken.
>>> 
>>> libcurl does not respect CURL_CA_BUNDLE; it assumes that the application
>>> will override the CA bundle path if it needs a special path, otherwise
>>> it assumes that the default path is fine (using Guix this is not the
>>> case).
>>> 
>>> The maintainers of the R curl package made the special case for Windows
>>> because it is not needed on GNU systems following the FHS.  The best fix
>>> here would be to patch libcurl such that it checks the CURL_CA_BUNDLE
>>> environment variable invariably, just like the curl command line tool
>>> does.  Until this is done I think we should path packages such as
>>> r-curl to make them usable.  Once we have agreed on a fix to libcurl we
>>> can remove all patches to individual packages using libcurl.
>>
>> That makes sense. Thank you for taking the time to explain.
>
> Sure!  Pushed to master as 8f309571d3847d4bca331061e881fa01d9badb77.

Thanks!

Patch

diff --git a/gnu/packages/web.scm b/gnu/packages/web.scm
index 87bc3e2..321a250 100644
--- a/gnu/packages/web.scm
+++ b/gnu/packages/web.scm
@@ -3168,6 +3168,19 @@  applications.")
                (base32
                 "1p24bcaf1wbfdi1r9ibyyp0l0zp4kzs4g3srv8vikz93hycm1qa6"))))
     (build-system r-build-system)
+    (arguments
+     `(#:phases
+       (modify-phases %standard-phases
+         ;; The environment variable CURL_CA_BUNDLE is only respected when
+         ;; running Windows, so we disable the platform checks.
+         (add-after 'unpack 'allow-CURL_CA_BUNDLE
+           (lambda _
+             (substitute* "R/onload.R"
+               (("if \\(!grepl\\(\"mingw\".*")
+                "if (FALSE)\n"))
+             (substitute* "src/handle.c"
+               (("#ifdef _WIN32") "#if 1"))
+             #t)))))
     (inputs
      `(("libcurl" ,curl)))
     (home-page "https://github.com/jeroenooms/curl")