Skip to content

Commit

Permalink
Cherry-pick 9e08e9d. rdar://118121639
Browse files Browse the repository at this point in the history 
    Cookies from AppSSO extension are getting stored in iframe even when CSP restricts page to be loaded in iframe
    https://bugs.webkit.org/show_bug.cgi?id=264447
    rdar://118121639

    Reviewed by Brent Fulgham.

    In https://bugs.webkit.org/show_bug.cgi?id=260100, we added CSP validation when setting cookies
    in the response of an AppSSO request. However, in that patch, we consider CSP options that are
    only relevant for i-frames in the redirect case. In NetworkResourceLoader::shouldInterruptLoadForXFrameOptions,
    we do an early return in non-main frame cases, but do not in the check for AppSSO.

    In SOAuthorizationCoordinator::tryAuthorize, it can be gleamed that a non-mainframe navigation implies
    a SubFrameSOAuthorizationSession will be created. Therefore we only need to perform these i-frame specific
    CSP checks whenever we have a SubFrameSOAuthorizationSession.

    * Source/WebKit/UIProcess/Cocoa/SOAuthorization/SOAuthorizationSession.mm:
    (WebKit::SOAuthorizationSession::shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions):
    * Source/WebKit/UIProcess/Cocoa/SOAuthorization/SOAuthorizationSession.h:
    (WebKit::SOAuthorizationSession::shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions):
    * Source/WebKit/UIProcess/Cocoa/SOAuthorization/SOAuthorizationSession.mm:
    (WebKit::SOAuthorizationSession::shouldInterruptLoadForXFrameOptions): Deleted.
    (WebKit::SOAuthorizationSession::shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions): Deleted.
    * Source/WebKit/UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.h:
    * Source/WebKit/UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.mm:
    (WebKit::SubFrameSOAuthorizationSession::shouldInterruptLoadForXFrameOptions):
    (WebKit::SubFrameSOAuthorizationSession::shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions):

    Canonical link: https://commits.webkit.org/270422@main

Identifier: 267815.554@safari-7617-branch
Canonical link: https://commits.webkit.org/267815.558@safari-7617.1.17.11-branch
  • Loading branch information
@rjepstein
Dan Robson authored and rjepstein committed Nov 10, 2023
1 parent 51c02b0 commit 6bdd1ae
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 66 deletions.
4 changes: 1 addition & 3 deletions Source/WebKit/UIProcess/Cocoa/SOAuthorization/SOAuthorizationSession.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,7 @@ class SOAuthorizationSession : public ThreadSafeRefCountedAndCanMakeThreadSafeWe
void continueStartAfterGetAuthorizationHints(const String&);
void continueStartAfterDecidePolicy(const SOAuthorizationLoadPolicy&);

bool shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions(const WebCore::ResourceResponse&);
bool shouldInterruptLoadForXFrameOptions(Vector<RefPtr<WebCore::SecurityOrigin>>&& frameAncestorOrigins, const String& xFrameOptions, const URL&);

virtual bool shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions(const WebCore::ResourceResponse&) { return false; }
State m_state { State::Idle };
RetainPtr<SOAuthorization> m_soAuthorization;
RefPtr<API::NavigationAction> m_navigationAction;
Expand Down
63 changes: 0 additions & 63 deletions Source/WebKit/UIProcess/Cocoa/SOAuthorization/SOAuthorizationSession.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -394,69 +394,6 @@ static bool isSameOrigin(const WebCore::ResourceRequest& request, const WebCore:
uiCallback(YES, nil);
}

bool SOAuthorizationSession::shouldInterruptLoadForXFrameOptions(Vector<RefPtr<SecurityOrigin>>&& frameAncestorOrigins, const String& xFrameOptions, const URL& url)
{
switch (parseXFrameOptionsHeader(xFrameOptions)) {
case XFrameOptionsDisposition::None:
case XFrameOptionsDisposition::AllowAll:
return false;
case XFrameOptionsDisposition::Deny:
return true;
case XFrameOptionsDisposition::SameOrigin: {
auto origin = SecurityOrigin::create(url);
for (auto& ancestorOrigin : frameAncestorOrigins) {
if (!origin->isSameSchemeHostPort(*ancestorOrigin))
return true;
}
return false;
}
case XFrameOptionsDisposition::Conflict: {
String errorMessage = "Multiple 'X-Frame-Options' headers with conflicting values ('" + xFrameOptions + "') encountered. Falling back to 'DENY'.";
AUTHORIZATIONSESSION_RELEASE_LOG("shouldInterruptLoadForXFrameOptions: %s", errorMessage.utf8().data());
return true;
}
case XFrameOptionsDisposition::Invalid: {
String errorMessage = "Invalid 'X-Frame-Options' header encountered: '" + xFrameOptions + "' is not a recognized directive. The header will be ignored.";
AUTHORIZATIONSESSION_RELEASE_LOG("shouldInterruptLoadForXFrameOptions: %s", errorMessage.utf8().data());
return false;
}
}
ASSERT_NOT_REACHED();
return false;
}

bool SOAuthorizationSession::shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions(const WebCore::ResourceResponse& response)
{
Vector<RefPtr<SecurityOrigin>> frameAncestorOrigins;
if (auto* targetFrame = m_navigationAction->targetFrame()) {
if (auto parentFrameHandle = targetFrame->parentFrameHandle()) {
for (auto* parent = WebFrameProxy::webFrame(parentFrameHandle->frameID()); parent; parent = parent->parentFrame()) {
auto origin = SecurityOrigin::create(parent->url());
RefPtr<SecurityOrigin> frameOrigin = origin.ptr();
frameAncestorOrigins.append(frameOrigin);
}
}
}

auto url = response.url();
ContentSecurityPolicy contentSecurityPolicy { URL { url }, nullptr, nullptr };
contentSecurityPolicy.didReceiveHeaders(ContentSecurityPolicyResponseHeaders { response }, m_navigationAction->request().httpReferrer());
if (!contentSecurityPolicy.allowFrameAncestors(frameAncestorOrigins, url))
return true;

if (!contentSecurityPolicy.overridesXFrameOptions()) {
String xFrameOptions = response.httpHeaderField(HTTPHeaderName::XFrameOptions);
if (!xFrameOptions.isNull() && shouldInterruptLoadForXFrameOptions(WTFMove(frameAncestorOrigins), xFrameOptions, response.url())) {
String errorMessage = makeString("Refused to display '", response.url().stringCenterEllipsizedToLength(), "' in a frame because it set 'X-Frame-Options' to '", xFrameOptions, "'.");
AUTHORIZATIONSESSION_RELEASE_LOG("shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions: %s", errorMessage.utf8().data());

return true;
}
}

return false;
}

#if PLATFORM(MAC)
void SOAuthorizationSession::dismissModalSheetIfNecessary()
{
Expand Down
3 changes: 3 additions & 0 deletions Source/WebKit/UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ class SubFrameSOAuthorizationSession final : public NavigationSOAuthorizationSes
void appendRequestToLoad(URL&&, Supplement&&);
void loadRequestToFrame();

bool shouldInterruptLoadForXFrameOptions(Vector<RefPtr<WebCore::SecurityOrigin>>&& frameAncestorOrigins, const String& xFrameOptions, const URL&);
bool shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions(const WebCore::ResourceResponse&) final;

WebCore::FrameIdentifier m_frameID;
Deque<std::pair<URL, Supplement>> m_requestsToLoad;
};
Expand Down
65 changes: 65 additions & 0 deletions Source/WebKit/UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,71 @@
}
}

bool SubFrameSOAuthorizationSession::shouldInterruptLoadForXFrameOptions(Vector<RefPtr<SecurityOrigin>>&& frameAncestorOrigins, const String& xFrameOptions, const URL& url)
{
switch (parseXFrameOptionsHeader(xFrameOptions)) {
case XFrameOptionsDisposition::None:
case XFrameOptionsDisposition::AllowAll:
return false;
case XFrameOptionsDisposition::Deny:
return true;
case XFrameOptionsDisposition::SameOrigin: {
auto origin = SecurityOrigin::create(url);
for (auto& ancestorOrigin : frameAncestorOrigins) {
if (!origin->isSameSchemeHostPort(*ancestorOrigin))
return true;
}
return false;
}
case XFrameOptionsDisposition::Conflict: {
String errorMessage = "Multiple 'X-Frame-Options' headers with conflicting values ('" + xFrameOptions + "') encountered. Falling back to 'DENY'.";
AUTHORIZATIONSESSION_RELEASE_LOG("shouldInterruptLoadForXFrameOptions: %s", errorMessage.utf8().data());
return true;
}
case XFrameOptionsDisposition::Invalid: {
String errorMessage = "Invalid 'X-Frame-Options' header encountered: '" + xFrameOptions + "' is not a recognized directive. The header will be ignored.";
AUTHORIZATIONSESSION_RELEASE_LOG("shouldInterruptLoadForXFrameOptions: %s", errorMessage.utf8().data());
return false;
}
}
ASSERT_NOT_REACHED();
return false;
}

bool SubFrameSOAuthorizationSession::shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions(const WebCore::ResourceResponse& response)
{
Vector<RefPtr<SecurityOrigin>> frameAncestorOrigins;

ASSERT(navigationAction());
if (auto* targetFrame = navigationAction()->targetFrame()) {
if (auto parentFrameHandle = targetFrame->parentFrameHandle()) {
for (auto* parent = WebFrameProxy::webFrame(parentFrameHandle->frameID()); parent; parent = parent->parentFrame()) {
auto origin = SecurityOrigin::create(parent->url());
RefPtr<SecurityOrigin> frameOrigin = origin.ptr();
frameAncestorOrigins.append(frameOrigin);
}
}
}

auto url = response.url();
ContentSecurityPolicy contentSecurityPolicy { URL { url }, nullptr, nullptr };
contentSecurityPolicy.didReceiveHeaders(ContentSecurityPolicyResponseHeaders { response }, navigationAction()->request().httpReferrer());
if (!contentSecurityPolicy.allowFrameAncestors(frameAncestorOrigins, url))
return true;

if (!contentSecurityPolicy.overridesXFrameOptions()) {
String xFrameOptions = response.httpHeaderField(HTTPHeaderName::XFrameOptions);
if (!xFrameOptions.isNull() && shouldInterruptLoadForXFrameOptions(WTFMove(frameAncestorOrigins), xFrameOptions, response.url())) {
String errorMessage = makeString("Refused to display '", response.url().stringCenterEllipsizedToLength(), "' in a frame because it set 'X-Frame-Options' to '", xFrameOptions, "'.");
AUTHORIZATIONSESSION_RELEASE_LOG("shouldInterruptLoadForCSPFrameAncestorsOrXFrameOptions: %s", errorMessage.utf8().data());

return true;
}
}

return false;
}

} // namespace WebKit

#undef AUTHORIZATIONSESSION_RELEASE_LOG
Expand Down

0 comments on commit 6bdd1ae

Please sign in to comment.