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

  • Done
  • quality assurance status badge
Details
3 participants
  • Leo Famulari
  • Marius Bakke
  • Solene Rapenne
Owner
unassigned
Submitted by
Solene Rapenne
Severity
normal

Debbugs page

Solene Rapenne wrote 4 years ago
(address . guix-patches@gnu.org)
20210525123604.2dc745b3@perso.pw
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
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

Toggle diff (51 lines)
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
-
Toggle diff (104 lines)
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
Leo Famulari wrote 4 years ago
(name . Solene Rapenne via Guix-patches via)(address . guix-patches@gnu.org)(address . 48648@debbugs.gnu.org)
YK0cotMnzZangjHR@jasmine.lan
On Tue, May 25, 2021 at 12:36:04PM +0200, Solene Rapenne via Guix-patches via wrote:
Toggle quote (3 lines)
> I removed the 2 patches for previous CVEs that are now merged within
> gnutls sources.

Thanks for this patch!

Toggle quote (7 lines)
> 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?

Toggle quote (3 lines)
> --- 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:


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


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

Marius Bakke wrote 4 years ago
(name . Solene Rapenne)(address . solene@perso.pw)
87o8cyppfh.fsf@gnu.org
Leo Famulari <leo@famulari.name> skriver:

Toggle quote (12 lines)
> 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:


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.
-----BEGIN PGP SIGNATURE-----

iIUEARYKAC0WIQRNTknu3zbaMQ2ddzTocYulkRQQdwUCYK1UAg8cbWFyaXVzQGdu
dS5vcmcACgkQ6HGLpZEUEHcwnAD7BGg0FF8lJ9lauP+ugd1YJxEboYj+sOiLGIt8
TQEfkL8A/iD6q9IbbquD/CWj6S4iyDFX7j3IJ7rwyu/2N374wPgC
=u1yq
-----END PGP SIGNATURE-----

Leo Famulari wrote 4 years ago
(name . Marius Bakke)(address . marius@gnu.org)
YK1XdYHMlBS6QOve@jasmine.lan
On Tue, May 25, 2021 at 09:46:10PM +0200, Marius Bakke wrote:
Toggle quote (4 lines)
> 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.

Toggle quote (8 lines)
> 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?
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAmCtV3UACgkQJkb6MLrK
fwgkcw//eQyFBuTstK4uSHap34Y/vHov+JwWXAjV449BIi54qxCRQmE8AmrRQtsZ
KlByuWhg5mwBrW4NTjqxxyv4iBPncJ/8fCsxnA6KwO6KGeJ7oSTAoaJA3Mr2gbmB
9nJVAImpexm6XmhrcP3iEPsXPqfVZKsU6KMCSDCfdSjFVy+fUMaiggMIK6ThEovP
DvmudtOV4ydjpnTHXnwc0oO6Vwu5KtVQZ/o15uZvZ/yIi4ZU0SL1M2PlaqHjrA04
2iXdzFMlaE4mpbuWwHByVczNYXgoE5GVOg1UsQQ2b9K/Jq3JyMC+MBAxCahsCju4
LKL73hB8ENTPW06YTX2doHUR+ScKxnlO/dPR5mhXSh9yY07WgcUM4WDFUIkm/Xhq
jO8HGEKt+7syz9O+paxOQGdubkQWPaTT7hW1pVl626aFlJJ+0jlzr0VPSBB0sUgZ
ntImaXMtZp9/nqGhJKppChe/kLhW3o7On8HAzDUeYdVl2pPe4fNeV4LcZrvCOnca
128sAUwFnmOIc9AGV1VlGm31G8K+8BeJFQAz7/0MC1DVUTMfNudStshHehSyE5FD
CBak3Ym5M368XsoZQddVCplvmu/BpnLvpbf0bz5vqOGMoLijQzhrL/gJ0fqYAFtX
0Rs0pwvJw7ub+AUCD047PV+9Vtmacgp9hQa5MRpdMv0LmLAmejQ=
=Oug9
-----END PGP SIGNATURE-----


Solene Rapenne wrote 4 years ago
(name . Leo Famulari)(address . leo@famulari.name)
20210525224757.5e28ca79@perso.pw
Le Tue, 25 May 2021 16:00:53 -0400,
Leo Famulari <leo@famulari.name> a écrit :

Toggle quote (17 lines)
> 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.
Leo Famulari wrote 4 years ago
(name . Solene Rapenne)(address . solene@perso.pw)
YK+spkUik0MPsk+E@jasmine.lan
On Tue, May 25, 2021 at 10:47:57PM +0200, Solene Rapenne wrote:
Toggle quote (2 lines)
> 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?
Solene Rapenne wrote 4 years ago
(name . Leo Famulari)(address . leo@famulari.name)
20210528190652.5eb6753c@perso.pw
Le Thu, 27 May 2021 10:28:54 -0400,
Leo Famulari <leo@famulari.name> a écrit :

Toggle quote (12 lines)
> 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(+)

Toggle diff (45 lines)
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
Leo Famulari wrote 4 years ago
(name . Solene Rapenne)(address . solene@perso.pw)
YLE9J1FaDUi53+JL@jasmine.lan
On Fri, May 28, 2021 at 07:06:52PM +0200, Solene Rapenne wrote:
Toggle quote (5 lines)
> 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
Closed
?
Your comment

This issue is archived.

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

To respond to this issue using the mumi CLI, first switch to it
mumi current 48648
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
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help