From 3d6a2b8d6b1a7e2b6cf1053f3d635e9d934f64fc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Feb 2018 18:35:43 +0100 Subject: [PATCH 1/3] libnm/keyfile/trivial: rename local variables in preparation of refactoring --- libnm-core/nm-keyfile-reader.c | 74 +++++++++++++++++----------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index f850638497..585e159d9a 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -993,79 +993,79 @@ has_cert_ext (const char *path) static gboolean handle_as_scheme (KeyfileReaderInfo *info, GBytes *bytes, NMSetting *setting, const char *key) { - const char *data; - gsize data_len, bin_len; + const char *bin; + gsize bin_len0, bin_decoded_len; - data = g_bytes_get_data (bytes, &data_len); + bin = g_bytes_get_data (bytes, &bin_len0); - g_return_val_if_fail (data && data_len > 0, FALSE); + g_return_val_if_fail (bin && bin_len0 > 0, FALSE); - /* to be a scheme, @data must be a zero terminated string, which is counted by @data_len */ - if (data[data_len - 1] != '\0') + /* to be a scheme, @bin must be a zero terminated string, which is counted by @bin_len0 */ + if (bin[bin_len0 - 1] != '\0') return FALSE; - data_len--; + bin_len0--; /* It's the PATH scheme, can just set plain data. - * In this case, @data_len includes */ - if ( data_len >= NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH) - && g_str_has_prefix (data, NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)) { - if (nm_setting_802_1x_check_cert_scheme (data, data_len + 1, NULL) == NM_SETTING_802_1X_CK_SCHEME_PATH) { - const char *path = &data[NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)]; - gs_free char *path_free = NULL; + * In this case, @bin_len0 includes */ + if ( bin_len0 >= NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH) + && g_str_has_prefix (bin, NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)) { + if (nm_setting_802_1x_check_cert_scheme (bin, bin_len0 + 1, NULL) == NM_SETTING_802_1X_CK_SCHEME_PATH) { + const char *path2 = &bin[NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)]; + gs_free char *path2_free = NULL; - if (path[0] != '/') { + if (path2[0] != '/') { /* we want to read absolute paths because we use keyfile as exchange * between different processes which might not have the same cwd. */ - path = path_free = get_cert_path (info->base_dir, (const guint8 *) path, - data_len - NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)); + path2 = path2_free = get_cert_path (info->base_dir, (const guint8 *) path2, + bin_len0 - NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)); } g_object_set (setting, key, bytes, NULL); - if (!g_file_test (path, G_FILE_TEST_EXISTS)) { + if (!g_file_test (path2, G_FILE_TEST_EXISTS)) { handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_INFO_MISSING_FILE, _("certificate or key file '%s' does not exist"), - path); + path2); } } else { handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, - _("invalid key/cert value path \"%s\""), data); + _("invalid key/cert value path \"%s\""), bin); } return TRUE; } - if ( data_len >= NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PKCS11) - && g_str_has_prefix (data, NM_KEYFILE_CERT_SCHEME_PREFIX_PKCS11)) { - if (nm_setting_802_1x_check_cert_scheme (data, data_len + 1, NULL) == NM_SETTING_802_1X_CK_SCHEME_PKCS11) { + if ( bin_len0 >= NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PKCS11) + && g_str_has_prefix (bin, NM_KEYFILE_CERT_SCHEME_PREFIX_PKCS11)) { + if (nm_setting_802_1x_check_cert_scheme (bin, bin_len0 + 1, NULL) == NM_SETTING_802_1X_CK_SCHEME_PKCS11) { g_object_set (setting, key, bytes, NULL); } else { handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, - _("invalid PKCS#11 URI \"%s\""), data); + _("invalid PKCS#11 URI \"%s\""), bin); } return TRUE; } - if ( data_len > NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB) - && g_str_has_prefix (data, NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB)) { - const char *cdata = data + NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB); - guchar *bin; + if ( bin_len0 > NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB) + && g_str_has_prefix (bin, NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB)) { + const char *cdata = bin + NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB); + guchar *bin_decoded; GBytes *bytes2; gsize i; gboolean valid_base64; - data_len -= NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB); + bin_len0 -= NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB); /* Let's be strict here. We expect valid base64, no funny stuff!! * We didn't write such invalid data ourselfes and refuse to read it as blob. */ - if ((valid_base64 = (data_len % 4 == 0))) { - for (i = 0; i < data_len; i++) { + if ((valid_base64 = (bin_len0 % 4 == 0))) { + for (i = 0; i < bin_len0; i++) { char c = cdata[i]; if (!( (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || (c == '+' || c == '/'))) { - if (c != '=' || i < data_len - 2) + if (c != '=' || i < bin_len0 - 2) valid_base64 = FALSE; else { - for (; i < data_len; i++) { + for (; i < bin_len0; i++) { if (cdata[i] != '=') valid_base64 = FALSE; } @@ -1080,18 +1080,18 @@ handle_as_scheme (KeyfileReaderInfo *info, GBytes *bytes, NMSetting *setting, co return TRUE; } - bin = g_base64_decode (cdata, &bin_len); + bin_decoded = g_base64_decode (cdata, &bin_decoded_len); - g_return_val_if_fail (bin_len > 0, FALSE); - if (nm_setting_802_1x_check_cert_scheme (bin, bin_len, NULL) != NM_SETTING_802_1X_CK_SCHEME_BLOB) { + g_return_val_if_fail (bin_decoded_len > 0, FALSE); + if (nm_setting_802_1x_check_cert_scheme (bin_decoded, bin_decoded_len, NULL) != NM_SETTING_802_1X_CK_SCHEME_BLOB) { /* The blob probably starts with "file://". Setting the cert data will confuse NMSetting8021x. * In fact this is a limitation of NMSetting8021x which does not support setting blobs that start * with file://. Just warn and return TRUE to signal that we ~handled~ the setting. */ - g_free (bin); + g_free (bin_decoded); handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, _("invalid key/cert value data:;base64,file://")); } else { - bytes2 = g_bytes_new_take (bin, bin_len); + bytes2 = g_bytes_new_take (bin_decoded, bin_decoded_len); g_object_set (setting, key, bytes2, NULL); g_bytes_unref (bytes2); } From cf774a1bfc0c064913203a0e456915e81e4d958b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Feb 2018 18:57:13 +0100 Subject: [PATCH 2/3] libnm/keyfile: refactor error paths for cert_parser() Refactor cert_parser() to return early. Also, rework handle_as_scheme() and handle_as_path() to check for success first and return early. This in the next step will allow to merge the functions. --- libnm-core/nm-keyfile-reader.c | 142 ++++++++++++++++++--------------- 1 file changed, 79 insertions(+), 63 deletions(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index 585e159d9a..93258355e4 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -994,21 +994,30 @@ static gboolean handle_as_scheme (KeyfileReaderInfo *info, GBytes *bytes, NMSetting *setting, const char *key) { const char *bin; - gsize bin_len0, bin_decoded_len; + gsize bin_len; + gsize bin_len0; - bin = g_bytes_get_data (bytes, &bin_len0); + bin = g_bytes_get_data (bytes, &bin_len); - g_return_val_if_fail (bin && bin_len0 > 0, FALSE); + g_return_val_if_fail (bin && bin_len > 0, FALSE); - /* to be a scheme, @bin must be a zero terminated string, which is counted by @bin_len0 */ - if (bin[bin_len0 - 1] != '\0') - return FALSE; - bin_len0--; +#define HAS_SCHEME_PREFIX(bin, bin_len, scheme) \ + ({ \ + const char *const _bin = (bin); \ + const gsize _bin_len = (bin_len); \ + \ + nm_assert (_bin && _bin_len > 0); \ + \ + ( _bin_len > NM_STRLEN (scheme) + 1 \ + && _bin[_bin_len - 1] == '\0' \ + && memcmp (_bin, scheme, NM_STRLEN (scheme)) == 0); \ + }) + + bin_len0 = bin_len - 1; /* It's the PATH scheme, can just set plain data. * In this case, @bin_len0 includes */ - if ( bin_len0 >= NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH) - && g_str_has_prefix (bin, NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)) { + if (HAS_SCHEME_PREFIX (bin, bin_len, NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)) { if (nm_setting_802_1x_check_cert_scheme (bin, bin_len0 + 1, NULL) == NM_SETTING_802_1X_CK_SCHEME_PATH) { const char *path2 = &bin[NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)]; gs_free char *path2_free = NULL; @@ -1032,8 +1041,8 @@ handle_as_scheme (KeyfileReaderInfo *info, GBytes *bytes, NMSetting *setting, co } return TRUE; } - if ( bin_len0 >= NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PKCS11) - && g_str_has_prefix (bin, NM_KEYFILE_CERT_SCHEME_PREFIX_PKCS11)) { + + if (HAS_SCHEME_PREFIX (bin, bin_len, NM_KEYFILE_CERT_SCHEME_PREFIX_PKCS11)) { if (nm_setting_802_1x_check_cert_scheme (bin, bin_len0 + 1, NULL) == NM_SETTING_802_1X_CK_SCHEME_PKCS11) { g_object_set (setting, key, bytes, NULL); } else { @@ -1042,13 +1051,14 @@ handle_as_scheme (KeyfileReaderInfo *info, GBytes *bytes, NMSetting *setting, co } return TRUE; } - if ( bin_len0 > NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB) - && g_str_has_prefix (bin, NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB)) { + + if (HAS_SCHEME_PREFIX (bin, bin_len, NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB)) { const char *cdata = bin + NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB); - guchar *bin_decoded; - GBytes *bytes2; + gs_unref_bytes GBytes *bytes2 = NULL; + gs_free guchar *bin_decoded = NULL; gsize i; gboolean valid_base64; + gsize bin_decoded_len = 0; bin_len0 -= NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB); @@ -1074,29 +1084,29 @@ handle_as_scheme (KeyfileReaderInfo *info, GBytes *bytes, NMSetting *setting, co } } } - if (!valid_base64) { + if (valid_base64) + bin_decoded = g_base64_decode (cdata, &bin_decoded_len); + + if (bin_decoded_len == 0) { handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, _("invalid key/cert value data:;base64, is not base64")); return TRUE; } - bin_decoded = g_base64_decode (cdata, &bin_decoded_len); - - g_return_val_if_fail (bin_decoded_len > 0, FALSE); if (nm_setting_802_1x_check_cert_scheme (bin_decoded, bin_decoded_len, NULL) != NM_SETTING_802_1X_CK_SCHEME_BLOB) { /* The blob probably starts with "file://". Setting the cert data will confuse NMSetting8021x. * In fact this is a limitation of NMSetting8021x which does not support setting blobs that start * with file://. Just warn and return TRUE to signal that we ~handled~ the setting. */ - g_free (bin_decoded); handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, _("invalid key/cert value data:;base64,file://")); - } else { - bytes2 = g_bytes_new_take (bin_decoded, bin_decoded_len); - g_object_set (setting, key, bytes2, NULL); - g_bytes_unref (bytes2); + return TRUE; } + + bytes2 = g_bytes_new_take (g_steal_pointer (&bin_decoded), bin_decoded_len); + g_object_set (setting, key, bytes2, NULL); return TRUE; } + return FALSE; } @@ -1184,27 +1194,27 @@ handle_as_path (KeyfileReaderInfo *info, gsize data_len; char *path; gboolean exists = FALSE; - GBytes *val; data = g_bytes_get_data (bytes, &data_len); path = nm_keyfile_detect_unqualified_path_scheme (info->base_dir, data, data_len, TRUE, &exists); - if (!path) - return FALSE; + if (path) { + gs_unref_bytes GBytes *val = NULL; - /* Construct the proper value as required for the PATH scheme */ - val = g_bytes_new_take (path, strlen (path) + 1); - g_object_set (setting, key, val, NULL); + /* Construct the proper value as required for the PATH scheme */ + val = g_bytes_new_take (path, strlen (path) + 1); + g_object_set (setting, key, val, NULL); - /* Warn if the certificate didn't exist */ - if (!exists) { - handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_INFO_MISSING_FILE, - _("certificate or key file '%s' does not exist"), - path); + /* Warn if the certificate didn't exist */ + if (!exists) { + handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_INFO_MISSING_FILE, + _("certificate or key file '%s' does not exist"), + path); + } + return TRUE; } - g_bytes_unref (val); - return TRUE; + return FALSE; } static void @@ -1212,37 +1222,43 @@ cert_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) { const char *setting_name = nm_setting_get_name (setting); gs_unref_bytes GBytes *bytes = NULL; - gsize bin_len; - const char *bin; + const char *bin = NULL; + gsize bin_len = 0; bytes = get_bytes (info, setting_name, key, TRUE, FALSE); - if (bytes) { - /* Try as a path + scheme (ie, starts with "file://") */ - if (handle_as_scheme (info, bytes, setting, key)) - return; - if (info->error) - return; - - /* If not, it might be a plain path */ - if (handle_as_path (info, bytes, setting, key)) - return; - if (info->error) - return; - + if (bytes) bin = g_bytes_get_data (bytes, &bin_len); - if (nm_setting_802_1x_check_cert_scheme (bin, bin_len, NULL) != NM_SETTING_802_1X_CK_SCHEME_BLOB) { - /* The blob probably starts with "file://" but contains invalid characters for a path. - * Setting the cert data will confuse NMSetting8021x. - * In fact, NMSetting8021x does not support setting such binary data, so just warn and - * continue. */ + if (bin_len == 0) { + if (!info->error) { handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, - _("invalid key/cert value is not a valid blob")); - } else - g_object_set (setting, key, bytes, NULL); - } else if (!info->error) { - handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, - _("invalid key/cert value")); + _("invalid key/cert value")); + } + return; } + + /* Try as a path + scheme (ie, starts with "file://") */ + if (handle_as_scheme (info, bytes, setting, key)) + return; + if (info->error) + return; + + /* If not, it might be a plain path */ + if (handle_as_path (info, bytes, setting, key)) + return; + if (info->error) + return; + + if (nm_setting_802_1x_check_cert_scheme (bin, bin_len, NULL) != NM_SETTING_802_1X_CK_SCHEME_BLOB) { + /* The blob probably starts with "file://" but contains invalid characters for a path. + * Setting the cert data will confuse NMSetting8021x. + * In fact, NMSetting8021x does not support setting such binary data, so just warn and + * continue. */ + handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, + _("invalid key/cert value is not a valid blob")); + return; + } + + g_object_set (setting, key, bytes, NULL); } static void From 2aebb343d90bfd27279eafc3b02a9fb171cefeaf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Feb 2018 19:11:05 +0100 Subject: [PATCH 3/3] libnm/keyfile: refactor cert_parser() by merge helper functions The parsing of the certificate consists of a series of checks, and if a check matches, we determine the type and are done. Moving these checks to different functions (that are only called once) makes it more complicated to understand what really happens. Merge them all together. --- libnm-core/nm-keyfile-reader.c | 276 ++++++++++++++------------------- 1 file changed, 118 insertions(+), 158 deletions(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index 93258355e4..1e75cbf922 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -990,126 +990,6 @@ has_cert_ext (const char *path) return FALSE; } -static gboolean -handle_as_scheme (KeyfileReaderInfo *info, GBytes *bytes, NMSetting *setting, const char *key) -{ - const char *bin; - gsize bin_len; - gsize bin_len0; - - bin = g_bytes_get_data (bytes, &bin_len); - - g_return_val_if_fail (bin && bin_len > 0, FALSE); - -#define HAS_SCHEME_PREFIX(bin, bin_len, scheme) \ - ({ \ - const char *const _bin = (bin); \ - const gsize _bin_len = (bin_len); \ - \ - nm_assert (_bin && _bin_len > 0); \ - \ - ( _bin_len > NM_STRLEN (scheme) + 1 \ - && _bin[_bin_len - 1] == '\0' \ - && memcmp (_bin, scheme, NM_STRLEN (scheme)) == 0); \ - }) - - bin_len0 = bin_len - 1; - - /* It's the PATH scheme, can just set plain data. - * In this case, @bin_len0 includes */ - if (HAS_SCHEME_PREFIX (bin, bin_len, NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)) { - if (nm_setting_802_1x_check_cert_scheme (bin, bin_len0 + 1, NULL) == NM_SETTING_802_1X_CK_SCHEME_PATH) { - const char *path2 = &bin[NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)]; - gs_free char *path2_free = NULL; - - if (path2[0] != '/') { - /* we want to read absolute paths because we use keyfile as exchange - * between different processes which might not have the same cwd. */ - path2 = path2_free = get_cert_path (info->base_dir, (const guint8 *) path2, - bin_len0 - NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)); - } - - g_object_set (setting, key, bytes, NULL); - if (!g_file_test (path2, G_FILE_TEST_EXISTS)) { - handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_INFO_MISSING_FILE, - _("certificate or key file '%s' does not exist"), - path2); - } - } else { - handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, - _("invalid key/cert value path \"%s\""), bin); - } - return TRUE; - } - - if (HAS_SCHEME_PREFIX (bin, bin_len, NM_KEYFILE_CERT_SCHEME_PREFIX_PKCS11)) { - if (nm_setting_802_1x_check_cert_scheme (bin, bin_len0 + 1, NULL) == NM_SETTING_802_1X_CK_SCHEME_PKCS11) { - g_object_set (setting, key, bytes, NULL); - } else { - handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, - _("invalid PKCS#11 URI \"%s\""), bin); - } - return TRUE; - } - - if (HAS_SCHEME_PREFIX (bin, bin_len, NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB)) { - const char *cdata = bin + NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB); - gs_unref_bytes GBytes *bytes2 = NULL; - gs_free guchar *bin_decoded = NULL; - gsize i; - gboolean valid_base64; - gsize bin_decoded_len = 0; - - bin_len0 -= NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB); - - /* Let's be strict here. We expect valid base64, no funny stuff!! - * We didn't write such invalid data ourselfes and refuse to read it as blob. */ - if ((valid_base64 = (bin_len0 % 4 == 0))) { - for (i = 0; i < bin_len0; i++) { - char c = cdata[i]; - - if (!( (c >= 'a' && c <= 'z') - || (c >= 'A' && c <= 'Z') - || (c >= '0' && c <= '9') - || (c == '+' || c == '/'))) { - if (c != '=' || i < bin_len0 - 2) - valid_base64 = FALSE; - else { - for (; i < bin_len0; i++) { - if (cdata[i] != '=') - valid_base64 = FALSE; - } - } - break; - } - } - } - if (valid_base64) - bin_decoded = g_base64_decode (cdata, &bin_decoded_len); - - if (bin_decoded_len == 0) { - handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, - _("invalid key/cert value data:;base64, is not base64")); - return TRUE; - } - - if (nm_setting_802_1x_check_cert_scheme (bin_decoded, bin_decoded_len, NULL) != NM_SETTING_802_1X_CK_SCHEME_BLOB) { - /* The blob probably starts with "file://". Setting the cert data will confuse NMSetting8021x. - * In fact this is a limitation of NMSetting8021x which does not support setting blobs that start - * with file://. Just warn and return TRUE to signal that we ~handled~ the setting. */ - handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, - _("invalid key/cert value data:;base64,file://")); - return TRUE; - } - - bytes2 = g_bytes_new_take (g_steal_pointer (&bin_decoded), bin_decoded_len); - g_object_set (setting, key, bytes2, NULL); - return TRUE; - } - - return FALSE; -} - char * nm_keyfile_detect_unqualified_path_scheme (const char *base_dir, gconstpointer pdata, @@ -1184,38 +1064,17 @@ out: return path; } -static gboolean -handle_as_path (KeyfileReaderInfo *info, - GBytes *bytes, - NMSetting *setting, - const char *key) -{ - const guint8 *data; - gsize data_len; - char *path; - gboolean exists = FALSE; - - data = g_bytes_get_data (bytes, &data_len); - - path = nm_keyfile_detect_unqualified_path_scheme (info->base_dir, data, data_len, TRUE, &exists); - if (path) { - gs_unref_bytes GBytes *val = NULL; - - /* Construct the proper value as required for the PATH scheme */ - val = g_bytes_new_take (path, strlen (path) + 1); - g_object_set (setting, key, val, NULL); - - /* Warn if the certificate didn't exist */ - if (!exists) { - handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_INFO_MISSING_FILE, - _("certificate or key file '%s' does not exist"), - path); - } - return TRUE; - } - - return FALSE; -} +#define HAS_SCHEME_PREFIX(bin, bin_len, scheme) \ + ({ \ + const char *const _bin = (bin); \ + const gsize _bin_len = (bin_len); \ + \ + nm_assert (_bin && _bin_len > 0); \ + \ + ( _bin_len > NM_STRLEN (scheme) + 1 \ + && _bin[_bin_len - 1] == '\0' \ + && memcmp (_bin, scheme, NM_STRLEN (scheme)) == 0); \ + }) static void cert_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) @@ -1224,6 +1083,8 @@ cert_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) gs_unref_bytes GBytes *bytes = NULL; const char *bin = NULL; gsize bin_len = 0; + char *path; + gboolean path_exists; bytes = get_bytes (info, setting_name, key, TRUE, FALSE); if (bytes) @@ -1236,17 +1097,116 @@ cert_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) return; } - /* Try as a path + scheme (ie, starts with "file://") */ - if (handle_as_scheme (info, bytes, setting, key)) + if (HAS_SCHEME_PREFIX (bin, bin_len, NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)) { + const char *path2 = &bin[NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)]; + gs_free char *path2_free = NULL; + + if (nm_setting_802_1x_check_cert_scheme (bin, bin_len, NULL) != NM_SETTING_802_1X_CK_SCHEME_PATH) { + handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, + _("invalid key/cert value path \"%s\""), bin); + return; + } + + g_object_set (setting, key, bytes, NULL); + + if (path2[0] != '/') { + /* we want to read absolute paths because we use keyfile as exchange + * between different processes which might not have the same cwd. */ + path2_free = get_cert_path (info->base_dir, (const guint8 *) path2, + bin_len - NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH) - 1); + path2 = path2_free; + } + + if (!g_file_test (path2, G_FILE_TEST_EXISTS)) { + handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_INFO_MISSING_FILE, + _("certificate or key file '%s' does not exist"), + path2); + } return; - if (info->error) + } + + if (HAS_SCHEME_PREFIX (bin, bin_len, NM_KEYFILE_CERT_SCHEME_PREFIX_PKCS11)) { + if (nm_setting_802_1x_check_cert_scheme (bin, bin_len, NULL) != NM_SETTING_802_1X_CK_SCHEME_PKCS11) { + handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, + _("invalid PKCS#11 URI \"%s\""), bin); + return; + } + + g_object_set (setting, key, bytes, NULL); return; + } + + if (HAS_SCHEME_PREFIX (bin, bin_len, NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB)) { + const char *cdata = bin + NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB); + gsize cdata_len = bin_len - NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB) - 1; + gs_free guchar *bin_decoded = NULL; + gsize bin_decoded_len = 0; + gsize i; + gboolean valid_base64; + gs_unref_bytes GBytes *val = NULL; + + /* Let's be strict here. We expect valid base64, no funny stuff!! + * We didn't write such invalid data ourselfes and refuse to read it as blob. */ + if ((valid_base64 = (cdata_len % 4 == 0))) { + for (i = 0; i < cdata_len; i++) { + char c = cdata[i]; + + if (!( (c >= 'a' && c <= 'z') + || (c >= 'A' && c <= 'Z') + || (c >= '0' && c <= '9') + || (c == '+' || c == '/'))) { + if (c != '=' || i < cdata_len - 2) + valid_base64 = FALSE; + else { + for (; i < cdata_len; i++) { + if (cdata[i] != '=') + valid_base64 = FALSE; + } + } + break; + } + } + } + if (valid_base64) + bin_decoded = g_base64_decode (cdata, &bin_decoded_len); + + if (bin_decoded_len == 0) { + handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, + _("invalid key/cert value data:;base64, is not base64")); + return; + } + + if (nm_setting_802_1x_check_cert_scheme (bin_decoded, bin_decoded_len, NULL) != NM_SETTING_802_1X_CK_SCHEME_BLOB) { + /* The blob probably starts with "file://". Setting the cert data will confuse NMSetting8021x. + * In fact this is a limitation of NMSetting8021x which does not support setting blobs that start + * with file://. Just warn and return TRUE to signal that we ~handled~ the setting. */ + handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, + _("invalid key/cert value data:;base64,file://")); + return; + } + + val = g_bytes_new_take (g_steal_pointer (&bin_decoded), bin_decoded_len); + g_object_set (setting, key, val, NULL); + return; + } /* If not, it might be a plain path */ - if (handle_as_path (info, bytes, setting, key)) - return; - if (info->error) + path = nm_keyfile_detect_unqualified_path_scheme (info->base_dir, bin, bin_len, TRUE, &path_exists); + if (path) { + gs_unref_bytes GBytes *val = NULL; + + /* Construct the proper value as required for the PATH scheme */ + val = g_bytes_new_take (path, strlen (path) + 1); + g_object_set (setting, key, val, NULL); + + /* Warn if the certificate didn't exist */ + if (!path_exists) { + handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_INFO_MISSING_FILE, + _("certificate or key file '%s' does not exist"), + path); + } return; + } if (nm_setting_802_1x_check_cert_scheme (bin, bin_len, NULL) != NM_SETTING_802_1X_CK_SCHEME_BLOB) { /* The blob probably starts with "file://" but contains invalid characters for a path.