Added a `UriCompliance.Violation.USER_INFO` to deprecate user info in `HttpURI` #12012

gregw · 2024-07-08T03:55:29Z

As per RFC9110 user info is deprecated in server implementations. The new violation for USER_DATA is included by default in 12.0.x, but will be removed in 12.1.x

Addresses CVE-2024-6763

As per [RFC9110](https://datatracker.ietf.org/doc/html/rfc9110#name-deprecation-of-userinfo-in-) user info is deprecated in server implementations. The new violation for USER_DATA is included by default in 12.0.x, but will be removed in 12.1.x

joakime

The RequestTest could use a minor improvement.

joakime · 2024-07-08T10:04:02Z

jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java

+            Arguments.of(UriCompliance.DEFAULT, "https://local/", 200, "local"),
+            Arguments.of(UriCompliance.DEFAULT, "https://other/", 400, "Authority!=Host"),
+            Arguments.of(UriCompliance.DEFAULT, "https://user@local/", 400, "Deprecated User Info"),
+            Arguments.of(UriCompliance.LEGACY, "https://user@local/", 200, "local"),
+            Arguments.of(UriCompliance.DEFAULT, "/%2F/", 400, "Ambiguous URI path separator"),
+            Arguments.of(UriCompliance.UNSAFE, "/%2F/", 200, "local")


Can we split the good and bad tests apart.

Also, can we use a hostname in the URI that is different than the one used in the Host: local header to make sure understand which host it is using?

Can we split the good and bad tests apart.

I did that, but it only saves a couple of lines and requires 40 lines to be duplicated, so a net loss. It also makes it hard to see the difference between the 200 and 400 tests, which is good to see exactly what we are testing. So I prefer it as a single test.

Also, can we use a hostname in the URI that is different than the one used in the Host: local header to make sure understand which host it is using?

I added a bunch more tests, including ones that allow HttpCompliance with different authorities.

joakime

LGTM.

Will be good to have it bake for a while too.

d2a-pnagel · 2024-09-11T11:23:27Z

This change causes unencoded [ and ] to be rejected too (400 Illegal Path Character). Not sure if that is intended?

joakime · 2024-09-11T11:38:49Z

@d2a-pnagel what does the raw (on the network) HTTP request look like that triggers this issue for you?

d2a-pnagel · 2024-09-11T11:43:30Z

Not sure about "on the network", but this is output from curl. Is that sufficient?

Jetty 12.0.11 returns a 404 Not Found for the same request.

> GET /[] HTTP/1.1
> Host: api:8080
> User-Agent: curl/7.64.0
> Accept: */*
>
< HTTP/1.1 400 Bad Request
< Server: Jetty(12.0.12)
< Cache-Control: must-revalidate,no-cache,no-store
< Content-Type: text/html;charset=iso-8859-1
< Content-Length: 437
< Connection: close
<
<html>
<head>
<meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1"/>
<title>Error 400 Illegal Path Character</title>
</head>
<body>
<h2>HTTP ERROR 400 Illegal Path Character</h2>
<table>
<tr><th>URI:</th><td>/badURI</td></tr>
<tr><th>STATUS:</th><td>400</td></tr>
<tr><th>MESSAGE:</th><td>Illegal Path Character</td></tr>

joakime · 2024-09-11T12:06:12Z

Not sure about "on the network", but this is output from curl. Is that sufficient?

Yes, that is sufficient.

The [ and ] are considered reserved characters in the gen-delims ABNF in the URI spec.
https://datatracker.ietf.org/doc/html/rfc3986#section-2.2

Those two characters are reserved for IPv6 or IPvLiteral authority sections on the URI.

The change from parsing the whole URI to just parsing the pathQuery is tripping up the gen-delims vs sub-delims nuance of the path parsing.

I'll open a new Issue about this.

joakime · 2024-09-11T12:11:40Z

Opened Issue #12259

delanym · 2024-10-16T06:00:27Z

Is there a PR for Jetty 11?

vishalnandagopal · 2024-10-16T07:12:59Z

Will this be backported to jetty 9.4.x?

joakime · 2024-10-16T10:50:37Z

Is there a PR for Jetty 11?
Will this be backported to jetty 9.4.x?

This will not be backported to an End of Community Support version of Jetty.

As stated in CVE-2024-6763, the use of Jetty server, or Jetty client, does not make you vulnerable to that CVE.

The code for HttpURI is also quite different in those older, no longer supported, versions of Jetty.

sbernard31 · 2024-10-21T15:38:19Z

For 9.x and 10.x.

The code for HttpURI is also quite different in those older, no longer supported, versions of Jetty.

Does it means "as it is different, it is not concerned by that issue" ? or more "it is different so not so easy to backport" ?

I maybe missed the information in provided links but I understand that 9.x and 10.x still get security fix, right ?
So the reason why this issue fixed for that version is the severity or ?

joakime · 2024-10-21T16:01:46Z

The change you see here is to flag the userInfo portion of the URI/URL as deprecated.
It follows the advice around RFC9110 - https://datatracker.ietf.org/doc/html/rfc9110#name-deprecation-of-userinfo-in-

Jetty 12 is RFC9110 and that has language around userInfo being an error / security issue.
Jetty 11/10 are RFC7230 is the initial spec to deprecate userInfo for http, and has optional language around userInfo support.
Jetty 9 has is RFC2616 and has no language around userInfo.

Nothing in Jetty Server or Jetty Client (all versions of Jetty) uses or reads from the userInfo portion of the URI/URL.
There is no vulnerability in Jetty Server or Jetty Client around this.

This is the reason there is no change for Jetty 11/10/9/8/7.

The details of the CVE point out the difference in parsing between RFC3986 (which all protocols use) and WhatWG Living URL document (that only Browsers use). That's where the flaw exists, that flaw cannot be addressed by Jetty.

The IETF and WhatWG need to get together and come up with a singular URI/URL spec to address this issue.

If you start to look around, you'll see that there are some projects that provide separate parsers for RFC3986 and WhatWG.

Example (from spring-framework)

Since Jetty operates within the HTTP protocols, we are RFC based.
A WhatWG parser in a server is meaningless, and harmful, and is a source of security issues (see all projects that used to exclusively use WhatWG parsers on the server side)

sbernard31 · 2024-10-21T16:08:49Z

@joakime sorry if I missed something.

But reading your comment, I understand that jetty-http 9 is not affected by : CVE-2024-6763 ?

If that's true, does it means there is an error in : Affected versions >= 7.0.0, <= 12.0.11

joakime · 2024-10-21T16:50:31Z

@sbernard31 the point of the CVE is to point out the issue between HttpURI and WhatWG.

The patch you are seeing here is to address another, slightly related, point with the userInfo portion with RFC9110.

The CVE points out that there is still issue in parsing differences between HttpURI and WhatWG, even with this patch.
There is no solution for that difference in parsing in the Jetty versions affected.

In fact, yes, I think the Affected versions is wrong, but in a different way, it should be open ended.
It should be >= 7.0.0 and that's it. But that is not possible in most of the CVE databases.
We debated if this should even be a CVE, as we cannot solve it without violating major portions of the HTTP IETF spec and the associated HTTP URL/URI IETF specs.

So the solution here, for Jetty 12, using a convenient RFC9110 rule, is to just deprecate the userInfo portion, and declare its existence a violation of the UriCompliance.

Jetty 9 doesn't even have a UriCompliance, nor is it RFC9110. This PR in Jetty 9 makes no sense.
We cannot force RFC9110 on Jetty 9 users, and the Jetty 9 users have no means to configure this UriCompliance rule it once it is implemented.
An all around bad idea for Jetty 9 users.

Like the CVE points out, if you are using the HttpURI directly in your webapp, and are using it in a proxy scenario which is used by Chrome, then stop using Jetty's HttpURI, and any other RFC3986 parser, and find a WhatWG parser instead.

sbernard31 · 2024-10-22T09:39:03Z

@joakime thx a lot for taking time to clarify that. I think I understand the issue a bit more now.

Let me explain how I fall on this PR :

I manage an open source library which are using jetty 9.x in this demo (not in the library itself but demo is deployed for testing purpose and so we try to avoid to deploy software with known security issue)
We are using jetty-server 9.x because for some reasons we target java 8 as minimal version (even if we don't recommend to use it)
We have some tooling to check security issue reported in CVE and recently tooling reports us about CVE-2024-6763.
I saw there is no fixed version for that CVE, so following CVE-2024-6763 lead me to this PR to understand the situation.

I think lot of people will face more or less same kind of issue and will follow the path above ☝️ and I'm not sure it's a good thing, because this will lead to alarm many people which are probably not really concerned. (because they are just using jetty-server or client and not HttpUri directly)

You also explain there is not so much reason to consider jetty 12.0.11 as fixed and 9.x / 10.x as not fixed.

You explain that for jetty 12 , you just "deprecate the userInfo portion, and declare its existence a violation of the UriCompliance".

Should it be a better solution to deliver a version for jetty 9.x / 10.x :

which deprecate HttpURI concerned API in . Explaining more or less what you explain to me here. So only developers using directly this API will be concerned and not indirect users (e.g. jetty-client or jetty-server users)
which will be considered as fixed in CVE.

I know that some could argue that "this will hide the issue because maybe some dev doesn't look at deprecated code" but maybe in this case as "this is a LOW severity" and "probably most user are using jetty-server or jetty-client directly", this could be OK and so will lead to less noises for most users ?

joakime · 2024-10-22T15:17:56Z

@sbernard31 just a reminder, Jetty 9/10/11 all go full EOL on January 1, 2025. (no more updates)
That's in 70 days. (or 45 business days)

sbernard31 · 2024-10-22T16:56:42Z

just a reminder, Jetty 9/10/11 all go full EOL on January 1, 2025. (no more updates)
That's in 70 days. (or 45 business days)

Thx bad new for us. 😬

gregw requested review from joakime and sbordet July 8, 2024 03:55

joakime requested changes Jul 8, 2024

View reviewed changes

gregw added 2 commits July 8, 2024 23:00

Updates from review

c5121d0

Updates from review

18e8198

gregw requested a review from joakime July 8, 2024 13:05

joakime approved these changes Jul 8, 2024

View reviewed changes

sbordet approved these changes Jul 8, 2024

View reviewed changes

gregw merged commit c880d93 into jetty-12.0.x Jul 9, 2024
10 checks passed

gregw deleted the experiment/12.0.x/deprecateHttpUriUserInfo branch July 9, 2024 23:03

joakime mentioned this pull request Sep 11, 2024

HttpURI.parseQuery rejects [ and ] characters in path section #12259

Open

dzhus mentioned this pull request Oct 18, 2024

EoCS (End of Community Support) for Jetty 10 / Jetty 11 - January 2024 #10485

Closed

sbernard31 mentioned this pull request Oct 23, 2024

Jetty 9.x will be soon no more maintained eclipse-leshan/leshan#1662

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added a `UriCompliance.Violation.USER_INFO` to deprecate user info in `HttpURI` #12012

Added a `UriCompliance.Violation.USER_INFO` to deprecate user info in `HttpURI` #12012

gregw commented Jul 8, 2024 •

edited by joakime

Loading

joakime left a comment

joakime Jul 8, 2024

gregw Jul 8, 2024

joakime left a comment

d2a-pnagel commented Sep 11, 2024 •

edited

Loading

joakime commented Sep 11, 2024

d2a-pnagel commented Sep 11, 2024 •

edited

Loading

joakime commented Sep 11, 2024

joakime commented Sep 11, 2024

delanym commented Oct 16, 2024

vishalnandagopal commented Oct 16, 2024

joakime commented Oct 16, 2024

sbernard31 commented Oct 21, 2024

joakime commented Oct 21, 2024 •

edited

Loading

sbernard31 commented Oct 21, 2024 •

edited

Loading

joakime commented Oct 21, 2024

sbernard31 commented Oct 22, 2024

joakime commented Oct 22, 2024

sbernard31 commented Oct 22, 2024

Added a UriCompliance.Violation.USER_INFO to deprecate user info in HttpURI #12012

Added a UriCompliance.Violation.USER_INFO to deprecate user info in HttpURI #12012

Conversation

gregw commented Jul 8, 2024 • edited by joakime Loading

joakime left a comment

Choose a reason for hiding this comment

joakime Jul 8, 2024

Choose a reason for hiding this comment

gregw Jul 8, 2024

Choose a reason for hiding this comment

joakime left a comment

Choose a reason for hiding this comment

d2a-pnagel commented Sep 11, 2024 • edited Loading

joakime commented Sep 11, 2024

d2a-pnagel commented Sep 11, 2024 • edited Loading

joakime commented Sep 11, 2024

joakime commented Sep 11, 2024

delanym commented Oct 16, 2024

vishalnandagopal commented Oct 16, 2024

joakime commented Oct 16, 2024

sbernard31 commented Oct 21, 2024

joakime commented Oct 21, 2024 • edited Loading

sbernard31 commented Oct 21, 2024 • edited Loading

joakime commented Oct 21, 2024

sbernard31 commented Oct 22, 2024

joakime commented Oct 22, 2024

sbernard31 commented Oct 22, 2024

Added a `UriCompliance.Violation.USER_INFO` to deprecate user info in `HttpURI` #12012

Added a `UriCompliance.Violation.USER_INFO` to deprecate user info in `HttpURI` #12012

gregw commented Jul 8, 2024 •

edited by joakime

Loading

d2a-pnagel commented Sep 11, 2024 •

edited

Loading

d2a-pnagel commented Sep 11, 2024 •

edited

Loading

joakime commented Oct 21, 2024 •

edited

Loading

sbernard31 commented Oct 21, 2024 •

edited

Loading