Fix handling of __Secure- and __Host- Cookie prefixes (#33717)

* Make checking for cookie prefixes case-insensitive

Cookie-Prefixes like "__Host-" and "__Secure-" are case insensitive
as per https://www.ietf.org/archive/id/draft-ietf-httpbis-rfc6265bis-15.html#name-storage-model.

This is tested by many WPT tests in cookies/prefix, for example
* cookies/prefix/__host.document-cookie.html
* cookies/prefix/__host.document-cookie.https.html

Since the implementation and the specification had diverged quite
significantly i also updated/added spec comments where appropriate
and slightly restructured code so its easier to follow. However,
the only change in behaviour is the prefix check described above.

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Update WPT expectations

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Remove unused import

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Fix cookie test cases

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Fix ignore cookie with __Host prefix and no specified path attribute

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

* Fix another cookie test case

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

---------

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
This commit is contained in:
Simon Wülker 2024-10-09 06:52:48 +02:00 committed by GitHub
parent a2b27012a5
commit ff6523c37e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 151 additions and 159 deletions

View file

@ -7,7 +7,7 @@
use std::borrow::ToOwned;
use std::net::{Ipv4Addr, Ipv6Addr};
use std::time::{Duration, SystemTime};
use std::time::SystemTime;
use cookie::Cookie;
use net_traits::pub_domains::is_pub_domain;
@ -38,55 +38,108 @@ impl ServoCookie {
request: &ServoUrl,
source: CookieSource,
) -> Option<ServoCookie> {
Cookie::parse(cookie_str)
.ok()
.map(|cookie| ServoCookie::new_wrapped(cookie, request, source))
.unwrap_or(None)
let cookie = Cookie::parse(cookie_str).ok()?;
ServoCookie::new_wrapped(cookie, request, source)
}
/// <http://tools.ietf.org/html/rfc6265#section-5.3>
/// Steps 6-22 from <https://www.ietf.org/archive/id/draft-ietf-httpbis-rfc6265bis-15.html#name-storage-model>
pub fn new_wrapped(
mut cookie: Cookie<'static>,
request: &ServoUrl,
source: CookieSource,
) -> Option<ServoCookie> {
// Step 3
let max_age: Option<Duration> =
cookie.max_age().and_then(|max_age| max_age.try_into().ok());
let (persistent, expiry_time) = match (max_age, cookie.expires_datetime()) {
(Some(max_age), _) => (true, Some(SystemTime::now() + max_age)),
(_, Some(date_time)) => (true, Some(date_time.into())),
_ => (false, None),
};
let persistent;
let expiry_time;
// Step 6. If the cookie-attribute-list contains an attribute with an attribute-name of "Max-Age":
if let Some(max_age) = cookie.max_age() {
// 1. Set the cookie's persistent-flag to true.
persistent = true;
// 2. Set the cookie's expiry-time to attribute-value of the last
// attribute in the cookie-attribute-list with an attribute-name of "Max-Age".
expiry_time = Some(SystemTime::now() + max_age);
}
// Otherwise, if the cookie-attribute-list contains an attribute with an attribute-name of "Expires":
else if let Some(date_time) = cookie.expires_datetime() {
// 1. Set the cookie's persistent-flag to true.
persistent = true;
// 2. Set the cookie's expiry-time to attribute-value of the last attribute in the
// cookie-attribute-list with an attribute-name of "Expires".
expiry_time = Some(date_time.into());
}
// Otherwise:
else {
// 1. Set the cookie's persistent-flag to false.
persistent = false;
// 2. Set the cookie's expiry-time to the latest representable date.
expiry_time = None;
}
let url_host = request.host_str().unwrap_or("").to_owned();
// Step 4
let mut domain = cookie.domain().unwrap_or("").to_owned();
// Step 7. If the cookie-attribute-list contains an attribute with an attribute-name of "Domain":
let mut domain = if let Some(domain) = cookie.domain() {
// 1. Let the domain-attribute be the attribute-value of the last attribute in the
// cookie-attribute-list [..]
// NOTE: This is done by the cookie crate
domain.to_owned()
}
// Otherwise:
else {
// 1. Let the domain-attribute be the empty string.
String::new()
};
// Step 5
// TODO Step 8. If the domain-attribute contains a character that is not in the range of [USASCII] characters,
// abort these steps and ignore the cookie entirely.
// NOTE: (is this done by the cookies crate?)
// Step 9. If the user agent is configured to reject "public suffixes" and the domain-attribute
// is a public suffix:
if is_pub_domain(&domain) {
// 1. If the domain-attribute is identical to the canonicalized request-host:
if domain == url_host {
domain = "".to_string();
} else {
// 1. Let the domain-attribute be the empty string.
domain = String::new();
}
// Otherwise:
else {
// 1.Abort these steps and ignore the cookie entirely.
return None;
}
}
// Step 6
let host_only = if !domain.is_empty() {
// Step 10. If the domain-attribute is non-empty:
let host_only;
if !domain.is_empty() {
// 1. If the canonicalized request-host does not domain-match the domain-attribute:
if !ServoCookie::domain_match(&url_host, &domain) {
// 1. Abort these steps and ignore the cookie entirely.
return None;
} else {
// 1. Set the cookie's host-only-flag to false.
host_only = false;
// 2. Set the cookie's domain to the domain-attribute.
cookie.set_domain(domain);
false
}
} else {
}
// Otherwise:
else {
// 1. Set the cookie's host-only-flag to true.
host_only = true;
// 2. Set the cookie's domain to the canonicalized request-host.
cookie.set_domain(url_host);
true
};
// Step 7
// Step 11. If the cookie-attribute-list contains an attribute with an attribute-name of "Path",
// set the cookie's path to attribute-value of the last attribute in the cookie-attribute-list
// with both an attribute-name of "Path" and an attribute-value whose length is no more than 1024 octets.
// Otherwise, set the cookie's path to the default-path of the request-uri.
let mut has_path_specified = true;
let mut path = cookie
.path()
@ -95,29 +148,81 @@ impl ServoCookie {
""
})
.to_owned();
// TODO: Why do we do this?
if !path.starts_with('/') {
path = ServoCookie::default_path(request.path()).to_string();
}
cookie.set_path(path);
// Step 10
if cookie.http_only().unwrap_or(false) && source == CookieSource::NonHTTP {
// Step 12. If the cookie-attribute-list contains an attribute with an attribute-name of "Secure",
// set the cookie's secure-only-flag to true. Otherwise, set the cookie's secure-only-flag to false.
let secure_only = cookie.secure().unwrap_or(false);
// Step 13. If the request-uri does not denote a "secure" connection (as defined by the user agent),
// and the cookie's secure-only-flag is true, then abort these steps and ignore the cookie entirely.
if secure_only && !request.is_secure_scheme() {
return None;
}
// https://tools.ietf.org/html/draft-west-cookie-prefixes-04#section-4
// Step 1 of cookie prefixes
if (cookie.name().starts_with("__Secure-") || cookie.name().starts_with("__Host-")) &&
!(cookie.secure().unwrap_or(false) && request.is_secure_scheme())
// Step 14. If the cookie-attribute-list contains an attribute with an attribute-name of "HttpOnly",
// set the cookie's http-only-flag to true. Otherwise, set the cookie's http-only-flag to false.
let http_only = cookie.http_only().unwrap_or(false);
// Step 15. If the cookie was received from a "non-HTTP" API and the cookie's
// http-only-flag is true, abort these steps and ignore the cookie entirely.
if http_only && source == CookieSource::NonHTTP {
return None;
}
// TODO: Step 16, Ignore cookies from insecure request uris based on existing cookies
// TODO: Steps 17-19, same-site-flag
// Step 20. If the cookie-name begins with a case-insensitive match for the string "__Secure-",
// abort these steps and ignore the cookie entirely unless the cookie's secure-only-flag is true.
let has_case_insensitive_prefix = |value: &str, prefix: &str| {
value
.get(..prefix.len())
.map_or(false, |p| p.eq_ignore_ascii_case(prefix))
};
if has_case_insensitive_prefix(cookie.name(), "__Secure-") &&
!cookie.secure().unwrap_or(false)
{
return None;
}
// Step 2 of cookie prefixes
if cookie.name().starts_with("__Host-") &&
!(host_only && has_path_specified && cookie.path().unwrap() == "/")
{
return None;
// Step 21. If the cookie-name begins with a case-insensitive match for the string "__Host-",
// abort these steps and ignore the cookie entirely unless the cookie meets all the following criteria:
if has_case_insensitive_prefix(cookie.name(), "__Host-") {
// 1. The cookie's secure-only-flag is true.
if !secure_only {
return None;
}
// 2. The cookie's host-only-flag is true.
if !host_only {
return None;
}
// 3. The cookie-attribute-list contains an attribute with an attribute-name of "Path",
// and the cookie's path is /.
if !has_path_specified || !cookie.path().is_some_and(|path| path == "/") {
return None;
}
}
// Step 22. If the cookie-name is empty and either of the following conditions are true,
// abort these steps and ignore the cookie entirely:
if cookie.name().is_empty() {
// 1. the cookie-value begins with a case-insensitive match for the string "__Secure-"
if has_case_insensitive_prefix(cookie.value(), "__Secure-") {
return None;
}
// 2. the cookie-value begins with a case-insensitive match for the string "__Host-"
if has_case_insensitive_prefix(cookie.value(), "__Host-") {
return None;
}
}
Some(ServoCookie {
@ -138,7 +243,7 @@ impl ServoCookie {
self.expiry_time = Some(SystemTime::UNIX_EPOCH)
}
// http://tools.ietf.org/html/rfc6265#section-5.1.4
/// <http://tools.ietf.org/html/rfc6265#section-5.1.4>
pub fn default_path(request_path: &str) -> &str {
// Step 2
if !request_path.starts_with('/') {
@ -156,7 +261,7 @@ impl ServoCookie {
&request_path[..rightmost_slash_idx]
}
// http://tools.ietf.org/html/rfc6265#section-5.1.4
/// <http://tools.ietf.org/html/rfc6265#section-5.1.4>
pub fn path_match(request_path: &str, cookie_path: &str) -> bool {
// A request-path path-matches a given cookie-path if at least one of
// the following conditions holds:
@ -175,7 +280,7 @@ impl ServoCookie {
))
}
// http://tools.ietf.org/html/rfc6265#section-5.1.3
/// <http://tools.ietf.org/html/rfc6265#section-5.1.3>
pub fn domain_match(string: &str, domain_string: &str) -> bool {
let string = &string.to_lowercase();
let domain_string = &domain_string.to_lowercase();
@ -187,7 +292,7 @@ impl ServoCookie {
string.parse::<Ipv6Addr>().is_err())
}
// http://tools.ietf.org/html/rfc6265#section-5.4 step 1
/// <http://tools.ietf.org/html/rfc6265#section-5.4> step 1
pub fn appropriate_for_url(&self, url: &ServoUrl, source: CookieSource) -> bool {
let domain = url.host_str();
if self.host_only {

View file

@ -79,13 +79,17 @@ fn fn_cookie_constructor() {
assert!(ServoCookie::new_wrapped(cookie, url, CookieSource::HTTP).is_none());
let cookie = cookie::Cookie::parse(" baz = bar ; Secure; Path = /foo/bar/").unwrap();
assert!(ServoCookie::new_wrapped(cookie, url, CookieSource::HTTP).is_some());
assert!(
ServoCookie::new_wrapped(cookie, url, CookieSource::HTTP).is_none(),
"Cookie with \"Secure\" attribute from non-secure source should be rejected"
);
let cookie = cookie::Cookie::parse(" baz = bar ; HttpOnly").unwrap();
assert!(ServoCookie::new_wrapped(cookie, url, CookieSource::NonHTTP).is_none());
let secure_url = &ServoUrl::parse("https://example.com/foo").unwrap();
let cookie = cookie::Cookie::parse(" baz = bar ; Secure; Path = /foo/bar/").unwrap();
let cookie = ServoCookie::new_wrapped(cookie, url, CookieSource::HTTP).unwrap();
let cookie = ServoCookie::new_wrapped(cookie, secure_url, CookieSource::HTTP).unwrap();
assert_eq!(cookie.cookie.value(), "bar");
assert_eq!(cookie.cookie.name(), "baz");
assert!(cookie.cookie.secure().unwrap_or(false));

View file

@ -1,9 +0,0 @@
[__host.document-cookie.html]
[__HoSt: Non-secure origin: 'Path=/;']
expected: FAIL
[__HoSt: Non-secure origin: 'Path=/;domain=web-platform.test']
expected: FAIL
[__HoSt: Non-secure origin: 'Path=/;MaxAge=10']
expected: FAIL

View file

@ -1,15 +0,0 @@
[__host.document-cookie.https.html]
[__HoSt: Secure origin: Does not set 'Path=/;']
expected: FAIL
[__HoSt: Secure origin: Does not set 'Secure; Path=/; Domain=web-platform.test; ']
expected: FAIL
[__HoSt: Secure origin: Does not set 'Path=/;MaxAge=10']
expected: FAIL
[__HoSt: Secure origin: Does not set 'Secure; Path=/; Domain=web-platform.test; MaxAge=10']
expected: FAIL
[__HoSt: Secure origin: Does not set 'Secure; Path=/cookies/resources/list.py']
expected: FAIL

View file

@ -1,12 +0,0 @@
[__host.header.html]
[__HoSt: Non-secure origin: Does not set 'Path=/;']
expected: FAIL
[__HoSt: Non-secure origin: Does not set 'Path=/;domain=web-platform.test']
expected: FAIL
[__HoSt: Non-secure origin: Does not set 'Path=/;MaxAge=10']
expected: FAIL
[__HoSt: Non-secure origin: Does not set 'Path=/;HttpOnly']
expected: FAIL

View file

@ -1,21 +0,0 @@
[__host.header.https.html]
[__HoSt: Secure origin: Does not set 'Path=/;']
expected: FAIL
[__HoSt: Secure origin: Does not set 'Secure; Path=/; Domain=web-platform.test; ']
expected: FAIL
[__HoSt: Secure origin: Does not set 'Path=/;MaxAge=10']
expected: FAIL
[__HoSt: Secure origin: Does not set 'Secure; Path=/; Domain=web-platform.test; MaxAge=10']
expected: FAIL
[__HoSt: Secure origin: Does not set 'Path=/;HttpOnly']
expected: FAIL
[__HoSt: Secure origin: Does not set 'Secure; Path=/; Domain=web-platform.test; HttpOnly']
expected: FAIL
[__HoSt: Secure origin: Does not set 'Secure; Path=/cookies/resources/list.py']
expected: FAIL

View file

@ -1,9 +0,0 @@
[__secure.document-cookie.html]
[__SeCuRe: Non-secure origin: Should not set 'Path=/;']
expected: FAIL
[__SeCuRe: Non-secure origin: Should not set 'Path=/;MaxAge=10']
expected: FAIL
[__SeCuRe: Non-secure origin: Should not set 'Path=/;domain=web-platform.test']
expected: FAIL

View file

@ -1,9 +0,0 @@
[__secure.document-cookie.https.html]
[__SeCuRe: Secure origin: Should not set 'Path=/;']
expected: FAIL
[__SeCuRe: Secure origin: Should not set 'Path=/;MaxAge=10']
expected: FAIL
[__SeCuRe: Secure origin: Should not set 'Path=/;domain=web-platform.test']
expected: FAIL

View file

@ -1,12 +0,0 @@
[__secure.header.html]
[__SeCuRe: Non-secure origin: Should not set 'Path=/;']
expected: FAIL
[__SeCuRe: Non-secure origin: Should not set 'Path=/;domain=web-platform.test']
expected: FAIL
[__SeCuRe: Non-secure origin: Should not set 'Path=/;MaxAge=10']
expected: FAIL
[__SeCuRe: Non-secure origin: Should not set 'Path=/;HttpOnly']
expected: FAIL

View file

@ -1,12 +0,0 @@
[__secure.header.https.html]
[__SeCuRe: secure origin: Should not set 'Path=/;']
expected: FAIL
[__SeCuRe: secure origin: Should not set 'Path=/;MaxAge=10']
expected: FAIL
[__SeCuRe: secure origin: Should not set 'Path=/;HttpOnly']
expected: FAIL
[__SeCuRe: secure origin: Should not set 'Path=/;domain=not-web-platform.test']
expected: FAIL

View file

@ -1,18 +0,0 @@
[document-cookie.non-secure.html]
[__SeCuRe: Non-secure origin: 'Path=/;']
expected: FAIL
[__SeCuRe: Non-secure origin: 'Path=/;domain=web-platform.test']
expected: FAIL
[__SeCuRe: Non-secure origin: 'Path=/;MaxAge=10']
expected: FAIL
[__HoSt: Non-secure origin: 'Path=/; ']
expected: FAIL
[__HoSt: Non-secure origin: 'Path=/; domain=web-platform.test']
expected: FAIL
[__HoSt: Non-secure origin: 'Path=/; MaxAge=10']
expected: FAIL