svn rev #24647: branches/krb5-1-9/src/ include/ lib/krb5/krb/

[tlyu@MIT.EDU]

http://src.mit.edu/fisheye/changelog/krb5/?cs=24647
Commit By: tlyu
Log Message:
ticket: 6870
version_fixed: 1.9.1
status: resolved

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.


Changed Files:
U   branches/krb5-1-9/src/include/k5-trace.h
U   branches/krb5-1-9/src/lib/krb5/krb/pac.c
Modified: branches/krb5-1-9/src/include/k5-trace.h
===================================================================
--- branches/krb5-1-9/src/include/k5-trace.h	2011-02-22 21:06:23 UTC (rev 24646)
+++ branches/krb5-1-9/src/include/k5-trace.h	2011-02-22 22:17:26 UTC (rev 24647)
@@ -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"))
 

Modified: branches/krb5-1-9/src/lib/krb5/krb/pac.c
===================================================================
--- branches/krb5-1-9/src/lib/krb5/krb/pac.c	2011-02-22 21:06:23 UTC (rev 24646)
+++ branches/krb5-1-9/src/lib/krb5/krb/pac.c	2011-02-22 22:17:26 UTC (rev 24647)
@@ -637,17 +637,8 @@
         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;
+    if (ret != 0)
         return ret;
-    } else if (ret != 0) {
-        return ret;
-    }
 
     if (privsvr != NULL) {
         ret = k5_pac_verify_kdc_checksum(context, pac, privsvr);
@@ -804,35 +795,20 @@
     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);
+                           req->ticket->enc_part2->client, key, NULL);
+    if (code != 0)
+        TRACE_MSPAC_VERIFY_FAIL(kcontext, code);
 
     /*
-     * 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 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 == ENOENT && !pacctx->pac->verified) {
-        code = 0;
-    }
-
-#if 0
-    /*
-     * Now, we could return 0 and just set pac->verified to FALSE.
-     * Thoughts?
-     */
-    if (code == KRB5KRB_AP_ERR_BAD_INTEGRITY) {
-        assert(pacctx->pac->verified == FALSE);
-        code = 0;
-    }
-#endif
-
-    return code;
+    return 0;
 }
 
 static void