Jetty 11.0.14 is less tolerant of non-compliant cookies than 11.0.13

[markslater]

Jetty version(s)
11.0.14

Java version/vendor (use: java -version)
openjdk version "11.0.18" 2023-01-17
OpenJDK Runtime Environment (build 11.0.18+10-post-Ubuntu-0ubuntu120.04.1)
OpenJDK 64-Bit Server VM (build 11.0.18+10-post-Ubuntu-0ubuntu120.04.1, mixed mode, sharing)

OS type/version
Linux 5.4.0-120-generic

Description
Jetty 11.0.14 is less tolerant of non-compliant cookies than 11.0.13.

Specifically, when Jetty 11.0.14 is presented with a cookie containing a space in the value, org.eclipse.jetty.server.Request.getCookies() throws a BadMessageException, whereas Jetty 11.0.13 parses the cookie successfully.

This is a particular problem because the cookies an application receives might be outside its control: The browser will send myapp.example.com invalid cookies set by badapp.example.com on the .example.com domain.

Reviewing the code, I think the intention was that 11.0.14 would behave in the same way as 11.0.13, but a failure case has slipped through.

How to reproduce?

Use this class:

package com.example;

import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.handler.gzip.GzipHandler;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;

import java.io.IOException;
import java.io.Writer;
import java.net.URI;
import java.net.http.HttpClient;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.net.http.HttpResponse.BodyHandlers;
import java.util.UUID;

public class CookieServer {
    public static void main(String[] args) throws Exception {
        final Server server = new Server(8080);
        final ServletContextHandler servletContextHandler = new ServletContextHandler();
        servletContextHandler.addServlet(new ServletHolder(new FooServlet()), "/foo");
        server.setHandler(servletContextHandler);
        server.start();
        try {
            final HttpResponse<Void> httpResponse = HttpClient.newHttpClient().send(
                    HttpRequest.newBuilder(new URI("http://localhost:8080/foo"))
                            .setHeader("cookie", "PS_DEVICEFEATURES=width:1920 height:1080")
                            .build(),
                    BodyHandlers.discarding()
            );
            System.out.println("httpResponse.statusCode() = " + httpResponse.statusCode());
        } finally {
            server.stop();
        }
    }

    private static final class FooServlet extends HttpServlet {
        @Override
        protected void doGet(HttpServletRequest req, HttpServletResponse resp) {
            try {
                req.getCookies();
            } catch (Exception e) {
                e.printStackTrace();
                throw e;
            }
        }
    }
}

Outputs:

org.eclipse.jetty.http.BadMessageException: 400: Invalid cookie
	at org.eclipse.jetty.server.Cookies.getCookies(Cookies.java:106)
	at org.eclipse.jetty.server.Request.getCookies(Request.java:870)
	at com.example.CookieServer$FooServlet.doGet(CookieServer.java:42)
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:500)
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:587)
	at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:764)
	at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:529)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:221)
	at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1382)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:176)
	at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:484)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:174)
	at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1304)
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:129)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:122)
	at org.eclipse.jetty.server.Server.handle(Server.java:563)
	at org.eclipse.jetty.server.HttpChannel.lambda$handle$0(HttpChannel.java:505)
	at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:762)
	at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:497)
	at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:282)
	at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:314)
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:100)
	at org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:936)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1080)
	at java.base/java.lang.Thread.run(Thread.java:829)
Caused by: org.eclipse.jetty.http.CookieParser$InvalidCookieException: Invalid cookie

Caused by: org.eclipse.jetty.http.CookieParser$InvalidCookieException: Invalid cookie
	at org.eclipse.jetty.http.RFC6265CookieParser.parseField(RFC6265CookieParser.java:381)
	at org.eclipse.jetty.http.CookieParser.parseFields(CookieParser.java:50)
	at org.eclipse.jetty.server.Cookies.getCookies(Cookies.java:102)
	... 25 more

[joakime]

Since you are using raw header setting, you should be encoding properly.

Per RFC6265 the space is invalid for a cookie value.

Try this instead ...

.setHeader("cookie", "PS_DEVICEFEATURES=width:1920%20height:1080")

RFC6265 will see PS_DEVICEFEATURES=width:1920 height:1080 as a cookie with name PS_DEVICEFEATURES that has a value of width:1920 and a cookie attribute of height:1080 with no value, but that attribute name is also invalid as the : (colon) is a token separator and not allowed in a cookie attribute.

[markslater]

I agree it should be encoded correctly, but the browser (Edge 110.0.1587.50) is sending it with spaces in; the reproducer is just trying to be as close to the browser as possible.

[sbordet]

That Edge is this broken is quite incredible 🤦🏼‍♂️

Do you know how this cookie is set?
Is it received so from the server?
Or it is set on the client via JavaScript?

[markslater]

Sorry, I don't know how it was set - Google seems to think it's something to do with PeopleSoft.

FWIW from the perspective of my application, that cookie is just chaff: I need the value of a different cookie, but to get that, Jetty has to parse the full set of cookies sent from the browser.

[joakime]

btw, the current versions of Tomcat also puke and reject the entire request on that bad Cookie.

[joakime]

According to various PeopleSoft docs i've been reading (along with various complaints about this cookie) ...

The PS_DEVICEFEATURES cookie is set entirely client side, it is never sent by a server.
It is used for reporting client capabilities to the PeopleSoft layer on the server side.

An example of a "complete" version of this header would be ...

PS_DEVICEFEATURES=maf:0 width:1920 height:1080 clientWidth:1920 clientHeight:945 pixelratio:1 touch:0 geolocation:1 websockets:1 webworkers:1 datepicker:1 dtpicker:1 timepicker:1 dnd:1 sessionstorage:1 localstorage:1 history:1 canvas:1 svg:1 postmessage:1 hc:0

The only way I can see us handling this entirely messed up Cookie is to just fail it and ignore the entire header field on failure.
That would mean that Cookie would be totally inaccessible from Jetty / Servlet APIs as a Cookie (but still be accessible as a raw Header).
It would be left out of any list of Cookies and whatnot.

[gregw]

Note that if you wish to use the old Cookie parser, then you need to set one of the LEGACY cookie compliance modes (or include BAD_QUOTES violation in a custom mode). This will use the old parser, but by the nature of the forgiveness you may be vulnerable to cookie smuggling if an intermediary does not forgive the cookies in the same way that Jetty does.

[markslater]

@joakime's Postel's law suggestion would work for me: It seems reasonable that invalid cookies might not be available to Jetty (or at least not trivially available), but given the server can't control what cookies get sent, it should be tolerant as far as possible, subject to security concerns as mentioned by @gregw.

[gregw]

@markslater we are as forgiving as we can be. The problem with bad cookies that have variations of bad quotes (including spaces without quotes), is that there is no specification for being forgiving. If an intermediary does it differently to us, then security concerns can result.

Hence the change to a parser that is as forgiving as it can be, without forgiving cookie boundaries. The new parser can thus skip to the next valid cookie if it sees a bad one.
Perhaps spaces in a cookie value is not that bad, and could be so forgiven, but the old parser treated that as a bad quote problem.

So the LEGACY mode is there to use if you really need spaces forgiven. Let's leave this issue open and we can consider if we can also handle spaces in the new parser in a way that allows bad cookies to be skipped.

[gregw]

I think supporting spaces in values is easy and safe. I have created PR #9471 that does it.