diff --git a/CHANGELOG.md b/CHANGELOG.md index e77c21d..c9211ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,3 +96,8 @@ Consequences: WSMan namespace variants. WSMan fault exceptions additionally carry the detailed `WSManFault` message (including the provider-level detail, e.g. WMI `WBEM_E_*` mnemonics) alongside the SOAP reason text. +- Code quality: the PMD report is clean and `pmd:check` now runs in `mvn verify`, so a new violation + of `pmd.xml` fails the build (issue #122). The cleanup is behavior-preserving — redundant + parentheses and modifiers removed, empty catch blocks named and commented, and two loops + restructured. The only signature change is the removal of the unused `target` parameter from the + `CipherGen` constructor (an internal NTLM helper). diff --git a/pom.xml b/pom.xml index 5d5105e..0cc1716 100644 --- a/pom.xml +++ b/pom.xml @@ -203,6 +203,31 @@ + + + maven-pmd-plugin + 3.26.0 + + + verify + + check + + + + + ${maven.compiler.release} + + pmd.xml + + true + + + diff --git a/src/main/java/org/metricshub/winrm/ShareRemoteDirectoryConsumer.java b/src/main/java/org/metricshub/winrm/ShareRemoteDirectoryConsumer.java index cc52bae..9367f36 100644 --- a/src/main/java/org/metricshub/winrm/ShareRemoteDirectoryConsumer.java +++ b/src/main/java/org/metricshub/winrm/ShareRemoteDirectoryConsumer.java @@ -30,5 +30,5 @@ public interface ShareRemoteDirectoryConsumer#get() and reports this as unused. + @SuppressWarnings("PMD.UnusedPrivateMethod") private boolean matches(final byte[] content) { return digestHex("SHA256".equals(algorithm) ? "SHA-256" : "SHA-1", content).equals(digest); } diff --git a/src/main/java/org/metricshub/winrm/WindowsRemoteExecutor.java b/src/main/java/org/metricshub/winrm/WindowsRemoteExecutor.java index 671e343..a2ec572 100644 --- a/src/main/java/org/metricshub/winrm/WindowsRemoteExecutor.java +++ b/src/main/java/org/metricshub/winrm/WindowsRemoteExecutor.java @@ -41,7 +41,7 @@ public interface WindowsRemoteExecutor extends AutoCloseable { * @throws WqlQuerySyntaxException if WQL query syntax is invalid * @throws WindowsRemoteException For any problem encountered */ - public List> executeWql(final String wqlQuery, final long timeout) + List> executeWql(final String wqlQuery, final long timeout) throws TimeoutException, WqlQuerySyntaxException, WindowsRemoteException; /** @@ -55,7 +55,7 @@ public List> executeWql(final String wqlQuery, final long ti * @throws WindowsRemoteException For any problem encountered * @throws TimeoutException To notify userName of timeout. */ - public WindowsRemoteCommandResult executeCommand( + WindowsRemoteCommandResult executeCommand( final String command, final String workingDirectory, final Charset charset, @@ -67,21 +67,21 @@ public WindowsRemoteCommandResult executeCommand( * * @return */ - public String getHostname(); + String getHostname(); /** * Get the username. * * @return */ - public String getUsername(); + String getUsername(); /** * Get the password. * * @return */ - public char[] getPassword(); + char[] getPassword(); /** * Close the executor and release its resources. Narrows {@link AutoCloseable#close()} so it does @@ -89,5 +89,5 @@ public WindowsRemoteCommandResult executeCommand( * {@link Exception}. */ @Override - public void close(); + void close(); } diff --git a/src/main/java/org/metricshub/winrm/WmiHelper.java b/src/main/java/org/metricshub/winrm/WmiHelper.java index 7db501c..a912998 100644 --- a/src/main/java/org/metricshub/winrm/WmiHelper.java +++ b/src/main/java/org/metricshub/winrm/WmiHelper.java @@ -74,7 +74,7 @@ public static String createNetworkResource(final String hostname, final String n */ public static boolean isLocalNetworkResource(final String networkResource) { Utils.checkNonNull(networkResource, "networkResource"); - return (!networkResource.startsWith("\\\\") + return !networkResource.startsWith("\\\\") || networkResource.startsWith("\\\\localhost\\") || @@ -86,7 +86,7 @@ public static boolean isLocalNetworkResource(final String networkResource) { || networkResource.startsWith("\\\\0000:0000:0000:0000:0000:0000:0000:0001\\") || - networkResource.toLowerCase().startsWith("\\\\" + Utils.getComputerName().toLowerCase() + "\\")); + networkResource.toLowerCase().startsWith("\\\\" + Utils.getComputerName().toLowerCase() + "\\"); } /** diff --git a/src/main/java/org/metricshub/winrm/command/WinRMCommandExecutor.java b/src/main/java/org/metricshub/winrm/command/WinRMCommandExecutor.java index f518142..22e12e9 100644 --- a/src/main/java/org/metricshub/winrm/command/WinRMCommandExecutor.java +++ b/src/main/java/org/metricshub/winrm/command/WinRMCommandExecutor.java @@ -101,7 +101,7 @@ public static WindowsRemoteCommandResult execute( : localFileToCopyList.stream().filter(Utils::isNotBlank).collect(Collectors.toList()); try ( - final WindowsRemoteExecutor winRMService = WinRMExecutorFactory.createInstance( + WindowsRemoteExecutor winRMService = WinRMExecutorFactory.createInstance( winRMEndpoint, timeout, ticketCache, diff --git a/src/main/java/org/metricshub/winrm/light/ByteArrayUtils.java b/src/main/java/org/metricshub/winrm/light/ByteArrayUtils.java index 54345a7..01c852a 100644 --- a/src/main/java/org/metricshub/winrm/light/ByteArrayUtils.java +++ b/src/main/java/org/metricshub/winrm/light/ByteArrayUtils.java @@ -92,7 +92,7 @@ public static long readLittleEndianUnsignedInt(final byte[] input, final int off } public static byte[] concat(final byte[]... sequences) { - try (final ByteArrayOutputStream out = new ByteArrayOutputStream()) { + try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { for (byte[] s : sequences) { out.write(s); } diff --git a/src/main/java/org/metricshub/winrm/light/CipherGen.java b/src/main/java/org/metricshub/winrm/light/CipherGen.java index 2bd0dc0..dbf90c8 100644 --- a/src/main/java/org/metricshub/winrm/light/CipherGen.java +++ b/src/main/java/org/metricshub/winrm/light/CipherGen.java @@ -75,7 +75,6 @@ public CipherGen( final String user, final String password, final byte[] challenge, - final String target, final byte[] targetInformation ) { this.random = random; diff --git a/src/main/java/org/metricshub/winrm/light/Envelopes.java b/src/main/java/org/metricshub/winrm/light/Envelopes.java index 00b3483..9e67dcb 100644 --- a/src/main/java/org/metricshub/winrm/light/Envelopes.java +++ b/src/main/java/org/metricshub/winrm/light/Envelopes.java @@ -59,7 +59,7 @@ private Envelopes() {} // --- WQL --------------------------------------------------------------- static String enumerateWql(final String url, final String namespace, final String wql, final long timeoutMs) { - return (envelopeOpen(false) + + return envelopeOpen(false) + header(url, wmiResourceUri(namespace), ACTION_ENUMERATE, timeoutMs, null, null) + "" + "" + @@ -67,18 +67,18 @@ static String enumerateWql(final String url, final String namespace, final Strin "" + escape(wql) + "" + - ""); + ""; } static String pull(final String url, final String namespace, final String context, final long timeoutMs) { - return (envelopeOpen(false) + + return envelopeOpen(false) + header(url, wmiResourceUri(namespace), ACTION_PULL, timeoutMs, null, null) + "" + "" + escape(context) + "" + "32000" + - ""); + ""; } // --- Command shell ----------------------------------------------------- @@ -91,13 +91,13 @@ static String createShell(final String url, final String workingDirectory, final final String workingDir = (workingDirectory == null || workingDirectory.trim().isEmpty()) ? "" : "" + escape(workingDirectory) + ""; - return (envelopeOpen(true) + + return envelopeOpen(true) + header(url, SHELL_RESOURCE_URI, ACTION_CREATE, timeoutMs, null, optionSet) + "" + "stdin" + "stdout stderr" + workingDir + - ""); + ""; } static String command(final String url, final String shellId, final String commandLine, final long timeoutMs) { @@ -105,35 +105,35 @@ static String command(final String url, final String shellId, final String comma "TRUE" + "FALSE" + ""; - return (envelopeOpen(true) + + return envelopeOpen(true) + header(url, SHELL_RESOURCE_URI, ACTION_COMMAND, timeoutMs, shellSelector(shellId), optionSet) + "" + escape(commandLine) + - ""); + ""; } static String receive(final String url, final String shellId, final String commandId, final long timeoutMs) { - return (envelopeOpen(true) + + return envelopeOpen(true) + header(url, SHELL_RESOURCE_URI, ACTION_RECEIVE, timeoutMs, shellSelector(shellId), null) + "stdout stderr"); + "\">stdout stderr"; } static String signal(final String url, final String shellId, final String commandId, final long timeoutMs) { - return (envelopeOpen(true) + + return envelopeOpen(true) + header(url, SHELL_RESOURCE_URI, ACTION_SIGNAL, timeoutMs, shellSelector(shellId), null) + "" + TERMINATE_CODE + - ""); + ""; } static String deleteShell(final String url, final String shellId, final long timeoutMs) { - return (envelopeOpen(true) + + return envelopeOpen(true) + header(url, SHELL_RESOURCE_URI, ACTION_DELETE, timeoutMs, shellSelector(shellId), null) + - ""); + ""; } // --- helpers ----------------------------------------------------------- @@ -143,8 +143,8 @@ private static String wmiResourceUri(final String namespace) { } private static String shellSelector(final String shellId) { - return ("" + escape(shellId) - + ""); + return "" + escape(shellId) + + ""; } /** @@ -165,7 +165,7 @@ private static String header( final String selectorSet, final String optionSet ) { - return ("" + + return "" + "" + url + "" + @@ -190,11 +190,11 @@ private static String header( "" + operationTimeout(timeoutMs) + "" + - ""); + ""; } private static String envelopeOpen(final boolean shell) { - return (""); + ">"; } private static String escape(final String s) { diff --git a/src/main/java/org/metricshub/winrm/light/HttpTransport.java b/src/main/java/org/metricshub/winrm/light/HttpTransport.java index 580dc98..9b5a10d 100644 --- a/src/main/java/org/metricshub/winrm/light/HttpTransport.java +++ b/src/main/java/org/metricshub/winrm/light/HttpTransport.java @@ -178,7 +178,7 @@ private boolean isStalePeerClosed() { } finally { try { socket.setSoTimeout(previousTimeout); - } catch (final IOException ignore) { + } catch (final IOException ignored) { // socket is being discarded on the stale path anyway } } @@ -218,7 +218,7 @@ private void ensureConnected() throws IOException { // cannot leak the freshly created SSLSocket. try { newSocket.close(); - } catch (final IOException ignore) { + } catch (final IOException ignored) { // best effort } socket = null; @@ -355,9 +355,10 @@ private byte[] readChunked() throws IOException { if (size == 0) { // After the terminating chunk come zero or more optional trailer fields, then a final // empty line. Consume them all, or leftover bytes desync the kept-alive NTLM socket. - String trailer; - while ((trailer = readLine()) != null && !trailer.isEmpty()) { - // discard trailer field + String trailer = readLine(); + while (trailer != null && !trailer.isEmpty()) { + // discard the trailer field and look at the next line + trailer = readLine(); } break; } @@ -379,7 +380,7 @@ public void close() { if (doomed != null) { try { doomed.close(); - } catch (final IOException ignore) { + } catch (final IOException ignored) { // best effort } } diff --git a/src/main/java/org/metricshub/winrm/light/KerberosAuthScheme.java b/src/main/java/org/metricshub/winrm/light/KerberosAuthScheme.java index 06de531..3ab3469 100644 --- a/src/main/java/org/metricshub/winrm/light/KerberosAuthScheme.java +++ b/src/main/java/org/metricshub/winrm/light/KerberosAuthScheme.java @@ -120,7 +120,7 @@ public void reset() { if (context != null) { try { context.dispose(); - } catch (final GSSException ignore) { + } catch (final GSSException ignored) { // disposing a dead context is best-effort } context = null; diff --git a/src/main/java/org/metricshub/winrm/light/MD4.java b/src/main/java/org/metricshub/winrm/light/MD4.java index a1dd746..44e8e42 100644 --- a/src/main/java/org/metricshub/winrm/light/MD4.java +++ b/src/main/java/org/metricshub/winrm/light/MD4.java @@ -126,84 +126,84 @@ private void processBuffer() { } private void round1(final int[] d) { - a = rotintlft((a + f(b, c, this.d) + d[0]), 3); - this.d = rotintlft((this.d + f(a, b, c) + d[1]), 7); - c = rotintlft((c + f(this.d, a, b) + d[2]), 11); - b = rotintlft((b + f(c, this.d, a) + d[3]), 19); - - a = rotintlft((a + f(b, c, this.d) + d[4]), 3); - this.d = rotintlft((this.d + f(a, b, c) + d[5]), 7); - c = rotintlft((c + f(this.d, a, b) + d[6]), 11); - b = rotintlft((b + f(c, this.d, a) + d[7]), 19); - - a = rotintlft((a + f(b, c, this.d) + d[8]), 3); - this.d = rotintlft((this.d + f(a, b, c) + d[9]), 7); - c = rotintlft((c + f(this.d, a, b) + d[10]), 11); - b = rotintlft((b + f(c, this.d, a) + d[11]), 19); - - a = rotintlft((a + f(b, c, this.d) + d[12]), 3); - this.d = rotintlft((this.d + f(a, b, c) + d[13]), 7); - c = rotintlft((c + f(this.d, a, b) + d[14]), 11); - b = rotintlft((b + f(c, this.d, a) + d[15]), 19); + a = rotintlft(a + f(b, c, this.d) + d[0], 3); + this.d = rotintlft(this.d + f(a, b, c) + d[1], 7); + c = rotintlft(c + f(this.d, a, b) + d[2], 11); + b = rotintlft(b + f(c, this.d, a) + d[3], 19); + + a = rotintlft(a + f(b, c, this.d) + d[4], 3); + this.d = rotintlft(this.d + f(a, b, c) + d[5], 7); + c = rotintlft(c + f(this.d, a, b) + d[6], 11); + b = rotintlft(b + f(c, this.d, a) + d[7], 19); + + a = rotintlft(a + f(b, c, this.d) + d[8], 3); + this.d = rotintlft(this.d + f(a, b, c) + d[9], 7); + c = rotintlft(c + f(this.d, a, b) + d[10], 11); + b = rotintlft(b + f(c, this.d, a) + d[11], 19); + + a = rotintlft(a + f(b, c, this.d) + d[12], 3); + this.d = rotintlft(this.d + f(a, b, c) + d[13], 7); + c = rotintlft(c + f(this.d, a, b) + d[14], 11); + b = rotintlft(b + f(c, this.d, a) + d[15], 19); } private void round2(final int[] d) { - a = rotintlft((a + g(b, c, this.d) + d[0] + 0x5a827999), 3); - this.d = rotintlft((this.d + g(a, b, c) + d[4] + 0x5a827999), 5); - c = rotintlft((c + g(this.d, a, b) + d[8] + 0x5a827999), 9); - b = rotintlft((b + g(c, this.d, a) + d[12] + 0x5a827999), 13); - - a = rotintlft((a + g(b, c, this.d) + d[1] + 0x5a827999), 3); - this.d = rotintlft((this.d + g(a, b, c) + d[5] + 0x5a827999), 5); - c = rotintlft((c + g(this.d, a, b) + d[9] + 0x5a827999), 9); - b = rotintlft((b + g(c, this.d, a) + d[13] + 0x5a827999), 13); - - a = rotintlft((a + g(b, c, this.d) + d[2] + 0x5a827999), 3); - this.d = rotintlft((this.d + g(a, b, c) + d[6] + 0x5a827999), 5); - c = rotintlft((c + g(this.d, a, b) + d[10] + 0x5a827999), 9); - b = rotintlft((b + g(c, this.d, a) + d[14] + 0x5a827999), 13); - - a = rotintlft((a + g(b, c, this.d) + d[3] + 0x5a827999), 3); - this.d = rotintlft((this.d + g(a, b, c) + d[7] + 0x5a827999), 5); - c = rotintlft((c + g(this.d, a, b) + d[11] + 0x5a827999), 9); - b = rotintlft((b + g(c, this.d, a) + d[15] + 0x5a827999), 13); + a = rotintlft(a + g(b, c, this.d) + d[0] + 0x5a827999, 3); + this.d = rotintlft(this.d + g(a, b, c) + d[4] + 0x5a827999, 5); + c = rotintlft(c + g(this.d, a, b) + d[8] + 0x5a827999, 9); + b = rotintlft(b + g(c, this.d, a) + d[12] + 0x5a827999, 13); + + a = rotintlft(a + g(b, c, this.d) + d[1] + 0x5a827999, 3); + this.d = rotintlft(this.d + g(a, b, c) + d[5] + 0x5a827999, 5); + c = rotintlft(c + g(this.d, a, b) + d[9] + 0x5a827999, 9); + b = rotintlft(b + g(c, this.d, a) + d[13] + 0x5a827999, 13); + + a = rotintlft(a + g(b, c, this.d) + d[2] + 0x5a827999, 3); + this.d = rotintlft(this.d + g(a, b, c) + d[6] + 0x5a827999, 5); + c = rotintlft(c + g(this.d, a, b) + d[10] + 0x5a827999, 9); + b = rotintlft(b + g(c, this.d, a) + d[14] + 0x5a827999, 13); + + a = rotintlft(a + g(b, c, this.d) + d[3] + 0x5a827999, 3); + this.d = rotintlft(this.d + g(a, b, c) + d[7] + 0x5a827999, 5); + c = rotintlft(c + g(this.d, a, b) + d[11] + 0x5a827999, 9); + b = rotintlft(b + g(c, this.d, a) + d[15] + 0x5a827999, 13); } private void round3(final int[] d) { - a = rotintlft((a + h(b, c, this.d) + d[0] + 0x6ed9eba1), 3); - this.d = rotintlft((this.d + h(a, b, c) + d[8] + 0x6ed9eba1), 9); - c = rotintlft((c + h(this.d, a, b) + d[4] + 0x6ed9eba1), 11); - b = rotintlft((b + h(c, this.d, a) + d[12] + 0x6ed9eba1), 15); - - a = rotintlft((a + h(b, c, this.d) + d[2] + 0x6ed9eba1), 3); - this.d = rotintlft((this.d + h(a, b, c) + d[10] + 0x6ed9eba1), 9); - c = rotintlft((c + h(this.d, a, b) + d[6] + 0x6ed9eba1), 11); - b = rotintlft((b + h(c, this.d, a) + d[14] + 0x6ed9eba1), 15); - - a = rotintlft((a + h(b, c, this.d) + d[1] + 0x6ed9eba1), 3); - this.d = rotintlft((this.d + h(a, b, c) + d[9] + 0x6ed9eba1), 9); - c = rotintlft((c + h(this.d, a, b) + d[5] + 0x6ed9eba1), 11); - b = rotintlft((b + h(c, this.d, a) + d[13] + 0x6ed9eba1), 15); - - a = rotintlft((a + h(b, c, this.d) + d[3] + 0x6ed9eba1), 3); - this.d = rotintlft((this.d + h(a, b, c) + d[11] + 0x6ed9eba1), 9); - c = rotintlft((c + h(this.d, a, b) + d[7] + 0x6ed9eba1), 11); - b = rotintlft((b + h(c, this.d, a) + d[15] + 0x6ed9eba1), 15); + a = rotintlft(a + h(b, c, this.d) + d[0] + 0x6ed9eba1, 3); + this.d = rotintlft(this.d + h(a, b, c) + d[8] + 0x6ed9eba1, 9); + c = rotintlft(c + h(this.d, a, b) + d[4] + 0x6ed9eba1, 11); + b = rotintlft(b + h(c, this.d, a) + d[12] + 0x6ed9eba1, 15); + + a = rotintlft(a + h(b, c, this.d) + d[2] + 0x6ed9eba1, 3); + this.d = rotintlft(this.d + h(a, b, c) + d[10] + 0x6ed9eba1, 9); + c = rotintlft(c + h(this.d, a, b) + d[6] + 0x6ed9eba1, 11); + b = rotintlft(b + h(c, this.d, a) + d[14] + 0x6ed9eba1, 15); + + a = rotintlft(a + h(b, c, this.d) + d[1] + 0x6ed9eba1, 3); + this.d = rotintlft(this.d + h(a, b, c) + d[9] + 0x6ed9eba1, 9); + c = rotintlft(c + h(this.d, a, b) + d[5] + 0x6ed9eba1, 11); + b = rotintlft(b + h(c, this.d, a) + d[13] + 0x6ed9eba1, 15); + + a = rotintlft(a + h(b, c, this.d) + d[3] + 0x6ed9eba1, 3); + this.d = rotintlft(this.d + h(a, b, c) + d[11] + 0x6ed9eba1, 9); + c = rotintlft(c + h(this.d, a, b) + d[7] + 0x6ed9eba1, 11); + b = rotintlft(b + h(c, this.d, a) + d[15] + 0x6ed9eba1, 15); } private static int f(final int x, final int y, final int z) { - return ((x & y) | (~x & z)); + return (x & y) | (~x & z); } private static int g(final int x, final int y, final int z) { - return ((x & y) | (x & z) | (y & z)); + return (x & y) | (x & z) | (y & z); } private static int h(final int x, final int y, final int z) { - return (x ^ y ^ z); + return x ^ y ^ z; } private static int rotintlft(final int val, final int numbits) { - return ((val << numbits) | (val >>> (32 - numbits))); + return (val << numbits) | (val >>> (32 - numbits)); } } diff --git a/src/main/java/org/metricshub/winrm/light/NTLMMessage.java b/src/main/java/org/metricshub/winrm/light/NTLMMessage.java index b96f426..b5cdc1f 100644 --- a/src/main/java/org/metricshub/winrm/light/NTLMMessage.java +++ b/src/main/java/org/metricshub/winrm/light/NTLMMessage.java @@ -82,13 +82,13 @@ static int readULong(final byte[] src, final int index) { if (src.length < index + 4) { return 0; } - return ((src[index] & 0xff) + return (src[index] & 0xff) | ((src[index + 1] & 0xff) << 8) | ((src[index + 2] & 0xff) << 16) | - ((src[index + 3] & 0xff) << 24)); + ((src[index + 3] & 0xff) << 24); } /** diff --git a/src/main/java/org/metricshub/winrm/light/NtlmCrypto.java b/src/main/java/org/metricshub/winrm/light/NtlmCrypto.java index ba83041..5754027 100644 --- a/src/main/java/org/metricshub/winrm/light/NtlmCrypto.java +++ b/src/main/java/org/metricshub/winrm/light/NtlmCrypto.java @@ -41,7 +41,7 @@ final class NtlmCrypto { private NtlmCrypto() {} static byte[] encryptAndSign(final WinRMSession session, final byte[] messageBody) { - try (final ByteArrayOutputStream out = new ByteArrayOutputStream()) { + try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { out.write(BOUNDARY_CR.getBytes(StandardCharsets.US_ASCII)); out.write("\tContent-Type: application/HTTP-SPNEGO-session-encrypted\r\n".getBytes(StandardCharsets.US_ASCII)); out.write( @@ -181,19 +181,26 @@ void skipOver(final String s) { void skipUntil(final String s) { final byte[] expected = s.getBytes(StandardCharsets.US_ASCII); int next = index; - outer: while (true) { - for (int i = 0; i < expected.length; i++) { - if (next + i >= bytes.length) { - throw new IllegalStateException("Encrypted-response framing terminated early looking for delimiter"); - } - if (expected[i] != bytes[next + i]) { - next++; - continue outer; - } + while (!matchesAt(next, expected)) { + next++; + } + index = next + expected.length; + } + + /** + * Whether {@code expected} occurs at the given offset, throwing when the remaining bytes are + * too few to hold it — i.e. the delimiter is missing from the response altogether. + */ + private boolean matchesAt(final int offset, final byte[] expected) { + for (int i = 0; i < expected.length; i++) { + if (offset + i >= bytes.length) { + throw new IllegalStateException("Encrypted-response framing terminated early looking for delimiter"); + } + if (expected[i] != bytes[offset + i]) { + return false; } - index = next + expected.length; - return; } + return true; } } } diff --git a/src/main/java/org/metricshub/winrm/light/Type1Message.java b/src/main/java/org/metricshub/winrm/light/Type1Message.java index e74f48d..8fdcf01 100644 --- a/src/main/java/org/metricshub/winrm/light/Type1Message.java +++ b/src/main/java/org/metricshub/winrm/light/Type1Message.java @@ -50,8 +50,8 @@ class Type1Message extends NTLMMessage { } static int getDefaultFlags() { - return ( // Required flags - NTLMEngineUtils.FLAG_REQUEST_NTLM_V1 + // Required flags + return NTLMEngineUtils.FLAG_REQUEST_NTLM_V1 | NTLMEngineUtils.FLAG_REQUEST_NTLM2_SESSION | @@ -63,7 +63,7 @@ static int getDefaultFlags() { | NTLMEngineUtils.FLAG_REQUEST_56BIT_ENCRYPTION | - NTLMEngineUtils.FLAG_REQUEST_UNICODE_ENCODING); + NTLMEngineUtils.FLAG_REQUEST_UNICODE_ENCODING; } /** diff --git a/src/main/java/org/metricshub/winrm/light/Type3Message.java b/src/main/java/org/metricshub/winrm/light/Type3Message.java index 0af4499..d805a48 100644 --- a/src/main/java/org/metricshub/winrm/light/Type3Message.java +++ b/src/main/java/org/metricshub/winrm/light/Type3Message.java @@ -39,7 +39,10 @@ public class Type3Message extends NTLMMessage { java.security.SecureRandom rnd = null; try { rnd = java.security.SecureRandom.getInstance("SHA1PRNG"); - } catch (final Exception ignore) {} + } catch (final Exception ignored) { + // SHA1PRNG is not guaranteed to be present: leave RND_GEN null and let the constructor + // reject the authentication attempt with a clear NtlmException. + } RND_GEN = rnd; } @@ -93,7 +96,6 @@ public class Type3Message extends NTLMMessage { user, password, nonce, - target, responseTargetInformation ); diff --git a/src/main/java/org/metricshub/winrm/light/WsmanClient.java b/src/main/java/org/metricshub/winrm/light/WsmanClient.java index a7a4bb6..0aed8a0 100644 --- a/src/main/java/org/metricshub/winrm/light/WsmanClient.java +++ b/src/main/java/org/metricshub/winrm/light/WsmanClient.java @@ -261,7 +261,9 @@ private Decoded request(final String soap) throws Exception { auth.reset(); } final byte[] body = soap.getBytes(StandardCharsets.UTF_8); - while (true) { + // Stays null while the loop retries the next authentication scheme on a fresh connection. + Decoded decoded = null; + while (decoded == null) { if (!auth.isAuthenticated()) { pendingAuthorization = auth.authenticate(transport); } @@ -301,8 +303,9 @@ private Decoded request(final String soap) throws Exception { if (resp.status != 200 && resp.status != 500) { throw new IllegalStateException("WSMan request failed: HTTP " + resp.status); } - return new Decoded(resp.status, parse(auth.unwrap(resp))); + decoded = new Decoded(resp.status, parse(auth.unwrap(resp))); } + return decoded; } // --- XML helpers -------------------------------------------------------- @@ -350,9 +353,9 @@ private static String text(final Document doc, final String localName) { * backend does. */ static boolean hasEnumerationElement(final Document doc, final String localName) { - return (doc.getElementsByTagNameNS(WS_ENUMERATION_NS, localName).getLength() > 0 + return doc.getElementsByTagNameNS(WS_ENUMERATION_NS, localName).getLength() > 0 || - doc.getElementsByTagNameNS(WSMAN_NS, localName).getLength() > 0); + doc.getElementsByTagNameNS(WSMAN_NS, localName).getLength() > 0; } /** First text content of an element matched by both namespace and local name. */ @@ -365,11 +368,11 @@ static void collectItems(final Document doc, final List> row // The Items wrapper comes in the WS-Enumeration namespace (EnumerateResponse) or the WSMan // namespace (PullResponse) depending on the operation; accept both, like the CXF backend, and // nothing else — a WMI property or class named "Items" must not be mistaken for the wrapper. - collectItems(doc.getElementsByTagNameNS(WS_ENUMERATION_NS, "Items"), rows); - collectItems(doc.getElementsByTagNameNS(WSMAN_NS, "Items"), rows); + collectRows(doc.getElementsByTagNameNS(WS_ENUMERATION_NS, "Items"), rows); + collectRows(doc.getElementsByTagNameNS(WSMAN_NS, "Items"), rows); } - private static void collectItems(final NodeList items, final List> rows) { + private static void collectRows(final NodeList items, final List> rows) { for (int i = 0; i < items.getLength(); i++) { final NodeList instances = items.item(i).getChildNodes(); for (int j = 0; j < instances.getLength(); j++) { @@ -494,7 +497,7 @@ public void close() { if (shell != null) { try { request(Envelopes.deleteShell(url, shell, timeoutMs)); - } catch (final Exception ignore) { + } catch (final Exception ignored) { // best-effort shell cleanup } } diff --git a/src/main/java/org/metricshub/winrm/service/WinRMEndpoint.java b/src/main/java/org/metricshub/winrm/service/WinRMEndpoint.java index e4d9b55..7c73bd1 100644 --- a/src/main/java/org/metricshub/winrm/service/WinRMEndpoint.java +++ b/src/main/java/org/metricshub/winrm/service/WinRMEndpoint.java @@ -220,13 +220,13 @@ public boolean equals(final Object obj) { return false; } final WinRMEndpoint other = (WinRMEndpoint) obj; - return (Objects.equals(endpoint, other.endpoint) + return Objects.equals(endpoint, other.endpoint) && Objects.equals(namespace, other.namespace) && Arrays.equals(password, other.password) && - Objects.equals(rawUsername, other.rawUsername)); + Objects.equals(rawUsername, other.rawUsername); } @Override diff --git a/src/main/java/org/metricshub/winrm/wql/WinRMWqlExecutor.java b/src/main/java/org/metricshub/winrm/wql/WinRMWqlExecutor.java index f2f9896..6031032 100644 --- a/src/main/java/org/metricshub/winrm/wql/WinRMWqlExecutor.java +++ b/src/main/java/org/metricshub/winrm/wql/WinRMWqlExecutor.java @@ -120,7 +120,7 @@ public static WinRMWqlExecutor executeWql( final WinRMEndpoint winRMEndpoint = new WinRMEndpoint(protocol, hostname, port, username, password, namespace); try ( - final WindowsRemoteExecutor winRMService = WinRMExecutorFactory.createInstance( + WindowsRemoteExecutor winRMService = WinRMExecutorFactory.createInstance( winRMEndpoint, timeout, ticketCache, diff --git a/src/test/java/org/metricshub/winrm/light/FakeWsmanServer.java b/src/test/java/org/metricshub/winrm/light/FakeWsmanServer.java index 3b50799..16fb878 100644 --- a/src/test/java/org/metricshub/winrm/light/FakeWsmanServer.java +++ b/src/test/java/org/metricshub/winrm/light/FakeWsmanServer.java @@ -81,6 +81,7 @@ static final class Scripted { private final Deque script = new ArrayDeque<>(); private final List decryptedRequests = new CopyOnWriteArrayList<>(); private volatile boolean closed; + private volatile boolean chunkedResponses; FakeWsmanServer(final String domain, final String user, final String password) throws IOException { this.expectedDomain = domain.toUpperCase(Locale.ROOT); @@ -104,6 +105,17 @@ FakeWsmanServer enqueue(final int status, final String soapBody) { return this; } + /** + * Serve the scripted bodies with {@code Transfer-Encoding: chunked} — several chunks, a chunk + * extension, and trailer fields after the terminating chunk — instead of {@code Content-Length}, + * like a real WinRM host does. A client that mis-reads the framing (e.g. leaves the trailers in + * the socket) desyncs the kept-alive connection and fails on the NEXT request. + */ + FakeWsmanServer withChunkedResponses() { + chunkedResponses = true; + return this; + } + /** The plaintext SOAP request bodies received so far, in order (after decryption). */ List decryptedRequests() { return new ArrayList<>(decryptedRequests); @@ -334,7 +346,7 @@ private static void writeSecurityBuffer(final ByteArrayOutputStream out, final i // --- minimal HTTP ----------------------------------------------------------- - private static void respond( + private void respond( final OutputStream out, final int status, final String extraHeader, @@ -342,6 +354,7 @@ private static void respond( final byte[] body ) throws IOException { final byte[] payload = body == null ? new byte[0] : body; + final boolean chunked = chunkedResponses && payload.length > 0; final StringBuilder head = new StringBuilder(); head.append("HTTP/1.1 ").append(status).append(' ').append(status == 200 ? "OK" : "Error").append("\r\n"); head.append("Server: FakeWsmanServer\r\n"); @@ -351,13 +364,39 @@ private static void respond( if (contentType != null) { head.append("Content-Type: ").append(contentType).append("\r\n"); } - head.append("Content-Length: ").append(payload.length).append("\r\n"); + head.append(chunked ? "Transfer-Encoding: chunked\r\n" : "Content-Length: " + payload.length + "\r\n"); head.append("\r\n"); out.write(head.toString().getBytes(StandardCharsets.ISO_8859_1)); - out.write(payload); + if (chunked) { + writeChunkedBody(out, payload); + } else { + out.write(payload); + } out.flush(); } + /** Write the payload as two chunks (the first with a chunk extension) plus a trailer field. */ + private static void writeChunkedBody(final OutputStream out, final byte[] payload) throws IOException { + final int split = Math.max(1, payload.length / 2); + writeChunk(out, payload, 0, split, ";boundary=middle"); + if (split < payload.length) { + writeChunk(out, payload, split, payload.length - split, ""); + } + out.write("0\r\nX-Fake-Trailer: done\r\n\r\n".getBytes(StandardCharsets.ISO_8859_1)); + } + + private static void writeChunk( + final OutputStream out, + final byte[] payload, + final int offset, + final int length, + final String extension + ) throws IOException { + out.write((Integer.toHexString(length) + extension + "\r\n").getBytes(StandardCharsets.ISO_8859_1)); + out.write(payload, offset, length); + out.write("\r\n".getBytes(StandardCharsets.ISO_8859_1)); + } + /** One parsed HTTP request: headers (lower-cased names) and the raw body. */ private static final class HttpRequest { diff --git a/src/test/java/org/metricshub/winrm/light/WsmanProtocolTest.java b/src/test/java/org/metricshub/winrm/light/WsmanProtocolTest.java index 7ad934c..555edf2 100644 --- a/src/test/java/org/metricshub/winrm/light/WsmanProtocolTest.java +++ b/src/test/java/org/metricshub/winrm/light/WsmanProtocolTest.java @@ -132,6 +132,58 @@ void wqlPagesAcrossEnumerateAndPullsOverEncryptedNtlm() throws Exception { assertTrue(requests.get(2).contains("uuid:CTX-2"), requests.get(2)); } + @Test + void wqlPagesOverChunkedResponsesWithTrailers() throws Exception { + // Real WinRM hosts answer with Transfer-Encoding: chunked. The client must reassemble the + // chunks AND consume the trailer fields that follow the terminating chunk — leftover trailer + // bytes desync the kept-alive NTLM connection, so the SECOND request on it is what fails. + server + .withChunkedResponses() + .enqueue( + 200, + envelope( + "" + + "uuid:CTX-1" + + "" + + service("Spooler", "Running") + + "" + + "" + ) + ) + .enqueue( + 200, + envelope( + "" + + "" + + service("WinRM", "Running") + + "" + + "" + + "" + ) + ); + + try (LightWinRMService service = client(PASSWORD)) { + final List> rows = service.executeWql("SELECT Name,State FROM Win32_Service", TIMEOUT); + + assertEquals(2, rows.size()); + assertEquals("Spooler", rows.get(0).get("Name")); + assertEquals("WinRM", rows.get(1).get("Name")); + } + + // The Pull was answered on the same connection: proof the trailers were fully drained. + final List requests = server.decryptedRequests(); + assertEquals(2, requests.size(), () -> String.join("\n---\n", requests)); + assertTrue(requests.get(1).contains("uuid:CTX-1"), requests.get(1)); + } + // --- Command shell lifecycle ------------------------------------------------ @Test