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

  • Done
  • quality assurance status badge
Details
3 participants
  • Ludovic Courtès
  • Marius Bakke
  • Ricardo Wurmus
Owner
unassigned
Submitted by
Marius Bakke
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 an email to 34102@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 34102
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch