Enabled NullAway for NTLM package (#1887)

* Enabled NullAway for NTLM package

* Renamed method parameter and added some comments

client/src/main/java/org/asynchttpclient/ntlm/NtlmEngine.java

@@ -27,11 +27,14 @@
 // fork from Apache HttpComponents
 package org.asynchttpclient.ntlm;
 
+import org.jetbrains.annotations.Contract;
+import org.jetbrains.annotations.Nullable;
+
 import javax.crypto.Cipher;
 import javax.crypto.spec.SecretKeySpec;
 import java.io.UnsupportedEncodingException;
 import java.nio.charset.Charset;
-import java.nio.charset.UnsupportedCharsetException;
+import java.nio.charset.StandardCharsets;
 import java.security.Key;
 import java.security.MessageDigest;
 import java.security.SecureRandom;
@@ -55,17 +58,7 @@ public final class NtlmEngine {
  /**
  * Unicode encoding
  */
- private static final Charset UNICODE_LITTLE_UNMARKED;
-
- static {
- Charset c;
- try {
- c = Charset.forName("UnicodeLittleUnmarked");
- } catch (UnsupportedCharsetException e) {
- c = null;
- }
- UNICODE_LITTLE_UNMARKED = c;
- }
+ private static final Charset UNICODE_LITTLE_UNMARKED = StandardCharsets.UTF_16LE;
 
  private static final byte[] MAGIC_CONSTANT = "KGS!@#$%".getBytes(US_ASCII);
 
@@ -92,7 +85,7 @@ public final class NtlmEngine {
  /**
  * Secure random generator
  */
- private static final SecureRandom RND_GEN;
+ private static final @Nullable SecureRandom RND_GEN;
 
  static {
  SecureRandom rnd = null;
@@ -132,17 +125,14 @@ public final class NtlmEngine {
  * @throws NtlmEngineException If {@encrypt(byte[],byte[])} fails.
  */
  private static String getType3Message(final String user, final String password, final String host, final String domain, final byte[] nonce,
- final int type2Flags, final String target, final byte[] targetInformation) {
+ final int type2Flags, final @Nullable String target, final byte @Nullable [] targetInformation) {
  return new Type3Message(domain, host, user, password, nonce, type2Flags, target, targetInformation).getResponse();
  }
 
  /**
  * Strip dot suffix from a name
  */
  private static String stripDotSuffix(final String value) {
- if (value == null) {
- return null;
- }
  final int index = value.indexOf('.');
  if (index != -1) {
  return value.substring(0, index);
@@ -153,14 +143,16 @@ private static String stripDotSuffix(final String value) {
  /**
  * Convert host to standard form
  */
- private static String convertHost(final String host) {
+ @Contract(value = "!null -> !null", pure = true)
+ private static @Nullable String convertHost(final String host) {
  return host != null ? stripDotSuffix(host).toUpperCase() : null;
  }
 
  /**
  * Convert domain to standard form
  */
- private static String convertDomain(final String domain) {
+ @Contract(value = "!null -> !null", pure = true)
+ private static @Nullable String convertDomain(final String domain) {
  return domain != null ? stripDotSuffix(domain).toUpperCase() : null;
  }
 
@@ -223,36 +215,36 @@ private static class CipherGen {
  protected final String user;
  protected final String password;
  protected final byte[] challenge;
- protected final String target;
- protected final byte[] targetInformation;
+ protected final @Nullable String target;
+ protected final byte @Nullable [] targetInformation;
 
  // Information we can generate but may be passed in (for testing)
- protected byte[] clientChallenge;
- protected byte[] clientChallenge2;
- protected byte[] secondaryKey;
- protected byte[] timestamp;
+ protected byte @Nullable [] clientChallenge;
+ protected byte @Nullable [] clientChallenge2;
+ protected byte @Nullable [] secondaryKey;
+ protected byte @Nullable [] timestamp;
 
  // Stuff we always generate
- protected byte[] lmHash;
- protected byte[] lmResponse;
- protected byte[] ntlmHash;
- protected byte[] ntlmResponse;
- protected byte[] ntlmv2Hash;
- protected byte[] lmv2Hash;
- protected byte[] lmv2Response;
- protected byte[] ntlmv2Blob;
- protected byte[] ntlmv2Response;
- protected byte[] ntlm2SessionResponse;
- protected byte[] lm2SessionResponse;
- protected byte[] lmUserSessionKey;
- protected byte[] ntlmUserSessionKey;
- protected byte[] ntlmv2UserSessionKey;
- protected byte[] ntlm2SessionResponseUserSessionKey;
- protected byte[] lanManagerSessionKey;
-
- CipherGen(final String domain, final String user, final String password, final byte[] challenge, final String target,
-  final byte[] targetInformation, final byte[] clientChallenge, final byte[] clientChallenge2, final byte[] secondaryKey,
-  final byte[] timestamp) {
+ protected byte @Nullable [] lmHash;
+ protected byte @Nullable [] lmResponse;
+ protected byte @Nullable [] ntlmHash;
+ protected byte @Nullable [] ntlmResponse;
+ protected byte @Nullable [] ntlmv2Hash;
+ protected byte @Nullable [] lmv2Hash;
+ protected byte @Nullable [] lmv2Response;
+ protected byte @Nullable [] ntlmv2Blob;
+ protected byte @Nullable [] ntlmv2Response;
+ protected byte @Nullable [] ntlm2SessionResponse;
+ protected byte @Nullable [] lm2SessionResponse;
+ protected byte @Nullable [] lmUserSessionKey;
+ protected byte @Nullable [] ntlmUserSessionKey;
+ protected byte @Nullable [] ntlmv2UserSessionKey;
+ protected byte @Nullable [] ntlm2SessionResponseUserSessionKey;
+ protected byte @Nullable [] lanManagerSessionKey;
+
+ CipherGen(final String domain, final String user, final String password, final byte[] challenge, final @Nullable String target,
+ final byte @Nullable [] targetInformation, final byte @Nullable [] clientChallenge, final byte @Nullable [] clientChallenge2,
+ final byte @Nullable [] secondaryKey, final byte @Nullable [] timestamp) {
  this.domain = domain;
  this.target = target;
  this.user = user;
@@ -265,8 +257,8 @@ private static class CipherGen {
  this.timestamp = timestamp;
  }
 
- CipherGen(final String domain, final String user, final String password, final byte[] challenge, final String target,
-  final byte[] targetInformation) {
+ CipherGen(final String domain, final String user, final String password, final byte[] challenge, final @Nullable String target,
+ final byte @Nullable [] targetInformation) {
  this(domain, user, password, challenge, target, targetInformation, null, null, null, null);
  }
 
@@ -380,8 +372,11 @@ public byte[] getTimestamp() {
 
  /**
  * Calculate the NTLMv2Blob
+ *
+ * @param targetInformation this parameter is the same object as the field targetInformation,
+ * but guaranteed to be not null. This is done to satisfy NullAway requirements
  */
- public byte[] getNTLMv2Blob() {
+ public byte[] getNTLMv2Blob(byte[] targetInformation) {
  if (ntlmv2Blob == null) {
  ntlmv2Blob = createBlob(getClientChallenge2(), targetInformation, getTimestamp());
  }
@@ -390,10 +385,13 @@ public byte[] getNTLMv2Blob() {
 
  /**
  * Calculate the NTLMv2Response
+ *
+ * @param targetInformation this parameter is the same object as the field targetInformation,
+ * but guaranteed to be not null. This is done to satisfy NullAway requirements
  */
- public byte[] getNTLMv2Response() {
+ public byte[] getNTLMv2Response(byte[] targetInformation) {
  if (ntlmv2Response == null) {
- ntlmv2Response = lmv2Response(getNTLMv2Hash(), challenge, getNTLMv2Blob());
+ ntlmv2Response = lmv2Response(getNTLMv2Hash(), challenge, getNTLMv2Blob(targetInformation));
  }
  return ntlmv2Response;
  }
@@ -457,12 +455,15 @@ public byte[] getNTLMUserSessionKey() {
 
  /**
  * GetNTLMv2UserSessionKey
+ *
+ * @param targetInformation this parameter is the same object as the field targetInformation,
+ * but guaranteed to be not null. This is done to satisfy NullAway requirements
  */
- public byte[] getNTLMv2UserSessionKey() {
+ public byte[] getNTLMv2UserSessionKey(byte[] targetInformation) {
  if (ntlmv2UserSessionKey == null) {
  final byte[] ntlmv2hash = getNTLMv2Hash();
  final byte[] truncatedResponse = new byte[16];
- System.arraycopy(getNTLMv2Response(), 0, truncatedResponse, 0, 16);
+ System.arraycopy(getNTLMv2Response(targetInformation), 0, truncatedResponse, 0, 16);
  ntlmv2UserSessionKey = hmacMD5(truncatedResponse, ntlmv2hash);
  }
  return ntlmv2UserSessionKey;
@@ -597,9 +598,6 @@ private static byte[] lmHash(final String password) {
  * the NTLM Response and the NTLMv2 and LMv2 Hashes.
  */
  private static byte[] ntlmHash(final String password) {
- if (UNICODE_LITTLE_UNMARKED == null) {
- throw new NtlmEngineException("Unicode not supported");
- }
  final byte[] unicodePassword = password.getBytes(UNICODE_LITTLE_UNMARKED);
  final MD4 md4 = new MD4();
  md4.update(unicodePassword);
@@ -613,9 +611,6 @@ private static byte[] ntlmHash(final String password) {
  * Responses.
  */
  private static byte[] lmv2Hash(final String domain, final String user, final byte[] ntlmHash) {
- if (UNICODE_LITTLE_UNMARKED == null) {
- throw new NtlmEngineException("Unicode not supported");
- }
  final HMACMD5 hmacMD5 = new HMACMD5(ntlmHash);
  // Upper case username, upper case domain!
  hmacMD5.update(user.toUpperCase(Locale.ROOT).getBytes(UNICODE_LITTLE_UNMARKED));
@@ -632,9 +627,6 @@ private static byte[] lmv2Hash(final String domain, final String user, final byt
  * Responses.
  */
  private static byte[] ntlmv2Hash(final String domain, final String user, final byte[] ntlmHash) {
- if (UNICODE_LITTLE_UNMARKED == null) {
- throw new NtlmEngineException("Unicode not supported");
- }
  final HMACMD5 hmacMD5 = new HMACMD5(ntlmHash);
  // Upper case username, mixed case target!!
  hmacMD5.update(user.toUpperCase(Locale.ROOT).getBytes(UNICODE_LITTLE_UNMARKED));
@@ -774,10 +766,11 @@ private static void oddParity(final byte[] bytes) {
  * NTLM message generation, base class
  */
  private static class NTLMMessage {
+ private static final byte[] EMPTY_BYTE_ARRAY = new byte[]{};
  /**
  * The current response
  */
- private byte[] messageContents;
+ private byte[] messageContents = EMPTY_BYTE_ARRAY;
 
  /**
  * The current output position
@@ -902,7 +895,7 @@ protected void addByte(final byte b) {
  *
  * @param bytes the bytes to add.
  */
- protected void addBytes(final byte[] bytes) {
+ protected void addBytes(final byte @Nullable [] bytes) {
  if (bytes == null) {
  return;
  }
@@ -1022,8 +1015,8 @@ String getResponse() {
  */
  static class Type2Message extends NTLMMessage {
  protected byte[] challenge;
- protected String target;
- protected byte[] targetInfo;
+ protected @Nullable String target;
+ protected byte @Nullable [] targetInfo;
  protected int flags;
 
  Type2Message(final String message) {
@@ -1090,14 +1083,14 @@ byte[] getChallenge() {
  /**
  * Retrieve the target
  */
- String getTarget() {
+ @Nullable String getTarget() {
  return target;
  }
 
  /**
  * Retrieve the target info
  */
- byte[] getTargetInfo() {
+ byte @Nullable [] getTargetInfo() {
  return targetInfo;
  }
 
@@ -1117,19 +1110,19 @@ static class Type3Message extends NTLMMessage {
  // Response flags from the type2 message
  protected int type2Flags;
 
- protected byte[] domainBytes;
- protected byte[] hostBytes;
+ protected byte @Nullable [] domainBytes;
+ protected byte @Nullable [] hostBytes;
  protected byte[] userBytes;
 
  protected byte[] lmResp;
  protected byte[] ntResp;
- protected byte[] sessionKey;
+ protected byte @Nullable [] sessionKey;
 
  /**
  * Constructor. Pass the arguments we will need
  */
  Type3Message(final String domain, final String host, final String user, final String password, final byte[] nonce,
- final int type2Flags, final String target, final byte[] targetInformation) {
+ final int type2Flags, final @Nullable String target, final byte @Nullable [] targetInformation) {
  // Save the flags
  this.type2Flags = type2Flags;
 
@@ -1149,12 +1142,12 @@ static class Type3Message extends NTLMMessage {
  // been tested
  if ((type2Flags & FLAG_TARGETINFO_PRESENT) != 0 && targetInformation != null && target != null) {
  // NTLMv2
- ntResp = gen.getNTLMv2Response();
+ ntResp = gen.getNTLMv2Response(targetInformation);
  lmResp = gen.getLMv2Response();
  if ((type2Flags & FLAG_REQUEST_LAN_MANAGER_KEY) != 0) {
  userSessionKey = gen.getLanManagerSessionKey();
  } else {
- userSessionKey = gen.getNTLMv2UserSessionKey();
+ userSessionKey = gen.getNTLMv2UserSessionKey(targetInformation);
  }
  } else {
  // NTLMv1
@@ -1198,9 +1191,6 @@ static class Type3Message extends NTLMMessage {
  } else {
  sessionKey = null;
  }
- if (UNICODE_LITTLE_UNMARKED == null) {
- throw new NtlmEngineException("Unicode not supported");
- }
  hostBytes = unqualifiedHost != null ? unqualifiedHost.getBytes(UNICODE_LITTLE_UNMARKED) : null;
  domainBytes = unqualifiedDomain != null ? unqualifiedDomain.toUpperCase(Locale.ROOT).getBytes(UNICODE_LITTLE_UNMARKED) : null;
  userBytes = user.getBytes(UNICODE_LITTLE_UNMARKED);

client/src/main/java/org/asynchttpclient/ntlm/NtlmEngineException.java

@@ -26,6 +26,8 @@
 package org.asynchttpclient.ntlm;
 
 
+import org.jetbrains.annotations.Nullable;
+
 /**
  * Signals NTLM protocol failure.
  */
@@ -49,7 +51,7 @@ class NtlmEngineException extends RuntimeException {
  * @param cause the <tt>Throwable</tt> that caused this exception, or <tt>null</tt>
  * if the cause is unavailable, unknown, or not a <tt>Throwable</tt>
  */
- NtlmEngineException(String message, Throwable cause) {
+ NtlmEngineException(@Nullable String message, Throwable cause) {
  super(message, cause);
  }
 }

client/src/test/java/org/asynchttpclient/ntlm/NtlmTest.java

@@ -27,9 +27,11 @@
 import org.asynchttpclient.ntlm.NtlmEngine.Type2Message;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.handler.AbstractHandler;
+import org.junit.jupiter.api.Test;
 
 import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.Base64;
 import java.util.concurrent.ExecutionException;
@@ -70,6 +72,14 @@ private void ntlmAuthTest(Realm.Builder realmBuilder) throws IOException, Interr
  }
  }
 
+ @Test
+ public void testUnicodeLittleUnmarkedEncoding() {
+ final Charset unicodeLittleUnmarked = Charset.forName("UnicodeLittleUnmarked");
+ final Charset utf16le = StandardCharsets.UTF_16LE;
+ assertEquals(unicodeLittleUnmarked, utf16le);
+ assertArrayEquals("Test @ テスト".getBytes(unicodeLittleUnmarked), "Test @ テスト".getBytes(utf16le));
+ }
+
  @RepeatedIfExceptionsTest(repeats = 5)
  public void lazyNTLMAuthTest() throws Exception {
  ntlmAuthTest(realmBuilderBase());

pom.xml

@@ -233,7 +233,7 @@
  -Xep:NullOptional:ERROR
  -XepExcludedPaths:.*/src/test/java/.*
  -XepOpt:NullAway:AnnotatedPackages=org.asynchttpclient
- -XepOpt:NullAway:UnannotatedSubPackages=org.asynchttpclient.netty,org.asynchttpclient.ntlm,org.asynchttpclient.request,org.asynchttpclient.ws
+ -XepOpt:NullAway:UnannotatedSubPackages=org.asynchttpclient.netty,org.asynchttpclient.request,org.asynchttpclient.ws
  -XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true
  -Xep:NullAway:ERROR
  </arg>