7043847: NTML impl of SaslServer throws UnsupportedOperationException from (un)wrap method

7043860: NTML impl of SaslServer doesn't throw ISE from getAuthorizationID() method
7043882: NTML impl of SaslServer doesn't throw ISE from getNegotiatedProperty() method
7043938: NTML impl of SaslClientFactory throws NPE instead of SaslException
7043959: NTML impl of SaslClientFactory throws NPE for null CallBackHandler instance
Reviewed-by: vinnie

src/share/classes/com/sun/security/ntlm/Client.java

@@ -69,14 +69,16 @@ public final class Client extends NTLM {
      * This method does not make any modification to this parameter, it neither
      * needs to access the content of this parameter after this method call,
      * so you are free to modify or nullify this parameter after this call.
-     * @throws NullPointerException if {@code username} or {@code password} is null.
-     * @throws NTLMException if {@code version} is illegal
+     * @throws NTLMException if {@code username} or {@code password} is null,
+     * or {@code version} is illegal.
+     *
      */
     public Client(String version, String hostname, String username,
             String domain, char[] password) throws NTLMException {
         super(version);
         if ((username == null || password == null)) {
-            throw new NullPointerException("username/password cannot be null");
+            throw new NTLMException(NTLMException.PROTOCOL,
+                    "username/password cannot be null");
         }
         this.hostname = hostname;
         this.username = username;
@@ -117,13 +119,13 @@ public final class Client extends NTLM {
      * @param nonce random 8-byte array to be used in message generation,
      * must not be null except for original NTLM v1
      * @return the message generated
-     * @throws NullPointerException if {@code type2} or {@code nonce} is null
-     * for NTLM v1.
-     * @throws NTLMException if the incoming message is invalid
+     * @throws NTLMException if the incoming message is invalid, or
+     * {@code nonce} is null for NTLM v1.
      */
     public byte[] type3(byte[] type2, byte[] nonce) throws NTLMException {
         if (type2 == null || (v != Version.NTLM && nonce == null)) {
-            throw new NullPointerException("type2 and nonce cannot be null");
+            throw new NTLMException(NTLMException.PROTOCOL,
+                    "type2 and nonce cannot be null");
         }
         debug("NTLM Client: Type 2 received\n");
         debug(type2);

src/share/classes/com/sun/security/ntlm/NTLMException.java

@@ -65,6 +65,11 @@ public final class NTLMException extends GeneralSecurityException {
      */
     public final static int BAD_VERSION = 5;
 
+    /**
+     * Protocol errors.
+     */
+    public final static int PROTOCOL = 6;
+
     private int errorCode;
 
     /**

src/share/classes/com/sun/security/ntlm/Server.java

@@ -62,12 +62,13 @@ public abstract class Server extends NTLM {
      * is selected, authentication succeeds if one of LM (or LMv2) or
      * NTLM (or NTLMv2) is verified.
      * @param domain the domain, must not be null
-     * @throws NullPointerException if {@code domain} is null.
+     * @throws NTLMException if {@code domain} is null.
      */
     public Server(String version, String domain) throws NTLMException {
         super(version);
         if (domain == null) {
-            throw new NullPointerException("domain cannot be null");
+            throw new NTLMException(NTLMException.PROTOCOL,
+                    "domain cannot be null");
         }
         this.allVersion = (version == null);
         this.domain = domain;
@@ -80,12 +81,13 @@ public abstract class Server extends NTLM {
      * @param nonce the random 8-byte array to be used in message generation,
      * must not be null
      * @return the message generated
-     * @throws NullPointerException if type1 or nonce is null
-     * @throws NTLMException if the incoming message is invalid
+     * @throws NTLMException if the incoming message is invalid, or
+     * {@code nonce} is null.
      */
-    public byte[] type2(byte[] type1, byte[] nonce) {
+    public byte[] type2(byte[] type1, byte[] nonce) throws NTLMException {
         if (nonce == null) {
-            throw new NullPointerException("nonce cannot be null");
+            throw new NTLMException(NTLMException.PROTOCOL,
+                    "nonce cannot be null");
         }
         debug("NTLM Server: Type 1 received\n");
         if (type1 != null) debug(type1);
@@ -105,13 +107,14 @@ public abstract class Server extends NTLM {
      * @param type3 the incoming Type3 message from client, must not be null
      * @param nonce the same nonce provided in {@link #type2}, must not be null
      * @return username and hostname of the client in a byte array
-     * @throws NullPointerException if {@code type3} or {@code nonce} is null
-     * @throws NTLMException if the incoming message is invalid
+     * @throws NTLMException if the incoming message is invalid, or
+     * {@code nonce} is null.
      */
     public String[] verify(byte[] type3, byte[] nonce)
             throws NTLMException {
         if (type3 == null || nonce == null) {
-            throw new NullPointerException("type1 or nonce cannot be null");
+            throw new NTLMException(NTLMException.PROTOCOL,
+                    "type1 or nonce cannot be null");
         }
         debug("NTLM Server: Type 3 received\n");
         if (type3 != null) debug(type3);

src/share/classes/com/sun/security/sasl/ntlm/FactoryImpl.java

@@ -70,6 +70,12 @@ SaslServerFactory{
             if (mechs[i].equals("NTLM") &&
                     PolicyUtils.checkPolicy(mechPolicies[0], props)) {
 
+                if (cbh == null) {
+                    throw new SaslException(
+                        "Callback handler with support for " +
+                        "RealmCallback, NameCallback, and PasswordCallback " +
+                        "required");
+                }
                 return new NTLMClient(mechs[i], authorizationId,
                     protocol, serverName, props, cbh);
             }
@@ -98,9 +104,9 @@ SaslServerFactory{
              }
              if (cbh == null) {
                  throw new SaslException(
-                        "Callback handler with support for AuthorizeCallback, "+
-                        "RealmCallback, NameCallback, and PasswordCallback " +
-                        "required");
+                     "Callback handler with support for " +
+                     "RealmCallback, NameCallback, and PasswordCallback " +
+                     "required");
              }
              return new NTLMServer(mech, protocol, serverName, props, cbh);
          }

src/share/classes/com/sun/security/sasl/ntlm/NTLMClient.java

@@ -107,7 +107,7 @@ final class NTLMClient implements SaslClient {
      * @param protocol non-null for Sasl, useless for NTLM
      * @param serverName non-null for Sasl, but can be null for NTLM
      * @param props can be null
-     * @param cbh can be null for Sasl, but will throw NPE for NTLM
+     * @param cbh can be null for Sasl, already null-checked in factory
      * @throws SaslException
      */
     NTLMClient(String mech, String authzid, String protocol, String serverName,
@@ -166,7 +166,7 @@ final class NTLMClient implements SaslClient {
                     pcb.getPassword());
         } catch (NTLMException ne) {
             throw new SaslException(
-                    "NTLM: Invalid version string: " + version, ne);
+                    "NTLM: client creation failure", ne);
         }
     }
 
@@ -183,17 +183,20 @@ final class NTLMClient implements SaslClient {
     @Override
     public byte[] unwrap(byte[] incoming, int offset, int len)
             throws SaslException {
-        throw new UnsupportedOperationException("Not supported.");
+        throw new IllegalStateException("Not supported.");
     }
 
     @Override
     public byte[] wrap(byte[] outgoing, int offset, int len)
             throws SaslException {
-        throw new UnsupportedOperationException("Not supported.");
+        throw new IllegalStateException("Not supported.");
     }
 
     @Override
     public Object getNegotiatedProperty(String propName) {
+        if (!isComplete()) {
+            throw new IllegalStateException("authentication not complete");
+        }
         switch (propName) {
             case Sasl.QOP:
                 return "auth";

src/share/classes/com/sun/security/sasl/ntlm/NTLMServer.java

@@ -106,7 +106,7 @@ final class NTLMServer implements SaslServer {
      * @param serverName not null for Sasl, can be null in NTLM. If non-null,
      * might be used as domain if not provided in props
      * @param props can be null
-     * @param cbh can be null for Sasl, but will throw NPE in auth for NTLM
+     * @param cbh can be null for Sasl, already null-checked in factory
      * @throws SaslException
      */
     NTLMServer(String mech, String protocol, String serverName,
@@ -132,7 +132,7 @@ final class NTLMServer implements SaslServer {
             domain = serverName;
         }
         if (domain == null) {
-            throw new NullPointerException("Domain must be provided as"
+            throw new SaslException("Domain must be provided as"
                     + " the serverName argument or in props");
         }
 
@@ -159,7 +159,7 @@ final class NTLMServer implements SaslServer {
             };
         } catch (NTLMException ne) {
             throw new SaslException(
-                    "NTLM: Invalid version string: " + version, ne);
+                    "NTLM: server creation failure", ne);
         }
         nonce = new byte[8];
     }
@@ -182,8 +182,8 @@ final class NTLMServer implements SaslServer {
                 hostname = out[1];
                 return null;
             }
-        } catch (GeneralSecurityException ex) {
-            throw new SaslException("", ex);
+        } catch (NTLMException ex) {
+            throw new SaslException("NTLM: generate response failure", ex);
         }
     }
 
@@ -194,23 +194,29 @@ final class NTLMServer implements SaslServer {
 
     @Override
     public String getAuthorizationID() {
+        if (!isComplete()) {
+            throw new IllegalStateException("authentication not complete");
+        }
         return authzId;
     }
 
     @Override
     public byte[] unwrap(byte[] incoming, int offset, int len)
             throws SaslException {
-        throw new UnsupportedOperationException("Not supported yet.");
+        throw new IllegalStateException("Not supported yet.");
     }
 
     @Override
     public byte[] wrap(byte[] outgoing, int offset, int len)
             throws SaslException {
-        throw new UnsupportedOperationException("Not supported yet.");
+        throw new IllegalStateException("Not supported yet.");
     }
 
     @Override
     public Object getNegotiatedProperty(String propName) {
+        if (!isComplete()) {
+            throw new IllegalStateException("authentication not complete");
+        }
         switch (propName) {
             case Sasl.QOP:
                 return "auth";

test/com/sun/security/sasl/ntlm/Conformance.java

+/*
+ * Copyright (c) 2011, 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 7043847 7043860 7043882 7043938 7043959
+ * @summary NTML impl of SaslServer conformance errors
+ */
+import java.io.IOException;
+import javax.security.sasl.*;
+import java.util.*;
+import javax.security.auth.callback.Callback;
+import javax.security.auth.callback.CallbackHandler;
+import javax.security.auth.callback.UnsupportedCallbackException;
+
+public class Conformance {
+
+    public static void main(String[] args) throws Exception {
+        try {
+            Sasl.createSaslClient(new String[] {"NTLM"}, "abc", "ldap",
+                    "server", new HashMap<String, Object>(), null);
+        } catch (SaslException se) {
+            System.out.println(se);
+        }
+        try {
+            Sasl.createSaslServer("NTLM", "ldap",
+                    "server", new HashMap<String, Object>(), null);
+        } catch (SaslException se) {
+            System.out.println(se);
+        }
+        try {
+            Sasl.createSaslClient(new String[] {"NTLM"}, "abc", "ldap",
+                    "server", null, new CallbackHandler() {
+                        @Override
+                        public void handle(Callback[] callbacks) throws
+                                IOException, UnsupportedCallbackException {  }
+                    });
+        } catch (SaslException se) {
+            System.out.println(se);
+        }
+        try {
+            SaslServer saslServer =
+                    Sasl.createSaslServer("NTLM", "ldap", "abc", null, new CallbackHandler() {
+                        @Override
+                        public void handle(Callback[] callbacks) throws IOException, UnsupportedCallbackException {  }
+                    });
+            System.err.println("saslServer = " + saslServer);
+            System.err.println("saslServer.isComplete() = " + saslServer.isComplete());
+            // IllegalStateException is expected here
+            saslServer.getNegotiatedProperty("prop");
+            System.err.println("No IllegalStateException");
+        } catch (IllegalStateException se) {
+            System.out.println(se);
+        }
+        try {
+            SaslServer saslServer =
+                    Sasl.createSaslServer("NTLM", "ldap", "abc", null, new CallbackHandler() {
+                        @Override
+                        public void handle(Callback[] callbacks) throws IOException, UnsupportedCallbackException {  }
+                    });
+            System.err.println("saslServer = " + saslServer);
+            System.err.println("saslServer.isComplete() = " + saslServer.isComplete());
+            // IllegalStateException is expected here
+            saslServer.getAuthorizationID();
+            System.err.println("No IllegalStateException");
+        } catch (IllegalStateException se) {
+            System.out.println(se);
+        }
+        try {
+            SaslServer saslServer =
+                    Sasl.createSaslServer("NTLM", "ldap", "abc", null, new CallbackHandler() {
+                        @Override
+                        public void handle(Callback[] callbacks) throws IOException, UnsupportedCallbackException {  }
+                    });
+            System.err.println("saslServer = " + saslServer);
+            System.err.println("saslServer.isComplete() = " + saslServer.isComplete());
+            // IllegalStateException is expected here
+            saslServer.wrap(new byte[0], 0, 0);
+            System.err.println("No IllegalStateException");
+        } catch (IllegalStateException se) {
+            System.out.println(se);
+        }
+    }
+}