pkinit plugin logic in pkinit_srv.c

[Craig Huckabee]

> On Aug 24, 2017, at 11:02 AM, Greg Hudson <ghudson at mit.edu> wrote:
> 
> On 08/24/2017 09:39 AM, Craig Huckabee wrote:
>> While running some tests with the latest development builds, I noticed that the plugin test logic in pkinit_srv.c might be flawed.  The comment in the plugin check codes says:
>> 
>>     /*
>>     * Check the certificate against each certauth module.  For the certificate
>>     * to be authorized at least one module must return 0, and no module can an
>>     * error code other than KRB5_PLUGIN_NO_HANDLE (pass).  Add indicators from
>>     * modules that return 0 or pass.
>>     */
>> 
>> but that’s not really true as each plugin returns KRB5KDC_ERR_CLIENT_NAME_MISMATCH when a match is not found.  This means the first plugin that fails kicks out of that loop and no other checks are performed.  I noticed this specifically because we were testing with certs that need the dbmatch module to work but it was never being called.
> 
> Can you elaborate on "each plugin returns
> KRB5KDC_ERR_CLIENT_NAME_MISMATCH when a match is not found"?  Of the
> built-in modules, I think only the san module would ever return that
> code, and should only return it when it finds PKINIT or UPN SANs in the
> certificate and they don't match.  It should return
> KRB5_PLUGIN_NO_HANDLE if the cert doesn't have either of those SAN types.
> 

Right - the san module returns it, but the dbmatch module also returns it, and the eku check returns KRB5KDC_ERR_INCONSISTENT_KEY_PURPOSE.

In my use case, I’m working with a DoD CAC so I have a SAN that looks like this "1234567890 at mil” - so to work around that I wanted to use the dbmatch feature to add the relation between that SAN and my actual principal.  

But when the san module fails out with KRB5KDC_ERR_CLIENT_NAME_MISMATCH, the rest of the possible plugins are not called so I’ll never reach that dbmatch.
Anything other than KRB5_PLUGIN_NO_HANDLE kicks it out of the loop.  I assume the intent is to process all possible plugins until a match is found so that check needs to be adjusted or maybe just removed.

Attempting to attach the patch a second time - this probably isn’t the “right” fix but helps illustrate what makes it work.


—Craig


diff --git a/src/plugins/preauth/pkinit/pkinit_srv.c b/src/plugins/preauth/pkinit/pkinit_srv.c
index 5da8892..303b49d 100644
--- a/src/plugins/preauth/pkinit/pkinit_srv.c
+++ b/src/plugins/preauth/pkinit/pkinit_srv.c
@@ -370,7 +370,7 @@ authorize_cert(krb5_context context, certauth_handle *certauth_modules,
                               &opts, db_ent, &ais);
         if (ret == 0)
             accepted = TRUE;
-        else if (ret != KRB5_PLUGIN_NO_HANDLE)
+        else if (ret != KRB5_PLUGIN_NO_HANDLE && ret != KRB5KDC_ERR_CLIENT_NAME_MISMATCH)
             goto cleanup;
 
         if (ais != NULL) {
@@ -383,6 +383,9 @@ authorize_cert(krb5_context context, certauth_handle *certauth_modules,
             h->vt.free_ind(context, h->moddata, ais);
             ais = NULL;
         }
+	if (accepted) {
+	    break;
+	}		
     }
 
     ret = accepted ? 0 : KRB5KDC_ERR_CLIENT_NAME_MISMATCH;





-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 1753 bytes
Desc: not available
Url : http://mailman.mit.edu/pipermail/krbdev/attachments/20170824/f5d2bd5d/attachment.bin