From 123b0cd0a513cad620997988237669104c58b3bf Mon Sep 17 00:00:00 2001 From: weijun Date: Fri, 19 Aug 2011 13:42:40 +0800 Subject: [PATCH] 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 --- .../classes/com/sun/security/ntlm/Client.java | 16 +-- .../com/sun/security/ntlm/NTLMException.java | 5 + .../classes/com/sun/security/ntlm/Server.java | 21 ++-- .../sun/security/sasl/ntlm/FactoryImpl.java | 12 +- .../sun/security/sasl/ntlm/NTLMClient.java | 11 +- .../sun/security/sasl/ntlm/NTLMServer.java | 20 ++-- .../sun/security/sasl/ntlm/Conformance.java | 104 ++++++++++++++++++ 7 files changed, 159 insertions(+), 30 deletions(-) create mode 100644 test/com/sun/security/sasl/ntlm/Conformance.java diff --git a/src/share/classes/com/sun/security/ntlm/Client.java b/src/share/classes/com/sun/security/ntlm/Client.java index aed8f3708..f4be91aa3 100644 --- a/src/share/classes/com/sun/security/ntlm/Client.java +++ b/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); diff --git a/src/share/classes/com/sun/security/ntlm/NTLMException.java b/src/share/classes/com/sun/security/ntlm/NTLMException.java index 279fd2623..7275b4ae6 100644 --- a/src/share/classes/com/sun/security/ntlm/NTLMException.java +++ b/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; /** diff --git a/src/share/classes/com/sun/security/ntlm/Server.java b/src/share/classes/com/sun/security/ntlm/Server.java index 1871375a4..bc23d473f 100644 --- a/src/share/classes/com/sun/security/ntlm/Server.java +++ b/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); diff --git a/src/share/classes/com/sun/security/sasl/ntlm/FactoryImpl.java b/src/share/classes/com/sun/security/sasl/ntlm/FactoryImpl.java index 6ee1b66f9..af234bce5 100644 --- a/src/share/classes/com/sun/security/sasl/ntlm/FactoryImpl.java +++ b/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); } diff --git a/src/share/classes/com/sun/security/sasl/ntlm/NTLMClient.java b/src/share/classes/com/sun/security/sasl/ntlm/NTLMClient.java index f015f02aa..dbb1d610d 100644 --- a/src/share/classes/com/sun/security/sasl/ntlm/NTLMClient.java +++ b/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"; diff --git a/src/share/classes/com/sun/security/sasl/ntlm/NTLMServer.java b/src/share/classes/com/sun/security/sasl/ntlm/NTLMServer.java index 1a76a7705..916f7691b 100644 --- a/src/share/classes/com/sun/security/sasl/ntlm/NTLMServer.java +++ b/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"; diff --git a/test/com/sun/security/sasl/ntlm/Conformance.java b/test/com/sun/security/sasl/ntlm/Conformance.java new file mode 100644 index 000000000..4c1301e25 --- /dev/null +++ b/test/com/sun/security/sasl/ntlm/Conformance.java @@ -0,0 +1,104 @@ +/* + * 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(), null); + } catch (SaslException se) { + System.out.println(se); + } + try { + Sasl.createSaslServer("NTLM", "ldap", + "server", new HashMap(), 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); + } + } +} -- GitLab