Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
package org.eclipse.jetty.client.util;

import java.net.URI;
import java.nio.ByteBuffer;
import java.nio.CharBuffer;
import java.nio.charset.CharacterCodingException;
import java.nio.charset.Charset;
import java.nio.charset.CodingErrorAction;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
Expand Down Expand Up @@ -120,10 +125,15 @@ else if (serverQOPValues.contains("auth-int"))
clientQOP = "auth-int";
}

// RFC 7616[3.3]: the only allowed value for the charset parameter is "UTF-8".
// Servers that do not send the charset parameter (RFC 2617) imply ISO-8859-1.
String charsetName = params.get("charset");
Charset charset = "UTF-8".equalsIgnoreCase(charsetName) ? StandardCharsets.UTF_8 : null;

String realm = getRealm();
if (ANY_REALM.equals(realm))
realm = headerInfo.getRealm();
return new DigestResult(headerInfo.getHeader(), response.getContent(), realm, user, password, algorithm, nonce, clientQOP, opaque);
return new DigestResult(headerInfo.getHeader(), response.getContent(), realm, user, password, algorithm, nonce, clientQOP, opaque, charset);
}

private MessageDigest getMessageDigest(String algorithm)
Expand All @@ -150,8 +160,14 @@ private class DigestResult implements Result
private final String nonce;
private final String qop;
private final String opaque;
private final Charset charset;

public DigestResult(HttpHeader header, byte[] content, String realm, String user, String password, String algorithm, String nonce, String qop, String opaque)
{
this(header, content, realm, user, password, algorithm, nonce, qop, opaque, null);
}

private DigestResult(HttpHeader header, byte[] content, String realm, String user, String password, String algorithm, String nonce, String qop, String opaque, Charset charset)
{
this.header = header;
this.content = content;
Expand All @@ -162,6 +178,7 @@ public DigestResult(HttpHeader header, byte[] content, String realm, String user
this.nonce = nonce;
this.qop = qop;
this.opaque = opaque;
this.charset = charset;
}

@Override
Expand All @@ -177,16 +194,19 @@ public void apply(Request request)
if (digester == null)
return;

// Retain ISO-8859-1 for RFC 2617 servers that do not send the charset parameter.
Charset cs = (charset == null) ? StandardCharsets.ISO_8859_1 : charset;

String a1 = user + ":" + realm + ":" + password;
String hashA1 = toHexString(digester.digest(a1.getBytes(StandardCharsets.ISO_8859_1)));
String hashA1 = toHexString(digester.digest(strictEncode(cs, a1)));

String query = request.getQuery();
String path = request.getPath();
String uri = (query == null) ? path : path + "?" + query;
String a2 = request.getMethod() + ":" + uri;
if ("auth-int".equals(qop))
a2 += ":" + toHexString(digester.digest(content));
String hashA2 = toHexString(digester.digest(a2.getBytes(StandardCharsets.ISO_8859_1)));
String hashA2 = toHexString(digester.digest(strictEncode(cs, a2)));

String nonceCount;
String clientNonce;
Expand All @@ -203,10 +223,21 @@ public void apply(Request request)
clientNonce = null;
a3 = hashA1 + ":" + nonce + ":" + hashA2;
}
String hashA3 = toHexString(digester.digest(a3.getBytes(StandardCharsets.ISO_8859_1)));
String hashA3 = toHexString(digester.digest(strictEncode(cs, a3)));

StringBuilder value = new StringBuilder("Digest");
value.append(" username=\"").append(user).append("\"");
if (userNameNeedsEncoding(user))
{
// RFC 7616[4]: usernames that are not valid in a quoted-string must be
// sent with the username* parameter, which requires charset=UTF-8.
if (charset == null)
throw new IllegalArgumentException("Unsupported username: " + user);
value.append(" username*=").append(encodeUserName(user, charset));
}
else
{
value.append(" username=\"").append(user).append("\"");
}
value.append(", realm=\"").append(realm).append("\"");
value.append(", nonce=\"").append(nonce).append("\"");
if (opaque != null)
Expand All @@ -224,6 +255,71 @@ public void apply(Request request)
request.header(header, value.toString());
}

/**
* <p>Encodes the given value with the given {@link Charset}, failing if any
* character cannot be represented in that {@link Charset}.</p>
* <p>{@link String#getBytes(Charset)} silently replaces unmappable characters
* with {@code '?'} (byte {@code 0x3F}), so that a two character password made
* of characters above U+00FF and the password {@code "??"} produce the same
* ISO-8859-1 bytes, and therefore the same digest, allowing an attacker that
* knows the username to authenticate with a colliding password.
* See GHSA-2fvj-hgj9-j2gr.</p>
*
* @param charset the {@link Charset} to encode with
* @param value the value to encode
* @return the encoded bytes
* @throws IllegalArgumentException if the value cannot be encoded without loss
*/
private byte[] strictEncode(Charset charset, String value)
{
try
{
ByteBuffer byteBuffer = charset.newEncoder()
.onMalformedInput(CodingErrorAction.REPORT)
.onUnmappableCharacter(CodingErrorAction.REPORT)
.encode(CharBuffer.wrap(value));
byte[] bytes = new byte[byteBuffer.remaining()];
byteBuffer.get(bytes);
return bytes;
}
catch (CharacterCodingException x)
{
throw new IllegalArgumentException("Could not encode digest parameters with charset " + charset.name(), x);
}
}

private boolean userNameNeedsEncoding(String user)
{
// Should be RFC 7230 quoted-string, but use here a simplified version.
for (int i = 0; i < user.length(); ++i)
{
char c = user.charAt(i);
if (c < 0x20 || c > 0x7E || c == '"' || c == '\\')
return true;
}
return false;
}

private String encodeUserName(String user, Charset charset)
{
// RFC 5987 extended value: charset "'" [ language ] "'" value-chars.
byte[] bytes = strictEncode(charset, user);
StringBuilder builder = new StringBuilder(charset.name()).append("''");
for (byte b : bytes)
{
int c = b & 0xFF;
boolean unreserved = (c >= 'A' && c <= 'Z') ||
(c >= 'a' && c <= 'z') ||
(c >= '0' && c <= '9') ||
c == '-' || c == '.' || c == '_' || c == '~';
if (unreserved)
builder.append((char)c);
else
builder.append(String.format("%%%02X", c));
}
return builder.toString();
}

private String nextNonceCount()
{
String padding = "00000000";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.List;
import java.util.NoSuchElementException;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
Expand Down Expand Up @@ -72,9 +73,11 @@
import static org.eclipse.jetty.client.api.Authentication.ANY_REALM;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalToIgnoringCase;
import static org.hamcrest.Matchers.instanceOf;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest
Expand Down Expand Up @@ -179,6 +182,82 @@ public void testDigestAnyRealm(Scenario scenario) throws Exception
testAuthentication(scenario, new DigestAuthentication(uri, ANY_REALM, "digest", "digest"));
}

/**
* A password that cannot be represented in ISO-8859-1 must be hashed with the
* charset the server advertises (UTF-8), see GHSA-2fvj-hgj9-j2gr.
*/
@ParameterizedTest
@ArgumentsSource(ScenarioProvider.class)
public void testDigestWithNonLatin1Password(Scenario scenario) throws Exception
{
startDigest(scenario, new EmptyServerHandler());
URI uri = URI.create(scenario.getScheme() + "://localhost:" + connector.getLocalPort());
// @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck
testAuthentication(scenario, new DigestAuthentication(uri, realm, "digest_utf8", "\u5BC6\u7801"));
}

/**
* A user name that cannot be carried in a {@code quoted-string} must be sent
* RFC 5987 encoded in the {@code username*} parameter.
*/
@ParameterizedTest
@ArgumentsSource(ScenarioProvider.class)
public void testDigestWithNonLatin1UserName(Scenario scenario) throws Exception
{
startDigest(scenario, new EmptyServerHandler());
URI uri = URI.create(scenario.getScheme() + "://localhost:" + connector.getLocalPort());
// @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck
testAuthentication(scenario, new DigestAuthentication(uri, realm, "\u7528\u6237", "digest"));
}

/**
* A server that does not advertise a charset implies the lossy ISO-8859-1, in which
* a password above U+00FF cannot be represented; rather than silently hashing a
* {@code '?'} for every such character - which produces a digest that collides with
* the digest of a password made of literal {@code '?'} characters - the client must
* fail the request, see GHSA-2fvj-hgj9-j2gr.
*/
@ParameterizedTest
@ArgumentsSource(ScenarioProvider.class)
public void testDigestNonLatin1PasswordWithoutCharsetFailsRatherThanColliding(Scenario scenario) throws Exception
{
AtomicReference<String> authorization = new AtomicReference<>();
start(scenario, new EmptyServerHandler()
{
@Override
protected void service(String target, org.eclipse.jetty.server.Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
String credentials = request.getHeader(HttpHeader.AUTHORIZATION.asString());
if (credentials == null)
{
// An RFC 2617 challenge, without the charset parameter.
response.setHeader(HttpHeader.WWW_AUTHENTICATE.asString(),
"Digest realm=\"" + realm + "\", nonce=\"1234567890\", algorithm=MD5, qop=\"auth\"");
response.setStatus(HttpStatus.UNAUTHORIZED_401);
}
else
{
authorization.set(credentials);
}
}
});

URI uri = URI.create(scenario.getScheme() + "://localhost:" + connector.getLocalPort());
// @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck
client.getAuthenticationStore().addAuthentication(new DigestAuthentication(uri, realm, "digest_utf8", "\u5BC6\u7801"));

ExecutionException failure = assertThrows(ExecutionException.class, () ->
client.newRequest("localhost", connector.getLocalPort())
.scheme(scenario.getScheme())
.path("/secure")
.timeout(5, TimeUnit.SECONDS)
.send());
assertThat(failure.getCause(), instanceOf(IllegalArgumentException.class));

// The client must not have sent any Authorization header at all.
assertNull(authorization.get());
}

private void testAuthentication(Scenario scenario, Authentication authentication) throws Exception
{
AuthenticationStore authenticationStore = client.getAuthenticationStore();
Expand Down
4 changes: 4 additions & 0 deletions jetty-client/src/test/resources/realm.properties
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@
basic:basic
basic_utf8:\u20AC
digest:digest
# A password and a user name that cannot be represented in ISO-8859-1, see GHSA-2fvj-hgj9-j2gr.
# \u5BC6\u7801 is Chinese for "password", \u7528\u6237 is Chinese for "user".
digest_utf8:\u5BC6\u7801
\u7528\u6237:digest
spnego_client:,admin
Loading