[ 
https://issues.apache.org/jira/browse/OAK-12259?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Julian Reschke resolved OAK-12259.
----------------------------------
    Fix Version/s: 2.4.0
       Resolution: Fixed

> 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
>            Assignee: Julian Reschke
>            Priority: Major
>             Fix For: 2.4.0
>
>
> 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)

Reply via email to