libnmc: fix secrets request on 2nd stage of 2FA authentication

Clients using nm-secret-agent-simple always asked for some default VPN
secrets, which are dependent on the VPN service, when the auth dialog
can't be used and the fallback method is used instead.

When using 2FA this has to be avoided in the 2nd step because those
default secrets were already requested and validated in the 1st step.

Fix it by adding a new "x-dynamic-challenge" prefix tag that can be used
in the hints received from the VPN plugin. This tag indicates that we
are in the 2nd step of a 2FA authentication. This way we know that we
don't have to request the default secrets this time. Note that the tag
name doesn't explicitly mention VPNs so it can be reused for other type
of connections in the future.

As the default secrets were requested always unconditionally when using
the fallback method, there is no possible workaround to this problem
that avoids having to change libnm-client.

The change is backwards compatible because VPN plugins were not using
the tag and the previous behaviour does not change if the tag is not
used. However, VPN plugins that want to properly support 2FA
aunthentication will need to bump the NM version dependency because
old daemons won't handle properly a hint with the new prefix tag.

Finally, move the macro that defines the "x-vpn-message:" tag in a public
header so it is more visible for users. It has been renamed and prefixed
with the NM_ namespace so it shouldn't collide with macros defined in
the VPN plugins.

2 files changed, 32 insertions, 6 deletions

diff --git a/src/libnm-core-public/nm-dbus-interface.h b/src/libnm-core-public/nm-dbus-interface.h
index 5acbf467f5..5467a99601 100644
--- a/src/libnm-core-public/nm-dbus-interface.h
+++ b/src/libnm-core-public/nm-dbus-interface.h
@@ -1415,4 +1415,18 @@ typedef enum /*< flags >*/ {
     NM_MPTCP_FLAGS_FULLMESH = 0x80,
 } NMMptcpFlags;
 
+/* For secrets requests, hints starting with "x-vpn-message:" are a message to show, not
+ * a secret to request
+ */
+#define NM_SECRET_TAG_VPN_MSG "x-vpn-message:"
+
+/* For secrets requests, hints starting with "x-dynamic-challenge:" are dynamic
+ * 2FA challenges that are requested in a second authentication step, after the password
+ * (or whatever auth method is used) was already successfully validated. Because of
+ * that, the default secrets of the service mustn't be requested (again).
+ *
+ * Note: currently only implemented for VPN, but can be extended.
+ */
+#define NM_SECRET_TAG_DYNAMIC_CHALLENGE "x-dynamic-challenge:"
+
 #endif /* __NM_DBUS_INTERFACE_H__ */
diff --git a/src/libnmc-base/nm-secret-agent-simple.c b/src/libnmc-base/nm-secret-agent-simple.c
index 1b9aa57142..02f492bc7c 100644
--- a/src/libnmc-base/nm-secret-agent-simple.c
+++ b/src/libnmc-base/nm-secret-agent-simple.c
@@ -416,8 +416,6 @@ add_vpn_secret_helper(GPtrArray    *secrets,
     }
 }
 
-#define VPN_MSG_TAG "x-vpn-message:"
-
 static gboolean
 add_vpn_secrets(RequestData *request, GPtrArray *secrets, char **msg)
 {
@@ -425,19 +423,33 @@ add_vpn_secrets(RequestData *request, GPtrArray *secrets, char **msg)
     const NmcVpnPasswordName *p;
     const char               *vpn_msg = NULL;
     char                    **iter;
+    char                     *secret_name;
+    bool                      is_challenge = FALSE;
 
     /* If hints are given, then always ask for what the hints require */
     if (request->hints) {
         for (iter = request->hints; *iter; iter++) {
-            if (!vpn_msg && g_str_has_prefix(*iter, VPN_MSG_TAG))
-                vpn_msg = &(*iter)[NM_STRLEN(VPN_MSG_TAG)];
-            else
-                add_vpn_secret_helper(secrets, s_vpn, *iter, *iter);
+            if (!vpn_msg && NM_STR_HAS_PREFIX(*iter, NM_SECRET_TAG_VPN_MSG)) {
+                vpn_msg = &(*iter)[NM_STRLEN(NM_SECRET_TAG_VPN_MSG)];
+            } else {
+                if (NM_STR_HAS_PREFIX(*iter, NM_SECRET_TAG_DYNAMIC_CHALLENGE)) {
+                    secret_name  = &(*iter)[NM_STRLEN(NM_SECRET_TAG_DYNAMIC_CHALLENGE)];
+                    is_challenge = TRUE;
+                } else {
+                    secret_name = *iter;
+                }
+
+                add_vpn_secret_helper(secrets, s_vpn, secret_name, secret_name);
+            }
         }
     }
 
     NM_SET_OUT(msg, g_strdup(vpn_msg));
 
+    /* If we are in the 2nd step of a 2FA authentication, don't ask again for the default secrets */
+    if (is_challenge)
+        return TRUE;
+
     /* Now add what client thinks might be required, because hints may be empty or incomplete */
     p = nm_vpn_get_secret_names(nm_setting_vpn_get_service_type(s_vpn));
     while (p && p->name) {