Bug 157252 - CSP: Add workaround for XtraMath
Summary: CSP: Add workaround for XtraMath
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Local Build
Hardware: iPhone / iPad Unspecified
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-05-01 17:33 PDT by Daniel Bates
Modified: 2016-05-02 09:27 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.56 KB, patch)
2016-05-01 17:36 PDT, Daniel Bates
no flags 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-05-01 17:33:19 PDT
Similar to the purpose of bug #157005, the app XtraMath depends on * matching an arbitrary protocol. Following bug #154122, we restrict matching of * to protocols HTTP, HTTPS in most circumstances. Add a app-specific workaround for this XtraMath.

<rdar://problem/25881955>
Comment 1 Daniel Bates 2016-05-01 17:36:11 PDT
Created attachment 277880 [details]
Patch
Comment 2 Darin Adler 2016-05-01 17:40:18 PDT
Comment on attachment 277880 [details]
Patch

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

Was about to say review+, but Andy beat me to it.

I do have this one comment:

> Source/WebKit/mac/WebView/WebView.mm:868
>  static bool shouldAllowContentSecurityPolicySourceStarToMatchAnyProtocol()

We should consider grouping these functions that make decisions about these kinds of quirks and workarounds in a header analogous to the RuntimeApplicationChecks one. This header would never mention specific application but would just list all the different quirks. The implementation file would contain all the expressions used to decide when each quirk applies, and comments about why each implements the right policy.

The implementations of the quirks would continue to be distributed throughout the code, but the policy of which quirks apply when would be grouped together.
Comment 3 Daniel Bates 2016-05-02 09:14:10 PDT
(In reply to comment #2)
> [...]
> I do have this one comment:
> 
> > Source/WebKit/mac/WebView/WebView.mm:868
> >  static bool shouldAllowContentSecurityPolicySourceStarToMatchAnyProtocol()
> 
> We should consider grouping these functions that make decisions about these
> kinds of quirks and workarounds in a header analogous to the
> RuntimeApplicationChecks one. This header would never mention specific
> application but would just list all the different quirks. The implementation
> file would contain all the expressions used to decide when each quirk
> applies, and comments about why each implements the right policy.
> 
> The implementations of the quirks would continue to be distributed
> throughout the code, but the policy of which quirks apply when would be
> grouped together.

I hope you do not mind that I defer such work to bug #157267 and keep this bug focused on the workaround.
Comment 4 Daniel Bates 2016-05-02 09:15:21 PDT
Comment on attachment 277880 [details]
Patch

Clearing flags on attachment: 277880

Committed r200323: <http://trac.webkit.org/changeset/200323>
Comment 5 Daniel Bates 2016-05-02 09:15:25 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Timothy Hatcher 2016-05-02 09:27:05 PDT
Comment on attachment 277880 [details]
Patch

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

> Source/WebKit/mac/WebView/WebView.mm:871
> +    static bool shouldAllowContentSecurityPolicySourceStarToMatchAnyProtocol = (IOSApplication::isEcobee() || IOSApplication::isQuora() || IOSApplication::isXtraMath()) && !WebKitLinkedOnOrAfter(WEBKIT_FIRST_VERSION_WITH_CONTENT_SECURITY_POLICY_SOURCE_STAR_PROTOCOL_RESTRICTION);

If we run into more apps doing this, I think we should consider dropping the bundle checks and just allow star to match any protocol for any app linked on older WebKit versions.