8186576: KerberosTicket does not properly handle renewable tickets at the end of their lifetime

Reviewed-by: mbalao

5 files changed, 98 insertions, 10 deletions

diff --git a/src/share/classes/com/sun/security/auth/module/Krb5LoginModule.java b/src/share/classes/com/sun/security/auth/module/Krb5LoginModule.java
index f0142cdffd..448764cc9f 100644
--- a/src/share/classes/com/sun/security/auth/module/Krb5LoginModule.java
+++ b/src/share/classes/com/sun/security/auth/module/Krb5LoginModule.java
@@ -1021,6 +1021,10 @@ public class Krb5LoginModule implements LoginModule {
             if (!creds.isRenewable())
                 throw new RefreshFailedException("This ticket" +
                                 " is not renewable");
+            if (creds.getRenewTill() == null) {
+                // Renewable ticket without renew-till. Illegal and ignored.
+                return creds;
+            }
             if (System.currentTimeMillis() > cred.getRenewTill().getTime())
                 throw new RefreshFailedException("This ticket is past "
                                              + "its last renewal time.");
diff --git a/src/share/classes/javax/security/auth/kerberos/KerberosTicket.java b/src/share/classes/javax/security/auth/kerberos/KerberosTicket.java
index 0327f04db5..dab1f038ac 100644
--- a/src/share/classes/javax/security/auth/kerberos/KerberosTicket.java
+++ b/src/share/classes/javax/security/auth/kerberos/KerberosTicket.java
@@ -308,11 +308,7 @@ public class KerberosTicket implements Destroyable, Refreshable,
         } else
            this.flags = new boolean[NUM_FLAGS];
 
-        if (this.flags[RENEWABLE_TICKET_FLAG]) {
-           if (renewTill == null)
-                throw new IllegalArgumentException("The renewable period "
-                       + "end time cannot be null for renewable tickets.");
-
+        if (this.flags[RENEWABLE_TICKET_FLAG] && renewTill != null) {
            this.renewTill = new Date(renewTill.getTime());
         }
 
@@ -553,6 +549,11 @@ public class KerberosTicket implements Destroyable, Refreshable,
         if (!isRenewable())
             throw new RefreshFailedException("This ticket is not renewable");
 
+        if (getRenewTill() == null) {
+            // Renewable ticket without renew-till. Illegal and ignored.
+            return;
+        }
+
         if (System.currentTimeMillis() > getRenewTill().getTime())
             throw new RefreshFailedException("This ticket is past "
                                              + "its last renewal time.");
diff --git a/src/share/classes/sun/security/krb5/KrbTgsReq.java b/src/share/classes/sun/security/krb5/KrbTgsReq.java
index c924da6967..6207a26ff3 100644
--- a/src/share/classes/sun/security/krb5/KrbTgsReq.java
+++ b/src/share/classes/sun/security/krb5/KrbTgsReq.java
@@ -35,6 +35,7 @@ import sun.security.krb5.internal.*;
 import sun.security.krb5.internal.crypto.*;
 import java.io.IOException;
 import java.net.UnknownHostException;
+import java.time.Instant;
 import java.util.Arrays;
 
 /**
@@ -250,7 +251,12 @@ public class KrbTgsReq {
         throws IOException, KrbException, UnknownHostException {
         KerberosTime req_till = null;
         if (till == null) {
-            req_till = new KerberosTime(0);
+            String d = Config.getInstance().get("libdefaults", "ticket_lifetime");
+            if (d != null) {
+                req_till = new KerberosTime(Instant.now().plusSeconds(Config.duration(d)));
+            } else {
+                req_till = new KerberosTime(0); // Choose KDC maximum allowed
+            }
         } else {
             req_till = till;
         }
diff --git a/test/sun/security/krb5/auto/KDC.java b/test/sun/security/krb5/auto/KDC.java
index a2219d755e..243abe66ac 100644
--- a/test/sun/security/krb5/auto/KDC.java
+++ b/test/sun/security/krb5/auto/KDC.java
@@ -30,8 +30,6 @@ import java.lang.reflect.Method;
 import java.security.SecureRandom;
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
-import java.time.temporal.TemporalAmount;
-import java.time.temporal.TemporalUnit;
 import java.util.*;
 import java.util.concurrent.*;
 
@@ -912,6 +910,18 @@ public class KDC {
             }
             bFlags[Krb5.TKT_OPTS_INITIAL] = true;
 
+            KerberosTime renewTill = etp.renewTill;
+            if (renewTill != null && body.kdcOptions.get(KDCOptions.RENEW)) {
+                // till should never pass renewTill
+                if (till.greaterThan(renewTill)) {
+                    till = renewTill;
+                }
+                if (System.getProperty("test.set.null.renew") != null) {
+                    // Testing 8186576, see NullRenewUntil.java.
+                    renewTill = null;
+                }
+            }
+
             TicketFlags tFlags = new TicketFlags(bFlags);
             EncTicketPart enc = new EncTicketPart(
                     tFlags,
@@ -920,7 +930,7 @@ public class KDC {
                     new TransitedEncoding(1, new byte[0]),  // TODO
                     new KerberosTime(new Date()),
                     body.from,
-                    till, etp.renewTill,
+                    till, renewTill,
                     body.addresses != null  // always set caddr
                             ? body.addresses
                             : new HostAddresses(
@@ -947,7 +957,7 @@ public class KDC {
                     tFlags,
                     new KerberosTime(new Date()),
                     body.from,
-                    till, etp.renewTill,
+                    till, renewTill,
                     service,
                     body.addresses != null  // always set caddr
                             ? body.addresses
diff --git a/test/sun/security/krb5/auto/NullRenewUntil.java b/test/sun/security/krb5/auto/NullRenewUntil.java
new file mode 100644
index 0000000000..2f7d4a4efb
--- /dev/null
+++ b/test/sun/security/krb5/auto/NullRenewUntil.java
@@ -0,0 +1,67 @@
+/*
+ * Copyright (c) 2017, 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 8186576
+ * @summary KerberosTicket does not properly handle renewable tickets
+ *          at the end of their lifetime
+ * @library /lib/testlibrary/
+ * @compile -XDignore.symbol.file NullRenewUntil.java
+ * @run main/othervm -Dsun.net.spi.nameservice.provider.1=ns,mock -Dtest.set.null.renew NullRenewUntil
+ */
+
+import jdk.testlibrary.Asserts;
+import sun.security.krb5.Config;
+
+import javax.security.auth.kerberos.KerberosTicket;
+
+public class NullRenewUntil {
+
+    public static void main(String[] args) throws Exception {
+
+        OneKDC kdc = new OneKDC(null);
+
+        KDC.saveConfig(OneKDC.KRB5_CONF, kdc,
+                "ticket_lifetime = 10s",
+                "renew_lifetime = 11s");
+        Config.refresh();
+
+        KerberosTicket ticket = Context
+                .fromUserPass(OneKDC.USER, OneKDC.PASS, false).s()
+                .getPrivateCredentials(KerberosTicket.class).iterator().next();
+
+        System.out.println(ticket);
+        Asserts.assertTrue(ticket.getRenewTill() != null, ticket.toString());
+
+        Thread.sleep(2000);
+
+        ticket.refresh();
+        System.out.println(ticket);
+        Asserts.assertTrue(ticket.getRenewTill() == null, ticket.toString());
+
+        Thread.sleep(2000);
+        ticket.refresh();
+        System.out.println(ticket);
+    }
+}