From 9c139b2c478c25ea8d313acb279c7bfbbe858303 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 4 Jan 2019 12:02:20 +0100
Subject: [PATCH] libnm: cleanup NMSettingVpn's get_secret_flags()

- most of the time, the secret-name is short and fits in a
  stack-allocated buffer.
  Optimize for that by using nm_construct_name_a().

- use _nm_utils_ascii_str_to_int64() instead of strtoul().

       tmp = strtoul ((const char *) val, NULL, 10);
       if ((errno != 0) || (tmp > NM_SETTING_SECRET_FLAGS_ALL)) {

  is not the right way to check for errors of strtoul().

- refactor the code to return-early on errors.

- since commit 9b96bfaa72 "setting-vpn: whatever is in vpn.secrets always
  is a secrets", we accept secrets without secret-flags as valid too.
  However, only do that, when we at least have a corresponding key in
  priv->secrets hash. If the secret name is not used at all, it's
  clearly not a secret.

- if the secret flags are not a valid number, pretend that the flags
  are still set to "none" (zero). That is because we use the presence
  of the "*-flags" data item as indication that this is in fact a
  secret. The user cannot use data items with such a name for another
  purpose, so on failure, we still claim that this is in fact a secret.
---
 libnm-core/nm-setting-vpn.c | 47 +++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/libnm-core/nm-setting-vpn.c b/libnm-core/nm-setting-vpn.c
index fcbeec6854..87c30f7b40 100644
--- a/libnm-core/nm-setting-vpn.c
+++ b/libnm-core/nm-setting-vpn.c
@@ -647,29 +647,42 @@ get_secret_flags (NMSetting *setting,
                   GError **error)
 {
 	NMSettingVpnPrivate *priv = NM_SETTING_VPN_GET_PRIVATE (setting);
-	gs_free char *flags_key = NULL;
-	gpointer val;
-	unsigned long tmp;
-	NMSettingSecretFlags flags = NM_SETTING_SECRET_FLAG_NONE;
+	gs_free char *flags_key_free = NULL;
+	const char *flags_key;
+	const char *flags_val;
+	gint64 i64;
 
-	flags_key = g_strdup_printf ("%s-flags", secret_name);
-	if (g_hash_table_lookup_extended (priv->data, flags_key, NULL, &val)) {
-		errno = 0;
-		tmp = strtoul ((const char *) val, NULL, 10);
-		if ((errno != 0) || (tmp > NM_SETTING_SECRET_FLAGS_ALL)) {
-			g_set_error (error,
-			             NM_CONNECTION_ERROR,
-			             NM_CONNECTION_ERROR_INVALID_PROPERTY,
-			             _("failed to convert value '%s' to uint"),
-			             (const char *) val);
+	flags_key = nm_construct_name_a ("%s-flags", secret_name, &flags_key_free);
+
+	if (!g_hash_table_lookup_extended (priv->data, flags_key, NULL, (gpointer *) &flags_val)) {
+		NM_SET_OUT (out_flags, NM_SETTING_SECRET_FLAG_NONE);
+
+		/* having no secret flag for the secret is fine, as long as there
+		 * is the secret itself... */
+		if (   verify_secret
+		    && !g_hash_table_contains (priv->secrets, secret_name)) {
+			g_set_error_literal (error,
+			                     NM_CONNECTION_ERROR,
+			                     NM_CONNECTION_ERROR_PROPERTY_NOT_SECRET,
+			                     _("secret flags property not found"));
 			g_prefix_error (error, "%s.%s: ", NM_SETTING_VPN_SETTING_NAME, flags_key);
 			return FALSE;
 		}
-		flags = (NMSettingSecretFlags) tmp;
+		return TRUE;
 	}
 
-	if (out_flags)
-		*out_flags = flags;
+	i64 = _nm_utils_ascii_str_to_int64 (flags_val, 10, 0, NM_SETTING_SECRET_FLAGS_ALL, -1);
+	if (i64 == -1) {
+		/* The flags keys is set to an unexpected value. That is a configuration
+		 * error. Note that keys named "*-flags" are reserved for secrets. The user
+		 * must not use this for anything but secret flags. Hence, we cannot fail
+		 * to read the secret, we pretend that the secret flag is set to the default
+		 * NM_SETTING_SECRET_FLAG_NONE. */
+		NM_SET_OUT (out_flags, NM_SETTING_SECRET_FLAG_NONE);
+		return TRUE;
+	}
+
+	NM_SET_OUT (out_flags, (NMSettingSecretFlags) i64);
 	return TRUE;
 }