diff options
author | Dan Williams <dcbw@redhat.com> | 2011-03-02 12:00:47 -0600 |
---|---|---|
committer | Dan Williams <dcbw@redhat.com> | 2011-03-02 12:00:47 -0600 |
commit | 28e6523b8d4eb031777dd0d3f5118bbfc8fa45a9 (patch) | |
tree | 17be4c968bfe8d2976dbe49cd0c513ffe551f705 | |
parent | 02f676c0010ccaa4d70099a32cac8dc4663612a9 (diff) | |
download | NetworkManager-28e6523b8d4eb031777dd0d3f5118bbfc8fa45a9.tar.gz |
libnm-util: rework certificate and private key handling
First, it was not easily possible to set a private key without
also providing a password. This used to be OK, but now with
secret flags it may be the case that when the connection is read,
there's no private key password. So functions that set the
private key must account for NULL passwords.
Unfortunately, the crytpo code did not handle this case well.
We need to be able to independently (a) verify that a file looks
like a certificate or private key and (b) that a given password
decrypts a private key. Previously the crypto code would fail
to verify the file when the password was NULL.
So this change fixes up the crytpo code for a more distinct
split between these two operations, such that if no password is
given, the file is still checked to ensure that it's a private
key or a certificate. If a password is given, the password is
checked against the private key file.
This commit also changes how private keys and certificates were
handled with the BLOB scheme. Previously only the first certificate
or first private key was included in the property data, while now
the entire file is encoded in the data. This is intended to fix
cases where multiple private keys or certificates are present in
a PEM file. It also allows clients to push certificate data to
NetworkManager for storage in system settings locations, which was
not as flexible before when only part of the certificate or key
was sent as the data.
-rw-r--r-- | libnm-util/crypto.c | 414 | ||||
-rw-r--r-- | libnm-util/crypto.h | 40 | ||||
-rw-r--r-- | libnm-util/nm-setting-8021x.c | 440 | ||||
-rw-r--r-- | libnm-util/nm-setting-8021x.h | 50 | ||||
-rw-r--r-- | libnm-util/tests/Makefile.am | 52 | ||||
-rw-r--r-- | libnm-util/tests/test-crypto.c | 145 | ||||
-rw-r--r-- | libnm-util/tests/test-need-secrets.c | 38 | ||||
-rw-r--r-- | libnm-util/tests/test-setting-8021x.c | 356 |
8 files changed, 914 insertions, 621 deletions
diff --git a/libnm-util/crypto.c b/libnm-util/crypto.c index 70872db414..e68b5c64a4 100644 --- a/libnm-util/crypto.c +++ b/libnm-util/crypto.c @@ -18,7 +18,7 @@ * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, * Boston, MA 02110-1301 USA. * - * (C) Copyright 2007 - 2009 Red Hat, Inc. + * (C) Copyright 2007 - 2011 Red Hat, Inc. */ #include <glib.h> @@ -41,36 +41,43 @@ _nm_crypto_error_quark (void) } -static const char *pem_rsa_key_begin = "-----BEGIN RSA PRIVATE KEY-----"; -static const char *pem_rsa_key_end = "-----END RSA PRIVATE KEY-----"; +#define PEM_RSA_KEY_BEGIN "-----BEGIN RSA PRIVATE KEY-----" +#define PEM_RSA_KEY_END "-----END RSA PRIVATE KEY-----" -static const char *pem_dsa_key_begin = "-----BEGIN DSA PRIVATE KEY-----"; -static const char *pem_dsa_key_end = "-----END DSA PRIVATE KEY-----"; +#define PEM_DSA_KEY_BEGIN "-----BEGIN DSA PRIVATE KEY-----" +#define PEM_DSA_KEY_END "-----END DSA PRIVATE KEY-----" -static const char *pem_cert_begin = "-----BEGIN CERTIFICATE-----"; -static const char *pem_cert_end = "-----END CERTIFICATE-----"; +#define PEM_CERT_BEGIN "-----BEGIN CERTIFICATE-----" +#define PEM_CERT_END "-----END CERTIFICATE-----" -static const char * -find_tag (const char *tag, const char *buf, gsize len) +static gboolean +find_tag (const char *tag, + const GByteArray *array, + gsize start_at, + gsize *out_pos) { gsize i, taglen; + gsize len = array->len - start_at; - taglen = strlen (tag); - if (len < taglen) - return NULL; + g_return_val_if_fail (out_pos != NULL, FALSE); - for (i = 0; i < len - taglen + 1; i++) { - if (memcmp (buf + i, tag, taglen) == 0) - return buf + i; + taglen = strlen (tag); + if (len >= taglen) { + for (i = 0; i < len - taglen + 1; i++) { + if (memcmp (array->data + start_at + i, tag, taglen) == 0) { + *out_pos = start_at + i; + return TRUE; + } + } } - return NULL; + return FALSE; } #define DEK_INFO_TAG "DEK-Info: " #define PROC_TYPE_TAG "Proc-Type: " static GByteArray * -parse_old_openssl_key_file (GByteArray *contents, +parse_old_openssl_key_file (const GByteArray *contents, int key_type, char **out_cipher, char **out_iv, @@ -79,8 +86,7 @@ parse_old_openssl_key_file (GByteArray *contents, GByteArray *bindata = NULL; char **lines = NULL; char **ln = NULL; - const char *pos; - const char *end; + gsize start = 0, end = 0; GString *str = NULL; int enc_tags = 0; char *iv = NULL; @@ -89,15 +95,16 @@ parse_old_openssl_key_file (GByteArray *contents, gsize tmp_len = 0; const char *start_tag; const char *end_tag; + guint8 save_end = 0; switch (key_type) { case NM_CRYPTO_KEY_TYPE_RSA: - start_tag = pem_rsa_key_begin; - end_tag = pem_rsa_key_end; + start_tag = PEM_RSA_KEY_BEGIN; + end_tag = PEM_RSA_KEY_END; break; case NM_CRYPTO_KEY_TYPE_DSA: - start_tag = pem_dsa_key_begin; - end_tag = pem_dsa_key_end; + start_tag = PEM_DSA_KEY_BEGIN; + end_tag = PEM_DSA_KEY_END; break; default: g_set_error (error, NM_CRYPTO_ERROR, @@ -108,23 +115,23 @@ parse_old_openssl_key_file (GByteArray *contents, return NULL; } - pos = find_tag (start_tag, (const char *) contents->data, contents->len); - if (!pos) + if (!find_tag (start_tag, contents, 0, &start)) goto parse_error; - pos += strlen (start_tag); - - end = find_tag (end_tag, pos, (const char *) contents->data + contents->len - pos); - if (end == NULL) { + start += strlen (start_tag); + if (!find_tag (end_tag, contents, start, &end)) { g_set_error (error, NM_CRYPTO_ERROR, NM_CRYPTO_ERR_FILE_FORMAT_INVALID, _("PEM key file had no end tag '%s'."), end_tag); goto parse_error; } - *((char *) end) = '\0'; - lines = g_strsplit (pos, "\n", 0); + save_end = contents->data[end]; + contents->data[end] = '\0'; + lines = g_strsplit ((const char *) (contents->data + start), "\n", 0); + contents->data[end] = save_end; + if (!lines || g_strv_length (lines) <= 1) { g_set_error (error, NM_CRYPTO_ERROR, NM_CRYPTO_ERR_FILE_FORMAT_INVALID, @@ -132,7 +139,7 @@ parse_old_openssl_key_file (GByteArray *contents, goto parse_error; } - str = g_string_new_len (NULL, end - pos); + str = g_string_new_len (NULL, end - start); if (!str) { g_set_error (error, NM_CRYPTO_ERROR, NM_CRYPTO_ERR_OUT_OF_MEMORY, @@ -242,64 +249,24 @@ parse_error: } static GByteArray * -file_to_g_byte_array (const char *filename, - gboolean privkey, - GError **error) +file_to_g_byte_array (const char *filename, GError **error) { - char *contents, *der = NULL; + char *contents; GByteArray *array = NULL; gsize length = 0; - const char *pos = NULL; - - if (!g_file_get_contents (filename, &contents, &length, error)) - return NULL; - - if (!privkey) - pos = find_tag (pem_cert_begin, contents, length); - - if (pos) { - const char *end; - - pos += strlen (pem_cert_begin); - end = find_tag (pem_cert_end, pos, contents + length - pos); - if (end == NULL) { - g_set_error (error, NM_CRYPTO_ERROR, - NM_CRYPTO_ERR_FILE_FORMAT_INVALID, - _("PEM certificate '%s' had no end tag '%s'."), - filename, pem_cert_end); - goto done; - } - contents[end - contents - 1] = '\0'; - der = (char *) g_base64_decode (pos, &length); - if (der == NULL || !length) { + if (g_file_get_contents (filename, &contents, &length, error)) { + array = g_byte_array_sized_new (length); + if (array) { + g_byte_array_append (array, (guint8 *) contents, length); + g_assert (array->len == length); + } else { g_set_error (error, NM_CRYPTO_ERROR, - NM_CRYPTO_ERR_DECODE_FAILED, - _("Failed to decode certificate.")); - goto done; + NM_CRYPTO_ERR_OUT_OF_MEMORY, + _("Not enough memory to store certificate data.")); } + g_free (contents); } - - array = g_byte_array_sized_new (length); - if (!array) { - g_set_error (error, NM_CRYPTO_ERROR, - NM_CRYPTO_ERR_OUT_OF_MEMORY, - _("Not enough memory to store certificate data.")); - goto done; - } - - g_byte_array_append (array, der ? (unsigned char *) der : (unsigned char *) contents, length); - if (array->len != length) { - g_set_error (error, NM_CRYPTO_ERROR, - NM_CRYPTO_ERR_OUT_OF_MEMORY, - _("Not enough memory to store file data.")); - g_byte_array_free (array, TRUE); - array = NULL; - } - -done: - g_free (der); - g_free (contents); return array; } @@ -414,13 +381,12 @@ error: return NULL; } -static char * +static GByteArray * decrypt_key (const char *cipher, int key_type, GByteArray *data, const char *iv, const char *password, - gsize *out_len, GError **error) { char *bin_iv = NULL; @@ -428,6 +394,10 @@ decrypt_key (const char *cipher, char *key = NULL; gsize key_len = 0; char *output = NULL; + gsize decrypted_len = 0; + GByteArray *decrypted = NULL; + + g_return_val_if_fail (password != NULL, NULL); bin_iv = convert_iv (iv, &bin_iv_len, error); if (!bin_iv) @@ -442,58 +412,45 @@ decrypt_key (const char *cipher, data, bin_iv, bin_iv_len, key, key_len, - out_len, + &decrypted_len, error); - if (!output) - goto out; - - if (*out_len == 0) { - g_free (output); - output = NULL; - goto out; + if (output && decrypted_len) { + decrypted = g_byte_array_sized_new (decrypted_len); + if (decrypted) + g_byte_array_append (decrypted, (guint8 *) output, decrypted_len); + else { + g_set_error (error, NM_CRYPTO_ERROR, + NM_CRYPTO_ERR_OUT_OF_MEMORY, + _("Not enough memory to store decrypted private key.")); + } } - + out: - if (key) { - /* Don't leak stale key material */ + /* Don't leak stale key material */ + if (key) memset (key, 0, key_len); - g_free (key); - } + g_free (output); + g_free (key); g_free (bin_iv); - return output; + + return decrypted; } GByteArray * -crypto_get_private_key_data (GByteArray *contents, - const char *password, - NMCryptoKeyType *out_key_type, - NMCryptoFileFormat *out_file_type, - GError **error) +crypto_decrypt_private_key_data (const GByteArray *contents, + const char *password, + NMCryptoKeyType *out_key_type, + GError **error) { - GByteArray *array = NULL; + GByteArray *decrypted = NULL; NMCryptoKeyType key_type = NM_CRYPTO_KEY_TYPE_RSA; GByteArray *data; char *iv = NULL; char *cipher = NULL; - char *decrypted = NULL; - gsize decrypted_len = 0; g_return_val_if_fail (contents != NULL, NULL); - g_return_val_if_fail (password != NULL, NULL); - g_return_val_if_fail (out_key_type != NULL, NULL); - g_return_val_if_fail (*out_key_type == NM_CRYPTO_KEY_TYPE_UNKNOWN, NULL); - g_return_val_if_fail (out_file_type != NULL, NULL); - g_return_val_if_fail (*out_file_type == NM_CRYPTO_FILE_FORMAT_UNKNOWN, NULL); - - /* Try PKCS#12 first */ - if (crypto_verify_pkcs12 (contents, password, NULL)) { - *out_key_type = NM_CRYPTO_KEY_TYPE_ENCRYPTED; - *out_file_type = NM_CRYPTO_FILE_FORMAT_PKCS12; - - array = g_byte_array_sized_new (contents->len); - g_byte_array_append (array, contents->data, contents->len); - return array; - } + if (out_key_type) + g_return_val_if_fail (*out_key_type == NM_CRYPTO_KEY_TYPE_UNKNOWN, NULL); /* OpenSSL non-standard legacy PEM files */ @@ -510,91 +467,137 @@ crypto_get_private_key_data (GByteArray *contents, g_set_error (error, NM_CRYPTO_ERROR, NM_CRYPTO_ERR_FILE_FORMAT_INVALID, _("Unable to determine private key type.")); - goto out; } } - decrypted = decrypt_key (cipher, - key_type, - data, - iv, - password, - &decrypted_len, - error); - if (!decrypted) - goto out; - - array = g_byte_array_sized_new (decrypted_len); - if (!array) { - g_set_error (error, NM_CRYPTO_ERROR, - NM_CRYPTO_ERR_OUT_OF_MEMORY, - _("Not enough memory to store decrypted private key.")); - goto out; + if (data) { + /* return the key type even if decryption failed */ + if (out_key_type) + *out_key_type = key_type; + + if (password) { + decrypted = decrypt_key (cipher, + key_type, + data, + iv, + password, + error); + } + g_byte_array_free (data, TRUE); } - g_byte_array_append (array, (const guint8 *) decrypted, decrypted_len); - *out_key_type = key_type; - *out_file_type = NM_CRYPTO_FILE_FORMAT_RAW_KEY; - -out: - if (decrypted) { - /* Don't expose key material */ - memset (decrypted, 0, decrypted_len); - g_free (decrypted); - } - if (data) - g_byte_array_free (data, TRUE); g_free (cipher); g_free (iv); - return array; + + return decrypted; } GByteArray * -crypto_get_private_key (const char *file, - const char *password, - NMCryptoKeyType *out_key_type, - NMCryptoFileFormat *out_file_type, - GError **error) +crypto_decrypt_private_key (const char *file, + const char *password, + NMCryptoKeyType *out_key_type, + GError **error) { GByteArray *contents; GByteArray *key = NULL; - contents = file_to_g_byte_array (file, TRUE, error); + contents = file_to_g_byte_array (file, error); if (contents) { - key = crypto_get_private_key_data (contents, password, out_key_type, out_file_type, error); + key = crypto_decrypt_private_key_data (contents, password, out_key_type, error); g_byte_array_free (contents, TRUE); } return key; } +static GByteArray * +extract_pem_cert_data (GByteArray *contents, GError **error) +{ + GByteArray *cert = NULL; + gsize start = 0, end = 0; + unsigned char *der = NULL; + guint8 save_end; + gsize length = 0; + + if (!find_tag (PEM_CERT_BEGIN, contents, 0, &start)) { + g_set_error (error, NM_CRYPTO_ERROR, + NM_CRYPTO_ERR_FILE_FORMAT_INVALID, + _("PEM certificate had no start tag '%s'."), + PEM_CERT_BEGIN); + goto done; + } + + start += strlen (PEM_CERT_BEGIN); + if (!find_tag (PEM_CERT_END, contents, start, &end)) { + g_set_error (error, NM_CRYPTO_ERROR, + NM_CRYPTO_ERR_FILE_FORMAT_INVALID, + _("PEM certificate had no end tag '%s'."), + PEM_CERT_END); + goto done; + } + + /* g_base64_decode() wants a NULL-terminated string */ + save_end = contents->data[end]; + contents->data[end] = '\0'; + der = g_base64_decode ((const char *) (contents->data + start), &length); + contents->data[end] = save_end; + + if (der && length) { + cert = g_byte_array_sized_new (length); + if (cert) { + g_byte_array_append (cert, der, length); + g_assert (cert->len == length); + } else { + g_set_error (error, NM_CRYPTO_ERROR, + NM_CRYPTO_ERR_OUT_OF_MEMORY, + _("Not enough memory to store certificate data.")); + } + } else { + g_set_error (error, NM_CRYPTO_ERROR, + NM_CRYPTO_ERR_DECODE_FAILED, + _("Failed to decode certificate.")); + } + +done: + g_free (der); + return cert; +} + GByteArray * crypto_load_and_verify_certificate (const char *file, NMCryptoFileFormat *out_file_format, GError **error) { - GByteArray *array; + GByteArray *array, *contents; g_return_val_if_fail (file != NULL, NULL); g_return_val_if_fail (out_file_format != NULL, NULL); g_return_val_if_fail (*out_file_format == NM_CRYPTO_FILE_FORMAT_UNKNOWN, NULL); - array = file_to_g_byte_array (file, FALSE, error); - if (!array) + contents = file_to_g_byte_array (file, error); + if (!contents) + return NULL; + + /* Check for PKCS#12 */ + if (crypto_is_pkcs12_data (contents)) { + *out_file_format = NM_CRYPTO_FILE_FORMAT_PKCS12; + return contents; + } + + array = extract_pem_cert_data (contents, error); + if (!array) { + g_byte_array_free (contents, TRUE); return NULL; + } *out_file_format = crypto_verify_cert (array->data, array->len, error); - if (*out_file_format == NM_CRYPTO_FILE_FORMAT_UNKNOWN) { - /* Try PKCS#12 */ - if (crypto_is_pkcs12_data (array)) { - *out_file_format = NM_CRYPTO_FILE_FORMAT_PKCS12; - g_clear_error (error); - } else { - g_byte_array_free (array, TRUE); - array = NULL; - } + g_byte_array_free (array, TRUE); + + if (*out_file_format != NM_CRYPTO_FILE_FORMAT_X509) { + g_byte_array_free (contents, TRUE); + contents = NULL; } - return array; + return contents; } gboolean @@ -606,16 +609,14 @@ crypto_is_pkcs12_data (const GByteArray *data) g_return_val_if_fail (data != NULL, FALSE); success = crypto_verify_pkcs12 (data, NULL, &error); - if (success) - return TRUE; - - /* 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_ERR_CIPHER_DECRYPT_FAILED)) - success = TRUE; - g_error_free (error); + 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_ERR_CIPHER_DECRYPT_FAILED)) + success = TRUE; + g_error_free (error); + } } - return success; } @@ -627,7 +628,7 @@ crypto_is_pkcs12_file (const char *file, GError **error) g_return_val_if_fail (file != NULL, FALSE); - contents = file_to_g_byte_array (file, TRUE, error); + contents = file_to_g_byte_array (file, error); if (contents) { success = crypto_is_pkcs12_data (contents); g_byte_array_free (contents, TRUE); @@ -635,3 +636,52 @@ crypto_is_pkcs12_file (const char *file, GError **error) return success; } +/* Verifies that a private key can be read, and if a password is given, that + * the private key can be decrypted with that password. + */ +NMCryptoFileFormat +crypto_verify_private_key_data (const GByteArray *contents, + const char *password, + GError **error) +{ + GByteArray *tmp; + NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN; + NMCryptoKeyType ktype = NM_CRYPTO_KEY_TYPE_UNKNOWN; + + g_return_val_if_fail (contents != NULL, FALSE); + + /* Check for PKCS#12 first */ + if (crypto_is_pkcs12_data (contents)) { + if (!password || crypto_verify_pkcs12 (contents, password, error)) + format = NM_CRYPTO_FILE_FORMAT_PKCS12; + } else { + tmp = crypto_decrypt_private_key_data (contents, password, &ktype, error); + if (tmp) { + /* Don't leave decrypted key data around */ + memset (tmp->data, 0, tmp->len); + g_byte_array_free (tmp, TRUE); + format = NM_CRYPTO_FILE_FORMAT_RAW_KEY; + } else if (!password && (ktype != NM_CRYPTO_KEY_TYPE_UNKNOWN)) + format = NM_CRYPTO_FILE_FORMAT_RAW_KEY; + } + return format; +} + +NMCryptoFileFormat +crypto_verify_private_key (const char *filename, + const char *password, + GError **error) +{ + GByteArray *contents; + NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN; + + g_return_val_if_fail (filename != NULL, FALSE); + + contents = file_to_g_byte_array (filename, error); + if (contents) { + format = crypto_verify_private_key_data (contents, password, error); + g_byte_array_free (contents, TRUE); + } + return format; +} + diff --git a/libnm-util/crypto.h b/libnm-util/crypto.h index 38471cea63..cdf053e722 100644 --- a/libnm-util/crypto.h +++ b/libnm-util/crypto.h @@ -18,9 +18,12 @@ * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, * Boston, MA 02110-1301 USA. * - * (C) Copyright 2007 - 2008 Red Hat, Inc. + * (C) Copyright 2007 - 2011 Red Hat, Inc. */ +#ifndef __CRYPTO_H__ +#define __CRYPTO_H__ + #include <glib.h> #define MD5_HASH_LEN 20 @@ -51,8 +54,7 @@ enum { typedef enum { NM_CRYPTO_KEY_TYPE_UNKNOWN = 0, NM_CRYPTO_KEY_TYPE_RSA, - NM_CRYPTO_KEY_TYPE_DSA, - NM_CRYPTO_KEY_TYPE_ENCRYPTED + NM_CRYPTO_KEY_TYPE_DSA } NMCryptoKeyType; typedef enum { @@ -69,26 +71,31 @@ gboolean crypto_init (GError **error); void crypto_deinit (void); -GByteArray * crypto_get_private_key_data (GByteArray *contents, - const char *password, - NMCryptoKeyType *out_key_type, - NMCryptoFileFormat *out_file_format, - GError **error); +GByteArray *crypto_decrypt_private_key_data (const GByteArray *contents, + const char *password, + NMCryptoKeyType *out_key_type, + GError **error); -GByteArray * crypto_get_private_key (const char *file, - const char *password, - NMCryptoKeyType *out_key_type, - NMCryptoFileFormat *out_file_format, - GError **error); +GByteArray *crypto_decrypt_private_key (const char *file, + const char *password, + NMCryptoKeyType *out_key_type, + GError **error); -GByteArray * crypto_load_and_verify_certificate (const char *file, - NMCryptoFileFormat *out_file_format, - GError **error); +GByteArray *crypto_load_and_verify_certificate (const char *file, + NMCryptoFileFormat *out_file_format, + GError **error); gboolean crypto_is_pkcs12_file (const char *file, GError **error); gboolean crypto_is_pkcs12_data (const GByteArray *data); +NMCryptoFileFormat crypto_verify_private_key_data (const GByteArray *contents, + const char *password, + GError **error); + +NMCryptoFileFormat crypto_verify_private_key (const char *file, + const char *password, + GError **error); /* Internal utils API bits for crypto providers */ @@ -129,3 +136,4 @@ gboolean crypto_verify_pkcs12 (const GByteArray *data, const char *password, GError **error); +#endif /* __CRYPTO_H__ */ diff --git a/libnm-util/nm-setting-8021x.c b/libnm-util/nm-setting-8021x.c index cb1c95116c..9f4640d6e1 100644 --- a/libnm-util/nm-setting-8021x.c +++ b/libnm-util/nm-setting-8021x.c @@ -456,6 +456,22 @@ nm_setting_802_1x_get_ca_cert_path (NMSetting8021x *setting) return (const char *) (NM_SETTING_802_1X_GET_PRIVATE (setting)->ca_cert->data + strlen (SCHEME_PATH)); } +static GByteArray * +path_to_scheme_value (const char *path) +{ + GByteArray *array; + + g_return_val_if_fail (path != NULL, NULL); + + /* Add the path scheme tag to the front, then the fielname */ + array = g_byte_array_sized_new (strlen (path) + strlen (SCHEME_PATH) + 1); + g_assert (array); + g_byte_array_append (array, (const guint8 *) SCHEME_PATH, strlen (SCHEME_PATH)); + g_byte_array_append (array, (const guint8 *) path, strlen (path)); + g_byte_array_append (array, (const guint8 *) "\0", 1); + return array; +} + /** * nm_setting_802_1x_set_ca_cert: * @setting: the #NMSetting8021x @@ -530,13 +546,9 @@ nm_setting_802_1x_set_ca_cert (NMSetting8021x *self, if (data) { if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) priv->ca_cert = data; - else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) { - /* Add the path scheme tag to the front, then the fielname */ - priv->ca_cert = g_byte_array_sized_new (strlen (value) + strlen (SCHEME_PATH) + 1); - g_byte_array_append (priv->ca_cert, (const guint8 *) SCHEME_PATH, strlen (SCHEME_PATH)); - g_byte_array_append (priv->ca_cert, (const guint8 *) value, strlen (value)); - g_byte_array_append (priv->ca_cert, (const guint8 *) "\0", 1); - } else + else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) + priv->ca_cert = path_to_scheme_value (value); + else g_assert_not_reached (); } } @@ -690,13 +702,9 @@ nm_setting_802_1x_set_client_cert (NMSetting8021x *self, if (data) { if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) priv->client_cert = data; - else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) { - /* Add the path scheme tag to the front, then the fielname */ - priv->client_cert = g_byte_array_sized_new (strlen (value) + strlen (SCHEME_PATH) + 1); - g_byte_array_append (priv->client_cert, (const guint8 *) SCHEME_PATH, strlen (SCHEME_PATH)); - g_byte_array_append (priv->client_cert, (const guint8 *) value, strlen (value)); - g_byte_array_append (priv->client_cert, (const guint8 *) "\0", 1); - } else + else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) + priv->client_cert = path_to_scheme_value (value); + else g_assert_not_reached (); } } @@ -949,13 +957,9 @@ nm_setting_802_1x_set_phase2_ca_cert (NMSetting8021x *self, if (data) { if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) priv->phase2_ca_cert = data; - else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) { - /* Add the path scheme tag to the front, then the fielname */ - priv->phase2_ca_cert = g_byte_array_sized_new (strlen (value) + strlen (SCHEME_PATH) + 1); - g_byte_array_append (priv->phase2_ca_cert, (const guint8 *) SCHEME_PATH, strlen (SCHEME_PATH)); - g_byte_array_append (priv->phase2_ca_cert, (const guint8 *) value, strlen (value)); - g_byte_array_append (priv->phase2_ca_cert, (const guint8 *) "\0", 1); - } else + else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) + priv->phase2_ca_cert = path_to_scheme_value (value); + else g_assert_not_reached (); } } @@ -1111,13 +1115,9 @@ nm_setting_802_1x_set_phase2_client_cert (NMSetting8021x *self, if (data) { if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) priv->phase2_client_cert = data; - else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) { - /* Add the path scheme tag to the front, then the fielname */ - priv->phase2_client_cert = g_byte_array_sized_new (strlen (value) + strlen (SCHEME_PATH) + 1); - g_byte_array_append (priv->phase2_client_cert, (const guint8 *) SCHEME_PATH, strlen (SCHEME_PATH)); - g_byte_array_append (priv->phase2_client_cert, (const guint8 *) value, strlen (value)); - g_byte_array_append (priv->phase2_client_cert, (const guint8 *) "\0", 1); - } else + else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) + priv->phase2_client_cert = path_to_scheme_value (value); + else g_assert_not_reached (); } } @@ -1254,6 +1254,24 @@ nm_setting_802_1x_get_private_key_path (NMSetting8021x *setting) return (const char *) (NM_SETTING_802_1X_GET_PRIVATE (setting)->private_key->data + strlen (SCHEME_PATH)); } +static GByteArray * +file_to_byte_array (const char *filename) +{ + char *contents; + GByteArray *array = NULL; + gsize length = 0; + + if (g_file_get_contents (filename, &contents, &length, NULL)) { + array = g_byte_array_sized_new (length); + if (array) { + g_byte_array_append (array, (guint8 *) contents, length); + g_assert (array->len == length); + } + g_free (contents); + } + return array; +} + /** * nm_setting_802_1x_set_private_key: * @setting: the #NMSetting8021x @@ -1262,20 +1280,30 @@ nm_setting_802_1x_get_private_key_path (NMSetting8021x *setting) * (PEM, DER, or PKCS#12 format). The path must be UTF-8 encoded; use * g_filename_to_utf8() to convert if needed. Passing NULL with any @scheme * clears the private key. - * @password: password used to decrypt the private key + * @password: password used to decrypt the private key, or %NULL if the password + * is unknown. If the password is given but fails to decrypt the private key, + * an error is returned. * @scheme: desired storage scheme for the private key * @out_format: on successful return, the type of the private key added * @error: on unsuccessful return, an error * - * Reads a private key from disk and sets the #NMSetting8021x:private-key - * property with the raw private key data if using the - * %NM_SETTING_802_1X_CK_SCHEME_BLOB scheme, or with the path to the private key - * file if using the %NM_SETTING_802_1X_CK_SCHEME_PATH scheme. - * * Private keys are used to authenticate the connecting client to the network * when EAP-TLS is used as either the "phase 1" or "phase 2" 802.1x * authentication method. * + * This function reads a private key from disk and sets the + * #NMSetting8021x:private-key property with the private key file data if using + * the %NM_SETTING_802_1X_CK_SCHEME_BLOB scheme, or with the path to the private + * key file if using the %NM_SETTING_802_1X_CK_SCHEME_PATH scheme. + * + * If @password is given, this function attempts to decrypt the private key to + * verify that @password is correct, and if it is, updates the + * #NMSetting8021x:private-key-password property with the given @password. If + * the decryption is unsuccessful, %FALSE is returned, @error is set, and no + * internal data is changed. If no @password is given, the private key is + * assumed to be valid, no decryption is performed, and the password may be set + * at a later time. + * * WARNING: the private key property is not a "secret" property, and thus * unencrypted private key data using the BLOB scheme may be readable by * unprivileged users. Private keys should always be encrypted with a private @@ -1293,8 +1321,6 @@ nm_setting_802_1x_set_private_key (NMSetting8021x *self, { NMSetting8021xPrivate *priv; NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN; - NMCryptoKeyType key_type = NM_CRYPTO_KEY_TYPE_UNKNOWN; - GByteArray *data; g_return_val_if_fail (NM_IS_SETTING_802_1X (self), FALSE); @@ -1308,12 +1334,26 @@ nm_setting_802_1x_set_private_key (NMSetting8021x *self, if (out_format) g_return_val_if_fail (*out_format == NM_SETTING_802_1X_CK_FORMAT_UNKNOWN, FALSE); + /* Ensure the private key is a recognized format and if the password was + * given, that it decrypts the private key. + */ + if (value) { + format = crypto_verify_private_key (value, password, NULL); + if (format == NM_CRYPTO_FILE_FORMAT_UNKNOWN) { + g_set_error (error, + NM_SETTING_802_1X_ERROR, + NM_SETTING_802_1X_ERROR_INVALID_PROPERTY, + NM_SETTING_802_1X_PRIVATE_KEY); + return FALSE; + } + } + priv = NM_SETTING_802_1X_GET_PRIVATE (self); - /* Clear out any previous private key blob */ + /* Clear out any previous private key data */ if (priv->private_key) { /* Try not to leave the private key around in memory */ - memset (priv->private_key, 0, priv->private_key->len); + memset (priv->private_key->data, 0, priv->private_key->len); g_byte_array_free (priv->private_key, TRUE); priv->private_key = NULL; } @@ -1321,81 +1361,23 @@ nm_setting_802_1x_set_private_key (NMSetting8021x *self, g_free (priv->private_key_password); priv->private_key_password = NULL; - if (!value) + if (value == NULL) return TRUE; - /* Verify the key and the private key password */ - data = crypto_get_private_key (value, - password, - &key_type, - &format, - error); - if (!data) { - /* As a special case for private keys, even if the decrypt fails, - * return the key's file type. - */ - if (out_format && crypto_is_pkcs12_file (value, NULL)) - *out_format = NM_SETTING_802_1X_CK_FORMAT_PKCS12; - - return FALSE; - } - - switch (format) { - case NM_CRYPTO_FILE_FORMAT_RAW_KEY: - if (out_format) - *out_format = NM_SETTING_802_1X_CK_FORMAT_RAW_KEY; - break; - case NM_CRYPTO_FILE_FORMAT_X509: - if (out_format) - *out_format = NM_SETTING_802_1X_CK_FORMAT_X509; - break; - case NM_CRYPTO_FILE_FORMAT_PKCS12: - if (out_format) - *out_format = NM_SETTING_802_1X_CK_FORMAT_PKCS12; - break; - default: - memset (data->data, 0, data->len); - g_byte_array_free (data, TRUE); - g_set_error (error, - NM_SETTING_802_1X_ERROR, - NM_SETTING_802_1X_ERROR_INVALID_PROPERTY, - NM_SETTING_802_1X_PRIVATE_KEY); - return FALSE; - } - - g_assert (data); + priv->private_key_password = g_strdup (password); if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) { - priv->private_key = data; - data = NULL; - - /* Always update the private key for blob + pkcs12 since the - * pkcs12 files are encrypted - */ - if (format == NM_CRYPTO_FILE_FORMAT_PKCS12) - priv->private_key_password = g_strdup (password); - } else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) { - /* Add the path scheme tag to the front, then the fielname */ - priv->private_key = g_byte_array_sized_new (strlen (value) + strlen (SCHEME_PATH) + 1); - g_byte_array_append (priv->private_key, (const guint8 *) SCHEME_PATH, strlen (SCHEME_PATH)); - g_byte_array_append (priv->private_key, (const guint8 *) value, strlen (value)); - g_byte_array_append (priv->private_key, (const guint8 *) "\0", 1); - - /* Always update the private key with paths since the key the - * cert refers to is encrypted. - */ - priv->private_key_password = g_strdup (password); - } else + /* Shouldn't fail this since we just verified the private key above */ + priv->private_key = file_to_byte_array (value); + g_assert (priv->private_key); + } else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) + priv->private_key = path_to_scheme_value (value); + else g_assert_not_reached (); - /* Clear and free private key data if it's no longer needed */ - if (data) { - memset (data->data, 0, data->len); - g_byte_array_free (data, TRUE); - } - /* As required by NM and wpa_supplicant, set the client-cert * property to the same PKCS#12 data. */ + g_assert (format != NM_CRYPTO_FILE_FORMAT_UNKNOWN); if (format == NM_CRYPTO_FILE_FORMAT_PKCS12) { if (priv->client_cert) g_byte_array_free (priv->client_cert, TRUE); @@ -1404,6 +1386,8 @@ nm_setting_802_1x_set_private_key (NMSetting8021x *self, g_byte_array_append (priv->client_cert, priv->private_key->data, priv->private_key->len); } + if (out_format) + *out_format = format; return priv->private_key != NULL; } @@ -1463,7 +1447,7 @@ nm_setting_802_1x_get_private_key_format (NMSetting8021x *setting) case NM_SETTING_802_1X_CK_SCHEME_BLOB: if (crypto_is_pkcs12_data (priv->private_key)) return NM_SETTING_802_1X_CK_FORMAT_PKCS12; - return NM_SETTING_802_1X_CK_FORMAT_X509; + 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)) @@ -1473,7 +1457,7 @@ nm_setting_802_1x_get_private_key_format (NMSetting8021x *setting) g_error_free (error); return NM_SETTING_802_1X_CK_FORMAT_UNKNOWN; } - return NM_SETTING_802_1X_CK_FORMAT_X509; + return NM_SETTING_802_1X_CK_FORMAT_RAW_KEY; default: break; } @@ -1587,26 +1571,36 @@ nm_setting_802_1x_get_phase2_private_key_path (NMSetting8021x *setting) * nm_setting_802_1x_set_phase2_private_key: * @setting: the #NMSetting8021x * @value: when @scheme is set to either %NM_SETTING_802_1X_CK_SCHEME_PATH or - * %NM_SETTING_802_1X_CK_SCHEME_BLOB, pass the path of the "phase2" private + * %NM_SETTING_802_1X_CK_SCHEME_BLOB, pass the path of the "phase2" private * key file (PEM, DER, or PKCS#12 format). The path must be UTF-8 encoded; * use g_filename_to_utf8() to convert if needed. Passing NULL with any - * @scheme clears the "phase2" private key. - * @password: password used to decrypt the private key + * @scheme clears the private key. + * @password: password used to decrypt the private key, or %NULL if the password + * is unknown. If the password is given but fails to decrypt the private key, + * an error is returned. * @scheme: desired storage scheme for the private key * @out_format: on successful return, the type of the private key added * @error: on unsuccessful return, an error * - * Reads a "phase 2" private key from disk and sets the - * #NMSetting8021x:phase2-private-key property with the raw private key data if - * using the %NM_SETTING_802_1X_CK_SCHEME_BLOB scheme, or with the path to the - * private key file if using the %NM_SETTING_802_1X_CK_SCHEME_PATH scheme. - * * Private keys are used to authenticate the connecting client to the network * when EAP-TLS is used as either the "phase 1" or "phase 2" 802.1x * authentication method. * - * WARNING: the phase2 private key property is not a "secret" property, and thus - * unencrypted private key data using the BLOB scheme may be readable by + * This function reads a private key from disk and sets the + * #NMSetting8021x:phase2-private-key property with the private key file data if + * using the %NM_SETTING_802_1X_CK_SCHEME_BLOB scheme, or with the path to the + * private key file if using the %NM_SETTING_802_1X_CK_SCHEME_PATH scheme. + * + * If @password is given, this function attempts to decrypt the private key to + * verify that @password is correct, and if it is, updates the + * #NMSetting8021x:phase2-private-key-password property with the given + * @password. If the decryption is unsuccessful, %FALSE is returned, @error is + * set, and no internal data is changed. If no @password is given, the private + * key is assumed to be valid, no decryption is performed, and the password may + * be set at a later time. + * + * WARNING: the "phase2" private key property is not a "secret" property, and + * thus unencrypted private key data using the BLOB scheme may be readable by * unprivileged users. Private keys should always be encrypted with a private * key password to prevent unauthorized access to unencrypted private key data. * @@ -1622,8 +1616,6 @@ nm_setting_802_1x_set_phase2_private_key (NMSetting8021x *self, { NMSetting8021xPrivate *priv; NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN; - NMCryptoKeyType key_type = NM_CRYPTO_KEY_TYPE_UNKNOWN; - GByteArray *data; g_return_val_if_fail (NM_IS_SETTING_802_1X (self), FALSE); @@ -1637,12 +1629,26 @@ nm_setting_802_1x_set_phase2_private_key (NMSetting8021x *self, if (out_format) g_return_val_if_fail (*out_format == NM_SETTING_802_1X_CK_FORMAT_UNKNOWN, FALSE); + /* Ensure the private key is a recognized format and if the password was + * given, that it decrypts the private key. + */ + if (value) { + format = crypto_verify_private_key (value, password, NULL); + if (format == NM_CRYPTO_FILE_FORMAT_UNKNOWN) { + g_set_error (error, + NM_SETTING_802_1X_ERROR, + NM_SETTING_802_1X_ERROR_INVALID_PROPERTY, + NM_SETTING_802_1X_PHASE2_PRIVATE_KEY); + return FALSE; + } + } + priv = NM_SETTING_802_1X_GET_PRIVATE (self); - /* Clear out any previous private key blob */ + /* Clear out any previous private key data */ if (priv->phase2_private_key) { /* Try not to leave the private key around in memory */ - memset (priv->phase2_private_key, 0, priv->phase2_private_key->len); + memset (priv->phase2_private_key->data, 0, priv->phase2_private_key->len); g_byte_array_free (priv->phase2_private_key, TRUE); priv->phase2_private_key = NULL; } @@ -1650,81 +1656,23 @@ nm_setting_802_1x_set_phase2_private_key (NMSetting8021x *self, g_free (priv->phase2_private_key_password); priv->phase2_private_key_password = NULL; - if (!value) + if (value == NULL) return TRUE; - /* Verify the key and the private key password */ - data = crypto_get_private_key (value, - password, - &key_type, - &format, - error); - if (!data) { - /* As a special case for private keys, even if the decrypt fails, - * return the key's file type. - */ - if (out_format && crypto_is_pkcs12_file (value, NULL)) - *out_format = NM_SETTING_802_1X_CK_FORMAT_PKCS12; - - return FALSE; - } - - switch (format) { - case NM_CRYPTO_FILE_FORMAT_RAW_KEY: - if (out_format) - *out_format = NM_SETTING_802_1X_CK_FORMAT_RAW_KEY; - break; - case NM_CRYPTO_FILE_FORMAT_X509: - if (out_format) - *out_format = NM_SETTING_802_1X_CK_FORMAT_X509; - break; - case NM_CRYPTO_FILE_FORMAT_PKCS12: - if (out_format) - *out_format = NM_SETTING_802_1X_CK_FORMAT_PKCS12; - break; - default: - memset (data->data, 0, data->len); - g_byte_array_free (data, TRUE); - g_set_error (error, - NM_SETTING_802_1X_ERROR, - NM_SETTING_802_1X_ERROR_INVALID_PROPERTY, - NM_SETTING_802_1X_PHASE2_PRIVATE_KEY); - return FALSE; - } - - g_assert (data); + priv->phase2_private_key_password = g_strdup (password); if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) { - priv->phase2_private_key = data; - data = NULL; - - /* Always update the private key for blob + pkcs12 since the - * pkcs12 files are encrypted - */ - if (format == NM_CRYPTO_FILE_FORMAT_PKCS12) - priv->phase2_private_key_password = g_strdup (password); - } else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) { - /* Add the path scheme tag to the front, then the fielname */ - priv->phase2_private_key = g_byte_array_sized_new (strlen (value) + strlen (SCHEME_PATH) + 1); - g_byte_array_append (priv->phase2_private_key, (const guint8 *) SCHEME_PATH, strlen (SCHEME_PATH)); - g_byte_array_append (priv->phase2_private_key, (const guint8 *) value, strlen (value)); - g_byte_array_append (priv->phase2_private_key, (const guint8 *) "\0", 1); - - /* Always update the private key with paths since the key the - * cert refers to is encrypted. - */ - priv->phase2_private_key_password = g_strdup (password); - } else + /* Shouldn't fail this since we just verified the private key above */ + priv->phase2_private_key = file_to_byte_array (value); + g_assert (priv->phase2_private_key); + } else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) + priv->phase2_private_key = path_to_scheme_value (value); + else g_assert_not_reached (); - /* Clear and free private key data if it's no longer needed */ - if (data) { - memset (data->data, 0, data->len); - g_byte_array_free (data, TRUE); - } - /* As required by NM and wpa_supplicant, set the client-cert * property to the same PKCS#12 data. */ + g_assert (format != NM_CRYPTO_FILE_FORMAT_UNKNOWN); if (format == NM_CRYPTO_FILE_FORMAT_PKCS12) { if (priv->phase2_client_cert) g_byte_array_free (priv->phase2_client_cert, TRUE); @@ -1733,6 +1681,8 @@ nm_setting_802_1x_set_phase2_private_key (NMSetting8021x *self, g_byte_array_append (priv->phase2_client_cert, priv->phase2_private_key->data, priv->phase2_private_key->len); } + if (out_format) + *out_format = format; return priv->phase2_private_key != NULL; } @@ -1760,7 +1710,7 @@ nm_setting_802_1x_get_phase2_private_key_format (NMSetting8021x *setting) case NM_SETTING_802_1X_CK_SCHEME_BLOB: if (crypto_is_pkcs12_data (priv->phase2_private_key)) return NM_SETTING_802_1X_CK_FORMAT_PKCS12; - return NM_SETTING_802_1X_CK_FORMAT_X509; + 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)) @@ -1770,7 +1720,7 @@ nm_setting_802_1x_get_phase2_private_key_format (NMSetting8021x *setting) g_error_free (error); return NM_SETTING_802_1X_CK_FORMAT_UNKNOWN; } - return NM_SETTING_802_1X_CK_FORMAT_X509; + return NM_SETTING_802_1X_CK_FORMAT_RAW_KEY; default: break; } @@ -1805,35 +1755,19 @@ need_private_key_password (const GByteArray *blob, const char *path, const char *password) { - /* Private key password is only un-needed if the private key scheme is BLOB, - * because BLOB keys are decrypted by the settings service. A private key - * password is required if the private key is PKCS#12 format, or if the - * private key scheme is PATH. - */ - if (path) { - GByteArray *tmp; - NMCryptoKeyType key_type = NM_CRYPTO_KEY_TYPE_UNKNOWN; - NMCryptoFileFormat key_format = NM_CRYPTO_FILE_FORMAT_UNKNOWN; - - /* check the password */ - tmp = crypto_get_private_key (path, password, &key_type, &key_format, NULL); - if (tmp) { - /* Decrypt/verify successful; password must be OK */ - g_byte_array_free (tmp, TRUE); - return FALSE; - } - } else if (blob) { - /* Non-PKCS#12 blob-scheme keys are already decrypted by their settings - * service, thus if the private key is not PKCS#12 format, a new password - * is not required. If the PKCS#12 key can be decrypted with the given - * password, then we don't need a new password either. - */ - if (!crypto_is_pkcs12_data (blob) || crypto_verify_pkcs12 (blob, password, NULL)) - return FALSE; - } else - g_warning ("%s: unknown private key password scheme", __func__); + NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN; - return TRUE; + /* Private key password is required */ + if (password) { + if (path) + format = crypto_verify_private_key (path, password, NULL); + else if (blob) + format = crypto_verify_private_key_data (blob, password, NULL); + else + g_warning ("%s: unknown private key password scheme", __func__); + } + + return (format == NM_CRYPTO_FILE_FORMAT_UNKNOWN); } static void @@ -3000,31 +2934,26 @@ nm_setting_802_1x_class_init (NMSetting8021xClass *setting_class) "Contains the private key when the 'eap' property " "is set to 'tls'. Key data is specified using a " "'scheme'; two are currently supported: blob and " - "path. When using the blob scheme and X.509 private " - "keys, this property should be set to the keys's " - "PEM or DER encoded data; if using DER-encoded " - "data the private key must be decrypted as the " - "DER format is incomplete. Use of decrypted " - "DER-format private keys is not recommended as it " - "may allow unprivileged users access to the " - "decrypted data. When using X.509 private keys " - "with the path scheme, this property should be " - "set to the full UTF-8 encoded path of the key, " + "path. When using the blob scheme and private " + "keys, this property should be set to the key's " + "encrypted PEM encoded data. When using private " + "keys with the path scheme, this property should " + "be set to the full UTF-8 encoded path of the key, " "prefixed with the string 'file://' and ending " "with a terminating NULL byte. When using " "PKCS#12 format private keys and the blob " "scheme, this property should be set to the " - "PKCS#12 data (which is encrypted) and the " + "PKCS#12 data and the 'private-key-password' " + "property must be set to password used to " + "decrypt the PKCS#12 certificate and key. When " + "using PKCS#12 files and the path scheme, this " + "property should be set to the full UTF-8 encoded " + "path of the key, prefixed with the string " + "'file://' and and ending with a terminating NULL " + "byte, and as with the blob scheme the " "'private-key-password' property must be set to " - "password used to decrypt the PKCS#12 certificate " - "and key. When using PKCS#12 files and the path " - "scheme, this property should be set to the full " - "UTF-8 encoded path of the key, prefixed with the " - "string 'file://' and and ending with a " - "terminating NULL byte, and as with the blob " - "scheme the 'private-key-password' property must " - "be set to the password used to decode the PKCS#12 " - "private key and certificate.", + "the password used to decode the PKCS#12 private " + "key and certificate.", DBUS_TYPE_G_UCHAR_ARRAY, G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE)); @@ -3082,30 +3011,25 @@ nm_setting_802_1x_class_init (NMSetting8021xClass *setting_class) "the 'phase2-eap' or 'phase2-autheap' property " "is set to 'tls'. Key data is specified using a " "'scheme'; two are currently supported: blob and " - "path. When using the blob scheme and X.509 private " - "keys, this property should be set to the keys's " - "PEM or DER encoded data; if using DER-encoded " - "data the private key must be decrypted as the " - "DER format is incomplete. Use of decrypted " - "DER-format private keys is not recommended as it " - "may allow unprivileged users access to the " - "decrypted data. When using X.509 private keys " - "with the path scheme, this property should be " - "set to the full UTF-8 encoded path of the key, " + "path. When using the blob scheme and private " + "keys, this property should be set to the key's " + "encrypted PEM encoded data. When using private " + "keys with the path scheme, this property should " + "be set to the full UTF-8 encoded path of the key, " "prefixed with the string 'file://' and ending " "with a terminating NULL byte. When using " "PKCS#12 format private keys and the blob " "scheme, this property should be set to the " - "PKCS#12 data (which is encrypted) and the " - "'private-key-password' property must be set to " - "password used to decrypt the PKCS#12 certificate " - "and key. When using PKCS#12 files and the path " - "scheme, this property should be set to the full " - "UTF-8 encoded path of the key, prefixed with the " - "string 'file://' and and ending with a " - "terminating NULL byte, and as with the blob " - "scheme the 'private-key-password' property must " - "be set to the password used to decode the PKCS#12 " + "PKCS#12 data and the 'phase2-private-key-password' " + "property must be set to password used to " + "decrypt the PKCS#12 certificate and key. When " + "using PKCS#12 files and the path scheme, this " + "property should be set to the full UTF-8 encoded " + "path of the key, prefixed with the string " + "'file://' and and ending with a terminating NULL " + "byte, and as with the blob scheme the " + "'phase2-private-key-password' property must be " + "set to the password used to decode the PKCS#12 " "private key and certificate.", DBUS_TYPE_G_UCHAR_ARRAY, G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE)); diff --git a/libnm-util/nm-setting-8021x.h b/libnm-util/nm-setting-8021x.h index 00cfedd043..bf587a9047 100644 --- a/libnm-util/nm-setting-8021x.h +++ b/libnm-util/nm-setting-8021x.h @@ -30,6 +30,18 @@ G_BEGIN_DECLS +/** + * NMSetting8021xCKFormat: + * @NM_SETTING_802_1X_CK_FORMAT_UNKNOWN: unknown file format + * @NM_SETTING_802_1X_CK_FORMAT_X509: file contains an X.509 format certificate + * @NM_SETTING_802_1X_CK_FORMAT_RAW_KEY: file contains an old-style OpenSSL PEM + * or DER private key + * @NM_SETTING_802_1X_CK_FORMAT_PKCS12: file contains a PKCS#12 certificate + * and private key + * + * #NMSetting8021xCKFormat values indicate the general type of a certificate + * or private key + */ typedef enum { NM_SETTING_802_1X_CK_FORMAT_UNKNOWN = 0, NM_SETTING_802_1X_CK_FORMAT_X509, @@ -37,12 +49,26 @@ typedef enum { NM_SETTING_802_1X_CK_FORMAT_PKCS12 } NMSetting8021xCKFormat; +/** + * NMSetting8021xCKScheme: + * @NM_SETTING_802_1X_CK_SCHEME_UNKNOWN: unknown certificate or private key + * scheme + * @NM_SETTING_802_1X_CK_SCHEME_BLOB: certificate or key is stored as the raw + * item data + * @NM_SETTING_802_1X_CK_SCHEME_PATH: certificate or key is stored as a path + * to a file containing the certificate or key data + * + * #NMSetting8021xCKScheme values indicate how a certificate or private key is + * stored in the setting properties, either as a blob of the item's data, or as + * a path to a certificate or private key file on the filesystem + */ typedef enum { NM_SETTING_802_1X_CK_SCHEME_UNKNOWN = 0, NM_SETTING_802_1X_CK_SCHEME_BLOB, NM_SETTING_802_1X_CK_SCHEME_PATH } NMSetting8021xCKScheme; + #define NM_TYPE_SETTING_802_1X (nm_setting_802_1x_get_type ()) #define NM_SETTING_802_1X(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_SETTING_802_1X, NMSetting8021x)) #define NM_SETTING_802_1X_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_SETTING_802_1X, NMSetting8021xClass)) @@ -93,29 +119,23 @@ GQuark nm_setting_802_1x_error_quark (void); #define NM_SETTING_802_1X_SYSTEM_CA_CERTS "system-ca-certs" /* PRIVATE KEY NOTE: when setting PKCS#12 private keys directly via properties - * using the "blob" scheme, the data must be passed in PKCS#12 format. In this - * case, the private key password must also be passed to NetworkManager, and the - * appropriate "client-cert" (or "phase2-client-cert") property of the - * NMSetting8021x object must also contain the exact same PKCS#12 data that the - * private key will when NetworkManager requests secrets. This is because the + * using the "blob" scheme, the data must be passed in PKCS#12 binary format. + * In this case, the appropriate "client-cert" (or "phase2-client-cert") + * property of the NMSetting8021x object must also contain the exact same + * PKCS#12 binary data that the private key does. This is because the * PKCS#12 file contains both the private key and client certificate, so both * properties need to be set to the same thing. When using the "path" scheme, - * just set both the private-key and client-cert properties to the same path, - * and set the private-key password correctly. + * just set both the private-key and client-cert properties to the same path. * * When setting OpenSSL-derived "traditional" format (ie S/MIME style, not * PKCS#8) RSA and DSA keys directly via properties with the "blob" scheme, they * should be passed to NetworkManager in PEM format with the "DEK-Info" and - * "Proc-Type" tags intact, or in decrypted binary DER format (not recommended, - * as this may allow unprivileged users to read the decrypted private key). - * When decryped keys are used (again, not recommended) the private key password - * should not be set. The recommended method for passing private keys to - * NetworkManager is via the "path" scheme with encrypted private keys, and a - * private key password. + * "Proc-Type" tags intact. Decrypted private keys should not be used as this + * is insecure and could allow unprivileged users to access the decrypted + * private key data. * * When using the "path" scheme, just set the private-key and client-cert - * properties to the paths to their respective objects, and set the private-key - * password correctly. + * properties to the paths to their respective objects. */ typedef struct { diff --git a/libnm-util/tests/Makefile.am b/libnm-util/tests/Makefile.am index 378a7388a4..1f45c6c85c 100644 --- a/libnm-util/tests/Makefile.am +++ b/libnm-util/tests/Makefile.am @@ -79,61 +79,49 @@ check-local: test-settings-defaults test-crypto test-need-secrets $(abs_builddir)/test-general # Private key and CA certificate in the same file (PEM) - $(abs_builddir)/test-setting-8021x \ - $(top_srcdir)/libnm-util/tests/certs/test_key_and_cert.pem \ - "test" \ - $(top_srcdir)/libnm-util/tests/certs/test-key-only-decrypted.der + $(abs_builddir)/test-setting-8021x $(srcdir)/certs/test_key_and_cert.pem "test" # Private key by itself (PEM) - $(abs_builddir)/test-setting-8021x \ - $(top_srcdir)/libnm-util/tests/certs/test-key-only.pem \ - "test" \ - $(top_srcdir)/libnm-util/tests/certs/test-key-only-decrypted.der + $(abs_builddir)/test-setting-8021x $(srcdir)/certs/test-key-only.pem "test" # Private key and CA certificate in the same file (pkcs12) - $(abs_builddir)/test-setting-8021x \ - $(top_srcdir)/libnm-util/tests/certs/test-cert.p12 \ - "test" + $(abs_builddir)/test-setting-8021x $(srcdir)/certs/test-cert.p12 "test" # Normal CA certificate - $(abs_builddir)/test-crypto --cert \ - $(top_srcdir)/libnm-util/tests/certs/test_ca_cert.pem + $(abs_builddir)/test-crypto --cert $(srcdir)/certs/test_ca_cert.pem # Another CA certificate - $(abs_builddir)/test-crypto --cert \ - $(top_srcdir)/libnm-util/tests/certs/test2_ca_cert.pem + $(abs_builddir)/test-crypto --cert $(srcdir)/certs/test2_ca_cert.pem # CA certificate without an ending newline - $(abs_builddir)/test-crypto --cert \ - $(top_srcdir)/libnm-util/tests/certs/ca-no-ending-newline.pem + $(abs_builddir)/test-crypto --cert $(srcdir)/certs/ca-no-ending-newline.pem # Combined user cert and private key - $(abs_builddir)/test-crypto --cert \ - $(top_srcdir)/libnm-util/tests/certs/test_key_and_cert.pem + $(abs_builddir)/test-crypto --cert $(srcdir)/certs/test_key_and_cert.pem # Another combined user cert and private key - $(abs_builddir)/test-crypto --cert \ - $(top_srcdir)/libnm-util/tests/certs/test2_key_and_cert.pem + $(abs_builddir)/test-crypto --cert $(srcdir)/certs/test2_key_and_cert.pem # Private key with 8 bytes of tail padding $(abs_builddir)/test-crypto --key \ - $(top_srcdir)/libnm-util/tests/certs/test_key_and_cert.pem \ - "test" + $(srcdir)/certs/test_key_and_cert.pem \ + "test" \ + $(srcdir)/certs/test-key-only-decrypted.der -# Private key with 6 bytes of tail padding +# Private key only (not combined with a cert) $(abs_builddir)/test-crypto --key \ - $(top_srcdir)/libnm-util/tests/certs/test2_key_and_cert.pem \ - "12345testing" + $(srcdir)/certs/test-key-only.pem \ + "test" \ + $(srcdir)/certs/test-key-only-decrypted.der + +# Private key with 6 bytes of tail padding + $(abs_builddir)/test-crypto --key $(srcdir)/certs/test2_key_and_cert.pem "12345testing" # PKCS#12 file - $(abs_builddir)/test-crypto --p12 \ - $(top_srcdir)/libnm-util/tests/certs/test-cert.p12 \ - "test" + $(abs_builddir)/test-crypto --p12 $(srcdir)/certs/test-cert.p12 "test" # Another PKCS#12 file - $(abs_builddir)/test-crypto --p12 \ - $(top_srcdir)/libnm-util/tests/certs/test2-cert.p12 \ - "12345testing" + $(abs_builddir)/test-crypto --p12 $(srcdir)/certs/test2-cert.p12 "12345testing" endif diff --git a/libnm-util/tests/test-crypto.c b/libnm-util/tests/test-crypto.c index a5466bc832..6cfb6ac277 100644 --- a/libnm-util/tests/test-crypto.c +++ b/libnm-util/tests/test-crypto.c @@ -18,7 +18,7 @@ * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, * Boston, MA 02110-1301 USA. * - * (C) Copyright 2007 - 2009 Red Hat, Inc. + * (C) Copyright 2007 - 2011 Red Hat, Inc. */ #include <glib.h> @@ -113,28 +113,46 @@ test_load_cert (const char *path, const char *desc) g_byte_array_free (array, TRUE); } +static GByteArray * +file_to_byte_array (const char *filename) +{ + char *contents; + GByteArray *array = NULL; + gsize length = 0; + + if (g_file_get_contents (filename, &contents, &length, NULL)) { + array = g_byte_array_sized_new (length); + if (array) { + g_byte_array_append (array, (guint8 *) contents, length); + g_assert (array->len == length); + } + g_free (contents); + } + return array; +} + static void test_load_private_key (const char *path, const char *password, + const char *decrypted_path, gboolean expect_fail, const char *desc) { NMCryptoKeyType key_type = NM_CRYPTO_KEY_TYPE_UNKNOWN; - NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN; - GByteArray *array; + GByteArray *array, *decrypted; GError *error = NULL; - array = crypto_get_private_key (path, password, &key_type, &format, &error); + array = crypto_decrypt_private_key (path, password, &key_type, &error); if (expect_fail) { ASSERT (array == NULL, desc, "unexpected success reading private key file '%s' with " "invalid password", path); - ASSERT (format == NM_CRYPTO_FILE_FORMAT_UNKNOWN, desc, - "unexpected success determining private key file '%s' " - "format with invalid password (expected %d, got %d)", - path, NM_CRYPTO_FILE_FORMAT_UNKNOWN, format); + ASSERT (key_type != NM_CRYPTO_KEY_TYPE_UNKNOWN, desc, + "unexpected failure determining private key file '%s' " + "type with invalid password (expected %d, got %d)", + path, NM_CRYPTO_KEY_TYPE_UNKNOWN, key_type); return; } @@ -142,13 +160,28 @@ test_load_private_key (const char *path, "couldn't read private key file '%s': %d %s", path, error->code, error->message); - ASSERT (format == NM_CRYPTO_FILE_FORMAT_RAW_KEY, desc, - "%s: unexpected private key file format (expected %d, got %d)", - path, NM_CRYPTO_FILE_FORMAT_RAW_KEY, format); - ASSERT (key_type == NM_CRYPTO_KEY_TYPE_RSA, desc, "%s: unexpected private key type (expected %d, got %d)", - path, NM_CRYPTO_KEY_TYPE_RSA, format); + path, NM_CRYPTO_KEY_TYPE_RSA, key_type); + + if (decrypted_path) { + /* Compare the crypto decrypted key against a known-good decryption */ + decrypted = file_to_byte_array (decrypted_path); + ASSERT (decrypted != NULL, desc, + "couldn't read decrypted private key file '%s': %d %s", + decrypted_path, error->code, error->message); + + ASSERT (decrypted->len > 0, desc, "decrypted key file invalid (size 0)"); + + ASSERT (decrypted->len == array->len, + desc, "decrypted key file (%d) and decrypted key data (%d) lengths don't match", + decrypted->len, array->len); + + ASSERT (memcmp (decrypted->data, array->data, array->len) == 0, + desc, "decrypted key file and decrypted key data don't match"); + + g_byte_array_free (decrypted, TRUE); + } g_byte_array_free (array, TRUE); } @@ -159,46 +192,35 @@ test_load_pkcs12 (const char *path, gboolean expect_fail, const char *desc) { - NMCryptoKeyType key_type = NM_CRYPTO_KEY_TYPE_UNKNOWN; NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN; - GByteArray *array; GError *error = NULL; - array = crypto_get_private_key (path, password, &key_type, &format, &error); + format = crypto_verify_private_key (path, password, &error); if (expect_fail) { - ASSERT (array == NULL, desc, + ASSERT (format == NM_CRYPTO_FILE_FORMAT_UNKNOWN, desc, "unexpected success reading PKCS#12 private key file " "'%s' with invalid password", path); - - /* PKCS#12 file format can be determined even if the password - * is wrong; check that. - */ - ASSERT (format == NM_CRYPTO_FILE_FORMAT_UNKNOWN, desc, - "unexpected success determining PKCS#12 private key " - "'%s' file format with invalid password (expected %d, " - "got %d)", - path, NM_CRYPTO_FILE_FORMAT_UNKNOWN, format); - ASSERT (key_type == NM_CRYPTO_KEY_TYPE_UNKNOWN, desc, - "unexpected success determining PKCS#12 private key " - "'%s' type with invalid password (expected %d, got %d)", - path, NM_CRYPTO_KEY_TYPE_UNKNOWN, key_type); - return; + } else { + ASSERT (format == NM_CRYPTO_FILE_FORMAT_PKCS12, desc, + "%s: unexpected PKCS#12 private key file format (expected %d, got " + "%d): %d %s", + path, NM_CRYPTO_FILE_FORMAT_PKCS12, format, error->code, error->message); } +} - ASSERT (array != NULL, desc, - "couldn't read PKCS#12 private key file '%s': %d %s", - path, error->code, error->message); +static void +test_load_pkcs12_no_password (const char *path, const char *desc) +{ + NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN; + GError *error = NULL; + /* We should still get a valid returned crypto file format */ + format = crypto_verify_private_key (path, NULL, &error); ASSERT (format == NM_CRYPTO_FILE_FORMAT_PKCS12, desc, - "%s: unexpected PKCS#12 private key file format (expected %d, got %d)", - path, NM_CRYPTO_FILE_FORMAT_RAW_KEY, format); - - ASSERT (key_type == NM_CRYPTO_KEY_TYPE_ENCRYPTED, desc, - "%s: unexpected PKCS#12 private key type (expected %d, got %d)", - path, NM_CRYPTO_KEY_TYPE_ENCRYPTED, format); - - g_byte_array_free (array, TRUE); + "%s: unexpected PKCS#12 private key file format (expected %d, got " + "%d): %d %s", + path, NM_CRYPTO_FILE_FORMAT_PKCS12, format, error->code, error->message); } static void @@ -211,10 +233,9 @@ test_is_pkcs12 (const char *path, gboolean expect_fail, const char *desc) ASSERT (is_pkcs12 == FALSE, desc, "unexpected success reading non-PKCS#12 file '%s'", path); - return; + } else { + ASSERT (is_pkcs12 == TRUE, desc, "couldn't read PKCS#12 file '%s'", path); } - - ASSERT (is_pkcs12 == TRUE, desc, "couldn't read PKCS#12 file '%s'", path); } static void @@ -223,23 +244,17 @@ test_encrypt_private_key (const char *path, const char *desc) { NMCryptoKeyType key_type = NM_CRYPTO_KEY_TYPE_UNKNOWN; - NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN; GByteArray *array, *encrypted, *re_decrypted; GError *error = NULL; - array = crypto_get_private_key (path, password, &key_type, &format, &error); - + array = crypto_decrypt_private_key (path, password, &key_type, &error); ASSERT (array != NULL, desc, "couldn't read private key file '%s': %d %s", path, error->code, error->message); - ASSERT (format == NM_CRYPTO_FILE_FORMAT_RAW_KEY, desc, - "%s: unexpected private key file format (expected %d, got %d)", - path, NM_CRYPTO_FILE_FORMAT_RAW_KEY, format); - ASSERT (key_type == NM_CRYPTO_KEY_TYPE_RSA, desc, "%s: unexpected private key type (expected %d, got %d)", - path, NM_CRYPTO_KEY_TYPE_RSA, format); + path, NM_CRYPTO_KEY_TYPE_RSA, key_type); /* Now re-encrypt the private key */ encrypted = nm_utils_rsa_key_encrypt (array, password, NULL, &error); @@ -249,20 +264,14 @@ test_encrypt_private_key (const char *path, /* Then re-decrypt the private key */ key_type = NM_CRYPTO_KEY_TYPE_UNKNOWN; - format = NM_CRYPTO_FILE_FORMAT_UNKNOWN; - re_decrypted = crypto_get_private_key_data (encrypted, password, &key_type, &format, &error); - + re_decrypted = crypto_decrypt_private_key_data (encrypted, password, &key_type, &error); ASSERT (re_decrypted != NULL, desc, "couldn't read private key file '%s': %d %s", path, error->code, error->message); - ASSERT (format == NM_CRYPTO_FILE_FORMAT_RAW_KEY, desc, - "%s: unexpected private key file format (expected %d, got %d)", - path, NM_CRYPTO_FILE_FORMAT_RAW_KEY, format); - ASSERT (key_type == NM_CRYPTO_KEY_TYPE_RSA, desc, "%s: unexpected private key type (expected %d, got %d)", - path, NM_CRYPTO_KEY_TYPE_RSA, format); + path, NM_CRYPTO_KEY_TYPE_RSA, key_type); /* Compare the original decrypted key with the re-decrypted key */ ASSERT (array->len == re_decrypted->len, desc, @@ -292,17 +301,21 @@ int main (int argc, char **argv) if (!strcmp (argv[1], "--cert")) test_load_cert (argv[2], "cert"); else if (!strcmp (argv[1], "--key")) { - ASSERT (argc == 4, "test-crypto", - "wrong number of arguments (--key <key file> <password>)"); + const char *decrypted_path = (argc == 5) ? argv[4] : NULL; + + ASSERT (argc == 4 || argc == 5, "test-crypto", + "wrong number of arguments (--key <key file> <password> [<decrypted key file>])"); - test_load_private_key (argv[2], argv[3], FALSE, "private-key"); - test_load_private_key (argv[2], "blahblahblah", TRUE, "private-key-bad-password"); + test_is_pkcs12 (argv[2], TRUE, "not-pkcs12"); + test_load_private_key (argv[2], argv[3], decrypted_path, FALSE, "private-key"); + test_load_private_key (argv[2], "blahblahblah", NULL, TRUE, "private-key-bad-password"); + test_load_private_key (argv[2], NULL, NULL, TRUE, "private-key-no-password"); test_encrypt_private_key (argv[2], argv[3], "private-key-rencrypt"); - test_is_pkcs12 (argv[2], TRUE, "is-pkcs12-not-pkcs12"); } else if (!strcmp (argv[1], "--p12")) { test_is_pkcs12 (argv[2], FALSE, "is-pkcs12"); test_load_pkcs12 (argv[2], argv[3], FALSE, "pkcs12-private-key"); test_load_pkcs12 (argv[2], "blahblahblah", TRUE, "pkcs12-private-key-bad-password"); + test_load_pkcs12_no_password (argv[2], "pkcs12-private-key-no-password"); } else { ASSERT (argc > 2, "test-crypto", "unknown test type (not --cert, --key, or --p12)"); } diff --git a/libnm-util/tests/test-need-secrets.c b/libnm-util/tests/test-need-secrets.c index 0c2634ea5b..e5db1fafac 100644 --- a/libnm-util/tests/test-need-secrets.c +++ b/libnm-util/tests/test-need-secrets.c @@ -212,19 +212,24 @@ test_need_tls_secrets_blob (void) "need-tls-secrets-blob-key", "hints should be NULL since no secrets were required"); - /* Connection is good; clear secrets and ensure private key password is not - * required because our blob is decrypted. - */ + /* Clear secrets and ensure password is again required */ nm_connection_clear_secrets (connection); hints = NULL; setting_name = nm_connection_need_secrets (connection, &hints); - ASSERT (setting_name == NULL, + ASSERT (setting_name != NULL, "need-tls-secrets-blob-key-password", - "unexpected secrets failure"); - ASSERT (hints == NULL, + "unexpected secrets success"); + ASSERT (strcmp (setting_name, NM_SETTING_802_1X_SETTING_NAME) == 0, + "need-tls-secrets-blob-key-password", + "unexpected setting secrets required"); + + ASSERT (hints != NULL, "need-tls-secrets-blob-key-password", - "hints should be NULL since no secrets were required"); + "expected returned secrets hints"); + ASSERT (find_hints_item (hints, NM_SETTING_802_1X_PRIVATE_KEY_PASSWORD), + "need-tls-secrets-blob-key-password", + "expected to require private key password, but it wasn't"); g_object_unref (connection); } @@ -391,19 +396,24 @@ test_need_tls_phase2_secrets_blob (void) "need-tls-phase2-secrets-blob-key", "hints should be NULL since no secrets were required"); - /* Connection is good; clear secrets and ensure private key password is not - * required because our blob is decrypted. - */ + /* Connection is good; clear secrets and ensure private key password is then required */ nm_connection_clear_secrets (connection); hints = NULL; setting_name = nm_connection_need_secrets (connection, &hints); - ASSERT (setting_name == NULL, + ASSERT (setting_name != NULL, "need-tls-phase2-secrets-blob-key-password", - "unexpected secrets failure"); - ASSERT (hints == NULL, + "unexpected secrets success"); + ASSERT (strcmp (setting_name, NM_SETTING_802_1X_SETTING_NAME) == 0, + "need-tls-phase2-secrets-blob-key-password", + "unexpected setting secrets required"); + + ASSERT (hints != NULL, "need-tls-phase2-secrets-blob-key-password", - "hints should be NULL since no secrets were required"); + "expected returned secrets hints"); + ASSERT (find_hints_item (hints, NM_SETTING_802_1X_PHASE2_PRIVATE_KEY_PASSWORD), + "need-tls-phase2-secrets-blob-key-password", + "expected to require private key password, but it wasn't"); g_object_unref (connection); } diff --git a/libnm-util/tests/test-setting-8021x.c b/libnm-util/tests/test-setting-8021x.c index 6202a1427f..6d1e3bd0ca 100644 --- a/libnm-util/tests/test-setting-8021x.c +++ b/libnm-util/tests/test-setting-8021x.c @@ -15,7 +15,7 @@ * with this program; if not, write to the Free Software Foundation, Inc., * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. * - * Copyright (C) 2008 - 2009 Red Hat, Inc. + * Copyright (C) 2008 - 2011 Red Hat, Inc. * */ @@ -30,8 +30,8 @@ #include "nm-setting-8021x.h" static void -compare_decrypted (const char *test, - const char *decrypted_path, +compare_blob_data (const char *test, + const char *key_path, const GByteArray *key) { char *contents = NULL; @@ -39,32 +39,48 @@ compare_decrypted (const char *test, GError *error = NULL; gboolean success; - success = g_file_get_contents (decrypted_path, &contents, &len, &error); + success = g_file_get_contents (key_path, &contents, &len, &error); ASSERT (success == TRUE, - test, "failed to read decrypted key file: %s", error->message); + test, "failed to read blob key file: %s", error->message); - ASSERT (len > 0, test, "decrypted key file invalid (size 0)"); + ASSERT (len > 0, test, "blob key file invalid (size 0)"); ASSERT (len == key->len, - test, "decrypted key file (%d) and decrypted key data (%d) lengths don't match", + test, "blob key file (%d) and setting key data (%d) lengths don't match", len, key->len); ASSERT (memcmp (contents, key->data, len) == 0, - test, "decrypted key file and decrypted key data don't match"); + test, "blob key file and blob key data don't match"); g_free (contents); } +#define SCHEME_PATH "file://" + +static void +check_scheme_path (GByteArray *value, const char *path) +{ + guint8 *p = value->data; + + g_assert (memcmp (p, SCHEME_PATH, strlen (SCHEME_PATH)) == 0); + p += strlen (SCHEME_PATH); + g_assert (memcmp (p, path, strlen (path)) == 0); + p += strlen (path); + g_assert (*p == '\0'); +} + static void test_private_key_import (const char *path, const char *password, - const char *decrypted_path, NMSetting8021xCKScheme scheme) { NMSetting8021x *s_8021x; gboolean success; NMSetting8021xCKFormat format = NM_SETTING_802_1X_CK_FORMAT_UNKNOWN; + NMSetting8021xCKFormat tmp_fmt; GError *error = NULL; + GByteArray *tmp_key = NULL, *client_cert = NULL; + const char *pw; s_8021x = (NMSetting8021x *) nm_setting_802_1x_new (); ASSERT (s_8021x != NULL, "private-key-import", "setting was NULL"); @@ -77,16 +93,48 @@ test_private_key_import (const char *path, &error); ASSERT (success == TRUE, "private-key-import", "error reading private key: %s", error->message); - - if ( scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB - && format != NM_SETTING_802_1X_CK_FORMAT_PKCS12) { - const GByteArray *key; - - ASSERT (decrypted_path != NULL, "private-key-import", "missing decrypted key file"); - - key = nm_setting_802_1x_get_private_key_blob (s_8021x); - ASSERT (key != NULL, "private-key-import", "missing private key blob"); - compare_decrypted ("private-key-import", decrypted_path, key); + ASSERT (format != NM_SETTING_802_1X_CK_FORMAT_UNKNOWN, + "private-key-import", "unexpected private key format (got %d)", format); + tmp_fmt = nm_setting_802_1x_get_private_key_format (s_8021x); + ASSERT (tmp_fmt == format, + "private-key-import", "unexpected re-read private key format (expected %d, got %d)", + format, tmp_fmt); + + /* Make sure the password is what we expect */ + pw = nm_setting_802_1x_get_private_key_password (s_8021x); + ASSERT (pw != NULL, + "private-key-import", "failed to get previous private key password"); + ASSERT (strcmp (pw, password) == 0, + "private-key-import", "failed to compare private key password"); + + if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) { + tmp_key = (GByteArray *) nm_setting_802_1x_get_private_key_blob (s_8021x); + ASSERT (tmp_key != NULL, "private-key-import", "missing private key blob"); + compare_blob_data ("private-key-import", path, tmp_key); + } else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) { + g_object_get (s_8021x, NM_SETTING_802_1X_PRIVATE_KEY, &tmp_key, NULL); + ASSERT (tmp_key != NULL, "private-key-import", "missing private key value"); + check_scheme_path (tmp_key, path); + g_byte_array_free (tmp_key, TRUE); + } else + g_assert_not_reached (); + + /* If it's PKCS#12 ensure the client cert is the same value */ + if (format == NM_SETTING_802_1X_CK_FORMAT_PKCS12) { + g_object_get (s_8021x, NM_SETTING_802_1X_PRIVATE_KEY, &tmp_key, NULL); + ASSERT (tmp_key != NULL, "private-key-import", "missing private key value"); + + g_object_get (s_8021x, NM_SETTING_802_1X_CLIENT_CERT, &client_cert, NULL); + ASSERT (client_cert != NULL, "private-key-import", "missing client certificate value"); + + /* make sure they are the same */ + ASSERT (tmp_key->len == client_cert->len, + "private-key-import", "unexpected different private key and client cert lengths"); + ASSERT (memcmp (tmp_key->data, client_cert->data, tmp_key->len) == 0, + "private-key-import", "unexpected different private key and client cert data"); + + g_byte_array_free (tmp_key, TRUE); + g_byte_array_free (client_cert, TRUE); } g_object_unref (s_8021x); @@ -95,13 +143,15 @@ test_private_key_import (const char *path, static void test_phase2_private_key_import (const char *path, const char *password, - const char *decrypted_path, NMSetting8021xCKScheme scheme) { NMSetting8021x *s_8021x; gboolean success; NMSetting8021xCKFormat format = NM_SETTING_802_1X_CK_FORMAT_UNKNOWN; + NMSetting8021xCKFormat tmp_fmt; GError *error = NULL; + GByteArray *tmp_key = NULL, *client_cert = NULL; + const char *pw; s_8021x = (NMSetting8021x *) nm_setting_802_1x_new (); ASSERT (s_8021x != NULL, "phase2-private-key-import", "setting was NULL"); @@ -114,17 +164,242 @@ test_phase2_private_key_import (const char *path, &error); ASSERT (success == TRUE, "phase2-private-key-import", "error reading private key: %s", error->message); + ASSERT (format != NM_SETTING_802_1X_CK_FORMAT_UNKNOWN, + "phase2-private-key-import", "unexpected private key format"); + tmp_fmt = nm_setting_802_1x_get_phase2_private_key_format (s_8021x); + ASSERT (tmp_fmt == format, + "phase2-private-key-import", "unexpected re-read private key format (expected %d, got %d)", + format, tmp_fmt); + + /* Make sure the password is what we expect */ + pw = nm_setting_802_1x_get_phase2_private_key_password (s_8021x); + ASSERT (pw != NULL, + "phase2-private-key-import", "failed to get previous private key password"); + ASSERT (strcmp (pw, password) == 0, + "phase2-private-key-import", "failed to compare private key password"); + + if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) { + tmp_key = (GByteArray *) nm_setting_802_1x_get_phase2_private_key_blob (s_8021x); + ASSERT (tmp_key != NULL, "phase2-private-key-import", "missing private key blob"); + compare_blob_data ("phase2-private-key-import", path, tmp_key); + } else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) { + g_object_get (s_8021x, NM_SETTING_802_1X_PHASE2_PRIVATE_KEY, &tmp_key, NULL); + ASSERT (tmp_key != NULL, "phase2-private-key-import", "missing private key value"); + check_scheme_path (tmp_key, path); + } else + g_assert_not_reached (); + + /* If it's PKCS#12 ensure the client cert is the same value */ + if (format == NM_SETTING_802_1X_CK_FORMAT_PKCS12) { + g_object_get (s_8021x, NM_SETTING_802_1X_PHASE2_PRIVATE_KEY, &tmp_key, NULL); + ASSERT (tmp_key != NULL, "private-key-import", "missing private key value"); + + g_object_get (s_8021x, NM_SETTING_802_1X_PHASE2_CLIENT_CERT, &client_cert, NULL); + ASSERT (client_cert != NULL, "private-key-import", "missing client certificate value"); + + /* make sure they are the same */ + ASSERT (tmp_key->len == client_cert->len, + "private-key-import", "unexpected different private key and client cert lengths"); + ASSERT (memcmp (tmp_key->data, client_cert->data, tmp_key->len) == 0, + "private-key-import", "unexpected different private key and client cert data"); + + g_byte_array_free (tmp_key, TRUE); + g_byte_array_free (client_cert, TRUE); + } - if ( scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB - && format != NM_SETTING_802_1X_CK_FORMAT_PKCS12) { - const GByteArray *key; + g_object_unref (s_8021x); +} - ASSERT (decrypted_path != NULL, "phase2-private-key-import", "missing decrypted key file"); +static void +test_wrong_password_keeps_data (const char *path, const char *password) +{ + NMSetting8021x *s_8021x; + gboolean success; + NMSetting8021xCKFormat format = NM_SETTING_802_1X_CK_FORMAT_UNKNOWN; + GError *error = NULL; + const char *pw; - key = nm_setting_802_1x_get_phase2_private_key_blob (s_8021x); - ASSERT (key != NULL, "phase2-private-key-import", "missing private key blob"); - compare_decrypted ("phase2-private-key-import", decrypted_path, key); - } + s_8021x = (NMSetting8021x *) nm_setting_802_1x_new (); + ASSERT (s_8021x != NULL, "wrong-password-keeps-data", "setting was NULL"); + + success = nm_setting_802_1x_set_private_key (s_8021x, + path, + password, + NM_SETTING_802_1X_CK_SCHEME_BLOB, + &format, + &error); + ASSERT (success == TRUE, + "wrong-password-keeps-data", "error reading private key: %s", error->message); + ASSERT (format != NM_SETTING_802_1X_CK_FORMAT_UNKNOWN, + "wrong-password-keeps-data", "unexpected private key format (got %d)", format); + + /* Now try to set it to something that's not a certificate */ + format = NM_SETTING_802_1X_CK_FORMAT_UNKNOWN; + success = nm_setting_802_1x_set_private_key (s_8021x, + "Makefile.am", + password, + NM_SETTING_802_1X_CK_SCHEME_BLOB, + &format, + &error); + ASSERT (success == FALSE, + "wrong-password-keeps-data", "unexpected success reading private key"); + ASSERT (error != NULL, + "wrong-password-keeps-data", "unexpected missing error"); + ASSERT (format == NM_SETTING_802_1X_CK_FORMAT_UNKNOWN, + "wrong-password-keeps-data", "unexpected success reading private key format"); + + /* Make sure the password hasn't changed */ + pw = nm_setting_802_1x_get_private_key_password (s_8021x); + ASSERT (pw != NULL, + "wrong-password-keeps-data", "failed to get previous private key password"); + ASSERT (strcmp (pw, password) == 0, + "wrong-password-keeps-data", "failed to compare private key password"); + + g_object_unref (s_8021x); +} + +static void +test_clear_private_key (const char *path, const char *password) +{ + NMSetting8021x *s_8021x; + gboolean success; + NMSetting8021xCKFormat format = NM_SETTING_802_1X_CK_FORMAT_UNKNOWN; + GError *error = NULL; + const char *pw; + + s_8021x = (NMSetting8021x *) nm_setting_802_1x_new (); + ASSERT (s_8021x != NULL, "clear-private-key", "setting was NULL"); + + success = nm_setting_802_1x_set_private_key (s_8021x, + path, + password, + NM_SETTING_802_1X_CK_SCHEME_BLOB, + &format, + &error); + ASSERT (success == TRUE, + "clear-private-key", "error reading private key: %s", error->message); + ASSERT (format != NM_SETTING_802_1X_CK_FORMAT_UNKNOWN, + "clear-private-key", "unexpected private key format (got %d)", format); + + /* Make sure the password is what we expect */ + pw = nm_setting_802_1x_get_private_key_password (s_8021x); + ASSERT (pw != NULL, + "clear-private-key", "failed to get previous private key password"); + ASSERT (strcmp (pw, password) == 0, + "clear-private-key", "failed to compare private key password"); + + /* Now clear it */ + success = nm_setting_802_1x_set_private_key (s_8021x, + NULL, + NULL, + NM_SETTING_802_1X_CK_SCHEME_BLOB, + NULL, + &error); + ASSERT (success == TRUE, + "clear-private-key", "unexpected failure clearing private key"); + ASSERT (error == NULL, + "clear-private-key", "unexpected error clearing private key"); + + /* Ensure the password is also now clear */ + ASSERT (nm_setting_802_1x_get_private_key_password (s_8021x) == NULL, + "clear-private-key", "unexpected private key password"); + + g_object_unref (s_8021x); +} + +static void +test_wrong_phase2_password_keeps_data (const char *path, const char *password) +{ + NMSetting8021x *s_8021x; + gboolean success; + NMSetting8021xCKFormat format = NM_SETTING_802_1X_CK_FORMAT_UNKNOWN; + GError *error = NULL; + const char *pw; + + s_8021x = (NMSetting8021x *) nm_setting_802_1x_new (); + ASSERT (s_8021x != NULL, "wrong-phase2-password-keeps-data", "setting was NULL"); + + success = nm_setting_802_1x_set_phase2_private_key (s_8021x, + path, + password, + NM_SETTING_802_1X_CK_SCHEME_BLOB, + &format, + &error); + ASSERT (success == TRUE, + "wrong-phase2-password-keeps-data", "error reading private key: %s", error->message); + ASSERT (format != NM_SETTING_802_1X_CK_FORMAT_UNKNOWN, + "wrong-phase2-password-keeps-data", "unexpected private key format (got %d)", format); + + /* Now try to set it to something that's not a certificate */ + format = NM_SETTING_802_1X_CK_FORMAT_UNKNOWN; + success = nm_setting_802_1x_set_phase2_private_key (s_8021x, + "Makefile.am", + password, + NM_SETTING_802_1X_CK_SCHEME_BLOB, + &format, + &error); + ASSERT (success == FALSE, + "wrong-phase2-password-keeps-data", "unexpected success reading private key"); + ASSERT (error != NULL, + "wrong-phase2-password-keeps-data", "unexpected missing error"); + ASSERT (format == NM_SETTING_802_1X_CK_FORMAT_UNKNOWN, + "wrong-phase2-password-keeps-data", "unexpected success reading private key format"); + + /* Make sure the password hasn't changed */ + pw = nm_setting_802_1x_get_phase2_private_key_password (s_8021x); + ASSERT (pw != NULL, + "wrong-phase2-password-keeps-data", "failed to get previous private key password"); + ASSERT (strcmp (pw, password) == 0, + "wrong-phase2-password-keeps-data", "failed to compare private key password"); + + g_object_unref (s_8021x); +} + +static void +test_clear_phase2_private_key (const char *path, const char *password) +{ + NMSetting8021x *s_8021x; + gboolean success; + NMSetting8021xCKFormat format = NM_SETTING_802_1X_CK_FORMAT_UNKNOWN; + GError *error = NULL; + const char *pw; + + s_8021x = (NMSetting8021x *) nm_setting_802_1x_new (); + ASSERT (s_8021x != NULL, "clear-phase2-private-key", "setting was NULL"); + + success = nm_setting_802_1x_set_phase2_private_key (s_8021x, + path, + password, + NM_SETTING_802_1X_CK_SCHEME_BLOB, + &format, + &error); + ASSERT (success == TRUE, + "clear-phase2-private-key", "error reading private key: %s", error->message); + ASSERT (format != NM_SETTING_802_1X_CK_FORMAT_UNKNOWN, + "clear-phase2-private-key", "unexpected private key format (got %d)", format); + + /* Make sure the password is what we expect */ + pw = nm_setting_802_1x_get_phase2_private_key_password (s_8021x); + ASSERT (pw != NULL, + "clear-phase2-private-key", "failed to get previous private key password"); + ASSERT (strcmp (pw, password) == 0, + "clear-phase2-private-key", "failed to compare private key password"); + + /* Now clear it */ + success = nm_setting_802_1x_set_phase2_private_key (s_8021x, + NULL, + NULL, + NM_SETTING_802_1X_CK_SCHEME_BLOB, + NULL, + &error); + ASSERT (success == TRUE, + "clear-phase2-private-key", "unexpected failure clearing private key"); + ASSERT (error == NULL, + "clear-phase2-private-key", "unexpected error clearing private key"); + + /* Ensure the password is also now clear */ + ASSERT (nm_setting_802_1x_get_phase2_private_key_password (s_8021x) == NULL, + "clear-phase2-private-key", "unexpected private key password"); g_object_unref (s_8021x); } @@ -134,13 +409,9 @@ int main (int argc, char **argv) GError *error = NULL; DBusGConnection *bus; char *base; - const char *decrypted = NULL; if (argc < 3) - FAIL ("init", "need at least two arguments: <path> <password> [decrypted private key]"); - - if (argc == 4) - decrypted = argv[3]; + FAIL ("init", "need at least two arguments: <path> <password>"); g_type_init (); bus = dbus_g_bus_get (DBUS_BUS_SESSION, NULL); @@ -148,12 +419,21 @@ int main (int argc, char **argv) if (!nm_utils_init (&error)) FAIL ("nm-utils-init", "failed to initialize libnm-util: %s", error->message); - /* The tests */ - test_private_key_import (argv[1], argv[2], NULL, NM_SETTING_802_1X_CK_SCHEME_PATH); - test_phase2_private_key_import (argv[1], argv[2], NULL, NM_SETTING_802_1X_CK_SCHEME_PATH); + /* Test phase1 and phase2 path scheme */ + test_private_key_import (argv[1], argv[2], NM_SETTING_802_1X_CK_SCHEME_PATH); + test_phase2_private_key_import (argv[1], argv[2], NM_SETTING_802_1X_CK_SCHEME_PATH); + + /* Test phase1 and phase2 blob scheme */ + test_private_key_import (argv[1], argv[2], NM_SETTING_802_1X_CK_SCHEME_BLOB); + test_phase2_private_key_import (argv[1], argv[2], NM_SETTING_802_1X_CK_SCHEME_BLOB); + + /* Test that using a wrong password does not change existing data */ + test_wrong_password_keeps_data (argv[1], argv[2]); + test_wrong_phase2_password_keeps_data (argv[1], argv[2]); - test_private_key_import (argv[1], argv[2], decrypted, NM_SETTING_802_1X_CK_SCHEME_BLOB); - test_phase2_private_key_import (argv[1], argv[2], decrypted, NM_SETTING_802_1X_CK_SCHEME_BLOB); + /* Test clearing the private key */ + test_clear_private_key (argv[1], argv[2]); + test_clear_phase2_private_key (argv[1], argv[2]); base = g_path_get_basename (argv[0]); fprintf (stdout, "%s: SUCCESS\n", base); |