User must not be able to delete his last active otp token

The 389-ds plugin for OTP last token is performing data initialization
in its ipa_otp_lasttoken_init method, which is wrong according to
the Plug-in Guide:
> For example, the init function should not attempt to perform an
> internal search or other internal operation, because the all of
> the subsystems are not up and running during the init phase.

This init method fills a structure containing the configuration of
allowed authentication types. As the method is called too early, the
method does not find any suffix and leaves the structure empty.
Subsequent calls find an empty structure and take the default values
(for authentication methods, the default is 1 = password).

Because of that, the code consider that the global configuration defines
password authentication method, and in this case it is allowed to delete
a user's last otp token.

The fix implements a SLAPI_PLUGIN_START_FN method that will be called
when 389-ds is ready to initialize the plugin data, ensuring that the
structure is properly initialized.

Fixes:
https://pagure.io/freeipa/issue/7012

Reviewed-By: Nathaniel McCallum <npmccallum@redhat.com>
Reviewed-By: Alexey Slaykovsky <alexey@slaykovsky.com>

1 files changed, 24 insertions, 8 deletions

diff --git a/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c b/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c
index a085a3a32..b7a2ba7f0 100644
--- a/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c
+++ b/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c
@@ -50,6 +50,7 @@
 #define OTP_CONTAINER "cn=otp,%s"
 
 static struct otp_config *otp_config;
+void *ipa_otp_lasttoken_plugin_id;
 
 static bool entry_is_token(Slapi_Entry *entry)
 {
@@ -255,6 +256,17 @@ static int postop_init(Slapi_PBlock *pb)
     return ret;
 }
 
+/* Init data structs */
+static int ipa_otp_lasttoken_start(Slapi_PBlock *pb)
+{
+    /* NOTE: We never call otp_config_fini() from a destructor. This is because
+     *       it may race with threaded requests at shutdown. This leak should
+     *       only occur when the DS is exiting, so it isn't a big deal.
+     */
+    otp_config = otp_config_init(ipa_otp_lasttoken_plugin_id);
+    return LDAP_SUCCESS;
+}
+
 int ipa_otp_lasttoken_init(Slapi_PBlock *pb)
 {
     static const Slapi_PluginDesc preop_desc = {
@@ -264,20 +276,24 @@ int ipa_otp_lasttoken_init(Slapi_PBlock *pb)
         "Protect the user's last active token"
     };
 
-    Slapi_ComponentId *plugin_id = NULL;
     int ret = 0;
 
-    ret |= slapi_pblock_get(pb, SLAPI_PLUGIN_IDENTITY, &plugin_id);
+    ret |= slapi_pblock_get(pb, SLAPI_PLUGIN_IDENTITY,
+                            &ipa_otp_lasttoken_plugin_id);
     ret |= slapi_pblock_set(pb, SLAPI_PLUGIN_VERSION, SLAPI_PLUGIN_VERSION_01);
     ret |= slapi_pblock_set(pb, SLAPI_PLUGIN_DESCRIPTION, (void *) &preop_desc);
     ret |= slapi_register_plugin("betxnpreoperation", 1, __func__, preop_init,
-                                 PLUGIN_NAME " betxnpreoperation", NULL, plugin_id);
+                                 PLUGIN_NAME " betxnpreoperation", NULL,
+                                 ipa_otp_lasttoken_plugin_id);
     ret |= slapi_register_plugin("postoperation", 1, __func__, postop_init,
-                                 PLUGIN_NAME " postoperation", NULL, plugin_id);
-    ret |= slapi_register_plugin("internalpostoperation", 1, __func__, intpostop_init,
-                                 PLUGIN_NAME " internalpostoperation", NULL, plugin_id);
+                                 PLUGIN_NAME " postoperation", NULL,
+                                 ipa_otp_lasttoken_plugin_id);
+    ret |= slapi_register_plugin("internalpostoperation", 1, __func__,
+                                 intpostop_init,
+                                 PLUGIN_NAME " internalpostoperation", NULL,
+                                 ipa_otp_lasttoken_plugin_id);
+    ret |= slapi_pblock_set(pb, SLAPI_PLUGIN_START_FN,
+                            (void *)ipa_otp_lasttoken_start);
 
-    /* NOTE: leak otp_config on process exit. */
-    otp_config = otp_config_init(plugin_id);
     return ret;
 }