[staging] Guix fails to download from TLSv1.3-enabled servers

DoneSubmitted by Marius Bakke.
Details
3 participants
  • Ludovic Courtès
  • Marius Bakke
  • Ricardo Wurmus
Owner
unassigned
Severity
serious
M
M
Marius Bakke wrote on 16 Jan 2019 14:33
(address . bug-guix@gnu.org)
875zuoiv6s.fsf@fastmail.com
Hello!

On the staging branch (with GnuTLS 3.6), `guix download` will negotiate
TLSv1.3 with servers that support it, and fail shortly after the initial
handshake:

$ ./pre-inst-env guix download https://data.iana.org
Starting download of /tmp/guix-file.vJ4v7h
From https://data.iana.org...
Throw to key `gnutls-error' with args `(#<gnutls-error-enum Resource temporarily unavailable, try again.> read_from_session_record_port)'.
failed to download "/tmp/guix-file.vJ4v7h" from "https://data.iana.org"
guix download: error: https://data.iana.org:download failed

The GnuTLS maintainer have written a blog post about TLS 1.3 porting[0],
and I suspect the problem is that Guix (or the GnuTLS Guile bindings)
does not handle the "GNUTLS_E_REAUTH_REQUEST" error code; however my
attempts at catching it (or any error code) has been unfruitful.

This is an obvious merge blocker, help wanted! Disabling TLS1.3 in the
priority string works as a last-resort workaround.

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAlw/MpsACgkQoqBt8qM6
VPrmBAf+Np1ZUW6Ig+q1x89okOiySN/6RlYhtDFOcB4VV3rvRa33HCXrsSpvauSw
WTloJ3qz7mMow0QeG9bPt+3YsO8HnhNoe/vmJTPtRs7nzPRrvFK9dDEn/sgmIrvg
Kxd95V2NLxnrEB3KiFzlf3rsZHMEC1zaBF9BgPEUYARheS2N0yH4N9U9HyieCH5S
ckqUHMH+PMuWYsUaqgXkD1XBYD7d7L9Hy/uLI3X47cJpLytBQB0TEmaOr2pqEgrg
bT1Gv0godCL1+bmRNv57DmKQXhKFNBgMsx+h12Lu/D/Z1rju+ywRxvJSS8jdLjY8
T6ldlxmOHUSfmYO9I1V+Tfi8bo+acg==
=s8VF
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 25 Jan 2019 14:32
control message for bug #34102
(address . control@debbugs.gnu.org)
875zucrhfy.fsf@gnu.org
severity 34102 serious
L
L
Ludovic Courtès wrote on 25 Jan 2019 14:43
Re: bug#34102: [staging] Guix fails to download from TLSv1.3-enabled servers
(name . Marius Bakke)(address . mbakke@fastmail.com)(address . 34102@debbugs.gnu.org)
87sgxgq2cy.fsf@gnu.org
Hi Marius,

Marius Bakke <mbakke@fastmail.com> skribis:

Toggle quote (11 lines)
> On the staging branch (with GnuTLS 3.6), `guix download` will negotiate
> TLSv1.3 with servers that support it, and fail shortly after the initial
> handshake:
>
> $ ./pre-inst-env guix download https://data.iana.org
> Starting download of /tmp/guix-file.vJ4v7h
> From https://data.iana.org...
> Throw to key `gnutls-error' with args `(#<gnutls-error-enum Resource temporarily unavailable, try again.> read_from_session_record_port)'.
> failed to download "/tmp/guix-file.vJ4v7h" from "https://data.iana.org"
> guix download: error: https://data.iana.org: download failed

Ouch, thanks for the heads-up!

Toggle quote (5 lines)
> The GnuTLS maintainer have written a blog post about TLS 1.3 porting[0],
> and I suspect the problem is that Guix (or the GnuTLS Guile bindings)
> does not handle the "GNUTLS_E_REAUTH_REQUEST" error code; however my
> attempts at catching it (or any error code) has been unfruitful.

I think we need to update the Guile bindings to wrap
GNUTLS_E_REAUTH_REQUEST, GNUTLS_POST_HANDSHAKE_AUTH, and
‘gnutls_reauth’, which are currently missing. Would you like to give it
a try?

What’s unclear to me from the blog post is exactly when
GNUTLS_E_REAUTH_REQUEST is delivered to the application. Is it the next
time the application calls some (possibly unrelated) GnuTLS function?

Toggle quote (3 lines)
> This is an obvious merge blocker, help wanted! Disabling TLS1.3 in the
> priority string works as a last-resort workaround.

Yes, that’s a stop-gap measure we should probably apply for now:
Toggle diff (16 lines)
diff --git a/guix/build/download.scm b/guix/build/download.scm
index c08221b3b2..23c9a4d466 100644
--- a/guix/build/download.scm
+++ b/guix/build/download.scm
@@ -268,7 +268,10 @@ host name without trailing dot."
     ;; "(gnutls) Priority Strings"); see <http://bugs.gnu.org/23311>.
     ;; Explicitly disable SSLv3, which is insecure:
     ;; <https://tools.ietf.org/html/rfc7568>.
-    (set-session-priorities! session "NORMAL:%COMPAT:-VERS-SSL3.0")
+    ;;
+    ;; FIXME: Since we currently fail to handle TLS 1.3, remove it; see
+    ;; <https://bugs.gnu.org/34102>.
+    (set-session-priorities! session "NORMAL:%COMPAT:-VERS-SSL3.0:-VERS-TLS1.3")
 
     (set-session-credentials! session
                               (if (and verify-certificate? ca-certs)
Any objections?

Thanks,
Ludo’.
R
R
Ricardo Wurmus wrote on 25 Jan 2019 15:04
(name . Ludovic Courtès)(address . ludo@gnu.org)
87r2d0vnng.fsf@elephly.net
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (24 lines)
>> This is an obvious merge blocker, help wanted! Disabling TLS1.3 in the
>> priority string works as a last-resort workaround.
>
> Yes, that’s a stop-gap measure we should probably apply for now:
>
> diff --git a/guix/build/download.scm b/guix/build/download.scm
> index c08221b3b2..23c9a4d466 100644
> --- a/guix/build/download.scm
> +++ b/guix/build/download.scm
> @@ -268,7 +268,10 @@ host name without trailing dot."
> ;; "(gnutls) Priority Strings"); see <http://bugs.gnu.org/23311>.
> ;; Explicitly disable SSLv3, which is insecure:
> ;; <https://tools.ietf.org/html/rfc7568>.
> - (set-session-priorities! session "NORMAL:%COMPAT:-VERS-SSL3.0")
> + ;;
> + ;; FIXME: Since we currently fail to handle TLS 1.3, remove it; see
> + ;; <https://bugs.gnu.org/34102>.
> + (set-session-priorities! session "NORMAL:%COMPAT:-VERS-SSL3.0:-VERS-TLS1.3")
>
> (set-session-credentials! session
> (if (and verify-certificate? ca-certs)
>
> Any objections?

I think it’s fine to do this to allow us to merge the staging branch
before fixing the problem in the Guile bindings.

--
Ricardo
L
L
Ludovic Courtès wrote on 27 Jan 2019 16:54
(name . Ricardo Wurmus)(address . rekado@elephly.net)
877eeqf64g.fsf@gnu.org
Hello,

Ricardo Wurmus <rekado@elephly.net> skribis:

Toggle quote (5 lines)
> Ludovic Courtès <ludo@gnu.org> writes:
>
>>> This is an obvious merge blocker, help wanted! Disabling TLS1.3 in the
>>> priority string works as a last-resort workaround.

[...]

Toggle quote (3 lines)
> I think it’s fine to do this to allow us to merge the staging branch
> before fixing the problem in the Guile bindings.

I pushed a variant of this patch as commit
e4ee84202633636b4c8cef4a332f0c74912a3b23.

Thanks,
Ludo’.
L
L
Ludovic Courtès wrote on 12 Jun 2019 14:34
(name . Marius Bakke)(address . mbakke@fastmail.com)(address . 34102@debbugs.gnu.org)
87sgsfvv6j.fsf@gnu.org
Hi Marius,

Marius Bakke <mbakke@fastmail.com> skribis:

Toggle quote (17 lines)
> $ ./pre-inst-env guix download https://data.iana.org
> Starting download of /tmp/guix-file.vJ4v7h
> From https://data.iana.org...
> Throw to key `gnutls-error' with args `(#<gnutls-error-enum Resource temporarily unavailable, try again.> read_from_session_record_port)'.
> failed to download "/tmp/guix-file.vJ4v7h" from "https://data.iana.org"
> guix download: error: https://data.iana.org: download failed
>
> The GnuTLS maintainer have written a blog post about TLS 1.3 porting[0],
> and I suspect the problem is that Guix (or the GnuTLS Guile bindings)
> does not handle the "GNUTLS_E_REAUTH_REQUEST" error code; however my
> attempts at catching it (or any error code) has been unfruitful.
>
> This is an obvious merge blocker, help wanted! Disabling TLS1.3 in the
> priority string works as a last-resort workaround.
>
> [0] https://nikmav.blogspot.com/2018/05/gnutls-and-tls-13.html

I’ve submitted a bunch of changes upstream to better support
post-handshake re-authentication:


In particular, this adds ‘connection-flag/post-handshake-auth’ and
‘connection-flag/auto-reauth’, which can be passed to ‘make-session’.

But as it turns out, there’s one patch that, alone, appears to fix the
issue above:


Ideally we’d wait for the next GnuTLS release that includes all of this.
However, if that helps, we can apply this patch to the ‘gnutls’ package
in ‘core-updates’ in the meantime.

WDYT?

Ludo’.
commit 7421ca2cfd2d9f4ac89bdec786eb745533430316
Author: Ludovic Courtès <ludo@gnu.org>
Date: Wed Jun 12 11:32:19 2019 +0200

guile: Loop upon EAGAIN or EINTR.
* guile/src/core.c (do_fill_port) [USING_GUILE_BEFORE_2_2]: Loop while
'gnutls_record_recv' returns GNUTLS_E_AGAIN or GNUTLS_E_INTERRUPTED.
(read_from_session_record_port) [!USING_GUILE_BEFORE_2_2]: Likewise.
Signed-off-by: Ludovic Courtès <ludo@gnu.org>

Toggle diff (40 lines)
diff --git a/guile/src/core.c b/guile/src/core.c
index 546d63a1e3..8b9aa62560 100644
--- a/guile/src/core.c
+++ b/guile/src/core.c
@@ -1,5 +1,5 @@
 /* GnuTLS --- Guile bindings for GnuTLS.
-   Copyright (C) 2007-2014, 2016 Free Software Foundation, Inc.
+   Copyright (C) 2007-2014, 2016, 2019 Free Software Foundation, Inc.
 
    GnuTLS is free software; you can redistribute it and/or
    modify it under the terms of the GNU Lesser General Public
@@ -869,8 +869,12 @@ do_fill_port (void *data)
   const fill_port_data_t *args = (fill_port_data_t *) data;
 
   c_port = args->c_port;
-  result = gnutls_record_recv (args->c_session,
-                               c_port->read_buf, c_port->read_buf_size);
+
+  do
+    result = gnutls_record_recv (args->c_session,
+				 c_port->read_buf, c_port->read_buf_size);
+  while (result == GNUTLS_E_AGAIN || result == GNUTLS_E_INTERRUPTED);
+
   if (EXPECT_TRUE (result > 0))
     {
       c_port->read_pos = c_port->read_buf;
@@ -1002,7 +1006,12 @@ read_from_session_record_port (SCM port, SCM dst, size_t start, size_t count)
 
   /* XXX: Leave guile mode when SCM_GNUTLS_SESSION_TRANSPORT_IS_FD is
      true?  */
-  result = gnutls_record_recv (c_session, read_buf, count);
+  /* We can get EAGAIN for example if we received a reauth request, even when
+     GNUTLS_AUTO_REAUTH is set.  In that case, loop again.  */
+  do
+    result = gnutls_record_recv (c_session, read_buf, count);
+  while (result == GNUTLS_E_AGAIN || result == GNUTLS_E_INTERRUPTED);
+
   if (EXPECT_FALSE (result < 0))
     /* FIXME: Silently swallowed! */
     scm_gnutls_error (result, FUNC_NAME);
L
L
Ludovic Courtès wrote on 27 Mar 2020 09:07
(name . Marius Bakke)(address . mbakke@fastmail.com)(address . 34102-done@debbugs.gnu.org)
87369u8bmd.fsf@gnu.org
Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (13 lines)
> I’ve submitted a bunch of changes upstream to better support
> post-handshake re-authentication:
>
> https://gitlab.com/gnutls/gnutls/merge_requests/1026
>
> In particular, this adds ‘connection-flag/post-handshake-auth’ and
> ‘connection-flag/auto-reauth’, which can be passed to ‘make-session’.
>
> But as it turns out, there’s one patch that, alone, appears to fix the
> issue above:
>
> https://gitlab.com/civodul/gnutls/commit/7421ca2cfd2d9f4ac89bdec786eb745533430316

This was fixed a while back in Guix proper, with commit
621fb83a1fde948b3b7eea37bdc378cbf1b3d11e.

Ludo’.
Closed
?
Your comment

This issue is archived.

To comment on this conversation send email to 34102@debbugs.gnu.org