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

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

[Solene Rapenne via Guix-patches via]

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@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@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@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@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@lassieur.org>
 ;;; Copyright © 2019 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
+;;; Copyright © 2021 Solene Rapenne <solene@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

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

[Leo Famulari]

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

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

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]

Leo Famulari <leo@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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

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

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

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?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Solene Rapenne via Guix-patches via]

Le Tue, 25 May 2021 16:00:53 -0400,
Leo Famulari <leo@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.

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

[Leo Famulari]

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?

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

[Solene Rapenne via Guix-patches via]

Le Thu, 27 May 2021 10:28:54 -0400,
Leo Famulari <leo@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@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@lassieur.org>
 ;;; Copyright © 2019 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
+;;; Copyright © 2021 Solene Rapenne <solene@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

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

[Leo Famulari]

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@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