152 lines
8.6 KiB
Diff
152 lines
8.6 KiB
Diff
From facb33a5cedaf4b7b96d3840a08210370a806870 Mon Sep 17 00:00:00 2001
|
|
From: Stuart Douglas <stuart.w.douglas@gmail.com>
|
|
Date: Tue, 3 Oct 2017 13:29:48 +0200
|
|
Subject: [PATCH] UNDERTOW-1190 client can use bogus uri in digest
|
|
authentication
|
|
|
|
---
|
|
.../impl/DigestAuthenticationMechanism.java | 19 +++++++-
|
|
.../DigestAuthenticationAuthTestCase.java | 45 ++++++++++++++++++-
|
|
.../security/digest/DigestAuthTestCase.java | 7 +--
|
|
3 files changed, 65 insertions(+), 6 deletions(-)
|
|
|
|
diff --git a/core/src/main/java/io/undertow/security/impl/DigestAuthenticationMechanism.java b/core/src/main/java/io/undertow/security/impl/DigestAuthenticationMechanism.java
|
|
index e5a75bd834..e01724b44b 100644
|
|
--- a/core/src/main/java/io/undertow/security/impl/DigestAuthenticationMechanism.java
|
|
+++ b/core/src/main/java/io/undertow/security/impl/DigestAuthenticationMechanism.java
|
|
@@ -42,6 +42,7 @@
|
|
import io.undertow.util.HeaderMap;
|
|
import io.undertow.util.Headers;
|
|
import io.undertow.util.HexConverter;
|
|
+import io.undertow.util.StatusCodes;
|
|
|
|
import java.nio.charset.StandardCharsets;
|
|
import java.security.MessageDigest;
|
|
@@ -181,7 +182,7 @@ public AuthenticationMechanismOutcome authenticate(final HttpServerExchange exch
|
|
return AuthenticationMechanismOutcome.NOT_ATTEMPTED;
|
|
}
|
|
|
|
- public AuthenticationMechanismOutcome handleDigestHeader(HttpServerExchange exchange, final SecurityContext securityContext) {
|
|
+ private AuthenticationMechanismOutcome handleDigestHeader(HttpServerExchange exchange, final SecurityContext securityContext) {
|
|
DigestContext context = exchange.getAttachment(DigestContext.ATTACHMENT_KEY);
|
|
Map<DigestAuthorizationToken, String> parsedHeader = context.getParsedHeader();
|
|
// Step 1 - Verify the set of tokens received to ensure valid values.
|
|
@@ -231,7 +232,21 @@ public AuthenticationMechanismOutcome handleDigestHeader(HttpServerExchange exch
|
|
return AuthenticationMechanismOutcome.NOT_AUTHENTICATED;
|
|
}
|
|
|
|
- // TODO - Validate the URI
|
|
+ if(parsedHeader.containsKey(DigestAuthorizationToken.DIGEST_URI)) {
|
|
+ String uri = parsedHeader.get(DigestAuthorizationToken.DIGEST_URI);
|
|
+ String requestURI = exchange.getRequestURI();
|
|
+ if(!exchange.getQueryString().isEmpty()) {
|
|
+ requestURI = requestURI + "?" + exchange.getQueryString();
|
|
+ }
|
|
+ if(!uri.equals(requestURI)) {
|
|
+ //just end the auth process
|
|
+ exchange.setStatusCode(StatusCodes.BAD_REQUEST);
|
|
+ exchange.endExchange();
|
|
+ return AuthenticationMechanismOutcome.NOT_AUTHENTICATED;
|
|
+ }
|
|
+ } else {
|
|
+ return AuthenticationMechanismOutcome.NOT_AUTHENTICATED;
|
|
+ }
|
|
|
|
if (parsedHeader.containsKey(DigestAuthorizationToken.OPAQUE)) {
|
|
if (!OPAQUE_VALUE.equals(parsedHeader.get(DigestAuthorizationToken.OPAQUE))) {
|
|
diff --git a/core/src/test/java/io/undertow/server/security/DigestAuthenticationAuthTestCase.java b/core/src/test/java/io/undertow/server/security/DigestAuthenticationAuthTestCase.java
|
|
index f9fa2b6aa4..d1aff564e1 100644
|
|
--- a/core/src/test/java/io/undertow/server/security/DigestAuthenticationAuthTestCase.java
|
|
+++ b/core/src/test/java/io/undertow/server/security/DigestAuthenticationAuthTestCase.java
|
|
@@ -163,7 +163,7 @@ private static String createAuthorizationLine(final String userName, final Strin
|
|
sb.append(DigestAuthorizationToken.USERNAME.getName()).append("=").append("\"userOne\"").append(",");
|
|
sb.append(DigestAuthorizationToken.REALM.getName()).append("=\"").append(REALM_NAME).append("\",");
|
|
sb.append(DigestAuthorizationToken.NONCE.getName()).append("=\"").append(nonce).append("\",");
|
|
- sb.append(DigestAuthorizationToken.DIGEST_URI.getName()).append("=\"/\",");
|
|
+ sb.append(DigestAuthorizationToken.DIGEST_URI.getName()).append("=\"" + uri + "\",");
|
|
String nonceCountHex = toHex(nonceCount);
|
|
String response = createResponse(userName, REALM_NAME, password, method, uri, nonce, nonceCountHex, cnonce);
|
|
sb.append(DigestAuthorizationToken.RESPONSE.getName()).append("=\"").append(response).append("\",");
|
|
@@ -243,6 +243,49 @@ static void _testDigestSuccess() throws Exception {
|
|
}
|
|
}
|
|
|
|
+ /**
|
|
+ * Test for a successful authentication.
|
|
+ *
|
|
+ * Also makes two additional calls to demonstrate nonce re-use with an incrementing nonce count.
|
|
+ */
|
|
+ @Test
|
|
+ public void testDigestBadUri() throws Exception {
|
|
+ _testDigestBadUri();
|
|
+ }
|
|
+
|
|
+ static void _testDigestBadUri() throws Exception {
|
|
+ TestHttpClient client = new TestHttpClient();
|
|
+ HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL());
|
|
+ HttpResponse result = client.execute(get);
|
|
+ assertEquals(StatusCodes.UNAUTHORIZED, result.getStatusLine().getStatusCode());
|
|
+ Header[] values = result.getHeaders(WWW_AUTHENTICATE.toString());
|
|
+ String value = getAuthHeader(DIGEST, values);
|
|
+
|
|
+ Map<DigestWWWAuthenticateToken, String> parsedHeader = DigestWWWAuthenticateToken.parseHeader(value.substring(7));
|
|
+ assertEquals(REALM_NAME, parsedHeader.get(DigestWWWAuthenticateToken.REALM));
|
|
+ assertEquals(DigestAlgorithm.MD5.getToken(), parsedHeader.get(DigestWWWAuthenticateToken.ALGORITHM));
|
|
+ assertEquals(DigestQop.AUTH.getToken(), parsedHeader.get(DigestWWWAuthenticateToken.MESSAGE_QOP));
|
|
+
|
|
+ String clientNonce = createNonce();
|
|
+ int nonceCount = 1;
|
|
+ String nonce = parsedHeader.get(DigestWWWAuthenticateToken.NONCE);
|
|
+ String opaque = parsedHeader.get(DigestWWWAuthenticateToken.OPAQUE);
|
|
+ assertNotNull(opaque);
|
|
+ // Send 5 requests with an incrementing nonce count on each call.
|
|
+ for (int i = 0; i < 5; i++) {
|
|
+ client = new TestHttpClient();
|
|
+ get = new HttpGet(DefaultServer.getDefaultServerURL());
|
|
+
|
|
+ int thisNonceCount = nonceCount++;
|
|
+ String authorization = createAuthorizationLine("userOne", "passwordOne", "GET", "/badUri", nonce, thisNonceCount,
|
|
+ clientNonce, opaque);
|
|
+
|
|
+ get.addHeader(AUTHORIZATION.toString(), authorization);
|
|
+ result = client.execute(get);
|
|
+ assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode());
|
|
+
|
|
+ }
|
|
+ }
|
|
/**
|
|
* Test for a failed authentication where a bad username is provided.
|
|
*/
|
|
diff --git a/servlet/src/test/java/io/undertow/servlet/test/security/digest/DigestAuthTestCase.java b/servlet/src/test/java/io/undertow/servlet/test/security/digest/DigestAuthTestCase.java
|
|
index d2ac85e0c6..7164634c47 100644
|
|
--- a/servlet/src/test/java/io/undertow/servlet/test/security/digest/DigestAuthTestCase.java
|
|
+++ b/servlet/src/test/java/io/undertow/servlet/test/security/digest/DigestAuthTestCase.java
|
|
@@ -119,7 +119,8 @@ public void testAuthType() throws Exception {
|
|
|
|
public void testCall(final String path, final String expectedResponse) throws Exception {
|
|
TestHttpClient client = new TestHttpClient();
|
|
- String url = DefaultServer.getDefaultServerURL() + "/servletContext/secured/" + path;
|
|
+ String servletPath = "/servletContext/secured/" + path;
|
|
+ String url = DefaultServer.getDefaultServerURL() + servletPath;
|
|
HttpGet get = new HttpGet(url);
|
|
HttpResponse result = client.execute(get);
|
|
assertEquals(StatusCodes.UNAUTHORIZED, result.getStatusLine().getStatusCode());
|
|
@@ -134,7 +135,7 @@ public void testCall(final String path, final String expectedResponse) throws Ex
|
|
|
|
String nonce = parsedHeader.get(DigestWWWAuthenticateToken.NONCE);
|
|
|
|
- String clientResponse = createResponse("user1", REALM_NAME, "password1", "GET", "/", nonce);
|
|
+ String clientResponse = createResponse("user1", REALM_NAME, "password1", "GET", servletPath, nonce);
|
|
|
|
client = new TestHttpClient();
|
|
get = new HttpGet(url);
|
|
@@ -143,7 +144,7 @@ public void testCall(final String path, final String expectedResponse) throws Ex
|
|
sb.append(DigestAuthorizationToken.USERNAME.getName()).append("=").append("\"user1\"").append(",");
|
|
sb.append(DigestAuthorizationToken.REALM.getName()).append("=\"").append(REALM_NAME).append("\",");
|
|
sb.append(DigestAuthorizationToken.NONCE.getName()).append("=\"").append(nonce).append("\",");
|
|
- sb.append(DigestAuthorizationToken.DIGEST_URI.getName()).append("=\"/\",");
|
|
+ sb.append(DigestAuthorizationToken.DIGEST_URI.getName()).append("=\"" + servletPath + "\",");
|
|
sb.append(DigestAuthorizationToken.RESPONSE.getName()).append("=\"").append(clientResponse).append("\"");
|
|
|
|
get.addHeader(AUTHORIZATION.toString(), sb.toString());
|