From 9aa02f6543f525740bb9e5594fcde835905ba6fd Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Sat, 19 Mar 2022 00:30:05 +0100
Subject: [PATCH] crypto: cleanup error handling in nm_crypto_is_pkcs12_data()

Our convention is that a function that fails MUST set the GError output.
No need to check for that in nm_crypto_is_pkcs12_data(). Simplify the
error paths.

Also, in gnutls' _nm_crypto_verify_pkcs12(), don't call gnutls_pkcs12_deinit()
before gnutls_strerror(). It's unclear whether that couldn't set a
different error reason.
---
 src/libnm-crypto/nm-crypto-gnutls.c |  6 +++---
 src/libnm-crypto/nm-crypto-nss.c    |  1 +
 src/libnm-crypto/nm-crypto.c        | 21 +++++++++------------
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/src/libnm-crypto/nm-crypto-gnutls.c b/src/libnm-crypto/nm-crypto-gnutls.c
index 7352a38e6..60adf4d3f 100644
--- a/src/libnm-crypto/nm-crypto-gnutls.c
+++ b/src/libnm-crypto/nm-crypto-gnutls.c
@@ -330,18 +330,18 @@ _nm_crypto_verify_pkcs12(const guint8 *data, gsize data_len, const char *passwor
     }
 
     err = gnutls_pkcs12_verify_mac(p12, password);
-
-    gnutls_pkcs12_deinit(p12);
-
     if (err != GNUTLS_E_SUCCESS) {
         g_set_error(error,
                     _NM_CRYPTO_ERROR,
                     _NM_CRYPTO_ERROR_DECRYPTION_FAILED,
                     _("Couldn't verify PKCS#12 file: %s"),
                     gnutls_strerror(err));
+        gnutls_pkcs12_deinit(p12);
         return FALSE;
     }
 
+    gnutls_pkcs12_deinit(p12);
+
     return TRUE;
 }
 
diff --git a/src/libnm-crypto/nm-crypto-nss.c b/src/libnm-crypto/nm-crypto-nss.c
index cd5966c42..b31ca55ee 100644
--- a/src/libnm-crypto/nm-crypto-nss.c
+++ b/src/libnm-crypto/nm-crypto-nss.c
@@ -509,6 +509,7 @@ out:
     if (pw.data)
         SECITEM_ZfreeItem(&pw, PR_FALSE);
 
+    nm_assert(!error || (success == (!*error)));
     return success;
 }
 
diff --git a/src/libnm-crypto/nm-crypto.c b/src/libnm-crypto/nm-crypto.c
index 4a38f0c4c..69d2b53f9 100644
--- a/src/libnm-crypto/nm-crypto.c
+++ b/src/libnm-crypto/nm-crypto.c
@@ -757,8 +757,8 @@ out:
 gboolean
 nm_crypto_is_pkcs12_data(const guint8 *data, gsize data_len, GError **error)
 {
-    GError  *local = NULL;
-    gboolean success;
+    gs_free_error GError *local = NULL;
+    gboolean              success;
 
     if (!data_len) {
         g_set_error(error,
@@ -774,17 +774,14 @@ nm_crypto_is_pkcs12_data(const guint8 *data, gsize data_len, GError **error)
         return FALSE;
 
     success = _nm_crypto_verify_pkcs12(data, data_len, NULL, &local);
-    if (success == FALSE) {
-        /* If the error was just a decryption error, then it's pkcs#12 */
-        if (local) {
-            if (g_error_matches(local, _NM_CRYPTO_ERROR, _NM_CRYPTO_ERROR_DECRYPTION_FAILED)) {
-                success = TRUE;
-                g_error_free(local);
-            } else
-                g_propagate_error(error, local);
-        }
+
+    /* If the error was just a decryption error, then it's pkcs#12 */
+    if (!success && !g_error_matches(local, _NM_CRYPTO_ERROR, _NM_CRYPTO_ERROR_DECRYPTION_FAILED)) {
+        g_propagate_error(error, g_steal_pointer(&local));
+        return FALSE;
     }
-    return success;
+
+    return TRUE;
 }
 
 gboolean