Don't try to decrypt PKCS#8 key if no password is supplied

This fixes failures with GnuTLS 3.5.4 which supports PBES1-DES-CBC-MD5
now.

Closes: #838594

3 files changed, 106 insertions, 0 deletions

diff --git a/debian/changelog b/debian/changelog
index f565e158..a65f3611 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -6,6 +6,9 @@ network-manager (1.4.0-4) UNRELEASED; urgency=medium
     Cherry-pick a couple of upstream commits which work around driver bugs
     when MAC address randomization is used. (Closes: #835822, #835553)
   * Bump debhelper compat level to 10.
+  * Don't try to decrypt PKCS#8 key if no password is supplied. This fixes
+    failures with GnuTLS 3.5.4 which supports PBES1-DES-CBC-MD5 now.
+    (Closes: #838594)
 
  -- Michael Biebl <biebl@debian.org>  Sat, 17 Sep 2016 14:51:02 +0200
 
diff --git a/debian/patches/crypto-don-t-try-to-decrypt-PKCS-8-key-if-no-password-is-.patch b/debian/patches/crypto-don-t-try-to-decrypt-PKCS-8-key-if-no-password-is-.patch
new file mode 100644
index 00000000..66fb8fa2
--- /dev/null
+++ b/debian/patches/crypto-don-t-try-to-decrypt-PKCS-8-key-if-no-password-is-.patch
@@ -0,0 +1,102 @@
+From: Beniamino Galvani <bgalvani@redhat.com>
+Date: Mon, 19 Sep 2016 09:35:23 +0200
+Subject: crypto: don't try to decrypt PKCS#8 key if no password is supplied
+
+crypto_verify_private_key_data() must try to decrypt the key only when
+a password is supplied.
+
+Previously the decrypt test always passed because we detected an
+unsupported cipher and faked success. Now since version 3.5.4 gnutls
+supports PBES1-DES-CBC-MD5 and the key is actually decrypted when a
+password is supplied.
+
+Also, don't assert that a wrong password works because we're now able
+to actually verify it (only with recent gnutls).
+
+https://bugzilla.gnome.org/show_bug.cgi?id=771623
+(cherry picked from commit 0e96d2373393f0bafc50001cfc2a3049c0454f72)
+(cherry picked from commit 626eb5df2921eecfdc75baf46df32ae69d25c44f)
+---
+ libnm-core/crypto.c            | 2 +-
+ libnm-core/crypto_gnutls.c     | 2 +-
+ libnm-core/tests/test-crypto.c | 9 ++++-----
+ libnm-util/crypto.c            | 2 +-
+ libnm-util/tests/test-crypto.c | 5 -----
+ 5 files changed, 7 insertions(+), 13 deletions(-)
+
+diff --git a/libnm-core/crypto.c b/libnm-core/crypto.c
+index c27f5c0..e734f40 100644
+--- a/libnm-core/crypto.c
++++ b/libnm-core/crypto.c
+@@ -712,7 +712,7 @@ crypto_verify_private_key_data (const guint8 *data,
+ 		/* Maybe it's PKCS#8 */
+ 		tmp = parse_pkcs8_key_file (data, data_len, &is_encrypted, NULL);
+ 		if (tmp) {
+-			if (crypto_verify_pkcs8 (tmp->data, tmp->len, is_encrypted, password, error))
++			if (!password || crypto_verify_pkcs8 (tmp->data, tmp->len, is_encrypted, password, error))
+ 				format = NM_CRYPTO_FILE_FORMAT_RAW_KEY;
+ 		} else {
+ 			char *cipher, *iv;
+diff --git a/libnm-core/crypto_gnutls.c b/libnm-core/crypto_gnutls.c
+index d09c937..53a3ba4 100644
+--- a/libnm-core/crypto_gnutls.c
++++ b/libnm-core/crypto_gnutls.c
+@@ -394,7 +394,7 @@ crypto_verify_pkcs8 (const guint8 *data,
+ 
+ 	if (err < 0) {
+ 		if (err == GNUTLS_E_UNKNOWN_CIPHER_TYPE) {
+-			/* HACK: gnutls doesn't support all the cipher types that openssl
++			/* HACK: gnutls < 3.5.4 doesn't support all the cipher types that openssl
+ 			 * can use with PKCS#8, so if we encounter one, we have to assume
+ 			 * the given password works.  gnutls needs to unsuckify, apparently.
+ 			 * Specifically, by default openssl uses pbeWithMD5AndDES-CBC
+diff --git a/libnm-core/tests/test-crypto.c b/libnm-core/tests/test-crypto.c
+index 9bab985..0c2ef48 100644
+--- a/libnm-core/tests/test-crypto.c
++++ b/libnm-core/tests/test-crypto.c
+@@ -364,12 +364,11 @@ test_pkcs8 (gconstpointer test_data)
+ 	password = parts[1];
+ 
+ 	test_is_pkcs12 (path, TRUE);
+-	test_load_pkcs8 (path, password, -1);
+-	/* Until gnutls and NSS grow support for all the ciphers that openssl
+-	 * can use with PKCS#8, we can't actually verify the password.  So we
+-	 * expect a bad password to work for the time being.
++	/* Note: NSS and gnutls < 3.5.4 don't support all the ciphers that openssl
++	 * can use with PKCS#8 and thus the password can't be actually verified with
++	 * such libraries.
+ 	 */
+-	test_load_pkcs8 (path, "blahblahblah", -1);
++	test_load_pkcs8 (path, password, -1);
+ 
+ 	g_free (path);
+ 	g_strfreev (parts);
+diff --git a/libnm-util/crypto.c b/libnm-util/crypto.c
+index 3dd89f9..9778ede 100644
+--- a/libnm-util/crypto.c
++++ b/libnm-util/crypto.c
+@@ -708,7 +708,7 @@ crypto_verify_private_key_data (const GByteArray *contents,
+ 		/* Maybe it's PKCS#8 */
+ 		tmp = parse_pkcs8_key_file (contents, &is_encrypted, error);
+ 		if (tmp) {
+-			if (crypto_verify_pkcs8 (tmp, is_encrypted, password, error))
++			if (!password || crypto_verify_pkcs8 (tmp, is_encrypted, password, error))
+ 				format = NM_CRYPTO_FILE_FORMAT_RAW_KEY;
+ 		} else {
+ 			g_clear_error (error);
+diff --git a/libnm-util/tests/test-crypto.c b/libnm-util/tests/test-crypto.c
+index 643418d..83a183d 100644
+--- a/libnm-util/tests/test-crypto.c
++++ b/libnm-util/tests/test-crypto.c
+@@ -334,11 +334,6 @@ test_pkcs8 (gconstpointer test_data)
+ 
+ 	test_is_pkcs12 (path, TRUE, "not-pkcs12");
+ 	test_load_pkcs8 (path, password, FALSE, "pkcs8-private-key");
+-	/* Until gnutls and NSS grow support for all the ciphers that openssl
+-	 * can use with PKCS#8, we can't actually verify the password.  So we
+-	 * expect a bad password to work for the time being.
+-	 */
+-	test_load_pkcs8 (path, "blahblahblah", FALSE, "pkcs8-private-key-bad-password");
+ 
+ 	g_free (path);
+ 	g_strfreev (parts);
diff --git a/debian/patches/series b/debian/patches/series
index eafd0374..5fbcbc0e 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -12,3 +12,4 @@ device-add-hack-to-wait-after-changing-MAC-address.patch
 shared-add-NM_MIN-NM_MAX-macros-to-replace-glib-s-MIN-MAX.patch
 device-workaround-driver-issue-with-delayed-change-of-MAC.patch
 device-wait-for-MAC-address-change-to-complete-before-set.patch
+crypto-don-t-try-to-decrypt-PKCS-8-key-if-no-password-is-.patch