Skip to content

Commit

Permalink
Fixes #10891 - Support the "Partitioned" cookie attribute.
Browse files Browse the repository at this point in the history 
Added support in oej.http.HttpCookie.
Bridged support for Servlet cookies via the cookie Comment attribute.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
  • Loading branch information
@sbordet
sbordet committed Nov 19, 2023
1 parent af13a72 commit f82844e
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 52 deletions.
77 changes: 54 additions & 23 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

// TODO consider replacing this with java.net.HttpCookie (once it supports RFC6265)
public class HttpCookie
{
private static final Logger LOG = LoggerFactory.getLogger(HttpCookie.class);
Expand All @@ -33,11 +32,18 @@ public class HttpCookie
private static final String __01Jan1970_COOKIE = DateGenerator.formatCookieDate(0).trim();

/**
* If this string is found within the comment parsed with {@link #isHttpOnlyInComment(String)} the check will return true
* String used in the {@code Comment} attribute of {@link java.net.HttpCookie},
* parsed with {@link #isHttpOnlyInComment(String)}, to support the {@code HttpOnly} attribute.
**/
public static final String HTTP_ONLY_COMMENT = "__HTTP_ONLY__";
/**
* These strings are used by {@link #getSameSiteFromComment(String)} to check for a SameSite specifier in the comment
* String used in the {@code Comment} attribute of {@link java.net.HttpCookie},
* parsed with {@link #isPartitionedInComment(String)}, to support the {@code Partitioned} attribute.
**/
public static final String PARTITIONED_COMMENT = "__PARTITIONED__";
/**
* The strings used in the {@code Comment} attribute of {@link java.net.HttpCookie},
* parsed with {@link #getSameSiteFromComment(String)}, to support the {@code SameSite} attribute.
**/
private static final String SAME_SITE_COMMENT = "__SAME_SITE_";
public static final String SAME_SITE_NONE_COMMENT = SAME_SITE_COMMENT + "NONE__";
Expand All @@ -53,7 +59,7 @@ public enum SameSite
{
NONE("None"), STRICT("Strict"), LAX("Lax");

private String attributeValue;
private final String attributeValue;

SameSite(String attributeValue)
{
Expand All @@ -77,6 +83,7 @@ public String getAttributeValue()
private final boolean _httpOnly;
private final long _expiration;
private final SameSite _sameSite;
private final boolean _partitioned;

public HttpCookie(String name, String value)
{
Expand Down Expand Up @@ -104,6 +111,11 @@ public HttpCookie(String name, String value, String domain, String path, long ma
}

public HttpCookie(String name, String value, String domain, String path, long maxAge, boolean httpOnly, boolean secure, String comment, int version, SameSite sameSite)
{
this(name, value, domain, path, maxAge, httpOnly, secure, comment, version, sameSite, false);
}

public HttpCookie(String name, String value, String domain, String path, long maxAge, boolean httpOnly, boolean secure, String comment, int version, SameSite sameSite, boolean partitioned)
{
_name = name;
_value = value;
Expand All @@ -116,6 +128,7 @@ public HttpCookie(String name, String value, String domain, String path, long ma
_version = version;
_expiration = maxAge < 0 ? -1 : NanoTime.now() + TimeUnit.SECONDS.toNanos(maxAge);
_sameSite = sameSite;
_partitioned = partitioned;
}

public HttpCookie(String setCookie)
Expand All @@ -136,8 +149,10 @@ public HttpCookie(String setCookie)
_comment = cookie.getComment();
_version = cookie.getVersion();
_expiration = _maxAge < 0 ? -1 : NanoTime.now() + TimeUnit.SECONDS.toNanos(_maxAge);
// support for SameSite values has not yet been added to java.net.HttpCookie
// Support for SameSite values has not yet been added to java.net.HttpCookie.
_sameSite = getSameSiteFromComment(cookie.getComment());
// Support for Partitioned has not yet been added to java.net.HttpCookie.
_partitioned = isPartitionedInComment(cookie.getComment());
}

/**
Expand Down Expand Up @@ -229,6 +244,14 @@ public boolean isExpired(long timeNanos)
return _expiration != -1 && NanoTime.isBefore(_expiration, timeNanos);
}

/**
* @return whether this cookie is partitioned
*/
public boolean isPartitioned()
{
return _partitioned;
}

/**
* @return a string representation of this cookie
*/
Expand Down Expand Up @@ -419,6 +442,8 @@ public String getRFC6265SetCookie()
buf.append("; SameSite=");
buf.append(_sameSite.getAttributeValue());
}
if (isPartitioned())
buf.append("; Partitioned");

return buf.toString();
}
Expand All @@ -428,23 +453,22 @@ public static boolean isHttpOnlyInComment(String comment)
return comment != null && comment.contains(HTTP_ONLY_COMMENT);
}

public static boolean isPartitionedInComment(String comment)
{
return comment != null && comment.contains(PARTITIONED_COMMENT);
}

public static SameSite getSameSiteFromComment(String comment)
{
if (comment != null)
{
if (comment.contains(SAME_SITE_STRICT_COMMENT))
{
return SameSite.STRICT;
}
if (comment.contains(SAME_SITE_LAX_COMMENT))
{
return SameSite.LAX;
}
if (comment.contains(SAME_SITE_NONE_COMMENT))
{
return SameSite.NONE;
}
}
if (comment == null)
return null;

if (comment.contains(SAME_SITE_STRICT_COMMENT))
return SameSite.STRICT;
if (comment.contains(SAME_SITE_LAX_COMMENT))
return SameSite.LAX;
if (comment.contains(SAME_SITE_NONE_COMMENT))
return SameSite.NONE;

return null;
}
Expand Down Expand Up @@ -488,21 +512,25 @@ public static SameSite getSameSiteDefault(Attributes contextAttributes)
public static String getCommentWithoutAttributes(String comment)
{
if (comment == null)
{
return null;
}

String strippedComment = comment.trim();

strippedComment = StringUtil.strip(strippedComment, HTTP_ONLY_COMMENT);
strippedComment = StringUtil.strip(strippedComment, PARTITIONED_COMMENT);
strippedComment = StringUtil.strip(strippedComment, SAME_SITE_NONE_COMMENT);
strippedComment = StringUtil.strip(strippedComment, SAME_SITE_LAX_COMMENT);
strippedComment = StringUtil.strip(strippedComment, SAME_SITE_STRICT_COMMENT);

return strippedComment.length() == 0 ? null : strippedComment;
return strippedComment.isEmpty() ? null : strippedComment;
}

public static String getCommentWithAttributes(String comment, boolean httpOnly, SameSite sameSite)
{
return getCommentWithAttributes(comment, httpOnly, sameSite, false);
}

public static String getCommentWithAttributes(String comment, boolean httpOnly, SameSite sameSite, boolean partitioned)
{
if (comment == null && sameSite == null)

This comment has been minimized.

Sign in to view

Copy link
@sbordet

sbordet Jul 4, 2024

Author Contributor

Please open an issue about this.

This comment has been minimized.

Sign in to view

Copy link
@janbartel

janbartel Jul 5, 2024

Contributor

@selckin I'm not sure this is the server's responsibility to police, if we look at https://github.com/privacycg/CHIPS?tab=readme-ov-file#how-to-enforce-design-principles it says:

User agents may only accept Partitioned cookies if their SameSite attribute is None.

Note: a Partitioned cookie without SameSite=None is effectively just a same-site cookie which cannot be sent in a third-party context anyway.

So it seems the onus is on the user agent to evaluate a Set-Cookie with both Partitioned and SameSite attributes.
return null;
Expand Down Expand Up @@ -535,6 +563,9 @@ public static String getCommentWithAttributes(String comment, boolean httpOnly,
}
}

if (partitioned)
builder.append(PARTITIONED_COMMENT);

if (builder.length() == 0)
return null;
return builder.toString();
Expand Down
12 changes: 7 additions & 5 deletions jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ public void testSetRFC6265Cookie() throws Exception

httpCookie = new HttpCookie("everything", "value", "domain", "path", 0, true, true, null, -1, HttpCookie.SameSite.STRICT);
assertEquals("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly; SameSite=Strict", httpCookie.getRFC6265SetCookie());

httpCookie = new HttpCookie("everything", "value", "domain", "path", 0, true, true, null, -1, HttpCookie.SameSite.STRICT, true);
assertEquals("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly; SameSite=Strict; Partitioned", httpCookie.getRFC6265SetCookie());
}

public static Stream<String> rfc6265BadNameSource()
Expand Down Expand Up @@ -336,7 +339,8 @@ public static Stream<Arguments> getCommentWithoutAttributesSource()
Arguments.of("__HTTP_ONLY____SAME_SITE_NONE__comment", "comment"),
// mixed - attributes at start and end
Arguments.of("__SAME_SITE_NONE__comment__HTTP_ONLY__", "comment"),
Arguments.of("__HTTP_ONLY__comment__SAME_SITE_NONE__", "comment")
Arguments.of("__HTTP_ONLY__comment__SAME_SITE_NONE__", "comment"),
Arguments.of("__PARTITIONED__comment__SAME_SITE_NONE__", "comment")
);
}

Expand All @@ -346,13 +350,9 @@ public void testGetCommentWithoutAttributes(String rawComment, String expectedCo
{
String actualComment = HttpCookie.getCommentWithoutAttributes(rawComment);
if (expectedComment == null)
{
assertNull(actualComment);
}
else
{
assertEquals(actualComment, expectedComment);
}
}

@Test
Expand All @@ -368,6 +368,8 @@ public void testGetCommentWithAttributes()
is("__HTTP_ONLY____SAME_SITE_NONE__"));
assertThat(HttpCookie.getCommentWithAttributes("hello", true, HttpCookie.SameSite.LAX),
is("hello__HTTP_ONLY____SAME_SITE_LAX__"));
assertThat(HttpCookie.getCommentWithAttributes("hello", true, HttpCookie.SameSite.LAX, true),
is("hello__HTTP_ONLY____SAME_SITE_LAX____PARTITIONED__"));

assertThat(HttpCookie.getCommentWithAttributes("__HTTP_ONLY____SAME_SITE_LAX__", false, null), nullValue());
assertThat(HttpCookie.getCommentWithAttributes("__HTTP_ONLY____SAME_SITE_LAX__", true, HttpCookie.SameSite.NONE),
Expand Down
4 changes: 3 additions & 1 deletion jetty-server/src/main/java/org/eclipse/jetty/server/Response.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ public void addCookie(Cookie cookie)
String comment = cookie.getComment();
// HttpOnly was supported as a comment in cookie flags before the java.net.HttpCookie implementation so need to check that
boolean httpOnly = cookie.isHttpOnly() || HttpCookie.isHttpOnlyInComment(comment);
boolean partitioned = HttpCookie.isPartitionedInComment(comment);
SameSite sameSite = HttpCookie.getSameSiteFromComment(comment);
comment = HttpCookie.getCommentWithoutAttributes(comment);

Expand All @@ -280,7 +281,8 @@ public void addCookie(Cookie cookie)
cookie.getSecure(),
comment,
cookie.getVersion(),
sameSite));
sameSite,
partitioned));
}
}

Expand Down
56 changes: 33 additions & 23 deletions jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -632,29 +632,26 @@ public String getSessionCookie()
*/
public HttpCookie getSessionCookie(HttpSession session, String contextPath, boolean requestIsSecure)
{
if (isUsingCookies())
{
SessionCookieConfig cookieConfig = getSessionCookieConfig();
String sessionPath = (cookieConfig.getPath() == null) ? contextPath : cookieConfig.getPath();
sessionPath = (StringUtil.isEmpty(sessionPath)) ? "/" : sessionPath;
String id = getExtendedId(session);
HttpCookie cookie = null;

cookie = new HttpCookie(
getSessionCookieName(_cookieConfig),
id,
cookieConfig.getDomain(),
sessionPath,
cookieConfig.getMaxAge(),
cookieConfig.isHttpOnly(),
cookieConfig.isSecure() || (isSecureRequestOnly() && requestIsSecure),
HttpCookie.getCommentWithoutAttributes(cookieConfig.getComment()),
0,
HttpCookie.getSameSiteFromComment(cookieConfig.getComment()));

return cookie;
}
return null;
if (!isUsingCookies())
return null;
SessionCookieConfig cookieConfig = getSessionCookieConfig();
String sessionPath = (cookieConfig.getPath() == null) ? contextPath : cookieConfig.getPath();
sessionPath = (StringUtil.isEmpty(sessionPath)) ? "/" : sessionPath;

Check warning on line 639 in jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java

View check run for this annotation

Webtide Jenkins / Static Analysis jdk17

UnnecessaryParentheses 

NORMAL:
These grouping parentheses are unnecessary; it is unlikely the code will be misinterpreted without them.
Raw output 
Did you mean: <pre><code>sessionPath = StringUtil.isEmpty(sessionPath) ? &quot;/&quot; : sessionPath;</code></pre><p><a href="https://errorprone.info/bugpattern/UnnecessaryParentheses">See ErrorProne documentation.</a></p>
String id = getExtendedId(session);
String comment = cookieConfig.getComment();
return new HttpCookie(
getSessionCookieName(_cookieConfig),
id,
cookieConfig.getDomain(),
sessionPath,
cookieConfig.getMaxAge(),
cookieConfig.isHttpOnly(),
cookieConfig.isSecure() || (isSecureRequestOnly() && requestIsSecure),
HttpCookie.getCommentWithoutAttributes(comment),
0,
HttpCookie.getSameSiteFromComment(comment),
HttpCookie.isPartitionedInComment(comment)
);
}

@ManagedAttribute("domain of the session cookie, or null for the default")
Expand Down Expand Up @@ -802,6 +799,19 @@ public void setHttpOnly(boolean httpOnly)
_httpOnly = httpOnly;
}

/**
* Sets whether session cookies should have the {@code Partitioned} attribute.
*
* @param partitioned whether session cookies should have the {@code Partitioned} attribute
* @see HttpCookie
*/
public void setPartitioned(boolean partitioned)
{
// Encode in comment whilst not supported by SessionConfig,
// so that it can be set/saved in web.xml and quickstart.
_sessionComment = HttpCookie.getCommentWithAttributes(_sessionComment, false, null, partitioned);
}

/**
* Set Session cookie sameSite mode.
* Currently this is encoded in the session comment until sameSite is supported by {@link SessionCookieConfig}
Expand Down

0 comments on commit f82844e

Please sign in to comment.