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

Marius Bakke wrote on 16 Jan 2019 14:33

Recipients:(address . bug-guix@gnu.org)

Message-ID: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.

[0] https://nikmav.blogspot.com/2018/05/gnutls-and-tls-13.html

Ludovic Courtès wrote on 25 Jan 2019 14:32

control message for bug #34102

Recipients:(address . control@debbugs.gnu.org)

Message-ID:875zucrhfy.fsf@gnu.org

severity 34102 serious

Ludovic Courtès wrote on 25 Jan 2019 14:43

Re: bug#34102: [staging] Guix fails to download from TLSv1.3-enabled servers

Recipients:(name . Marius Bakke)(address . mbakke@fastmail.com)(address . 34102@debbugs.gnu.org)

Message-ID: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’.

Ricardo Wurmus wrote on 25 Jan 2019 15:04

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)

Message-ID: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

Ludovic Courtès wrote on 27 Jan 2019 16:54

Recipients:(name . Ricardo Wurmus)(address . rekado@elephly.net)

Message-ID: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’.

Ludovic Courtès wrote on 12 Jun 2019 14:34

Recipients:(name . Marius Bakke)(address . mbakke@fastmail.com)(address . 34102@debbugs.gnu.org)

Message-ID: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:

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

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);

Ludovic Courtès wrote on 27 Mar 2020 09:07

Recipients:(name . Marius Bakke)(address . mbakke@fastmail.com)(address . 34102-done@debbugs.gnu.org)

Message-ID: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

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date