Skip to content
Permalink
Browse files
Cap cookie lifetimes to 7 days for responses from third party IP addr…
…esses

https://bugs.webkit.org/show_bug.cgi?id=246477
rdar://100831206

Reviewed by John Wilander.

Safari currently caps the lifetime of cookies to 7 days, if third-party CNAME cloaking is detected.
This helps to mitigate many instances where CNAME cloaking is used to store cookies on device (in
the first party context) for far longer than a third party cookie would normally be allowed to;
however, in the case where the resolved CNAME is empty, we end up skipping this mitigation
altogether.

This means that websites can use direct A/AAAA records (instead of CNAME mapping) to cloak third
party requests as first party and subsequently store cookies in the first party context, bypassing
the aforementioned defense.

To strengthen our existing protections, we implement a heuristic to fall back on comparing resolved
IP addresses only in the case where the resolved CNAME of the incoming response is empty. If the IP
address of the response is _mostly_ different than the IP address of the main resource response
(i.e. by comparing the matching subnet mask length of the two addresses), then we apply the same
level of mitigation as we otherwise would for third party CNAMEs.

For now, the minimum matching subnet mask length to consider as "third party" or not is arbitrarily
chosen to be half the IP address length (i.e. 16 for IPv4, and 64 for IPv6). This could be enhanced
in the future, given facilities to query for the IP network block that contains the main resource's
IP address and checking whether the incoming response address falls within that range.

* Source/WebCore/platform/network/DNS.cpp:
(WebCore::IPAddress::isolatedCopy const):

Add an `isolatedCopy` method, so that we're able to perform a cross-thread copy of `IPAddress`.

(WebCore::IPAddress::matchingNetMaskLength const):

Add a helper method to compute the length of the matching subnet mask between the current IP address
and the given address. If the two IP addresses are of different families (i.e. v4 and v6), this
method returns 0.

* Source/WebCore/platform/network/DNS.h:
(WebCore::IPAddress::fromSockAddrIn6):

Minor style fix - add a missing space after the initializer.

* Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.h:
* Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
(WebKit::NetworkDataTaskCocoa::shouldApplyCookiePolicyForThirdPartyCloaking const):

Adjust this to check for third party IP addresses, in the case where the incoming response's CNAME
is empty.

(WebKit::NetworkDataTaskCocoa::updateFirstPartyInfoForSession):
(WebKit::shouldCapCookieExpiryForThirdPartyIPAddress):
(WebKit::NetworkDataTaskCocoa::applyCookiePolicyForThirdPartyCloaking):
(WebKit::NetworkDataTaskCocoa::NetworkDataTaskCocoa):
(WebKit::NetworkDataTaskCocoa::willPerformHTTPRedirection):
(WebKit::NetworkDataTaskCocoa::shouldApplyCookiePolicyForThirdPartyCNAMECloaking const): Deleted.
(WebKit::NetworkDataTaskCocoa::applyCookiePolicyForThirdPartyCNAMECloaking): Deleted.

Rename these to reference "ThirdPartyCloaking" instead of "ThirdPartyCNAMECloaking", since this now
applies to both.

* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* Tools/TestWebKitAPI/Tests/WebCore/IPAddressTests.cpp: Added.
(TestWebKitAPI::TEST):

Add a couple of API tests to exercise the new functionality in `WebCore::IPAddress`.

Canonical link: https://commits.webkit.org/255849@main
  • Loading branch information
whsieh committed Oct 21, 2022
1 parent 8070eaf commit b0305b173106ba984cbc0475b3681daea137390c
Show file tree
Hide file tree
Showing 7 changed files with 176 additions and 24 deletions.
@@ -76,4 +76,51 @@ std::optional<IPAddress> IPAddress::fromString(const String& string)
return std::nullopt;
}

IPAddress IPAddress::isolatedCopy() const
{
if (isIPv4())
return IPAddress { ipv4Address() };

if (isIPv6())
return IPAddress { ipv6Address() };

RELEASE_ASSERT_NOT_REACHED();
return IPAddress { WTF::HashTableEmptyValue };
}

unsigned IPAddress::matchingNetMaskLength(const IPAddress& other) const
{
const unsigned char* addressData = nullptr;
const unsigned char* otherAddressData = nullptr;
size_t addressLengthInBytes = 0;
if (isIPv4() && other.isIPv4()) {
addressLengthInBytes = sizeof(struct in_addr);
addressData = reinterpret_cast<const unsigned char*>(&ipv4Address());
otherAddressData = reinterpret_cast<const unsigned char*>(&other.ipv4Address());
} else if (isIPv6() && other.isIPv6()) {
addressLengthInBytes = sizeof(struct in6_addr);
addressData = reinterpret_cast<const unsigned char*>(&ipv6Address());
otherAddressData = reinterpret_cast<const unsigned char*>(&other.ipv6Address());
} else
return 0;

std::optional<size_t> firstNonMatchingIndex;
unsigned matchingBits = 0;
for (size_t i = 0; i < addressLengthInBytes; ++i) {
if (addressData[i] != otherAddressData[i]) {
firstNonMatchingIndex = i;
break;
}
matchingBits += 8;
}

if (firstNonMatchingIndex) {
// Count the number of matching leading bits in the first byte that is different between the two addresses.
// For instance, 0xF2 and 0xF3 would have 7 matching bits.
matchingBits += clz<unsigned char>(addressData[*firstNonMatchingIndex] ^ otherAddressData[*firstNonMatchingIndex]);
}

return matchingBits;
}

}
@@ -57,6 +57,8 @@ class IPAddress {
{
}

WEBCORE_EXPORT IPAddress isolatedCopy() const;
WEBCORE_EXPORT unsigned matchingNetMaskLength(const IPAddress& other) const;
WEBCORE_EXPORT static std::optional<IPAddress> fromString(const String&);

bool isIPv4() const { return std::holds_alternative<struct in_addr>(m_address); }
@@ -83,7 +85,7 @@ inline std::optional<IPAddress> IPAddress::fromSockAddrIn6(const struct sockaddr
if (address.sin6_family == AF_INET6)
return IPAddress { address.sin6_addr };
if (address.sin6_family == AF_INET)
return IPAddress {reinterpret_cast<const struct sockaddr_in&>(address).sin_addr };
return IPAddress { reinterpret_cast<const struct sockaddr_in&>(address).sin_addr };
return { };
}

@@ -360,7 +360,11 @@ std::optional<WebCore::IPAddress> NetworkSession::firstPartyHostIPAddress(const
if (firstPartyHost.isEmpty())
return std::nullopt;

return m_firstPartyHostIPAddresses.get(firstPartyHost);
auto iterator = m_firstPartyHostIPAddresses.find(firstPartyHost);
if (iterator == m_firstPartyHostIPAddresses.end())
return std::nullopt;

return { iterator->value };
}
#endif // ENABLE(TRACKING_PREVENTION)

@@ -101,8 +101,8 @@ class NetworkDataTaskCocoa final : public NetworkDataTask {
#if ENABLE(TRACKING_PREVENTION)
static NSHTTPCookieStorage *statelessCookieStorage();
void updateFirstPartyInfoForSession(const URL&);
bool shouldApplyCookiePolicyForThirdPartyCNAMECloaking() const;
void applyCookiePolicyForThirdPartyCNAMECloaking(const WebCore::ResourceRequest&);
bool shouldApplyCookiePolicyForThirdPartyCloaking() const;
void applyCookiePolicyForThirdPartyCloaking(const WebCore::ResourceRequest&);
void blockCookies();
void unblockCookies();
bool needsFirstPartyCookieBlockingLatchModeQuirk(const URL& firstPartyURL, const URL& requestURL, const URL& redirectingURL) const;
@@ -166,15 +166,15 @@ static float toNSURLSessionTaskPriority(WebCore::ResourceLoadPriority priority)
return { };
}

bool NetworkDataTaskCocoa::shouldApplyCookiePolicyForThirdPartyCNAMECloaking() const
bool NetworkDataTaskCocoa::shouldApplyCookiePolicyForThirdPartyCloaking() const
{
auto* session = networkSession();
return session && session->networkStorageSession() && session->networkStorageSession()->resourceLoadStatisticsEnabled();
}

void NetworkDataTaskCocoa::updateFirstPartyInfoForSession(const URL& requestURL)
{
if (!shouldApplyCookiePolicyForThirdPartyCNAMECloaking() || requestURL.host().isEmpty())
if (!shouldApplyCookiePolicyForThirdPartyCloaking() || requestURL.host().isEmpty())
return;

auto* session = networkSession();
@@ -186,9 +186,25 @@ static float toNSURLSessionTaskPriority(WebCore::ResourceLoadPriority priority)
session->setFirstPartyHostIPAddress(requestURL.host().toString(), ipAddress);
}

void NetworkDataTaskCocoa::applyCookiePolicyForThirdPartyCNAMECloaking(const WebCore::ResourceRequest& request)
static NSArray<NSHTTPCookie *> *cookiesByCappingExpiry(NSArray<NSHTTPCookie *> *cookies, Seconds ageCap)
{
if (isTopLevelNavigation() || !shouldApplyCookiePolicyForThirdPartyCNAMECloaking())
auto *cappedCookies = [NSMutableArray arrayWithCapacity:cookies.count];
for (NSHTTPCookie *cookie in cookies)
[cappedCookies addObject:WebCore::NetworkStorageSession::capExpiryOfPersistentCookie(cookie, ageCap)];
return cappedCookies;
}

static bool shouldCapCookieExpiryForThirdPartyIPAddress(const WebCore::IPAddress& remote, const WebCore::IPAddress& firstParty)
{
auto matchingLength = remote.matchingNetMaskLength(firstParty);
if (remote.isIPv4())
return matchingLength < 4 * sizeof(struct in_addr);
return matchingLength < 4 * sizeof(struct in6_addr);
}

void NetworkDataTaskCocoa::applyCookiePolicyForThirdPartyCloaking(const WebCore::ResourceRequest& request)
{
if (isTopLevelNavigation() || !shouldApplyCookiePolicyForThirdPartyCloaking())
return;

if (request.isThirdParty()) {
@@ -200,38 +216,50 @@ static float toNSURLSessionTaskPriority(WebCore::ResourceLoadPriority priority)
// subresource but it resolves to a different CNAME than the top
// site request, a.k.a. third-party CNAME cloaking.
auto firstPartyURL = request.firstPartyForCookies();
auto firstPartyHostCNAME = networkSession()->firstPartyHostCNAMEDomain(firstPartyURL.host().toString());
auto firstPartyHostName = firstPartyURL.host().toString();
auto firstPartyHostCNAME = networkSession()->firstPartyHostCNAMEDomain(firstPartyHostName);
auto firstPartyAddress = networkSession()->firstPartyHostIPAddress(firstPartyHostName);

m_task.get()._cookieTransformCallback = makeBlockPtr([requestURL = crossThreadCopy(request.url()), firstPartyURL = crossThreadCopy(firstPartyURL), firstPartyHostCNAME = crossThreadCopy(firstPartyHostCNAME), thirdPartyCNAMEDomainForTesting = crossThreadCopy(networkSession()->thirdPartyCNAMEDomainForTesting()), ageCapForCNAMECloakedCookies = crossThreadCopy(m_ageCapForCNAMECloakedCookies), weakTask = WeakObjCPtr<NSURLSessionDataTask>(m_task.get()), debugLoggingEnabled = networkSession()->networkStorageSession()->resourceLoadStatisticsDebugLoggingEnabled()] (NSArray<NSHTTPCookie*> *cookiesSetInResponse) -> NSArray<NSHTTPCookie*> * {
m_task.get()._cookieTransformCallback = makeBlockPtr([requestURL = crossThreadCopy(request.url()), firstPartyURL = crossThreadCopy(firstPartyURL), firstPartyHostCNAME = crossThreadCopy(firstPartyHostCNAME), firstPartyAddress = crossThreadCopy(firstPartyAddress), thirdPartyCNAMEDomainForTesting = crossThreadCopy(networkSession()->thirdPartyCNAMEDomainForTesting()), ageCapForCNAMECloakedCookies = crossThreadCopy(m_ageCapForCNAMECloakedCookies), weakTask = WeakObjCPtr<NSURLSessionDataTask>(m_task.get()), debugLoggingEnabled = networkSession()->networkStorageSession()->resourceLoadStatisticsDebugLoggingEnabled()] (NSArray<NSHTTPCookie*> *cookiesSetInResponse) -> NSArray<NSHTTPCookie*> * {
auto task = weakTask.get();
if (!task || ![cookiesSetInResponse count])
return cookiesSetInResponse;

auto cnameDomain = lastCNAMEDomain([task _resolvedCNAMEChain]);
if (cnameDomain.isEmpty() && thirdPartyCNAMEDomainForTesting)
cnameDomain = *thirdPartyCNAMEDomainForTesting;

if (cnameDomain.isEmpty()) {
if (!thirdPartyCNAMEDomainForTesting)
if (!firstPartyAddress)
return cookiesSetInResponse;
cnameDomain = *thirdPartyCNAMEDomainForTesting;

auto remoteAddress = WebCore::IPAddress::fromString(lastRemoteIPAddress(task.get()));
if (!remoteAddress)
return cookiesSetInResponse;

if (shouldCapCookieExpiryForThirdPartyIPAddress(*remoteAddress, *firstPartyAddress)) {
cookiesSetInResponse = cookiesByCappingExpiry(cookiesSetInResponse, ageCapForCNAMECloakedCookies);
if (debugLoggingEnabled) {
for (NSHTTPCookie *cookie in cookiesSetInResponse)
RELEASE_LOG_INFO(ITPDebug, "Capped the expiry of third-party IP address cookie named %{public}@.", cookie.name);
}
}

return cookiesSetInResponse;
}

// CNAME cloaking is a first-party sub resource that resolves
// through a CNAME that differs from the first-party domain and
// also differs from the top frame host's CNAME, if one exists.
if (!cnameDomain.matches(firstPartyURL) && (!firstPartyHostCNAME || cnameDomain != *firstPartyHostCNAME)) {

NSUInteger count = [cookiesSetInResponse count];
// Don't use RetainPtr here. This array has to be retained and
// auto released to not be released before returned to the code
// executing the block.
auto* cappedCookies = [NSMutableArray arrayWithCapacity:count];
for (NSUInteger i = 0; i < count; ++i) {
NSHTTPCookie *cookie = (NSHTTPCookie *)[cookiesSetInResponse objectAtIndex:i];
cookie = WebCore::NetworkStorageSession::capExpiryOfPersistentCookie(cookie, ageCapForCNAMECloakedCookies);
[cappedCookies addObject:cookie];
if (debugLoggingEnabled)
RELEASE_LOG_INFO(ITPDebug, "Capped the expiry of third-party CNAME cloaked cookie named %{public}s.", [[cookie name] UTF8String]);
cookiesSetInResponse = cookiesByCappingExpiry(cookiesSetInResponse, ageCapForCNAMECloakedCookies);
if (debugLoggingEnabled) {
for (NSHTTPCookie *cookie in cookiesSetInResponse)
RELEASE_LOG_INFO(ITPDebug, "Capped the expiry of third-party CNAME cloaked cookie named %{public}@.", cookie.name);
}
return cappedCookies;
}

return cookiesSetInResponse;
@@ -427,7 +455,7 @@ static inline bool computeIsAlwaysOnLoggingAllowed(NetworkSession& session)
}

#if ENABLE(TRACKING_PREVENTION)
applyCookiePolicyForThirdPartyCNAMECloaking(request);
applyCookiePolicyForThirdPartyCloaking(request);
if (shouldBlockCookies) {
#if !RELEASE_LOG_DISABLED
if (m_session->shouldLogCookieInformation())
@@ -588,7 +616,7 @@ static inline bool computeIsAlwaysOnLoggingAllowed(NetworkSession& session)
#endif

#if ENABLE(TRACKING_PREVENTION)
applyCookiePolicyForThirdPartyCNAMECloaking(request);
applyCookiePolicyForThirdPartyCloaking(request);
if (!m_hasBeenSetToUseStatelessCookieStorage) {
if (m_storedCredentialsPolicy == WebCore::StoredCredentialsPolicy::EphemeralStateless
|| (m_session->networkStorageSession() && m_session->networkStorageSession()->shouldBlockCookies(request, m_frameID, m_pageID, m_shouldRelaxThirdPartyCookieBlocking)))
@@ -1181,6 +1181,7 @@
F4B0168425AE08F800E445C4 /* DisableSpellcheckPlugIn.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4B0167F25AE02D600E445C4 /* DisableSpellcheckPlugIn.mm */; };
F4B825D81EF4DBFB006E417F /* compressed-files.zip in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4B825D61EF4DBD4006E417F /* compressed-files.zip */; };
F4B86D4F20BCD5B20099A7E6 /* paint-significant-area-milestone.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4B86D4E20BCD5970099A7E6 /* paint-significant-area-milestone.html */; };
F4B8A5C128F8B54F00E3F54F /* IPAddressTests.cpp in Sources */ = {isa = PBXBuildFile; fileRef = F4B8A5C028F8B54F00E3F54F /* IPAddressTests.cpp */; };
F4BC0B142146C849002A0478 /* FocusPreservationTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4BC0B132146C849002A0478 /* FocusPreservationTests.mm */; };
F4BDA43027F8D19C00F9647D /* element-fullscreen.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4BDA42F27F8CF5600F9647D /* element-fullscreen.html */; };
F4BFA68E1E4AD08000154298 /* LegacyDragAndDropTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F4BFA68C1E4AD08000154298 /* LegacyDragAndDropTests.mm */; };
@@ -3419,6 +3420,7 @@
F4B0168225AE060F00E445C4 /* DisableSpellcheck.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = DisableSpellcheck.mm; sourceTree = "<group>"; };
F4B825D61EF4DBD4006E417F /* compressed-files.zip */ = {isa = PBXFileReference; lastKnownFileType = archive.zip; path = "compressed-files.zip"; sourceTree = "<group>"; };
F4B86D4E20BCD5970099A7E6 /* paint-significant-area-milestone.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "paint-significant-area-milestone.html"; sourceTree = "<group>"; };
F4B8A5C028F8B54F00E3F54F /* IPAddressTests.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = IPAddressTests.cpp; sourceTree = "<group>"; };
F4BC0B132146C849002A0478 /* FocusPreservationTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = FocusPreservationTests.mm; sourceTree = "<group>"; };
F4BDA42E27F8BF2F00F9647D /* FullscreenVideoTextRecognition.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = FullscreenVideoTextRecognition.mm; sourceTree = "<group>"; };
F4BDA42F27F8CF5600F9647D /* element-fullscreen.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "element-fullscreen.html"; sourceTree = "<group>"; };
@@ -4168,6 +4170,7 @@
7A909A731D877475007E10F8 /* IntPointTests.cpp */,
7A909A741D877475007E10F8 /* IntRectTests.cpp */,
7A909A751D877475007E10F8 /* IntSizeTests.cpp */,
F4B8A5C028F8B54F00E3F54F /* IPAddressTests.cpp */,
CD5FF4962162E27E004BD86F /* ISOBox.cpp */,
278E3E1823CD82FA005A6B80 /* KeyedCoding.cpp */,
14464012167A8305000BD218 /* LayoutUnitTests.cpp */,
@@ -6254,6 +6257,7 @@
7A909A811D877480007E10F8 /* IntPointTests.cpp in Sources */,
7A909A821D877480007E10F8 /* IntRectTests.cpp in Sources */,
7A909A831D877480007E10F8 /* IntSizeTests.cpp in Sources */,
F4B8A5C128F8B54F00E3F54F /* IPAddressTests.cpp in Sources */,
5C0BF8931DD599BD00B00328 /* IsNavigationActionTrusted.mm in Sources */,
CD5FF49F2162E943004BD86F /* ISOBox.cpp in Sources */,
7CCE7EA51A411A0800447C4C /* JavaScriptTestMac.mm in Sources */,
@@ -0,0 +1,67 @@
/*
* Copyright (C) 2022 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
* THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
* PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
* BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
* THE POSSIBILITY OF SUCH DAMAGE.
*/

#include "config.h"

#include <WebCore/DNS.h>
#include <wtf/text/WTFString.h>

namespace TestWebKitAPI {

#if OS(UNIX)

TEST(IPAddressTests, MatchingNetMaskLength)
{
auto address1 = WebCore::IPAddress::fromString("17.100.120.255"_s);
auto address2 = WebCore::IPAddress::fromString("17.100.100.255"_s);
auto address3 = WebCore::IPAddress::fromString("2001:db8::1234:5678"_s);
auto address4 = WebCore::IPAddress::fromString("2001:db8::1111:0000"_s);
auto address5 = WebCore::IPAddress::fromString("::1234:5678"_s);
auto address6 = WebCore::IPAddress::fromString("::"_s);

EXPECT_EQ(address1->matchingNetMaskLength(*address2), 19U);
EXPECT_EQ(address2->matchingNetMaskLength(*address1), 19U);
EXPECT_EQ(address1->matchingNetMaskLength(*address3), 0U);
EXPECT_EQ(address3->matchingNetMaskLength(*address1), 0U);
EXPECT_EQ(address3->matchingNetMaskLength(*address4), 102U);
EXPECT_EQ(address4->matchingNetMaskLength(*address3), 102U);
EXPECT_EQ(address3->matchingNetMaskLength(*address5), 2U);
EXPECT_EQ(address5->matchingNetMaskLength(*address3), 2U);
EXPECT_EQ(address5->matchingNetMaskLength(*address6), 99U);
EXPECT_EQ(address6->matchingNetMaskLength(*address5), 99U);
}

TEST(IPAddressTests, InvalidAddresses)
{
EXPECT_EQ(WebCore::IPAddress::fromString(""_s), std::nullopt);
EXPECT_EQ(WebCore::IPAddress::fromString("foo"_s), std::nullopt);
EXPECT_EQ(WebCore::IPAddress::fromString("2001:88888::"_s), std::nullopt);
EXPECT_EQ(WebCore::IPAddress::fromString("192.168.255.256"_s), std::nullopt);
}

#endif // OS(UNIX)

} // namespace TestWebKitAPI

0 comments on commit b0305b1

Please sign in to comment.