Tutanota: Out-of-beta security review

[armhub]

[bedhub]

953ea10

[mpfau]

fe739507f1b89d2de52a5b498e668fc9cb264c69
988d2e00ad3f21b63dc1d7ad182de03c33fe277b
06bf731930b9c039b434c3818ba8ec4830624578

[mpfau]

New client security review

We planned to do an extensive security review of the new beta client before releasing it and realized that from 18.10.2018 until 24.10.2018.

The following is a summary of what we reviewed, tested and what we found as part of the security review. We did not find any severe issue. We did find a few bugs that could influence client security if a layer of protection (e.g. CSP) would not be working for some reason.

This security review focused on the client security (not server) and targeted the following topics:
Injections and XSS, exposure of sensitive data, security misconfigurations, insecure deserializations and components with known vulnerabilities.

All of the tests described in the following sections have been executed in local test environments and did not influence the production systems. We worked in pairs during the security review.

Injections and XSS

There are two mechanisms that protect Tutanota users from code injections and XSS. These are Content Security Policy (CSP) and sanitizing. We tried to exploit each of them separately. We also tried to identify problems with raw HTML insertions and insecure evaluations of code.

Content Security Policy

We completely disabled sanitizing in order for being able to test the effectiveness of the CSP. Afterwards, we sent mails that contained exploits to our test accounts and verified that CSP chimed in and that non of the exploits work. We ran these tests in our iOS and Android apps and on the web client as the CSP is enabled for the apps via a meta tag while we use headers for the web app.

Result: No exploits found :heavy_check_mark:

Sanitizer

In order to test the sanitizing, we completely disabled the CSP, opened exploit mails and posted arbitrary HTML to the editors and verified that the sanitizer doesn't permit injections.

We also reviewed the code snippets where the sanitizer settings.

Result: With disabled CSP, it was possible to inject javascript via the users signature and by pasting to the editor component :heavy_exclamation_mark:
Fix: This has been fixed with #709 :heavy_check_mark:

Raw HTML DOM insertions

We manually reviewed code and verified that the sanitizer is used where user content is inserted into the dom. This mainly includes all usages of m.trust() and all usages of content editables (handled by squire).

Result: Custom logos and other custom page contents for whitelabel customers where not sanitized and therefore only protected by CSP :heavy_exclamation_mark:
Neither insertAdjacentHTML nor innerHTML or document.createElement("script") are used within the src folder.
Fix: This has been fixed with #710 :heavy_check_mark:

Email privacy tester

We ran the run email privacy tester on Chromium, Firefox, Safari and Edge.

Result: No problems found :heavy_check_mark:

Eval

We reviewed the source code for potential evaluations of insecure code (eval() or new Function()).

Result: No problems found :heavy_check_mark:

Exposure of sensitive data

We verified that all encryption primitives have been implemented and used correctly.

MAC enabled

We checked for the web client, the Android and iOS apps, that MACs are enabled for all data encryption operations and that manipulated MACs lead to the expected error messages (e.g. for a mail body).

Result: MACs are used as expected on all three platforms and error messages are displayed in case of manipulated MACs :heavy_check_mark:

Random IVs are used

We reviewed all code parts where IVs are generated and made sure, that the IVs for data encryption are random.

Result: We only use a static IV for key encryption and random ones in all other cases :heavy_check_mark:

Session key handling for encrypted mails

We tested, if a case exists where we send session keys for encrypted mails to the server. These tests where executed for the following cases:

1. Send encrypted mail to another Tutanota user including attachment
2. Store draft of unencrypted mail to none Tutanota user
3. Send encrypted mail to non-Tutanota user including attachment
4. Send unencrypted mail to non-Tutanota user including attachment

Result: The session keys were only sent to the server in case of unencrypted mails :heavy_check_mark:

PRNG and entropy

We tested that enough entropy is collected and used before generating keys on web client and in apps. We also verified that entropy is collected continously.

We made sure that JSBN (RSA implementation) and all native key generation implementations are seeded from our PRNG implementation.

Result: There is enough entropy and we collect entropy continously :heavy_check_mark:

Secure local data storage

We verified that the indexeddb database name has no connection to the users mail address and that all search words and suggestions on the index are encrypted.

We also examined different kinds of attacks on the local database.

In addition to that, we also checked how the push identifier and the local config are stored and if they contain sensitive data.

Result: If an attacker gains control on the indexeddb, statistical analysis based on word frequencies become possible due to the design of the SearchIndexMeta table :heavy_exclamation_mark:
Fix: This will be fixed with #711

Crypto compatibility tests

We checked that all compatibility tests for cryptographic primitives run successfully on all platforms.

Result: All tests evaluated successfully :heavy_check_mark:

Security misconfigurations

We identified security headers as the most important feature that needs to be configured correctly in order to increase security (especially CSP).

We also checked for auth data leaks and leaks to the browsing history.

Security headers

We reviewed CORS, HSTS, CSP headers manually (CORS, CSP) and ran the following tests:

• https://observatory.mozilla.org/analyze/mail.tutanota.com
• https://securityheaders.com/?q=mail.tutanota.com&followRedirects=on

Result: CSP could be more strict (frame-ancestors, default-src, base-uri, child-src) :heavy_exclamation_mark:
The Feature-Policy-Header is not yet used by Tutanota as it is currently only in a draft state and not yet widely supported.
Fix: The CSP headers have been improved with #713

Auth data leaks

We checked that passwords or other auth data are not stored in dom or browser cache and focused on the following areas:

• login
• registration
• password change
• add user
• add contactFormUser
• add externalUsers
• changing externalUserPassword
• TOTP codes

Result: We already removed the input text for passwords during login when switching to another view. However, we noted, that the dom input field was still accessible :heavy_exclamation_mark:
A password that could not be converted to a Uint8array could have been printed to the console since version 3.38.6 :heavy_exclamation_mark:
External login view & register view were not clearing password field when switching views :heavy_exclamation_mark:
An authVerifier was saved as Login._authVerifierAfterNextRequest for the current session but never used. This should be removed. :heavy_exclamation_mark:
TOTP secrets were cached for the current session when processing EntityEvents
Fix: This has been fixed with 953ea10 :heavy_check_mark:

Browsing history

We checked that no user data is stored in the browsing history.

Result: The mail address is part of the title and all urls are store in history as long as you are not using incognito mode :heavy_exclamation_mark:

Insecure deserialization

We created an overview diagram about all interfaces that the web and all native apps use and reviewed all data conversions on those component borders.

The client security highly depends on JSON.parse for deserialization as we use JSON nearly everywhere. We also use URL to parse mailto strings. For calling javascript from native code parts, we use evaluateJs on Android and evaluateJavascript on iOS (both invoked with a single base64 encoded string).

Result: All deserializations are fine :heavy_check_mark:

Components with known vulnerabilities

We verified that there are no security issues in used libraries (npm audit). We also checked that there are no unused libraries (depcheck) and that we are using up-to-date versions of the libraries (npm outdated).

In addition to that, we did an extensive code review for all runtime dependencies of Tutanota. We focused especially on eval and new Function() calls, rest requests using XMLHttpRequest and fetch, DOM manipulations using document.createElement, insertAdjacentHTML nor innerHTML.

These were our findings from reviewing all dependencies:

• Eval / new Function()
  
  
  
  • Bluebird uses eval to prevent v8 optimizations. Eval is never executed (see https://github.com/petkaantonov/bluebird/blob/49da1ac256c7ee0fb1e07679791399f24648b933/src/util.js#L216)
  
  
  • Bluebird uses new Function dynamic code generation, but this is disabled for browsers (if (!true))
  
  
• XMLHttpRequest and fetch
  
  
  
  • mithril uses XMLHttpRequest in m.request (not used by us)
  
  
• document.createElement, insertAdjacentHTML and innerHTML
  
  
  
  • mithril creates script elements for jsonp (not used by us)
  
  
  • mithril manipulates dom in functions createHTML and createElement (as expected)
  
  
  • mithril uses innerHTML for setting content editable contents and for inserting trusted html into the dom (as expected)
  
  
  • bluebird creates divs (document.createElement("div")) only when not used in the browser
  
  
  • dompurify uses document.createElement, insertAdjacentHTML for sanitization purposes
  
  
  • SystemJs uses document.createElement to load imports
  
  
  • core-js (polyfill) uses document.createElement to implement IE8 compatibility
  
  
  • squire uses document.createElement to insert content into the ContentEditable
  
  

Result: The package.json referenced some libraries that were not up-to-date and unused. Security vulnerabilities were part of development time dependencies :heavy_exclamation_mark:
Fix: We updated dependencies, removed unused ones with #712 :heavy_check_mark:. One development time dependency with a vulnerability can't be replaced at the moment.