GNU bug report logs - #48648
[PATCH] gnu: gnutls: Update to 3.6.16 [fixes CVE-2021-20305].

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 48648 in the body.
You can then email your comments to 48648 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Solene Rapenne <solene <at> perso.pw>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: gnutls: Update to 3.6.16 [fixes CVE-2021-20305].
Date: Tue, 25 May 2021 12:36:04 +0200

I removed the 2 patches for previous CVEs that are now merged within
gnutls sources.

I deliberately committed it to master branch despite
guix refresh --list-dependent gnutls returns 5287 packages and that
https://guix.gnu.org/manual/en/guix.html#Submitting-Patches says such
packages with more than 3000 impacted packages should be committed
on core-updates. I did this because it's a minor update to fix a CVE
so this would be weird to wait 6 months for this update.

---
 .../patches/gnutls-CVE-2021-20231.patch       | 62 -------------------
 .../patches/gnutls-CVE-2021-20232.patch       | 60 ------------------
 gnu/packages/tls.scm                          |  9 ++-
 3 files changed, 4 insertions(+), 127 deletions(-)
 delete mode 100644 gnu/packages/patches/gnutls-CVE-2021-20231.patch
 delete mode 100644 gnu/packages/patches/gnutls-CVE-2021-20232.patch

diff --git a/gnu/packages/patches/gnutls-CVE-2021-20231.patch b/gnu/packages/patches/gnutls-CVE-2021-20231.patch
deleted file mode 100644
index 5186522eee..0000000000
--- a/gnu/packages/patches/gnutls-CVE-2021-20231.patch
+++ /dev/null
@@ -1,62 +0,0 @@
-From 15beb4b193b2714d88107e7dffca781798684e7e Mon Sep 17 00:00:00 2001
-From: Daiki Ueno <ueno <at> gnu.org>
-Date: Fri, 29 Jan 2021 14:06:05 +0100
-Subject: [PATCH 1/2] key_share: avoid use-after-free around realloc
-
-Signed-off-by: Daiki Ueno <ueno <at> gnu.org>
----
- lib/ext/key_share.c | 12 +++++-------
- 1 file changed, 5 insertions(+), 7 deletions(-)
-
-diff --git a/lib/ext/key_share.c b/lib/ext/key_share.c
-index ab8abf8fe..a8c4bb5cf 100644
---- a/lib/ext/key_share.c
-+++ b/lib/ext/key_share.c
-@@ -664,14 +664,14 @@ key_share_send_params(gnutls_session_t session,
- {
- 	unsigned i;
- 	int ret;
--	unsigned char *lengthp;
--	unsigned int cur_length;
- 	unsigned int generated = 0;
- 	const gnutls_group_entry_st *group;
- 	const version_entry_st *ver;
- 
- 	/* this extension is only being sent on client side */
- 	if (session->security_parameters.entity == GNUTLS_CLIENT) {
-+		unsigned int length_pos;
-+
- 		ver = _gnutls_version_max(session);
- 		if (unlikely(ver == NULL || ver->key_shares == 0))
- 			return 0;
-@@ -679,16 +679,13 @@ key_share_send_params(gnutls_session_t session,
- 		if (!have_creds_for_tls13(session))
- 			return 0;
- 
--		/* write the total length later */
--		lengthp = &extdata->data[extdata->length];
-+		length_pos = extdata->length;
- 
- 		ret =
- 		    _gnutls_buffer_append_prefix(extdata, 16, 0);
- 		if (ret < 0)
- 			return gnutls_assert_val(ret);
- 
--		cur_length = extdata->length;
--
- 		if (session->internals.hsk_flags & HSK_HRR_RECEIVED) { /* we know the group */
- 			group = get_group(session);
- 			if (unlikely(group == NULL))
-@@ -736,7 +733,8 @@ key_share_send_params(gnutls_session_t session,
- 		}
- 
- 		/* copy actual length */
--		_gnutls_write_uint16(extdata->length - cur_length, lengthp);
-+		_gnutls_write_uint16(extdata->length - length_pos - 2,
-+				     &extdata->data[length_pos]);
- 
- 	} else { /* server */
- 		ver = get_version(session);
--- 
-2.30.2
-
diff --git a/gnu/packages/patches/gnutls-CVE-2021-20232.patch b/gnu/packages/patches/gnutls-CVE-2021-20232.patch
deleted file mode 100644
index dc3a0be690..0000000000
--- a/gnu/packages/patches/gnutls-CVE-2021-20232.patch
+++ /dev/null
@@ -1,60 +0,0 @@
-From 75a937d97f4fefc6f9b08e3791f151445f551cb3 Mon Sep 17 00:00:00 2001
-From: Daiki Ueno <ueno <at> gnu.org>
-Date: Fri, 29 Jan 2021 14:06:23 +0100
-Subject: [PATCH 2/2] pre_shared_key: avoid use-after-free around realloc
-
-Signed-off-by: Daiki Ueno <ueno <at> gnu.org>
----
- lib/ext/pre_shared_key.c | 15 ++++++++++++---
- 1 file changed, 12 insertions(+), 3 deletions(-)
-
-diff --git a/lib/ext/pre_shared_key.c b/lib/ext/pre_shared_key.c
-index a042c6488..380bf39ed 100644
---- a/lib/ext/pre_shared_key.c
-+++ b/lib/ext/pre_shared_key.c
-@@ -267,7 +267,7 @@ client_send_params(gnutls_session_t session,
- 	size_t spos;
- 	gnutls_datum_t username = {NULL, 0};
- 	gnutls_datum_t user_key = {NULL, 0}, rkey = {NULL, 0};
--	gnutls_datum_t client_hello;
-+	unsigned client_hello_len;
- 	unsigned next_idx;
- 	const mac_entry_st *prf_res = NULL;
- 	const mac_entry_st *prf_psk = NULL;
-@@ -428,8 +428,7 @@ client_send_params(gnutls_session_t session,
- 	assert(extdata->length >= sizeof(mbuffer_st));
- 	assert(ext_offset >= (ssize_t)sizeof(mbuffer_st));
- 	ext_offset -= sizeof(mbuffer_st);
--	client_hello.data = extdata->data+sizeof(mbuffer_st);
--	client_hello.size = extdata->length-sizeof(mbuffer_st);
-+	client_hello_len = extdata->length-sizeof(mbuffer_st);
- 
- 	next_idx = 0;
- 
-@@ -440,6 +439,11 @@ client_send_params(gnutls_session_t session,
- 	}
- 
- 	if (prf_res && rkey.size > 0) {
-+		gnutls_datum_t client_hello;
-+
-+		client_hello.data = extdata->data+sizeof(mbuffer_st);
-+		client_hello.size = client_hello_len;
-+
- 		ret = compute_psk_binder(session, prf_res,
- 					 binders_len, binders_pos,
- 					 ext_offset, &rkey, &client_hello, 1,
-@@ -474,6 +478,11 @@ client_send_params(gnutls_session_t session,
- 	}
- 
- 	if (prf_psk && user_key.size > 0 && info) {
-+		gnutls_datum_t client_hello;
-+
-+		client_hello.data = extdata->data+sizeof(mbuffer_st);
-+		client_hello.size = client_hello_len;
-+
- 		ret = compute_psk_binder(session, prf_psk,
- 					 binders_len, binders_pos,
- 					 ext_offset, &user_key, &client_hello, 0,
--- 
-2.30.2
-
diff --git a/gnu/packages/tls.scm b/gnu/packages/tls.scm
index 174438ad87..ed8fc6532a 100644
--- a/gnu/packages/tls.scm
+++ b/gnu/packages/tls.scm
@@ -15,6 +15,7 @@
 ;;; Copyright © 2018 Clément Lassieur <clement <at> lassieur.org>
 ;;; Copyright © 2019 Mathieu Othacehe <m.othacehe <at> gmail.com>
 ;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke <at> gnu.org>
+;;; Copyright © 2021 Solene Rapenne <solene <at> perso.pw>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -164,7 +165,7 @@ living in the same process.")
 (define-public gnutls
   (package
     (name "gnutls")
-    (version "3.6.15")
+    (version "3.6.16")
     (source (origin
               (method url-fetch)
               ;; Note: Releases are no longer on ftp.gnu.org since the
@@ -173,12 +174,10 @@ living in the same process.")
                                   (version-major+minor version)
                                   "/gnutls-" version ".tar.xz"))
               (patches (search-patches "gnutls-skip-trust-store-test.patch"
-                                       "gnutls-cross.patch"
-                                       "gnutls-CVE-2021-20231.patch"
-                                       "gnutls-CVE-2021-20232.patch"))
+                                       "gnutls-cross.patch"))
               (sha256
                (base32
-                "0n0m93ymzd0q9hbknxc2ycanz49sqlkyyf73g9fk7n787llc7a0f"))))
+                "1czk511pslz367shf32f2jvvkp7y1323bcv88c2qng98mj0v6y8v"))))
     (build-system gnu-build-system)
     (arguments
      `(#:tests? ,(not (or (%current-target-system)
-- 
2.31.1

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Solene Rapenne via Guix-patches via <guix-patches <at> gnu.org>
Cc: 48648 <at> debbugs.gnu.org
Subject: Re: [bug#48648] [PATCH] gnu: gnutls: Update to 3.6.16 [fixes
 CVE-2021-20305].
Date: Tue, 25 May 2021 11:49:54 -0400

On Tue, May 25, 2021 at 12:36:04PM +0200, Solene Rapenne via Guix-patches via wrote:
> I removed the 2 patches for previous CVEs that are now merged within
> gnutls sources.

Thanks for this patch!

> I deliberately committed it to master branch despite
> guix refresh --list-dependent gnutls returns 5287 packages and that
> https://guix.gnu.org/manual/en/guix.html#Submitting-Patches says such
> packages with more than 3000 impacted packages should be committed
> on core-updates. I did this because it's a minor update to fix a CVE
> so this would be weird to wait 6 months for this update.

Whether or not the update is minor, we still have to use a "graft" [0]
to change packages with this many dependents on the master branch.

Due to the "functional packaging model" of Guix, every dependent of
GnuTLS must be recompiled when the GnuTLS package is changed. We would
constantly be rebuilding nearly every single package if we did not use
grafts for security updates, and that would be infeasible and
inefficient.

Grafts effectively rewrite binary references in compiled software, so
it's kind of a kludge. The binary interface of the new grafted
replacement must be compatible with the original package, and if it's
not, the problems can be hidden and subtle.

For that reason, it's important to make the smallest change possible
when grafting, to reduce the chance of breakage.

So, the question is, does 3.6.16 include only the fix for
CVE-2021-20305? Or does it also include other changes? If the former, we
should instead cherry-pick the CVE bug fix instead of updating.

Can you look into that and let us know?

> --- a/gnu/packages/patches/gnutls-CVE-2021-20231.patch
> +++ /dev/null

If we do decide to update to 3.6.16, it's also necessary to deregister
the removed patch files in 'gnu/local.mk'. Check this commit for an
example:

https://git.savannah.gnu.org/cgit/guix.git/commit/?id=7c4c781aa40c42d4cd10b8d9482199f3db345e1b

Finally, here is an example of setting up a graft that includes a single
new patch file:

https://git.savannah.gnu.org/cgit/guix.git/commit/?id=7c4c781aa40c42d4cd10b8d9482199f3db345e1b

And here is an example of a graft that "updates" a package:

https://git.savannah.gnu.org/cgit/guix.git/commit/?id=250a216cdc2d5425ee0053f3e614d54e0fb6aa90

Message #14 received at 48648 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <marius <at> gnu.org>
To: Leo Famulari <leo <at> famulari.name>, 48648 <at> debbugs.gnu.org
Cc: Solene Rapenne <solene <at> perso.pw>
Subject: Re: [bug#48648] [PATCH] gnu: gnutls: Update to 3.6.16 [fixes
 CVE-2021-20305].
Date: Tue, 25 May 2021 21:46:10 +0200

[Message part 1 (text/plain, inline)]

Leo Famulari <leo <at> famulari.name> skriver:

> Grafts effectively rewrite binary references in compiled software, so
> it's kind of a kludge. The binary interface of the new grafted
> replacement must be compatible with the original package, and if it's
> not, the problems can be hidden and subtle.
>
> For that reason, it's important to make the smallest change possible
> when grafting, to reduce the chance of breakage.
>
> So, the question is, does 3.6.16 include only the fix for
> CVE-2021-20305? Or does it also include other changes? If the former, we
> should instead cherry-pick the CVE bug fix instead of updating.

GnuTLS usually mention whether or not an update is ABI-compatible:

  https://lists.gnupg.org/pipermail/gnutls-help/2021-May/004707.html

However it's good practice to verify that with something like 'abidiff'
(from the 'libabigail' package).  I.e.:

  abidiff $(guix build gnutls)/lib/libgnutls.so \
          $(./pre-inst-env guix build gnutls)/lib/libgnutls.so

(this won't work because of multiple outputs, but you get the drill)

When there is no change, the graft _should_ be perfectly safe.  If there
are changes, it becomes a judgement call.  The 'abidiff' output is of
great assistance in that case.

Anyway, just some general notes on grafting.  Thanks a lot for looking
after security issues Solene.

[signature.asc (application/pgp-signature, inline)]

Message #17 received at 48648 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Marius Bakke <marius <at> gnu.org>
Cc: 48648 <at> debbugs.gnu.org, Solene Rapenne <solene <at> perso.pw>
Subject: Re: [bug#48648] [PATCH] gnu: gnutls: Update to 3.6.16 [fixes
 CVE-2021-20305].
Date: Tue, 25 May 2021 16:00:53 -0400

[Message part 1 (text/plain, inline)]

On Tue, May 25, 2021 at 09:46:10PM +0200, Marius Bakke wrote:
> GnuTLS usually mention whether or not an update is ABI-compatible:
> 
>   https://lists.gnupg.org/pipermail/gnutls-help/2021-May/004707.html

Ah, that's great. They say it's compatible.

> However it's good practice to verify that with something like 'abidiff'
> (from the 'libabigail' package).  I.e.:
> 
>   abidiff $(guix build gnutls)/lib/libgnutls.so \
>           $(./pre-inst-env guix build gnutls)/lib/libgnutls.so
> 
> (this won't work because of multiple outputs, but you get the drill)

Solene, can you try it and let us know the result?

[signature.asc (application/pgp-signature, inline)]

Message #20 received at 48648 <at> debbugs.gnu.org (full text, mbox):

From: Solene Rapenne <solene <at> perso.pw>
To: Leo Famulari <leo <at> famulari.name>
Cc: 48648 <at> debbugs.gnu.org, Marius Bakke <marius <at> gnu.org>
Subject: Re: [bug#48648] [PATCH] gnu: gnutls: Update to 3.6.16 [fixes
 CVE-2021-20305].
Date: Tue, 25 May 2021 22:47:57 +0200

Le Tue, 25 May 2021 16:00:53 -0400,
Leo Famulari <leo <at> famulari.name> a écrit :

> On Tue, May 25, 2021 at 09:46:10PM +0200, Marius Bakke wrote:
> > GnuTLS usually mention whether or not an update is ABI-compatible:
> > 
> >   https://lists.gnupg.org/pipermail/gnutls-help/2021-May/004707.html  
> 
> Ah, that's great. They say it's compatible.
> 
> > However it's good practice to verify that with something like 'abidiff'
> > (from the 'libabigail' package).  I.e.:
> > 
> >   abidiff $(guix build gnutls)/lib/libgnutls.so \
> >           $(./pre-inst-env guix build gnutls)/lib/libgnutls.so
> > 
> > (this won't work because of multiple outputs, but you get the drill)  
> 
> Solene, can you try it and let us know the result?

abidiff is an interesting program, very useful.

$ abidiff \
/gnu/store/5yvzilh78996627i8avq532sl2c03i95-gnutls-3.6.15/lib/libgnutls.so \
/gnu/store/akc7l65z459pnifrr6bcm97cjvmpvp9k-gnutls-3.6.16/lib/libgnutls.so

$ echo $?
0

I understand from the output that there is no ABI change.

Message #23 received at 48648 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Solene Rapenne <solene <at> perso.pw>
Cc: 48648 <at> debbugs.gnu.org, Marius Bakke <marius <at> gnu.org>
Subject: Re: [bug#48648] [PATCH] gnu: gnutls: Update to 3.6.16 [fixes
 CVE-2021-20305].
Date: Thu, 27 May 2021 10:28:54 -0400

On Tue, May 25, 2021 at 10:47:57PM +0200, Solene Rapenne wrote:
> I understand from the output that there is no ABI change.

Great! So, what's left for this patch is to set up the graft.

Concretely, that means creating a new variable 'gnutls-3.6.16' that
inherits from 'gnutls' and adjusts the version and source fields. Then,
add a replacement field to the new 'gnutls' package that uses
'gnutls-3.6.16'.

Can you send a revised patch?

Message #26 received at 48648 <at> debbugs.gnu.org (full text, mbox):

From: Solene Rapenne <solene <at> perso.pw>
To: Leo Famulari <leo <at> famulari.name>
Cc: 48648 <at> debbugs.gnu.org, Marius Bakke <marius <at> gnu.org>
Subject: Re: [bug#48648] [PATCH] gnu: gnutls: Update to 3.6.16 [fixes
 CVE-2021-20305].
Date: Fri, 28 May 2021 19:06:52 +0200

Le Thu, 27 May 2021 10:28:54 -0400,
Leo Famulari <leo <at> famulari.name> a écrit :

> On Tue, May 25, 2021 at 10:47:57PM +0200, Solene Rapenne wrote:
> > I understand from the output that there is no ABI change.  
> 
> Great! So, what's left for this patch is to set up the graft.
> 
> Concretely, that means creating a new variable 'gnutls-3.6.16' that
> inherits from 'gnutls' and adjusts the version and source fields. Then,
> add a replacement field to the new 'gnutls' package that uses
> 'gnutls-3.6.16'.
> 
> Can you send a revised patch?

here is the new patch

From 086ebe0c9e2a8999d1ce46ffa75291ea5a25f2ed Mon Sep 17 00:00:00 2001
From: Solene Rapenne <solene <at> perso.pw>
Date: Fri, 28 May 2021 19:05:23 +0200
Subject: [PATCH] gnu: gnutls: Replace with 3.6.16 [fixes CVE-2021-20305].

---
 gnu/packages/tls.scm | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/gnu/packages/tls.scm b/gnu/packages/tls.scm
index 174438ad87..55410f3911 100644
--- a/gnu/packages/tls.scm
+++ b/gnu/packages/tls.scm
@@ -15,6 +15,7 @@
 ;;; Copyright © 2018 Clément Lassieur <clement <at> lassieur.org>
 ;;; Copyright © 2019 Mathieu Othacehe <m.othacehe <at> gmail.com>
 ;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke <at> gnu.org>
+;;; Copyright © 2021 Solene Rapenne <solene <at> perso.pw>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -165,6 +166,7 @@ living in the same process.")
   (package
     (name "gnutls")
     (version "3.6.15")
+    (replacement gnutls-3.6.16)
     (source (origin
               (method url-fetch)
               ;; Note: Releases are no longer on ftp.gnu.org since the
@@ -258,6 +260,22 @@ required structures.")
     (properties '((ftp-server . "ftp.gnutls.org")
                   (ftp-directory . "/gcrypt/gnutls")))))
 
+;; Replacement package to fix CVE-2021-20305.
+(define gnutls-3.6.16
+  (package
+    (inherit gnutls)
+    (version "3.6.16")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "mirror://gnupg/gnutls/v"
+                                  (version-major+minor version)
+                                  "/gnutls-" version ".tar.xz"))
+              (patches (search-patches "gnutls-skip-trust-store-test.patch"
+                                       "gnutls-cross.patch"))
+              (sha256
+               (base32
+                "1czk511pslz367shf32f2jvvkp7y1323bcv88c2qng98mj0v6y8v"))))))
+
 (define-public gnutls/guile-2.0
   ;; GnuTLS for Guile 2.0.
   (package/inherit gnutls
-- 
2.31.1

Message #31 received at 48648-done <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Solene Rapenne <solene <at> perso.pw>
Cc: 48648-done <at> debbugs.gnu.org, Marius Bakke <marius <at> gnu.org>
Subject: Re: [bug#48648] [PATCH] gnu: gnutls: Update to 3.6.16 [fixes
 CVE-2021-20305].
Date: Fri, 28 May 2021 14:57:43 -0400

On Fri, May 28, 2021 at 07:06:52PM +0200, Solene Rapenne wrote:
> From 086ebe0c9e2a8999d1ce46ffa75291ea5a25f2ed Mon Sep 17 00:00:00 2001
> From: Solene Rapenne <solene <at> perso.pw>
> Date: Fri, 28 May 2021 19:05:23 +0200
> Subject: [PATCH] gnu: gnutls: Replace with 3.6.16 [fixes CVE-2021-20305].

Thank you!

I wrote the commit message and pushed as
0b70eb03cbcf5df7de9f468d9e2a3b53379779fe

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.