Bug 157005 - CSP: Add app-specific workaround for Ecobee and Quora
Summary: CSP: Add app-specific workaround for Ecobee and Quora
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Local Build
Hardware: iPhone / iPad Unspecified
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks: 157100
  Show dependency treegraph
 
Reported: 2016-04-25 16:37 PDT by Daniel Bates
Modified: 2016-04-27 13:32 PDT (History)
5 users (show)

See Also:


Attachments
Patch (8.82 KB, patch)
2016-04-25 16:51 PDT, Daniel Bates
bfulgham: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2016-04-25 16:37:45 PDT
As of the time of writing, the Ecobee and Quora iOS apps depend on * matching a non-HTTP/HTTPS protocol. Following bug #154122, we restrict matching of * to protocols HTTP, HTTPS in most circumstances. While we work with Ecobee and Quora to resolve this issue in their app, we should add a workaround to avoid breaking these apps.
Comment 1 Daniel Bates 2016-04-25 16:38:22 PDT
<rdar://problem/25560776>
Comment 2 Daniel Bates 2016-04-25 16:51:39 PDT
Created attachment 277294 [details]
Patch

I did not write a layout test for this because the proposed setting is specific to the workaround for Quora and Ecobee and not available to embedding clients. This setting should be short lived. Let me know if it is preferred to expose this setting via InternalSettings.in and write a test for this change.
Comment 3 Brent Fulgham 2016-04-25 19:55:57 PDT
Comment on attachment 277294 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=277294&action=review

r=me

> Source/WebCore/platform/RuntimeApplicationChecks.h:77
> +WEBCORE_EXPORT bool isQuora();

I wonder if it would be better to just have a generic "allowContentSecurityPolicySourceStarToMatchAnyProtocol" in case we discover other apps with this same quirk? Since we treat these two apps the same way, I'm not sure there is any benefit to separating them.
Comment 4 Daniel Bates 2016-04-26 10:53:02 PDT
(In reply to comment #3)
> > Source/WebCore/platform/RuntimeApplicationChecks.h:77
> > +WEBCORE_EXPORT bool isQuora();
> 
> I wonder if it would be better to just have a generic
> "allowContentSecurityPolicySourceStarToMatchAnyProtocol" in case we discover
> other apps with this same quirk?

I would prefer that we make an exception for Ecobee and Quora instead of enabling this quirk for all apps or all apps linked before the change made in bug #154122 because the change made in bug #154122 improves app security by disallowing all non-HTTP/HTTPs (e.g. file:) subresources that would have been allowed by a policy that contained *. We can evaluate this decision again should we identify more affected apps.

> Since we treat these two apps the same way, I'm not sure there is any benefit to separating them.

From my understanding the preferred way to accomplish this is to enable the setting allowContentSecurityPolicySourceStarToMatchAnyProtocol for any app built linked against WebKit before WEBKIT_FIRST_VERSION_WITH_CONTENT_SECURITY_POLICY_SOURCE_STAR_PROTOCOL_RESTRICTION. That is, revert the changes to RuntimeApplicationChecks.h and update shouldAllowContentSecurityPolicySourceStarToMatchAnyProtocol() to read:

static bool shouldAllowContentSecurityPolicySourceStarToMatchAnyProtocol()
{
#if PLATFORM(IOS)
    static bool shouldAllowContentSecurityPolicySourceStarToMatchAnyProtocol = !WebKitLinkedOnOrAfter(WEBKIT_FIRST_VERSION_WITH_CONTENT_SECURITY_POLICY_SOURCE_STAR_PROTOCOL_RESTRICTION);
    return shouldAllowContentSecurityPolicySourceStarToMatchAnyProtocol;
#else
    return false;
#endif
}
Comment 5 Daniel Bates 2016-04-27 09:48:33 PDT
Committed r200130: <http://trac.webkit.org/changeset/200130>