From facb33a5cedaf4b7b96d3840a08210370a806870 Mon Sep 17 00:00:00 2001 From: Stuart Douglas 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 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 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());