Don't reject AP-REQs based on PACs

Experience has shown that it was a mistake to fail AP-REQ verification
based on failure to verify the signature of PAC authdata contained in
the ticket.  We've had two rounds of interoperability issues with the
hmac-md5 checksum code, an interoperability issue OSX generating
unsigned PACs, and another problem where PACs are copied by older KDCs
from a cross-realm TGT into the service ticket.  If a PAC signature
cannot be verified, just don't mark it as verified and continue on
with the AP exchange.

ticket: 6870
target_version: 1.9.1
tags: pullup

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@24640 dc483132-0cff-0310-8789-dd5450dbe970

2 files changed, 13 insertions, 38 deletions

diff --git a/src/include/k5-trace.h b/src/include/k5-trace.h
index 0df77fc19..96a6acf5f 100644
--- a/src/include/k5-trace.h
+++ b/src/include/k5-trace.h
@@ -197,9 +197,8 @@
     TRACE(c, (c, "Negotiating for enctypes in authenticator: {etypes}", \
               etypes))
 
-#define TRACE_MSPAC_NOSRVCKSUM(c) \
-    TRACE(c, (c, "MS PAC lacks a server checksum.  "\
-              "Apple Open Directory bug?"))
+#define TRACE_MSPAC_VERIFY_FAIL(c, err) \
+    TRACE(c, (c, "PAC checksum verification failed: {kerr}", err))
 #define TRACE_MSPAC_DISCARD_UNVERF(c) \
     TRACE(c, (c, "Filtering out unverified MS PAC"))
 
diff --git a/src/lib/krb5/krb/pac.c b/src/lib/krb5/krb/pac.c
index 47b3a3b91..067bb17c6 100644
--- a/src/lib/krb5/krb/pac.c
+++ b/src/lib/krb5/krb/pac.c
@@ -637,17 +637,8 @@ krb5_pac_verify(krb5_context context,
         return EINVAL;
 
     ret = k5_pac_verify_server_checksum(context, pac, server);
-    if (ret == ENOENT) {
-        /*
-         * Apple Mac OS X Server Open Directory KDC (at least 10.6)
-         * appears to provide a PAC that lacks a server checksum.
-         */
-        TRACE_MSPAC_NOSRVCKSUM(context);
-        pac->verified = FALSE;
-        return ret;
-    } else if (ret != 0) {
+    if (ret != 0)
         return ret;
-    }
 
     if (privsvr != NULL) {
         ret = k5_pac_verify_kdc_checksum(context, pac, privsvr);
@@ -804,35 +795,20 @@ mspac_verify(krb5_context kcontext,
     if (pacctx->pac == NULL)
         return EINVAL;
 
-    code = krb5_pac_verify(kcontext,
-                           pacctx->pac,
+    code = krb5_pac_verify(kcontext, pacctx->pac,
                            req->ticket->enc_part2->times.authtime,
-                           req->ticket->enc_part2->client,
-                           key,
-                           NULL);
-
-    /*
-     * If the server checksum is not found, return success to
-     * krb5int_authdata_verify() to work around an apparent Open
-     * Directory bug.  Non-verified PACs won't be returned by
-     * mspac_get_attribute().
-     */
-    if (code == ENOENT && !pacctx->pac->verified) {
-        code = 0;
-    }
+                           req->ticket->enc_part2->client, key, NULL);
+    if (code != 0)
+        TRACE_MSPAC_VERIFY_FAIL(kcontext, code);
 
-#if 0
     /*
-     * Now, we could return 0 and just set pac->verified to FALSE.
-     * Thoughts?
+     * If the above verification failed, don't fail the whole authentication,
+     * just don't mark the PAC as verified.  A checksum mismatch can occur if
+     * the PAC was copied from a cross-realm TGT by an ignorant KDC, and Apple
+     * Mac OS X Server Open Directory (as of 10.6) generates PACs with no
+     * server checksum at all.
      */
-    if (code == KRB5KRB_AP_ERR_BAD_INTEGRITY) {
-        assert(pacctx->pac->verified == FALSE);
-        code = 0;
-    }
-#endif
-
-    return code;
+    return 0;
 }
 
 static void