Plugin project proposal

[Zhanna Tsitkova]

Well, in the proposed implementation the plugin module writer would do the following:

plhandle
plugin_pwd_qlty_krb_create()
{
plhandle handle;
plugin_pwd_qlty* api = malloc(sizeof(plugin_pwd_qlty));
api->version = 1;
...
api->pwd_qlty_init    = _plugin_pwd_qlty_init;
api->pwd_qlty_check   = _plugin_pwd_qlty_check;
api->pwd_qlty_cleanup = _plugin_pwd_qlty_clean;
handle.api = api;
return handle;
}

and then define each of the methods:

static kadm5_ret_t     
_plugin_pwd_qlty_check(...l)
{
// do actual quality validation and return
}

In my opinion this approach is friendlier to the writer of the plugin implementation. The writer can concentrate on the particular functionality without caring the burden of other implementations.

Thanks,
Zhanna

________________________________________
From: Nicolas Williams [Nicolas.Williams at oracle.com]
Sent: Thursday, July 15, 2010 3:31 PM
To: Zhanna Tsitkova
Cc: rra at stanford.edu; krbdev at mit.edu
Subject: Re: Plugin project proposal

On Thu, Jul 15, 2010 at 03:06:11PM -0400, Zhanna Tsitkova wrote:
> On Jul 15, 2010, at 2:57 PM, Nicolas Williams wrote:
> >What is used as a key to the table?
>
> Suppose we have the following v-table for plugin password quality:
> /* PWD_QLTY API */
> typedef struct {
> int version;
> char plugin_id[MAX_PL_NAME_LEN];
> kadm5_ret_t (*pwd_qlty_init)(kadm5_server_handle_t);
> void (*pwd_qlty_cleanup)();
>  kadm5_ret_t (*pwd_qlty_check)(kadm5_server_handle_t, char*,int,
> kadm5_policy_ent_t, krb5_principal);
> } plugin_pwd_qlty;
>
> Then the key may be "pwd_qlty_check"

I think this is still ridiculously complicated.

It'd all be simpler with a dlsym() over v-table approach, but even with
a v-table approach it'd be simpler to have a function like this:

kadm5_ret_t
pwd_qlty_check(kadm5_server_handle_t h, char *pw, int what_is_this_arg?,
        kadm5_policy_ent_t pol, krb5_principal princ)
{
        int i;
        kadm5_ret_t ret = <default_return_value>;

        if (pwd_qlty_plugin_count == -1) {
                LOAD_PWD_QLTY_PLUGINS();
        }

        if (pwd_qlty_plugin_count == 0)
                return (ret);

        for (i = 0; i < pwd_qlty_plugin_count; i++) {
                if (pwd_qlty_plugins[i].version == 1) {
                        ret = pwd_qlty_plugins[i].vtable->pwd_qlty_check(h,
                            what_is_this_arg?, pol, princ);
                } else if pwd_qlty_plugins[i].version == 2) {
                        ...
                } else ... {
                        ...
                }
                if (ret != <OK>)
                        return (ret);
        }

        return(ret);
}

That's it.  Initially there will be a single version, of course, so the
actual code would be a bit simpler.

Also, if you need to get a per-plugin handle, you could, and it'd not be
much more complicated:


kadm5_ret_t
pwd_qlty_check(kadm5_server_handle_t h, char *pw, int what_is_this_arg?,
        kadm5_policy_ent_t pol, krb5_principal princ)
{
        int i;
        kadm5_ret_t ret = <default_return_value>;

        if (pwd_qlty_plugin_count == -1) {
                LOAD_PWD_QLTY_PLUGINS();
        }

        if (pwd_qlty_plugin_count == 0)
                return (ret);

        for (i = 0; i < pwd_qlty_plugin_count; i++) {
                if (pwd_qlty_plugins[i].version == 1) {
                        if (pwd_qlty_plugins[i].handle == NULL) {
                                pwd_qlty_plugins[i].handle =
                                    pwd_qlty_plugins[i].vtable->pwd_qlty_init(h);
                        }
                        ret = pwd_qlty_plugins[i].vtable->pwd_qlty_check(h,
                            what_is_this_arg?, pol, princ);
                } else if pwd_qlty_plugins[i].version == 2) {
                        ...
                } else ... {
                        ...
                }
                if (ret != <OK>)
                        return (ret);
        }

        return(ret);
}

Or you could move the pwd_qlty_init thing into LOAD_PWD_QLTY_PLUGINS().

Either way what you're left with is pretty simple.

Nico
--