diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/util/DigestAuthentication.java b/jetty-client/src/main/java/org/eclipse/jetty/client/util/DigestAuthentication.java index 93683efd64dc..7fa11e4f4d6b 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/util/DigestAuthentication.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/util/DigestAuthentication.java @@ -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; @@ -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) @@ -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; @@ -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 @@ -177,8 +194,11 @@ 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(); @@ -186,7 +206,7 @@ public void apply(Request request) 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; @@ -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) @@ -224,6 +255,71 @@ public void apply(Request request) request.header(header, value.toString()); } + /** + *

Encodes the given value with the given {@link Charset}, failing if any + * character cannot be represented in that {@link Charset}.

+ *

{@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.

+ * + * @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"; diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java index 14fdafb4ecbd..d7338687a633 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java @@ -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; @@ -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 @@ -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 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(); diff --git a/jetty-client/src/test/resources/realm.properties b/jetty-client/src/test/resources/realm.properties index 095506f9efa6..12a26a677431 100644 --- a/jetty-client/src/test/resources/realm.properties +++ b/jetty-client/src/test/resources/realm.properties @@ -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 diff --git a/jetty-jaspi/src/main/java/org/eclipse/jetty/security/jaspi/modules/DigestAuthModule.java b/jetty-jaspi/src/main/java/org/eclipse/jetty/security/jaspi/modules/DigestAuthModule.java index 3bdfadb2787a..8316826fbfc1 100644 --- a/jetty-jaspi/src/main/java/org/eclipse/jetty/security/jaspi/modules/DigestAuthModule.java +++ b/jetty-jaspi/src/main/java/org/eclipse/jetty/security/jaspi/modules/DigestAuthModule.java @@ -19,7 +19,7 @@ package org.eclipse.jetty.security.jaspi.modules; import java.io.IOException; -import java.nio.charset.StandardCharsets; +import java.net.URI; import java.security.MessageDigest; import java.util.Base64; import java.util.Map; @@ -41,6 +41,8 @@ import org.eclipse.jetty.util.security.Constraint; import org.eclipse.jetty.util.security.Credential; +import static java.nio.charset.StandardCharsets.UTF_8; + @Deprecated public class DigestAuthModule extends BaseAuthModule { @@ -94,6 +96,10 @@ public AuthStatus validateRequest(MessageInfo messageInfo, Subject clientSubject if (LOG.isDebugEnabled()) LOG.debug("Credentials: " + credentials); QuotedStringTokenizer tokenizer = new QuotedStringTokenizer(credentials, "=, ", true, false); + // Only the double quote is special in a HTTP field value; the single quote + // is a literal character and must be preserved, for example in the + // RFC 5987 encoded username* parameter, whose value is charset'lang'chars. + tokenizer.setSingle(false); final Digest digest = new Digest(request.getMethod()); String last = null; String name = null; @@ -121,6 +127,8 @@ public AuthStatus validateRequest(MessageInfo messageInfo, Subject clientSubject { if ("username".equalsIgnoreCase(name)) digest.username = tok; + else if ("username*".equalsIgnoreCase(name)) + digest.usernameStar = tok; else if ("realm".equalsIgnoreCase(name)) digest.realm = tok; else if ("nonce".equalsIgnoreCase(name)) @@ -140,17 +148,20 @@ else if ("response".equalsIgnoreCase(name)) } } - int n = checkNonce(digest.nonce, timestamp); - - if (n > 0) + if (resolveUserName(digest)) { - if (login(clientSubject, digest.username, digest, Constraint.__DIGEST_AUTH, messageInfo)) + int n = checkNonce(digest.nonce, timestamp); + + if (n > 0) { - return AuthStatus.SUCCESS; + if (login(clientSubject, digest.username, digest, Constraint.__DIGEST_AUTH, messageInfo)) + { + return AuthStatus.SUCCESS; + } } + else if (n == 0) + stale = true; } - else if (n == 0) - stale = true; } if (!isMandatory(messageInfo)) @@ -163,7 +174,7 @@ else if (n == 0) response.setHeader(HttpHeader.WWW_AUTHENTICATE.asString(), "Digest realm=\"" + realmName + "\", domain=\"" + domain + "\", nonce=\"" + newNonce(timestamp) + - "\", algorithm=MD5, qop=\"auth\"" + (useStale ? (" stale=" + stale) : "")); + "\", algorithm=MD5, qop=\"auth\", charset=UTF-8" + (useStale ? (" stale=" + stale) : "")); response.sendError(HttpServletResponse.SC_UNAUTHORIZED); return AuthStatus.SEND_CONTINUE; } @@ -177,6 +188,39 @@ else if (n == 0) } } + /** + *

Resolves the effective username of the given digest, which may have been sent + * either as the {@code username} parameter or, for usernames that are not valid in + * a {@code quoted-string}, as the RFC 5987 encoded {@code username*} parameter.

+ * + * @param digest the digest data whose username must be resolved + * @return whether the username could be resolved + */ + private boolean resolveUserName(Digest digest) + { + if (digest.usernameStar == null) + return true; + + // RFC 7616[3.4]: username and username* are mutually exclusive. + if (digest.username != null) + return false; + + try + { + String encodedPrefix = "UTF-8''"; + if (!digest.usernameStar.regionMatches(true, 0, encodedPrefix, 0, encodedPrefix.length())) + return false; + digest.username = URI.create("scheme:" + digest.usernameStar.substring(encodedPrefix.length())).getSchemeSpecificPart(); + return digest.username != null; + } + catch (Throwable x) + { + // Likely a badly encoded username. + LOG.ignore(x); + return false; + } + } + public String newNonce(long ts) { // long ts=request.getTimeStamp(); @@ -279,6 +323,7 @@ private static class Digest extends Credential String method = null; String username = null; + String usernameStar = null; String realm = null; String nonce = null; String nc = null; @@ -311,18 +356,24 @@ public boolean check(Object credentials) else { // calc A1 digest - md.update(username.getBytes(StandardCharsets.ISO_8859_1)); + // RFC 7616[4]: hashing must be done with the charset advertised in + // the WWW-Authenticate header, which is UTF-8. ISO-8859-1 must not be + // used here: it silently maps every character above U+00FF to '?', + // so that different passwords produce the same H(A1), letting an + // attacker that knows the username authenticate with a colliding + // password. See GHSA-2fvj-hgj9-j2gr. + md.update(username.getBytes(UTF_8)); md.update((byte)':'); - md.update(realm.getBytes(StandardCharsets.ISO_8859_1)); + md.update(realm.getBytes(UTF_8)); md.update((byte)':'); - md.update(password.getBytes(StandardCharsets.ISO_8859_1)); + md.update(password.getBytes(UTF_8)); ha1 = md.digest(); } // calc A2 digest md.reset(); - md.update(method.getBytes(StandardCharsets.ISO_8859_1)); + md.update(method.getBytes(UTF_8)); md.update((byte)':'); - md.update(uri.getBytes(StandardCharsets.ISO_8859_1)); + md.update(uri.getBytes(UTF_8)); byte[] ha2 = md.digest(); // calc digest @@ -332,17 +383,17 @@ public boolean check(Object credentials) // request-digest = <"> < KD ( H(A1), unq(nonce-value) ":" H(A2) // ) > <"> - md.update(TypeUtil.toString(ha1, 16).getBytes(StandardCharsets.ISO_8859_1)); + md.update(TypeUtil.toString(ha1, 16).getBytes(UTF_8)); md.update((byte)':'); - md.update(nonce.getBytes(StandardCharsets.ISO_8859_1)); + md.update(nonce.getBytes(UTF_8)); md.update((byte)':'); - md.update(nc.getBytes(StandardCharsets.ISO_8859_1)); + md.update(nc.getBytes(UTF_8)); md.update((byte)':'); - md.update(cnonce.getBytes(StandardCharsets.ISO_8859_1)); + md.update(cnonce.getBytes(UTF_8)); md.update((byte)':'); - md.update(qop.getBytes(StandardCharsets.ISO_8859_1)); + md.update(qop.getBytes(UTF_8)); md.update((byte)':'); - md.update(TypeUtil.toString(ha2, 16).getBytes(StandardCharsets.ISO_8859_1)); + md.update(TypeUtil.toString(ha2, 16).getBytes(UTF_8)); byte[] digest = md.digest(); // check digest diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java index 05a673561160..b46179b98fe2 100644 --- a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java +++ b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java @@ -19,7 +19,7 @@ package org.eclipse.jetty.security.authentication; import java.io.IOException; -import java.nio.charset.StandardCharsets; +import java.net.URI; import java.security.MessageDigest; import java.security.SecureRandom; import java.util.Base64; @@ -49,6 +49,8 @@ import org.eclipse.jetty.util.security.Constraint; import org.eclipse.jetty.util.security.Credential; +import static java.nio.charset.StandardCharsets.UTF_8; + /** * The nonce max age in ms can be set with the {@link SecurityHandler#setInitParameter(String, String)} * using the name "maxNonceAge". The nonce max count can be set with {@link SecurityHandler#setInitParameter(String, String)} @@ -129,6 +131,10 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b if (LOG.isDebugEnabled()) LOG.debug("Credentials: " + credentials); QuotedStringTokenizer tokenizer = new QuotedStringTokenizer(credentials, "=, ", true, false); + // Only the double quote is special in a HTTP field value; the single quote + // is a literal character and must be preserved, for example in the + // RFC 5987 encoded username* parameter, whose value is charset'lang'chars. + tokenizer.setSingle(false); final Digest digest = new Digest(request.getMethod()); String last = null; String name = null; @@ -156,6 +162,8 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b { if ("username".equalsIgnoreCase(name)) digest.username = tok; + else if ("username*".equalsIgnoreCase(name)) + digest.usernameStar = tok; else if ("realm".equalsIgnoreCase(name)) digest.realm = tok; else if ("nonce".equalsIgnoreCase(name)) @@ -175,19 +183,22 @@ else if ("response".equalsIgnoreCase(name)) } } - int n = checkNonce(digest, baseRequest); - - if (n > 0) + if (resolveUserName(digest)) { - //UserIdentity user = _loginService.login(digest.username,digest); - UserIdentity user = login(digest.username, digest, req); - if (user != null) + int n = checkNonce(digest, baseRequest); + + if (n > 0) { - return new UserAuthentication(getAuthMethod(), user); + //UserIdentity user = _loginService.login(digest.username,digest); + UserIdentity user = login(digest.username, digest, req); + if (user != null) + { + return new UserAuthentication(getAuthMethod(), user); + } } + else if (n == 0) + stale = true; } - else if (n == 0) - stale = true; } if (!DeferredAuthentication.isDeferred(response)) @@ -200,6 +211,7 @@ else if (n == 0) "\", nonce=\"" + newNonce(baseRequest) + "\", algorithm=MD5" + ", qop=\"auth\"" + + ", charset=UTF-8" + ", stale=" + stale); response.sendError(HttpServletResponse.SC_UNAUTHORIZED); @@ -214,6 +226,60 @@ else if (n == 0) } } + /** + *

Resolves the effective username of the given digest, which may have been sent + * either as the {@code username} parameter or, for usernames that are not valid in + * a {@code quoted-string}, as the RFC 5987 encoded {@code username*} parameter.

+ *

On success, {@link Digest#username} holds the plain username, which is what + * both {@code H(A1)} and the {@link org.eclipse.jetty.security.LoginService} use.

+ * + * @param digest the digest data whose username must be resolved + * @return whether the username could be resolved + */ + private boolean resolveUserName(Digest digest) + { + if (digest.usernameStar == null) + return true; + + // RFC 7616[3.4]: username and username* are mutually exclusive. + if (!digest.username.isEmpty()) + return false; + + String username = resolveEncodedUserName(digest.usernameStar); + if (username == null) + return false; + + digest.username = username; + return true; + } + + /** + *

Resolves the encoded username received in the {@code Authorization} header.

+ *

The username is encoded with RFC 5987 and this method decodes it.

+ *

This method only decodes RFC 5987 usernames encoded with UTF-8 and no + * language; for example {@code UTF-8''caf%C3%A9} is decoded as {@code cafe} + * with an acute accent on the last character.

+ * + * @param encodedUserName the encoded username + * @return the decoded username, or {@code null} if it could not be decoded + */ + protected String resolveEncodedUserName(String encodedUserName) + { + try + { + String encodedPrefix = "UTF-8''"; + if (!encodedUserName.regionMatches(true, 0, encodedPrefix, 0, encodedPrefix.length())) + return null; + return URI.create("scheme:" + encodedUserName.substring(encodedPrefix.length())).getSchemeSpecificPart(); + } + catch (Throwable x) + { + // Likely a badly encoded username. + LOG.ignore(x); + return null; + } + } + @Override public UserIdentity login(String username, Object credentials, ServletRequest request) { @@ -311,6 +377,7 @@ private static class Digest extends Credential private static final long serialVersionUID = -2484639019549527724L; final String method; String username = ""; + String usernameStar; String realm = ""; String nonce = ""; String nc = ""; @@ -345,18 +412,25 @@ public boolean check(Object credentials) else { // calc A1 digest - md.update(username.getBytes(StandardCharsets.ISO_8859_1)); + // RFC 7616[4]: hashing must be done with the charset advertised in + // the WWW-Authenticate header, which is UTF-8. ISO-8859-1 must not be + // used here: it silently maps every character above U+00FF to '?', + // so that a two character password made of characters above U+00FF + // and the password "??" produce the same H(A1), letting an attacker + // that knows the username authenticate with a colliding password. + // See GHSA-2fvj-hgj9-j2gr. + md.update(username.getBytes(UTF_8)); md.update((byte)':'); - md.update(realm.getBytes(StandardCharsets.ISO_8859_1)); + md.update(realm.getBytes(UTF_8)); md.update((byte)':'); - md.update(password.getBytes(StandardCharsets.ISO_8859_1)); + md.update(password.getBytes(UTF_8)); ha1 = md.digest(); } // calc A2 digest md.reset(); - md.update(method.getBytes(StandardCharsets.ISO_8859_1)); + md.update(method.getBytes(UTF_8)); md.update((byte)':'); - md.update(uri.getBytes(StandardCharsets.ISO_8859_1)); + md.update(uri.getBytes(UTF_8)); byte[] ha2 = md.digest(); // calc digest @@ -366,17 +440,17 @@ public boolean check(Object credentials) // request-digest = <"> < KD ( H(A1), unq(nonce-value) ":" H(A2) // ) > <"> - md.update(TypeUtil.toString(ha1, 16).getBytes(StandardCharsets.ISO_8859_1)); + md.update(TypeUtil.toString(ha1, 16).getBytes(UTF_8)); md.update((byte)':'); - md.update(nonce.getBytes(StandardCharsets.ISO_8859_1)); + md.update(nonce.getBytes(UTF_8)); md.update((byte)':'); - md.update(nc.getBytes(StandardCharsets.ISO_8859_1)); + md.update(nc.getBytes(UTF_8)); md.update((byte)':'); - md.update(cnonce.getBytes(StandardCharsets.ISO_8859_1)); + md.update(cnonce.getBytes(UTF_8)); md.update((byte)':'); - md.update(qop.getBytes(StandardCharsets.ISO_8859_1)); + md.update(qop.getBytes(UTF_8)); md.update((byte)':'); - md.update(TypeUtil.toString(ha2, 16).getBytes(StandardCharsets.ISO_8859_1)); + md.update(TypeUtil.toString(ha2, 16).getBytes(UTF_8)); byte[] digest = md.digest(); // check digest diff --git a/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java b/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java index 824542fd5976..9bb202f9c460 100644 --- a/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java +++ b/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.security; import java.io.IOException; +import java.nio.charset.Charset; import java.security.MessageDigest; import java.util.ArrayList; import java.util.Arrays; @@ -74,6 +75,7 @@ import org.junit.jupiter.params.provider.MethodSource; import static java.nio.charset.StandardCharsets.ISO_8859_1; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; @@ -90,6 +92,8 @@ public class ConstraintTest { private static final String TEST_REALM = "TestRealm"; + private static final String PASSWORD_UTF8 = "密码"; // Chinese for "password", not representable in ISO-8859-1. + private static final String USERNAME_UTF8 = "用户"; // Chinese for "user", not representable in ISO-8859-1. private Server _server; private LocalConnector _connector; private ConstraintSecurityHandler _security; @@ -121,6 +125,8 @@ public void setupServer() loginService.putUser("user2", new Password("password"), new String[]{"user"}); loginService.putUser("admin", new Password("password"), new String[]{"user", "administrator"}); loginService.putUser("user3", new Password("password"), new String[]{"foo"}); + loginService.putUser("utf8user", new Password(PASSWORD_UTF8), new String[]{"user"}); + loginService.putUser(USERNAME_UTF8, new Password("password"), new String[]{"user"}); contextHandler.setContextPath("/ctx"); _server.setHandler(contextHandler); @@ -779,21 +785,32 @@ public void testBasic(Scenario scenario) throws Exception private static String CNONCE = "1234567890"; private String digest(String nonce, String username, String password, String uri, String nc) throws Exception + { + // RFC 7616 clients hash with the charset advertised by the server, which is UTF-8. + return digest(UTF_8, nonce, username, password, uri, nc); + } + + /** + * Computes the digest {@code response} parameter with an explicit {@link Charset}, + * so that tests can simulate both RFC 7616 clients (UTF-8) and legacy RFC 2617 + * clients (ISO-8859-1). + */ + private String digest(Charset charset, String nonce, String username, String password, String uri, String nc) throws Exception { MessageDigest md = MessageDigest.getInstance("MD5"); byte[] ha1; // calc A1 digest - md.update(username.getBytes(ISO_8859_1)); + md.update(username.getBytes(charset)); md.update((byte)':'); - md.update("TestRealm".getBytes(ISO_8859_1)); + md.update("TestRealm".getBytes(charset)); md.update((byte)':'); - md.update(password.getBytes(ISO_8859_1)); + md.update(password.getBytes(charset)); ha1 = md.digest(); // calc A2 digest md.reset(); - md.update("GET".getBytes(ISO_8859_1)); + md.update("GET".getBytes(charset)); md.update((byte)':'); - md.update(uri.getBytes(ISO_8859_1)); + md.update(uri.getBytes(charset)); byte[] ha2 = md.digest(); // calc digest @@ -803,23 +820,47 @@ private String digest(String nonce, String username, String password, String uri // request-digest = <"> < KD ( H(A1), unq(nonce-value) ":" H(A2) // ) > <"> - md.update(TypeUtil.toString(ha1, 16).getBytes(ISO_8859_1)); + md.update(TypeUtil.toString(ha1, 16).getBytes(charset)); md.update((byte)':'); - md.update(nonce.getBytes(ISO_8859_1)); + md.update(nonce.getBytes(charset)); md.update((byte)':'); - md.update(nc.getBytes(ISO_8859_1)); + md.update(nc.getBytes(charset)); md.update((byte)':'); - md.update(CNONCE.getBytes(ISO_8859_1)); + md.update(CNONCE.getBytes(charset)); md.update((byte)':'); - md.update("auth".getBytes(ISO_8859_1)); + md.update("auth".getBytes(charset)); md.update((byte)':'); - md.update(TypeUtil.toString(ha2, 16).getBytes(ISO_8859_1)); + md.update(TypeUtil.toString(ha2, 16).getBytes(charset)); byte[] digest = md.digest(); // check digest return TypeUtil.toString(digest, 16); } + private String startDigestAndGetNonce() throws Exception + { + DigestAuthenticator authenticator = new DigestAuthenticator(); + authenticator.setMaxNonceCount(5); + _security.setAuthenticator(authenticator); + _server.start(); + + String response = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n\r\n"); + assertThat(response, startsWith("HTTP/1.1 401 Unauthorized")); + // RFC 7616[3.3]: the server must tell the client which charset to hash with. + assertThat(response, containsString("charset=UTF-8")); + + Matcher matcher = Pattern.compile("nonce=\"([^\"]*)\",").matcher(response); + assertTrue(matcher.find()); + return matcher.group(1); + } + + private String digestRequest(String authorization) throws Exception + { + return _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n" + + "Authorization: " + authorization + "\r\n" + + "\r\n"); + } + @Test public void testDigest() throws Exception { @@ -905,6 +946,70 @@ public void testDigest() throws Exception assertThat(response, containsString("stale=true")); } + /** + * Regression test for GHSA-2fvj-hgj9-j2gr (CVE-2026-10050): hashing the digest + * parameters with ISO-8859-1 silently replaces every character above U+00FF with + * {@code '?'}, so an attacker that knows the username of a user whose password + * contains such characters can authenticate with a colliding password made of + * {@code '?'} characters. + */ + @Test + public void testDigestNonLatin1PasswordCollisionIsRejected() throws Exception + { + String nonce = startDigestAndGetNonce(); + + // The colliding password produces exactly the same digest as the real + // password when the parameters are hashed with the lossy ISO-8859-1. + String collidingResponse = digest(ISO_8859_1, nonce, "utf8user", "??", "/ctx/auth/info", "1"); + assertThat(collidingResponse, is(digest(ISO_8859_1, nonce, "utf8user", PASSWORD_UTF8, "/ctx/auth/info", "1"))); + + String response = digestRequest("Digest username=\"utf8user\", qop=auth, cnonce=\"" + CNONCE + "\", " + + "uri=\"/ctx/auth/info\", realm=\"TestRealm\", nc=1, nonce=\"" + nonce + "\", " + + "response=\"" + collidingResponse + "\""); + assertThat(response, startsWith("HTTP/1.1 401 Unauthorized")); + + // The real password, hashed with ISO-8859-1 as a vulnerable client would, + // must not authenticate either. + response = digestRequest("Digest username=\"utf8user\", qop=auth, cnonce=\"" + CNONCE + "\", " + + "uri=\"/ctx/auth/info\", realm=\"TestRealm\", nc=2, nonce=\"" + nonce + "\", " + + "response=\"" + digest(ISO_8859_1, nonce, "utf8user", PASSWORD_UTF8, "/ctx/auth/info", "2") + "\""); + assertThat(response, startsWith("HTTP/1.1 401 Unauthorized")); + + // The real password, hashed with UTF-8 as the challenge asked for, authenticates. + response = digestRequest("Digest username=\"utf8user\", qop=auth, cnonce=\"" + CNONCE + "\", " + + "uri=\"/ctx/auth/info\", realm=\"TestRealm\", nc=3, nonce=\"" + nonce + "\", " + + "response=\"" + digest(UTF_8, nonce, "utf8user", PASSWORD_UTF8, "/ctx/auth/info", "3") + "\""); + assertThat(response, startsWith("HTTP/1.1 200 OK")); + } + + /** + * A username that cannot be carried in a {@code quoted-string} is sent RFC 5987 + * encoded in the {@code username*} parameter, and must be decoded before both the + * {@code H(A1)} computation and the login service lookup. + */ + @Test + public void testDigestNonLatin1UserName() throws Exception + { + String nonce = startDigestAndGetNonce(); + + String response = digestRequest("Digest username*=UTF-8''%E7%94%A8%E6%88%B7, qop=auth, cnonce=\"" + CNONCE + "\", " + + "uri=\"/ctx/auth/info\", realm=\"TestRealm\", nc=1, nonce=\"" + nonce + "\", " + + "response=\"" + digest(UTF_8, nonce, USERNAME_UTF8, "password", "/ctx/auth/info", "1") + "\""); + assertThat(response, startsWith("HTTP/1.1 200 OK")); + + // RFC 7616[3.4]: username and username* are mutually exclusive. + response = digestRequest("Digest username=\"utf8user\", username*=UTF-8''%E7%94%A8%E6%88%B7, " + + "qop=auth, cnonce=\"" + CNONCE + "\", uri=\"/ctx/auth/info\", realm=\"TestRealm\", nc=2, nonce=\"" + nonce + "\", " + + "response=\"" + digest(UTF_8, nonce, USERNAME_UTF8, "password", "/ctx/auth/info", "2") + "\""); + assertThat(response, startsWith("HTTP/1.1 401 Unauthorized")); + + // A username* that is not UTF-8 encoded is rejected. + response = digestRequest("Digest username*=ISO-8859-1''%E7%94%A8%E6%88%B7, qop=auth, cnonce=\"" + CNONCE + "\", " + + "uri=\"/ctx/auth/info\", realm=\"TestRealm\", nc=3, nonce=\"" + nonce + "\", " + + "response=\"" + digest(UTF_8, nonce, USERNAME_UTF8, "password", "/ctx/auth/info", "3") + "\""); + assertThat(response, startsWith("HTTP/1.1 401 Unauthorized")); + } + @Test public void testFormDispatch() throws Exception {