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 new file mode 100644 index 0000000..5b035b8 --- /dev/null +++ b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/AuthorizationScheme.java @@ -0,0 +1,23 @@ +package com.networknt.aws.lambda.handler.middleware.security; + +/** Scheme classification without retaining or exposing credential material. */ +enum AuthorizationScheme { + BASIC, BEARER, UNKNOWN; + + static AuthorizationScheme parse(String header) { + if (header == null) return UNKNOWN; + for (AuthorizationScheme scheme : new AuthorizationScheme[] {BASIC, BEARER}) { + String prefix = scheme.name(); + if (header.regionMatches(true, 0, prefix, 0, prefix.length()) + && (header.length() == prefix.length() || header.charAt(prefix.length()) == ' ')) { + return scheme; + } + } + return UNKNOWN; + } + + 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(); + } +} 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 6227721..949fb66 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 @@ -7,14 +7,13 @@ import com.networknt.utility.MapUtil; import com.networknt.basicauth.BasicAuthConfig; import com.networknt.basicauth.UserAuth; -import com.networknt.config.Config; import com.networknt.ldap.LdapUtil; import com.networknt.status.Status; import com.networknt.utility.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; +import java.security.MessageDigest; import java.util.HashMap; import java.util.List; import java.util.Optional; @@ -67,21 +66,19 @@ public Status execute(LightLambdaExchange exchange) { if(auth.trim().isEmpty()) { return this.handleAnonymousAuth(exchange, requestPath, config); } - // verify the header with the config file. assuming it is basic authentication first. - if (BASIC_PREFIX.equalsIgnoreCase(auth.substring(0, 5))) { - // check if the length is greater than 6 for issue1513 - if(auth.trim().length() == 5) { - LOG.error("Invalid/Unsupported authorization header {}", auth); - return new Status(INVALID_AUTHORIZATION_HEADER, auth); - } else { - return this.handleBasicAuth(exchange, requestPath, auth); - } - } else if (BEARER_PREFIX.equalsIgnoreCase(auth.substring(0, 6))) { + AuthorizationScheme scheme = AuthorizationScheme.parse(auth); + // A bare scheme must not reach handleBearerToken, which only checks paths. + if (scheme != AuthorizationScheme.UNKNOWN && !AuthorizationScheme.hasCredentials(auth, scheme)) { + LOG.error("Invalid/Unsupported authorization header."); + return new Status(INVALID_AUTHORIZATION_HEADER, scheme.name()); + } + if (scheme == AuthorizationScheme.BASIC) { + return this.handleBasicAuth(exchange, requestPath, auth); + } else if (scheme == AuthorizationScheme.BEARER) { return this.handleBearerToken(exchange, requestPath, auth, config); - } else { - LOG.error("Invalid/Unsupported authorization header {}", auth.substring(0, 10)); - return new Status(INVALID_AUTHORIZATION_HEADER, auth.substring(0, 10)); } + LOG.error("Invalid/Unsupported authorization header."); + return new Status(INVALID_AUTHORIZATION_HEADER, "unknown"); } } @@ -147,6 +144,10 @@ private Status handleAnonymousAuth(LightLambdaExchange exchange, String requestP * @return Status to indicate if an error or success. */ public Status handleBasicAuth(LightLambdaExchange exchange, String requestPath, String auth) { + if (!AuthorizationScheme.hasCredentials(auth, AuthorizationScheme.BASIC)) { + 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) { @@ -156,36 +157,32 @@ public Status handleBasicAuth(LightLambdaExchange exchange, String requestPath, if (pos != -1) { String username = credentials.substring(0, pos); String password = credentials.substring(pos + 1); - if(LOG.isTraceEnabled()) LOG.trace("input username = {}, password = {}", username, StringUtils.maskHalfString(password)); UserAuth user = config.getUsers().get(username); - // if user cannot be found in the config, return immediately. - if (user == null) { - LOG.error("User '{}' is not found in the configuration file.", username); + // Reserved users describe authorization policies, never Basic identities. + if (user == null || StringUtils.isEmpty(password) + || 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."); return new Status(INVALID_USERNAME_OR_PASSWORD); } - // At this point, we know the user is found in the config file. - if (username.equals(user.getUsername()) - && StringUtils.isEmpty(user.getPassword()) - && config.isEnableAD()) { - // Call LdapUtil with LDAP authentication and authorization given user is matched, password is empty, and AD is enabled. - if(LOG.isTraceEnabled()) LOG.trace("Call LdapUtil with LDAP authentication and authorization for user = {}", username); + 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(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); return new Status(INVALID_USERNAME_OR_PASSWORD); } } else { - if(LOG.isTraceEnabled()) LOG.trace("Validate basic auth based on config username {} and password {}", user.getUsername(), StringUtils.maskHalfString(user.getPassword())); - // if username matches config, password matches config, and path matches config, pass - if (!(user.getUsername().equals(username) - && password.equals(user.getPassword()))) { - LOG.error("Invalid username or password with authorization header = {}", StringUtils.maskHalfString(auth)); + // Compare the configured local password without early character mismatches. + if (!(user.getPassword() != null + && MessageDigest.isEqual(password.getBytes(UTF_8), user.getPassword().getBytes(UTF_8)))) { + 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); } } // Here we have passed the authentication. Let's do the authorization with the paths. - if(LOG.isTraceEnabled()) LOG.trace("Username and password validation is done for user = {}", username); + if(LOG.isTraceEnabled()) LOG.trace("Basic username and password validation is done."); boolean match = false; for (String path : user.getPaths()) { if (requestPath.startsWith(path)) { @@ -194,14 +191,15 @@ public Status handleBasicAuth(LightLambdaExchange exchange, String requestPath, } } if (!match) { - LOG.error("Request path '{}' is not authorized for user '{}", requestPath, user.getUsername()); + String safePath = requestPath.replace('\r', ' ').replace('\n', ' '); + LOG.error("Request path '{}' is not authorized for user '{}'", safePath, user.getUsername()); if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); - return new Status(NOT_AUTHORIZED_REQUEST_PATH, requestPath, user.getUsername()); + 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, auth.substring(0, 10)); + return new Status(INVALID_BASIC_HEADER, BASIC_PREFIX); } return successMiddlewareStatus(); } @@ -211,7 +209,7 @@ public Status handleBasicAuth(LightLambdaExchange exchange, String requestPath, * @param user * @return true if Ldap auth success, false if Ldap auth failure */ - private static boolean handleLdapAuth(UserAuth user, String password) { + protected boolean handleLdapAuth(UserAuth user, String password) { boolean isAuthenticated = LdapUtil.authenticate(user.getUsername(), password); if (!isAuthenticated) { LOG.error("user '" + user.getUsername() + "' Ldap authentication failed"); @@ -235,7 +233,7 @@ private Status handleBearerToken(LightLambdaExchange exchange, String requestPat if (!config.isAllowBearerToken()) { LOG.error("Not a basic authentication header, and bearer token is not allowed."); if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); - return new Status(INVALID_BASIC_HEADER, auth.substring(0, 10)); + return new Status(INVALID_BASIC_HEADER, BEARER_PREFIX); } else { // bearer token is allowed, we need to validate it and check the allowed paths. UserAuth user = config.getUsers().get(BasicAuthConfig.BEARER); diff --git a/src/main/java/com/networknt/aws/lambda/handler/middleware/security/UnifiedSecurityMiddleware.java b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/UnifiedSecurityMiddleware.java index 667a82c..38d0e2f 100644 --- a/src/main/java/com/networknt/aws/lambda/handler/middleware/security/UnifiedSecurityMiddleware.java +++ b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/UnifiedSecurityMiddleware.java @@ -88,13 +88,14 @@ public Status execute(LightLambdaExchange exchange) { return new Status(MISSING_AUTH_TOKEN); } else { String authorization = optionalAuth.get(); - // make sure that the length is greater than 5. - if (authorization.trim().length() <= 5) { - LOG.error("Invalid/Unsupported authorization header {}", authorization); - return new Status(INVALID_AUTHORIZATION_HEADER, authorization); + AuthorizationScheme scheme = AuthorizationScheme.parse(authorization); + if (!AuthorizationScheme.hasCredentials(authorization, scheme) + || scheme == AuthorizationScheme.UNKNOWN) { + LOG.error("Invalid/Unsupported authorization header."); + return new Status(INVALID_AUTHORIZATION_HEADER, + scheme == AuthorizationScheme.UNKNOWN ? "unknown" : scheme.name()); } - // check if it is basic or bearer and handler it differently. - if (BASIC_PREFIX.equalsIgnoreCase(authorization.substring(0, 5))) { + if (scheme == AuthorizationScheme.BASIC) { Map handlers = Handler.getHandlers(); BasicAuthMiddleware handler = (BasicAuthMiddleware) handlers.get(BASIC_PREFIX.toLowerCase()); if (handler == null) { @@ -107,7 +108,7 @@ public Status execute(LightLambdaExchange exchange) { } return handler.handleBasicAuth(exchange, reqPath, authorization); } - } else if (BEARER_PREFIX.equalsIgnoreCase(authorization.substring(0, 6))) { + } else if (scheme == AuthorizationScheme.BEARER) { // in the case that a bearer token is used, there are three scenarios: both jwt and swt are true, only jwt is true and only swt is true // in the first case, we need to identify if the token is jwt or swt before calling the right handler to verify it. Map handlers = Handler.getHandlers(); @@ -173,9 +174,8 @@ public Status execute(LightLambdaExchange exchange) { } } } else { - String s = authorization.length() > 10 ? authorization.substring(0, 10) : authorization; - LOG.error("Invalid/Unsupported authorization header {}", s); - return new Status(INVALID_AUTHORIZATION_HEADER, s); + LOG.error("Invalid/Unsupported authorization header."); + return new Status(INVALID_AUTHORIZATION_HEADER, "unknown"); } } } else if (pathPrefixAuth.isApikey()) { diff --git a/src/test/java/com/networknt/aws/lambda/middleware/security/BasicAuthMiddlewareTest.java b/src/test/java/com/networknt/aws/lambda/middleware/security/BasicAuthMiddlewareTest.java index 5f073e2..ae7046f 100644 --- a/src/test/java/com/networknt/aws/lambda/middleware/security/BasicAuthMiddlewareTest.java +++ b/src/test/java/com/networknt/aws/lambda/middleware/security/BasicAuthMiddlewareTest.java @@ -11,12 +11,18 @@ import com.networknt.aws.lambda.handler.middleware.specification.OpenApiMiddleware; import com.networknt.aws.lambda.utility.HeaderKey; import com.networknt.basicauth.BasicAuthConfig; +import com.networknt.basicauth.UserAuth; +import com.networknt.aws.lambda.handler.middleware.security.UnifiedSecurityMiddleware; import com.networknt.status.Status; import com.networknt.utility.Constants; import com.networknt.utility.MapUtil; import org.apache.commons.codec.binary.Base64; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import org.slf4j.LoggerFactory; import java.util.Map; @@ -42,6 +48,143 @@ private static String encodeCredentials(String username, String password) { return encodeCredentialsFullFormat(username, password, ":"); } + @Test + public void testEmptyConfiguredPasswordIsRejected() { + Assertions.assertEquals("", BasicAuthConfig.load().getUsers().get("blankPassword").getPassword()); + var requestEvent = TestUtils.createTestRequestEvent(); + requestEvent.setPath("/v2/pet"); + requestEvent.getHeaders().put(HeaderKey.AUTHORIZATION, "BASIC " + encodeCredentials("blankPassword", "")); + var exchange = new LightLambdaExchange(new LambdaContext("empty-password"), null); + exchange.setInitialRequest(requestEvent); + Status status = new BasicAuthMiddleware("basic-auth").execute(exchange); + Assertions.assertEquals(401, status.getStatusCode()); + Assertions.assertEquals("ERR10047", status.getCode()); + } + + @Test + public void testShortMalformedAuthorizationHeadersAreRejected() { + for (String header : new String[] {"x", "Basic eA", "Bearer", "Bearer x", "BasicSecret", "Basic"}) { + var requestEvent = TestUtils.createTestRequestEvent(); + requestEvent.setPath("/v2/pet"); + requestEvent.getHeaders().put(HeaderKey.AUTHORIZATION, header); + var exchange = new LightLambdaExchange(new LambdaContext("short-header"), null); + exchange.setInitialRequest(requestEvent); + Status status = new BasicAuthMiddleware("basic-auth").execute(exchange); + Assertions.assertEquals(401, status.getStatusCode(), header); + Assertions.assertEquals(header.equals("Basic eA") || header.equals("Bearer x") + ? "ERR10046" : "ERR12003", status.getCode(), header); + } + } + + @Test + public void testBareBearerRejectedWhenBearerTokensAllowed() { + var middleware = new BasicAuthMiddleware("basic-auth-bearer"); + Assertions.assertTrue(BasicAuthConfig.load("basic-auth-bearer").isAllowBearerToken()); + for (String header : new String[] {"Bearer", "Bearer ", "bearer "}) { + Status status = middleware.execute(exchangeFor(header)); + Assertions.assertEquals(401, status.getStatusCode(), header); + Assertions.assertEquals("ERR12003", status.getCode(), header); + } + } + + private static LightLambdaExchange exchangeFor(String header) { + var request = TestUtils.createTestRequestEvent(); + request.setPath("/v2/pet"); + request.getHeaders().put(HeaderKey.AUTHORIZATION, header); + var exchange = new LightLambdaExchange(new LambdaContext("security-review"), null); + exchange.setInitialRequest(request); + return exchange; + } + + private static class RecordingLdapMiddleware extends BasicAuthMiddleware { + int ldapCalls; + + RecordingLdapMiddleware() { + super("basic-auth-ldap"); + } + + @Override + protected boolean handleLdapAuth(UserAuth user, String password) { + ldapCalls++; + return true; + } + } + + @Test + public void testLdapRejectsEmptyPasswordsAndReservedUsersBeforeBind() { + Assertions.assertTrue(BasicAuthConfig.load("basic-auth-ldap").isEnableAD()); + var middleware = new RecordingLdapMiddleware(); + for (String[] credentials : new String[][] { + {"ldapUser", ""}, {"anonymous", "password"}, {"bearer", "password"}}) { + Status status = middleware.execute(exchangeFor("Basic " + encodeCredentials(credentials[0], credentials[1]))); + Assertions.assertEquals("ERR10047", status.getCode()); + Assertions.assertEquals(401, status.getStatusCode()); + } + Assertions.assertEquals(0, middleware.ldapCalls); + // Positive control proves this configuration actually dispatches ordinary identities to LDAP. + Assertions.assertEquals(200, middleware.execute(exchangeFor("Basic " + encodeCredentials("ldapUser", "password"))).getStatusCode()); + Assertions.assertEquals(1, middleware.ldapCalls); + } + + @Test + public void testPublicBasicEntryPointRejectsMalformedHeaders() { + var middleware = new BasicAuthMiddleware("basic-auth"); + for (String header : new String[] {null, "", "x", "Basic", "Basic ", "BasicSecret", "Bearer x"}) { + Status status = middleware.handleBasicAuth(exchangeFor("ignored"), "/v2/pet", header); + Assertions.assertEquals("ERR12003", status.getCode()); + Assertions.assertEquals(401, status.getStatusCode()); + } + } + + @Test + public void testUnifiedRejectsMalformedHeadersWithoutEchoingCredentials() { + var middleware = new UnifiedSecurityMiddleware("unified-security-review"); + for (String header : new String[] {"x", "abcde", "Secret credential-material", "Basic", "Basic ", "Bearer", "Bearer ", "BasicSecret"}) { + Status status = middleware.execute(exchangeFor(header)); + Assertions.assertEquals("ERR12003", status.getCode(), header); + Assertions.assertEquals(401, status.getStatusCode(), header); + Assertions.assertFalse(status.toString().contains(header), header); + } + } + + @Test + public void testAuthorizationLogsDoNotEchoHeadersAndSanitizeDeniedPath() { + Logger basicLogger = (Logger) LoggerFactory.getLogger(BasicAuthMiddleware.class); + Logger unifiedLogger = (Logger) LoggerFactory.getLogger(UnifiedSecurityMiddleware.class); + var appender = new ListAppender(); + appender.start(); + basicLogger.addAppender(appender); + unifiedLogger.addAppender(appender); + try { + var basic = new BasicAuthMiddleware("basic-auth"); + var unified = new UnifiedSecurityMiddleware("unified-security-review"); + for (String header : new String[] {"abcde", "Secret credential-material"}) { + basic.execute(exchangeFor(header)); + unified.execute(exchangeFor(header)); + Assertions.assertTrue(appender.list.stream().noneMatch(event -> event.getFormattedMessage().contains(header))); + } + var exchange = exchangeFor("Basic " + encodeCredentials("user1", "user1pass")); + exchange.getRequest().setPath("/denied\r\nforged-line"); + Status status = basic.execute(exchange); + Assertions.assertEquals("ERR10071", status.getCode()); + Assertions.assertFalse(status.toString().contains("\r") || status.toString().contains("\n")); + Assertions.assertTrue(status.toString().contains("/denied forged-line")); + Assertions.assertTrue(appender.list.stream().anyMatch(event -> event.getFormattedMessage() + .equals("Request path '/denied forged-line' is not authorized for user 'user1'"))); + } finally { + basicLogger.detachAppender(appender); + unifiedLogger.detachAppender(appender); + appender.stop(); + } + } + + @Test + public void testMixedCaseBasicSchemeAndColonInPasswordParsing() { + var basic = new BasicAuthMiddleware("basic-auth"); + Assertions.assertEquals(200, basic.execute(exchangeFor("bAsIc " + encodeCredentials("user2", "password"))).getStatusCode()); + Assertions.assertEquals("ERR10047", basic.execute(exchangeFor("Basic " + encodeCredentials("user2", "password:extra"))).getCode()); + } + /** * Test with right credentials but incorrect path. Expect 401 status code. */ diff --git a/src/test/resources/config/basic-auth-ldap.yml b/src/test/resources/config/basic-auth-ldap.yml index e545859..3c44fd6 100644 --- a/src/test/resources/config/basic-auth-ldap.yml +++ b/src/test/resources/config/basic-auth-ldap.yml @@ -30,3 +30,7 @@ users: paths: - /v1/party - /info + + - username: bearer + paths: + - /v2/pet diff --git a/src/test/resources/config/basic-auth.yml b/src/test/resources/config/basic-auth.yml index dd0b98a..fccf4d8 100644 --- a/src/test/resources/config/basic-auth.yml +++ b/src/test/resources/config/basic-auth.yml @@ -19,6 +19,10 @@ users: password: user1pass paths: - /v1/address + - username: blankPassword + password: "" + paths: + - /v2/pet - username: user2 password: CRYPT:2f9fefa2c00d6ead365a6a20967f235d:e84bb6cccc5c5887f0331186:98eb70096c4d354e88f6e7863cdccf22897a12c2d669e26c paths: diff --git a/src/test/resources/config/unified-security-review.yml b/src/test/resources/config/unified-security-review.yml new file mode 100644 index 0000000..37cb9ea --- /dev/null +++ b/src/test/resources/config/unified-security-review.yml @@ -0,0 +1,5 @@ +enabled: true +pathPrefixAuths: + - prefix: /v2 + basic: true + jwt: true