libnm-core: make failure cases for nm_connection_normalize() more robust against bugs

We would expect that attempts to normalize a connection
are successful as verify() indicates. This way, a connection
is not modified if it cannot be fixed, and a connection will
be valid and modified after attempts to normalization.

However, there might be subtle, unexpected ways how this can fail.

For example, if NMSettingConnection:verify() detects a missing base type
setting, it returns NORMALIZABLE_ERROR if it finds a valid
NMSettingConnection:type. Normalization then adds an empty, default
setting. However, a new verify() might fail due to other reasons.
This would be a bug in NMSettingConnection:verify() which must not
indicate that it is able to normalize the connection, when it actually
is unable to do so.

Such bugs need fixing, but the code should be more robust to this case
because there might be complex, unanticipated situations.
Especially since NM relies on having a valid connection after normalize(),
so a strict error-out behavior is important.

Signed-off-by: Thomas Haller <thaller@redhat.com>

1 files changed, 27 insertions, 9 deletions

diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c
index 1f112603a1..24b1cd47f6 100644
--- a/libnm-core/nm-connection.c
+++ b/libnm-core/nm-connection.c
@@ -902,7 +902,16 @@ nm_connection_normalize (NMConnection *connection,
 	    success == NM_SETTING_VERIFY_SUCCESS) {
 		if (normalizable_error)
 			g_propagate_error (error, normalizable_error);
-		goto EXIT;
+		if (modified)
+			*modified = FALSE;
+		if (success == NM_SETTING_VERIFY_ERROR && error && !*error) {
+			g_set_error_literal (error,
+			                     NM_CONNECTION_ERROR,
+			                     NM_CONNECTION_ERROR_UNKNOWN,
+			                     _("Unexpected failure to verify the connection"));
+			g_return_val_if_reached (FALSE);
+		}
+		return success == NM_SETTING_VERIFY_SUCCESS;
 	}
 	g_assert (success == NM_SETTING_VERIFY_NORMALIZABLE || success == NM_SETTING_VERIFY_NORMALIZABLE_ERROR);
 	g_clear_error (&normalizable_error);
@@ -919,17 +928,26 @@ nm_connection_normalize (NMConnection *connection,
 	/* Verify anew. */
 	success = _nm_connection_verify (connection, error);
 
-	/* we would expect, that after normalization, the connection can be verified. */
-	g_return_val_if_fail (success == NM_SETTING_VERIFY_SUCCESS, success);
-
-	/* we would expect, that the connection was modified during normalization. */
-	g_return_val_if_fail (was_modified, success);
-
-EXIT:
 	if (modified)
 		*modified = was_modified;
 
-	return success == NM_SETTING_VERIFY_SUCCESS;
+	if (success != NM_SETTING_VERIFY_SUCCESS) {
+		/* we would expect, that after normalization, the connection can be verified.
+		 * Also treat NM_SETTING_VERIFY_NORMALIZABLE as failure, because there is something
+		 * odd going on. */
+		if (error && !*error) {
+			g_set_error_literal (error,
+			                     NM_CONNECTION_ERROR,
+			                     NM_CONNECTION_ERROR_UNKNOWN,
+			                     _("Unexpected failure to normalize the connection"));
+		}
+		g_return_val_if_reached (FALSE);
+	}
+
+	/* we would expect, that the connection was modified during normalization. */
+	g_return_val_if_fail (was_modified, TRUE);
+
+	return TRUE;
 }
 
 /**