8201627: Kerberos sequence number issues

Reviewed-by: valeriep

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

@@ -1,5 +1,5 @@
 /*
- * 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
@@ -28,6 +28,8 @@ package sun.security.jgss.krb5;
 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;
@@ -35,6 +37,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;
 
     /**
@@ -78,7 +107,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);
     }
 
     /**
@@ -143,10 +175,12 @@ class InitSecContextToken extends InitialToken {
                              apReqSeqNumber.intValue() :
                              0);
         context.resetPeerSequenceNumber(peerSeqNumber);
-        if (!context.getMutualAuthState())
+        if (!context.getMutualAuthState()) {
-            // Use the same sequence number as the peer
+            context.resetMySequenceNumber(
-            // (Behaviour exhibited by the Windows SSPI server)
+                    ACCEPTOR_USE_INITIATOR_SEQNUM
-            context.resetMySequenceNumber(peerSeqNumber);
+                            ? peerSeqNumber
+                            : 0);
+        }
         context.setAuthTime(
                 new KerberosTime(apReq.getCreds().getAuthTime()).toString());
         context.setTktFlags(apReq.getCreds().getFlags());

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

@@ -1,5 +1,5 @@
 /*
- * 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
@@ -499,11 +499,11 @@ abstract class MessageToken_v2 extends Krb5Token {
      */
     class MessageTokenHeader {
 
-         private int tokenId;
+        private int tokenId;
-         private byte[] bytes = new byte[TOKEN_HEADER_SIZE];
+        private byte[] bytes = new byte[TOKEN_HEADER_SIZE];
 
-         // Writes a new token header
+        // Writes a new token header
-         public MessageTokenHeader(int tokenId, boolean conf) throws GSSException {
+        public MessageTokenHeader(int tokenId, boolean conf) throws GSSException {
 
             this.tokenId = tokenId;
 
@@ -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/jdk/sun/security/krb5/auto/Basic.java

@@ -23,12 +23,15 @@
 
 /*
  * @test
- * @bug 7152176 8194486
+ * @bug 7152176 8194486 8201627
  * @summary More krb5 tests
  * @library /test/lib
  * @compile -XDignore.symbol.file Basic.java
  * @run main jdk.test.lib.FileInstaller TestHosts TestHosts
  * @run main/othervm -Djdk.net.hosts.file=TestHosts Basic
+ * @run main/othervm -Djdk.net.hosts.file=TestHosts
+ *      -Dsun.security.krb5.acceptor.sequence.number.nonmutual=zero
+ *      Basic
  */
 
 import sun.security.jgss.GSSUtil;
@@ -47,6 +50,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/jdk/sun/security/krb5/auto/BasicKrb5Test.java

@@ -97,7 +97,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/jdk/sun/security/krb5/auto/BasicProc.java

@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8009977 8186884 8194486
+ * @bug 8009977 8186884 8194486 8201627
  * @summary A test to launch multiple Java processes using either Java GSS
  *          or native GSS
  * @library ../../../../java/security/testlibrary/ /test/lib
@@ -37,9 +37,9 @@ import java.nio.file.Paths;
 import java.nio.file.attribute.PosixFilePermission;
 import java.util.Arrays;
 import java.util.PropertyPermission;
-import java.util.Random;
 import java.util.Set;
 
+import jdk.test.lib.Asserts;
 import jdk.test.lib.Platform;
 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(Proc.binIn()); // AP-REP
-                c.take(token);
+                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(s.take(token)); // AP-REP
-                Proc.binOut(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();
+                msg = s2.unwrap(Proc.binIn(), true);
-                s2.take(token); // AP-REP
+                Asserts.assertTrue(Arrays.equals(msg, MSG));
-                Random r = new Random();
+                s2.verifyMic(Proc.binIn(), msg);
-                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");
-                }
                 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
+                token = b.take(Proc.binIn()); // AP-REQ
-                Proc.binOut(b.take(token)); // AP-REP
+                Asserts.assertTrue(token == null);
-                msg = b.unwrap(Proc.binIn(), true); // enc1
+                Proc.binOut(b.wrap(MSG, true));
-                if (!Arrays.equals(msg, b.unwrap(Proc.binIn(), true))) {  // enc2
+                Proc.binOut(b.getMic(MSG));
-                    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
                 break;
         }
     }
@@ -281,20 +270,18 @@ public class BasicProc {
         }
         pb.start();
 
-        // Client and server handshake
+        // Client and server
-        ps.println(pc.readData());
+        ps.println(pc.readData()); // AP-REQ
-        pc.println(ps.readData());
+        pc.println(ps.readData()); // AP-REP
 
-        // Server and backend handshake
+        ps.println(pc.readData()); // KRB-PRIV
-        pb.println(ps.readData());
+        ps.println(pc.readData()); // KRB-SAFE
-        ps.println(pb.readData());
 
-        // wrap/unwrap/getMic/verifyMic and plain text
+        // Server and backend
-        pb.println(ps.readData());
+        pb.println(ps.readData()); // AP-REQ
-        pb.println(ps.readData());
+
-        pb.println(ps.readData());
+        ps.println(pb.readData()); // KRB-PRIV
-        ps.println(pb.readData());
+        ps.println(pb.readData()); // KRB-SAFE
-        ps.println(pb.readData());
 
         if ((pc.waitFor() | ps.waitFor() | pb.waitFor()) != 0) {
             throw new Exception("Process failed");

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

@@ -1,5 +1,5 @@
 /*
- * 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);
+                System.out.printf("unwrap from %s", me.name);
-                MessageProp p1 = new MessageProp(0, privacy);
+                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");
         }