# HG changeset patch
# User weijun
# Date 1436780674 -28800
# Node ID abad00f2c0276e0ba494be8498a586b25f41086e
# Parent  7ae76e376fcde69be20e70ad966c179420280103
8058290: JAAS Krb5LoginModule has suspect ticket-renewal logic, relies on clockskew grace
Reviewed-by: mullan

diff -r 7ae76e376fcd -r abad00f2c027 jdk/src/jdk.security.auth/share/classes/com/sun/security/auth/module/Krb5LoginModule.java
--- a/jdk/src/jdk.security.auth/share/classes/com/sun/security/auth/module/Krb5LoginModule.java	Fri Jul 10 16:40:12 2015 +0100
+++ b/jdk/src/jdk.security.auth/share/classes/com/sun/security/auth/module/Krb5LoginModule.java	Mon Jul 13 17:44:34 2015 +0800
@@ -122,8 +122,9 @@
  * must also be set to true; Otherwise a configuration error will
  * be returned.</dd>
  * <dt>{@code renewTGT}:</dt>
- * <dd>Set this to true, if you want to renew
- * the TGT. If this is set, {@code useTicketCache} must also be
+ * <dd>Set this to true, if you want to renew the TGT when it's more than
+ * half-way expired (the time until expiration is less than the time
+ * since start time). If this is set, {@code useTicketCache} must also be
  * set to true; otherwise a configuration error will be returned.</dd>
  * <dt>{@code doNotPrompt}:</dt>
  * <dd>Set this to true if you do not want to be
@@ -649,17 +650,19 @@
                     (principal, ticketCacheName);
 
                 if (cred != null) {
-                    // check to renew credentials
+                    if (renewTGT && isOld(cred)) {
+                        // renew if ticket is old.
+                        Credentials newCred = renewCredentials(cred);
+                        if (newCred != null) {
+                            cred = newCred;
+                        }
+                    }
                     if (!isCurrent(cred)) {
-                        if (renewTGT) {
-                            cred = renewCredentials(cred);
-                        } else {
-                            // credentials have expired
-                            cred = null;
-                            if (debug)
-                                System.out.println("Credentials are" +
-                                                " no longer valid");
-                        }
+                        // credentials have expired
+                        cred = null;
+                        if (debug)
+                            System.out.println("Credentials are" +
+                                    " no longer valid");
                     }
                 }
 
@@ -968,7 +971,7 @@
         }
     }
 
-    private boolean isCurrent(Credentials creds)
+    private static boolean isCurrent(Credentials creds)
     {
         Date endTime = creds.getEndTime();
         if (endTime != null) {
@@ -977,6 +980,23 @@
         return true;
     }
 
+    private static boolean isOld(Credentials creds)
+    {
+        Date endTime = creds.getEndTime();
+        if (endTime != null) {
+            Date authTime = creds.getAuthTime();
+            long now = System.currentTimeMillis();
+            if (authTime != null) {
+                // pass the mid between auth and end
+                return now - authTime.getTime() > endTime.getTime() - now;
+            } else {
+                // will expire in less than 2 hours
+                return now <= endTime.getTime() - 1000*3600*2L;
+            }
+        }
+        return false;
+    }
+
     private Credentials renewCredentials(Credentials creds)
     {
         Credentials lcreds;
diff -r 7ae76e376fcd -r abad00f2c027 jdk/test/sun/security/krb5/auto/KDC.java
--- a/jdk/test/sun/security/krb5/auto/KDC.java	Fri Jul 10 16:40:12 2015 +0100
+++ b/jdk/test/sun/security/krb5/auto/KDC.java	Mon Jul 13 17:44:34 2015 +0800
@@ -811,7 +811,7 @@
                     new TransitedEncoding(1, new byte[0]),  // TODO
                     new KerberosTime(new Date()),
                     body.from,
-                    till, body.rtime,
+                    till, etp.renewTill,
                     body.addresses != null ? body.addresses
                             : etp.caddr,
                     null);
@@ -834,7 +834,7 @@
                     tFlags,
                     new KerberosTime(new Date()),
                     body.from,
-                    till, body.rtime,
+                    till, etp.renewTill,
                     service,
                     body.addresses
                     );
diff -r 7ae76e376fcd -r abad00f2c027 jdk/test/sun/security/krb5/auto/Renew.java
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/sun/security/krb5/auto/Renew.java	Mon Jul 13 17:44:34 2015 +0800
@@ -0,0 +1,106 @@
+/*
+ * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8058290
+ * @summary JAAS Krb5LoginModule has suspect ticket-renewal logic,
+ *          relies on clockskew grace
+ * @modules java.base/sun.net.spi.nameservice
+ *          java.base/sun.security.util
+ *          java.security.jgss/sun.security.krb5
+ *          java.security.jgss/sun.security.krb5.internal
+ *          java.security.jgss/sun.security.krb5.internal.ccache
+ *          java.security.jgss/sun.security.krb5.internal.crypto
+ *          java.security.jgss/sun.security.krb5.internal.ktab
+ * @compile -XDignore.symbol.file Renew.java
+ * @run main/othervm Renew 1
+ * @run main/othervm Renew 2
+ * @run main/othervm Renew 3
+ */
+
+import sun.security.krb5.Config;
+
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.Date;
+import javax.security.auth.kerberos.KerberosTicket;
+
+public class Renew {
+
+    public static void main(String[] args) throws Exception {
+
+        // Three test cases:
+        // 1. renewTGT=false
+        // 2. renewTGT=true with a short life time, renew will happen
+        // 3. renewTGT=true with a long life time, renew won't happen
+        int test = Integer.parseInt(args[0]);
+
+        OneKDC k = new OneKDC(null);
+        KDC.saveConfig(OneKDC.KRB5_CONF, k,
+                "renew_lifetime = 1d",
+                "ticket_lifetime = " + (test == 2? "10s": "8h"));
+        Config.refresh();
+        k.writeJAASConf();
+
+        // KDC would save ccache in a file
+        System.setProperty("test.kdc.save.ccache", "cache.here");
+
+        Files.write(Paths.get(OneKDC.JAAS_CONF), Arrays.asList(
+                "first {",
+                "   com.sun.security.auth.module.Krb5LoginModule required;",
+                "};",
+                "second {",
+                "   com.sun.security.auth.module.Krb5LoginModule required",
+                "   doNotPrompt=true",
+                "   renewTGT=" + (test != 1),
+                "   useTicketCache=true",
+                "   ticketCache=cache.here;",
+                "};"
+        ));
+
+        Context c;
+
+        // The first login uses username and password
+        c = Context.fromUserPass(OneKDC.USER, OneKDC.PASS, false);
+        Date d1 = c.s().getPrivateCredentials(KerberosTicket.class).iterator().next().getAuthTime();
+
+        // 6s is longer than half of 10s
+        Thread.sleep(6000);
+
+        // The second login uses the cache
+        c = Context.fromJAAS("second");
+        Date d2 = c.s().getPrivateCredentials(KerberosTicket.class).iterator().next().getAuthTime();
+
+        if (test == 2) {
+            if (d1.equals(d2)) {
+                throw new Exception("Ticket not renewed");
+            }
+        } else {
+            if (!d1.equals(d2)) {
+                throw new Exception("Ticket renewed");
+            }
+        }
+    }
+}