GNU bug report logs - #27308
[PATCH] gnu: gnutls: Replace with 3.5.13.

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 27308 in the body.
You can then email your comments to 27308 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: Marius Bakke <mbakke <at> fastmail.com>
To: guix-patches <at> gnu.org
Cc: Marius Bakke <mbakke <at> fastmail.com>
Subject: [PATCH] gnu: gnutls: Replace with 3.5.13.
Date: Sat, 10 Jun 2017 15:58:51 +0200

This update addresses the following security advisories:

GNUTLS-SA-2017-3 (aka CVE-2017-7869) and GNUTLS-SA-2017-4.

These links contain more information about the vulnerabilities and releases:

https://gnutls.org/security.html
https://gnutls.org/news.html

* gnu/packages/patches/gnutls-skip-pkgconfig-test.patch,
gnu/packages/patches/gnutls-skip-trust-store-test.patch: New files.
* gnu/local.mk (dist_patch_DATA): Register patches.
* gnu/packages/tls.scm (gnutls)[replacement]: New field.
(gnutls-3.5.13): New variable.
---
 gnu/local.mk                                       |  2 ++
 .../patches/gnutls-skip-pkgconfig-test.patch       | 24 ++++++++++++++++++++++
 .../patches/gnutls-skip-trust-store-test.patch     | 15 ++++++++++++++
 gnu/packages/tls.scm                               | 20 ++++++++++++++++++
 4 files changed, 61 insertions(+)
 create mode 100644 gnu/packages/patches/gnutls-skip-pkgconfig-test.patch
 create mode 100644 gnu/packages/patches/gnutls-skip-trust-store-test.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 686c3c639..70b4a44a1 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -640,6 +640,8 @@ dist_patch_DATA =						\
   %D%/packages/patches/gmp-faulty-test.patch			\
   %D%/packages/patches/gnome-tweak-tool-search-paths.patch	\
   %D%/packages/patches/gnucash-price-quotes-perl.patch		\
+  %D%/packages/patches/gnutls-skip-trust-store-test.patch	\
+  %D%/packages/patches/gnutls-skip-pkgconfig-test.patch		\
   %D%/packages/patches/gobject-introspection-absolute-shlib-path.patch \
   %D%/packages/patches/gobject-introspection-cc.patch		\
   %D%/packages/patches/gobject-introspection-girepository.patch	\
diff --git a/gnu/packages/patches/gnutls-skip-pkgconfig-test.patch b/gnu/packages/patches/gnutls-skip-pkgconfig-test.patch
new file mode 100644
index 000000000..1fad7c14e
--- /dev/null
+++ b/gnu/packages/patches/gnutls-skip-pkgconfig-test.patch
@@ -0,0 +1,24 @@
+FIXME: The static test fails with an error such as:
+
+/tmp/guix-build-gnutls-3.5.13.drv-0/ccOnGPmc.o: In function `main':
+c.29617.tmp.c:(.text+0x5): undefined reference to `gnutls_global_init'
+collect2: error: ld returned 1 exit status
+FAIL pkgconfig.sh (exit status: 1)
+
+diff --git a/tests/pkgconfig.sh b/tests/pkgconfig.sh
+index 6bd4e62f9..05aab8278 100755
+--- a/tests/pkgconfig.sh
++++ b/tests/pkgconfig.sh
+@@ -57,11 +57,7 @@ echo "Trying dynamic linking with:"
+ echo "  * flags: $(${PKGCONFIG} --libs gnutls)"
+ echo "  * common: ${COMMON}"
+ echo "  * lib: ${CFLAGS}"
+-cc ${TMPFILE} -o ${TMPFILE_O} $(${PKGCONFIG} --libs gnutls) $(${PKGCONFIG} --cflags gnutls) ${COMMON}
+-
+-echo ""
+-echo "Trying static linking with $(${PKGCONFIG} --libs --static gnutls)"
+-cc ${TMPFILE} -o ${TMPFILE_O} $(${PKGCONFIG} --static --libs gnutls) $(${PKGCONFIG} --cflags gnutls) ${COMMON}
++gcc ${TMPFILE} -o ${TMPFILE_O} $(${PKGCONFIG} --libs gnutls) $(${PKGCONFIG} --cflags gnutls) ${COMMON}
+ 
+ rm -f ${TMPFILE} ${TMPFILE_O}
+ 
diff --git a/gnu/packages/patches/gnutls-skip-trust-store-test.patch b/gnu/packages/patches/gnutls-skip-trust-store-test.patch
new file mode 100644
index 000000000..e0536712a
--- /dev/null
+++ b/gnu/packages/patches/gnutls-skip-trust-store-test.patch
@@ -0,0 +1,15 @@
+Version 3.5.11 added a test to check that the default trust store is readable.
+It does not exist in the build environment, so pretend everything is fine.
+
+diff a/tests/trust-store.c b/tests/trust-store.c
+--- a/tests/trust-store.c
++++ b/tests/trust-store.c
+@@ -61,7 +61,7 @@
+ 	} else if (ret < 0) {
+ 		fail("error loading system trust store: %s\n", gnutls_strerror(ret));
+ 	} else if (ret == 0) {
+-		fail("no certificates were found in system trust store!\n");
++		success("no trust store in the Guix build environment!\n");
+ 	}
+ 
+ 	gnutls_certificate_free_credentials(x509_cred);
diff --git a/gnu/packages/tls.scm b/gnu/packages/tls.scm
index 8964abb2f..69dcb015b 100644
--- a/gnu/packages/tls.scm
+++ b/gnu/packages/tls.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2016, 2017 Efraim Flashner <efraim <at> flashner.co.il>
 ;;; Copyright © 2016, 2017 ng0 <contact.ng0 <at> cryptolab.net>
 ;;; Copyright © 2016 Hartmut Goebel <h.goebel <at> crazy-compilers.com>
+;;; Copyright © 2017 Marius Bakke <mbakke <at> fastmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -142,6 +143,7 @@ living in the same process.")
 (define-public gnutls
   (package
     (name "gnutls")
+    (replacement gnutls-3.5.13)
     (version "3.5.9")
     (source (origin
              (method url-fetch)
@@ -214,6 +216,24 @@ required structures.")
     (properties '((ftp-server . "ftp.gnutls.org")
                   (ftp-directory . "/gcrypt/gnutls")))))
 
+(define gnutls-3.5.13
+  (package
+    (inherit gnutls)
+    (version "3.5.13")
+    (replacement #f)
+    (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-skip-pkgconfig-test.patch"))
+              (sha256
+               (base32
+                "15ihq6p0hnnhs8cnjrkj40dmlcaa1jjg8xg0g2ydbnlqs454ixbr"))))))
+
 (define-public gnutls/guile-2.2
   ;; GnuTLS for Guile 2.2.  This is supported by GnuTLS >= 3.5.5.
   (package
-- 
2.13.1

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 27308 <at> debbugs.gnu.org
Subject: Re: bug#27308: [PATCH] gnu: gnutls: Replace with 3.5.13.
Date: Sat, 10 Jun 2017 16:29:44 +0200

Hi Marius,

Marius Bakke <mbakke <at> fastmail.com> skribis:

> This update addresses the following security advisories:
>
> GNUTLS-SA-2017-3 (aka CVE-2017-7869) and GNUTLS-SA-2017-4.
>
> These links contain more information about the vulnerabilities and releases:
>
> https://gnutls.org/security.html
> https://gnutls.org/news.html
>
> * gnu/packages/patches/gnutls-skip-pkgconfig-test.patch,
> gnu/packages/patches/gnutls-skip-trust-store-test.patch: New files.
> * gnu/local.mk (dist_patch_DATA): Register patches.
> * gnu/packages/tls.scm (gnutls)[replacement]: New field.
> (gnutls-3.5.13): New variable.

Assuming binary compatibility, that looks good to me.

While you’re at it, could you update GnuTLS in ‘core-updates’?

For the trust-store.c test, we could ask upstream to arrange so that the
test is skipped when the trust store doesn’t exist; would the test still
make sense?

Thanks,
Ludo’.

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

From: Marius Bakke <mbakke <at> fastmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 27308 <at> debbugs.gnu.org
Subject: Re: bug#27308: [PATCH] gnu: gnutls: Replace with 3.5.13.
Date: Sat, 10 Jun 2017 18:16:07 +0200

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

Ludovic Courtès <ludo <at> gnu.org> writes:

> Hi Marius,
>
> Marius Bakke <mbakke <at> fastmail.com> skribis:
>
>> This update addresses the following security advisories:
>>
>> GNUTLS-SA-2017-3 (aka CVE-2017-7869) and GNUTLS-SA-2017-4.
>>
>> These links contain more information about the vulnerabilities and releases:
>>
>> https://gnutls.org/security.html
>> https://gnutls.org/news.html
>>
>> * gnu/packages/patches/gnutls-skip-pkgconfig-test.patch,
>> gnu/packages/patches/gnutls-skip-trust-store-test.patch: New files.
>> * gnu/local.mk (dist_patch_DATA): Register patches.
>> * gnu/packages/tls.scm (gnutls)[replacement]: New field.
>> (gnutls-3.5.13): New variable.
>
> Assuming binary compatibility, that looks good to me.

The release notes since version 3.5.9 explicitly mention no API or ABI
changes.

> While you’re at it, could you update GnuTLS in ‘core-updates’?

Indeed; that was the intention. Will merge-and-ungraft after committing.

> For the trust-store.c test, we could ask upstream to arrange so that the
> test is skipped when the trust store doesn’t exist; would the test still
> make sense?

The test *only* checks that the --default-trust-store exists. However,
the current solution is rather hacky, will check for proper skipping
mechanisms.

I tested this graft on my profile, but apparently the grafting code
checks the store item length and refuses since the .13 is one byte
longer than .9:

Backtrace:
In ice-9/boot-9.scm:
 160: 14 [catch #t #<catch-closure 8c5dc0> ...]
In unknown file:
   ?: 13 [apply-smob/1 #<catch-closure 8c5dc0>]
In ice-9/boot-9.scm:
  66: 12 [call-with-prompt prompt0 ...]
In ice-9/eval.scm:
 432: 11 [eval # #]
In ice-9/boot-9.scm:
2412: 10 [save-module-excursion #<procedure 8e6840 at ice-9/boot-9.scm:4084:3 ()>]
4089: 9 [#<procedure 8e6840 at ice-9/boot-9.scm:4084:3 ()>]
1734: 8 [%start-stack load-stack #<procedure 8f6e20 at ice-9/boot-9.scm:4080:10 ()>]
1739: 7 [#<procedure 8f8960 ()>]
In unknown file:
   ?: 6 [primitive-load "/gnu/store/aaxbysgk1j098i8i6ag24jslnizwmdlw-ffmpeg-3.3.2-guile-builder"]
In ice-9/eval.scm:
 387: 5 [eval # ()]
In ice-9/boot-9.scm:
 797: 4 [for-each #<procedure d0cd20 at ice-9/eval.scm:416:20 (a b)> # #]
In /gnu/store/9a54ididkvfkgkv7rgjw07vmdc16k9cv-module-import/guix/build/graft.scm:
 262: 3 [rewrite-directory "/gnu/store/kx3gc2swra9f2clkrgxall1bb5mcxhpc-ffmpeg-3.3.2" ...]
In srfi/srfi-1.scm:
 575: 2 [map #<procedure ae0860 at /gnu/store/9a54ididkvfkgkv7rgjw07vmdc16k9cv-module-import/guix/build/graft.scm:262:11 (expr)> ...]
In /gnu/store/9a54ididkvfkgkv7rgjw07vmdc16k9cv-module-import/guix/build/graft.scm:
 268: 1 [#<procedure ae0860 at /gnu/store/9a54ididkvfkgkv7rgjw07vmdc16k9cv-module-import/guix/build/graft.scm:262:11 (expr)> #]
In unknown file:
   ?: 0 [scm-error misc-error #f ...]

ERROR: In procedure scm-error:
ERROR: replacement length differs from the original length "56dbd2gw33g3wdxmq78lr39lamg8gxnq-gnutls-3.5.9" "78kvf0ma45z3h14850wzkcvz3zqg59xy-gnutls-3.5.13"
builder for `/gnu/store/hjzqpxdirqv5hmlyc2cg1pisnchnfisi-ffmpeg-3.3.2.drv' failed with exit code 1
cannot build derivation `/gnu/store/dn6qzxbp9xk659ypldnpgdb07fvx4343-profile.drv': 1 dependencies couldn't be built
guix package: error: build failed: build of `/gnu/store/dn6qzxbp9xk659ypldnpgdb07fvx4343-profile.drv' failed

Not sure what to do about it. Ideas?

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

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

From: Marius Bakke <mbakke <at> fastmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 27308 <at> debbugs.gnu.org
Subject: Re: bug#27308: [PATCH] gnu: gnutls: Replace with 3.5.13.
Date: Sat, 10 Jun 2017 18:37:19 +0200

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

Marius Bakke <mbakke <at> fastmail.com> writes:

> I tested this graft on my profile, but apparently the grafting code
> checks the store item length and refuses since the .13 is one byte
> longer than .9:

[...]

> ERROR: In procedure scm-error:
> ERROR: replacement length differs from the original length "56dbd2gw33g3wdxmq78lr39lamg8gxnq-gnutls-3.5.9" "78kvf0ma45z3h14850wzkcvz3zqg59xy-gnutls-3.5.13"

The attached patch allows the graft to proceed, but I'm not sure about
the sanity of it all. Thoughts?

[0001-build-graft-Allow-longer-replacement-store-names.patch (text/x-patch, attachment)]

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

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

From: Marius Bakke <mbakke <at> fastmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 27308 <at> debbugs.gnu.org
Subject: Re: bug#27308: [PATCH] gnu: gnutls: Replace with 3.5.13.
Date: Sat, 10 Jun 2017 20:07:57 +0200

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

Marius Bakke <mbakke <at> fastmail.com> writes:

> Marius Bakke <mbakke <at> fastmail.com> writes:
>
>> I tested this graft on my profile, but apparently the grafting code
>> checks the store item length and refuses since the .13 is one byte
>> longer than .9:
>
> [...]
>
>> ERROR: In procedure scm-error:
>> ERROR: replacement length differs from the original length "56dbd2gw33g3wdxmq78lr39lamg8gxnq-gnutls-3.5.9" "78kvf0ma45z3h14850wzkcvz3zqg59xy-gnutls-3.5.13"
>
> The attached patch allows the graft to proceed, but I'm not sure about
> the sanity of it all. Thoughts?

[...]

> Subject: [PATCH] build: graft: Allow longer replacement store names.

Thinking further about this, replacing a string of a fixed length with
that of another sounds highly unsafe. So I'm not sure what the best
approach here is. Maybe some dummy version number like 3.5.a? Or simply
keep 3.5.9?

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

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

From: Leo Famulari <leo <at> famulari.name>
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 27308 <at> debbugs.gnu.org
Subject: Re: bug#27308: [PATCH] gnu: gnutls: Replace with 3.5.13.
Date: Sat, 10 Jun 2017 19:05:37 -0400

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

On Sat, Jun 10, 2017 at 08:07:57PM +0200, Marius Bakke wrote:
> Thinking further about this, replacing a string of a fixed length with
> that of another sounds highly unsafe. So I'm not sure what the best
> approach here is. Maybe some dummy version number like 3.5.a? Or simply
> keep 3.5.9?

We did something similar when grafting bash [0], changing 4.4.0 to
4.4.A. It's not great, but it worked.

[0] commit 50b8a527efe375ac5377670ff0f159fbbce45312 (gnu: bash: Add
graft for patch #7 [fixes CVE-2017-5932].).

https://git.savannah.gnu.org/cgit/guix.git/commit/?id=50b8a527efe375ac5377670ff0f159fbbce45312

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

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

From: Marius Bakke <mbakke <at> fastmail.com>
To: Leo Famulari <leo <at> famulari.name>
Cc: 27308-done <at> debbugs.gnu.org
Subject: Re: bug#27308: [PATCH] gnu: gnutls: Replace with 3.5.13.
Date: Sun, 11 Jun 2017 01:25:38 +0200

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

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

> On Sat, Jun 10, 2017 at 08:07:57PM +0200, Marius Bakke wrote:
>> Thinking further about this, replacing a string of a fixed length with
>> that of another sounds highly unsafe. So I'm not sure what the best
>> approach here is. Maybe some dummy version number like 3.5.a? Or simply
>> keep 3.5.9?
>
> We did something similar when grafting bash [0], changing 4.4.0 to
> 4.4.A. It's not great, but it worked.

Ha, I already settled on 'D'. Works until version .16 ;-)

I realized 'guile2.2-gnutls' needed (replacement #f) and inheriting the
fixed sources as well. Pushed!

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

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 27308 <at> debbugs.gnu.org
Subject: Re: bug#27308: [PATCH] gnu: gnutls: Replace with 3.5.13.
Date: Sun, 11 Jun 2017 15:57:57 +0200

Marius Bakke <mbakke <at> fastmail.com> skribis:

> Marius Bakke <mbakke <at> fastmail.com> writes:
>
>> I tested this graft on my profile, but apparently the grafting code
>> checks the store item length and refuses since the .13 is one byte
>> longer than .9:

[...]

> index 16df169ec..2b3b99cb1 100644
> --- a/guix/build/graft.scm
> +++ b/guix/build/graft.scm
> @@ -263,9 +263,9 @@ file name pairs."
>               (((= hash+rest (origin-hash origin-string))
>                 .
>                 (= hash+rest (replacement-hash replacement-string)))
> -              (unless (= (string-length origin-string)
> -                         (string-length replacement-string))
> -                (error "replacement length differs from the original length"
> +              (unless (<= (string-length origin-string)
> +                          (string-length replacement-string))
> +                (error "replacement length is shorter than the original length"

That won’t work.

The workaround is to use a version string with the right length, say
“3.5.A”.  It’s enough to allow users to distinguish it from the affected
version, so that’s okay IMO.

Ludo’.

Previous Next

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