Issues with multiple pkinit KDC certauth plugins

[Ken Hornstein]

So as I have discussed ad nauseum, I have been working on replacing our
old busted fork of the MIT PKINIT plugin with stock MIT code.  I was
hoping to do this for our KDC upgrade based on MIT Kerberos 1.19.1
by the use of the certauth plugin interface.  MIT was kind enough to add
a capability to set the HWAUTH ticket flag from this interface, which
is a capability we needed.  But in doing this work I ran into an issue
that I am not sure how to solve.

First, let me re-state the details of the certauth interface for those
people not familiar with it, and if I am wrong then please someone correct
me.  The certauth plugin is called by the PKINIT plugin to see if a
certificate is authorized to be used to authenticate to a certain
principal.  The certauth "authorize" function can return 0 (success),
one of the listed error codes (failure), KRB5_PLUGIN_NO_HANDLE (pass),
or KRB5_CERTAUTH_HWAUTH (success + set hwauth bit).

There are 3 internal plugins: pkinit_san, pkinit_eku, and dbmatch.
pkinit_san is checking for a principal name in the SAN in the certificate.
That one doesn't apply to us, since in our PKI certificates never have
principal names in them.  pkinit_eku checks to see if the client certificate
has the correct extended key usage; if that succeeds, it returns
KRB5_PLUGIN_NO_HANDLE.

dbmatch checks to see if the principal is tagged with a string containing
a certificate matching rule; if the rule matches the client certificate,
then it returns 0.  If the rule does NOT match, then it fails.  But
if there is no string associated with the principal, it returns
KRB5_PLUGIN_NO_HANDLE (and this is important).  We use this to map
certificates to principals.

So we use in our Kerberos implementation the HW_AUTH bit in the ticket
to indicate that you have used a hardware-based token with PKINIT to
get your ticket to make authorization decisions (eventually, we're
going to migrate to authentication indicators, but we're kind of in
a chicken and egg problem there, so we still need the HW_AUTH bit in
the ticket).  Like I said previously, MIT was kind enough to add the
KRB5_CERTAUTH_HWAUTH return code.  But I realized today that this
presents a problem for us.

Our deployment is weird in that not everyone is necessarily assigned
a smartcard; those people use another token that does NOT use PKI and
uses the SAM-2 preauth mechanism.  So we have a number of principals in
our database that would not necessarily have database match rules.

My plan was to write a certauth module which I tentatively would call
'hwauth' that would check to see if the client certificate had a special
OID that indicated that it was on a smartcard and if it was it would
return KRB5_CERAUTH_HWAUTH.

The problem we would run into would be this:

- Someone not associated with us could hold a valid smartcard in our
  PKI (there are literally millions of active issued smartcards in this
  PKIs, so this is very easy).

- This someone could run:

  kinit non-pkinit-user at OUR.REALM

- Because 'non-pkinit-user' does not have a match rule associated with it,
  the dbmatch plugin would return KRB5_PLUGIN_NO_HANDLE.  But when the
  hwauth plugin would return KRB5_CERTAUTH_HWAUTH, that counts as
  'success' ... and would authorize this random smartcard holder to
  get a TGT for this principal.

So, yikes, not good.  And of course I didn't really think this situation
through when I asked for KRB5_CERTAUTH_HWAUTH, so this one is kind of
on me.  Dang it.  At least I discovered this before I deployed anything.

It occurs to me that the simplest solution here would be to
add an additional return code that meant "pass + add hwauth to
ticket".  Like it could be called KRB5_CERTAUTH_HWAUTH_PASS.  Or
KRB5_CERTAUTH_HWAUTH_NO_HANDLE, or something else.  Really, I don't care
about the name so much.  That seems like a small amount of code that
wouldn't result in an API change.  Other things like adding a configuration
variable that would cause dbmatch to return a hard failure upon a
missing dbmatch rule could possibly be an option but that seems
like it might be more complicated than warranted.  I'm open to
other suggestions.  If we're fine with a new return code I'd gladly
work up a pull request for that.

--Ken