Skip to content

Commit

Permalink
[UNDERTOW-2523] Update the Jakarta Servlet specification to Jakarta S…
Browse files Browse the repository at this point in the history
…ervlet 6.1. Note this changed the Cookie specification to RFC 6265 which required test changes.

https://issues.redhat.com/browse/UNDERTOW-2523
Signed-off-by: James R. Perkins <jperkins@redhat.com>
  • Loading branch information
jamezp committed Dec 3, 2024
1 parent 27d7983 commit b0c4fa6
Show file tree
Hide file tree
Showing 31 changed files with 388 additions and 78 deletions.
13 changes: 12 additions & 1 deletion core/src/main/java/io/undertow/UndertowOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -212,14 +212,25 @@ public class UndertowOptions {
*/
public static final Option<Boolean> ALLOW_EQUALS_IN_COOKIE_VALUE = Option.simple(UndertowOptions.class, "ALLOW_EQUALS_IN_COOKIE_VALUE", Boolean.class);

/**
* If this is true then Undertow will disable RFC6265 compliant cookie parsing for Set-Cookie header instead of legacy backward compatible behavior.
* <p>
* default is {@code false}
* </p>
*/
public static final Option<Boolean> DISABLE_RFC6265_COOKIE_PARSING = Option.simple(UndertowOptions.class, "DISABLE_RFC6265_COOKIE_PARSING", Boolean.class);

public static final boolean DEFAULT_DISABLE_RFC6265_COOKIE_PARSING = false;

/**
* If this is true then Undertow will enable RFC6265 compliant cookie validation for Set-Cookie header instead of legacy backward compatible behavior.
*
* default is false
*/
public static final Option<Boolean> ENABLE_RFC6265_COOKIE_VALIDATION = Option.simple(UndertowOptions.class, "ENABLE_RFC6265_COOKIE_VALIDATION", Boolean.class);

public static final boolean DEFAULT_ENABLE_RFC6265_COOKIE_VALIDATION = false;
// As of Jakarta 6.1, RFC 6265 is used for the cookie specification https://github.com/jakartaee/servlet/issues/37
public static final boolean DEFAULT_ENABLE_RFC6265_COOKIE_VALIDATION = true;

/**
* If we should attempt to use SPDY for HTTPS connections.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* JBoss, Home of Professional Open Source.
*
* Copyright 2024 Red Hat, Inc., and individual contributors
* as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.undertow.attribute;

import io.undertow.server.HttpServerExchange;
import io.undertow.server.SSLSessionInfo;

/**
* An attribute which describes the secure protocol. This is the protocol resolved from the {@link SSLSessionInfo#getSecureProtocol()}.
* @author <a href="mailto:jperkins@redhat.com">James R. Perkins</a>
*/
public class SecureProtocolAttribute implements ExchangeAttribute {

public static final SecureProtocolAttribute INSTANCE = new SecureProtocolAttribute();

@Override
public String readAttribute(final HttpServerExchange exchange) {
final SSLSessionInfo ssl = exchange.getConnection().getSslSessionInfo();
if (ssl == null || ssl.getSecureProtocol() == null) {
return null;
}
return ssl.getSecureProtocol();
}

@Override
public void writeAttribute(final HttpServerExchange exchange, final String newValue) throws ReadOnlyAttributeException {
throw new ReadOnlyAttributeException("SSL Protocol", newValue);
}

@Override
public String toString() {
return "%{SECURE_PROTOCOL}";
}

public static final class Builder implements ExchangeAttributeBuilder {

@Override
public String name() {
return "Secure Protocol";
}

@Override
public ExchangeAttribute build(final String token) {
if (token.equals("%{SECURE_PROTOCOL}")) {
return INSTANCE;
}
return null;
}

@Override
public int priority() {
return 0;
}
}
}
35 changes: 35 additions & 0 deletions core/src/main/java/io/undertow/server/BasicSSLSessionInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public class BasicSSLSessionInfo implements SSLSessionInfo {
private final java.security.cert.Certificate[] peerCertificate;
private final X509Certificate[] certificate;
private final Integer keySize;
private final String secureProtocol;

/**
*
Expand All @@ -54,9 +55,24 @@ public class BasicSSLSessionInfo implements SSLSessionInfo {
* @throws CertificateException If the client cert could not be decoded
*/
public BasicSSLSessionInfo(byte[] sessionId, String cypherSuite, String certificate, Integer keySize) throws java.security.cert.CertificateException, CertificateException {
this(sessionId, cypherSuite, certificate, keySize, null);
}

/**
*
* @param sessionId The SSL session ID
* @param cypherSuite The cypher suite name
* @param certificate A string representation of the client certificate
* @param keySize The key-size used by the cypher
* @param secureProtocol the secure protocol, example {@code TLSv1.2}
* @throws java.security.cert.CertificateException If the client cert could not be decoded
* @throws CertificateException If the client cert could not be decoded
*/
public BasicSSLSessionInfo(byte[] sessionId, String cypherSuite, String certificate, Integer keySize, String secureProtocol) throws java.security.cert.CertificateException, CertificateException {
this.sessionId = sessionId;
this.cypherSuite = cypherSuite;
this.keySize = keySize;
this.secureProtocol = secureProtocol;
if (certificate != null) {
java.security.cert.CertificateFactory cf = java.security.cert.CertificateFactory.getInstance("X.509");
byte[] certificateBytes = certificate.getBytes(StandardCharsets.US_ASCII);
Expand Down Expand Up @@ -123,6 +139,20 @@ public BasicSSLSessionInfo(String sessionId, String cypherSuite, String certific
this(sessionId == null ? null : fromHex(sessionId), cypherSuite, certificate, keySize);
}

/**
*
* @param sessionId The encoded SSL session ID
* @param cypherSuite The cypher suite name
* @param certificate A string representation of the client certificate
* @param keySize The key-size used by the cypher
* @param secureProtocol the secure protocol, example {@code TLSv1.2}
* @throws java.security.cert.CertificateException If the client cert could not be decoded
* @throws CertificateException If the client cert could not be decoded
*/
public BasicSSLSessionInfo(String sessionId, String cypherSuite, String certificate, Integer keySize, String secureProtocol) throws java.security.cert.CertificateException, CertificateException {
this(sessionId == null ? null : fromHex(sessionId), cypherSuite, certificate, keySize, secureProtocol);
}

@Override
public byte[] getSessionId() {
if(sessionId == null) {
Expand Down Expand Up @@ -174,6 +204,11 @@ public SSLSession getSSLSession() {
return null;
}

@Override
public String getSecureProtocol() {
return secureProtocol;
}

private static byte[] fromHex(String sessionId) {
try {
return HexConverter.convertFromHex(sessionId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ public SSLSession getSSLSession() {
return channel.getSslSession();
}

@Override
public String getSecureProtocol() {
return channel.getSslSession().getProtocol();
}

//Suppress incorrect resource leak warning.
@SuppressWarnings("resource")
public void renegotiateBufferRequest(HttpServerExchange exchange, SslClientAuthMode newAuthMode) throws IOException {
Expand Down
49 changes: 43 additions & 6 deletions core/src/main/java/io/undertow/server/Connectors.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Date;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.Executor;
import java.util.concurrent.RejectedExecutionException;

Expand All @@ -57,6 +60,7 @@ public class Connectors {

private static final boolean[] ALLOWED_TOKEN_CHARACTERS = new boolean[256];
private static final boolean[] ALLOWED_SCHEME_CHARACTERS = new boolean[256];
private static final Set<String> KNOWN_ATTRIBUTE_NAMES = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);

static {
for(int i = 0; i < ALLOWED_TOKEN_CHARACTERS.length; ++i) {
Expand Down Expand Up @@ -108,6 +112,16 @@ public class Connectors {
}
}
}

KNOWN_ATTRIBUTE_NAMES.add("Path");
KNOWN_ATTRIBUTE_NAMES.add("Domain");
KNOWN_ATTRIBUTE_NAMES.add("Discard");
KNOWN_ATTRIBUTE_NAMES.add("Secure");
KNOWN_ATTRIBUTE_NAMES.add("HttpOnly");
KNOWN_ATTRIBUTE_NAMES.add("Max-Age");
KNOWN_ATTRIBUTE_NAMES.add("Expires");
KNOWN_ATTRIBUTE_NAMES.add("Comment");
KNOWN_ATTRIBUTE_NAMES.add("SameSite");
}
/**
* Flattens the exchange cookie map into the response header map. This should be called by a
Expand Down Expand Up @@ -224,17 +238,17 @@ private static String addRfc6265ResponseCookieToExchange(final Cookie cookie) {
header.append("; Domain=");
header.append(cookie.getDomain());
}
if (cookie.isDiscard()) {
header.append("; Discard");
}
if (cookie.isSecure()) {
header.append("; Secure");
}
if (cookie.isHttpOnly()) {
header.append("; HttpOnly");
}
if (cookie.getMaxAge() != null) {
if (cookie.getMaxAge() >= 0) {
// TODO (jrp) Per the TCK test "RFC 6265 - server should only send +ve values for Max-Age"
// TODO (jrp) This is possibly per https://datatracker.ietf.org/doc/html/rfc6265#section-5.2.2, however
// TODO (jrp) I'm not sure not adding the value is correct.
if (cookie.getMaxAge() > 0) {
header.append("; Max-Age=");
header.append(cookie.getMaxAge());
}
Expand Down Expand Up @@ -270,6 +284,7 @@ private static String addRfc6265ResponseCookieToExchange(final Cookie cookie) {
header.append(cookie.getSameSiteMode());
}
}
appendAttributes(cookie, header);
return header.toString();
}

Expand Down Expand Up @@ -298,7 +313,10 @@ private static String addVersion0ResponseCookieToExchange(final Cookie cookie) {
header.append("; Expires=");
header.append(DateUtils.toOldCookieDateString(cookie.getExpires()));
} else if (cookie.getMaxAge() != null) {
if (cookie.getMaxAge() >= 0) {
// TODO (jrp) Per the TCK test "RFC 6265 - server should only send +ve values for Max-Age"
// TODO (jrp) This is possibly per https://datatracker.ietf.org/doc/html/rfc6265#section-5.2.2, however
// TODO (jrp) I'm not sure not adding the value is correct.
if (cookie.getMaxAge() > 0) {
header.append("; Max-Age=");
header.append(cookie.getMaxAge());
}
Expand All @@ -320,6 +338,7 @@ private static String addVersion0ResponseCookieToExchange(final Cookie cookie) {
header.append(cookie.getSameSiteMode());
}
}
appendAttributes(cookie, header);
return header.toString();

}
Expand Down Expand Up @@ -350,7 +369,10 @@ private static String addVersion1ResponseCookieToExchange(final Cookie cookie) {
header.append("; HttpOnly");
}
if (cookie.getMaxAge() != null) {
if (cookie.getMaxAge() >= 0) {
// TODO (jrp) Per the TCK test "RFC 6265 - server should only send +ve values for Max-Age"
// TODO (jrp) This is possibly per https://datatracker.ietf.org/doc/html/rfc6265#section-5.2.2, however
// TODO (jrp) I'm not sure not adding the value is correct.
if (cookie.getMaxAge() > 0) {
header.append("; Max-Age=");
header.append(cookie.getMaxAge());
}
Expand Down Expand Up @@ -386,6 +408,7 @@ private static String addVersion1ResponseCookieToExchange(final Cookie cookie) {
header.append(cookie.getSameSiteMode());
}
}
appendAttributes(cookie, header);
return header.toString();
}

Expand Down Expand Up @@ -656,4 +679,18 @@ public static boolean areRequestHeadersValid(HeaderMap headers) {
}
return true;
}

private static void appendAttributes(final Cookie cookie, final StringBuilder header) {
for (Map.Entry<String, String> entry : cookie.getAttributes().entrySet()) {
if (KNOWN_ATTRIBUTE_NAMES.contains(entry.getKey())) {
continue;
}
header.append("; ")
.append(entry.getKey());
if (!entry.getValue().isBlank()) {
header.append('=')
.append(entry.getValue());
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1245,7 +1245,7 @@ public Iterable<Cookie> requestCookies() {
Cookies.parseRequestCookies(
getConnection().getUndertowOptions().get(UndertowOptions.MAX_COOKIES, UndertowOptions.DEFAULT_MAX_COOKIES),
getConnection().getUndertowOptions().get(UndertowOptions.ALLOW_EQUALS_IN_COOKIE_VALUE, false),
requestHeaders.get(Headers.COOKIE), requestCookiesParam);
requestHeaders.get(Headers.COOKIE), requestCookiesParam, getConnection().getUndertowOptions().get(UndertowOptions.DISABLE_RFC6265_COOKIE_PARSING, UndertowOptions.DEFAULT_DISABLE_RFC6265_COOKIE_PARSING));
}
return requestCookies;
}
Expand Down
15 changes: 12 additions & 3 deletions core/src/main/java/io/undertow/server/SSLSessionInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@

package io.undertow.server;

import org.xnio.SslClientAuthMode;

import javax.net.ssl.SSLSession;
import java.io.IOException;
import javax.net.ssl.SSLSession;

import org.xnio.SslClientAuthMode;

/**
* SSL session information.
Expand Down Expand Up @@ -126,4 +126,13 @@ default int getKeySize() {
*/
SSLSession getSSLSession();

/**
* Returns the {@linkplain SSLSession#getProtocol() secure protocol}, if applicable, for the curren session.
*
* @return the secure protocol or {@code null} if one could not be found
*/
default String getSecureProtocol() {
return getSSLSession() == null ? null : getSSLSession().getProtocol();
}

}
34 changes: 34 additions & 0 deletions core/src/main/java/io/undertow/server/handlers/Cookie.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package io.undertow.server.handlers;

import java.util.Date;
import java.util.Map;

/**
* A HTTP cookie.
Expand Down Expand Up @@ -86,6 +87,39 @@ default Cookie setSameSiteMode(final String mode) {
throw new UnsupportedOperationException("Not implemented");
}

/**
* Returns the attribute associated with the name or {@code null} if no attribute is associated with the name.
*
* @param name the name of the attribute
*
* @return the value or {@code null} if not found
*/
default String getAttribute(final String name) {
throw new UnsupportedOperationException("Not implemented");
}

/**
* Sets an attribute for the cookie. If the value is {@code null}, the attribute is removed. If the value is not
* {@code null}, the attribute is added to the attributes for this cookie.
*
* @param name the name of the attribute
* @param value the value of the attribute or {@code null} to remove it
*
* @return this cookie
*/
default Cookie setAttribute(final String name, final String value) {
throw new UnsupportedOperationException("Not implemented");
}

/**
* Returns an unmodifiable map of the attributes associated with this cookie.
*
* @return an unmodifiable map of the attributes
*/
default Map<String, String> getAttributes() {
throw new UnsupportedOperationException("Not implemented");
}

@Override
default int compareTo(final Object other) {
final Cookie o = (Cookie) other;
Expand Down
Loading

0 comments on commit b0c4fa6

Please sign in to comment.