Skip to content

Commit 6565a6c

Browse files
markt-asfmarkt-asf
committed
Fix the handling of invalid users with DIGEST authentication
1 parent 6dd75be commit 6565a6c

4 files changed

Lines changed: 29 additions & 97 deletions

File tree

‎java/org/apache/catalina/realm/RealmBase.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,12 +1146,19 @@ protected String getDigest(String username, String realmName) {
11461146
* @return the digest for the specified user
11471147
*/
11481148
protected String getDigest(String username, String realmName, String algorithm) {
1149+
String password = getPassword(username);
1150+
1151+
// Short-cut null password case
1152+
if (password == null) {
1153+
return null;
1154+
}
1155+
11491156
if (hasMessageDigest(algorithm)) {
11501157
// Use pre-generated digest
1151-
return getPassword(username);
1158+
return password;
11521159
}
11531160

1154-
String digestValue = username + ":" + realmName + ":" + getPassword(username);
1161+
String digestValue = username + ":" + realmName + ":" + password;
11551162

11561163
byte[] valueBytes;
11571164
try {

‎test/org/apache/catalina/authenticator/TestDigestAuthenticatorAlgorithms.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,21 +194,21 @@ public void testDigestAuthentication() throws Exception {
194194
}
195195

196196

197-
protected static String getNonce(String authHeader) {
197+
private static String getNonce(String authHeader) {
198198
int start = authHeader.indexOf("nonce=\"") + 7;
199199
int end = authHeader.indexOf('\"', start);
200200
return authHeader.substring(start, end);
201201
}
202202

203203

204-
protected static String getOpaque(String authHeader) {
204+
private static String getOpaque(String authHeader) {
205205
int start = authHeader.indexOf("opaque=\"") + 8;
206206
int end = authHeader.indexOf('\"', start);
207207
return authHeader.substring(start, end);
208208
}
209209

210210

211-
private static String buildDigestResponse(String user, String pwd, String uri, String realm, AuthDigest algorithm,
211+
static String buildDigestResponse(String user, String pwd, String uri, String realm, AuthDigest algorithm,
212212
List<String> authHeaders, String nc, String cnonce, String qop) {
213213

214214
// Find auth header with correct algorithm

‎test/org/apache/catalina/authenticator/TestDigestAuthenticatorB.java‎

Lines changed: 14 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,9 @@
3535
import org.apache.catalina.startup.Tomcat;
3636
import org.apache.catalina.startup.TomcatBaseTest;
3737
import org.apache.tomcat.util.buf.ByteChunk;
38-
import org.apache.tomcat.util.buf.HexUtils;
3938
import org.apache.tomcat.util.descriptor.web.LoginConfig;
4039
import org.apache.tomcat.util.descriptor.web.SecurityCollection;
4140
import org.apache.tomcat.util.descriptor.web.SecurityConstraint;
42-
import org.apache.tomcat.util.security.ConcurrentMessageDigest;
4341

4442
@RunWith(Parameterized.class)
4543
public class TestDigestAuthenticatorB extends TomcatBaseTest {
@@ -55,8 +53,10 @@ public class TestDigestAuthenticatorB extends TomcatBaseTest {
5553
@Parameterized.Parameters(name = "{index}")
5654
public static Collection<Object[]> parameters() {
5755
List<Object[]> parameterSets = new ArrayList<>();
58-
parameterSets.add(new Object[] { validRole, validUser, validPassword });
59-
parameterSets.add(new Object[] { "**", validUser, validPassword });
56+
parameterSets.add(new Object[] { validRole, validUser, validPassword, Boolean.TRUE });
57+
parameterSets.add(new Object[] { "**", validUser, validPassword, Boolean.TRUE });
58+
parameterSets.add(new Object[] { "**", validUser, "null", Boolean.FALSE });
59+
parameterSets.add(new Object[] { "**", "invalid", "null", Boolean.FALSE });
6060
return parameterSets;
6161
}
6262

@@ -69,6 +69,9 @@ public static Collection<Object[]> parameters() {
6969
@Parameter(2)
7070
public String clientPassword;
7171

72+
@Parameter(3)
73+
public boolean validCredentials;
74+
7275

7376
@Test
7477
public void testDigestAuthentication() throws Exception {
@@ -115,99 +118,18 @@ public void testDigestAuthentication() throws Exception {
115118

116119
// Second request should
117120
List<String> auth = new ArrayList<>();
118-
auth.add(buildDigestResponse(clientUserName, clientPassword, targetURI, realmName, AuthDigest.SHA_256,
119-
respHeaders.get(AuthenticatorBase.AUTH_HEADER_NAME), "00000001", clientNonce, DigestAuthenticator.QOP));
121+
auth.add(TestDigestAuthenticatorAlgorithms.buildDigestResponse(clientUserName, clientPassword, targetURI,
122+
realmName, AuthDigest.SHA_256, respHeaders.get(AuthenticatorBase.AUTH_HEADER_NAME), "00000001",
123+
clientNonce, DigestAuthenticator.QOP));
120124
Map<String,List<String>> reqHeaders = new HashMap<>();
121125
reqHeaders.put("authorization", auth);
122126
rc = getUrl("http://localhost:" + getPort() + targetURI, bc, reqHeaders, null);
123127

124-
Assert.assertEquals(200, rc);
125-
Assert.assertEquals("OK", bc.toString());
126-
}
127-
128-
129-
protected static String getNonce(String authHeader) {
130-
int start = authHeader.indexOf("nonce=\"") + 7;
131-
int end = authHeader.indexOf('\"', start);
132-
return authHeader.substring(start, end);
133-
}
134-
135-
136-
protected static String getOpaque(String authHeader) {
137-
int start = authHeader.indexOf("opaque=\"") + 8;
138-
int end = authHeader.indexOf('\"', start);
139-
return authHeader.substring(start, end);
140-
}
141-
142-
143-
private static String buildDigestResponse(String user, String pwd, String uri, String realm, AuthDigest algorithm,
144-
List<String> authHeaders, String nc, String cnonce, String qop) {
145-
146-
// Find auth header with correct algorithm
147-
String nonce = null;
148-
String opaque = null;
149-
for (String authHeader : authHeaders) {
150-
nonce = getNonce(authHeader);
151-
opaque = getOpaque(authHeader);
152-
if (authHeader.contains("algorithm=" + algorithm.getRfcName())) {
153-
break;
154-
}
155-
}
156-
if (nonce == null || opaque == null) {
157-
Assert.fail();
158-
}
159-
160-
String a1 = user + ":" + realm + ":" + pwd;
161-
String a2 = "GET:" + uri;
162-
163-
String digestA1 = digest(algorithm.getJavaName(), a1);
164-
String digestA2 = digest(algorithm.getJavaName(), a2);
165-
166-
String response;
167-
if (qop == null) {
168-
response = digestA1 + ":" + nonce + ":" + digestA2;
128+
if (validCredentials) {
129+
Assert.assertEquals(200, rc);
130+
Assert.assertEquals("OK", bc.toString());
169131
} else {
170-
response = digestA1 + ":" + nonce + ":" + nc + ":" + cnonce + ":" + qop + ":" + digestA2;
171-
}
172-
173-
String digestResponse = digest(algorithm.getJavaName(), response);
174-
175-
StringBuilder auth = new StringBuilder();
176-
auth.append("Digest username=\"");
177-
auth.append(user);
178-
auth.append("\", realm=\"");
179-
auth.append(realm);
180-
auth.append("\", algorithm=");
181-
auth.append(algorithm.getRfcName());
182-
auth.append(", nonce=\"");
183-
auth.append(nonce);
184-
auth.append("\", uri=\"");
185-
auth.append(uri);
186-
auth.append("\", opaque=\"");
187-
auth.append(opaque);
188-
auth.append("\", response=\"");
189-
auth.append(digestResponse);
190-
auth.append("\"");
191-
if (qop != null) {
192-
auth.append(", qop=");
193-
auth.append(qop);
194-
auth.append("");
132+
Assert.assertEquals(401, rc);
195133
}
196-
if (nc != null) {
197-
auth.append(", nc=");
198-
auth.append(nc);
199-
}
200-
if (cnonce != null) {
201-
auth.append(", cnonce=\"");
202-
auth.append(cnonce);
203-
auth.append("\"");
204-
}
205-
206-
return auth.toString();
207-
}
208-
209-
210-
private static String digest(String algorithm, String input) {
211-
return HexUtils.toHexString(ConcurrentMessageDigest.digest(algorithm, input.getBytes()));
212134
}
213135
}

‎webapps/docs/changelog.xml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,9 @@
169169
<code>false</code>, meaning user names are treated in a case insensitive
170170
manner. (markt)
171171
</add>
172+
<fix>
173+
Correct the handling of invalid users with DIGEST authentication. (markt)
174+
</fix>
172175
</changelog>
173176
</subsection>
174177
<subsection name="Coyote">

0 commit comments

Comments
 (0)