Herman Ciechanowiec created OAK-12259:
-----------------------------------------
Summary: oak-http: OakServlet mis-parses HTTP Basic credentials -
passwords containing a colon are silently truncated and malformed headers cause
an HTTP 500
Key: OAK-12259
URL: https://issues.apache.org/jira/browse/OAK-12259
Project: Jackrabbit Oak
Issue Type: Bug
Components: oak-http
Environment: Any deployment that exposes the repository over HTTP via
{{org.apache.jackrabbit.oak.http.OakServlet}} using HTTP Basic authentication.
Reporter: Herman Ciechanowiec
h2. Description
{{OakServlet}} authenticates incoming HTTP requests by decoding the
{{Authorization: Basic <base64>}} header and parsing the decoded
{{user-id:password}} string. The current implementation is:
{code:java}
String authorization = request.getHeader("Authorization");
if (authorization != null && authorization.startsWith("Basic ")) {
String[] basic =
Base64.decode(authorization.substring("Basic
".length())).split(":");
credentials = new SimpleCredentials(basic[0], basic[1].toCharArray());
} else {
throw new LoginException();
}
{code}
(File:
{{{}oak-http/src/main/java/org/apache/jackrabbit/oak/http/OakServlet.java{}}},
inside {{{}service(...){}}}.)
This violates [RFC 7617|https://www.rfc-editor.org/rfc/rfc7617] ("The 'Basic'
HTTP Authentication Scheme"), which states that the credentials are the
concatenation of the user-id, a single colon ({{{}:{}}}) character, and the
password. The user-id _must not_ contain a colon, but the password _may_
contain any character, including colons. Therefore everything after the *first*
colon is the password.
Two distinct defects result from using {{String.split(":")}} with no limit:
h3. 1. Password truncation / weakened authentication (CWE-287)
{{split(":")}} splits on *every* colon and the code keeps only element
{{{}[1]{}}}. A password that contains one or more colons is therefore silently
truncated to the substring between the first and second colon. The repository
then authenticates the user against this shortened secret.
*Example:* a user whose password is {{p4ss:w0rd:!}} is authenticated using only
{{p4ss}} (the substring between the first and second colon). The effective
secret the server verifies is shorter than the one the user actually set,
reducing the keyspace an attacker must search and breaking the user's
expectation about password strength.
h3. 2. Unhandled exception -> HTTP 500 (instead of 401)
If the decoded value contains no colon (e.g. a client sends just a user-id, or
an empty password {{user:}} - {{String.split}} drops trailing empty strings),
accessing {{basic[1]}} throws {{{}ArrayIndexOutOfBoundsException{}}}. This
runtime exception is *not* caught by the surrounding handlers (which only catch
{{LoginException}} and {{{}NoSuchWorkspaceException{}}}), so it escapes as an
HTTP 500 with a stack trace rather than a proper 401 Unauthorized challenge.
h2. Steps to reproduce
*Defect 1 (password truncation):*
# Create a user whose password contains a colon, e.g. {{{}p4ss:w0rd:!{}}}.
# Send a request with: {{Authorization: Basic <base64 of "user:p4ss:w0rd:!">}}
# Observe that authentication is evaluated against {{{}p4ss{}}}, not the full
password. A login that should fail with the wrong password can succeed, and the
full password is never the one actually checked.
*Defect 2 (HTTP 500):*
# Send a request with an Authorization header whose decoded value has no
colon, e.g. Base64 of {{{}nocolon{}}}: {{Authorization: Basic bm9jb2xvbg==}}
# Observe an HTTP 500 ({{{}ArrayIndexOutOfBoundsException{}}}) instead of a
clean 401 Unauthorized.
h2. Expected behaviour
* The password is taken as everything after the first colon, preserving any
colons it contains (RFC 7617 compliant).
* A missing/empty/non-Basic Authorization header, or a decoded value with no
colon, results in a 401 Unauthorized challenge - never an HTTP 500.
h2. Actual behaviour
* Passwords containing colons are truncated at the first colon, weakening
authentication.
* A decoded value without a colon yields HTTP 500
({{{}ArrayIndexOutOfBoundsException{}}}) instead of 401.
h2. Impact
* *Security:* effective passwords are silently weakened whenever they contain
a colon; the secret actually verified is shorter than the one configured.
* *Robustness/availability:* malformed credential headers produce 500 errors
and leak a stack trace instead of returning a standard 401 challenge.
h2. Proposed fix
Split only on the first colon ({{{}indexOf(':'){}}} + {{{}substring{}}}),
reject a missing colon with a {{LoginException}} (mapped to 401 by the existing
handler), and reject a missing/non-Basic header the same way. The credential
parsing could be extracted into a small private helper for clarity, for example:
{code:java}
private Credentials parseBasicCredentials(String authorization)
throws LoginException {
if (authorization == null || !authorization.startsWith("Basic ")) {
throw new LoginException(
"Missing or unsupported Authorization header; expected Basic
scheme");
}
String decoded = Base64.decode(authorization.substring("Basic ".length()));
int colon = decoded.indexOf(':');
if (colon < 0) {
throw new LoginException(
"Malformed Basic credentials: missing ':' separator");
}
String userId = decoded.substring(0, colon);
String password = decoded.substring(colon + 1);
return new SimpleCredentials(userId, password.toCharArray());
}
{code}
This is a contained bug fix with no API change, so no feature toggle is
required.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
