bug#14884: TLS connection not terminated properly

bug#14884: TLS connection not terminated properly

[Ludovic Courtès]

As reported by Mark Weaver and others, fetching from
https://archive.apache.org leads an error:

--8<---------------cut here---------------start------------->8---
$ guix build -S subversion --no-substitutes
The following derivation will be built:
   /nix/store/0qm0bggyhrdhrk1ks8hs2pya5n0ikx57-subversion-1.7.8.tar.bz2.drv
@ build-started /nix/store/0qm0bggyhrdhrk1ks8hs2pya5n0ikx57-subversion-1.7.8.tar.bz2.drv - x86_64-linux /nix/var/log/nix/drvs/0q//m0bggyhrdhrk1ks8hs2pya5n0ikx57-subversion-1.7.8.tar.bz2.drv.bz2
starting download of `/nix/store/i35q1vm2sl27sjhs7mx8n2m05056ya9x-subversion-1.7.8.tar.bz2' from `https://archive.apache.org/dist/subversion/subversion-1.7.8.tar.bz2'...
https://archive.apache.org/.../subversion-1.7.8.tar.bz2  99.0% of 5882.7 KiBERROR: Throw to key `gnutls-error' with args `(#<gnutls-error-enum The TLS connection was non-properly terminated.> fill_session_record_port_input)'.
failed to download "/nix/store/i35q1vm2sl27sjhs7mx8n2m05056ya9x-subversion-1.7.8.tar.bz2" from "https://archive.apache.org/dist/subversion/subversion-1.7.8.tar.bz2"
--8<---------------cut here---------------end--------------->8---

We discussed it on IRC some time ago:

<mark_weaver> I just tried, and the wget from guix also works.
<civodul> ok
<mark_weaver> maybe wget is ignoring that particular TLS error, dunno.
* civodul tries  [23:22]
<civodul> i can reproduce it
<mark_weaver> I see something about it on this page:
	      http://download.opensuse.org/distribution/12.1/repo/oss/ChangeLog
								        [23:29]
<mark_weaver> For glib-networking update to version 2.29.92, it says "Fixed a
	      problem when linking against GNUTLS 3.0, where connections would
	      sometimes return the error "The TLS connection was non-properly
	      terminated". (bgo#659233)"  [23:30]
<mark_weaver> I'm not sure what bug tracking system that bug number is in.
<civodul> the rationale is discussed at
	  http://comments.gmane.org/gmane.comp.encryption.gpg.gnutls.devel/4842
								        [23:32]
<mark_weaver> https://bugzilla.gnome.org/show_bug.cgi?id=659233  [23:33]
<mark_weaver> well, I suppose we could just use plain http for that URL.
								        [23:35]
<civodul> sure :-)  [23:36]
<civodul> though the problem is worth fixing
<mark_weaver> is it a problem on our end, or on the apache archive server?
								        [23:37]
<mark_weaver> given that we will check the SHAsum on the downloaded file, I
	      suppose there's no harm in ignoring that error for downloads, in
	      any case.  [23:38]
<civodul> yes, that's what i was thinking  [23:39]
<civodul> but it's actually tricky to ignore
<civodul> because we pass a TLS port to the download code
<mark_weaver> here's what glib-networking did, fwiw:
	      https://bug659233.bugzilla-attachments.gnome.org/attachment.cgi?id=196741
								        [23:40]

The problem is that the exception is raised by the TLS session record
port’s fill_input method, so there’s no nice call site to wrap into
‘catch’.

We could catch around the ‘dump-port’ call in (guix build download), but
we’d lose info about how much data has actually been transferred.

So for now, I will just:

  1. use http://archive.apache.org instead of https;
  2. ignore this problem altogether, unless this behavior is found to be
     widespread.

Comments welcome.

Ludo’.

bug#14884: TLS connection not terminated properly

[Ludovic Courtès]

ludo@gnu.org (Ludovic Courtès) skribis:

> As reported by Mark Weaver and others, fetching from
> https://archive.apache.org leads an error:
>
> $ guix build -S subversion --no-substitutes
> The following derivation will be built:
>    /nix/store/0qm0bggyhrdhrk1ks8hs2pya5n0ikx57-subversion-1.7.8.tar.bz2.drv
> @ build-started /nix/store/0qm0bggyhrdhrk1ks8hs2pya5n0ikx57-subversion-1.7.8.tar.bz2.drv - x86_64-linux /nix/var/log/nix/drvs/0q//m0bggyhrdhrk1ks8hs2pya5n0ikx57-subversion-1.7.8.tar.bz2.drv.bz2
> starting download of `/nix/store/i35q1vm2sl27sjhs7mx8n2m05056ya9x-subversion-1.7.8.tar.bz2' from `https://archive.apache.org/dist/subversion/subversion-1.7.8.tar.bz2'...
> https://archive.apache.org/.../subversion-1.7.8.tar.bz2  99.0% of 5882.7 KiBERROR: Throw to key `gnutls-error' with args `(#<gnutls-error-enum The TLS connection was non-properly terminated.> fill_session_record_port_input)'.
> failed to download "/nix/store/i35q1vm2sl27sjhs7mx8n2m05056ya9x-subversion-1.7.8.tar.bz2" from "https://archive.apache.org/dist/subversion/subversion-1.7.8.tar.bz2"

We were discussing it on IRC and, boom!, I remembered that I fixed
something which may help with this:

  http://git.sv.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=802a25b1ed5c738aa5f9d3d01f33eb89b22afd1b

And indeed, that patch fixes the problem.

I guess we’ll have to add that patch to Guile in ‘core-updates’, so we
can actually benefit from it when building source derivations.

Thanks,
Ludo’.

bug#14884: TLS connection not terminated properly

[Andreas Enge]

On Tue, Jan 21, 2014 at 05:56:05PM +0100, Ludovic Courtès wrote:
> I guess we’ll have to add that patch to Guile in ‘core-updates’, so we
> can actually benefit from it when building source derivations.

Are the sources not fetched with the system guile in guix? So that we would
first need to "guix package -i guile" to profit from the patch?

In any case, a fix would be more than welcome, as none of the python modules
can currently be downloaded from pypi.python.org. And they have been garbage
collected on hydra.

Andreas

[PATCH] gnu: patch Guile 2.0 to fix bug #14884.

[Cyril Roelandt]

* gnu/packages/patches/guile-web.patch: New file.
* gnu-system.am: Add it
* gnu/packages/guile.scm (guile-2.0): Add the patch.
---
 gnu-system.am                        |  1 +
 gnu/packages/guile.scm               |  3 +-
 gnu/packages/patches/guile-web.patch | 78 ++++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/guile-web.patch

diff --git a/gnu-system.am b/gnu-system.am
index a2377fd..88313e2 100644
--- a/gnu-system.am
+++ b/gnu-system.am
@@ -233,6 +233,7 @@ dist_patch_DATA =						\
   gnu/packages/patches/guile-default-utf8.patch			\
   gnu/packages/patches/guile-linux-syscalls.patch		\
   gnu/packages/patches/guile-relocatable.patch			\
+  gnu/packages/patches/guile-web.patch 				\
   gnu/packages/patches/hop-bigloo-4.0b.patch			\
   gnu/packages/patches/libevent-dns-tests.patch			\
   gnu/packages/patches/libtool-skip-tests.patch			\
diff --git a/gnu/packages/guile.scm b/gnu/packages/guile.scm
index 653d42c..867a646 100644
--- a/gnu/packages/guile.scm
+++ b/gnu/packages/guile.scm
@@ -113,7 +113,8 @@ without requiring the source code to be rewritten.")
                                 ".tar.xz"))
             (sha256
              (base32
-              "0nw9y8vjyz4r61v06p9msks5lm58pd91irmzg4k487vmv743h2pp"))))
+              "0nw9y8vjyz4r61v06p9msks5lm58pd91irmzg4k487vmv743h2pp"))
+            (patches (list (search-patch "guile-web.patch")))))
    (build-system gnu-build-system)
    (native-inputs `(("pkgconfig" ,pkg-config)))
    (inputs `(("libffi" ,libffi)
diff --git a/gnu/packages/patches/guile-web.patch b/gnu/packages/patches/guile-web.patch
new file mode 100644
index 0000000..3ac6497
--- /dev/null
+++ b/gnu/packages/patches/guile-web.patch
@@ -0,0 +1,78 @@
+From 802a25b1ed5c738aa5f9d3d01f33eb89b22afd1b Mon Sep 17 00:00:00 2001
+From: Ludovic Courtès <ludo@gnu.org>
+Date: Wed, 15 Jan 2014 22:41:23 +0000
+Subject: web: Don't throw if a response is longer than its Content-Length says.
+
+* module/web/response.scm (make-delimited-input-port): Read at most LEN
+  bytes from PORT, instead of trying to read more and returning an error
+  if more is available.  Try again when 'get-bytevector-n!' return zero.
+* test-suite/tests/web-response.test (example-1): Add garbage after the
+  body itself.
+---
+diff --git a/module/web/response.scm b/module/web/response.scm
+index 570a2d7..58e3f11 100644
+--- a/module/web/response.scm
++++ b/module/web/response.scm
+@@ -1,6 +1,6 @@
+ ;;; HTTP response objects
+ 
+-;; Copyright (C)  2010, 2011, 2012, 2013 Free Software Foundation, Inc.
++;; Copyright (C) 2010, 2011, 2012, 2013, 2014 Free Software Foundation, Inc.
+ 
+ ;; This library is free software; you can redistribute it and/or
+ ;; modify it under the terms of the GNU Lesser General Public
+@@ -246,16 +246,21 @@ closes PORT, unless KEEP-ALIVE? is true."
+                   bytes-read len))
+ 
+   (define (read! bv start count)
+-    (let ((ret (get-bytevector-n! port bv start count)))
+-      (if (eof-object? ret)
+-          (if (= bytes-read len)
+-              0
+-              (fail))
+-          (begin
+-            (set! bytes-read (+ bytes-read ret))
+-            (if (> bytes-read len)
+-                (fail)
+-                ret)))))
++    ;; Read at most LEN bytes in total.  HTTP/1.1 doesn't say what to do
++    ;; when a server provides more than the Content-Length, but it seems
++    ;; wise to just stop reading at LEN.
++    (let ((count (min count (- len bytes-read))))
++      (let loop ((ret (get-bytevector-n! port bv start count)))
++        (cond ((eof-object? ret)
++               (if (= bytes-read len)
++                   0                              ; EOF
++                   (fail)))
++              ((and (zero? ret) (> count 0))
++               ;; Do not return zero since zero means EOF, so try again.
++               (loop (get-bytevector-n! port bv start count)))
++              (else
++               (set! bytes-read (+ bytes-read ret))
++               ret)))))
+ 
+   (define close
+     (and (not keep-alive?)
+diff --git a/test-suite/tests/web-response.test b/test-suite/tests/web-response.test
+index f9679f5..99b1293 100644
+--- a/test-suite/tests/web-response.test
++++ b/test-suite/tests/web-response.test
+@@ -1,6 +1,6 @@
+ ;;;; web-response.test --- HTTP responses       -*- mode: scheme; coding: utf-8; -*-
+ ;;;;
+-;;;;   Copyright (C) 2010, 2011, 2012 Free Software Foundation, Inc.
++;;;;   Copyright (C) 2010, 2011, 2012, 2014 Free Software Foundation, Inc.
+ ;;;;
+ ;;;; This library is free software; you can redistribute it and/or
+ ;;;; modify it under the terms of the GNU Lesser General Public
+@@ -39,7 +39,9 @@ Content-Encoding: gzip\r
+ Content-Length: 36\r
+ Content-Type: text/html; charset=utf-8\r
+ \r
+-abcdefghijklmnopqrstuvwxyz0123456789")
++abcdefghijklmnopqrstuvwxyz0123456789
++-> Here is trailing garbage that should be ignored because it is
++   beyond Content-Length.")
+ 
+ (define example-2
+   "HTTP/1.1 200 OK\r
-- 
1.8.4.rc3

bug#14884: TLS connection not terminated properly

[Cyril Roelandt]

On 01/23/2014 08:44 PM, Andreas Enge wrote:
> On Tue, Jan 21, 2014 at 05:56:05PM +0100, Ludovic Courtès wrote:
>> I guess we’ll have to add that patch to Guile in ‘core-updates’, so we
>> can actually benefit from it when building source derivations.
>
> Are the sources not fetched with the system guile in guix? So that we would
> first need to "guix package -i guile" to profit from the patch?
>

I sent a patch to the mailing list, to apply on core-updates, as 
suggested by Ludo. After installing Guile from Guix and re-building a 
gazillion packages, I can install python-setuptools, which comes from PyPI.

> In any case, a fix would be more than welcome, as none of the python modules
> can currently be downloaded from pypi.python.org. And they have been garbage
> collected on hydra.

Development of Python packages is only possible on the core-updates 
branch though - that might be an issue.

Cyril.

bug#14884: TLS connection not terminated properly

[Ludovic Courtès]

Andreas Enge <andreas@enge.fr> skribis:

> On Tue, Jan 21, 2014 at 05:56:05PM +0100, Ludovic Courtès wrote:
>> I guess we’ll have to add that patch to Guile in ‘core-updates’, so we
>> can actually benefit from it when building source derivations.
>
> Are the sources not fetched with the system guile in guix? So that we would
> first need to "guix package -i guile" to profit from the patch?

Derivations for the sources use the ‘guile’ package from Guix, and ‘guix
download’ uses whatever Guile was found when Guix was configured.

I think we’ll apply the patch Cyril provided, but it may be that Guile
2.0.10 will be out before we merge ‘core-updates’, in which case things
will be even simpler.

Ludo’.

bug#14884: TLS connection not terminated properly

[Andreas Enge]

On Fri, Jan 24, 2014 at 02:08:15PM +0100, Ludovic Courtès wrote:
> Derivations for the sources use the ‘guile’ package from Guix, and ‘guix
> download’ uses whatever Guile was found when Guix was configured.

To be sure I understood correctly:
   guix build hello -S
uses guile from guix, and
   guix download ftp://.../hello.tar.gz
uses the guile with which guix was compiled?

On Fri, Jan 24, 2014 at 03:34:29AM +0100, Cyril Roelandt wrote:
> Development of Python packages is only possible on the core-updates
> branch though - that might be an issue.

I do not think so. One could use the core-updates branch to download
the package sources. Once they are in the nix store, they can be built
with master.

Andreas

bug#14884: TLS connection not terminated properly

[Ludovic Courtès]

Andreas Enge <andreas@enge.fr> skribis:

> On Fri, Jan 24, 2014 at 02:08:15PM +0100, Ludovic Courtès wrote:
>> Derivations for the sources use the ‘guile’ package from Guix, and ‘guix
>> download’ uses whatever Guile was found when Guix was configured.
>
> To be sure I understood correctly:
>    guix build hello -S
> uses guile from guix, and
>    guix download ftp://.../hello.tar.gz
> uses the guile with which guix was compiled?

Exactly.

> On Fri, Jan 24, 2014 at 03:34:29AM +0100, Cyril Roelandt wrote:
>> Development of Python packages is only possible on the core-updates
>> branch though - that might be an issue.
>
> I do not think so. One could use the core-updates branch to download
> the package sources. Once they are in the nix store, they can be built
> with master.

Right.  Not convenient, but that won’t last long.

Alternately, I think you can do:

  wget https://.../foo.tgz
  guile -c '(use-modules (guix)) \
    (add-to-store (open-connection) "foo.tgz" #f "sha256" "foo.tgz")'

  
Ludo’.

Re: [PATCH] gnu: patch Guile 2.0 to fix bug #14884.

[Ludovic Courtès]

Cyril Roelandt <tipecaml@gmail.com> skribis:

> * gnu/packages/patches/guile-web.patch: New file.
> * gnu-system.am: Add it
> * gnu/packages/guile.scm (guile-2.0): Add the patch.

Thanks for the patch.  I am tempted to wait a bit to see how things go
with Guile 2.0.10.  Maybe we’ll just upgrade when it’s out.

Ludo’.

bug#14884: TLS connection not terminated properly

[Ludovic Courtès]

ludo@gnu.org (Ludovic Courtès) skribis:

> ludo@gnu.org (Ludovic Courtès) skribis:
>
>> As reported by Mark Weaver and others, fetching from
>> https://archive.apache.org leads an error:
>>
>> $ guix build -S subversion --no-substitutes
>> The following derivation will be built:
>>    /nix/store/0qm0bggyhrdhrk1ks8hs2pya5n0ikx57-subversion-1.7.8.tar.bz2.drv
>> @ build-started /nix/store/0qm0bggyhrdhrk1ks8hs2pya5n0ikx57-subversion-1.7.8.tar.bz2.drv - x86_64-linux /nix/var/log/nix/drvs/0q//m0bggyhrdhrk1ks8hs2pya5n0ikx57-subversion-1.7.8.tar.bz2.drv.bz2
>> starting download of `/nix/store/i35q1vm2sl27sjhs7mx8n2m05056ya9x-subversion-1.7.8.tar.bz2' from `https://archive.apache.org/dist/subversion/subversion-1.7.8.tar.bz2'...
>> https://archive.apache.org/.../subversion-1.7.8.tar.bz2  99.0% of 5882.7 KiBERROR: Throw to key `gnutls-error' with args `(#<gnutls-error-enum The TLS connection was non-properly terminated.> fill_session_record_port_input)'.
>> failed to download "/nix/store/i35q1vm2sl27sjhs7mx8n2m05056ya9x-subversion-1.7.8.tar.bz2" from "https://archive.apache.org/dist/subversion/subversion-1.7.8.tar.bz2"
>
> We were discussing it on IRC and, boom!, I remembered that I fixed
> something which may help with this:
>
>   http://git.sv.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=802a25b1ed5c738aa5f9d3d01f33eb89b22afd1b
>
> And indeed, that patch fixes the problem.

Now that Guile 2.0.11 is in Guix master, we can close this bug.

Thanks,
Ludo’.