Implement default Content-Security-Policy that prevents clickjacking

[francisbeaudoin]

Details

The Shopify CLI uses the shopify_app gem to create a simple rails application by running shopify app create rails. Per our security requirement, apps on the Shopify App Store must set the proper Content Security Policy frame-ancestors directive to avoid clickjacking attacks.

Therefore, the generated app should have a defined Content-Security-Policy that at least sets the frame-ancestors directive. As for other directives (i.e. script-src, img-src, ...), they should be as strict as possible.

[resistorsoftware]

How do you do this on a per store basis? Set it in the Application Controller which all other controllers come from? What would it look like?

I tried this in my AuthenticatedController

content_security_policy do |policy|
    policy.frame_ancestors :https, -> { "https://#{current_shopify_domain} https://admin.shopify.com;" }
end

Inspecting the results showed the frame ancestors were set. So that is all it is?

[resistorsoftware]

It totally sucks that this issue remains open since February with zero resolution. A simple App from this gem will not pass Shopify robot on Clickjacking check, due to the CSP.

[denoyaro]

@resistorsoftware Does the solution you proposed of adding the CSP in the controller pass the Shopify check?

[resistorsoftware]

No. The problem is as I pointed out back then, the CSP seems to be run as part of the Rack response regardless of serving up a page in an embedded app or not. So during CSP initialization Shopify App gem helpers are not instantiated. Hence the Shopify App check that sends GDPR tests fails.

So if you CSP in Authenticated controller, that works. But since there is no CSP for Webhook controller, that fails the testing. But since the testing is completely hidden from us, and we only get a vague email about the failure modes, I am only able to guess that this is it.

Again, my complaints about this whole process can fall on deaf ears, but enough people are having trouble that it does seem like someone could do a better job on this from both side. The library side AND the testing side.

[denoyaro]

@resistorsoftware thanks for you response. From the docs, https://shopify.dev/apps/store/security/iframe-protection, I was under the impression that the CSP is only needed for routes that render HTML content, and not necessarily the webhooks.

Also in your solution I noticed you include the 'scheme-source' in the frame ancestors directive, while the example given in the docs only has the 'host-source'. Could this be why the test is failing - the frame ancestor directive is not considered set properly?

[resistorsoftware]

Hi,

I am aware of the docs saying render the CSP for routes in an embedded App. Not a problem. I do that. Thing is, it is easy when you have current_shopify_session set. So when I do it in the Authenicated Controller, those routes are CSP protected but then the App fails on CSP. OK. Why. Webhook route?

Anyway, I am asking the community at large here that knows both the testing and the library, to chime in. The Shopify solution does not work, as current_shopify_session helper in not available in initializer.

And yes, I removed the scheme-source which was also part of the given Shopify CSP solution. I am not about to make a complete PR on this without Shopify chiming in about what the tests really want, and the community saying, this is the way to go.

[denoyaro]

@fbeaudoin-shopify any idea how to properly implement this in order to pass the Shopify check?

[francisbeaudoin]

@denoyaro The frame-ancestors directive for the Content-Security-Policy header is only necessary for authenticated requests (routes). The example that @resistorsoftware provided that uses a lambda within the initializer is a good implementation but it is probably missing a fallback to [:https, "*.myshopify.com", "admin.shopify.com"] for unauthenticated requests (when current_shopify_domainis nil).

As for webhook endpoints, they don't require the Content-Security-Policy header.

[resistorsoftware]

@fbeaudoin-shopify So what exact code should be in this gem. You talk around the issue, but no one has provided a solution to the actual issue.

Thanks though for explaining it in more detail than anywhere else, by anyone else, for the most part. We learn piece by piece. Slowly.

[francisbeaudoin]

@resistorsoftware Unfortunately the team owning the gem has better context than me to implement hence this issue that I created.

[resistorsoftware]

It is so weird that such a small issue can take so many months to iron out. In the meantime, the crap shoot with the robot continues.

[denoyaro]

@fbeaudoin-shopify Thanks for your response. Since according to https://shopify.dev/apps/store/security/iframe-protection, the embedded app should only be frameable in an authenticated shop domain, shouldn't the fallback be Content-Security-Policy: frame-ancestors 'none';

[jkowens]

For anyone else who stumbles on this thread, it looks like this is addressed in an upcoming release: #1474

[nelsonwittwer]

#1474 should address the concerns pointed out here, so we are going to close this issue.

If you still encounter this issue with the latest stable version, please reopen using the issue template. You can also contribute directly by submitting a pull request– see the CONTRIBUTING.md(.github/CONTRIBUTING.md) file for guidelines