Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CB-12770: revise security documentation #703

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kerrishotts
Copy link
Contributor

This is a rough first pass at revising the security documentation. Comments are very welcome!

** NOT READY FOR MERGE YET **

What does this PR do?

Update security documentation

Checklist

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.

Copy link
Contributor

@filmaj filmaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together @kerrishotts ! A few comments but I think this is a major improvement.


### Do not cache sensitive data

If usernames, password, geolocation information, and other sensitive data is cached, then it could potentially be retrieved later by an unauthorized user or application.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this statement is a little alarmist without providing concrete examples. It would also depend on what one means by "caching." Have you seen or heard of instances of this kind of attack happening in Cordova apps?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this needs to be worded better and have some concrete examples. :-) I think "caching" is the wrong word here (this was copied from the previous version) -- "store" is probably more accurate.

I think attacks would either have to be device-in-hand attacks (since you'd need to get into the file system -- that's when you hope the device is protected with a pin & is encrypted!) or malware / viruses that abused some security hole (or ran amuck on a rooted/JB'd device). OR, if the app stored data in a public area on the file system (say, on Android), that would be risky too. (Hopefully no one would do that, though...!) So yes... more examples of what is meant by this would be good.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Stored" is a better choice. i.e.: localStorage, sqlite, etc.

As for the threat model, mostly device-in-hand, but also XSS. If the user Jailbreaks or Roots... the horse has bolted... not sure there is any way to protect that, heh.


### Do not assume that your source code is secure

Since a Cordova application is built from HTML and JavaScript assets that get packaged in a native container, you should not consider your code to be secure. It is possible to reverse engineer a Cordova application.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can qualify the "it is possible to reverse engineer a Cordova app" statement? E.g. "an iOS or Android application can be simply unpacked and unzipped to reveal its web assets such as HTML and JS."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm -- I like that much better. :-)


Due to the way the native portion of Cordova communicates with your web code, it is possible for any code executing within the main webview context to communicate with any installed plugins. This means that you should _never_ permit untrusted content within the primary webview. This can include third-party advertisements, sites within an `iframe`, and even content injected via `innerHTML`.

If you must inject content into the primary webview, be certain that it has been properly sanitized so that no JavaScript can be executed. _Do not try to sanitize content on your own; use a vetted third-party library instead!_
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any recommendations on libraries to use for sanitization?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OWASP has one that supports many different platforms: https://www.owasp.org/index.php/Category:OWASP_Enterprise_Security_API#tab=Home


The Content Security Policy `meta` tag, or CSP for short, is a very powerful mechanism you can use to control trusted sources of content. You can restrict various content types and restrict the domains from which content can be loaded. You can also disable unsafe and risky HTML and JavaScript, which can further increase the security of your app. The CSP tag should be placed in your app's `index.html` file.

> **Note**: If your app has multiple HTML files and navigates between them using the browser's navigation features, you should include the CSP in each file. If using a framework, you only need to include the CSP on `index.html`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like the 'if using a framework' line could be misinterpreted by someone with a bit less experience. I would change "if using a framework" to "if your app is a single-page application" or something like that. My intention with that change is to reinforce the already-made point that CSPs are per-page.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, you're right. I'll change that. :-)


Chances are good that when you add a CSP to your app, you'll encounter some problems. Thankfully both Google Chrome's developer tools and Safari's web inspector will make it glaringly obvious when the CSP has been violated. Watch the console for any violations, and fix accordingly. Generally the error messages are pretty verbose, indicating exactly what resource was rejected, and why.

TODO: include example
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah an example would be rad! 💃

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

;-) Will do :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kerrishotts ping me if I can help with an example


## Iframes and the Callback Id Mechanism
* `allow-navigation` tags control the resources to which the webview is allowed to navigate (top-level navigations only)
* `allow-intent` tags control the intents that can be used — for example, `tel:*` might be used to allow the dialoer to be displayed when the user taps a telephone link. This setting applies only to hyperlinks and `window.open()`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dialoer -> dialer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch!


### Use InAppBrowser for outside links

Use the InAppBrowser when opening links to any outside website. This is much safer than whitelisting a domain name and including the content directly in your application because the InAppBrowser will use the native browser's security features and will not give the website access to your Cordova environment. Even if you trust the third party website and include it directly in your application, that third party website could link to malicious web content.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to InAppBrowser would be great here


The above CSP enforces the following:

* Media can be loaded from anywhere
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverse order of explanations to match the example meta tag

* Inline styles are permitted (`'unsafe-inline'`)
* All other network requests can only be from (or to) the app itself, `data:` URLs, the iOS Cordova bridge (`gap:`), or to Android's TalkBack accessibility feature (`https://ssl.gstatic.com`)

By defalt, using this CSP will prevent _inline JavaScript_ and `eval()`. There are occasions, unfortunately, where a library may need one or the other, but this is rare and becoming moreso. If you must override this functionality, you can do so using the `script-src` directive.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo "defalt" => "default"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants