From: Tom Yu <tlyu@mit.edu>
Date: Tue, 22 Feb 2011 22:17:26 +0000 (+0000)
Subject: pull up r24640 from trunk
X-Git-Tag: krb5-1.9.1-beta1~7
X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=028125b8c2ba963cb28710de68706b2bb14c6c3b;p=krb5.git

pull up r24640 from trunk

 ------------------------------------------------------------------------
 r24640 | ghudson | 2011-02-16 18:34:37 -0500 (Wed, 16 Feb 2011) | 14 lines

 ticket: 6870
 subject: Don't reject AP-REQs based on PACs
 target_version: 1.9.1
 tags: pullup

 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
version_fixed: 1.9.1
status: resolved

git-svn-id: svn://anonsvn.mit.edu/krb5/branches/krb5-1-9@24647 dc483132-0cff-0310-8789-dd5450dbe970
---

diff --git a/src/include/k5-trace.h b/src/include/k5-trace.h
index 1bd4b4451..b85e59c7d 100644
--- a/src/include/k5-trace.h
+++ b/src/include/k5-trace.h
@@ -194,9 +194,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