8201627: Kerberos sequence number issues

Reviewed-by: mbalao

src/share/classes/sun/security/action/GetPropertyAction.java

@@ -108,4 +108,27 @@ public class GetPropertyAction implements PrivilegedAction<String> {
                     new GetPropertyAction(theProp));
         }
     }
+
+    /**
+     * Convenience method to get a property without going through doPrivileged
+     * if no security manager is present. This is unsafe for inclusion in a
+     * public API but allowable here since this class is now encapsulated.
+     *
+     * Note that this method performs a privileged action using caller-provided
+     * inputs. The caller of this method should take care to ensure that the
+     * inputs are not tainted and the returned property is not made accessible
+     * to untrusted code if it contains sensitive information.
+     *
+     * @param theProp the name of the system property.
+     * @param defaultVal the default value.
+     */
+    public static String privilegedGetProperty(String theProp,
+            String defaultVal) {
+        if (System.getSecurityManager() == null) {
+            return System.getProperty(theProp, defaultVal);
+        } else {
+            return AccessController.doPrivileged(
+                    new GetPropertyAction(theProp, defaultVal));
+        }
+    }
 }

src/share/classes/sun/security/jgss/krb5/InitSecContextToken.java

 /*
- * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2018, 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
@@ -29,6 +29,8 @@ import com.sun.security.jgss.AuthorizationDataEntry;
 import org.ietf.jgss.*;
 import java.io.InputStream;
 import java.io.IOException;
+
+import sun.security.action.GetPropertyAction;
 import sun.security.krb5.*;
 import java.net.InetAddress;
 import sun.security.krb5.internal.AuthorizationData;
@@ -36,6 +38,33 @@ import sun.security.krb5.internal.KerberosTime;
 
 class InitSecContextToken extends InitialToken {
 
+    // If non-mutual authentication is requested, there is no AP-REP message.
+    // The acceptor thus has no chance to send the seq-number field to the
+    // initiator. In this case, the initiator and acceptor should has an
+    // agreement to derive acceptor's initial seq-number if the acceptor wishes
+    // to send messages to the initiator.
+
+    // If this flag is true, it will the same as the initiator's initial
+    // seq-number (as MIT krb5 and Windows SSPI do). Otherwise, it will be zero
+    // (as Heimdal does). The default value is true.
+    private static final boolean ACCEPTOR_USE_INITIATOR_SEQNUM;
+
+    static {
+        // The ACCEPTOR_USE_INITIATOR_SEQNUM value is determined by the system
+        // property "sun.security.krb5.acceptor.sequence.number.nonmutual",
+        // which can be set to "initiator", "zero" or "0".
+        String propName = "sun.security.krb5.acceptor.sequence.number.nonmutual";
+        String s = GetPropertyAction.privilegedGetProperty(propName, "initiator");
+        if (s.equals("initiator")) {
+            ACCEPTOR_USE_INITIATOR_SEQNUM = true;
+        } else if (s.equals("zero") || s.equals("0")) {
+            ACCEPTOR_USE_INITIATOR_SEQNUM = false;
+        } else {
+            throw new AssertionError("Unrecognized value for " + propName
+                    + ": " + s);
+        }
+    }
+
     private KrbApReq apReq = null;
 
     /**
@@ -79,7 +108,10 @@ class InitSecContextToken extends InitialToken {
             context.setKey(Krb5Context.SESSION_KEY, serviceTicket.getSessionKey());
 
         if (!mutualRequired)
-            context.resetPeerSequenceNumber(0);
+            context.resetPeerSequenceNumber(
+                    ACCEPTOR_USE_INITIATOR_SEQNUM
+                    ? apReq.getSeqNumber().intValue()
+                    : 0);
     }
 
     /**
@@ -144,10 +176,12 @@ class InitSecContextToken extends InitialToken {
                              apReqSeqNumber.intValue() :
                              0);
         context.resetPeerSequenceNumber(peerSeqNumber);
-        if (!context.getMutualAuthState())
-            // Use the same sequence number as the peer
-            // (Behaviour exhibited by the Windows SSPI server)
-            context.resetMySequenceNumber(peerSeqNumber);
+        if (!context.getMutualAuthState()) {
+            context.resetMySequenceNumber(
+                    ACCEPTOR_USE_INITIATOR_SEQNUM
+                            ? peerSeqNumber
+                            : 0);
+        }
         context.setAuthTime(
                 new KerberosTime(apReq.getCreds().getAuthTime()).toString());
         context.setTktFlags(apReq.getCreds().getFlags());

src/share/classes/sun/security/jgss/krb5/MessageToken_v2.java

 /*
- * Copyright (c) 2004, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2004, 2018, 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
@@ -609,7 +609,7 @@ abstract class MessageToken_v2 extends Krb5Token {
             prop.setQOP(0);
 
             // sequence number
-            seqNumber = readBigEndian(bytes, 0, 8);
+            seqNumber = readBigEndian(bytes, 12, 4);
         }
 
         /**

test/sun/security/krb5/auto/Basic.java

@@ -23,10 +23,12 @@
 
 /*
  * @test
- * @bug 7152176
+ * @bug 7152176 8201627
  * @summary More krb5 tests
  * @compile -XDignore.symbol.file Basic.java
- * @run main/othervm -Dsun.net.spi.nameservice.provider.1=ns,mock Basic
+ * @run main/othervm -Dsun.net.spi.nameservice.provider.1=ns,mock
+ *      -Dsun.security.krb5.acceptor.sequence.number.nonmutual=zero
+ *      Basic
  */
 
 import sun.security.jgss.GSSUtil;
@@ -45,6 +47,7 @@ public class Basic {
 
         c.startAsClient(OneKDC.SERVER, GSSUtil.GSS_KRB5_MECH_OID);
         c.x().requestCredDeleg(true);
+        c.x().requestMutualAuth(false);
         s.startAsServer(GSSUtil.GSS_KRB5_MECH_OID);
 
         Context.handshake(c, s);

test/sun/security/krb5/auto/BasicKrb5Test.java

@@ -79,7 +79,7 @@ public class BasicKrb5Test {
         String etype = null;
         for (String arg: args) {
             if (arg.equals("-s")) Context.usingStream = true;
-            else if(arg.equals("-C")) conf = false;
+            else if (arg.equals("-C")) conf = false;
             else etype = arg;
         }
 

test/sun/security/krb5/auto/BasicProc.java

@@ -23,10 +23,10 @@
 
 /*
  * @test
- * @bug 8009977 8186884
+ * @bug 8009977 8186884 8201627
  * @summary A test to launch multiple Java processes using either Java GSS
  *          or native GSS
- * @library ../../../../java/security/testlibrary/
+ * @library ../../../../java/security/testlibrary /lib/testlibrary
  * @compile -XDignore.symbol.file BasicProc.java
  * @run main/othervm -Dsun.net.spi.nameservice.provider.1=ns,mock BasicProc launcher
  */
@@ -38,9 +38,9 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.PropertyPermission;
-import java.util.Random;
 import java.util.Set;
 
+import jdk.testlibrary.Asserts;
 import org.ietf.jgss.Oid;
 import sun.security.krb5.Config;
 
@@ -84,6 +84,7 @@ public class BasicProc {
     private static final String REALM = "REALM";
 
     private static final int MSGSIZE = 1024;
+    private static final byte[] MSG = new byte[MSGSIZE];
 
     public static void main(String[] args) throws Exception {
 
@@ -165,9 +166,11 @@ public class BasicProc {
                         Context.fromUserPass(USER, PASS, false);
                 c.startAsClient(SERVER, oid);
                 c.x().requestCredDeleg(true);
+                c.x().requestMutualAuth(true);
                 Proc.binOut(c.take(new byte[0])); // AP-REQ
-                token = Proc.binIn(); // AP-REP
-                c.take(token);
+                c.take(Proc.binIn()); // AP-REP
+                Proc.binOut(c.wrap(MSG, true));
+                Proc.binOut(c.getMic(MSG));
                 break;
             case "server":
                 Context s = args[1].equals("n") ?
@@ -175,41 +178,27 @@ public class BasicProc {
                         Context.fromUserKtab(SERVER, KTAB_S, true);
                 s.startAsServer(oid);
                 token = Proc.binIn(); // AP-REQ
-                token = s.take(token);
-                Proc.binOut(token); // AP-REP
+                Proc.binOut(s.take(token)); // AP-REP
+                msg = s.unwrap(Proc.binIn(), true);
+                Asserts.assertTrue(Arrays.equals(msg, MSG));
+                s.verifyMic(Proc.binIn(), msg);
                 Context s2 = s.delegated();
                 s2.startAsClient(BACKEND, oid);
+                s2.x().requestMutualAuth(false);
                 Proc.binOut(s2.take(new byte[0])); // AP-REQ
-                token = Proc.binIn();
-                s2.take(token); // AP-REP
-                Random r = new Random();
-                msg = new byte[MSGSIZE];
-                r.nextBytes(msg);
-                Proc.binOut(s2.wrap(msg, true)); // enc1
-                Proc.binOut(s2.wrap(msg, true)); // enc2
-                Proc.binOut(s2.wrap(msg, true)); // enc3
-                s2.verifyMic(Proc.binIn(), msg); // mic
-                byte[] msg2 = Proc.binIn(); // msg
-                if (!Arrays.equals(msg, msg2)) {
-                    throw new Exception("diff msg");
-                }
+                msg = s2.unwrap(Proc.binIn(), true);
+                Asserts.assertTrue(Arrays.equals(msg, MSG));
+                s2.verifyMic(Proc.binIn(), msg);
                 break;
             case "backend":
                 Context b = args[1].equals("n") ?
                         Context.fromThinAir() :
                         Context.fromUserKtab(BACKEND, KTAB_B, true);
                 b.startAsServer(oid);
-                token = Proc.binIn(); // AP-REQ
-                Proc.binOut(b.take(token)); // AP-REP
-                msg = b.unwrap(Proc.binIn(), true); // enc1
-                if (!Arrays.equals(msg, b.unwrap(Proc.binIn(), true))) {  // enc2
-                    throw new Exception("diff msg");
-                }
-                if (!Arrays.equals(msg, b.unwrap(Proc.binIn(), true))) {  // enc3
-                    throw new Exception("diff msg");
-                }
-                Proc.binOut(b.getMic(msg)); // mic
-                Proc.binOut(msg); // msg
+                token = b.take(Proc.binIn()); // AP-REQ
+                Asserts.assertTrue(token == null);
+                Proc.binOut(b.wrap(MSG, true));
+                Proc.binOut(b.getMic(MSG));
                 break;
         }
     }
@@ -281,20 +270,18 @@ public class BasicProc {
         }
         pb.start();
 
-        // Client and server handshake
-        ps.println(pc.readData());
-        pc.println(ps.readData());
+        // Client and server
+        ps.println(pc.readData()); // AP-REQ
+        pc.println(ps.readData()); // AP-REP
+
+        ps.println(pc.readData()); // KRB-PRIV
+        ps.println(pc.readData()); // KRB-SAFE
 
-        // Server and backend handshake
-        pb.println(ps.readData());
-        ps.println(pb.readData());
+        // Server and backend
+        pb.println(ps.readData()); // AP-REQ
 
-        // wrap/unwrap/getMic/verifyMic and plain text
-        pb.println(ps.readData());
-        pb.println(ps.readData());
-        pb.println(ps.readData());
-        ps.println(pb.readData());
-        ps.println(pb.readData());
+        ps.println(pb.readData()); // KRB-PRIV
+        ps.println(pb.readData()); // KRB-SAFE
 
         if ((pc.waitFor() | ps.waitFor() | pb.waitFor()) != 0) {
             throw new Exception("Process failed");

test/sun/security/krb5/auto/Context.java

 /*
- * Copyright (c) 2008, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2018, 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
@@ -475,18 +475,21 @@ public class Context {
                     out = me.x.wrap(input, 0, input.length, p1);
                 }
                 System.out.println(printProp(p1));
+                if ((x.getConfState() && privacy) != p1.getPrivacy()) {
+                    throw new Exception("unexpected privacy status");
+                }
                 return out;
             }
         }, t);
     }
 
-    public byte[] unwrap(byte[] t, final boolean privacy)
+    public byte[] unwrap(byte[] t, final boolean privacyExpected)
             throws Exception {
         return doAs(new Action() {
             @Override
             public byte[] run(Context me, byte[] input) throws Exception {
-                System.out.printf("unwrap %s privacy from %s: ", privacy?"with":"without", me.name);
-                MessageProp p1 = new MessageProp(0, privacy);
+                System.out.printf("unwrap from %s", me.name);
+                MessageProp p1 = new MessageProp(0, true);
                 byte[] bytes;
                 if (usingStream) {
                     ByteArrayOutputStream os = new ByteArrayOutputStream();
@@ -496,6 +499,9 @@ public class Context {
                     bytes = me.x.unwrap(input, 0, input.length, p1);
                 }
                 System.out.println(printProp(p1));
+                if (p1.getPrivacy() != privacyExpected) {
+                    throw new Exception("Unexpected privacy: " + p1.getPrivacy());
+                }
                 return bytes;
             }
         }, t);
@@ -537,6 +543,10 @@ public class Context {
                             p1);
                 }
                 System.out.println(printProp(p1));
+                if (p1.isUnseqToken() || p1.isOldToken()
+                        || p1.isDuplicateToken() || p1.isGapToken()) {
+                    throw new Exception("Wrong sequence number detected");
+                }
                 return null;
             }
         }, t);
@@ -572,7 +582,7 @@ public class Context {
         System.out.printf("-------------------- TRANSMIT from %s to %s------------------------\n",
                 s1.name, s2.name);
         byte[] wrapped = s1.wrap(messageBytes, true);
-        byte[] unwrapped = s2.unwrap(wrapped, true);
+        byte[] unwrapped = s2.unwrap(wrapped, s2.x.getConfState());
         if (!Arrays.equals(messageBytes, unwrapped)) {
             throw new Exception("wrap/unwrap mismatch");
         }