-
Notifications
You must be signed in to change notification settings - Fork 557
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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."
There was a problem hiding this comment.
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!_ |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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`. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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! 💃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
;-) Will do :-)
There was a problem hiding this comment.
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()`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dialoer -> dialer
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo "defalt" => "default"
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