diff --git a/README.md b/README.md index fc72d87..e7dcde3 100644 --- a/README.md +++ b/README.md @@ -166,3 +166,7 @@ Before merging this dependent PR, merge networknt/light-aws-lambda#168. Resolve Fetch each sibling repository’s master and verify the selected commit is its ancestor with `git merge-base --is-ancestor origin/master`. Replace the corresponding values in `ci/jackson3-source-refs.json` with those full 40-character merge SHAs in a reviewed dependent change. Keep the remaining closure pins and the qualified core/policy SHAs unchanged. Run fresh source-build CI on that dependent commit and review its resolved-source artifact before merging. A squash or merge commit must be qualified even when its runtime tree matches the earlier pin. For later sibling updates after this PR merges, use the same manifest-update PR and fresh dependent CI procedure. Never silently advance pins or infer qualification from the sibling’s own green build. + +## Basic Auth credential hardening + +See [A20/A25 assessment and credential ownership](doc/security/basic-auth-credentials.md) for the scoped Basic Auth hardening and remaining memory/logging limits. diff --git a/doc/security/basic-auth-credentials.md b/doc/security/basic-auth-credentials.md new file mode 100644 index 0000000..7be77c1 --- /dev/null +++ b/doc/security/basic-auth-credentials.md @@ -0,0 +1,56 @@ +# Basic Auth credential handling (A20 / A25) + +[Issue #191](https://github.com/networknt/light-lambda-native/issues/191) covers +partner Checkmarx A25 rows 90–97 and A20 row 98. + +## Re-evaluation + +At master `b6d27a7d42ecdc94efe387acdd1cb4b3f47a7ce7` and 2.x +`785235d5324697958a573cfa6ad6e664615e19ab`, the reported A25 password/header +logging paths were already mitigated: authentication error messages and LDAP +dispatch traces omit submitted credentials. The scoped fix therefore addresses +A20, with regression coverage protecting A25. Denied-path diagnostics still +include the configured username and sanitized path; this is not a promise to +remove all identity data from logs. + +## Ownership and cleanup + +Local Basic authentication no longer creates an immutable decoded credential +pair or password String. A request-local closeable owns the decoded bytes and +password bytes and clears them after authentication/authorization, including +early returns and exceptions. The shared scheme check scans characters without creating a credential substring. +Temporary header/payload copies and configured +password comparison bytes are also cleared. Local comparison continues to use +`MessageDigest.isEqual`. Existing raw `username:password` compatibility, Base64 +decoding, Unicode passwords, colons in passwords, status codes, reserved-user +rules and LDAP dispatch are retained. The scheme is removed at a character +boundary, preserving the existing case-insensitive Unicode scheme matching. +Malformed UTF-8 in Base64-decoded credentials retains the historical UTF-8 +replacement behavior for local comparison and LDAP; normalization uses temporary +character/byte buffers cleared after use, without an immutable credential String. Conversion uses explicit encoder/decoder +output buffers sized from their advertised maximum expansion, rather than the +convenience charset methods that can resize and discard an earlier buffer. +Every supplied conversion array is cleared in a finally block, including unused +capacity; returned byte copies remain owned by their existing cleanup scopes. + +The protected `handleLdapAuth(UserAuth, String)` extension and the LDAP library +still require a String. Only that branch constructs a password String; it cannot +be zeroed. No borrowed request header or configured password is modified. +The Lambda request, configuration, decoder/JVM internals and downstream LDAP +implementation may retain secrets. Clearing owned arrays reduces extra retention +and does not guarantee complete heap erasure or establish a remote exploit. +Protect request/access/audit logs, memory dumps, diagnostics and transport. + +## Verification boundaries + +Focused JVM tests exercise the existing Basic authentication contract, owned +buffer cleanup for successful/malformed/empty/exceptional scopes and through the +real middleware's return and LDAP-exception paths, and synthetic +local/LDAP outcomes at TRACE. Log checks inspect messages, argument arrays, MDC +and throwable causes/suppressed exceptions; returned status content is checked +for canaries as well. Distinct password prefix/suffix fragments and the Unicode +suffix are checked in addition to full encoded credentials. Negative controls +verify these checks catch partial logging and omitted middleware cleanup. +LDAP is a local test double. No live LDAP bind, AWS +deployment, native-image rebuild or partner rescan is implied by these tests. +Deployment requires rebuilding and distributing the native Lambda artifact. diff --git a/src/main/java/com/networknt/aws/lambda/handler/middleware/security/AuthorizationScheme.java b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/AuthorizationScheme.java index 5b035b8..db73930 100644 --- a/src/main/java/com/networknt/aws/lambda/handler/middleware/security/AuthorizationScheme.java +++ b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/AuthorizationScheme.java @@ -17,7 +17,15 @@ static AuthorizationScheme parse(String header) { } static boolean hasCredentials(String header, AuthorizationScheme scheme) { - return scheme != UNKNOWN && parse(header) == scheme && header.length() > scheme.name().length() + 1 - && !header.substring(scheme.name().length() + 1).trim().isEmpty(); + if (scheme == UNKNOWN || parse(header) != scheme) return false; + return hasTrimmedContent(header, scheme.name().length() + 1); + } + + // Match String.trim() semantics without copying credential material into another String. + static boolean hasTrimmedContent(String header, int start) { + for (int i = start; i < header.length(); i++) { + if (header.charAt(i) > ' ') return true; + } + return false; } } diff --git a/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthCredentials.java b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthCredentials.java new file mode 100644 index 0000000..8672800 --- /dev/null +++ b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthCredentials.java @@ -0,0 +1,115 @@ +package com.networknt.aws.lambda.handler.middleware.security; + +import org.apache.commons.codec.binary.Base64; + +import java.security.MessageDigest; +import java.nio.ByteBuffer; +import java.nio.CharBuffer; +import java.nio.charset.CoderResult; +import java.nio.charset.CodingErrorAction; +import java.util.Arrays; + +import static java.nio.charset.StandardCharsets.UTF_8; + +/** Request-local, owned copies only; the original request and configuration remain borrowed. */ +final class BasicAuthCredentials implements AutoCloseable { + private final byte[] decoded; + private final byte[] password; + private final int separator; + + private BasicAuthCredentials(byte[] decoded) { + this.decoded = decoded; + separator = separator(decoded); + password = separator < 0 ? new byte[0] : Arrays.copyOfRange(decoded, separator + 1, decoded.length); + } + + static BasicAuthCredentials parse(String auth) { + byte[] payload = encodeUtf8(CharBuffer.wrap(auth, 6, auth.length())); + byte[] decoded = null; + try { + // Preserve the existing raw username:password compatibility and lenient Base64 decoder. + if (separator(payload) >= 0) { + decoded = payload; + } else { + decoded = Base64.decodeBase64(payload); + byte[] normalized = normalizeUtf8(decoded); + Arrays.fill(decoded, (byte) 0); + decoded = normalized; + } + BasicAuthCredentials credentials = new BasicAuthCredentials(decoded); + if (decoded == payload) payload = null; + decoded = null; // Ownership moves to the closeable credentials. + return credentials; + } finally { + if (payload != null) Arrays.fill(payload, (byte) 0); + if (decoded != null) Arrays.fill(decoded, (byte) 0); + } + } + + // Caller-supplied output prevents convenience charset APIs from discarding grown buffers. + private static byte[] encodeUtf8(CharBuffer characters) { + var encoder = UTF_8.newEncoder() + .onMalformedInput(CodingErrorAction.REPLACE) + .onUnmappableCharacter(CodingErrorAction.REPLACE); + int capacity = Math.multiplyExact(characters.remaining(), (int) Math.ceil(encoder.maxBytesPerChar())); + byte[] bytes = new byte[capacity]; + try { + ByteBuffer output = ByteBuffer.wrap(bytes); + requireUnderflow(encoder.encode(characters, output, true)); + requireUnderflow(encoder.flush(output)); + return Arrays.copyOf(bytes, output.position()); + } finally { + Arrays.fill(bytes, (byte) 0); + } + } + + // Preserve the former new String(decoded, UTF_8) replacement semantics without a String. + private static byte[] normalizeUtf8(byte[] decoded) { + var decoder = UTF_8.newDecoder() + .onMalformedInput(CodingErrorAction.REPLACE) + .onUnmappableCharacter(CodingErrorAction.REPLACE); + int capacity = Math.multiplyExact(decoded.length, (int) Math.ceil(decoder.maxCharsPerByte())); + char[] characters = new char[capacity]; + try { + CharBuffer output = CharBuffer.wrap(characters); + requireUnderflow(decoder.decode(ByteBuffer.wrap(decoded), output, true)); + requireUnderflow(decoder.flush(output)); + output.flip(); + return encodeUtf8(output); + } finally { + Arrays.fill(characters, '\0'); + } + } + + private static void requireUnderflow(CoderResult result) { + if (!result.isUnderflow()) throw new IllegalStateException("Unexpected UTF-8 conversion result"); + } + + private static int separator(byte[] bytes) { + for (int i = 0; i < bytes.length; i++) if (bytes[i] == ':') return i; + return -1; + } + + boolean hasSeparator() { return separator >= 0; } + boolean isPasswordEmpty() { return password.length == 0; } + String username() { return new String(decoded, 0, separator, UTF_8); } + + boolean matches(String configuredPassword) { + if (configuredPassword == null) return false; + byte[] expected = encodeUtf8(CharBuffer.wrap(configuredPassword)); + try { + return MessageDigest.isEqual(password, expected); + } finally { + Arrays.fill(expected, (byte) 0); + } + } + + // The existing LDAP API/extension requires a String; only this branch creates it. + String ldapPassword() { return new String(password, UTF_8); } + + @Override + public void close() { + Arrays.fill(decoded, (byte) 0); + Arrays.fill(password, (byte) 0); + } +} diff --git a/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddleware.java b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddleware.java index 949fb66..23e9a99 100644 --- a/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddleware.java +++ b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddleware.java @@ -13,13 +13,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.security.MessageDigest; import java.util.HashMap; import java.util.List; import java.util.Optional; -import static java.nio.charset.StandardCharsets.UTF_8; - public class BasicAuthMiddleware implements MiddlewareHandler { private static final Logger LOG = LoggerFactory.getLogger(BasicAuthMiddleware.class); static final String BEARER_PREFIX = "BEARER"; @@ -63,7 +60,7 @@ public Status execute(LightLambdaExchange exchange) { } else { /* contains auth header */ String auth = optionalAuth.get(); - if(auth.trim().isEmpty()) { + if (!AuthorizationScheme.hasTrimmedContent(auth, 0)) { return this.handleAnonymousAuth(exchange, requestPath, config); } AuthorizationScheme scheme = AuthorizationScheme.parse(auth); @@ -148,18 +145,15 @@ public Status handleBasicAuth(LightLambdaExchange exchange, String requestPath, LOG.error("Invalid/Unsupported Basic authorization header."); return new Status(INVALID_AUTHORIZATION_HEADER, BASIC_PREFIX); } - String credentials = auth.substring(6); - int pos = credentials.indexOf(':'); - if (pos == -1) { - credentials = new String(org.apache.commons.codec.binary.Base64.decodeBase64(credentials), UTF_8); - } - pos = credentials.indexOf(':'); - if (pos != -1) { - String username = credentials.substring(0, pos); - String password = credentials.substring(pos + 1); + try (BasicAuthCredentials credentials = parseCredentials(auth)) { + if (!credentials.hasSeparator()) { + LOG.error("Invalid basic authentication header. It must be username:password base64 encode."); + return new Status(INVALID_BASIC_HEADER, BASIC_PREFIX); + } + String username = credentials.username(); UserAuth user = config.getUsers().get(username); // Reserved users describe authorization policies, never Basic identities. - if (user == null || StringUtils.isEmpty(password) + if (user == null || credentials.isPasswordEmpty() || BasicAuthConfig.ANONYMOUS.equals(username) || BasicAuthConfig.BEARER.equals(username)) { LOG.error("Invalid Basic username or password."); if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); @@ -168,14 +162,13 @@ public Status handleBasicAuth(LightLambdaExchange exchange, String requestPath, if (StringUtils.isEmpty(user.getPassword()) && config.isEnableAD()) { // Delegate configured identities without a local password to LDAP. if(LOG.isTraceEnabled()) LOG.trace("Call LdapUtil for Basic authentication."); - if (!handleLdapAuth(user, password)) { + if (!handleLdapAuth(user, credentials.ldapPassword())) { if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); return new Status(INVALID_USERNAME_OR_PASSWORD); } } else { // Compare the configured local password without early character mismatches. - if (!(user.getPassword() != null - && MessageDigest.isEqual(password.getBytes(UTF_8), user.getPassword().getBytes(UTF_8)))) { + if (!credentials.matches(user.getPassword())) { LOG.error("Invalid Basic username or password."); if (LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); return new Status(INVALID_USERNAME_OR_PASSWORD); @@ -196,14 +189,15 @@ public Status handleBasicAuth(LightLambdaExchange exchange, String requestPath, if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); return new Status(NOT_AUTHORIZED_REQUEST_PATH, safePath, user.getUsername()); } - } else { - LOG.error("Invalid basic authentication header. It must be username:password base64 encode."); - if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); - return new Status(INVALID_BASIC_HEADER, BASIC_PREFIX); } return successMiddlewareStatus(); } + // Package-local boundary lets lifecycle tests observe buffers used by the real middleware. + BasicAuthCredentials parseCredentials(String auth) { + return BasicAuthCredentials.parse(auth); + } + /** * Handle LDAP authentication and authorization * @param user diff --git a/src/test/java/com/networknt/aws/lambda/handler/middleware/security/AuthorizationSchemeCredentialsTest.java b/src/test/java/com/networknt/aws/lambda/handler/middleware/security/AuthorizationSchemeCredentialsTest.java new file mode 100644 index 0000000..001f517 --- /dev/null +++ b/src/test/java/com/networknt/aws/lambda/handler/middleware/security/AuthorizationSchemeCredentialsTest.java @@ -0,0 +1,25 @@ +package com.networknt.aws.lambda.handler.middleware.security; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +class AuthorizationSchemeCredentialsTest { + @Test + void credentialPresencePreservesSchemeAndTrimSemantics() { + for (AuthorizationScheme scheme : new AuthorizationScheme[]{AuthorizationScheme.BASIC, AuthorizationScheme.BEARER}) { + assertFalse(AuthorizationScheme.hasCredentials(null, scheme)); + assertFalse(AuthorizationScheme.hasCredentials(scheme.name(), scheme)); + assertFalse(AuthorizationScheme.hasCredentials(scheme.name() + "Secret", scheme)); + for (char whitespace = 0; whitespace <= ' '; whitespace++) { + String prefix = scheme.name() + " " + whitespace; + assertFalse(AuthorizationScheme.hasCredentials(prefix, scheme)); + assertTrue(AuthorizationScheme.hasCredentials(prefix + "credential", scheme)); + } + assertTrue(AuthorizationScheme.hasCredentials(scheme.name().toLowerCase(java.util.Locale.ROOT) + " x", scheme)); + assertTrue(AuthorizationScheme.hasCredentials(scheme.name() + " \u00a0", scheme)); + } + assertFalse(AuthorizationScheme.hasCredentials("Basic x", AuthorizationScheme.BEARER)); + assertFalse(AuthorizationScheme.hasCredentials("Basic x", AuthorizationScheme.UNKNOWN)); + } +} diff --git a/src/test/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthCredentialsTest.java b/src/test/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthCredentialsTest.java new file mode 100644 index 0000000..b1c25f4 --- /dev/null +++ b/src/test/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthCredentialsTest.java @@ -0,0 +1,126 @@ +package com.networknt.aws.lambda.handler.middleware.security; + +import org.junit.jupiter.api.Test; + +import java.util.Base64; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.*; + +class BasicAuthCredentialsTest { + @Test + void preservesRawAndEncodedUnicodeCredentialsAndClearsOwnedBuffers() throws Exception { + String pair = "user:secret-ë:extra"; + for (String payload : new String[]{pair, Base64.getEncoder().encodeToString(pair.getBytes(UTF_8))}) { + BasicAuthCredentials credentials = BasicAuthCredentials.parse("Basic " + payload); + byte[] decoded = buffer(credentials, "decoded"); + byte[] password = buffer(credentials, "password"); + try (credentials) { + assertTrue(credentials.hasSeparator()); + assertEquals("user", credentials.username()); + assertFalse(credentials.isPasswordEmpty()); + assertTrue(credentials.matches("secret-ë:extra")); + assertFalse(credentials.matches("secret-ë")); + assertFalse(credentials.matches(null)); + assertEquals("secret-ë:extra", credentials.ldapPassword()); + } + assertZero(decoded); + assertZero(password); + credentials.close(); + assertZero(password); + assertEquals("user:secret-ë:extra", pair); // Borrowed immutable inputs are not erased. + } + } + + @Test + void clearsMalformedEmptyAndExceptionalScopes() throws Exception { + for (String payload : new String[]{"no-colon", "user:", "user:secret"}) { + String header = "Basic " + Base64.getEncoder().encodeToString(payload.getBytes(UTF_8)); + BasicAuthCredentials credentials = BasicAuthCredentials.parse(header); + byte[] decoded = buffer(credentials, "decoded"); + byte[] password = buffer(credentials, "password"); + assertThrows(IllegalStateException.class, () -> { + try (credentials) { + assertEquals(payload.contains(":"), credentials.hasSeparator()); + assertEquals(!payload.endsWith("secret"), credentials.isPasswordEmpty()); + throw new IllegalStateException("synthetic failure"); + } + }); + assertZero(decoded); + assertZero(password); + } + } + + @Test + void schemeIsRemovedAtCharacterBoundaryIncludingUnicodeCaseVariants() { + for (String scheme : new String[]{"Basic", "bAsIc", "Bas\u0131c", "Bas\u0130c", "Ba\u017fic"}) { + for (String payload : new String[]{"user:password", Base64.getEncoder().encodeToString("user:password".getBytes(UTF_8))}) { + assertEquals(AuthorizationScheme.BASIC, AuthorizationScheme.parse(scheme + " " + payload)); + try (var credentials = BasicAuthCredentials.parse(scheme + " " + payload)) { + assertEquals("user", credentials.username()); + assertTrue(credentials.matches("password")); + } + } + } + } + + @Test + void malformedUtf8RetainsHistoricalReplacementForLocalComparisonAndLdap() { + for (byte[] malformed : new byte[][]{{(byte) 0xc3, '('}, {(byte) 0xe2}, {(byte) 0xff}}) { + byte[] prefix = "user:".getBytes(UTF_8); + byte[] pair = java.util.Arrays.copyOf(prefix, prefix.length + malformed.length); + System.arraycopy(malformed, 0, pair, prefix.length, malformed.length); + String previousPassword = new String(malformed, UTF_8); + try (var credentials = BasicAuthCredentials.parse("Basic " + Base64.getEncoder().encodeToString(pair))) { + assertEquals("user", credentials.username()); + assertTrue(credentials.matches(previousPassword)); + assertEquals(previousPassword, credentials.ldapPassword()); + assertFalse(credentials.matches("incorrect")); + } + } + } + + @Test + void explicitlySizedConversionHandlesMixedWidthUnicodeWithoutOverflow() throws Exception { + String password = "aé漢🙂:".repeat(1024); + String pair = "user:" + password; + for (String payload : new String[]{pair, Base64.getEncoder().encodeToString(pair.getBytes(UTF_8))}) { + BasicAuthCredentials credentials = BasicAuthCredentials.parse("Basic " + payload); + byte[] decoded = buffer(credentials, "decoded"); + byte[] secret = buffer(credentials, "password"); + try (credentials) { + assertEquals("user", credentials.username()); + assertTrue(credentials.matches(password)); + assertEquals(password, credentials.ldapPassword()); + } + assertZero(decoded); + assertZero(secret); + } + } + + @Test + void explicitlySizedNormalizationHandlesWorstCaseReplacementExpansion() throws Exception { + byte[] prefix = "user:".getBytes(UTF_8); + byte[] pair = java.util.Arrays.copyOf(prefix, prefix.length + 1024); + java.util.Arrays.fill(pair, prefix.length, pair.length, (byte) 0xff); + BasicAuthCredentials credentials = BasicAuthCredentials.parse("Basic " + Base64.getEncoder().encodeToString(pair)); + byte[] decoded = buffer(credentials, "decoded"); + byte[] secret = buffer(credentials, "password"); + try (credentials) { + assertTrue(credentials.matches("\uFFFD".repeat(1024))); + assertEquals("\uFFFD".repeat(1024), credentials.ldapPassword()); + } + assertZero(decoded); + assertZero(secret); + } + + private static byte[] buffer(BasicAuthCredentials credentials, String name) throws Exception { + var field = BasicAuthCredentials.class.getDeclaredField(name); + field.setAccessible(true); + return (byte[]) field.get(credentials); + } + + private static void assertZero(byte[] buffer) { + for (byte value : buffer) assertEquals(0, value, "Owned credential buffer was not cleared"); + } +} diff --git a/src/test/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddlewareLifecycleTest.java b/src/test/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddlewareLifecycleTest.java new file mode 100644 index 0000000..bdf3ffd --- /dev/null +++ b/src/test/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddlewareLifecycleTest.java @@ -0,0 +1,110 @@ +package com.networknt.aws.lambda.handler.middleware.security; + +import com.networknt.aws.lambda.LambdaContext; +import com.networknt.aws.lambda.LightLambdaExchange; +import com.networknt.aws.lambda.TestUtils; +import com.networknt.basicauth.UserAuth; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.Base64; +import java.util.List; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.*; + +class BasicAuthMiddlewareLifecycleTest { + private static final String SECRET = "q7V9p2Z8~R4m6N1t3~H5k0W2s9ü"; + + private static class ObservedMiddleware extends BasicAuthMiddleware { + final List buffers = new ArrayList<>(); + boolean ldapAccepted; + boolean ldapThrows; + ObservedMiddleware() { super("basic-auth-memory"); } + + @Override BasicAuthCredentials parseCredentials(String auth) { + BasicAuthCredentials credentials = super.parseCredentials(auth); + for (String name : new String[]{"decoded", "password"}) { + try { + var field = BasicAuthCredentials.class.getDeclaredField(name); + field.setAccessible(true); + buffers.add((byte[]) field.get(credentials)); + } catch (ReflectiveOperationException failure) { + throw new AssertionError(failure); + } + } + return credentials; + } + + @Override protected boolean handleLdapAuth(UserAuth user, String password) { + assertEquals(SECRET, password); + if (ldapThrows) throw new IllegalStateException("synthetic LDAP failure"); + return ldapAccepted; + } + + void assertClosed() { + assertEquals(2, buffers.size(), "Observe buffers from exactly one real middleware parse"); + for (byte[] buffer : buffers) for (byte value : buffer) assertEquals(0, value); + buffers.clear(); + } + } + + @Test + void actualBasicHandlerClosesOnEveryAuthenticationAndAuthorizationReturn() { + var middleware = new ObservedMiddleware(); + for (boolean encoded : new boolean[]{false, true}) { + check(middleware, "local-user:" + SECRET, encoded, "/allowed", 200); + check(middleware, "local-user:" + SECRET, encoded, "/denied", 401); + check(middleware, "local-user:wrong", encoded, "/allowed", 401); + check(middleware, "local-user:", encoded, "/allowed", 401); + check(middleware, "unknown-user:" + SECRET, encoded, "/allowed", 401); + check(middleware, "anonymous:" + SECRET, encoded, "/allowed", 401); + check(middleware, "bearer:" + SECRET, encoded, "/allowed", 401); + middleware.ldapAccepted = false; + check(middleware, "ldap-user:" + SECRET, encoded, "/allowed", 401); + middleware.ldapAccepted = true; + check(middleware, "ldap-user:" + SECRET, encoded, "/allowed", 200); + check(middleware, "ldap-user:" + SECRET, encoded, "/denied", 401); + } + check(middleware, "missing-colon", true, "/allowed", 401); + } + + @Test + void actualBasicHandlerClosesWhenLdapThrows() { + var middleware = new ObservedMiddleware(); + middleware.ldapThrows = true; + String header = "Basic " + Base64.getEncoder().encodeToString(("ldap-user:" + SECRET).getBytes(UTF_8)); + assertThrows(IllegalStateException.class, () -> middleware.handleBasicAuth(exchange(header), "/allowed", header)); + middleware.assertClosed(); + } + + @Test + void unicodeSchemeAndMalformedUtf8RemainCompatibleThroughExecute() { + var middleware = new ObservedMiddleware(); + String header = "Bas\u0131c local-user:" + SECRET; + assertEquals(200, middleware.execute(exchange(header)).getStatusCode()); + middleware.assertClosed(); + byte[] prefix = "replacement-user:".getBytes(UTF_8); + byte[] pair = java.util.Arrays.copyOf(prefix, prefix.length + 2); + pair[prefix.length] = (byte) 0xc3; + pair[prefix.length + 1] = '('; + header = "Basic " + Base64.getEncoder().encodeToString(pair); + assertEquals(200, middleware.execute(exchange(header)).getStatusCode()); + middleware.assertClosed(); + } + + private static void check(ObservedMiddleware middleware, String pair, boolean encoded, String path, int expected) { + String header = "Basic " + (encoded ? Base64.getEncoder().encodeToString(pair.getBytes(UTF_8)) : pair); + assertEquals(expected, middleware.handleBasicAuth(exchange(header), path, header).getStatusCode()); + middleware.assertClosed(); + } + + private static LightLambdaExchange exchange(String header) { + var request = TestUtils.createTestRequestEvent(); + request.setPath("/allowed"); + request.getHeaders().put("Authorization", header); + var exchange = new LightLambdaExchange(new LambdaContext("lifecycle-fixture"), null); + exchange.setInitialRequest(request); + return exchange; + } +} diff --git a/src/test/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthPrivacyTest.java b/src/test/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthPrivacyTest.java new file mode 100644 index 0000000..2eb17e5 --- /dev/null +++ b/src/test/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthPrivacyTest.java @@ -0,0 +1,122 @@ +package com.networknt.aws.lambda.handler.middleware.security; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.classic.spi.IThrowableProxy; +import ch.qos.logback.core.read.ListAppender; +import com.networknt.aws.lambda.LambdaContext; +import com.networknt.aws.lambda.LightLambdaExchange; +import com.networknt.aws.lambda.TestUtils; +import com.networknt.basicauth.UserAuth; +import com.networknt.status.Status; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; + +import java.util.Arrays; +import java.util.Base64; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.*; + +class BasicAuthPrivacyTest { + private static final java.util.List ENCODED_CANARIES = new java.util.ArrayList<>(); + private static final String SECRET = "q7V9p2Z8~R4m6N1t3~H5k0W2s9ü"; + + private static class LdapFixture extends BasicAuthMiddleware { + boolean accepted; + boolean fail; + int calls; + LdapFixture() { super("basic-auth-memory"); } + @Override protected boolean handleLdapAuth(UserAuth user, String password) { + calls++; + assertEquals(SECRET, password); + if (fail) throw new IllegalStateException("synthetic LDAP failure"); + return accepted; + } + } + + @Test + void reportedA25PathsRemainPrivateAtTraceAcrossLocalAndLdapOutcomes() { + ENCODED_CANARIES.clear(); + Logger root = (Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME); + Level previous = root.getLevel(); + var logs = new ListAppender(); + logs.start(); + root.addAppender(logs); + root.setLevel(Level.TRACE); + try { + var middleware = new LdapFixture(); + for (boolean encoded : new boolean[]{false, true}) { + check(middleware, "local-user:" + SECRET, encoded, "/allowed", 200); + check(middleware, "local-user:" + SECRET, encoded, "/denied", 401); + check(middleware, "local-user:wrong-" + SECRET, encoded, "/allowed", 401); + check(middleware, "unknown-user:" + SECRET, encoded, "/allowed", 401); + check(middleware, "local-user:", encoded, "/allowed", 401); + check(middleware, "anonymous:" + SECRET, encoded, "/allowed", 401); + middleware.accepted = false; + check(middleware, "ldap-user:" + SECRET, encoded, "/allowed", 401); + middleware.accepted = true; + check(middleware, "ldap-user:" + SECRET, encoded, "/allowed", 200); + } + check(middleware, "malformed-secret-canary", true, "/allowed", 401); + middleware.fail = true; + assertThrows(IllegalStateException.class, () -> middleware.execute(exchange( + "Basic " + Base64.getEncoder().encodeToString(("ldap-user:" + SECRET).getBytes(UTF_8)), "/allowed"))); + assertEquals(5, middleware.calls); + assertTrue(logs.list.stream().anyMatch(event -> event.getLevel() == Level.TRACE)); + for (ILoggingEvent event : logs.list) { + privateText(event.getFormattedMessage()); + privateText(Arrays.deepToString(event.getArgumentArray())); + privateText(String.valueOf(event.getMDCPropertyMap())); + privateThrowable(event.getThrowableProxy()); + } + } finally { + root.setLevel(previous); + root.detachAppender(logs); + logs.stop(); + } + } + + private static void check(BasicAuthMiddleware middleware, String pair, boolean encoded, String path, int expected) { + ENCODED_CANARIES.add(Base64.getEncoder().encodeToString(pair.getBytes(UTF_8))); + String header = "Basic " + (encoded ? Base64.getEncoder().encodeToString(pair.getBytes(UTF_8)) : pair); + var exchange = exchange(header, path); + Status result = middleware.execute(exchange); + assertEquals(expected, result.getStatusCode()); + privateText(result.toString()); + } + + private static LightLambdaExchange exchange(String header, String path) { + var request = TestUtils.createTestRequestEvent(); + request.setPath(path); + request.getHeaders().put("Authorization", header); + var exchange = new LightLambdaExchange(new LambdaContext("privacy-fixture"), null); + exchange.setInitialRequest(request); + return exchange; + } + + private static void privateText(String text) { + assertFalse(text.contains("secret-canary"), text); + assertFalse(text.contains(SECRET.substring(0, 4)), text); + assertFalse(text.contains(SECRET.substring(SECRET.length() - 4)), text); + assertFalse(text.contains("ü"), text); + for (String canary : ENCODED_CANARIES) assertFalse(text.contains(canary), text); + assertFalse(text.contains(Base64.getEncoder().encodeToString(("local-user:" + SECRET).getBytes(UTF_8))), text); + assertFalse(text.contains(Base64.getEncoder().encodeToString(("ldap-user:" + SECRET).getBytes(UTF_8))), text); + } + + @Test + void canaryDetectorRejectsPasswordPrefixAndSuffixLeaks() { + assertThrows(AssertionError.class, () -> privateText("prefix=" + SECRET.substring(0, 4))); + assertThrows(AssertionError.class, () -> privateText("suffix=" + SECRET.substring(SECRET.length() - 4))); + assertThrows(AssertionError.class, () -> privateText("suffix=ü")); + } + + private static void privateThrowable(IThrowableProxy throwable) { + if (throwable == null) return; + privateText(String.valueOf(throwable.getMessage())); + privateThrowable(throwable.getCause()); + if (throwable.getSuppressed() != null) for (var suppressed : throwable.getSuppressed()) privateThrowable(suppressed); + } +} diff --git a/src/test/resources/config/basic-auth-memory.yml b/src/test/resources/config/basic-auth-memory.yml new file mode 100644 index 0000000..13258bb --- /dev/null +++ b/src/test/resources/config/basic-auth-memory.yml @@ -0,0 +1,16 @@ +enabled: true +allowAnonymous: false +allowBearerToken: false +enableAD: true +users: + - username: local-user + password: "q7V9p2Z8~R4m6N1t3~H5k0W2s9ü" + paths: + - /allowed + - username: ldap-user + paths: + - /allowed + - username: replacement-user + password: "\uFFFD(" + paths: + - /allowed