From 549df33b5e6884a0fd14b140743a9c96ea3e8472 Mon Sep 17 00:00:00 2001 From: weijun Date: Wed, 17 Apr 2013 10:15:33 +0800 Subject: [PATCH] 8011124: Make KerberosTime immutable Reviewed-by: xuelei --- .../classes/sun/security/krb5/KrbApReq.java | 12 +- .../sun/security/krb5/KrbAppMessage.java | 14 +- .../classes/sun/security/krb5/KrbCred.java | 2 +- .../classes/sun/security/krb5/KrbTgsReq.java | 3 +- .../security/krb5/internal/KerberosTime.java | 202 ++++++------------ .../security/krb5/internal/KrbCredInfo.java | 12 +- .../security/krb5/internal/LastReqEntry.java | 2 +- .../security/krb5/internal/PAEncTSEnc.java | 2 +- .../krb5/internal/ccache/Credentials.java | 42 ++-- test/sun/security/krb5/MicroTime.java | 6 +- 10 files changed, 110 insertions(+), 187 deletions(-) diff --git a/src/share/classes/sun/security/krb5/KrbApReq.java b/src/share/classes/sun/security/krb5/KrbApReq.java index 535bb0dc5..2bc01f693 100644 --- a/src/share/classes/sun/security/krb5/KrbApReq.java +++ b/src/share/classes/sun/security/krb5/KrbApReq.java @@ -204,7 +204,7 @@ public class KrbApReq { int usage) throws KrbException, IOException { - ctime = new KerberosTime(KerberosTime.NOW); + ctime = KerberosTime.now(); init(options, tgs_creds.ticket, tgs_creds.key, @@ -287,14 +287,14 @@ public class KrbApReq { authenticator = new Authenticator(temp2); ctime = authenticator.ctime; cusec = authenticator.cusec; - authenticator.ctime.setMicroSeconds(authenticator.cusec); + authenticator.ctime = + authenticator.ctime.withMicroSeconds(authenticator.cusec); if (!authenticator.cname.equals(enc_ticketPart.cname)) { throw new KrbApErrException(Krb5.KRB_AP_ERR_BADMATCH); } - KerberosTime currTime = new KerberosTime(KerberosTime.NOW); - if (!authenticator.ctime.inClockSkew(currTime)) + if (!authenticator.ctime.inClockSkew()) throw new KrbApErrException(Krb5.KRB_AP_ERR_SKEW); // start to check if it is a replay attack. @@ -304,7 +304,7 @@ public class KrbApReq { if (table.get(time, authenticator.cname.toString()) != null) { throw new KrbApErrException(Krb5.KRB_AP_ERR_REPEAT); } else { - table.put(client, time, currTime.getTime()); + table.put(client, time, System.currentTimeMillis()); } if (initiator != null) { @@ -329,7 +329,7 @@ public class KrbApReq { // else // save authenticator to check for later - KerberosTime now = new KerberosTime(KerberosTime.NOW); + KerberosTime now = KerberosTime.now(); if ((enc_ticketPart.starttime != null && enc_ticketPart.starttime.greaterThanWRTClockSkew(now)) || diff --git a/src/share/classes/sun/security/krb5/KrbAppMessage.java b/src/share/classes/sun/security/krb5/KrbAppMessage.java index cf19cf982..47aa1681a 100644 --- a/src/share/classes/sun/security/krb5/KrbAppMessage.java +++ b/src/share/classes/sun/security/krb5/KrbAppMessage.java @@ -71,12 +71,18 @@ abstract class KrbAppMessage { } if (packetTimestamp != null) { - packetTimestamp.setMicroSeconds(packetUsec); - if (!packetTimestamp.inClockSkew()) + if (packetUsec != null) { + packetTimestamp = + packetTimestamp.withMicroSeconds(packetUsec.intValue()); + } + if (!packetTimestamp.inClockSkew()) { throw new KrbApErrException(Krb5.KRB_AP_ERR_SKEW); - } else - if (timestampRequired) + } + } else { + if (timestampRequired) { throw new KrbApErrException(Krb5.KRB_AP_ERR_SKEW); + } + } // XXX check replay cache // if (rcache.repeated(packetTimestamp, packetUsec, packetSAddress)) diff --git a/src/share/classes/sun/security/krb5/KrbCred.java b/src/share/classes/sun/security/krb5/KrbCred.java index 9a1318119..64dada1d3 100644 --- a/src/share/classes/sun/security/krb5/KrbCred.java +++ b/src/share/classes/sun/security/krb5/KrbCred.java @@ -103,7 +103,7 @@ public class KrbCred { delegatedCreds.renewTill, tgService, delegatedCreds.cAddr); - timeStamp = new KerberosTime(KerberosTime.NOW); + timeStamp = KerberosTime.now(); KrbCredInfo[] credInfos = {credInfo}; EncKrbCredPart encPart = new EncKrbCredPart(credInfos, diff --git a/src/share/classes/sun/security/krb5/KrbTgsReq.java b/src/share/classes/sun/security/krb5/KrbTgsReq.java index 9cd84c6d9..32263b537 100644 --- a/src/share/classes/sun/security/krb5/KrbTgsReq.java +++ b/src/share/classes/sun/security/krb5/KrbTgsReq.java @@ -147,8 +147,7 @@ public class KrbTgsReq { princName = cname; servName = sname; - ctime = new KerberosTime(KerberosTime.NOW); - + ctime = KerberosTime.now(); // check if they are valid arguments. The optional fields // should be consistent with settings in KDCOptions. diff --git a/src/share/classes/sun/security/krb5/internal/KerberosTime.java b/src/share/classes/sun/security/krb5/internal/KerberosTime.java index ce141419f..3beaac873 100644 --- a/src/share/classes/sun/security/krb5/internal/KerberosTime.java +++ b/src/share/classes/sun/security/krb5/internal/KerberosTime.java @@ -30,18 +30,20 @@ package sun.security.krb5.internal; -import java.util.TimeZone; -import sun.security.util.*; +import sun.security.krb5.Asn1Exception; import sun.security.krb5.Config; import sun.security.krb5.KrbException; -import sun.security.krb5.Asn1Exception; -import java.util.Date; -import java.util.GregorianCalendar; -import java.util.Calendar; +import sun.security.util.DerInputStream; +import sun.security.util.DerOutputStream; +import sun.security.util.DerValue; + import java.io.IOException; +import java.util.Calendar; +import java.util.Date; +import java.util.TimeZone; /** - * Implements the ASN.1 KerberosTime type. + * Implements the ASN.1 KerberosTime type. This is an immutable class. * * * KerberosTime ::= GeneralizedTime -- with no fractional seconds @@ -62,55 +64,38 @@ import java.io.IOException; * same class can be used as a precise timestamp in Authenticator etc. */ -public class KerberosTime implements Cloneable { +public class KerberosTime { - private long kerberosTime; // milliseconds since epoch, a Date.getTime() value - private int microSeconds; // the last three digits of the microsecond value + private final long kerberosTime; // milliseconds since epoch, Date.getTime() + private final int microSeconds; // last 3 digits of the real microsecond // The time when this class is loaded. Used in setNow() private static long initMilli = System.currentTimeMillis(); private static long initMicro = System.nanoTime() / 1000; - private static long syncTime; private static boolean DEBUG = Krb5.DEBUG; - public static final boolean NOW = true; - public static final boolean UNADJUSTED_NOW = false; - - public KerberosTime(long time) { - kerberosTime = time; - } - + // Do not make this public. It's a little confusing that micro + // is only the last 3 digits of microsecond. private KerberosTime(long time, int micro) { kerberosTime = time; microSeconds = micro; } - public Object clone() { - return new KerberosTime(kerberosTime, microSeconds); + /** + * Creates a KerberosTime object from milliseconds since epoch. + */ + public KerberosTime(long time) { + this(time, 0); } // This constructor is used in the native code // src/windows/native/sun/security/krb5/NativeCreds.c public KerberosTime(String time) throws Asn1Exception { - kerberosTime = toKerberosTime(time); - } - - /** - * Constructs a KerberosTime object. - * @param encoding a DER-encoded data. - * @exception Asn1Exception if an error occurs while decoding an ASN1 encoded data. - * @exception IOException if an I/O error occurs while reading encoded data. - */ - public KerberosTime(DerValue encoding) throws Asn1Exception, IOException { - GregorianCalendar calendar = new GregorianCalendar(); - Date temp = encoding.getGeneralizedTime(); - kerberosTime = temp.getTime(); + this(toKerberosTime(time), 0); } private static long toKerberosTime(String time) throws Asn1Exception { - // this method only used by KerberosTime class. - // ASN.1 GeneralizedTime format: // "19700101000000Z" @@ -133,30 +118,34 @@ public class KerberosTime implements Cloneable { Integer.parseInt(time.substring(8, 10)), Integer.parseInt(time.substring(10, 12)), Integer.parseInt(time.substring(12, 14))); - - //The Date constructor assumes the setting are local relative - //and converts the time to UTC before storing it. Since we - //want the internal representation to correspond to local - //and not UTC time we subtract the UTC time offset. - return (calendar.getTime().getTime()); - - } - - // should be moved to sun.security.krb5.util class - public static String zeroPad(String s, int length) { - StringBuffer temp = new StringBuffer(s); - while (temp.length() < length) - temp.insert(0, '0'); - return temp.toString(); + return calendar.getTimeInMillis(); } + /** + * Creates a KerberosTime object from a Date object. + */ public KerberosTime(Date time) { - kerberosTime = time.getTime(); // (time.getTimezoneOffset() * 60000L); + this(time.getTime(), 0); } - public KerberosTime(boolean initToNow) { - if (initToNow) { - setNow(); + /** + * Creates a KerberosTime object for now. It uses System.nanoTime() + * to get a more precise time than "new Date()". + */ + public static KerberosTime now() { + long newMilli = System.currentTimeMillis(); + long newMicro = System.nanoTime() / 1000; + long microElapsed = newMicro - initMicro; + long calcMilli = initMilli + microElapsed/1000; + if (calcMilli - newMilli > 100 || newMilli - calcMilli > 100) { + if (DEBUG) { + System.out.println("System time adjusted"); + } + initMilli = newMilli; + initMicro = newMicro; + return new KerberosTime(newMilli, 0); + } else { + return new KerberosTime(calcMilli, (int)(microElapsed % 1000)); } } @@ -169,13 +158,13 @@ public class KerberosTime implements Cloneable { calendar.clear(); calendar.setTimeInMillis(kerberosTime); - return zeroPad(Integer.toString(calendar.get(Calendar.YEAR)), 4) + - zeroPad(Integer.toString(calendar.get(Calendar.MONTH) + 1), 2) + - zeroPad(Integer.toString(calendar.get(Calendar.DAY_OF_MONTH)), 2) + - zeroPad(Integer.toString(calendar.get(Calendar.HOUR_OF_DAY)), 2) + - zeroPad(Integer.toString(calendar.get(Calendar.MINUTE)), 2) + - zeroPad(Integer.toString(calendar.get(Calendar.SECOND)), 2) + 'Z'; - + return String.format("%04d%02d%02d%02d%02d%02dZ", + calendar.get(Calendar.YEAR), + calendar.get(Calendar.MONTH) + 1, + calendar.get(Calendar.DAY_OF_MONTH), + calendar.get(Calendar.HOUR_OF_DAY), + calendar.get(Calendar.MINUTE), + calendar.get(Calendar.SECOND)); } /** @@ -194,40 +183,8 @@ public class KerberosTime implements Cloneable { return kerberosTime; } - - public void setTime(Date time) { - kerberosTime = time.getTime(); // (time.getTimezoneOffset() * 60000L); - microSeconds = 0; - } - - public void setTime(long time) { - kerberosTime = time; - microSeconds = 0; - } - public Date toDate() { - Date temp = new Date(kerberosTime); - temp.setTime(temp.getTime()); - return temp; - } - - public void setNow() { - long newMilli = System.currentTimeMillis(); - long newMicro = System.nanoTime() / 1000; - long microElapsed = newMicro - initMicro; - long calcMilli = initMilli + microElapsed/1000; - if (calcMilli - newMilli > 100 || newMilli - calcMilli > 100) { - if (DEBUG) { - System.out.println("System time adjusted"); - } - initMilli = newMilli; - initMicro = newMicro; - setTime(newMilli); - microSeconds = 0; - } else { - setTime(calcMilli); - microSeconds = (int)(microElapsed % 1000); - } + return new Date(kerberosTime); } public int getMicroSeconds() { @@ -235,45 +192,25 @@ public class KerberosTime implements Cloneable { return temp_long.intValue() + microSeconds; } - public void setMicroSeconds(int usec) { - microSeconds = usec % 1000; - Integer temp_int = new Integer(usec); - long temp_long = temp_int.longValue() / 1000L; - kerberosTime = kerberosTime - (kerberosTime % 1000L) + temp_long; + /** + * Returns a new KerberosTime object with the original seconds + * and the given microseconds. + */ + public KerberosTime withMicroSeconds(int usec) { + return new KerberosTime( + kerberosTime - kerberosTime%1000L + usec/1000L, + usec%1000); } - public void setMicroSeconds(Integer usec) { - if (usec != null) { - microSeconds = usec.intValue() % 1000; - long temp_long = usec.longValue() / 1000L; - kerberosTime = kerberosTime - (kerberosTime % 1000L) + temp_long; - } - } - - public boolean inClockSkew(int clockSkew) { - KerberosTime now = new KerberosTime(KerberosTime.NOW); - - if (java.lang.Math.abs(kerberosTime - now.kerberosTime) > - clockSkew * 1000L) - return false; - return true; + private boolean inClockSkew(int clockSkew) { + return java.lang.Math.abs(kerberosTime - System.currentTimeMillis()) + <= clockSkew * 1000L; } public boolean inClockSkew() { return inClockSkew(getDefaultSkew()); } - public boolean inClockSkew(int clockSkew, KerberosTime now) { - if (java.lang.Math.abs(kerberosTime - now.kerberosTime) > - clockSkew * 1000L) - return false; - return true; - } - - public boolean inClockSkew(KerberosTime time) { - return inClockSkew(getDefaultSkew(), time); - } - public boolean greaterThanWRTClockSkew(KerberosTime time, int clockSkew) { if ((kerberosTime - time.kerberosTime) > clockSkew * 1000L) return true; @@ -317,24 +254,22 @@ public class KerberosTime implements Cloneable { return temp_long.intValue(); } - public void setSeconds(int sec) { - Integer temp_int = new Integer(sec); - kerberosTime = temp_int.longValue() * 1000L; - } - /** * Parse (unmarshal) a kerberostime from a DER input stream. This form * parsing might be used when expanding a value which is part of * a constructed sequence and uses explicitly tagged type. * * @exception Asn1Exception on error. - * @param data the Der input stream value, which contains one or more marshaled value. + * @param data the Der input stream value, which contains + * one or more marshaled value. * @param explicitTag tag number. * @param optional indicates if this data field is optional * @return an instance of KerberosTime. * */ - public static KerberosTime parse(DerInputStream data, byte explicitTag, boolean optional) throws Asn1Exception, IOException { + public static KerberosTime parse( + DerInputStream data, byte explicitTag, boolean optional) + throws Asn1Exception, IOException { if ((optional) && (((byte)data.peekByte() & (byte)0x1F)!= explicitTag)) return null; DerValue der = data.getDerValue(); @@ -343,7 +278,8 @@ public class KerberosTime implements Cloneable { } else { DerValue subDer = der.getData().getDerValue(); - return new KerberosTime(subDer); + Date temp = subDer.getGeneralizedTime(); + return new KerberosTime(temp.getTime(), 0); } } diff --git a/src/share/classes/sun/security/krb5/internal/KrbCredInfo.java b/src/share/classes/sun/security/krb5/internal/KrbCredInfo.java index 4acf451cc..18fa7412f 100644 --- a/src/share/classes/sun/security/krb5/internal/KrbCredInfo.java +++ b/src/share/classes/sun/security/krb5/internal/KrbCredInfo.java @@ -187,14 +187,10 @@ public class KrbCredInfo { kcred.pname = (PrincipalName)pname.clone(); if (flags != null) kcred.flags = (TicketFlags)flags.clone(); - if (authtime != null) - kcred.authtime = (KerberosTime)authtime.clone(); - if (starttime != null) - kcred.starttime = (KerberosTime)starttime.clone(); - if (endtime != null) - kcred.endtime = (KerberosTime)endtime.clone(); - if (renewTill != null) - kcred.renewTill = (KerberosTime)renewTill.clone(); + kcred.authtime = authtime; + kcred.starttime = starttime; + kcred.endtime = endtime; + kcred.renewTill = renewTill; if (sname != null) kcred.sname = (PrincipalName)sname.clone(); if (caddr != null) diff --git a/src/share/classes/sun/security/krb5/internal/LastReqEntry.java b/src/share/classes/sun/security/krb5/internal/LastReqEntry.java index fa4ae0a5e..396d3b8df 100644 --- a/src/share/classes/sun/security/krb5/internal/LastReqEntry.java +++ b/src/share/classes/sun/security/krb5/internal/LastReqEntry.java @@ -90,7 +90,7 @@ public class LastReqEntry { public Object clone() { LastReqEntry newEntry = new LastReqEntry(); newEntry.lrType = lrType; - newEntry.lrValue = (KerberosTime)lrValue.clone(); + newEntry.lrValue = lrValue; return newEntry; } } diff --git a/src/share/classes/sun/security/krb5/internal/PAEncTSEnc.java b/src/share/classes/sun/security/krb5/internal/PAEncTSEnc.java index e5f2f755b..833756470 100644 --- a/src/share/classes/sun/security/krb5/internal/PAEncTSEnc.java +++ b/src/share/classes/sun/security/krb5/internal/PAEncTSEnc.java @@ -65,7 +65,7 @@ public class PAEncTSEnc { } public PAEncTSEnc() { - KerberosTime now = new KerberosTime(KerberosTime.NOW); + KerberosTime now = KerberosTime.now(); pATimeStamp = now; pAUSec = new Integer(now.getMicroSeconds()); } diff --git a/src/share/classes/sun/security/krb5/internal/ccache/Credentials.java b/src/share/classes/sun/security/krb5/internal/ccache/Credentials.java index f27a1588e..7128545a2 100644 --- a/src/share/classes/sun/security/krb5/internal/ccache/Credentials.java +++ b/src/share/classes/sun/security/krb5/internal/ccache/Credentials.java @@ -68,14 +68,11 @@ public class Credentials { sname = (PrincipalName) new_sname.clone(); key = (EncryptionKey) new_key.clone(); - authtime = (KerberosTime) new_authtime.clone(); - if (new_starttime != null) { - starttime = (KerberosTime) new_starttime.clone(); - } - endtime = (KerberosTime) new_endtime.clone(); - if (new_renewTill != null) { - renewTill = (KerberosTime) new_renewTill.clone(); - } + authtime = new_authtime; + starttime = new_starttime; + endtime = new_endtime; + renewTill = new_renewTill; + if (new_caddr != null) { caddr = (HostAddresses) new_caddr.clone(); } @@ -104,14 +101,11 @@ public class Credentials { ticket = (Ticket) kdcRep.ticket.clone(); key = (EncryptionKey) kdcRep.encKDCRepPart.key.clone(); flags = (TicketFlags) kdcRep.encKDCRepPart.flags.clone(); - authtime = (KerberosTime) kdcRep.encKDCRepPart.authtime.clone(); - if (kdcRep.encKDCRepPart.starttime != null) { - starttime = (KerberosTime) kdcRep.encKDCRepPart.starttime.clone(); - } - endtime = (KerberosTime) kdcRep.encKDCRepPart.endtime.clone(); - if (kdcRep.encKDCRepPart.renewTill != null) { - renewTill = (KerberosTime) kdcRep.encKDCRepPart.renewTill.clone(); - } + authtime = kdcRep.encKDCRepPart.authtime; + starttime = kdcRep.encKDCRepPart.starttime; + endtime = kdcRep.encKDCRepPart.endtime; + renewTill = kdcRep.encKDCRepPart.renewTill; + sname = (PrincipalName) kdcRep.encKDCRepPart.sname.clone(); caddr = (HostAddresses) kdcRep.encKDCRepPart.caddr.clone(); secondTicket = (Ticket) new_secondTicket.clone(); @@ -128,18 +122,10 @@ public class Credentials { sname = (PrincipalName) kdcRep.encKDCRepPart.sname.clone(); cname = (PrincipalName) kdcRep.cname.clone(); key = (EncryptionKey) kdcRep.encKDCRepPart.key.clone(); - authtime = (KerberosTime) kdcRep.encKDCRepPart.authtime.clone(); - if (kdcRep.encKDCRepPart.starttime != null) { - starttime = (KerberosTime) kdcRep.encKDCRepPart.starttime.clone(); - } else { - starttime = null; - } - endtime = (KerberosTime) kdcRep.encKDCRepPart.endtime.clone(); - if (kdcRep.encKDCRepPart.renewTill != null) { - renewTill = (KerberosTime) kdcRep.encKDCRepPart.renewTill.clone(); - } else { - renewTill = null; - } + authtime = kdcRep.encKDCRepPart.authtime; + starttime = kdcRep.encKDCRepPart.starttime; + endtime = kdcRep.encKDCRepPart.endtime; + renewTill = kdcRep.encKDCRepPart.renewTill; // if (kdcRep.msgType == Krb5.KRB_AS_REP) { // isEncInSKey = false; // secondTicket = null; diff --git a/test/sun/security/krb5/MicroTime.java b/test/sun/security/krb5/MicroTime.java index 6bbf8b8e9..e22f330df 100644 --- a/test/sun/security/krb5/MicroTime.java +++ b/test/sun/security/krb5/MicroTime.java @@ -22,7 +22,7 @@ */ /* * @test - * @bug 6882687 + * @bug 6882687 8011124 * @summary KerberosTime too imprecise */ @@ -32,11 +32,11 @@ public class MicroTime { public static void main(String[] args) throws Exception { // We count how many different KerberosTime values // can be acquired within one second. - KerberosTime t1 = new KerberosTime(true); + KerberosTime t1 = KerberosTime.now(); KerberosTime last = t1; int count = 0; while (true) { - KerberosTime t2 = new KerberosTime(true); + KerberosTime t2 = KerberosTime.now(); if (t2.getTime() - t1.getTime() > 1000) break; if (!last.equals(t2)) { last = t2; -- GitLab