[bug#30400] [PATCH] gnu: clementine: Remove Spotify downloader.

[bug#30400] [PATCH] gnu: clementine: Remove Spotify downloader.

[Pierre Langlois]

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

Hi Guix!

I realized today that a new version of crypto++ was out so I set out to
update it but Clementine failed to compile. There's a patch on master [0]
that fixes the issue so I was going to apply it when I realized the bit
of code in question downloads a binary blob to support Spotify.  It
turns out there's a button in the preference menu that allows users to
download this proprietary blob, and crypto++ appears to be used to check
it.

So I thought that rather than apply the patch, we should probably remove
support for this download button all together. There is a flag that
allows not compiling "spotifyblobdownloader.cpp".

Also, this was also the only place in the codebase that used crypto++ so
we can drop this input.

What do you think?

[0]: https://github.com/clementine-player/Clementine/commit/38c1f8777cecc7b2f91fecbcc48c41479839304b.patch


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-clementine-Remove-Spotify-downloader.patch --]
[-- Type: text/x-patch, Size: 4380 bytes --]

From 63d16f402b1c756a650f5f6bad15b4818c747bf1 Mon Sep 17 00:00:00 2001
From: Pierre Langlois <pierre.langlois@gmx.com>
Date: Thu, 8 Feb 2018 22:03:36 +0000
Subject: [PATCH 1/2] gnu: clementine: Remove Spotify downloader.

Clementine has a button in the preference menu that allows downloading a
binary blob to add support for Spofify.  Let's remove this button.  It turns
out this is the only part of the code base that uses crypto++, let's remove
this dependency too.

* gnu/packages/music.scm (clementine)[arguments]: Remove crypto++ support. Set
HAVE_SPOTIFY_DOWNLOADER to FALSE.
[inputs]: Remove crypto++ input.
* gnu/packages/patches/clementine-remove-crypto++-dependency.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
---
 gnu/local.mk                                          |  1 +
 gnu/packages/music.scm                                | 15 +++++----------
 .../clementine-remove-crypto++-dependency.patch       | 19 +++++++++++++++++++
 3 files changed, 25 insertions(+), 10 deletions(-)
 create mode 100644 gnu/packages/patches/clementine-remove-crypto++-dependency.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 2ef483df0..06e78c2f5 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -579,6 +579,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/chmlib-inttypes.patch			\
   %D%/packages/patches/clang-libc-search-path.patch		\
   %D%/packages/patches/clang-3.8-libc-search-path.patch		\
+  %D%/packages/patches/clementine-remove-crypto++-dependency.patch	\
   %D%/packages/patches/clementine-use-openssl.patch		\
   %D%/packages/patches/clisp-remove-failing-test.patch		\
   %D%/packages/patches/clucene-pkgconfig.patch			\
diff --git a/gnu/packages/music.scm b/gnu/packages/music.scm
index 2db8ffea3..c66e2c488 100644
--- a/gnu/packages/music.scm
+++ b/gnu/packages/music.scm
@@ -226,18 +226,14 @@ score, keyboard, guitar, drum and controller views.")
                        "sha2" ;; Replaced by openssl.
                        "taglib"
                        "tinysvcmdns")))
-                (patches (search-patches "clementine-use-openssl.patch"))))
+                (patches (search-patches "clementine-use-openssl.patch"
+                                         "clementine-remove-crypto++-dependency.patch"))))
       (build-system cmake-build-system)
       (arguments
        '(#:test-target "clementine_test"
-         #:configure-flags
-         (let ((crypto (assoc-ref %build-inputs "crypto++")))
-           (list "-DENABLE_VISUALISATIONS=OFF" ; requires unpackaged "projectm"
-                 "-DCRYPTOPP_FOUND=TRUE"
-                 (string-append "-DCRYPTOPP_INCLUDE_DIRS=" crypto "/include")
-                 (string-append "-DCRYPTOPP_LIBRARY_DIRS=" crypto "/lib")
-                 (string-append "-DCRYPTOPP_LIBRARIES=" crypto "/lib/libcryptopp.a")
-                 "-DUSE_SYSTEM_SHA2=TRUE"))
+         #:configure-flags (list "-DENABLE_VISUALISATIONS=OFF" ; requires unpackaged "projectm"
+                                 "-DHAVE_SPOTIFY_DOWNLOADER=FALSE"
+                                 "-DUSE_SYSTEM_SHA2=TRUE")
          #:phases
          (modify-phases %standard-phases
            (add-after 'install 'wrap-program
@@ -254,7 +250,6 @@ score, keyboard, guitar, drum and controller views.")
       (inputs
        `(("boost" ,boost)
          ("chromaprint" ,chromaprint)
-         ("crypto++" ,crypto++)
          ("fftw" ,fftw)
          ("glib" ,glib)
          ("glu" ,glu)
diff --git a/gnu/packages/patches/clementine-remove-crypto++-dependency.patch b/gnu/packages/patches/clementine-remove-crypto++-dependency.patch
new file mode 100644
index 000000000..e7cf80dde
--- /dev/null
+++ b/gnu/packages/patches/clementine-remove-crypto++-dependency.patch
@@ -0,0 +1,19 @@
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index 3202b8b..473ecb9 100755
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -279,14 +279,6 @@ optional_component(LIBPULSE ON "Pulse audio integration"
+ 
+ optional_component(VISUALISATIONS ON "Visualisations")
+ 
+-if(NOT HAVE_SPOTIFY_BLOB AND NOT CRYPTOPP_FOUND)
+-  message(FATAL_ERROR "Either crypto++ must be available or the non-GPL Spotify "
+-          "code must be compiled in")
+-elseif(CRYPTOPP_FOUND)
+-  set(HAVE_CRYPTOPP ON)
+-  set(HAVE_SPOTIFY_DOWNLOADER ON)
+-endif()
+-
+ # Find DBus if it's enabled
+ if (HAVE_DBUS)
+   find_package(Qt5 COMPONENTS DBus)
-- 
2.16.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-gnu-crypto-Update-to-6.0.0.patch --]
[-- Type: text/x-patch, Size: 4519 bytes --]

From 9d9b90882491ff2ebf6f3a3580d4d7dbcdf57c72 Mon Sep 17 00:00:00 2001
From: Pierre Langlois <pierre.langlois@gmx.com>
Date: Thu, 8 Feb 2018 09:57:41 +0000
Subject: [PATCH 2/2] gnu: crypto++: Update to 6.0.0.

* gnu/packages/crypto.scm (crypto++): Update to 6.0.0.
* gnu/packages/patches/crypto++-fix-dos-in-asn.1-decoders.patch: Delete
file.
* gnu/local.mk (dist_patch_DATA): Remove it.
---
 gnu/local.mk                                       |  1 -
 gnu/packages/crypto.scm                            |  6 +-
 .../crypto++-fix-dos-in-asn.1-decoders.patch       | 65 ----------------------
 3 files changed, 2 insertions(+), 70 deletions(-)
 delete mode 100644 gnu/packages/patches/crypto++-fix-dos-in-asn.1-decoders.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 06e78c2f5..88d55964e 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -598,7 +598,6 @@ dist_patch_DATA =						\
   %D%/packages/patches/crawl-upgrade-saves.patch		\
   %D%/packages/patches/crda-optional-gcrypt.patch		\
   %D%/packages/patches/crossmap-allow-system-pysam.patch	\
-  %D%/packages/patches/crypto++-fix-dos-in-asn.1-decoders.patch \
   %D%/packages/patches/clucene-contribs-lib.patch               \
   %D%/packages/patches/cube-nocheck.patch			\
   %D%/packages/patches/cursynth-wave-rand.patch			\
diff --git a/gnu/packages/crypto.scm b/gnu/packages/crypto.scm
index e616c9223..a3a28fed0 100644
--- a/gnu/packages/crypto.scm
+++ b/gnu/packages/crypto.scm
@@ -581,7 +581,7 @@ data on your platform, so the seed itself will be as random as possible.
 (define-public crypto++
   (package
     (name "crypto++")
-    (version "5.6.5")
+    (version "6.0.0")
     (source (origin
               (method url-fetch/zipbomb)
               (uri (string-append "https://cryptopp.com/cryptopp"
@@ -589,9 +589,7 @@ data on your platform, so the seed itself will be as random as possible.
                                   ".zip"))
               (sha256
                (base32
-                "0d1cqdz369ivi082k59025wvxzywvkizw7i0pf5h0a1izs3g8pm7"))
-              (patches
-               (search-patches "crypto++-fix-dos-in-asn.1-decoders.patch"))))
+                "1nidm6xbdza5cbgf5md2zznmaq692rfyjasycwipl6rzdfwjvb34"))))
     (build-system gnu-build-system)
     (arguments
      `(#:make-flags
diff --git a/gnu/packages/patches/crypto++-fix-dos-in-asn.1-decoders.patch b/gnu/packages/patches/crypto++-fix-dos-in-asn.1-decoders.patch
deleted file mode 100644
index 88b2e7f25..000000000
--- a/gnu/packages/patches/crypto++-fix-dos-in-asn.1-decoders.patch
+++ /dev/null
@@ -1,65 +0,0 @@
-From 3d9181d7bdd8e491f745dbc9e34bd20b6f6da069 Mon Sep 17 00:00:00 2001
-From: Gergely Nagy <ngg@tresorit.com>
-Date: Wed, 14 Dec 2016 13:19:01 +0100
-Subject: [PATCH] Fix possible DoS in ASN.1 decoders (CVE-2016-9939)
-
----
- asn.cpp | 10 ++++++++++
- asn.h   |  2 ++
- 2 files changed, 12 insertions(+)
-
-diff --git a/asn.cpp b/asn.cpp
-index 297ff010..2e923ef7 100644
---- a/asn.cpp
-+++ b/asn.cpp
-@@ -123,6 +123,8 @@ size_t BERDecodeOctetString(BufferedTransformation &bt, SecByteBlock &str)
- 	size_t bc;

- 	if (!BERLengthDecode(bt, bc))

- 		BERDecodeError();

-+	if (bc > bt.MaxRetrievable())

-+		BERDecodeError();

- 

- 	str.New(bc);

- 	if (bc != bt.Get(str, bc))

-@@ -139,6 +141,8 @@ size_t BERDecodeOctetString(BufferedTransformation &bt, BufferedTransformation &
- 	size_t bc;

- 	if (!BERLengthDecode(bt, bc))

- 		BERDecodeError();

-+	if (bc > bt.MaxRetrievable())

-+		BERDecodeError();

- 

- 	bt.TransferTo(str, bc);

- 	return bc;

-@@ -161,6 +165,8 @@ size_t BERDecodeTextString(BufferedTransformation &bt, std::string &str, byte as
- 	size_t bc;

- 	if (!BERLengthDecode(bt, bc))

- 		BERDecodeError();

-+	if (bc > bt.MaxRetrievable())

-+		BERDecodeError();

- 

- 	SecByteBlock temp(bc);

- 	if (bc != bt.Get(temp, bc))

-@@ -188,6 +194,10 @@ size_t BERDecodeBitString(BufferedTransformation &bt, SecByteBlock &str, unsigne
- 	size_t bc;

- 	if (!BERLengthDecode(bt, bc))

- 		BERDecodeError();

-+	if (bc == 0)

-+		BERDecodeError();

-+	if (bc > bt.MaxRetrievable())

-+		BERDecodeError();

- 

- 	byte unused;

- 	if (!bt.Get(unused))

-diff --git a/asn.h b/asn.h
-index ed9de52c..33f0dd09 100644
---- a/asn.h
-+++ b/asn.h
-@@ -498,6 +498,8 @@ void BERDecodeUnsigned(BufferedTransformation &in, T &w, byte asnTag = INTEGER,
- 	bool definite = BERLengthDecode(in, bc);

- 	if (!definite)

- 		BERDecodeError();

-+	if (bc > in.MaxRetrievable())

-+		BERDecodeError();

- 

- 	SecByteBlock buf(bc);

- 

-- 
2.16.1

[bug#30400] [PATCH] gnu: clementine: Remove Spotify downloader.

[Leo Famulari]

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

On Thu, Feb 08, 2018 at 11:09:19PM +0000, Pierre Langlois wrote:
> Hi Guix!
> 
> I realized today that a new version of crypto++ was out so I set out to
> update it but Clementine failed to compile. There's a patch on master [0]
> that fixes the issue so I was going to apply it when I realized the bit
> of code in question downloads a binary blob to support Spotify.  It
> turns out there's a button in the preference menu that allows users to
> download this proprietary blob, and crypto++ appears to be used to check
> it.

Thanks for pointing this out! I reindented the #:configure-flags and
added some comments to the first patch, and pushed as
af414f34fb6e3609e572880f3b0b3bbcb1b3bfc6

My understanding of the FSDG [0] makes me think that we'll need to go
further and, in an origin snippet, remove the code performs the download
of the non-free component. This would mean that `guix build --source
clementine` would return something that follows the FSDG. CC-ing Ludo
for clarification.

[0]
https://www.gnu.org/distros/free-system-distribution-guidelines.en.html

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

[bug#30400] [PATCH] gnu: clementine: Remove Spotify downloader.

[Leo Famulari]

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

On Thu, Feb 08, 2018 at 11:09:19PM +0000, Pierre Langlois wrote:
> From 9d9b90882491ff2ebf6f3a3580d4d7dbcdf57c72 Mon Sep 17 00:00:00 2001
> From: Pierre Langlois <pierre.langlois@gmx.com>
> Date: Thu, 8 Feb 2018 09:57:41 +0000
> Subject: [PATCH 2/2] gnu: crypto++: Update to 6.0.0.
> 
> * gnu/packages/crypto.scm (crypto++): Update to 6.0.0.
> * gnu/packages/patches/crypto++-fix-dos-in-asn.1-decoders.patch: Delete
> file.
> * gnu/local.mk (dist_patch_DATA): Remove it.

I didn't push this patch yet because 1) it doesn't apply and 2) I have
to go AFK now and don't have time to make it apply. I'll do it later
today unless somebody does it first.

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

[bug#30400] [PATCH] gnu: clementine: Remove Spotify downloader.

[Ludovic Courtès]

Hello,

Leo Famulari <leo@famulari.name> skribis:

> On Thu, Feb 08, 2018 at 11:09:19PM +0000, Pierre Langlois wrote:
>> Hi Guix!
>> 
>> I realized today that a new version of crypto++ was out so I set out to
>> update it but Clementine failed to compile. There's a patch on master [0]
>> that fixes the issue so I was going to apply it when I realized the bit
>> of code in question downloads a binary blob to support Spotify.  It
>> turns out there's a button in the preference menu that allows users to
>> download this proprietary blob, and crypto++ appears to be used to check
>> it.
>
> Thanks for pointing this out! I reindented the #:configure-flags and
> added some comments to the first patch, and pushed as
> af414f34fb6e3609e572880f3b0b3bbcb1b3bfc6
>
> My understanding of the FSDG [0] makes me think that we'll need to go
> further and, in an origin snippet, remove the code performs the download
> of the non-free component. This would mean that `guix build --source
> clementine` would return something that follows the FSDG. CC-ing Ludo
> for clarification.

Correct, removal of non-free code should go in ‘snippet’ for the reason
you gave.

Thanks for looking into it,
Ludo’.

bug#30400: [PATCH] gnu: clementine: Remove Spotify downloader.

[Ludovic Courtès]

Leo Famulari <leo@famulari.name> skribis:

> On Thu, Feb 08, 2018 at 11:09:19PM +0000, Pierre Langlois wrote:
>> From 9d9b90882491ff2ebf6f3a3580d4d7dbcdf57c72 Mon Sep 17 00:00:00 2001
>> From: Pierre Langlois <pierre.langlois@gmx.com>
>> Date: Thu, 8 Feb 2018 09:57:41 +0000
>> Subject: [PATCH 2/2] gnu: crypto++: Update to 6.0.0.
>> 
>> * gnu/packages/crypto.scm (crypto++): Update to 6.0.0.
>> * gnu/packages/patches/crypto++-fix-dos-in-asn.1-decoders.patch: Delete
>> file.
>> * gnu/local.mk (dist_patch_DATA): Remove it.
>
> I didn't push this patch yet because 1) it doesn't apply and 2) I have
> to go AFK now and don't have time to make it apply. I'll do it later
> today unless somebody does it first.

Pushed as 6ca15c9ef615979862a07c02bd316e7f12a0c7ca.

Thanks!

Ludo'.