Pasword quality pluggable interface project review

[Marcus Watts]

Greg Hudson sent:
> Date:    Sun, 29 Aug 2010 12:16:59 EDT
> To:      ghudson at MIT.EDU
> cc:      krbdev at MIT.EDU
> From:    ghudson at MIT.EDU
> Subject: Re: Pasword quality pluggable interface project review
> 
> I uncovered one subtle issue during implementation: if a module's
> check method decides it doesn't like a new password, what error code
> should it return?
> 
> There are three error codes in kadm_err.et: KADM5_PASS_Q_TOOSHORT,
> KADM5_PASS_Q_CLASS, and KADM5_PASS_Q_DICT.  Those error codes are
> treated specially by kadmind's process_chpw_request().  But if plugins
> are not tracking <kadm5/admin.h>, they won't have access to those
> error codes.
> 
> krb5-strength sidesteps this issue by tying into find_word(), so that
> all module errors are converted to KADM5_PASS_Q_DICT in libkadm5srv.
> 
> I can think of a few options which don't reintroduce <kadm5/admin.h>
> into the API:
> 
>   * Add a boolean result argument to the check method, so that a
>     failing password is not an error from the perspective of the
>     pluggable interface.
> 
>   * Add a string result argument to the check method (to be set to
>     NULL if the password passes quality checks), in the hopes that a
>     module-generated explanation could be conveyed to the user.  No
>     idea how this would ever be localized, though.  Also, the password
>     change protocol doesn't appear to have a way to communicate such
>     errors (looking at our implementation, anyway), so such strings
>     would only show up in the kadmind log.
> 
>   * Create a new error in the krb5 table (or actually, the k5e1
>     expansion table) for unspecified password quality failures, and
>     treat that error code specially in process_chpw_request().
...

1. error codes and mechanism
2. why load a plugin more than once?
3. my modified version of rra's mechanism

____ 1. error codes and mechanism

Modern versions of k5 install kadm5/admin.h.  I don't see a problem with
including it.  It would be odd that something that's going to be running
inside kadmind's process space can't see/use the public api for kadmind.
Being able to return the exact error code desired, IMHO, is the right
thing to do.
[	In my modified copy of Russ's code, I allow the plugin to
	directly return the code of its choice, and I map integer value 1
	to KADM5_PASS_Q_DICT to cover plugins that don't want to include
	kadm5/admin.h (in 1.6.3, this was an issue.)

	For the implementation I did of this, I have another header file,
	<krb5/pwcheck_plugin.h>, to describe the things that only the password
	quality check module needs to know.	 ]

I agree the existing error codes are pretty narrowly scoped.  I'm not
sure how I'd add more to be useful, given the error codes should be
remotely visible.

You alluded to behavior of KRB5_KPASSWD_HARDERROR
vs. KRB5_KPASSWD_SOFTERROR in process_chpw_request.  If that's important,
then perhaps the plugin interface should return that as a separate flag.
For what i can see, this only affects the error message ("Server error"
vs.  "Password change rejected") - while quality messages are important,
this particular mechanism might not be the best way.

____ 2. why load a plugin more than once?

The inspiration for me is looking at what people do with Pam.  The
original pam designers at sun put together a relatively simple framework,
and probably expected people who might customize this would write their
own "kitchen sink" plugin.  People using the linux pam framework stretch
this a good deal.  First there are many more plugins that do things.
Then secondly there are enhancements to the control flow logic because
"sufficient" and "requisite" just isn't enough.

So, to take an example.  Suppose we have a plugin that can check a
dictionary.  And suppose we have an environment where we have "admins"
who should have extra strong passwords, and "friends" who can't type
but don't need strong passwords.  If I have a plugin that allows me to
select the dictionary based on policy then I can load it twice:
	dictionary-check policy=admins dictionary=big-bad-dictionary
	dictionary-check policy=friends dictionary=wimpy-little-dictionary
If I have to load it only once, then the dictionary check module itself
has to have its own logic to accomplish the same result, and
any other module where this might be useful also has to have similar logic:
	dictionary-check admins -> big-bad-dictionary friends -> wimpy-little-dictionary
I think that's not as nice, in part because people are less likely
to think how to make their plugins that flexible.

For control flow, I was thinking that it should be possible to
write "meta" plugins that could run other plugins in turn,
then do "or", "and" or other operations as desired.

____ 3. my modified version of rra's mechanism

This may not be of interest to you, but in case it's helpful, here's
source to the plugin patch I devised from what Russ had,
/afs/umich.edu/group/itd/build/mdw/krb5.15x/patches/krb5-1.6.3-pwcheck2.patch
which includes 2 sample plugins, "hesiod" and "exec".

The "exec" plugin is intended to provide the functionality of
USE_PASSWORD_SERVER which invokes /usr/sbin/mkpassdb on password changes.
"exec" is another example of a plugin one might choose to load more
than once.

I also grabbed some of the mail I exchanged with Russ on this, which
should suffice to explain the main features of this patch,
/afs/umich.edu/user/m/d/mdw/wp/k5.77
(obviously it could stand considerably more editting.)
This talks about "password synchronizing" - and I think this was
a requirement at one time of somebody (not me!) - but neither Russ
nor I think this is the right place to do this today.
That means neither of us really like USE_PASSWORD_SERVER.

					-Marcus Watts