Add HTTP Basic authentication as a third auth scheme #169
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,145 @@ | ||
| package org.metricshub.winrm.light; | ||
|
|
||
| /*- | ||
| * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ | ||
| * WinRM Java Client | ||
| * ჻჻჻჻჻჻ | ||
| * Copyright (C) 2023 - 2026 MetricsHub | ||
| * ჻჻჻჻჻჻ | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| * ╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱ | ||
| */ | ||
|
|
||
| import java.nio.ByteBuffer; | ||
| import java.nio.CharBuffer; | ||
| import java.nio.charset.CoderResult; | ||
| import java.nio.charset.CharsetEncoder; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.util.Arrays; | ||
| import java.util.Base64; | ||
|
|
||
| /** | ||
| * HTTP Basic authentication scheme. The credential (base64 of {@code user:password}) rides the | ||
| * {@code Authorization} header of EVERY request, so there is no stateful handshake and no message | ||
| * protection — the payload travels as plaintext SOAP. Confidentiality therefore relies on the | ||
| * transport: over HTTPS (TLS) the credential and the SOAP are protected; over plain HTTP they are | ||
| * sent in the clear and must not be used. | ||
| * <p> | ||
| * The caller's {@code char[]} password is kept only as a reference (never copied into a | ||
| * {@code String}), exactly like the NTLM scheme, so the caller remains the single owner of the | ||
| * secret and can wipe it after {@code close()}. The derived {@code Authorization} header is held | ||
| * as a wipeable {@code byte[]}, because Base64 is reversible: it is erased on | ||
| * {@link #reset()}. {@code close()} erases it directly when it can acquire the connection permit; | ||
| * when an in-flight operation or streaming handle still holds the permit (a timed-out worker, for | ||
| * example) the last operation to release the connection erases it instead, so the credential | ||
| * cannot survive the close either way. | ||
| */ | ||
| final class BasicAuthScheme extends PlaintextSoapAuthScheme { | ||
|
|
||
| // The full "Basic <base64>" header, held wipeable: it is a reversible copy of the credential, | ||
| // so it must not outlive the caller's own password array (erased in reset()). volatile: reset() | ||
| // can be called from another thread (close() or a worker releasing the connection) while a | ||
| // worker is reading the header. | ||
| private volatile byte[] authorization; | ||
| private final String username; | ||
| private final char[] password; | ||
|
|
||
| /** | ||
| * @param username the account name (a domain-qualified name keeps its domain prefix, which is | ||
| * how the server locates the account). The caller passes an already whitespace-stripped | ||
| * account (the service rebuilds it from the endpoint's normalized domain/username parts), | ||
| * so it is used verbatim here. | ||
| * @param password the account password, kept as {@code char[]} so the caller owns the single | ||
| * wipeable copy of the secret | ||
| */ | ||
| BasicAuthScheme(final String username, final char[] password) { | ||
| this.username = username; | ||
| this.password = password; | ||
| this.authorization = buildAuthorizationHeader(username, password); | ||
| } | ||
|
|
||
| /** | ||
| * Build the full {@code Authorization} header value, encoding the password to UTF-8 | ||
| * straight from the caller's {@code char[]} (a {@link CharBuffer} view) — the secret is | ||
| * never copied into a {@code String}, per the credentials contract. | ||
| * | ||
| * @param username the account name (may be domain-qualified) | ||
| * @param password the account password | ||
| * @return the ASCII bytes of {@code Basic <base64(user:password)>} | ||
| */ | ||
| private static byte[] buildAuthorizationHeader(final String username, final char[] password) { | ||
| final byte[] user = username.getBytes(StandardCharsets.UTF_8); | ||
| final byte[] secret = new byte[password.length * 3]; // UTF-8 never exceeds 3 bytes/char | ||
| final ByteBuffer secretBuffer = ByteBuffer.wrap(secret); | ||
| final CharsetEncoder encoder = StandardCharsets.UTF_8.newEncoder(); | ||
| final CoderResult result = encoder.encode(CharBuffer.wrap(password), secretBuffer, true); | ||
| // A reported error can only be a malformed-input one: an unpaired surrogate in the password. | ||
| if (result.isError()) { | ||
| throw new IllegalArgumentException("The password contains unpaired surrogate characters"); | ||
| } | ||
| encoder.flush(secretBuffer); | ||
| final int secretLength = secretBuffer.position(); | ||
| final byte[] raw = new byte[user.length + 1 + secretLength]; | ||
| System.arraycopy(user, 0, raw, 0, user.length); | ||
| raw[user.length] = (byte) ':'; | ||
| System.arraycopy(secret, 0, raw, user.length + 1, secretLength); | ||
| Arrays.fill(secret, (byte) 0); | ||
| final byte[] base64 = Base64.getEncoder().encode(raw); | ||
| Arrays.fill(raw, (byte) 0); | ||
| final byte[] header = new byte["Basic ".length() + base64.length]; | ||
| System.arraycopy("Basic ".getBytes(StandardCharsets.US_ASCII), 0, header, 0, "Basic ".length()); | ||
| System.arraycopy(base64, 0, header, "Basic ".length(), base64.length); | ||
| Arrays.fill(base64, (byte) 0); | ||
| return header; | ||
| } | ||
|
|
||
| @Override | ||
| public String authenticate(final HttpTransport transport) throws Exception { | ||
| // No handshake: Basic has no server challenge. Mark the connection authenticated so the | ||
| // client proceeds straight to the first real request (which carries the Authorization header). | ||
| // A reset() when the connection dropped may have erased the credential, so re-derive it | ||
| // from the caller's still-live password array (the same behavior as the NTLM scheme, which | ||
| // keeps the password by reference to re-handshake a dropped connection). | ||
| if (authorization == null) { | ||
| authorization = buildAuthorizationHeader(username, password); | ||
| } | ||
|
Comment on lines
+113
to
+115
There was a problem hiding this comment.
When Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Acknowledged, and I agree with your own framing here, so I'm leaving the code as-is rather than adding a guard. The mechanism is real in theory, but it needs two things to coincide, and the second is the disqualifier:
(2) is a caller leaking an AutoCloseable handle, which we can't protect against in general: any abandoned handle retains whatever state it holds, for any scheme. We deliberately don't build a leak-detection layer for that. On the cases we can cover, we already do: close() resets the auth state unconditionally in its finally (so the credential is erased the moment the client closes, including when an idle or in-flight handle holds the permit), and releaseConnection() has the post-release backstop for the post-close re-authentication that the Kerberos no-socket-I/O path can produce. The only residual is the abandoned-handle case above, and only for Basic, which is a testing-only scheme (disabled on the WinRM service by default), not a default transport. So I'm not adding the post-auth closed gate to send() — it would be another layer on the same close-race for a scenario that also requires the caller to drop an auto-closeable handle, and I'd rather keep send() lean. If you'd prefer defense-in-depth there anyway, I'm happy to add the |
||
| authenticated = true; | ||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public String requestAuthorization() { | ||
| // Stateless: the same header repeats on every request, not just the first. | ||
| if (authorization == null) { | ||
| throw new IllegalStateException("The Basic credential was erased before the connection was re-authenticated"); | ||
| } | ||
| return new String(authorization, StandardCharsets.US_ASCII); | ||
|
There was a problem hiding this comment.
The new Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. You're right that the per-request
Happy to file that as a follow-up if the stronger guarantee is wanted. |
||
| } | ||
|
|
||
| @Override | ||
| public void reset() { | ||
| // Erase the Base64 credential — a reversible copy of the password — so that wiping the | ||
| // caller's char[] afterward leaves no live copy of the password. Called by close() when the | ||
| // connection permit is acquirable, and by WsmanClient when the last in-flight operation | ||
| // releases the connection after close() could not acquire the permit (a timed-out worker). | ||
| // The two callers are different threads with no shared lock, so this must be re-entrant and | ||
| // idempotent: capture the reference in a local so a concurrent reset() cannot null the field | ||
| // between the null-check and the fill (filling a twice-erased array is harmless). | ||
| authenticated = false; | ||
| final byte[] header = authorization; | ||
| if (header != null) { | ||
| Arrays.fill(header, (byte) 0); | ||
| authorization = null; | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new wipe in
reset()does not cover a Basic scheme that never becomes active: with an ordered list such as[NTLM, BASIC], this constructor eagerly derives the reversible credential, butFallbackAuthScheme.reset()resets only the active NTLM candidate. If NTLM succeeds and the closed client remains referenced, the inactive Basic candidate therefore retainsauthorizationafter the caller wipes the password; derive it lazily or dispose every fallback candidate when closing.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in e307dcd — I took your second suggested option:
FallbackAuthScheme.reset()now walks all candidates rather than just the active one, so a Basic scheme that never became active (e.g.[NTLM, BASIC]where NTLM wins) has its derived credential erased onclose()and on a dropped connection.startIndexis left untouched, so the fallback order survives and the nextauthenticate()still retries the last-accepted scheme first. New regression test:FallbackAuthSchemeTest.resetClearsEveryCandidateNotJustTheActiveOneasserts both candidates are reset and that the first scheme is the one retried.