Patchwork OpenSSL 1.1.0c security update required

login
register
mail settings
Submitter Marius Bakke
Date Dec. 9, 2016, 10:11 a.m.
Message ID <87fulxxvic.fsf@kirby.i-did-not-set--mail-host-address--so-tickle-me>
Download mbox | patch
Permalink /patch/18314/
State New
Headers show

Comments

Marius Bakke - Dec. 9, 2016, 10:11 a.m.
Leo Famulari <leo@famulari.name> writes:

> On Sat, Nov 12, 2016 at 12:21:44PM +0100, Ludovic Courtès wrote:
>> Leo Famulari <leo@famulari.name> skribis:
>> > They changed how library runpaths are recorded at build time, and so our
>> > packaging no longer works:
>> >
>> > https://github.com/openssl/openssl/pull/1699
>> 
>> I would expect ld-wrapper to do the right thing regardless of what
>> OpenSSL’s build system does, no?
>
> So far, we've had to do some extra work in the openssl-next package
> definition:
>
> (add-after 'configure 'patch-runpath
>   (lambda* (#:key outputs #:allow-other-keys)
>     (let ((lib (string-append (assoc-ref outputs "out") "/lib")))
>       (substitute* "Makefile.shared"
>         (("\\$\\$\\{SHAREDCMD\\} \\$\\$\\{SHAREDFLAGS\\}")
>          (string-append "$${SHAREDCMD} $${SHAREDFLAGS}"
>                         " -Wl,-rpath," lib)))
>       #t)))))))))
>
> This phase still works to help OpenSSL's libraries find the libssl and
> libcrypto libraries, but the OpenSSL executable itself now lacks a
> reference to those libraries:
>
> starting phase `validate-runpath'
> validating RUNPATH of 5 binaries in "/gnu/store/d4669fp9lhvi85i97kbhwk3xprgqpv6v-openssl-1.1.0c/lib"...
> validating RUNPATH of 1 binaries in "/gnu/store/d4669fp9lhvi85i97kbhwk3xprgqpv6v-openssl-1.1.0c/bin"...
> /gnu/store/d4669fp9lhvi85i97kbhwk3xprgqpv6v-openssl-1.1.0c/bin/openssl: error: depends on 'libssl.so.1.1', which cannot be found in RUNPATH ("/gnu/store/iwgi9001dmmihrjg4rqhd6pa6788prjw-glibc-2.24/lib" "/gnu/store/cdi08kw7r6r684w8mk0xq0dkgpjhfpmd-gcc-4.9.4-lib/lib" "/gnu/store/cdi08kw7r6r684w8mk0xq0dkgpjhfpmd-gcc-4.9.4-lib/lib/gcc/x86_64-unknown-linux-gnu/4.9.4/../../..")
> /gnu/store/d4669fp9lhvi85i97kbhwk3xprgqpv6v-openssl-1.1.0c/bin/openssl: error: depends on 'libcrypto.so.1.1', which cannot be found in RUNPATH ("/gnu/store/iwgi9001dmmihrjg4rqhd6pa6788prjw-glibc-2.24/lib" "/gnu/store/cdi08kw7r6r684w8mk0xq0dkgpjhfpmd-gcc-4.9.4-lib/lib" "/gnu/store/cdi08kw7r6r684w8mk0xq0dkgpjhfpmd-gcc-4.9.4-lib/lib/gcc/x86_64-unknown-linux-gnu/4.9.4/../../..")
> validating RUNPATH of 0 binaries in "/gnu/store/wdzvwl9kx3iiq4fk2qyxg7sjxqq2qx3x-openssl-1.1.0c-static/lib"...
> phase `validate-runpath' failed after 0.0 seconds
> builder for `/gnu/store/ja9xpivxkfvbm2p6zs1vicdkk4ppq1is-openssl-1.1.0c.drv' failed with exit code 1
>
> Upstream discussion:
> https://github.com/openssl/openssl/pull/1688
> https://github.com/openssl/openssl/pull/1699
>
> My understanding is that this change was made for openssl@1.0.2 as well,
> so we will need to address it for the next big OpenSSL update. We should
> try packaging a Git snapshot of 1.0.2 now, so that we are ready.

I did this change for openssl@1.1.0 (attached). The 'config(ure)' script
now takes a -rpath flag which works as advertised.

However by duplicating the 'configure' phase, I discovered that the
'version' variable actually gets the inherited value when using
'substitute-keyword-arguments', and had to duplicate the
'remove-miscellany' phase as well, since it tried deleting a directory
called '$out/share/openssl-1.0.2j'. Should I file a bug for this, or
is it something intrinsically unfixable?
Leo Famulari - Dec. 10, 2016, 1:21 a.m.
On Fri, Dec 09, 2016 at 11:11:07AM +0100, Marius Bakke wrote:
> I did this change for openssl@1.1.0 (attached). The 'config(ure)' script
> now takes a -rpath flag which works as advertised.

Thanks for taking this on!

> However by duplicating the 'configure' phase, I discovered that the
> 'version' variable actually gets the inherited value when using
> 'substitute-keyword-arguments', and had to duplicate the
> 'remove-miscellany' phase as well, since it tried deleting a directory
> called '$out/share/openssl-1.0.2j'. Should I file a bug for this, or
> is it something intrinsically unfixable?

I don't know, but the patch looks good enough to me, so can you go ahead
and push it? We can deduplicate the phases later.

> From 2fa175873823afb4b2e05c9ed26772c900a2f5ef Mon Sep 17 00:00:00 2001
> From: Marius Bakke <mbakke@fastmail.com>
> Date: Fri, 9 Dec 2016 09:48:38 +0100
> Subject: [PATCH] gnu: openssl-next: Update to 1.1.0c [fixes
>  CVE-{7053,7054,7055}].
> 
> * gnu/packages/tls.scm (openssl-next): Update to 1.1.0c.
> [arguments]: Duplicate 'configure' to add rpath flag previously handled by
> now-defunct 'patch-runpath' phase. Duplicate 'remove-miscellany' phase.

[...]

> +           (replace 'configure
>               (lambda* (#:key outputs #:allow-other-keys)
> -               (let ((lib (string-append (assoc-ref outputs "out") "/lib")))
> -                 (substitute* "Makefile.shared"
> -                   (("\\$\\$\\{SHAREDCMD\\} \\$\\$\\{SHAREDFLAGS\\}")
> -                    (string-append "$${SHAREDCMD} $${SHAREDFLAGS}"
> -                                   " -Wl,-rpath," lib)))
> +               (let* ((out (assoc-ref outputs "out"))
> +                      (lib (string-append out "/lib")))
> +                 (zero?
> +                  (system* "./config"
> +                           "shared"                   ;build shared libraries
> +                           "--libdir=lib"
> +
> +                           ;; The default for this catch-all directory is
> +                           ;; PREFIX/ssl.  Change that to something more
> +                           ;; conventional.
> +                           (string-append "--openssldir=" out
> +                                          "/share/openssl-" ,version)
> +
> +                           (string-append "--prefix=" out)
> +
> +                           (string-append "-Wl,-rpath," lib)

This is much clearer than patching the Makefile!

Patch

From 2fa175873823afb4b2e05c9ed26772c900a2f5ef Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@fastmail.com>
Date: Fri, 9 Dec 2016 09:48:38 +0100
Subject: [PATCH] gnu: openssl-next: Update to 1.1.0c [fixes
 CVE-{7053,7054,7055}].

* gnu/packages/tls.scm (openssl-next): Update to 1.1.0c.
[arguments]: Duplicate 'configure' to add rpath flag previously handled by
now-defunct 'patch-runpath' phase. Duplicate 'remove-miscellany' phase.
---
 gnu/packages/tls.scm | 46 ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/gnu/packages/tls.scm b/gnu/packages/tls.scm
index 854ba1cb4..b581d59ce 100644
--- a/gnu/packages/tls.scm
+++ b/gnu/packages/tls.scm
@@ -355,7 +355,7 @@  required structures.")
   (package
     (inherit openssl)
     (name "openssl")
-    (version "1.1.0b")
+    (version "1.1.0c")
     (source (origin
              (method url-fetch)
              (uri (list (string-append "ftp://ftp.openssl.org/source/"
@@ -366,7 +366,7 @@  required structures.")
               (patches (search-patches "openssl-1.1.0-c-rehash-in.patch"))
               (sha256
                (base32
-                "1xznrqvb1dbngv2k2nb6da6fdw00c01sy2i36yjdxr4vpxrf0pd4"))))
+                "1xfn5ydl14myd9wgxm4nxy5a42cpp1g12ijf3g9m4mz0l90n8hzw"))))
     (outputs '("out"
                "doc"        ;1.3MiB of man3 pages
                "static"))   ; 5.5MiB of .a files
@@ -377,13 +377,43 @@  required structures.")
            (delete 'patch-tests)          ; These two phases are not needed by
            (delete 'patch-Makefile.org)   ; OpenSSL 1.1.0.
 
-           (add-after 'configure 'patch-runpath
+           ;; Override configure phase since -rpath is now a configure option.
+           (replace 'configure
              (lambda* (#:key outputs #:allow-other-keys)
-               (let ((lib (string-append (assoc-ref outputs "out") "/lib")))
-                 (substitute* "Makefile.shared"
-                   (("\\$\\$\\{SHAREDCMD\\} \\$\\$\\{SHAREDFLAGS\\}")
-                    (string-append "$${SHAREDCMD} $${SHAREDFLAGS}"
-                                   " -Wl,-rpath," lib)))
+               (let* ((out (assoc-ref outputs "out"))
+                      (lib (string-append out "/lib")))
+                 (zero?
+                  (system* "./config"
+                           "shared"                   ;build shared libraries
+                           "--libdir=lib"
+
+                           ;; The default for this catch-all directory is
+                           ;; PREFIX/ssl.  Change that to something more
+                           ;; conventional.
+                           (string-append "--openssldir=" out
+                                          "/share/openssl-" ,version)
+
+                           (string-append "--prefix=" out)
+
+                           (string-append "-Wl,-rpath," lib)
+
+                           ;; XXX FIXME: Work around a code generation bug in GCC
+                           ;; 4.9.3 on ARM when compiled with -mfpu=neon.  See:
+                           ;; <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66917>
+                           ,@(if (and (not (%current-target-system))
+                                      (string-prefix? "armhf" (%current-system)))
+                                 '("-mfpu=vfpv3")
+                                 '()))))))
+
+           ;; XXX: Duplicate this phase since 'version' evaluates to the
+           ;; inherited package when using substitute-keyword-arguments.
+           (replace 'remove-miscellany
+             (lambda* (#:key outputs #:allow-other-keys)
+               ;; The 'misc' directory contains random undocumented shell and Perl
+               ;; scripts.  Remove them to avoid retaining a reference on Perl.
+               (let ((out (assoc-ref outputs "out")))
+                 (delete-file-recursively (string-append out "/share/openssl-"
+                                                         ,version "/misc"))
                  #t)))))))))
 
 (define-public libressl
-- 
2.11.0