From 51c47c0a9d77f04d04c6cde7f1254623328898f9 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Tue, 14 May 2019 13:59:00 +0200
Subject: [PATCH 1/3] ifcfg-rh: write client certificate even if it is pkcs12
The writer should only persist properties without too much additional
logic, which should be instead embedded in the setting itself.
(cherry picked from commit a995244e9bf526b2d10143858655c3ea3731bf91)
(cherry picked from commit 5a5cd8d05dfbde11b0983e09a5a37f6929bb2178)
---
.../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 4 ----
.../plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 24 ++++++-------------
2 files changed, 7 insertions(+), 21 deletions(-)
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
index e5423b181..9b7511064 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
@@ -3117,10 +3117,6 @@ eap_tls_reader (const char *eap_method,
&client_cert,
error))
return FALSE;
- /* FIXME: writer does not actually write IEEE_8021X_CLIENT_CERT_PASSWORD and other
- * certificate related passwords. It should, because otherwise persisting such profiles
- * to ifcfg looses information. As this currently only matters for PKCS11 URIs, it seems
- * a seldom used feature so that it is not fixed yet. */
_secret_set_from_ifcfg (s_8021x,
ifcfg,
keys_ifcfg,
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
index 80b1bffe1..90f06e183 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
@@ -358,23 +358,13 @@ write_8021x_certs (NMSetting8021x *s_8021x,
if (!write_object (s_8021x, ifcfg, secrets, blobs, otype, error))
return FALSE;
- /* Client certificate */
- if (otype->vtable->format_func (s_8021x) == NM_SETTING_802_1X_CK_FORMAT_PKCS12) {
- /* Don't need a client certificate with PKCS#12 since the file is both
- * the client certificate and the private key in one file.
- */
- svSetValueStr (ifcfg,
- phase2 ? "IEEE_8021X_INNER_CLIENT_CERT" : "IEEE_8021X_CLIENT_CERT",
- NULL);
- } else {
- /* Save the client certificate */
- if (!write_object (s_8021x, ifcfg, secrets, blobs,
- phase2
- ? &setting_8021x_scheme_vtable[NM_SETTING_802_1X_SCHEME_TYPE_PHASE2_CLIENT_CERT]
- : &setting_8021x_scheme_vtable[NM_SETTING_802_1X_SCHEME_TYPE_CLIENT_CERT],
- error))
- return FALSE;
- }
+ /* Save the client certificate */
+ if (!write_object (s_8021x, ifcfg, secrets, blobs,
+ phase2
+ ? &setting_8021x_scheme_vtable[NM_SETTING_802_1X_SCHEME_TYPE_PHASE2_CLIENT_CERT]
+ : &setting_8021x_scheme_vtable[NM_SETTING_802_1X_SCHEME_TYPE_CLIENT_CERT],
+ error))
+ return FALSE;
return TRUE;
}
--
2.20.1
From c62a97f608c4c28cbefe1b5b57bec5f6da24b342 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Tue, 14 May 2019 14:32:19 +0200
Subject: [PATCH 2/3] ifcfg-rh: don't check for 802.1x private key or client
cert in reader
Let the setting check it in verify().
(cherry picked from commit d9b3b2b8cec9fdb984a6103240688dc46f33866e)
(cherry picked from commit c28db67a781388e1f742b3406e26a35c8c2522a8)
---
.../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
index 9b7511064..da3b89e1a 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
@@ -3077,6 +3077,7 @@ eap_tls_reader (const char *eap_method,
svGetValueStr (ifcfg, "IEEE_8021X_IDENTITY", &identity_free),
NULL);
+ /* CA certificate */
if (!_cert_set_from_ifcfg (s_8021x,
ifcfg,
phase2 ? "IEEE_8021X_INNER_CA_CERT" : "IEEE_8021X_CA_CERT",
@@ -3090,6 +3091,7 @@ eap_tls_reader (const char *eap_method,
phase2 ? "IEEE_8021X_INNER_CA_CERT_PASSWORD" : "IEEE_8021X_CA_CERT_PASSWORD",
phase2 ? NM_SETTING_802_1X_PHASE2_CA_CERT_PASSWORD : NM_SETTING_802_1X_CA_CERT_PASSWORD);
+ /* Private key */
if (!_cert_set_from_ifcfg (s_8021x,
ifcfg,
phase2 ? "IEEE_8021X_INNER_PRIVATE_KEY" : "IEEE_8021X_PRIVATE_KEY",
@@ -3102,14 +3104,8 @@ eap_tls_reader (const char *eap_method,
keys_ifcfg,
phase2 ? "IEEE_8021X_INNER_PRIVATE_KEY_PASSWORD" : "IEEE_8021X_PRIVATE_KEY_PASSWORD",
phase2 ? NM_SETTING_802_1X_PHASE2_PRIVATE_KEY_PASSWORD : NM_SETTING_802_1X_PRIVATE_KEY_PASSWORD);
- if (!privkey) {
- g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
- "Missing %s for EAP method '%s'.",
- phase2 ? "IEEE_8021X_INNER_PRIVATE_KEY" : "IEEE_8021X_PRIVATE_KEY",
- eap_method);
- return FALSE;
- }
+ /* Client certificate */
if (!_cert_set_from_ifcfg (s_8021x,
ifcfg,
phase2 ? "IEEE_8021X_INNER_CLIENT_CERT" : "IEEE_8021X_CLIENT_CERT",
@@ -3122,12 +3118,6 @@ eap_tls_reader (const char *eap_method,
keys_ifcfg,
phase2 ? "IEEE_8021X_INNER_CLIENT_CERT_PASSWORD" : "IEEE_8021X_CLIENT_CERT_PASSWORD",
phase2 ? NM_SETTING_802_1X_PHASE2_CLIENT_CERT_PASSWORD : NM_SETTING_802_1X_CLIENT_CERT_PASSWORD);
- if (!client_cert) {
- g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
- "Missing certificate for EAP method '%s'.",
- eap_method);
- return FALSE;
- }
return TRUE;
}
--
2.20.1
From b3935bb0f25bede6e9c29735314f42f4bd773e09 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Tue, 14 May 2019 15:27:45 +0200
Subject: [PATCH 3/3] ifcfg-rh: use PKCS #12 private key also as client cert in
reader
Before commit e3ac45c02610 the reader set the private key in the
setting using the libnm function, which also set the key as client
certificate if it was in PKCS #12 format.
After the commit, existing connections with a PKCS #12 private key but
without a client certificate became invalid. Restore the old behavior.
Fixes: e3ac45c02610 ('ifcfg-rh: don't use 802-1x certifcate setter functions')
(cherry picked from commit 9a410fc312c50ac405c57ff4e9eb692e798e248d)
(cherry picked from commit 51896e1e6b24e0b5d6aefce3c4945d27a5b9f5b7)
---
Makefile.am | 2 ++
.../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 28 ++++++++++++++++--
.../plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 21 +++++++++----
...fg-test-wired-8021x-tls-p12-no-client-cert | 13 ++++++++
.../tests/network-scripts/test_client.p12 | Bin 0 -> 2848 bytes
.../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 23 ++++++++++++++
6 files changed, 79 insertions(+), 8 deletions(-)
create mode 100644 src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-8021x-tls-p12-no-client-cert
create mode 100644 src/settings/plugins/ifcfg-rh/tests/network-scripts/test_client.p12
diff --git a/Makefile.am b/Makefile.am
index d78bfdeda..8c470df31 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3014,6 +3014,7 @@ EXTRA_DIST += \
src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-8021x-peap-mschapv2 \
src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-8021x-tls-agent \
src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-8021x-tls-always \
+ src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-8021x-tls-p12-no-client-cert \
src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-auto-negotiate-on \
src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-autoip \
src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-ctc-static \
@@ -3083,6 +3084,7 @@ EXTRA_DIST += \
src/settings/plugins/ifcfg-rh/tests/network-scripts/route6-test-wired-ipv6-manual \
src/settings/plugins/ifcfg-rh/tests/network-scripts/test1_key_and_cert.pem \
src/settings/plugins/ifcfg-rh/tests/network-scripts/test_ca_cert.pem \
+ src/settings/plugins/ifcfg-rh/tests/network-scripts/test_client.p12 \
$(NULL)
# make target dependencies can't have colons in their names, which ends up
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
index da3b89e1a..317e22f7a 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
@@ -3071,6 +3071,10 @@ eap_tls_reader (const char *eap_method,
gs_unref_bytes GBytes *privkey = NULL;
gs_unref_bytes GBytes *client_cert = NULL;
gs_free char *identity_free = NULL;
+ gs_free char *value_to_free = NULL;
+ const char *client_cert_var;
+ const char *client_cert_prop;
+ NMSetting8021xCKFormat format;
g_object_set (s_8021x,
NM_SETTING_802_1X_IDENTITY,
@@ -3106,10 +3110,12 @@ eap_tls_reader (const char *eap_method,
phase2 ? NM_SETTING_802_1X_PHASE2_PRIVATE_KEY_PASSWORD : NM_SETTING_802_1X_PRIVATE_KEY_PASSWORD);
/* Client certificate */
+ client_cert_var = phase2 ? "IEEE_8021X_INNER_CLIENT_CERT" : "IEEE_8021X_CLIENT_CERT";
+ client_cert_prop = phase2 ? NM_SETTING_802_1X_PHASE2_CLIENT_CERT : NM_SETTING_802_1X_CLIENT_CERT;
if (!_cert_set_from_ifcfg (s_8021x,
ifcfg,
- phase2 ? "IEEE_8021X_INNER_CLIENT_CERT" : "IEEE_8021X_CLIENT_CERT",
- phase2 ? NM_SETTING_802_1X_PHASE2_CLIENT_CERT : NM_SETTING_802_1X_CLIENT_CERT,
+ client_cert_var,
+ client_cert_prop,
&client_cert,
error))
return FALSE;
@@ -3119,6 +3125,24 @@ eap_tls_reader (const char *eap_method,
phase2 ? "IEEE_8021X_INNER_CLIENT_CERT_PASSWORD" : "IEEE_8021X_CLIENT_CERT_PASSWORD",
phase2 ? NM_SETTING_802_1X_PHASE2_CLIENT_CERT_PASSWORD : NM_SETTING_802_1X_CLIENT_CERT_PASSWORD);
+ /* In the past when the private key and client certificate
+ * were the same PKCS #12 file we used to write only the
+ * private key variable. Still support that even if it means
+ * that we have to look into the file content, which makes
+ * the connection not self-contained.
+ */
+ if ( !client_cert
+ && privkey
+ && !svGetValue (ifcfg, client_cert_var, &value_to_free)) {
+ if (phase2)
+ format = nm_setting_802_1x_get_phase2_private_key_format (s_8021x);
+ else
+ format = nm_setting_802_1x_get_private_key_format (s_8021x);
+
+ if (format == NM_SETTING_802_1X_CK_FORMAT_PKCS12)
+ g_object_set (s_8021x, client_cert_prop, privkey, NULL);
+ }
+
return TRUE;
}
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
index 90f06e183..6e2bc8493 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
@@ -209,6 +209,7 @@ write_object (NMSetting8021x *s_8021x,
GHashTable *secrets,
GHashTable *blobs,
const Setting8021xSchemeVtable *objtype,
+ gboolean force_write,
GError **error)
{
NMSetting8021xCKScheme scheme;
@@ -287,7 +288,7 @@ write_object (NMSetting8021x *s_8021x,
*/
standard_file = utils_cert_path (svFileGetName (ifcfg), objtype->vtable->file_suffix, extension);
g_hash_table_replace (blobs, standard_file, NULL);
- svUnsetValue (ifcfg, objtype->ifcfg_rh_key);
+ svSetValue (ifcfg, objtype->ifcfg_rh_key, force_write ? "" : NULL);
return TRUE;
}
@@ -338,31 +339,39 @@ write_8021x_certs (NMSetting8021x *s_8021x,
shvarFile *ifcfg,
GError **error)
{
- const Setting8021xSchemeVtable *otype = NULL;
+ const Setting8021xSchemeVtable *pk_otype = NULL;
+ gs_free char *value_to_free = NULL;
/* CA certificate */
if (!write_object (s_8021x, ifcfg, secrets, blobs,
phase2
? &setting_8021x_scheme_vtable[NM_SETTING_802_1X_SCHEME_TYPE_PHASE2_CA_CERT]
: &setting_8021x_scheme_vtable[NM_SETTING_802_1X_SCHEME_TYPE_CA_CERT],
+ FALSE,
error))
return FALSE;
/* Private key */
if (phase2)
- otype = &setting_8021x_scheme_vtable[NM_SETTING_802_1X_SCHEME_TYPE_PHASE2_PRIVATE_KEY];
+ pk_otype = &setting_8021x_scheme_vtable[NM_SETTING_802_1X_SCHEME_TYPE_PHASE2_PRIVATE_KEY];
else
- otype = &setting_8021x_scheme_vtable[NM_SETTING_802_1X_SCHEME_TYPE_PRIVATE_KEY];
+ pk_otype = &setting_8021x_scheme_vtable[NM_SETTING_802_1X_SCHEME_TYPE_PRIVATE_KEY];
/* Save the private key */
- if (!write_object (s_8021x, ifcfg, secrets, blobs, otype, error))
+ if (!write_object (s_8021x, ifcfg, secrets, blobs, pk_otype, FALSE, error))
return FALSE;
- /* Save the client certificate */
+ /* Save the client certificate.
+ * If there is a private key, always write a property for the
+ * client certificate even if it is empty, so that the reader
+ * doesn't have to read the private key file to determine if it
+ * is a PKCS #12 one which serves also as client certificate.
+ */
if (!write_object (s_8021x, ifcfg, secrets, blobs,
phase2
? &setting_8021x_scheme_vtable[NM_SETTING_802_1X_SCHEME_TYPE_PHASE2_CLIENT_CERT]
: &setting_8021x_scheme_vtable[NM_SETTING_802_1X_SCHEME_TYPE_CLIENT_CERT],
+ !!svGetValue (ifcfg, pk_otype->ifcfg_rh_key, &value_to_free),
error))
return FALSE;
--
2.20.1