Add dependency-free "light" WinRM backend behind a runtime toggle - #109
Merged
Conversation
Introduce org.metricshub.winrm.light: a pure-JDK WinRM/WS-Management client that speaks NTLM (masqueraded as Negotiate) with message encryption over HTTP, using templated SOAP envelopes and the JDK-default XML factories. It carries no Apache CXF / JAX-WS / JAXB / Woodstox dependencies and is immune by construction to the JAXP ServiceLoader poisoning that breaks WinRM in some host apps (see MetricsHub/metricshub-community#1271). - LightWinRMService implements WindowsRemoteExecutor, so it is a drop-in for the CXF-based WinRMService (WQL + command execution, same return shapes/validation). - WsmanClient does the NTLM handshake + seal/sign, WQL Enumerate/Pull, and the full command shell lifecycle (Create/Command/Receive/Signal/Delete). The NTLM crypto is ported from the existing (proven) encryption classes; the one subtle fix vs a naive port is uppercasing the domain before NTOWFv2 (Apache NTCredentials does this and the Type 3 domain field is uppercased on the wire, so a lowercase domain passes auth but fails message integrity -> HTTP 400). - WinRMExecutorFactory selects the backend via -Dorg.metricshub.winrm.backend (default "cxf"; "light" opts in). WinRMWqlExecutor and WinRMCommandExecutor now depend on the WindowsRemoteExecutor interface, so callers are backend-agnostic. WindowsRemoteExecutor.close() is narrowed to declare no checked exception. Timeouts are enforced as a wall-clock deadline via Utils.execute (throwing TimeoutException) exactly like the CXF backend; the transport re-authenticates if the connection is dropped, and closes the socket on any I/O error. Verified against a real Windows Server 2008 R2 host over HTTP/5985: light and CXF backends return identical WQL data and command output/exit codes, with light ~5x faster (no CXF init). mvn verify: 35 tests pass, prettier/checkstyle/pmd clean. Part of the winrm-light roadmap (#103, #104); feature parity and the recorded- exchange test rig are tracked in #106 and #107. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1a19ef149
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…parsing - SmbTempShare now obtains its WinRM command executor via WinRMExecutorFactory instead of WinRMService directly, so command-with-file-copy honors -Dorg.metricshub.winrm.backend=light (the SMB transfer stays smbj, which is backend-independent; only the WSMan orchestration follows the toggle). The default (cxf) path is unchanged. - WsmanClient.parse() disables DOCTYPE/external-entity resolution (XXE hardening) before parsing WSMan responses, matching what CXF's DOMUtils already does. WSMan responses never carry a DOCTYPE, so rejecting it is safe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@codex Please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb266b89a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… race Address two P1 Codex findings on PR #109: - WsmanClient.decryptResponse now refuses any post-authentication response that is not multipart/encrypted. Over plaintext HTTP the NTLM seal is the only thing protecting response integrity, so a forged/unencrypted 200/500 from a proxy or on-path attacker must never be parsed as trusted SOAP. - WsmanClient.close no longer issues a graceful shell Delete while a request is still in flight (requestInFlight guard). On a command timeout, Utils.execute abandons the worker mid-Receive while try-with-resources calls close(); sending a second SOAP exchange over the same socket and stateful RC4 session would race the worker (cipher-sequence corruption, crossed responses, or a stall until the socket read timeout). Instead close() hard-closes the transport, unblocking the worker; the abandoned shell is reaped by the server IdleTimeout. - HttpTransport.close reads the socket into a local before closing so a concurrent close (main thread unblocking a worker's blocking read) cannot NPE on a check-then-use race. Verified vs anaxagore (light backend): WQL and command still succeed; a command that exceeds a 3s timeout now returns in 3s instead of stalling ~13s. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1da7d38df5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address three P2 Codex findings on PR #109: - WsmanClient now selects the Negotiate challenge across ALL WWW-Authenticate headers (order-independent) and tolerates challenges combined in one header, matching only the base64 token at a scheme boundary. Previously firstHeader() took only the first header and split on the first space, so a server/proxy that advertised another scheme first, or combined challenges, broke authentication. - NTLM protocol constants (the signing/sealing magic constants in WinRMSession and the multipart framing/boundary strings in NtlmCrypto) are now encoded with an explicit US-ASCII charset. Relying on the platform default charset could derive wrong keys or wrong framing on a JVM whose default charset is not ASCII-compatible. - HttpTransport.readChunked now consumes every trailer line up to the terminating empty line after the final chunk, instead of a single line. A chunked response carrying trailer fields previously left bytes in the kept-alive socket, desyncing the next request on the NTLM-bound connection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On this branch, the dependency-free light backend becomes the default WinRM backend; the CXF backend is reachable via -Dorg.metricshub.winrm.backend=cxf. CXF is intentionally NOT removed: it remains the parity baseline (differential testing) and the fallback for HTTPS/Kerberos until the light backend covers them, and is removed only as the final step before this branch merges to main. - WinRMExecutorFactory: default property value is now "light"; explicit value "cxf" selects WinRMService, everything else (incl. default) selects LightWinRMService. Javadoc/field comment updated. - LightWinRMService: the HTTPS and non-NTLM rejection messages now name the escape hatch (-Dorg.metricshub.winrm.backend=cxf), since these are now the default-path failures for those capabilities. - Tests WinRMWqlExecutorTest, WinRMCommandExecutorTest, SmbTempShareTest mocked WinRMService.createInstance, which only worked because the factory delegated to it under the cxf default. They now mock the seam production actually calls, WinRMExecutorFactory.createInstance (and return a WindowsRemoteExecutor), which also survives the eventual CXF removal. - WinRMExecutorFactoryTest: defaultBackendIsCxf -> defaultBackendIsLight; added cxfBackendSelectedViaProperty and defaultBackendRejectsHttps (the latter pins the intentional HTTPS regression so a silent fallback cannot be reintroduced). - README documents both backends and the toggle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@codex Please review once more |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbb0ad4430
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… scope Address three Codex findings on the light-as-default change (dbb0ad4): - P1: LightWinRMService.createInstance now rejects ANY authentication list that contains a scheme it cannot honour, even when NTLM is also present. The list is an ordered fallback, so accepting [KERBEROS, NTLM] silently ignored the preferred Kerberos (and ticketCache) and downgraded to NTLM — weaker, and failing against NTLM-disabled servers. It now fails toward the -Dorg.metricshub.winrm.backend=cxf escape hatch instead. - P2: the light backend no longer re-parses the endpoint with java.net.URI, whose getHost()/getPort() return null/-1 for NetBIOS names with underscores (and Unicode hosts) that WinRMEndpoint accepts — which left the default backend unable to reach hosts the CXF backend could. It now uses WinRMEndpoint.getProtocol()/getHostname() and a new WinRMEndpoint.getPort() accessor. - P2: WQL enumeration control elements (EndOfSequence, EnumerationContext) are now matched by the WS-Enumeration namespace instead of local name alone, so a WMI property named "EndOfSequence" inside <Items> can no longer end the enumeration early and truncate results. Tests: WinRMExecutorFactoryTest.lightBackendRejectsMixedKerberosNtlm; WinRMEndpointTest.testGetPort and testUnderscoreHostnameEndpoint. Live-verified vs anaxagore: 49-row WQL, single-row WQL, and command all succeed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df65e9787c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A single NTLM connection is a serial channel: one socket, stateful RC4 ciphers with sequence numbers, and one shellId. The prior requestInFlight flag only recorded activity; it did not exclude. When one executor is shared across threads (notably the cached SmbTempShare), concurrent callers could interleave on the socket and cipher streams, reading each other's responses or advancing the signing/sealing sequence out of order -> corrupted exchanges and checksum failures. Replace requestInFlight with a ReentrantLock held for the whole high-level operation (wql: Enumerate + all Pulls; executeCommand: Create + Command + Receive loop + Signal), so operations never interleave and the shared shellId stays consistent. close() uses tryLock (never lock): if it cannot acquire the lock an operation is in progress (e.g. a worker abandoned by a command timeout, still blocked on a socket read), so it skips the graceful shell Delete and hard-closes the transport, which unblocks that worker's read exactly as before. Verified vs anaxagore (light backend): 12 concurrent ops on one shared executor all succeed with no corruption; under aggressive load the only failures are clean, decryptable server quota faults (integrity intact), whereas the CXF backend under the same shared-executor load returns HTTP 400s. The command-timeout path still returns in ~3s (not ~13s), confirming close() still unblocks abandoned workers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@codex Please review once more |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40d54105ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…cket Socket.isClosed() only reflects LOCAL closure, so a WinRM server or intermediary that silently drops an idle keep-alive connection was invisible: isConnected() stayed true, the next operation reused the stale socket, and its write/read failed — losing the first command or query after an idle gap (most visibly for a cached SmbTempShare executor sitting between polling cycles). HttpTransport.isConnected() now proactively probes a reused connection with a 1ms blocking read once it has been idle past VALIDATE_AFTER_INACTIVITY_MS (1s): a healthy idle keep-alive has nothing to read and times out (alive), whereas EOF or unexpected bytes mean the peer closed it (stale) — in which case the socket is closed and isConnected() returns false. request() already resets the connection- bound NTLM session and reconnects when the transport is not connected, so the handshake re-runs transparently before sending; no request is lost. Chose proactive detection over retry-after-failure deliberately: retrying a failed send could double-execute a non-idempotent command if the drop happened after the server processed it, whereas detecting staleness before sending has no such risk. The 1s threshold keeps the probe from firing between the back-to-back requests of a single operation, so hot paths pay nothing. Verified vs anaxagore (light backend): a shared executor reused across 2s idle gaps succeeds every round (probe classifies the live connection as healthy and does not disturb its stream); a 49-row multi-Pull WQL is unaffected; the command-timeout path still returns in ~3s. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@codex please review once more |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c8af0b95e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tate guard (P2 review) Address three P2 Codex findings on PR #109: - WinRMExecutorFactory now accepts only "light" (default) and "cxf"; any other value (a typo like "cxff", or an unsupported future value) throws WinRMException instead of silently falling through to the light backend — which could run a different implementation than the operator requested and emit misleading escape-hatch hints. - WsmanClient converts the public long timeout to the int a Socket accepts via a clamped helper (min(millis, Integer.MAX_VALUE - 10_000)), so a large but valid timeout no longer narrows to a negative/garbage value, and HttpTransport's +10s read-timeout headroom cannot overflow int. The full long remains authoritative for the WSMan OperationTimeout and the service-level wall-clock deadline. - LightWinRMService tracks a closed flag: close() is idempotent (releases the connection once) and marks the executor closed, and executeWql/executeCommand now reject use after close with IllegalStateException instead of silently reviving the instance with a fresh handshake — matching the close() contract and the CXF path. Tests: WinRMExecutorFactoryTest.unsupportedBackendValueRejected and closedLightExecutorRejectsOperations. mvn verify 42 tests green; live-verified vs anaxagore (wql, cmd, and the unknown-backend rejection). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 23, 2026
Closed
The light backend now handles NTLM over HTTPS in addition to HTTP. - LightTls: the HTTPS socket factory. Validates by default — the JDK default SSLSocketFactory honors the platform trust store (and -Djavax.net.ssl.trustStore) and HttpTransport enables hostname verification. This is the opposite of the CXF path (which trusts every certificate; see #74). Opt out only via the system property org.metricshub.winrm.tls.insecure=true (trust-all + skip hostname check), for self-signed test hosts — insecure, not for production. - HttpTransport: when given an SSLSocketFactory it wraps the connection in TLS, sets endpointIdentificationAlgorithm=HTTPS (hostname verification) before connect, and forces startHandshake() so certificate failures surface immediately. The connect-failure cleanup now also catches RuntimeException so a TLS-setup failure cannot leak the socket. - WsmanClient: over HTTPS, WinRM exchanges PLAINTEXT SOAP inside TLS. So it authenticates NTLM WITHOUT negotiating SEAL (TYPE1_FLAGS_PLAIN) and marks the session authenticated without deriving RC4 keys; request() sends plaintext and decodeResponse() parses it directly. The "reject unencrypted response after auth" guard stays HTTP-only (over HTTP the seal is the sole integrity guard; over TLS the transport provides it). The HTTP path is byte-identical to before. - LightWinRMService no longer rejects HTTPS; it builds the TLS factory from the endpoint protocol. Build green (44 tests, incl. LightTlsTest and HTTPS acceptance in WinRMExecutorFactoryTest). A 14-agent adversarial review found no security, framing, or HTTP-regression issues (only the socket-leak nit fixed above). NOT yet live-verified end-to-end — pending a WinRM HTTPS listener to test against. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…r change) Preparation for Kerberos (#105): factor the mechanism-specific parts of WsmanClient behind a small package-private AuthScheme interface (authenticate / isAuthenticated / reset / wrap / wrapContentType / unwrap). WsmanClient is now mechanism-agnostic and just delegates; a new mechanism is added by implementing the interface rather than branching the client. - NtlmAuthScheme holds the existing NTLM logic verbatim (WinRMSession, Type1/2/3, NtlmCrypto, the HTTP-seal vs HTTPS-plaintext split, and the domain-uppercasing fix). Wire behavior is unchanged — the code moved, it did not change. - The shared Negotiate-token parser moved onto HttpTransport.Response.negotiateToken() (both NTLM and Kerberos ride under the Negotiate scheme). - WsmanClient's constructor now takes an AuthScheme; LightWinRMService builds the NtlmAuthScheme and injects it. Re-verified NTLM end-to-end after the move: HTTP/5985 against anaxagore (seal path) and HTTPS/5986 against a domain host (plaintext-over-TLS) both succeed (WQL + command). mvn verify 44 tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
KerberosAuthScheme authenticates with the JDK GSS-API only (no Apache/CXF): a JAAS Krb5LoginModule obtains a TGT from the username+password (or a ticket cache), then a GSSContext (SPNEGO) mints a service ticket for HTTP/<fqdn> and emits the AP-REQ under the Negotiate header. HTTPS only — like the CXF backend, which never implemented Kerberos message encryption over plain HTTP; the SOAP rides plaintext inside TLS, so wrap/unwrap are pass-throughs. Realm/KDC resolution is left to the ambient krb5 config (krb5.conf or -Djava.security.krb5.*), exactly as the CXF path did. LightWinRMService now resolves the requested AuthenticationEnum list into schemes in the caller's order: null/empty -> NTLM only; a single scheme is used directly; several become an ordered FallbackAuthScheme (e.g. [KERBEROS, NTLM] -> try Kerberos, fall back to NTLM). Kerberos is dropped from the candidate list over plain HTTP (unavailable there), so a mixed list over HTTP uses NTLM and a Kerberos-only request over HTTP fails toward the escape hatch. This supersedes the earlier hard-rejection of [KERBEROS, NTLM] (the user chose ordered fallback, since single-scheme-only would break the List API's backward compatibility). The ticketCache argument, previously dropped, is now used. Live-verified against tc-win2016 (SENTRY domain, Win2016, HTTPS/5986) with domain account sentry\dev-admin: Kerberos command + WQL succeed; and with a broken KDC the ordered [KERBEROS, NTLM] list falls back to NTLM and succeeds. mvn verify 45 tests green (incl. kerberosOverHttpsAccepted, mixedKerberosNtlmFallsBackToNtlmOverHttp). Fallback triggers on a client-side handshake failure (the common case: Kerberos unavailable/unconfigured). Falling back on a server-side rejection of an otherwise-valid ticket is not yet handled. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Locale, java.util.regex.Matcher and java.util.regex.Pattern became unused in WsmanClient once the NTLM orchestration (domain-uppercasing, Negotiate-token regex) moved into NtlmAuthScheme and HttpTransport.Response.negotiateToken(). The CI Checkstyle check treats UnusedImports as a failure (the verify-bound checkstyle:check only fails on error severity, so the local build missed it). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…iew) Address two findings from the Kerberos adversarial review. NTLM and Kerberos both send their Type 3 / AP-REQ on the FIRST real request, so a credential/token rejection surfaces as an HTTP 401 in WsmanClient.request() AFTER authenticate() has already set the scheme "authenticated" — not during the handshake. - Wedge (the serious one, affected NTLM too): a 401 on a connection we believed authenticated left isAuthenticated() true, so request() skipped re-auth and re-sent with a null Authorization header, 401-looping until the executor was recreated. request() now treats a 401 as a rejection: it drops the connection and resets the auth state, so the next operation cleanly re-handshakes instead of wedging. (The rejected request was not processed server-side, so re-sending is safe.) - Server-side fallback: an ordered [KERBEROS, NTLM] list only fell back when Kerberos failed client-side (no TGT/KDC). If the server rejected an otherwise-valid ticket (clock skew, channel-binding/CBT), NTLM was never tried. AuthScheme gains advance(); FallbackAuthScheme advances past a server-rejected scheme, and request() retries the next scheme once on a fresh connection after a 401. On a dropped connection it still re-handshakes with the same already-accepted scheme rather than restarting fallback. Tests: FallbackAuthSchemeTest (client-side fallback, advance() past a rejected scheme, all-fail). Live-verified unchanged: NTLM HTTP+HTTPS, Kerberos HTTPS, and client-side fallback all still succeed. mvn verify 48 tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@codex Please review! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 324154b29c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…(P2 review) Two P2 Codex findings: - FallbackAuthScheme no longer takes a shortcut that reused the previously-selected scheme on reconnect without falling through. It now always runs the candidate loop from startIndex (which still points at the last-successful scheme, so that one is retried first), so if its re-authentication fails — e.g. an expired TGT or a briefly unavailable KDC on a long-lived executor that had been using Kerberos — the loop falls through to the remaining candidates (NTLM) instead of throwing. - WsmanClient.close() now calls auth.reset() (under the same operation-lock guard as the graceful shell Delete) so a successfully authenticated Kerberos client disposes its GSSContext at teardown rather than leaking native GSS/security-context resources until GC. Skipped when the lock is not held (a timed-out worker still owns the auth scheme); the transport hard-close then unblocks it, preserving concurrent-close behavior. Test: FallbackAuthSchemeTest.reAuthFailureOfActiveSchemeFallsThrough. Live-verified unchanged: NTLM HTTP+HTTPS, Kerberos HTTPS, and client-side fallback all still succeed. mvn verify 49 tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The light backend is the default and validates TLS certificates by default, unlike the CXF client which trusted all certificates — so WinRM-over-HTTPS to self-signed hosts now fails unless the cert is trusted, TLS validation is disabled (-Dorg.metricshub.winrm.tls.insecure=true), or the CXF backend is selected (-Dorg.metricshub.winrm.backend=cxf). Added a prominent upgrade warning to README.md and the documentation site, a new CHANGELOG.md carrying the release notes, and refreshed the stale "WinRM backends" section (light now does NTLM over HTTP/HTTPS and Kerberos over HTTPS; CXF is opt-in and slated for removal). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@codex Please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 782604e963
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…e chars (P2 review) WSMan may split a multibyte character (e.g. UTF-8) across separate Stream elements or Receive responses. receiveLoop decoded each base64 chunk to a String independently, so a character straddling a chunk boundary was turned into replacement characters, permanently corrupting stdout/stderr. collectStreams now appends the raw decoded bytes per stream into a ByteArrayOutputStream, and receiveLoop decodes the accumulated bytes with the charset once, after the command completes. ASCII output is unaffected; a boundary-split multibyte character is now reassembled correctly. Live-verified unchanged: multi-chunk command output (ipconfig /all) assembles correctly over NTLM, and command execution over Kerberos/HTTPS still works. mvn verify 49 tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Authentication rejections now raise the same message as CXF:
'Authentication error on <endpoint> with user name "<user>"'.
- Operations on a closed executor raise CXF's exact IllegalStateException
message ('This instance has been closed and a new one must be created.').
- WSMan OperationTimeout is formatted exactly like CXF
(DecimalFormat PT#.###S, ROOT locale, millisecond precision).
- EndOfSequence and Items are recognized in both the WS-Enumeration and
the WSMan namespace variants (as CXF does), and in nothing else, so WMI
properties named like the markers cannot corrupt an enumeration.
- Fault summaries additionally carry the detailed WSManFault Message
(provider detail incl. WMI WBEM_E_* mnemonics) next to the SOAP reason
text; MetricsHub matches on those mnemonics to classify errors.
Live-verified against anaxagore (NTLM/HTTP, encrypted) and tc-win2016
(NTLM/HTTPS): ok-wql/ok-cmd identical, bad-class/bad-namespace carry the
same server fault text, bad-creds messages byte-identical across backends.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…107) - FakeWsmanServer (test scope): an in-process WSMan server speaking the real NTLM handshake — fixed server challenge, genuine NTLMv2 proof verification against a configured password, session-key recovery from the wire Type 3 — and real NTLM message encryption, by reusing the light client's crypto primitives through a mirrored WinRMSession (new package-private applyKeys(flags, exportedSessionKey, mirror) seam; the client path delegates to it unchanged). - WsmanProtocolTest: end-to-end CI tests for the full protocol path with no Windows host — NTLM-encrypted WQL Enumerate/Pull paging (both Items and EndOfSequence namespace variants), the command shell lifecycle (multibyte output split across Receive responses, operation-timeout Receive retry, shell-not-found tolerated on the terminate Signal), fault mapping incl. the WBEM_E_* detail, and the exact wrong-password message. Also pins the client's decrypted request bodies on the wire (OperationTimeout format, WINRS options, MaxElements, terminate code). - BackendDifferentialTest: documented one-command differential run (CXF vs light) against a real host, skipped unless winrm.diff.host is set. Live-verified against tc-win2016 (HTTPS/NTLM): WQL, command, and fault text all match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
….0.0) The light client (introduced in 1.x and proven at parity across #103-#107) becomes the sole implementation. The public API is unchanged. - Delete WinRMService and the service.client CXF internals (invocation handler, interceptors, the Apache-forked NTLM + encryption packages, KerberosUtils, TrustAllX509Manager), plus the dead duplicate copies in service/. Remove KerberosCredentialsException (CXF-only). - Drop the cxf / jaxws / jaxb / jaxws-rt dependencies and the cxf-codegen wsdl2java step; delete the WSDL/XSD/binding/catalog resources. Only smbj remains as a runtime dependency. Main jar: ~10.6 MB shaded -> 101 KB. - WinRMExecutorFactory keeps the org.metricshub.winrm.backend property but now rejects backend=cxf with a clear removal message (and any other value) instead of selecting a backend the operator did not ask for. - Replace the CXF-mocking tests: WinRMServiceTest, WinRMInvocationHandlerTest, KerberosUtilsTest, and CatalogResolutionTest are gone; the command test mocks WindowsRemoteExecutor instead of WinRMService; the differential BackendDifferentialTest is replaced by WinRMLiteTest -> WinRMLiveTest, a single-backend live smoke run. - Version 2.0.00-SNAPSHOT. README/site/CHANGELOG rewritten for the removal. 'mvn verify site' is green again (the javadoc build no longer trips over the CXF-generated sources). Live-smoke-verified against anaxagore (NTLM/HTTP): WQL + command pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces
org.metricshub.winrm.light— a pure-JDK WinRM/WS-Management client that speaks NTLM (masqueraded as Negotiate) with message encryption over HTTP, using templated SOAP envelopes and the JDK-default XML factories. It carries no Apache CXF / JAX-WS / JAXB / SAAJ / Woodstox dependencies, and is immune by construction to the JAXPServiceLoaderpoisoning that breaks WinRM in some host applications (MetricsHub/metricshub-community#1271).This is the productionization step of the winrm-light roadmap (#103 framework-free core, #104 NTLM message encryption over HTTP) — proven end-to-end against a real host, wired behind the existing public API with a backend toggle, and left default-off so nothing changes for current consumers until we choose to flip it.
What's here
LightWinRMService implements WindowsRemoteExecutor— a drop-in for the CXFWinRMService: sameexecuteWql/executeCommandbehaviour, return shapes, and argument validation.WsmanClient— the NTLM handshake + seal/sign, WQL Enumerate/Pull paging, and the full command shell lifecycle (Create → Command → Receive-loop with operation-timeout retry → Signal → Delete-on-close). NTLM crypto is ported from the existing, provenservice.client.encryption/auth.ntlmclasses.WinRMExecutorFactory— selects the backend via-Dorg.metricshub.winrm.backend(defaultcxf;lightopts in).WinRMWqlExecutorandWinRMCommandExecutornow depend on theWindowsRemoteExecutorinterface, so callers are backend-agnostic.WindowsRemoteExecutor.close()is narrowed to declare no checked exception.The one subtle bug worth calling out
A naive port fails with a bare HTTP 400 after authentication succeeds. Root cause: the domain must be uppercased before computing NTOWFv2. Apache's
NTCredentialsdoes this, andType3Message.buildMessagewrites the DomainName field uppercased on the wire — so the server derives its session key fromDOMAINwhile a lowercase client sealed withdomain. Auth passes (the NTProof validates) but message integrity fails.WsmanClientnow uppercases the domain; this was confirmed by the WinRM Operational log (Event 169 auth-OK + Event 140 error 5) and by flipping the single line to green.Robustness / parity (from an adversarial multi-dimension review)
Utils.execute(throwsTimeoutException) exactly like the CXF backend, bounding the Receive/Pull loops. Verified:ping -n 20with a 3 s timeout now throwsTimeoutExceptioninstead of hanging.Connection: close), and closes the socket on any I/O error so a failed connect can't wedge the client.EndOfSequence(matching CXF); WMI namespace case is preserved; parser errors are thrown rather than printed to stderr (clean for a monitoring agent).Verification
Against a real Windows Server 2008 R2 host over HTTP/5985 with NTLM message encryption, driven through the real public API (
WinRMWqlExecutor/WinRMCommandExecutor):Win32_OperatingSystemhostname,ver)dir Z:\nopecmd /c exit 7Light is ~5× faster (no CXF/JAXB init).
mvn verify: 35 tests pass, prettier/checkstyle/pmd clean. Diff is additive — the CXF path is untouched and remains the default.Not in this PR (tracked separately)
WinRMExecutorFactoryTestcovering backend selection and the light capability guards.🤖 Generated with Claude Code