From a230346fd6b2311402fe4f3bcb81251439461a48 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Thu, 21 Jun 2018 09:29:29 +1000 Subject: [PATCH 1/2] [AuthcHeader] Check auth scheme case insensitively According to [RFC 7617](https://tools.ietf.org/html/rfc7617), the authorization scheme should not be case sensitive Applicable to bearer token where "Bearer " auth scheme is used. Ref: [RFC 6750](https://tools.ietf.org/html/rfc6750) [RFC 7235](https://tools.ietf.org/html/rfc7235#page-10) This is problem for some of HTTP clients using values like `BASIC`, `basic`, `BEARER` etc. This commit modifes the check to be case insensitive. Closes #31486 --- .../authc/support/UsernamePasswordToken.java | 13 ++++++++----- .../xpack/security/authc/TokenService.java | 4 ++-- .../security/authc/TokenServiceTests.java | 18 +++++++++++++++++- .../support/UsernamePasswordTokenTests.java | 7 ++++--- 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/UsernamePasswordToken.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/UsernamePasswordToken.java index 4fdf32608dd6a..d8e58c29d237b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/UsernamePasswordToken.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/UsernamePasswordToken.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.core.security.authc.support; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.xpack.core.security.authc.AuthenticationToken; @@ -20,6 +21,8 @@ public class UsernamePasswordToken implements AuthenticationToken { public static final String BASIC_AUTH_PREFIX = "Basic "; public static final String BASIC_AUTH_HEADER = "Authorization"; + // authorization scheme check is case-insensitive + private static final boolean IGNORE_CASE_AUTH_HEADER_MATCH = true; private final String username; private final SecureString password; @@ -79,15 +82,15 @@ public int hashCode() { public static UsernamePasswordToken extractToken(ThreadContext context) { String authStr = context.getHeader(BASIC_AUTH_HEADER); - if (authStr == null) { - return null; - } - return extractToken(authStr); } private static UsernamePasswordToken extractToken(String headerValue) { - if (headerValue.startsWith(BASIC_AUTH_PREFIX) == false) { + if (Strings.isNullOrEmpty(headerValue)) { + return null; + } + if (headerValue.regionMatches(IGNORE_CASE_AUTH_HEADER_MATCH, 0, BASIC_AUTH_PREFIX, 0, + BASIC_AUTH_PREFIX.length()) == false) { // the header does not start with 'Basic ' so we cannot use it, but it may be valid for another realm return null; } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java index 2934fb8062de4..e644c83708665 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java @@ -1007,9 +1007,9 @@ private void maybeStartTokenRemover() { */ private String getFromHeader(ThreadContext threadContext) { String header = threadContext.getHeader("Authorization"); - if (Strings.hasLength(header) && header.startsWith("Bearer ") + if (Strings.hasText(header) && header.regionMatches(true, 0, "Bearer ", 0, "Bearer ".length()) && header.length() > "Bearer ".length()) { - return header.substring("Bearer ".length()); + return header.substring("Bearer ".length()).trim(); } return null; } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java index 28cf4bf95c924..d5e67f3996a7b 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java @@ -71,6 +71,7 @@ import static org.elasticsearch.repositories.ESBlobStoreTestCase.randomBytes; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; @@ -162,7 +163,7 @@ public void testAttachAndGetToken() throws Exception { mockGetTokenFromId(token); ThreadContext requestContext = new ThreadContext(Settings.EMPTY); - requestContext.putHeader("Authorization", "Bearer " + tokenService.getUserTokenString(token)); + requestContext.putHeader("Authorization", randomFrom("Bearer ", "BEARER ", "bearer ") + tokenService.getUserTokenString(token)); try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) { PlainActionFuture future = new PlainActionFuture<>(); @@ -183,6 +184,21 @@ public void testAttachAndGetToken() throws Exception { } } + public void testInvalidAuthorizationHeader() throws Exception { + TokenService tokenService = new TokenService(tokenServiceEnabledSettings, systemUTC(), client, securityIndex, clusterService); + ThreadContext requestContext = new ThreadContext(Settings.EMPTY); + String token = randomFrom("", " "); + String authScheme = randomFrom("Bearer ", "BEARER ", "bearer ", "Basic "); + requestContext.putHeader("Authorization", authScheme + token); + + try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) { + PlainActionFuture future = new PlainActionFuture<>(); + tokenService.getAndValidateToken(requestContext, future); + UserToken serialized = future.get(); + assertThat(serialized, nullValue()); + } + } + public void testRotateKey() throws Exception { TokenService tokenService = new TokenService(tokenServiceEnabledSettings, systemUTC(), client, securityIndex, clusterService); Authentication authentication = new Authentication(new User("joe", "admin"), new RealmRef("native_realm", "native", "node1"), null); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/UsernamePasswordTokenTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/UsernamePasswordTokenTests.java index 57c452798844c..86b9635851bb6 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/UsernamePasswordTokenTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/UsernamePasswordTokenTests.java @@ -45,7 +45,8 @@ public void testPutToken() throws Exception { public void testExtractToken() throws Exception { ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - String header = "Basic " + Base64.getEncoder().encodeToString("user1:test123".getBytes(StandardCharsets.UTF_8)); + final String header = randomFrom("Basic ", "basic ", "BASIC ") + + Base64.getEncoder().encodeToString("user1:test123".getBytes(StandardCharsets.UTF_8)); threadContext.putHeader(UsernamePasswordToken.BASIC_AUTH_HEADER, header); UsernamePasswordToken token = UsernamePasswordToken.extractToken(threadContext); assertThat(token, notNullValue()); @@ -54,7 +55,7 @@ public void testExtractToken() throws Exception { } public void testExtractTokenInvalid() throws Exception { - String[] invalidValues = { "Basic ", "Basic f" }; + final String[] invalidValues = { "Basic ", "Basic f", "basic " }; for (String value : invalidValues) { ThreadContext threadContext = new ThreadContext(Settings.EMPTY); threadContext.putHeader(UsernamePasswordToken.BASIC_AUTH_HEADER, value); @@ -70,7 +71,7 @@ public void testExtractTokenInvalid() throws Exception { public void testHeaderNotMatchingReturnsNull() { ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - String header = randomFrom("BasicBroken", "invalid", "Basic"); + final String header = randomFrom("Basic", "BasicBroken", "invalid", " basic "); threadContext.putHeader(UsernamePasswordToken.BASIC_AUTH_HEADER, header); UsernamePasswordToken extracted = UsernamePasswordToken.extractToken(threadContext); assertThat(extracted, nullValue()); From c03a9d1de52bbea19edaf798dc1bfa5f7449be2f Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Fri, 22 Jun 2018 02:15:34 +1000 Subject: [PATCH 2/2] [Security] Address review comment Closes #31486 --- .../org/elasticsearch/xpack/security/authc/TokenService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java index e644c83708665..8b6dd8295d399 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java @@ -1009,7 +1009,7 @@ private String getFromHeader(ThreadContext threadContext) { String header = threadContext.getHeader("Authorization"); if (Strings.hasText(header) && header.regionMatches(true, 0, "Bearer ", 0, "Bearer ".length()) && header.length() > "Bearer ".length()) { - return header.substring("Bearer ".length()).trim(); + return header.substring("Bearer ".length()); } return null; }