From 0e96d2373393f0bafc50001cfc2a3049c0454f72 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani 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 --- 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 c27f5c0dc9..e734f40d89 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 d09c937979..53a3ba4adf 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 9bab985f72..0c2ef48a51 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 3dd89f951a..9778edebcb 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 643418d2fb..83a183dd18 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); -- cgit v1.2.1