Skip to content

Commit bf72170

Browse files
bizybotYogesh Gaikwad
authored and
Yogesh Gaikwad
committed
[SECURITY] Set Auth-scheme preference (#33156)
Some browsers (eg. Firefox) behave differently when presented with multiple auth schemes in 'WWW-Authenticate' header. The expected behavior is that browser select the most secure auth-scheme before trying others, but Firefox selects the first presented auth scheme and tries the next ones sequentially. As the browser interpretation is something that we do not control, we can at least present the auth schemes in most to least secure order as the server's preference. This commit modifies the code to collect and sort the auth schemes presented by most to least secure. The priority of the auth schemes is fixed, the lower number denoting more secure auth-scheme. The current order of schemes based on the ES supported auth-scheme is [Negotiate, Bearer,Basic] and when we add future support for other schemes we will need to update the code. If need be we will make this configuration customizable in future. Unit test to verify the WWW-Authenticate header values are sorted by server preference as more secure to least secure auth schemes. Tested with Firefox, Chrome, Internet Explorer 11. Closes#32699
1 parent 15efb2b commit bf72170

File tree

2 files changed

+58
-7
lines changed

2 files changed

+58
-7
lines changed

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/DefaultAuthenticationFailureHandler.java

+35-3
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@
1212
import org.elasticsearch.transport.TransportMessage;
1313
import org.elasticsearch.xpack.core.XPackField;
1414

15+
import java.util.ArrayList;
1516
import java.util.Collections;
1617
import java.util.List;
1718
import java.util.Map;
19+
import java.util.stream.Collectors;
1820

1921
import static org.elasticsearch.xpack.core.security.support.Exceptions.authenticationError;
2022

@@ -44,12 +46,42 @@ public DefaultAuthenticationFailureHandler() {
4446
* be sent as failure response.
4547
* @see Realm#getAuthenticationFailureHeaders()
4648
*/
47-
public DefaultAuthenticationFailureHandler(Map<String, List<String>> failureResponseHeaders) {
49+
public DefaultAuthenticationFailureHandler(final Map<String, List<String>> failureResponseHeaders) {
4850
if (failureResponseHeaders == null || failureResponseHeaders.isEmpty()) {
49-
failureResponseHeaders = Collections.singletonMap("WWW-Authenticate",
51+
this.defaultFailureResponseHeaders = Collections.singletonMap("WWW-Authenticate",
5052
Collections.singletonList("Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\""));
53+
} else {
54+
this.defaultFailureResponseHeaders = Collections.unmodifiableMap(failureResponseHeaders.entrySet().stream().collect(Collectors
55+
.toMap(entry -> entry.getKey(), entry -> {
56+
if (entry.getKey().equalsIgnoreCase("WWW-Authenticate")) {
57+
List<String> values = new ArrayList<>(entry.getValue());
58+
Collections.sort(values, (o1, o2) -> authSchemePriority(o1).compareTo(authSchemePriority(o2)));
59+
return Collections.unmodifiableList(values);
60+
} else {
61+
return Collections.unmodifiableList(entry.getValue());
62+
}
63+
})));
64+
}
65+
}
66+
67+
/**
68+
* For given 'WWW-Authenticate' header value returns the priority based on
69+
* the auth-scheme. Lower number denotes more secure and preferred
70+
* auth-scheme than the higher number.
71+
*
72+
* @param headerValue string starting with auth-scheme name
73+
* @return integer value denoting priority for given auth scheme.
74+
*/
75+
private static Integer authSchemePriority(final String headerValue) {
76+
if (headerValue.regionMatches(true, 0, "negotiate", 0, "negotiate".length())) {
77+
return 0;
78+
} else if (headerValue.regionMatches(true, 0, "bearer", 0, "bearer".length())) {
79+
return 1;
80+
} else if (headerValue.regionMatches(true, 0, "basic", 0, "basic".length())) {
81+
return 2;
82+
} else {
83+
return 3;
5184
}
52-
this.defaultFailureResponseHeaders = Collections.unmodifiableMap(failureResponseHeaders);
5385
}
5486

5587
@Override

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/DefaultAuthenticationFailureHandlerTests.java

+23-4
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public void testAuthenticationRequired() {
5050
if (testDefault) {
5151
assertWWWAuthenticateWithSchemes(ese, basicAuthScheme);
5252
} else {
53-
assertWWWAuthenticateWithSchemes(ese, basicAuthScheme, bearerAuthScheme);
53+
assertWWWAuthenticateWithSchemes(ese, bearerAuthScheme, basicAuthScheme);
5454
}
5555
}
5656

@@ -83,12 +83,12 @@ public void testExceptionProcessingRequest() {
8383
assertThat(ese.getHeader("WWW-Authenticate"), is(notNullValue()));
8484
assertThat(ese, is(sameInstance(cause)));
8585
if (withAuthenticateHeader == false) {
86-
assertWWWAuthenticateWithSchemes(ese, basicAuthScheme, bearerAuthScheme, negotiateAuthScheme);
86+
assertWWWAuthenticateWithSchemes(ese, negotiateAuthScheme, bearerAuthScheme, basicAuthScheme);
8787
} else {
8888
if (selectedScheme.contains("Negotiate ")) {
8989
assertWWWAuthenticateWithSchemes(ese, selectedScheme);
9090
} else {
91-
assertWWWAuthenticateWithSchemes(ese, basicAuthScheme, bearerAuthScheme, negotiateAuthScheme);
91+
assertWWWAuthenticateWithSchemes(ese, negotiateAuthScheme, bearerAuthScheme, basicAuthScheme);
9292
}
9393
}
9494
assertThat(ese.getMessage(), equalTo("unauthorized"));
@@ -102,11 +102,30 @@ public void testExceptionProcessingRequest() {
102102
assertThat(ese, is(notNullValue()));
103103
assertThat(ese.getHeader("WWW-Authenticate"), is(notNullValue()));
104104
assertThat(ese.getMessage(), equalTo("error attempting to authenticate request"));
105-
assertWWWAuthenticateWithSchemes(ese, basicAuthScheme, bearerAuthScheme, negotiateAuthScheme);
105+
assertWWWAuthenticateWithSchemes(ese, negotiateAuthScheme, bearerAuthScheme, basicAuthScheme);
106106
}
107107

108108
}
109109

110+
public void testSortsWWWAuthenticateHeaderValues() {
111+
final String basicAuthScheme = "Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\"";
112+
final String bearerAuthScheme = "Bearer realm=\"" + XPackField.SECURITY + "\"";
113+
final String negotiateAuthScheme = randomFrom("Negotiate", "Negotiate Ijoijksdk");
114+
final Map<String, List<String>> failureResponeHeaders = new HashMap<>();
115+
final List<String> supportedSchemes = Arrays.asList(basicAuthScheme, bearerAuthScheme, negotiateAuthScheme);
116+
Collections.shuffle(supportedSchemes, random());
117+
failureResponeHeaders.put("WWW-Authenticate", supportedSchemes);
118+
final DefaultAuthenticationFailureHandler failuerHandler = new DefaultAuthenticationFailureHandler(failureResponeHeaders);
119+
120+
final ElasticsearchSecurityException ese = failuerHandler.exceptionProcessingRequest(Mockito.mock(RestRequest.class), null,
121+
new ThreadContext(Settings.builder().build()));
122+
123+
assertThat(ese, is(notNullValue()));
124+
assertThat(ese.getHeader("WWW-Authenticate"), is(notNullValue()));
125+
assertThat(ese.getMessage(), equalTo("error attempting to authenticate request"));
126+
assertWWWAuthenticateWithSchemes(ese, negotiateAuthScheme, bearerAuthScheme, basicAuthScheme);
127+
}
128+
110129
private void assertWWWAuthenticateWithSchemes(final ElasticsearchSecurityException ese, final String... schemes) {
111130
assertThat(ese.getHeader("WWW-Authenticate").size(), is(schemes.length));
112131
assertThat(ese.getHeader("WWW-Authenticate"), contains(schemes));

0 commit comments

Comments
 (0)