Skip to content

Commit 206921a

Browse files
markt-asfmarkt-asf
committed
Fix a bug in the handling of the special ** role
Requests to URIs with a constraint that only specified the ** role were receiving 403 responses rather than 401.
1 parent 930d524 commit 206921a

File tree

3 files changed

+223
-2
lines changed

3 files changed

+223
-2
lines changed

‎java/org/apache/catalina/authenticator/AuthenticatorBase.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -529,9 +529,11 @@ public void invoke(Request request, Response response) throws IOException, Servl
529529
if (constraints != null) {
530530
hasAuthConstraint = true;
531531
for (int i = 0; i < constraints.length && hasAuthConstraint; i++) {
532-
if (!constraints[i].getAuthConstraint()) {
532+
if (constraints[i].getAllRoles() || constraints[i].getAuthenticatedUsers()) {
533+
// NO-OP - has hasAuthConstraint
534+
} else if (!constraints[i].getAuthConstraint()) {
533535
hasAuthConstraint = false;
534-
} else if (!constraints[i].getAllRoles() && !constraints[i].getAuthenticatedUsers()) {
536+
} else {
535537
String[] roles = constraints[i].findAuthRoles();
536538
if (roles == null || roles.length == 0) {
537539
hasAuthConstraint = false;

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

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.catalina.authenticator;
18+
19+
import java.util.ArrayList;
20+
import java.util.Collection;
21+
import java.util.HashMap;
22+
import java.util.List;
23+
import java.util.Map;
24+
25+
import org.junit.Assert;
26+
import org.junit.Test;
27+
import org.junit.runner.RunWith;
28+
import org.junit.runners.Parameterized;
29+
import org.junit.runners.Parameterized.Parameter;
30+
31+
import org.apache.catalina.Context;
32+
import org.apache.catalina.authenticator.DigestAuthenticator.AuthDigest;
33+
import org.apache.catalina.startup.TesterMapRealm;
34+
import org.apache.catalina.startup.TesterServlet;
35+
import org.apache.catalina.startup.Tomcat;
36+
import org.apache.catalina.startup.TomcatBaseTest;
37+
import org.apache.tomcat.util.buf.ByteChunk;
38+
import org.apache.tomcat.util.buf.HexUtils;
39+
import org.apache.tomcat.util.descriptor.web.LoginConfig;
40+
import org.apache.tomcat.util.descriptor.web.SecurityCollection;
41+
import org.apache.tomcat.util.descriptor.web.SecurityConstraint;
42+
import org.apache.tomcat.util.security.ConcurrentMessageDigest;
43+
44+
@RunWith(Parameterized.class)
45+
public class TestDigestAuthenticatorB extends TomcatBaseTest {
46+
47+
private static final String targetURI = "/test";
48+
private static final String validUser = "user";
49+
private static final String validPassword = "password";
50+
private static final String validRole = "role";
51+
private static final String realmName = "realm";
52+
private static final String clientNonce = "cnonce";
53+
54+
55+
@Parameterized.Parameters(name = "{index}")
56+
public static Collection<Object[]> parameters() {
57+
List<Object[]> parameterSets = new ArrayList<>();
58+
parameterSets.add(new Object[] { validRole, validUser, validPassword });
59+
parameterSets.add(new Object[] { "**", validUser, validPassword });
60+
return parameterSets;
61+
}
62+
63+
@Parameter(0)
64+
public String serverPermittedRole;
65+
66+
@Parameter(1)
67+
public String clientUserName;
68+
69+
@Parameter(2)
70+
public String clientPassword;
71+
72+
73+
@Test
74+
public void testDigestAuthentication() throws Exception {
75+
// Configure a context with digest authentication and a single protected resource
76+
Tomcat tomcat = getTomcatInstance();
77+
78+
// No file system docBase required
79+
Context ctxt = getProgrammaticRootContext();
80+
81+
// Add protected servlet
82+
Tomcat.addServlet(ctxt, "TesterServlet", new TesterServlet());
83+
ctxt.addServletMappingDecoded(targetURI, "TesterServlet");
84+
SecurityCollection collection = new SecurityCollection();
85+
collection.addPatternDecoded(targetURI);
86+
SecurityConstraint sc = new SecurityConstraint();
87+
sc.addAuthRole(serverPermittedRole);
88+
sc.addCollection(collection);
89+
ctxt.addConstraint(sc);
90+
91+
// Configure the Realm
92+
TesterMapRealm realm = new TesterMapRealm();
93+
realm.addUser(validUser, validPassword);
94+
realm.addUserRole(validUser, validRole);
95+
96+
ctxt.setRealm(realm);
97+
98+
// Configure the authenticator
99+
LoginConfig lc = new LoginConfig();
100+
lc.setAuthMethod("DIGEST");
101+
lc.setRealmName(realmName);
102+
ctxt.setLoginConfig(lc);
103+
DigestAuthenticator digestAuthenticator = new DigestAuthenticator();
104+
ctxt.getPipeline().addValve(digestAuthenticator);
105+
106+
tomcat.start();
107+
108+
// The first request will always fail - but we need the challenge
109+
Map<String,List<String>> respHeaders = new HashMap<>();
110+
ByteChunk bc = new ByteChunk();
111+
int rc = getUrl("http://localhost:" + getPort() + targetURI, bc, respHeaders);
112+
Assert.assertEquals(401, rc);
113+
Assert.assertTrue(bc.getLength() > 0);
114+
bc.recycle();
115+
116+
// Second request should
117+
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));
120+
Map<String,List<String>> reqHeaders = new HashMap<>();
121+
reqHeaders.put("authorization", auth);
122+
rc = getUrl("http://localhost:" + getPort() + targetURI, bc, reqHeaders, null);
123+
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;
169+
} 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("");
195+
}
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()));
212+
}
213+
}

‎webapps/docs/changelog.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,12 @@
131131
<bug>70000</bug>: fix duplication of special headers in the response
132132
after commit, following fix for <bug>69967</bug>. (remm)
133133
</fix>
134+
<fix>
135+
Correct the handling of URIs mapped to a security constraint that only
136+
specifies the special <code>**</code> role for all authenticated users.
137+
Requests without authentication were receiving 403 responses rather than
138+
401 responses. (markt)
139+
</fix>
134140
</changelog>
135141
</subsection>
136142
<subsection name="Coyote">

0 commit comments

Comments
 (0)