Skip to content

Commit 4dee060

Browse files
AsapinAsapin
authored
Enabled NullAway for NTLM package (#1887)
* Enabled NullAway for NTLM package * Renamed method parameter and added some comments
1 parent 8f8b637 commit 4dee060

File tree

4 files changed

+80
-78
lines changed

4 files changed

+80
-78
lines changed

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

Lines changed: 66 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,14 @@
2727
// fork from Apache HttpComponents
2828
package org.asynchttpclient.ntlm;
2929

30+
import org.jetbrains.annotations.Contract;
31+
import org.jetbrains.annotations.Nullable;
32+
3033
import javax.crypto.Cipher;
3134
import javax.crypto.spec.SecretKeySpec;
3235
import java.io.UnsupportedEncodingException;
3336
import java.nio.charset.Charset;
34-
import java.nio.charset.UnsupportedCharsetException;
37+
import java.nio.charset.StandardCharsets;
3538
import java.security.Key;
3639
import java.security.MessageDigest;
3740
import java.security.SecureRandom;
@@ -55,17 +58,7 @@ public final class NtlmEngine {
5558
/**
5659
* Unicode encoding
5760
*/
58-
private static final Charset UNICODE_LITTLE_UNMARKED;
59-
60-
static {
61-
Charset c;
62-
try {
63-
c = Charset.forName("UnicodeLittleUnmarked");
64-
} catch (UnsupportedCharsetException e) {
65-
c = null;
66-
}
67-
UNICODE_LITTLE_UNMARKED = c;
68-
}
61+
private static final Charset UNICODE_LITTLE_UNMARKED = StandardCharsets.UTF_16LE;
6962

7063
private static final byte[] MAGIC_CONSTANT = "KGS!@#$%".getBytes(US_ASCII);
7164

@@ -92,7 +85,7 @@ public final class NtlmEngine {
9285
/**
9386
* Secure random generator
9487
*/
95-
private static final SecureRandom RND_GEN;
88+
private static final @Nullable SecureRandom RND_GEN;
9689

9790
static {
9891
SecureRandom rnd = null;
@@ -132,17 +125,14 @@ public final class NtlmEngine {
132125
* @throws NtlmEngineException If {@encrypt(byte[],byte[])} fails.
133126
*/
134127
private static String getType3Message(final String user, final String password, final String host, final String domain, final byte[] nonce,
135-
final int type2Flags, final String target, final byte[] targetInformation) {
128+
final int type2Flags, final @Nullable String target, final byte @Nullable [] targetInformation) {
136129
return new Type3Message(domain, host, user, password, nonce, type2Flags, target, targetInformation).getResponse();
137130
}
138131

139132
/**
140133
* Strip dot suffix from a name
141134
*/
142135
private static String stripDotSuffix(final String value) {
143-
if (value == null) {
144-
return null;
145-
}
146136
final int index = value.indexOf('.');
147137
if (index != -1) {
148138
return value.substring(0, index);
@@ -153,14 +143,16 @@ private static String stripDotSuffix(final String value) {
153143
/**
154144
* Convert host to standard form
155145
*/
156-
private static String convertHost(final String host) {
146+
@Contract(value = "!null -> !null", pure = true)
147+
private static @Nullable String convertHost(final String host) {
157148
return host != null ? stripDotSuffix(host).toUpperCase() : null;
158149
}
159150

160151
/**
161152
* Convert domain to standard form
162153
*/
163-
private static String convertDomain(final String domain) {
154+
@Contract(value = "!null -> !null", pure = true)
155+
private static @Nullable String convertDomain(final String domain) {
164156
return domain != null ? stripDotSuffix(domain).toUpperCase() : null;
165157
}
166158

@@ -223,36 +215,36 @@ private static class CipherGen {
223215
protected final String user;
224216
protected final String password;
225217
protected final byte[] challenge;
226-
protected final String target;
227-
protected final byte[] targetInformation;
218+
protected final @Nullable String target;
219+
protected final byte @Nullable [] targetInformation;
228220

229221
// Information we can generate but may be passed in (for testing)
230-
protected byte[] clientChallenge;
231-
protected byte[] clientChallenge2;
232-
protected byte[] secondaryKey;
233-
protected byte[] timestamp;
222+
protected byte @Nullable [] clientChallenge;
223+
protected byte @Nullable [] clientChallenge2;
224+
protected byte @Nullable [] secondaryKey;
225+
protected byte @Nullable [] timestamp;
234226

235227
// Stuff we always generate
236-
protected byte[] lmHash;
237-
protected byte[] lmResponse;
238-
protected byte[] ntlmHash;
239-
protected byte[] ntlmResponse;
240-
protected byte[] ntlmv2Hash;
241-
protected byte[] lmv2Hash;
242-
protected byte[] lmv2Response;
243-
protected byte[] ntlmv2Blob;
244-
protected byte[] ntlmv2Response;
245-
protected byte[] ntlm2SessionResponse;
246-
protected byte[] lm2SessionResponse;
247-
protected byte[] lmUserSessionKey;
248-
protected byte[] ntlmUserSessionKey;
249-
protected byte[] ntlmv2UserSessionKey;
250-
protected byte[] ntlm2SessionResponseUserSessionKey;
251-
protected byte[] lanManagerSessionKey;
252-
253-
CipherGen(final String domain, final String user, final String password, final byte[] challenge, final String target,
254-
final byte[] targetInformation, final byte[] clientChallenge, final byte[] clientChallenge2, final byte[] secondaryKey,
255-
final byte[] timestamp) {
228+
protected byte @Nullable [] lmHash;
229+
protected byte @Nullable [] lmResponse;
230+
protected byte @Nullable [] ntlmHash;
231+
protected byte @Nullable [] ntlmResponse;
232+
protected byte @Nullable [] ntlmv2Hash;
233+
protected byte @Nullable [] lmv2Hash;
234+
protected byte @Nullable [] lmv2Response;
235+
protected byte @Nullable [] ntlmv2Blob;
236+
protected byte @Nullable [] ntlmv2Response;
237+
protected byte @Nullable [] ntlm2SessionResponse;
238+
protected byte @Nullable [] lm2SessionResponse;
239+
protected byte @Nullable [] lmUserSessionKey;
240+
protected byte @Nullable [] ntlmUserSessionKey;
241+
protected byte @Nullable [] ntlmv2UserSessionKey;
242+
protected byte @Nullable [] ntlm2SessionResponseUserSessionKey;
243+
protected byte @Nullable [] lanManagerSessionKey;
244+
245+
CipherGen(final String domain, final String user, final String password, final byte[] challenge, final @Nullable String target,
246+
final byte @Nullable [] targetInformation, final byte @Nullable [] clientChallenge, final byte @Nullable [] clientChallenge2,
247+
final byte @Nullable [] secondaryKey, final byte @Nullable [] timestamp) {
256248
this.domain = domain;
257249
this.target = target;
258250
this.user = user;
@@ -265,8 +257,8 @@ private static class CipherGen {
265257
this.timestamp = timestamp;
266258
}
267259

268-
CipherGen(final String domain, final String user, final String password, final byte[] challenge, final String target,
269-
final byte[] targetInformation) {
260+
CipherGen(final String domain, final String user, final String password, final byte[] challenge, final @Nullable String target,
261+
final byte @Nullable [] targetInformation) {
270262
this(domain, user, password, challenge, target, targetInformation, null, null, null, null);
271263
}
272264

@@ -380,8 +372,11 @@ public byte[] getTimestamp() {
380372

381373
/**
382374
* Calculate the NTLMv2Blob
375+
*
376+
* @param targetInformation this parameter is the same object as the field targetInformation,
377+
* but guaranteed to be not null. This is done to satisfy NullAway requirements
383378
*/
384-
public byte[] getNTLMv2Blob() {
379+
public byte[] getNTLMv2Blob(byte[] targetInformation) {
385380
if (ntlmv2Blob == null) {
386381
ntlmv2Blob = createBlob(getClientChallenge2(), targetInformation, getTimestamp());
387382
}
@@ -390,10 +385,13 @@ public byte[] getNTLMv2Blob() {
390385

391386
/**
392387
* Calculate the NTLMv2Response
388+
*
389+
* @param targetInformation this parameter is the same object as the field targetInformation,
390+
* but guaranteed to be not null. This is done to satisfy NullAway requirements
393391
*/
394-
public byte[] getNTLMv2Response() {
392+
public byte[] getNTLMv2Response(byte[] targetInformation) {
395393
if (ntlmv2Response == null) {
396-
ntlmv2Response = lmv2Response(getNTLMv2Hash(), challenge, getNTLMv2Blob());
394+
ntlmv2Response = lmv2Response(getNTLMv2Hash(), challenge, getNTLMv2Blob(targetInformation));
397395
}
398396
return ntlmv2Response;
399397
}
@@ -457,12 +455,15 @@ public byte[] getNTLMUserSessionKey() {
457455

458456
/**
459457
* GetNTLMv2UserSessionKey
458+
*
459+
* @param targetInformation this parameter is the same object as the field targetInformation,
460+
* but guaranteed to be not null. This is done to satisfy NullAway requirements
460461
*/
461-
public byte[] getNTLMv2UserSessionKey() {
462+
public byte[] getNTLMv2UserSessionKey(byte[] targetInformation) {
462463
if (ntlmv2UserSessionKey == null) {
463464
final byte[] ntlmv2hash = getNTLMv2Hash();
464465
final byte[] truncatedResponse = new byte[16];
465-
System.arraycopy(getNTLMv2Response(), 0, truncatedResponse, 0, 16);
466+
System.arraycopy(getNTLMv2Response(targetInformation), 0, truncatedResponse, 0, 16);
466467
ntlmv2UserSessionKey = hmacMD5(truncatedResponse, ntlmv2hash);
467468
}
468469
return ntlmv2UserSessionKey;
@@ -597,9 +598,6 @@ private static byte[] lmHash(final String password) {
597598
* the NTLM Response and the NTLMv2 and LMv2 Hashes.
598599
*/
599600
private static byte[] ntlmHash(final String password) {
600-
if (UNICODE_LITTLE_UNMARKED == null) {
601-
throw new NtlmEngineException("Unicode not supported");
602-
}
603601
final byte[] unicodePassword = password.getBytes(UNICODE_LITTLE_UNMARKED);
604602
final MD4 md4 = new MD4();
605603
md4.update(unicodePassword);
@@ -613,9 +611,6 @@ private static byte[] ntlmHash(final String password) {
613611
* Responses.
614612
*/
615613
private static byte[] lmv2Hash(final String domain, final String user, final byte[] ntlmHash) {
616-
if (UNICODE_LITTLE_UNMARKED == null) {
617-
throw new NtlmEngineException("Unicode not supported");
618-
}
619614
final HMACMD5 hmacMD5 = new HMACMD5(ntlmHash);
620615
// Upper case username, upper case domain!
621616
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
632627
* Responses.
633628
*/
634629
private static byte[] ntlmv2Hash(final String domain, final String user, final byte[] ntlmHash) {
635-
if (UNICODE_LITTLE_UNMARKED == null) {
636-
throw new NtlmEngineException("Unicode not supported");
637-
}
638630
final HMACMD5 hmacMD5 = new HMACMD5(ntlmHash);
639631
// Upper case username, mixed case target!!
640632
hmacMD5.update(user.toUpperCase(Locale.ROOT).getBytes(UNICODE_LITTLE_UNMARKED));
@@ -774,10 +766,11 @@ private static void oddParity(final byte[] bytes) {
774766
* NTLM message generation, base class
775767
*/
776768
private static class NTLMMessage {
769+
private static final byte[] EMPTY_BYTE_ARRAY = new byte[]{};
777770
/**
778771
* The current response
779772
*/
780-
private byte[] messageContents;
773+
private byte[] messageContents = EMPTY_BYTE_ARRAY;
781774

782775
/**
783776
* The current output position
@@ -902,7 +895,7 @@ protected void addByte(final byte b) {
902895
*
903896
* @param bytes the bytes to add.
904897
*/
905-
protected void addBytes(final byte[] bytes) {
898+
protected void addBytes(final byte @Nullable [] bytes) {
906899
if (bytes == null) {
907900
return;
908901
}
@@ -1022,8 +1015,8 @@ String getResponse() {
10221015
*/
10231016
static class Type2Message extends NTLMMessage {
10241017
protected byte[] challenge;
1025-
protected String target;
1026-
protected byte[] targetInfo;
1018+
protected @Nullable String target;
1019+
protected byte @Nullable [] targetInfo;
10271020
protected int flags;
10281021

10291022
Type2Message(final String message) {
@@ -1090,14 +1083,14 @@ byte[] getChallenge() {
10901083
/**
10911084
* Retrieve the target
10921085
*/
1093-
String getTarget() {
1086+
@Nullable String getTarget() {
10941087
return target;
10951088
}
10961089

10971090
/**
10981091
* Retrieve the target info
10991092
*/
1100-
byte[] getTargetInfo() {
1093+
byte @Nullable [] getTargetInfo() {
11011094
return targetInfo;
11021095
}
11031096

@@ -1117,19 +1110,19 @@ static class Type3Message extends NTLMMessage {
11171110
// Response flags from the type2 message
11181111
protected int type2Flags;
11191112

1120-
protected byte[] domainBytes;
1121-
protected byte[] hostBytes;
1113+
protected byte @Nullable [] domainBytes;
1114+
protected byte @Nullable [] hostBytes;
11221115
protected byte[] userBytes;
11231116

11241117
protected byte[] lmResp;
11251118
protected byte[] ntResp;
1126-
protected byte[] sessionKey;
1119+
protected byte @Nullable [] sessionKey;
11271120

11281121
/**
11291122
* Constructor. Pass the arguments we will need
11301123
*/
11311124
Type3Message(final String domain, final String host, final String user, final String password, final byte[] nonce,
1132-
final int type2Flags, final String target, final byte[] targetInformation) {
1125+
final int type2Flags, final @Nullable String target, final byte @Nullable [] targetInformation) {
11331126
// Save the flags
11341127
this.type2Flags = type2Flags;
11351128

@@ -1149,12 +1142,12 @@ static class Type3Message extends NTLMMessage {
11491142
// been tested
11501143
if ((type2Flags & FLAG_TARGETINFO_PRESENT) != 0 && targetInformation != null && target != null) {
11511144
// NTLMv2
1152-
ntResp = gen.getNTLMv2Response();
1145+
ntResp = gen.getNTLMv2Response(targetInformation);
11531146
lmResp = gen.getLMv2Response();
11541147
if ((type2Flags & FLAG_REQUEST_LAN_MANAGER_KEY) != 0) {
11551148
userSessionKey = gen.getLanManagerSessionKey();
11561149
} else {
1157-
userSessionKey = gen.getNTLMv2UserSessionKey();
1150+
userSessionKey = gen.getNTLMv2UserSessionKey(targetInformation);
11581151
}
11591152
} else {
11601153
// NTLMv1
@@ -1198,9 +1191,6 @@ static class Type3Message extends NTLMMessage {
11981191
} else {
11991192
sessionKey = null;
12001193
}
1201-
if (UNICODE_LITTLE_UNMARKED == null) {
1202-
throw new NtlmEngineException("Unicode not supported");
1203-
}
12041194
hostBytes = unqualifiedHost != null ? unqualifiedHost.getBytes(UNICODE_LITTLE_UNMARKED) : null;
12051195
domainBytes = unqualifiedDomain != null ? unqualifiedDomain.toUpperCase(Locale.ROOT).getBytes(UNICODE_LITTLE_UNMARKED) : null;
12061196
userBytes = user.getBytes(UNICODE_LITTLE_UNMARKED);

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
package org.asynchttpclient.ntlm;
2727

2828

29+
import org.jetbrains.annotations.Nullable;
30+
2931
/**
3032
* Signals NTLM protocol failure.
3133
*/
@@ -49,7 +51,7 @@ class NtlmEngineException extends RuntimeException {
4951
* @param cause the <tt>Throwable</tt> that caused this exception, or <tt>null</tt>
5052
* if the cause is unavailable, unknown, or not a <tt>Throwable</tt>
5153
*/
52-
NtlmEngineException(String message, Throwable cause) {
54+
NtlmEngineException(@Nullable String message, Throwable cause) {
5355
super(message, cause);
5456
}
5557
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,11 @@
2727
import org.asynchttpclient.ntlm.NtlmEngine.Type2Message;
2828
import org.eclipse.jetty.server.Request;
2929
import org.eclipse.jetty.server.handler.AbstractHandler;
30+
import org.junit.jupiter.api.Test;
3031

3132
import java.io.IOException;
3233
import java.nio.ByteBuffer;
34+
import java.nio.charset.Charset;
3335
import java.nio.charset.StandardCharsets;
3436
import java.util.Base64;
3537
import java.util.concurrent.ExecutionException;
@@ -70,6 +72,14 @@ private void ntlmAuthTest(Realm.Builder realmBuilder) throws IOException, Interr
7072
}
7173
}
7274

75+
@Test
76+
public void testUnicodeLittleUnmarkedEncoding() {
77+
final Charset unicodeLittleUnmarked = Charset.forName("UnicodeLittleUnmarked");
78+
final Charset utf16le = StandardCharsets.UTF_16LE;
79+
assertEquals(unicodeLittleUnmarked, utf16le);
80+
assertArrayEquals("Test @ テスト".getBytes(unicodeLittleUnmarked), "Test @ テスト".getBytes(utf16le));
81+
}
82+
7383
@RepeatedIfExceptionsTest(repeats = 5)
7484
public void lazyNTLMAuthTest() throws Exception {
7585
ntlmAuthTest(realmBuilderBase());

‎pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@
233233
-Xep:NullOptional:ERROR
234234
-XepExcludedPaths:.*/src/test/java/.*
235235
-XepOpt:NullAway:AnnotatedPackages=org.asynchttpclient
236-
-XepOpt:NullAway:UnannotatedSubPackages=org.asynchttpclient.netty,org.asynchttpclient.ntlm,org.asynchttpclient.request,org.asynchttpclient.ws
236+
-XepOpt:NullAway:UnannotatedSubPackages=org.asynchttpclient.netty,org.asynchttpclient.request,org.asynchttpclient.ws
237237
-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true
238238
-Xep:NullAway:ERROR
239239
</arg>

0 commit comments

Comments
 (0)