Vendor feedback - some false positives

[scottohara]

Hi team,

I ran CSRT against one of our apps today and I wanted to provide some feedback on the results to help you fine tune the checks.

I asked over in CDAC where would be the best place for me to provide this feedback, and I was directed here.

So here goes...

Requirement 1 - Transport Layer Security

I believe this one is a known false positive. We host our apps on Heroku, and I'm aware that there has been quite a lot of discussion internally on this.

Last I heard, Heroku has been given an exemption from this rule until June 31, 2021; at which point we expect Heroku will have deprecated TLS 1.x.

Requirement 2 - Cache Control

Many of our apps are dynamic content macros for Confluence. We implement these using the Cacheable app iframes pattern, which is an official recommended pattern from Atlassian.

One of the key features/requirements of this pattern is to ensure that the response from the apps endpoint is cacheable by the browser.

This guidance seems to be at odds with the security requirement that says:

The application must disable caching on all HTTPS pages that contain sensitive data by using no-cache and no-store instead of private in the Cache-Control header.

In our case, the endpoints flagged by the report use Cache-Control: private (as per the linked guidance above), and they do no not contain any sensitive information returned from the server.

Requirement 5 - Authentication and Authorization of Application Resources

Similar to above, the Cacheable app iframe pattern shifts authentication, authorization and license checks to the client, via the AP.context API.

This means that the server endpoints no longer need authentication information to be provided, and are expected to return 200 OK responses.

(Note: Requirement 13 notes an exemption for Cacheable app iframes; and we believe the same exemption should apply to requirements 2 & 5)

Requirement 16 - App Name and Domain Branding Violations

Our app's Heroku domain (https://confluence-open-api.herokuapp.com) was flagged by this rule.

We are aware of the branding guidelines - our app was originally called "Confluence Open API Documentation" when it was first published to the Marketplace in 2016; and we were asked to rebrand it to "Open API Documentation for Confluence" shortly thereafter to comply.

However we would argue that the Heroku domain used by the app is not customer facing (i.e. we don't show it anywhere in the app itself, nor do we host things such as our app's documentation or supporting pages at that URL). It is where our Connect app and it's descriptor live, only.

In our opinion, we don't believe that the Heroku domain where a Connect app is published should contravene the branding guidelines. The rules for domains state:

When choosing a domain, don't use domains that misrepresent your company as Atlassian. Atlassian (or our products) should never be used in your domain. This is misleading because it represents you as Atlassian.

We interpret this as only applying to domains that a customer is likely to visit in their browser (or receive communication from, such as emails); where it is possible for the customer to confuse the domain as Atlassian. This is not the case for a Heroku domain that hosts a Connect app.

(Also, requirement 16 does not appear in the documented set of security requirements)

Anyway, I hope this feedback is useful to you in further improving the CSRT before you start scanning Marketplace apps in the new year.

[seanmarpo]

Hey @scottohara -- I appreciate you taking the time to run CSRT and provide such detailed feedback. I'll do my best to respond to each concern in the order it was asked.

1. I actually have this on an internal list of action items. I will create a public Github issue for this item.
2. Both the Cache-Control and Referrer Policy header checks are quite subjective. We are aware of this, but we're not entirely sure how we determine what is a sensitive endpoint and what is not. Rest assured, these issues won't be ticketed until these checks provide more confidence. We are also being asked in different forums for the ability to ignore certain checks. This may be another solution for folks that want to run CSRT in an automated fashion.
3. I appreciate the call-out here, and I'll make a public Github issue to follow-up on this.
4. The branding checks are something we considered internally to help with app approval processes. You are encouraged to run the tool with the --skip-branding argument to avoid concerns with these findings unless you'd like the checks to run.

[gtch]

We are seeing the same problem with requirements 2 & 5 on the cacheable app iframe that we use in our Jira app ProForma: the CSRT reports a failed check that we think is a false positive. Sounds like you agree that the authentication requirement 5 is a bug that will be fixed, so that's great.

I encourage you to also treat the caching requirement 2 on cacheable iframes as a bug that needs fixing, ie make the CSRT expect caching where the descriptor has "cacheable":true. I wouldn't want to have to disable all cache checks just because we have on one cacheable endpoint; that would mean if we accidentally mess up the cache header on our other endpoints the CSRT wouldn't detect that.

[seanmarpo]

Modules marked as cacheable will no longer be reported for failing Requirement 5 (Authentication/Authorization Checks).

This does mean those endpoints are unfortunately being ignored for Cache-Control/Referrer-Policy checks. I will be creating a follow-up action item to deal with that in a future update.

Apps hosted on Heroku should now reflect a pass on Requirement 1.