From 926f4e1473ca7ee4a5f786fcd20cce52afb7a955 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Mon, 1 Dec 2014 12:17:39 -0500 Subject: [PATCH 1/9] libnm: drop nm_utils_deinit() It was a no-op anyway. --- libnm-core/crypto.h | 2 -- libnm-core/crypto_gnutls.c | 5 ----- libnm-core/crypto_nss.c | 5 ----- libnm-core/nm-utils.c | 23 ++--------------------- libnm-core/nm-utils.h | 3 +-- libnm-core/tests/test-crypto.c | 2 -- libnm/libnm.ver | 1 - 7 files changed, 3 insertions(+), 38 deletions(-) diff --git a/libnm-core/crypto.h b/libnm-core/crypto.h index b7b7dc4ae7..e98f31d923 100644 --- a/libnm-core/crypto.h +++ b/libnm-core/crypto.h @@ -46,8 +46,6 @@ typedef enum { gboolean crypto_init (GError **error); -void crypto_deinit (void); - GByteArray *crypto_decrypt_openssl_private_key_data (const guint8 *data, gsize data_len, const char *password, diff --git a/libnm-core/crypto_gnutls.c b/libnm-core/crypto_gnutls.c index e123830d2c..dab06debd7 100644 --- a/libnm-core/crypto_gnutls.c +++ b/libnm-core/crypto_gnutls.c @@ -56,11 +56,6 @@ crypto_init (GError **error) return TRUE; } -void -crypto_deinit (void) -{ -} - gboolean crypto_md5_hash (const char *salt, const gsize salt_len, diff --git a/libnm-core/crypto_nss.c b/libnm-core/crypto_nss.c index e7cf2eb030..6ab935c3a2 100644 --- a/libnm-core/crypto_nss.c +++ b/libnm-core/crypto_nss.c @@ -72,11 +72,6 @@ crypto_init (GError **error) return TRUE; } -void -crypto_deinit (void) -{ -} - gboolean crypto_md5_hash (const char *salt, const gsize salt_len, diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index f1f5a9e8ca..f3187b6f01 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -210,7 +210,7 @@ get_encodings_for_lang (const char *lang, return success; } -/* init, deinit for libnm_util */ +/* init libnm */ static void __attribute__((constructor)) _check_symbols (void) @@ -231,9 +231,7 @@ static gboolean initialized = FALSE; * @error: location to store error, or %NULL * * Initializes libnm; should be called when starting and program that - * uses libnm. Sets up an atexit() handler to ensure de-initialization - * is performed, but calling nm_utils_deinit() to explicitly deinitialize - * libnm can also be done. This function can be called more than once. + * uses libnm. This function can be called more than once. * * Returns: %TRUE if the initialization was successful, %FALSE on failure. **/ @@ -254,23 +252,6 @@ nm_utils_init (GError **error) return TRUE; } -/** - * nm_utils_deinit: - * - * Frees all resources used internally by libnm. This function is called - * from an atexit() handler, set up by nm_utils_init(), but is safe to be called - * more than once. Subsequent calls have no effect until nm_utils_init() is - * called again. - **/ -void -nm_utils_deinit (void) -{ - if (initialized) { - crypto_deinit (); - initialized = FALSE; - } -} - gboolean _nm_utils_is_manager_process; /* ssid helpers */ diff --git a/libnm-core/nm-utils.h b/libnm-core/nm-utils.h index dcfae00e09..013e26ca4e 100644 --- a/libnm-core/nm-utils.h +++ b/libnm-core/nm-utils.h @@ -39,9 +39,8 @@ G_BEGIN_DECLS -/* init, deinit nm_utils */ +/* init libnm */ gboolean nm_utils_init (GError **error); -void nm_utils_deinit (void); /* SSID helpers */ gboolean nm_utils_is_empty_ssid (const guint8 *ssid, gsize len); diff --git a/libnm-core/tests/test-crypto.c b/libnm-core/tests/test-crypto.c index b9e958d7b8..2fab2fec69 100644 --- a/libnm-core/tests/test-crypto.c +++ b/libnm-core/tests/test-crypto.c @@ -462,8 +462,6 @@ main (int argc, char **argv) ret = g_test_run (); - crypto_deinit (); - return ret; } diff --git a/libnm/libnm.ver b/libnm/libnm.ver index d81a24c7eb..6dab82052a 100644 --- a/libnm/libnm.ver +++ b/libnm/libnm.ver @@ -755,7 +755,6 @@ global: nm_utils_ap_mode_security_valid; nm_utils_bin2hexstr; nm_utils_check_virtual_device_compatibility; - nm_utils_deinit; nm_utils_escape_ssid; nm_utils_file_is_certificate; nm_utils_file_is_pkcs12; From 539fac8b675e6a9e5bbb5374e32e3d7615fcf0b7 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 2 Dec 2014 09:26:39 -0500 Subject: [PATCH 2/9] libnm-util: Note that nm_utils_deinit() is a no-op nm_utils_deinit() is a no-op, so don't suggest that people need to call it. --- libnm-util/crypto.h | 2 -- libnm-util/crypto_gnutls.c | 5 ----- libnm-util/crypto_nss.c | 5 ----- libnm-util/nm-utils.c | 16 ++++------------ libnm-util/tests/test-crypto.c | 7 +------ 5 files changed, 5 insertions(+), 30 deletions(-) diff --git a/libnm-util/crypto.h b/libnm-util/crypto.h index edb79007f1..315d8fec83 100644 --- a/libnm-util/crypto.h +++ b/libnm-util/crypto.h @@ -70,8 +70,6 @@ GQuark _nm_crypto_error_quark (void); gboolean crypto_init (GError **error); -void crypto_deinit (void); - GByteArray *crypto_decrypt_private_key_data (const GByteArray *contents, const char *password, NMCryptoKeyType *out_key_type, diff --git a/libnm-util/crypto_gnutls.c b/libnm-util/crypto_gnutls.c index 03cb1b4761..3bec24ad67 100644 --- a/libnm-util/crypto_gnutls.c +++ b/libnm-util/crypto_gnutls.c @@ -56,11 +56,6 @@ crypto_init (GError **error) return TRUE; } -void -crypto_deinit (void) -{ -} - gboolean crypto_md5_hash (const char *salt, const gsize salt_len, diff --git a/libnm-util/crypto_nss.c b/libnm-util/crypto_nss.c index 52ee261d4a..edd19b54fe 100644 --- a/libnm-util/crypto_nss.c +++ b/libnm-util/crypto_nss.c @@ -71,11 +71,6 @@ crypto_init (GError **error) return TRUE; } -void -crypto_deinit (void) -{ -} - gboolean crypto_md5_hash (const char *salt, const gsize salt_len, diff --git a/libnm-util/nm-utils.c b/libnm-util/nm-utils.c index 371bb49ca8..9735394275 100644 --- a/libnm-util/nm-utils.c +++ b/libnm-util/nm-utils.c @@ -222,10 +222,8 @@ static gboolean initialized = FALSE; * nm_utils_init: * @error: location to store error, or %NULL * - * Initializes libnm-util; should be called when starting and program that - * uses libnm-util. Sets up an atexit() handler to ensure de-initialization - * is performed, but calling nm_utils_deinit() to explicitly deinitialize - * libnm-util can also be done. This function can be called more than once. + * Initializes libnm-util; should be called when starting any program that + * uses libnm-util. This function can be called more than once. * * Returns: %TRUE if the initialization was successful, %FALSE on failure. **/ @@ -249,18 +247,12 @@ nm_utils_init (GError **error) /** * nm_utils_deinit: * - * Frees all resources used internally by libnm-util. This function is called - * from an atexit() handler, set up by nm_utils_init(), but is safe to be called - * more than once. Subsequent calls have no effect until nm_utils_init() is - * called again. + * No-op. Although this function still exists for ABI compatibility reasons, it + * does not have any effect, and does not ever need to be called. **/ void nm_utils_deinit (void) { - if (initialized) { - crypto_deinit (); - initialized = FALSE; - } } /* ssid helpers */ diff --git a/libnm-util/tests/test-crypto.c b/libnm-util/tests/test-crypto.c index 9be9f2ae56..4bf2b3be74 100644 --- a/libnm-util/tests/test-crypto.c +++ b/libnm-util/tests/test-crypto.c @@ -412,7 +412,6 @@ int main (int argc, char **argv) { GError *error = NULL; - int ret; nmtst_init (&argc, &argv, TRUE); @@ -462,10 +461,6 @@ main (int argc, char **argv) "pkcs8-enc-key.pem, 1234567890", test_pkcs8); - ret = g_test_run (); - - crypto_deinit (); - - return ret; + return g_test_run (); } From 34519eee136ef95666f77482315edde9d48250e5 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Mon, 1 Dec 2014 13:37:43 -0500 Subject: [PATCH 3/9] tests: add a test of libnm-core's crypto_md5_hash() --- libnm-core/tests/test-crypto.c | 54 ++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/libnm-core/tests/test-crypto.c b/libnm-core/tests/test-crypto.c index 2fab2fec69..afc91a178e 100644 --- a/libnm-core/tests/test-crypto.c +++ b/libnm-core/tests/test-crypto.c @@ -401,6 +401,58 @@ test_pkcs8 (gconstpointer test_data) g_strfreev (parts); } +#define SALT "sodium chloride" +#define SHORT_PASSWORD "short" +#define LONG_PASSWORD "this is a longer password than the short one" +#define SHORT_DIGEST 16 +#define LONG_DIGEST 57 + +struct { + const char *salt, *password; + gsize digest_size; + const char *result; +} md5_tests[] = { + { NULL, SHORT_PASSWORD, SHORT_DIGEST, + "4f09daa9d95bcb166a302407a0e0babe" }, + { NULL, SHORT_PASSWORD, LONG_DIGEST, + "4f09daa9d95bcb166a302407a0e0babeb7d62e5baf706830d007c253f0fe7584ad7e92dc00a599ec277293c298ae70ee3904c348e23be61c91" }, + { SALT, SHORT_PASSWORD, SHORT_DIGEST, + "774771f7292210233b5724991d1f9894" }, + { SALT, SHORT_PASSWORD, LONG_DIGEST, + "774771f7292210233b5724991d1f98941a6ffdb45e4dc7fa04b1fa6aceed379c1ade0577bc8f261d109942ed5736921c052664d72e0d5bade9" }, + { NULL, LONG_PASSWORD, SHORT_DIGEST, + "e9c03517f81ff29bb777dac21fb1699c" }, + { NULL, LONG_PASSWORD, LONG_DIGEST, + "e9c03517f81ff29bb777dac21fb1699c50968c7ccd8db4f0a59d00ffd87b05876d45f25a927d51a8400c35af60fbd64584349a8b7435d62fd9" }, + { SALT, LONG_PASSWORD, SHORT_DIGEST, + "4e5c076e2f85f5e03994acbf3a9e10d6" }, + { SALT, LONG_PASSWORD, LONG_DIGEST, + "4e5c076e2f85f5e03994acbf3a9e10d61a6969c9fdf47ae8b1f7e2725b3767b05cc974bfcb5344b630c91761e015e09d7794b5065662533bc9" }, +}; + +static void +test_md5 (void) +{ + char digest[LONG_DIGEST], *hex; + int i; + GError *error = NULL; + + for (i = 0; i < G_N_ELEMENTS (md5_tests); i++) { + memset (digest, 0, sizeof (digest)); + crypto_md5_hash (md5_tests[i].salt, + md5_tests[i].salt ? strlen (md5_tests[i].salt) : 0, + md5_tests[i].password, + strlen (md5_tests[i].password), + digest, md5_tests[i].digest_size, + &error); + g_assert_no_error (error); + + hex = nm_utils_bin2hexstr (digest, md5_tests[i].digest_size, -1); + g_assert_cmpstr (hex, ==, md5_tests[i].result); + g_free (hex); + } +} + NMTST_DEFINE (); int @@ -460,6 +512,8 @@ main (int argc, char **argv) "pkcs8-enc-key.pem, 1234567890", test_pkcs8); + g_test_add_func ("/libnm/crypto/md5", test_md5); + ret = g_test_run (); return ret; From 48ff21b5bc42daa8b6f72db8d82fd9b21fde842e Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Mon, 1 Dec 2014 13:46:14 -0500 Subject: [PATCH 4/9] libnm-core: reimplement crypto_md5_hash() using GChecksum Reimplement crypto_md5_hash() using GChecksum. Remove the gboolean return value and GError argument, since it cannot fail now. --- libnm-core/crypto.c | 70 ++++++++++++++++++++++++++-------- libnm-core/crypto.h | 13 +++---- libnm-core/crypto_gnutls.c | 57 --------------------------- libnm-core/crypto_nss.c | 55 -------------------------- libnm-core/nm-utils.c | 37 +++--------------- libnm-core/tests/test-crypto.c | 5 +-- 6 files changed, 66 insertions(+), 171 deletions(-) diff --git a/libnm-core/crypto.c b/libnm-core/crypto.c index 592f4e41b5..086f1d6714 100644 --- a/libnm-core/crypto.c +++ b/libnm-core/crypto.c @@ -404,25 +404,15 @@ make_des_aes_key (const char *cipher, key = g_malloc0 (digest_len + 1); - if (!crypto_md5_hash (salt, - salt_len, - password, - strlen (password), - key, - digest_len, - error)) - goto error; + crypto_md5_hash (salt, + salt_len, + password, + strlen (password), + key, + digest_len); *out_len = digest_len; return key; - -error: - if (key) { - /* Don't leak stale key material */ - memset (key, 0, digest_len); - g_free (key); - } - return NULL; } static GByteArray * @@ -746,3 +736,51 @@ crypto_verify_private_key (const char *filename, } return format; } + +void +crypto_md5_hash (const char *salt, + const gsize salt_len, + const char *password, + gsize password_len, + char *buffer, + gsize buflen) +{ + GChecksum *ctx; + int nkey = buflen; + gsize digest_len; + int count = 0; + char digest[16]; + char *p = buffer; + + g_assert_cmpint (g_checksum_type_get_length (G_CHECKSUM_MD5), ==, sizeof (digest)); + + if (salt) + g_return_if_fail (salt_len >= 8); + + g_return_if_fail (password != NULL); + g_return_if_fail (password_len > 0); + g_return_if_fail (buffer != NULL); + g_return_if_fail (buflen > 0); + + ctx = g_checksum_new (G_CHECKSUM_MD5); + + while (nkey > 0) { + int i = 0; + + g_checksum_reset (ctx); + if (count++) + g_checksum_update (ctx, (const guchar *) digest, digest_len); + g_checksum_update (ctx, (const guchar *) password, password_len); + if (salt) + g_checksum_update (ctx, (const guchar *) salt, 8); /* Only use 8 bytes of salt */ + g_checksum_get_digest (ctx, (guchar *) digest, &digest_len); + + while (nkey && (i < digest_len)) { + *(p++) = digest[i++]; + nkey--; + } + } + + memset (digest, 0, sizeof (digest)); + g_checksum_free (ctx); +} diff --git a/libnm-core/crypto.h b/libnm-core/crypto.h index e98f31d923..392ec57481 100644 --- a/libnm-core/crypto.h +++ b/libnm-core/crypto.h @@ -78,13 +78,12 @@ NMCryptoFileFormat crypto_verify_private_key (const char *file, /* Internal utils API bits for crypto providers */ -gboolean crypto_md5_hash (const char *salt, - const gsize salt_len, - const char *password, - gsize password_len, - char *buffer, - gsize buflen, - GError **error); +void crypto_md5_hash (const char *salt, + const gsize salt_len, + const char *password, + gsize password_len, + char *buffer, + gsize buflen); char * crypto_decrypt (const char *cipher, int key_type, diff --git a/libnm-core/crypto_gnutls.c b/libnm-core/crypto_gnutls.c index dab06debd7..03923f6307 100644 --- a/libnm-core/crypto_gnutls.c +++ b/libnm-core/crypto_gnutls.c @@ -56,63 +56,6 @@ crypto_init (GError **error) return TRUE; } -gboolean -crypto_md5_hash (const char *salt, - const gsize salt_len, - const char *password, - gsize password_len, - char *buffer, - gsize buflen, - GError **error) -{ - gcry_md_hd_t ctx; - gcry_error_t err; - int nkey = buflen; - const gsize digest_len = 16; - int count = 0; - char digest[MD5_HASH_LEN]; - char *p = buffer; - - if (salt) - g_return_val_if_fail (salt_len >= SALT_LEN, FALSE); - - g_return_val_if_fail (password != NULL, FALSE); - g_return_val_if_fail (password_len > 0, FALSE); - g_return_val_if_fail (buffer != NULL, FALSE); - g_return_val_if_fail (buflen > 0, FALSE); - - err = gcry_md_open (&ctx, GCRY_MD_MD5, 0); - if (err) { - g_set_error (error, NM_CRYPTO_ERROR, - NM_CRYPTO_ERROR_FAILED, - _("Failed to initialize the MD5 engine: %s / %s."), - gcry_strsource (err), gcry_strerror (err)); - return FALSE; - } - - while (nkey > 0) { - int i = 0; - - if (count++) - gcry_md_write (ctx, digest, digest_len); - gcry_md_write (ctx, password, password_len); - if (salt) - gcry_md_write (ctx, salt, SALT_LEN); /* Only use 8 bytes of salt */ - gcry_md_final (ctx); - memcpy (digest, gcry_md_read (ctx, 0), digest_len); - gcry_md_reset (ctx); - - while (nkey && (i < digest_len)) { - *(p++) = digest[i++]; - nkey--; - } - } - - memset (digest, 0, sizeof (digest)); - gcry_md_close (ctx); - return TRUE; -} - char * crypto_decrypt (const char *cipher, int key_type, diff --git a/libnm-core/crypto_nss.c b/libnm-core/crypto_nss.c index 6ab935c3a2..4dd85f3bdc 100644 --- a/libnm-core/crypto_nss.c +++ b/libnm-core/crypto_nss.c @@ -72,61 +72,6 @@ crypto_init (GError **error) return TRUE; } -gboolean -crypto_md5_hash (const char *salt, - const gsize salt_len, - const char *password, - gsize password_len, - char *buffer, - gsize buflen, - GError **error) -{ - PK11Context *ctx; - int nkey = buflen; - unsigned int digest_len; - int count = 0; - char digest[MD5_HASH_LEN]; - char *p = buffer; - - if (salt) - g_return_val_if_fail (salt_len >= 8, FALSE); - - g_return_val_if_fail (password != NULL, FALSE); - g_return_val_if_fail (password_len > 0, FALSE); - g_return_val_if_fail (buffer != NULL, FALSE); - g_return_val_if_fail (buflen > 0, FALSE); - - ctx = PK11_CreateDigestContext (SEC_OID_MD5); - if (!ctx) { - g_set_error (error, NM_CRYPTO_ERROR, - NM_CRYPTO_ERROR_FAILED, - _("Failed to initialize the MD5 context: %d."), - PORT_GetError ()); - return FALSE; - } - - while (nkey > 0) { - int i = 0; - - PK11_DigestBegin (ctx); - if (count++) - PK11_DigestOp (ctx, (const unsigned char *) digest, digest_len); - PK11_DigestOp (ctx, (const unsigned char *) password, password_len); - if (salt) - PK11_DigestOp (ctx, (const unsigned char *) salt, 8); /* Only use 8 bytes of salt */ - PK11_DigestFinal (ctx, (unsigned char *) digest, &digest_len, sizeof (digest)); - - while (nkey && (i < digest_len)) { - *(p++) = digest[i++]; - nkey--; - } - } - - memset (digest, 0, sizeof (digest)); - PK11_DestroyContext (ctx, PR_TRUE); - return TRUE; -} - char * crypto_decrypt (const char *cipher, int key_type, diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index f3187b6f01..8f63a51988 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -1976,33 +1976,15 @@ nm_utils_uuid_generate (void) char * nm_utils_uuid_generate_from_string (const char *s) { - GError *error = NULL; uuid_t *uuid; char *buf = NULL; - if (!nm_utils_init (&error)) { - g_warning ("error initializing crypto: (%d) %s", - error ? error->code : 0, - error ? error->message : "unknown"); - if (error) - g_error_free (error); - return NULL; - } - uuid = g_malloc0 (sizeof (*uuid)); - if (!crypto_md5_hash (NULL, 0, s, strlen (s), (char *) uuid, sizeof (*uuid), &error)) { - g_warning ("error generating UUID: (%d) %s", - error ? error->code : 0, - error ? error->message : "unknown"); - if (error) - g_error_free (error); - goto out; - } + crypto_md5_hash (NULL, 0, s, strlen (s), (char *) uuid, sizeof (*uuid)); buf = g_malloc0 (37); uuid_unparse_lower (*uuid, &buf[0]); -out: g_free (uuid); return buf; } @@ -2012,8 +1994,7 @@ make_key (const char *cipher, const char *salt, const gsize salt_len, const char *password, - gsize *out_len, - GError **error) + gsize *out_len) { char *key; guint32 digest_len = 24; /* DES-EDE3-CBC */ @@ -2030,13 +2011,8 @@ make_key (const char *cipher, key = g_malloc0 (digest_len + 1); - if (!crypto_md5_hash (salt, salt_len, password, strlen (password), key, digest_len, error)) { - *out_len = 0; - memset (key, 0, digest_len); - g_free (key); - key = NULL; - } else - *out_len = digest_len; + crypto_md5_hash (salt, salt_len, password, strlen (password), key, digest_len); + *out_len = digest_len; return key; } @@ -2097,10 +2073,7 @@ nm_utils_rsa_key_encrypt_helper (const char *cipher, if (!crypto_randomize (salt, salt_len, error)) goto out; - key = make_key (cipher, &salt[0], salt_len, in_password, &key_len, error); - if (!key) - goto out; - + key = make_key (cipher, &salt[0], salt_len, in_password, &key_len); enc = crypto_encrypt (cipher, data, len, salt, salt_len, key, key_len, &enc_len, error); if (!enc) goto out; diff --git a/libnm-core/tests/test-crypto.c b/libnm-core/tests/test-crypto.c index afc91a178e..a5513122b5 100644 --- a/libnm-core/tests/test-crypto.c +++ b/libnm-core/tests/test-crypto.c @@ -435,7 +435,6 @@ test_md5 (void) { char digest[LONG_DIGEST], *hex; int i; - GError *error = NULL; for (i = 0; i < G_N_ELEMENTS (md5_tests); i++) { memset (digest, 0, sizeof (digest)); @@ -443,9 +442,7 @@ test_md5 (void) md5_tests[i].salt ? strlen (md5_tests[i].salt) : 0, md5_tests[i].password, strlen (md5_tests[i].password), - digest, md5_tests[i].digest_size, - &error); - g_assert_no_error (error); + digest, md5_tests[i].digest_size); hex = nm_utils_bin2hexstr (digest, md5_tests[i].digest_size, -1); g_assert_cmpstr (hex, ==, md5_tests[i].result); From bddc0de51e519e7b4622beb1db8d0a3af2da166e Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Mon, 1 Dec 2014 14:13:06 -0500 Subject: [PATCH 5/9] libnm-core: call crypto_init() on the fly Rather than requiring crypto_init() to have been called beforehand, just have every method that depends on it call it itself. This required adding a GError argument to crypto_is_pkcs12_data(), which in turn required a few other changes elsewhere. --- libnm-core/crypto.c | 44 ++++++++++++++++++++++++++-------- libnm-core/crypto.h | 2 +- libnm-core/crypto_gnutls.c | 18 ++++++++++++++ libnm-core/crypto_nss.c | 18 ++++++++++++++ libnm-core/nm-setting-8021x.c | 26 ++++++++------------ libnm-core/nm-utils.c | 3 --- libnm-core/tests/test-crypto.c | 11 ++++----- 7 files changed, 86 insertions(+), 36 deletions(-) diff --git a/libnm-core/crypto.c b/libnm-core/crypto.c index 086f1d6714..e769bb4ab3 100644 --- a/libnm-core/crypto.c +++ b/libnm-core/crypto.c @@ -482,6 +482,9 @@ crypto_decrypt_openssl_private_key_data (const guint8 *data, if (out_key_type) g_return_val_if_fail (*out_key_type == NM_CRYPTO_KEY_TYPE_UNKNOWN, NULL); + if (!crypto_init (error)) + return NULL; + parsed = parse_old_openssl_key_file (data, data_len, &key_type, &cipher, &iv, NULL); /* return the key type even if decryption failed */ if (out_key_type) @@ -527,6 +530,9 @@ crypto_decrypt_openssl_private_key (const char *file, GByteArray *contents; GByteArray *key = NULL; + if (!crypto_init (error)) + return NULL; + contents = file_to_g_byte_array (file, error); if (contents) { key = crypto_decrypt_openssl_private_key_data (contents->data, contents->len, @@ -594,12 +600,15 @@ crypto_load_and_verify_certificate (const char *file, g_return_val_if_fail (out_file_format != NULL, NULL); g_return_val_if_fail (*out_file_format == NM_CRYPTO_FILE_FORMAT_UNKNOWN, NULL); + if (!crypto_init (error)) + return NULL; + contents = file_to_g_byte_array (file, error); if (!contents) return NULL; /* Check for PKCS#12 */ - if (crypto_is_pkcs12_data (contents->data, contents->len)) { + if (crypto_is_pkcs12_data (contents->data, contents->len, NULL)) { *out_file_format = NM_CRYPTO_FILE_FORMAT_PKCS12; return contents; } @@ -628,20 +637,26 @@ crypto_load_and_verify_certificate (const char *file, gboolean crypto_is_pkcs12_data (const guint8 *data, - gsize data_len) + gsize data_len, + GError **error) { - GError *error = NULL; + GError *local = NULL; gboolean success; g_return_val_if_fail (data != NULL, FALSE); - success = crypto_verify_pkcs12 (data, data_len, NULL, &error); + if (!crypto_init (error)) + return FALSE; + + success = crypto_verify_pkcs12 (data, data_len, NULL, &local); if (success == FALSE) { /* If the error was just a decryption error, then it's pkcs#12 */ - if (error) { - if (g_error_matches (error, NM_CRYPTO_ERROR, NM_CRYPTO_ERROR_DECRYPTION_FAILED)) + if (local) { + if (g_error_matches (local, NM_CRYPTO_ERROR, NM_CRYPTO_ERROR_DECRYPTION_FAILED)) { success = TRUE; - g_error_free (error); + g_error_free (local); + } else + g_propagate_error (error, local); } } return success; @@ -655,9 +670,12 @@ crypto_is_pkcs12_file (const char *file, GError **error) g_return_val_if_fail (file != NULL, FALSE); + if (!crypto_init (error)) + return FALSE; + contents = file_to_g_byte_array (file, error); if (contents) { - success = crypto_is_pkcs12_data (contents->data, contents->len); + success = crypto_is_pkcs12_data (contents->data, contents->len, error); g_byte_array_free (contents, TRUE); } return success; @@ -681,8 +699,11 @@ crypto_verify_private_key_data (const guint8 *data, g_return_val_if_fail (data != NULL, NM_CRYPTO_FILE_FORMAT_UNKNOWN); g_return_val_if_fail (out_is_encrypted == NULL || *out_is_encrypted == FALSE, NM_CRYPTO_FILE_FORMAT_UNKNOWN); + if (!crypto_init (error)) + return NM_CRYPTO_FILE_FORMAT_UNKNOWN; + /* Check for PKCS#12 first */ - if (crypto_is_pkcs12_data (data, data_len)) { + if (crypto_is_pkcs12_data (data, data_len, NULL)) { is_encrypted = TRUE; if (!password || crypto_verify_pkcs12 (data, data_len, password, error)) format = NM_CRYPTO_FILE_FORMAT_PKCS12; @@ -727,7 +748,10 @@ crypto_verify_private_key (const char *filename, GByteArray *contents; NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN; - g_return_val_if_fail (filename != NULL, FALSE); + g_return_val_if_fail (filename != NULL, NM_CRYPTO_FILE_FORMAT_UNKNOWN); + + if (!crypto_init (error)) + return NM_CRYPTO_FILE_FORMAT_UNKNOWN; contents = file_to_g_byte_array (filename, error); if (contents) { diff --git a/libnm-core/crypto.h b/libnm-core/crypto.h index 392ec57481..1a712dd7d0 100644 --- a/libnm-core/crypto.h +++ b/libnm-core/crypto.h @@ -63,7 +63,7 @@ GByteArray *crypto_load_and_verify_certificate (const char *file, gboolean crypto_is_pkcs12_file (const char *file, GError **error); -gboolean crypto_is_pkcs12_data (const guint8 *data, gsize len); +gboolean crypto_is_pkcs12_data (const guint8 *data, gsize len, GError **error); NMCryptoFileFormat crypto_verify_private_key_data (const guint8 *data, gsize data_len, diff --git a/libnm-core/crypto_gnutls.c b/libnm-core/crypto_gnutls.c index 03923f6307..96dddb9029 100644 --- a/libnm-core/crypto_gnutls.c +++ b/libnm-core/crypto_gnutls.c @@ -75,6 +75,9 @@ crypto_decrypt (const char *cipher, gboolean success = FALSE; gsize pad_len, real_iv_len; + if (!crypto_init (error)) + return NULL; + if (!strcmp (cipher, CIPHER_DES_EDE3_CBC)) { cipher_mech = GCRY_CIPHER_3DES; real_iv_len = SALT_LEN; @@ -196,6 +199,9 @@ crypto_encrypt (const char *cipher, guint32 i; gsize salt_len; + if (!crypto_init (error)) + return NULL; + if (!strcmp (cipher, CIPHER_DES_EDE3_CBC)) { cipher_mech = GCRY_CIPHER_3DES; salt_len = SALT_LEN; @@ -291,6 +297,9 @@ crypto_verify_cert (const unsigned char *data, gnutls_datum_t dt; int err; + if (!crypto_init (error)) + return NM_CRYPTO_FILE_FORMAT_UNKNOWN; + err = gnutls_x509_crt_init (&der); if (err < 0) { g_set_error (error, NM_CRYPTO_ERROR, @@ -335,6 +344,9 @@ crypto_verify_pkcs12 (const guint8 *data, g_return_val_if_fail (data != NULL, FALSE); + if (!crypto_init (error)) + return FALSE; + dt.data = (unsigned char *) data; dt.size = data_len; @@ -389,6 +401,9 @@ crypto_verify_pkcs8 (const guint8 *data, g_return_val_if_fail (data != NULL, FALSE); + if (!crypto_init (error)) + return FALSE; + dt.data = (unsigned char *) data; dt.size = data_len; @@ -431,6 +446,9 @@ crypto_verify_pkcs8 (const guint8 *data, gboolean crypto_randomize (void *buffer, gsize buffer_len, GError **error) { + if (!crypto_init (error)) + return FALSE; + gcry_randomize (buffer, buffer_len, GCRY_STRONG_RANDOM); return TRUE; } diff --git a/libnm-core/crypto_nss.c b/libnm-core/crypto_nss.c index 4dd85f3bdc..d0c3506b88 100644 --- a/libnm-core/crypto_nss.c +++ b/libnm-core/crypto_nss.c @@ -97,6 +97,9 @@ crypto_decrypt (const char *cipher, unsigned int pad_len = 0, extra = 0; guint32 i, real_iv_len = 0; + if (!crypto_init (error)) + return NULL; + if (!strcmp (cipher, CIPHER_DES_EDE3_CBC)) { cipher_mech = CKM_DES3_CBC_PAD; real_iv_len = 8; @@ -264,6 +267,9 @@ crypto_encrypt (const char *cipher, gboolean success = FALSE; gsize padded_buf_len, pad_len; + if (!crypto_init (error)) + return NULL; + if (!strcmp (cipher, CIPHER_DES_EDE3_CBC)) cipher_mech = CKM_DES3_CBC_PAD; else if (!strcmp (cipher, CIPHER_AES_CBC)) @@ -368,6 +374,9 @@ crypto_verify_cert (const unsigned char *data, { CERTCertificate *cert; + if (!crypto_init (error)) + return NM_CRYPTO_FILE_FORMAT_UNKNOWN; + /* Try DER/PEM first */ cert = CERT_DecodeCertFromPackage ((char *) data, len); if (!cert) { @@ -401,6 +410,9 @@ crypto_verify_pkcs12 (const guint8 *data, if (error) g_return_val_if_fail (*error == NULL, FALSE); + if (!crypto_init (error)) + return FALSE; + /* PKCS#12 passwords are apparently UCS2 BIG ENDIAN, and NSS doesn't do * any conversions for us. */ @@ -485,6 +497,9 @@ crypto_verify_pkcs8 (const guint8 *data, { g_return_val_if_fail (data != NULL, FALSE); + if (!crypto_init (error)) + return FALSE; + /* NSS apparently doesn't do PKCS#8 natively, but you have to put the * PKCS#8 key into a PKCS#12 file and import that?? So until we figure * all that out, we can only assume the password is valid. @@ -497,6 +512,9 @@ crypto_randomize (void *buffer, gsize buffer_len, GError **error) { SECStatus s; + if (!crypto_init (error)) + return FALSE; + s = PK11_GenerateRandom (buffer, buffer_len); if (s != SECSuccess) { g_set_error_literal (error, NM_CRYPTO_ERROR, diff --git a/libnm-core/nm-setting-8021x.c b/libnm-core/nm-setting-8021x.c index d5e17b97a4..369359f47a 100644 --- a/libnm-core/nm-setting-8021x.c +++ b/libnm-core/nm-setting-8021x.c @@ -1871,15 +1871,15 @@ nm_setting_802_1x_get_private_key_format (NMSetting8021x *setting) switch (nm_setting_802_1x_get_private_key_scheme (setting)) { case NM_SETTING_802_1X_CK_SCHEME_BLOB: if (crypto_is_pkcs12_data (g_bytes_get_data (priv->private_key, NULL), - g_bytes_get_size (priv->private_key))) + g_bytes_get_size (priv->private_key), + NULL)) return NM_SETTING_802_1X_CK_FORMAT_PKCS12; return NM_SETTING_802_1X_CK_FORMAT_RAW_KEY; case NM_SETTING_802_1X_CK_SCHEME_PATH: path = nm_setting_802_1x_get_private_key_path (setting); if (crypto_is_pkcs12_file (path, &error)) return NM_SETTING_802_1X_CK_FORMAT_PKCS12; - if (error) { - /* Couldn't read the file or something */ + if (error && error->domain == G_FILE_ERROR) { g_error_free (error); return NM_SETTING_802_1X_CK_FORMAT_UNKNOWN; } @@ -2151,15 +2151,15 @@ nm_setting_802_1x_get_phase2_private_key_format (NMSetting8021x *setting) switch (nm_setting_802_1x_get_phase2_private_key_scheme (setting)) { case NM_SETTING_802_1X_CK_SCHEME_BLOB: if (crypto_is_pkcs12_data (g_bytes_get_data (priv->phase2_private_key, NULL), - g_bytes_get_size (priv->phase2_private_key))) + g_bytes_get_size (priv->phase2_private_key), + NULL)) return NM_SETTING_802_1X_CK_FORMAT_PKCS12; return NM_SETTING_802_1X_CK_FORMAT_RAW_KEY; case NM_SETTING_802_1X_CK_SCHEME_PATH: path = nm_setting_802_1x_get_phase2_private_key_path (setting); if (crypto_is_pkcs12_file (path, &error)) return NM_SETTING_802_1X_CK_FORMAT_PKCS12; - if (error) { - /* Couldn't read the file or something */ + if (error && error->domain == G_FILE_ERROR) { g_error_free (error); return NM_SETTING_802_1X_CK_FORMAT_UNKNOWN; } @@ -2300,7 +2300,8 @@ verify_tls (NMSetting8021x *self, gboolean phase2, GError **error) /* If the private key is PKCS#12, check that it matches the client cert */ if (crypto_is_pkcs12_data (g_bytes_get_data (priv->phase2_private_key, NULL), - g_bytes_get_size (priv->phase2_private_key))) { + g_bytes_get_size (priv->phase2_private_key), + NULL)) { if (!g_bytes_equal (priv->phase2_private_key, priv->phase2_client_cert)) { g_set_error (error, NM_CONNECTION_ERROR, @@ -2347,7 +2348,8 @@ verify_tls (NMSetting8021x *self, gboolean phase2, GError **error) /* If the private key is PKCS#12, check that it matches the client cert */ if (crypto_is_pkcs12_data (g_bytes_get_data (priv->private_key, NULL), - g_bytes_get_size (priv->private_key))) { + g_bytes_get_size (priv->private_key), + NULL)) { if (!g_bytes_equal (priv->private_key, priv->client_cert)) { g_set_error (error, NM_CONNECTION_ERROR, @@ -3078,7 +3080,6 @@ nm_setting_802_1x_class_init (NMSetting8021xClass *setting_class) { GObjectClass *object_class = G_OBJECT_CLASS (setting_class); NMSettingClass *parent_class = NM_SETTING_CLASS (setting_class); - GError *error = NULL; g_type_class_add_private (setting_class, sizeof (NMSetting8021xPrivate)); @@ -3859,11 +3860,4 @@ nm_setting_802_1x_class_init (NMSetting8021xClass *setting_class) G_PARAM_READWRITE | G_PARAM_CONSTRUCT | G_PARAM_STATIC_STRINGS)); - - /* Initialize crypto lbrary. */ - if (!nm_utils_init (&error)) { - g_warning ("Couldn't initilize nm-utils/crypto system: %d %s", - error->code, error->message); - g_error_free (error); - } } diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 8f63a51988..c7c8421963 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -244,9 +244,6 @@ nm_utils_init (GError **error) bindtextdomain (GETTEXT_PACKAGE, LOCALEDIR); bind_textdomain_codeset (GETTEXT_PACKAGE, "UTF-8"); - if (!crypto_init (error)) - return FALSE; - _nm_dbus_errors_init (); } return TRUE; diff --git a/libnm-core/tests/test-crypto.c b/libnm-core/tests/test-crypto.c index a5513122b5..287abfbe3b 100644 --- a/libnm-core/tests/test-crypto.c +++ b/libnm-core/tests/test-crypto.c @@ -221,15 +221,14 @@ test_is_pkcs12 (const char *path, gboolean expect_fail) GError *error = NULL; is_pkcs12 = crypto_is_pkcs12_file (path, &error); - /* crypto_is_pkcs12_file() only returns an error if it couldn't read the - * file, which we don't expect to happen here. - */ - g_assert_no_error (error); - if (expect_fail) + if (expect_fail) { + g_assert_error (error, NM_CRYPTO_ERROR, NM_CRYPTO_ERROR_INVALID_DATA); g_assert (!is_pkcs12); - else + } else { + g_assert_no_error (error); g_assert (is_pkcs12); + } } static void From 3b86cc047ebd895313d66125a25f5c3d3f8b594b Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Mon, 1 Dec 2014 14:43:01 -0500 Subject: [PATCH 6/9] libnm: remove nm_utils_init() from the public API Remove nm_utils_init() from the public API, and just do it as a constructor instead. --- include/nm-test-utils.h | 5 --- libnm-core/nm-utils.c | 34 ++++++------------- libnm-core/nm-utils.h | 3 -- libnm-core/tests/test-setting-dcb.c | 10 ++---- libnm-core/tests/test-settings-defaults.c | 7 ++-- libnm/generate-setting-docs.py | 2 -- libnm/libnm.ver | 1 - libnm/nm-client.c | 7 ---- libnm/nm-manager.c | 10 ------ src/main-utils.c | 3 -- .../plugins/ifupdown/tests/test-ifupdown.c | 4 --- 11 files changed, 15 insertions(+), 71 deletions(-) diff --git a/include/nm-test-utils.h b/include/nm-test-utils.h index ca6d97d598..44a81bbe51 100644 --- a/include/nm-test-utils.h +++ b/include/nm-test-utils.h @@ -164,7 +164,6 @@ inline static void __nmtst_init (int *argc, char ***argv, gboolean assert_logging, const char *log_level, const char *log_domains) { static gsize atexit_registered = 0; - GError *error = NULL; const char *nmtst_debug; gboolean is_debug = FALSE; char *c_log_level = NULL, *c_log_domains = NULL; @@ -300,10 +299,6 @@ __nmtst_init (int *argc, char ***argv, gboolean assert_logging, const char *log_ g_setenv ("G_MESSAGES_DEBUG", "all", TRUE); } - if (!nm_utils_init (&error)) - g_error ("failed to initialize libnm-util: %s", error->message); - g_assert (!error); - /* Delay messages until we setup logging. */ for (i = 0; i < debug_messages->len; i++) __NMTST_LOG (g_message, "%s", g_array_index (debug_messages, const char *, i)); diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index c7c8421963..df5444c321 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -212,41 +212,29 @@ get_encodings_for_lang (const char *lang, /* init libnm */ +static gboolean initialized = FALSE; + static void __attribute__((constructor)) -_check_symbols (void) +_nm_utils_init (void) { GModule *self; gpointer func; + if (initialized) + return; + initialized = TRUE; + self = g_module_open (NULL, 0); if (g_module_symbol (self, "nm_util_get_private", &func)) g_error ("libnm-util symbols detected; Mixing libnm with libnm-util/libnm-glib is not supported"); g_module_close (self); -} -static gboolean initialized = FALSE; + bindtextdomain (GETTEXT_PACKAGE, LOCALEDIR); + bind_textdomain_codeset (GETTEXT_PACKAGE, "UTF-8"); -/** - * nm_utils_init: - * @error: location to store error, or %NULL - * - * Initializes libnm; should be called when starting and program that - * uses libnm. This function can be called more than once. - * - * Returns: %TRUE if the initialization was successful, %FALSE on failure. - **/ -gboolean -nm_utils_init (GError **error) -{ - if (!initialized) { - initialized = TRUE; + g_type_init (); - bindtextdomain (GETTEXT_PACKAGE, LOCALEDIR); - bind_textdomain_codeset (GETTEXT_PACKAGE, "UTF-8"); - - _nm_dbus_errors_init (); - } - return TRUE; + _nm_dbus_errors_init (); } gboolean _nm_utils_is_manager_process; diff --git a/libnm-core/nm-utils.h b/libnm-core/nm-utils.h index 013e26ca4e..debcc83bbe 100644 --- a/libnm-core/nm-utils.h +++ b/libnm-core/nm-utils.h @@ -39,9 +39,6 @@ G_BEGIN_DECLS -/* init libnm */ -gboolean nm_utils_init (GError **error); - /* SSID helpers */ gboolean nm_utils_is_empty_ssid (const guint8 *ssid, gsize len); const char *nm_utils_escape_ssid (const guint8 *ssid, gsize len); diff --git a/libnm-core/tests/test-setting-dcb.c b/libnm-core/tests/test-setting-dcb.c index 1489919075..8adab8ce88 100644 --- a/libnm-core/tests/test-setting-dcb.c +++ b/libnm-core/tests/test-setting-dcb.c @@ -302,21 +302,15 @@ test_dcb_bandwidth_sums (void) #define TPATH "/libnm/settings/dcb/" -int main (int argc, char **argv) +int +main (int argc, char **argv) { - GError *error = NULL; - gboolean success; - g_test_init (&argc, &argv, NULL); #if !GLIB_CHECK_VERSION (2, 35, 0) g_type_init (); #endif - success = nm_utils_init (&error); - g_assert_no_error (error); - g_assert (success); - #if !GLIB_CHECK_VERSION(2,34,0) g_log_set_always_fatal (G_LOG_LEVEL_CRITICAL); #endif diff --git a/libnm-core/tests/test-settings-defaults.c b/libnm-core/tests/test-settings-defaults.c index 73e707158d..0e78618723 100644 --- a/libnm-core/tests/test-settings-defaults.c +++ b/libnm-core/tests/test-settings-defaults.c @@ -101,18 +101,15 @@ test_defaults (GType type, const char *name) g_object_unref (setting); } -int main (int argc, char **argv) +int +main (int argc, char **argv) { - GError *error = NULL; char *base; #if !GLIB_CHECK_VERSION (2, 35, 0) g_type_init (); #endif - if (!nm_utils_init (&error)) - FAIL ("nm-utils-init", "failed to initialize libnm: %s", error->message); - /* The tests */ test_defaults (NM_TYPE_SETTING_CONNECTION, NM_SETTING_CONNECTION_SETTING_NAME); test_defaults (NM_TYPE_SETTING_802_1X, NM_SETTING_802_1X_SETTING_NAME); diff --git a/libnm/generate-setting-docs.py b/libnm/generate-setting-docs.py index 1d99b8621a..e1e35a86d9 100755 --- a/libnm/generate-setting-docs.py +++ b/libnm/generate-setting-docs.py @@ -159,8 +159,6 @@ args = parser.parse_args() if args.gir is None or args.output is None: usage() -NM.utils_init() - girxml = ET.parse(args.gir).getroot() outfile = open(args.output, mode='w') diff --git a/libnm/libnm.ver b/libnm/libnm.ver index 6dab82052a..ec29475dd1 100644 --- a/libnm/libnm.ver +++ b/libnm/libnm.ver @@ -770,7 +770,6 @@ global: nm_utils_iface_valid_name; nm_utils_inet4_ntop; nm_utils_inet6_ntop; - nm_utils_init; nm_utils_ip4_addresses_from_variant; nm_utils_ip4_addresses_to_variant; nm_utils_ip4_dns_from_variant; diff --git a/libnm/nm-client.c b/libnm/nm-client.c index 2483ece0bb..729dab62a6 100644 --- a/libnm/nm-client.c +++ b/libnm/nm-client.c @@ -1788,13 +1788,6 @@ init_async (GAsyncInitable *initable, int io_priority, { NMClientPrivate *priv = NM_CLIENT_GET_PRIVATE (initable); NMClientInitData *init_data; - GError *error = NULL; - - if (!nm_utils_init (&error)) { - g_simple_async_report_take_gerror_in_idle (G_OBJECT (initable), - callback, user_data, error); - return; - } init_data = g_slice_new0 (NMClientInitData); init_data->client = NM_CLIENT (initable); diff --git a/libnm/nm-manager.c b/libnm/nm-manager.c index 3c5903b6da..1abb513021 100644 --- a/libnm/nm-manager.c +++ b/libnm/nm-manager.c @@ -1306,9 +1306,6 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) { NMManager *manager = NM_MANAGER (initable); - if (!nm_utils_init (error)) - return FALSE; - if (!nm_manager_parent_initable_iface->init (initable, cancellable, error)) return FALSE; @@ -1380,13 +1377,6 @@ init_async (GAsyncInitable *initable, int io_priority, gpointer user_data) { NMManagerInitData *init_data; - GError *error = NULL; - - if (!nm_utils_init (&error)) { - g_simple_async_report_take_gerror_in_idle (G_OBJECT (initable), - callback, user_data, error); - return; - } init_data = g_slice_new0 (NMManagerInitData); init_data->manager = NM_MANAGER (initable); diff --git a/src/main-utils.c b/src/main-utils.c index 5b13f6c590..0fa3644f3e 100644 --- a/src/main-utils.c +++ b/src/main-utils.c @@ -242,9 +242,6 @@ nm_main_utils_early_setup (const char *progname, /* Ensure gettext() gets the right environment (bgo #666516) */ setlocale (LC_ALL, ""); - - bindtextdomain (GETTEXT_PACKAGE, NMLOCALEDIR); - bind_textdomain_codeset (GETTEXT_PACKAGE, "UTF-8"); textdomain (GETTEXT_PACKAGE); if (getuid () != 0) { diff --git a/src/settings/plugins/ifupdown/tests/test-ifupdown.c b/src/settings/plugins/ifupdown/tests/test-ifupdown.c index 59b16e0066..1c100ac2e2 100644 --- a/src/settings/plugins/ifupdown/tests/test-ifupdown.c +++ b/src/settings/plugins/ifupdown/tests/test-ifupdown.c @@ -847,14 +847,10 @@ test20_source_stanza (const char *path) int main (int argc, char **argv) { - GError *error = NULL; - #if !GLIB_CHECK_VERSION (2, 35, 0) g_type_init (); #endif - if (!nm_utils_init (&error)) - FAIL ("nm-utils-init", "failed to initialize libnm: %s", error->message); nm_logging_setup ("WARN", "DEFAULT", NULL, NULL); g_test_init (&argc, &argv, NULL); From 446038680040de495f0e7438b94c7bea1c8d233e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 2 Dec 2014 11:54:27 +0100 Subject: [PATCH 7/9] libnm-core: combine duplicate crypto_make_des_aes_key() function --- libnm-core/crypto.c | 16 ++++++++-------- libnm-core/crypto.h | 7 +++++++ libnm-core/nm-utils.c | 31 ++----------------------------- 3 files changed, 17 insertions(+), 37 deletions(-) diff --git a/libnm-core/crypto.c b/libnm-core/crypto.c index e769bb4ab3..74063379fe 100644 --- a/libnm-core/crypto.c +++ b/libnm-core/crypto.c @@ -368,13 +368,13 @@ error: return NULL; } -static char * -make_des_aes_key (const char *cipher, - const char *salt, - const gsize salt_len, - const char *password, - gsize *out_len, - GError **error) +char * +crypto_make_des_aes_key (const char *cipher, + const char *salt, + const gsize salt_len, + const char *password, + gsize *out_len, + GError **error) { char *key; guint32 digest_len; @@ -439,7 +439,7 @@ decrypt_key (const char *cipher, return NULL; /* Convert the password and IV into a DES or AES key */ - key = make_des_aes_key (cipher, bin_iv, bin_iv_len, password, &key_len, error); + key = crypto_make_des_aes_key (cipher, bin_iv, bin_iv_len, password, &key_len, error); if (!key || !key_len) goto out; diff --git a/libnm-core/crypto.h b/libnm-core/crypto.h index 1a712dd7d0..f36975581e 100644 --- a/libnm-core/crypto.h +++ b/libnm-core/crypto.h @@ -85,6 +85,13 @@ void crypto_md5_hash (const char *salt, char *buffer, gsize buflen); +char *crypto_make_des_aes_key (const char *cipher, + const char *salt, + const gsize salt_len, + const char *password, + gsize *out_len, + GError **error); + char * crypto_decrypt (const char *cipher, int key_type, const guint8 *data, diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index df5444c321..7daf72d39b 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -1974,34 +1974,6 @@ nm_utils_uuid_generate_from_string (const char *s) return buf; } -static char * -make_key (const char *cipher, - const char *salt, - const gsize salt_len, - const char *password, - gsize *out_len) -{ - char *key; - guint32 digest_len = 24; /* DES-EDE3-CBC */ - - g_return_val_if_fail (salt != NULL, NULL); - g_return_val_if_fail (salt_len >= 8, NULL); - g_return_val_if_fail (password != NULL, NULL); - g_return_val_if_fail (out_len != NULL, NULL); - - if (!strcmp (cipher, "DES-EDE3-CBC")) - digest_len = 24; - else if (!strcmp (cipher, "AES-128-CBC")) - digest_len = 16; - - key = g_malloc0 (digest_len + 1); - - crypto_md5_hash (salt, salt_len, password, strlen (password), key, digest_len); - *out_len = digest_len; - - return key; -} - /** * nm_utils_rsa_key_encrypt_helper: * @cipher: cipher to use for encryption ("DES-EDE3-CBC" or "AES-128-CBC") @@ -2058,7 +2030,8 @@ nm_utils_rsa_key_encrypt_helper (const char *cipher, if (!crypto_randomize (salt, salt_len, error)) goto out; - key = make_key (cipher, &salt[0], salt_len, in_password, &key_len); + key = crypto_make_des_aes_key (cipher, &salt[0], salt_len, in_password, &key_len, NULL); + g_return_val_if_fail (key, NULL); enc = crypto_encrypt (cipher, data, len, salt, salt_len, key, key_len, &enc_len, error); if (!enc) goto out; From ef3de46c432743e2449612369d13eee66b22cb89 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 2 Dec 2014 12:26:38 +0100 Subject: [PATCH 8/9] libnm-core: relax restrictions on input arguments for crypto_md5_hash() crypto_md5_hash() only has two users: (a) crypto_make_des_aes_key() (b) nm_utils_uuid_generate_from_string() For (b) it is just a complicated way to compute the MD5 hash. The restrictions on salt and password don't matter. Actually they are harmful because we cannot compute the MD5 hash of the empty word. For (a), the caller should make sure to pass whatever restrictions he wants to enforce on the data. For example, it is counterintuitive, that crypto_md5_hash() would require @salt_len, enforce it to be at least 8 bytes, and then just use the first 8 bytes. If the caller (a) wants that behavior, he should make sure that he passes in 8 bytes. Likewise for the empty word. If the caller does not want to compute the hash of empty passwords, he must not hash them. Indeed, all of this was enforced by assertions, any caller already did the right thing. --- libnm-core/crypto.c | 25 ++++++++++++++----------- libnm-core/crypto.h | 4 ++-- libnm-core/tests/test-crypto.c | 6 +++++- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/libnm-core/crypto.c b/libnm-core/crypto.c index 74063379fe..62629a3f72 100644 --- a/libnm-core/crypto.c +++ b/libnm-core/crypto.c @@ -405,7 +405,7 @@ crypto_make_des_aes_key (const char *cipher, key = g_malloc0 (digest_len + 1); crypto_md5_hash (salt, - salt_len, + 8, password, strlen (password), key, @@ -763,9 +763,9 @@ crypto_verify_private_key (const char *filename, void crypto_md5_hash (const char *salt, - const gsize salt_len, + gssize salt_len, const char *password, - gsize password_len, + gssize password_len, char *buffer, gsize buflen) { @@ -778,25 +778,28 @@ crypto_md5_hash (const char *salt, g_assert_cmpint (g_checksum_type_get_length (G_CHECKSUM_MD5), ==, sizeof (digest)); - if (salt) - g_return_if_fail (salt_len >= 8); - - g_return_if_fail (password != NULL); - g_return_if_fail (password_len > 0); + g_return_if_fail (password_len == 0 || password); g_return_if_fail (buffer != NULL); g_return_if_fail (buflen > 0); + g_return_if_fail (salt_len == 0 || salt); ctx = g_checksum_new (G_CHECKSUM_MD5); + if (salt_len < 0) + salt_len = strlen (salt); + if (password_len < 0) + password_len = strlen (password); + while (nkey > 0) { int i = 0; g_checksum_reset (ctx); if (count++) g_checksum_update (ctx, (const guchar *) digest, digest_len); - g_checksum_update (ctx, (const guchar *) password, password_len); - if (salt) - g_checksum_update (ctx, (const guchar *) salt, 8); /* Only use 8 bytes of salt */ + if (password_len > 0) + g_checksum_update (ctx, (const guchar *) password, password_len); + if (salt_len > 0) + g_checksum_update (ctx, (const guchar *) salt, salt_len); g_checksum_get_digest (ctx, (guchar *) digest, &digest_len); while (nkey && (i < digest_len)) { diff --git a/libnm-core/crypto.h b/libnm-core/crypto.h index f36975581e..434f108d0a 100644 --- a/libnm-core/crypto.h +++ b/libnm-core/crypto.h @@ -79,9 +79,9 @@ NMCryptoFileFormat crypto_verify_private_key (const char *file, /* Internal utils API bits for crypto providers */ void crypto_md5_hash (const char *salt, - const gsize salt_len, + gssize salt_len, const char *password, - gsize password_len, + gssize password_len, char *buffer, gsize buflen); diff --git a/libnm-core/tests/test-crypto.c b/libnm-core/tests/test-crypto.c index 287abfbe3b..05157f0185 100644 --- a/libnm-core/tests/test-crypto.c +++ b/libnm-core/tests/test-crypto.c @@ -438,7 +438,11 @@ test_md5 (void) for (i = 0; i < G_N_ELEMENTS (md5_tests); i++) { memset (digest, 0, sizeof (digest)); crypto_md5_hash (md5_tests[i].salt, - md5_tests[i].salt ? strlen (md5_tests[i].salt) : 0, + /* crypto_md5_hash() used to clamp salt_len to 8. It + * doesn't any more, so we need to do it here now to + * get output that matches md5_tests[i].result. + */ + md5_tests[i].salt ? 8 : 0, md5_tests[i].password, strlen (md5_tests[i].password), digest, md5_tests[i].digest_size); From d91bcc4960d02cc8dba5341fb1ef2b85f2a377f6 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 3 Dec 2014 08:23:44 -0500 Subject: [PATCH 9/9] libnm-core: drop nm_utils_rsa_key_encrypt(), _encrypt_aes() In general, we shouldn't end up with an unencrypted copy of a certificate key anyway, so this function ought to be unnecessary (or at least, not broadly useful enough to be in the public API). nm-applet's GConf migration tool needs it, but that will eventually go away, and until then it can just use libnm-util. --- libnm-core/nm-core-internal.h | 6 +++ libnm-core/nm-utils.c | 94 +++++----------------------------- libnm-core/nm-utils.h | 10 ---- libnm-core/tests/test-crypto.c | 28 +--------- libnm/libnm.ver | 2 - 5 files changed, 20 insertions(+), 120 deletions(-) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index f4f4fe4c75..c0bbd71eac 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -121,4 +121,10 @@ void _nm_dbus_errors_init (void); extern gboolean _nm_utils_is_manager_process; +GByteArray *nm_utils_rsa_key_encrypt (const guint8 *data, + gsize len, + const char *in_password, + char **out_password, + GError **error); + #endif diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 7daf72d39b..7f6cf49a1f 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -1975,8 +1975,7 @@ nm_utils_uuid_generate_from_string (const char *s) } /** - * nm_utils_rsa_key_encrypt_helper: - * @cipher: cipher to use for encryption ("DES-EDE3-CBC" or "AES-128-CBC") + * nm_utils_rsa_key_encrypt: * @data: (array length=len): RSA private key data to be encrypted * @len: length of @data * @in_password: (allow-none): existing password to use, if any @@ -1986,18 +1985,17 @@ nm_utils_uuid_generate_from_string (const char *s) * * Encrypts the given RSA private key data with the given password (or generates * a password if no password was given) and converts the data to PEM format - * suitable for writing to a file. + * suitable for writing to a file. It uses Triple DES cipher for the encryption. * * Returns: (transfer full): on success, PEM-formatted data suitable for writing * to a PEM-formatted certificate/private key file. **/ -static GByteArray * -nm_utils_rsa_key_encrypt_helper (const char *cipher, - const guint8 *data, - gsize len, - const char *in_password, - char **out_password, - GError **error) +GByteArray * +nm_utils_rsa_key_encrypt (const guint8 *data, + gsize len, + const char *in_password, + char **out_password, + GError **error) { char salt[16]; int salt_len; @@ -2009,7 +2007,6 @@ nm_utils_rsa_key_encrypt_helper (const char *cipher, const char *p; GByteArray *ret = NULL; - g_return_val_if_fail (!g_strcmp0 (cipher, CIPHER_DES_EDE3_CBC) || !g_strcmp0 (cipher, CIPHER_AES_CBC), NULL); g_return_val_if_fail (data != NULL, NULL); g_return_val_if_fail (len > 0, NULL); if (out_password) @@ -2022,17 +2019,13 @@ nm_utils_rsa_key_encrypt_helper (const char *cipher, in_password = tmp_password = nm_utils_bin2hexstr (pw_buf, sizeof (pw_buf), -1); } - if (g_strcmp0 (cipher, CIPHER_AES_CBC) == 0) - salt_len = 16; - else - salt_len = 8; - + salt_len = 8; if (!crypto_randomize (salt, salt_len, error)) goto out; - key = crypto_make_des_aes_key (cipher, &salt[0], salt_len, in_password, &key_len, NULL); + key = crypto_make_des_aes_key (CIPHER_DES_EDE3_CBC, &salt[0], salt_len, in_password, &key_len, NULL); g_return_val_if_fail (key, NULL); - enc = crypto_encrypt (cipher, data, len, salt, salt_len, key, key_len, &enc_len, error); + enc = crypto_encrypt (CIPHER_DES_EDE3_CBC, data, len, salt, salt_len, key, key_len, &enc_len, error); if (!enc) goto out; @@ -2042,7 +2035,7 @@ nm_utils_rsa_key_encrypt_helper (const char *cipher, /* Convert the salt to a hex string */ tmp = nm_utils_bin2hexstr (salt, salt_len, salt_len * 2); - g_string_append_printf (pem, "DEK-Info: %s,%s\n\n", cipher, tmp); + g_string_append_printf (pem, "DEK-Info: %s,%s\n\n", CIPHER_DES_EDE3_CBC, tmp); g_free (tmp); /* Convert the encrypted key to a base64 string */ @@ -2083,69 +2076,6 @@ out: return ret; } -/** - * nm_utils_rsa_key_encrypt: - * @data: (array length=len): RSA private key data to be encrypted - * @len: length of @data - * @in_password: (allow-none): existing password to use, if any - * @out_password: (out) (allow-none): if @in_password was %NULL, a random - * password will be generated and returned in this argument - * @error: detailed error information on return, if an error occurred - * - * Encrypts the given RSA private key data with the given password (or generates - * a password if no password was given) and converts the data to PEM format - * suitable for writing to a file. It uses Triple DES cipher for the encryption. - * - * Returns: (transfer full): on success, PEM-formatted data suitable for writing - * to a PEM-formatted certificate/private key file. - **/ -GByteArray * -nm_utils_rsa_key_encrypt (const guint8 *data, - gsize len, - const char *in_password, - char **out_password, - GError **error) -{ - - - return nm_utils_rsa_key_encrypt_helper (CIPHER_DES_EDE3_CBC, - data, len, - in_password, - out_password, - error); -} - -/** - * nm_utils_rsa_key_encrypt_aes: - * @data: (array length=len): RSA private key data to be encrypted - * @len: length of @data - * @in_password: (allow-none): existing password to use, if any - * @out_password: (out) (allow-none): if @in_password was %NULL, a random - * password will be generated and returned in this argument - * @error: detailed error information on return, if an error occurred - * - * Encrypts the given RSA private key data with the given password (or generates - * a password if no password was given) and converts the data to PEM format - * suitable for writing to a file. It uses AES cipher for the encryption. - * - * Returns: (transfer full): on success, PEM-formatted data suitable for writing - * to a PEM-formatted certificate/private key file. - **/ -GByteArray * -nm_utils_rsa_key_encrypt_aes (const guint8 *data, - gsize len, - const char *in_password, - char **out_password, - GError **error) -{ - - return nm_utils_rsa_key_encrypt_helper (CIPHER_AES_CBC, - data, len, - in_password, - out_password, - error); -} - static gboolean file_has_extension (const char *filename, const char *extensions[]) { diff --git a/libnm-core/nm-utils.h b/libnm-core/nm-utils.h index debcc83bbe..85f5afd06a 100644 --- a/libnm-core/nm-utils.h +++ b/libnm-core/nm-utils.h @@ -123,16 +123,6 @@ GPtrArray *nm_utils_ip_routes_from_variant (GVariant *value, char *nm_utils_uuid_generate (void); char *nm_utils_uuid_generate_from_string (const char *s); -GByteArray *nm_utils_rsa_key_encrypt (const guint8 *data, - gsize len, - const char *in_password, - char **out_password, - GError **error); -GByteArray *nm_utils_rsa_key_encrypt_aes (const guint8 *data, - gsize len, - const char *in_password, - char **out_password, - GError **error); gboolean nm_utils_file_is_certificate (const char *filename); gboolean nm_utils_file_is_private_key (const char *filename, gboolean *out_encrypted); gboolean nm_utils_file_is_pkcs12 (const char *filename); diff --git a/libnm-core/tests/test-crypto.c b/libnm-core/tests/test-crypto.c index 05157f0185..0777de3f33 100644 --- a/libnm-core/tests/test-crypto.c +++ b/libnm-core/tests/test-crypto.c @@ -33,6 +33,7 @@ #include "crypto.h" #include "nm-utils.h" #include "nm-errors.h" +#include "nm-core-internal.h" #include "nm-test-utils.h" @@ -254,28 +255,6 @@ test_load_pkcs8 (const char *path, } } -static gboolean -is_cipher_aes (const char *path) -{ - char *contents; - gsize length = 0; - const char *cipher; - gboolean is_aes = FALSE; - - if (!g_file_get_contents (path, &contents, &length, NULL)) - return FALSE; - - cipher = strstr (contents, "DEK-Info: "); - if (cipher) { - cipher += strlen ("DEK-Info: "); - if (g_str_has_prefix (cipher, "AES-128-CBC")) - is_aes = TRUE; - } - - g_free (contents); - return is_aes; -} - static void test_encrypt_private_key (const char *path, const char *password) @@ -290,10 +269,7 @@ test_encrypt_private_key (const char *path, g_assert_cmpint (key_type, ==, NM_CRYPTO_KEY_TYPE_RSA); /* Now re-encrypt the private key */ - if (is_cipher_aes (path)) - encrypted = nm_utils_rsa_key_encrypt_aes (array->data, array->len, password, NULL, &error); - else - encrypted = nm_utils_rsa_key_encrypt (array->data, array->len, password, NULL, &error); + encrypted = nm_utils_rsa_key_encrypt (array->data, array->len, password, NULL, &error); g_assert_no_error (error); g_assert (encrypted != NULL); diff --git a/libnm/libnm.ver b/libnm/libnm.ver index ec29475dd1..58d886029d 100644 --- a/libnm/libnm.ver +++ b/libnm/libnm.ver @@ -788,8 +788,6 @@ global: nm_utils_ipaddr_valid; nm_utils_is_empty_ssid; nm_utils_is_uuid; - nm_utils_rsa_key_encrypt; - nm_utils_rsa_key_encrypt_aes; nm_utils_same_ssid; nm_utils_security_type_get_type; nm_utils_security_valid;