Code Monkey home page Code Monkey logo

cwa-app-android's Introduction

Corona Warn App - Android

Documentation • Contribute • Support • Changelog • Licensing

The goal of this project is to develop the official Corona-Warn-App for Germany based on the exposure notification API from Apple and Google. The apps (for both iOS and Android) use Bluetooth technology to exchange anonymous encrypted data with other mobile phones (on which the app is also installed) in the vicinity of an app user's phone. The data is stored locally on each user's device, preventing authorities or other parties from accessing or controlling the data. This repository contains the native Android implementation of the Corona-Warn-App. Visit our FAQ page for more information and common issues

Known Issues

  • Activating exposure logging on a test version of the Android app requires a Google account which has been allowlisted for this purpose by Google (see Exposure Notifications implementation guide - Prerequisites). Google only allowlists official development partners: general CWA community members are not included. If you try to activate exposure logging and your account is not allowlisted then you will see the error message "Cause 3". Other functionality in the app which does not rely on exposure logging can however still be tested.

  • The Storage is currently based on Encrypted Shared Preferences and SQL Cipher (SQLite) - this could change in the future

  • Test Coverage is generally low and needs to be improved. We appreciate your help here!

  • In General every TODO comment within the code or the documentation can be regarded as an issue. You are free to tackle the TODOs anytime!

  • We are aware of the Play Store Limitations of GMS.

  • Without your own server instance (e.g. a local Docker setup), you will not be able to test the Application. For help, please refer to the server implementation.

  • Strings including translations are provided and approved by the UA team. For suggestions / findings please open a dedicated issue.

Architecture & Documentation

The full documentation for the Corona-Warn-App is in the cwa-documentation repository. The documentation repository contains technical documents, architecture information, UI/UX specifications, and whitepapers related to this implementation.

Support & Feedback

The following channels are available for discussions, feedback, and support requests:

Type Channel
General Discussion
Feature Requests
Concept Feedback
Android App Issue
Backend Issue
Other Requests

How to Contribute

Contribution and feedback are encouraged and always welcome. For more information about how to contribute, the project structure, as well as additional contribution information, see our Contribution Guidelines. By participating in this project, you agree to abide by its Code of Conduct at all times.

Contributors

The German government has asked SAP and Deutsche Telekom to develop the Corona-Warn-App for Germany as open source software. Deutsche Telekom is providing the network and mobile technology and will operate and run the backend for the app in a safe, scalable and stable manner. SAP is responsible for the app development, its framework and the underlying platform. Therefore, development teams of SAP and Deutsche Telekom are contributing to this project. At the same time our commitment to open source means that we are enabling -in fact encouraging- all interested parties to contribute and become part of its developer community.

Repositories

A list of all public repositories from the Corona-Warn-App can be found here.

Licensing

Copyright (c) 2020-2023 SAP SE or an SAP affiliate company.

Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License.

You may obtain a copy of the License from here.

Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the LICENSE for the specific language governing permissions and limitations under the License.

Please see the detailed licensing information via the REUSE Tool for more details.

cwa-app-android's People

Contributors

alexanderalferov avatar androidmedagalaxy avatar axelherbstreith avatar bmitr avatar chiljamgossow avatar chris-cwa avatar corona-warn-app-technical-user avatar cv113 avatar d4rken avatar fabian-k avatar fynngodau avatar harambasicluka avatar htooisap avatar janetback avatar jurajkusnier avatar kaddasz avatar kathrinluecke avatar kolyaopahle avatar lukaslechnerdev avatar marcauberer avatar marcmuschko avatar mtwalli avatar natalialemmerth avatar oemerb avatar oliver-zimmerman avatar pwoessner avatar ralfgehrer avatar samuraikek avatar schauersbergern avatar service-tip-git avatar

Stargazers

avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar

Watchers

avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar avatar

Forkers

tobyeb everton137 ironjan rugy jkoendev murmuras-tech rickordd madphone floriangrundig xdevs23 jedsada-gh rteusner twobiers nathanmaxsauter aroaik maximilian-beck99 cmongin mrdyro davedacoda joschahenningsen berkacikel45 sin96 strubbi77 rockystevejobs odonzyk kulikkrisz snipem balrog35 alacambra mihango jeehut dizzy-fork sandorzilizi krishnasony teboly frischifrisch stefanschoof ka7 thomasderzweifler 0xflotus zhan2046 asdlei99 mseele teleriddler abhisheksinha1506 afsalkp007 rrdlpl ouissalb jameshnsears david-oh-git coralibre sonic-india brainsucker92 activesince93 emre-one nf17 ignacioeloyola arman9189 ipv64 mlenkeit indianadschones alifesoftware daniel7sd jomapp mistrydarshan99 ni-wi schaa0 davidhdj asmuelle olivertauth headbrain dancuprum the-invincible stefan-thor johbrose cosee nk111 pocmo mzdn tobiasdroste tkowark aussekalega the-boogeyman311 grap3-fru1t whakapapa athanasiosmitrentsis lostmekka mahoe kbobrowski sebastianwolf-sap friedger richtertobias realvorl ciremaina manne05 stm-dev kinght200 pr1metine fewi999 lucs7

cwa-app-android's Issues

[BSI][0.5.4-alpha] Hardcoded TAN

Rating: Medium

Description:
The app implements a hardcoded TAN header in each request. This means that all users can have this TAN in their requests. This violates the general purpose of TAN, which is used for user authentication or authorization.

The impact of this vulnerability depends on what this TAN can allow users to access.

Proof of Concept:

KeySubmissionRequest.getHeaders()
    ......
        headers["cwa-authorization"] = "TAN 123456"
        this.addMarker("headers:$headers")
        return headers
    }

[BSI][20200603] Provisioning URL-Scheme Abuse for MitM Attack

Rating: Medium

Description:

The app implements its own URI scheme coronawarnapp:// which accepts several parameters consisting of the different backends with which the app communicpates.
An attacker could abuse the URI scheme to create malicious links with their own domains functioning as proxy between the app and the actual backend. This would allow an attacker to receive all communication between the app and the backend.
This attack vector either requires social engineering to convince the target of clicking the link or another vulnerability or malicious app on the phone which can trigger the URL. As long as there is no name check for the provided domains or certificate pinning implemented, this functionality should not be included in the release version.

Proof of Concept:

An attack can be as simple as getting the victim to click this link:

<html>
<body>
<a href="coronawarnapp://launch?SUBMISSION_CDN_URL=https://malicious.scdn.proxy&DOWNLOAD_CDN_URL=https://malicious.dcdn.proxy&VERIFICATION_CDN_URL=https://malicious.vcdn.proxy">Click Me</a>
</body>
</html>

Alternatively, a malicious app can invoke the request itself, via webview or via an installed browser, given the called app is aware of the custom URI scheme. This is basically only limited by the creativity of an attacker.

Backend URLs should not be changeable by intents.

Describe the bug

DynamicURLs object is used to store the backend URLs. All urls are defined as var so they can be changed any time. This is a security issue. Currently they are only changed in LauncherActivity depending on the URL schema that is used to launch the app. So every app could start the "Corona-Warn-App" with any backend urls.
I assume that it was introduced to make testing possible. But the risk forget to remove it later for the production version is to high.

Expected behaviour

Define the backend URLs in DynamicURLs as "val" read only variables.

Steps to reproduce the issue

Look at the code in LauncherActivity.kt and DynamicURLs.kt files.

Possible Fix

Define the backend URLs in DynamicURLs as "val" read only variables.
(Maybe you could also introduce flavors which allow URL changing for testing proposes but in my opinion it would be a security nightmare when any other app could change the backend URLs of this app.)

Potentially confusing prompts when enabling Exposure Notifications

Current Implementation

User may face unexpected / confusing questions about Location permissions when turning on Exposure Notification. Just a heads up - I was not able to test it with Corona Warn App yet but the same happens on both Latvian and Italian apps which are Exposure Notification-based, it seems to be triggered by Google Services and not by country-specific apps.

en_permissions

Suggested Enhancement

User may get a heads-up from Corona Warn App that he has to agree to turn on Location service, and can disagree with "Improving location accuracy". Not sure why the last prompt appears, perhaps it will be patched by Google since it is not necessary for the app to work properly. I was able to reproduce 3 prompts on Android 6 but on Android 10 only first 2 prompts appear.

French app gives following heads-up to the user (it has to get standard "Location" access though, but from user perspective it does not differ much from prompts above):

Important message: Attention, Android will ask you for access to this phone's location. StopCovid will only use Bluetooth detection of nearby phones, and never location data

Expected Benefits

May be possible to increase number of users agreeing to first two prompts (which is necessary for the app to work), and assure users that sharing location data with Google is not necessary and they can safely disagree without decreasing app functionality.

Internal Tracking ID: EXPOSUREAPP-2890

Links are not clickable

Describe the bug

The URLs in Privacy (Datenschutzerklärung) and Legal (Rechtliche Hinweise) are shown as links (colored and underlined) but not clickable.

The eMail-address in Imprint (Impressum) is neither highlighted nor clickable.

Expected behaviour

Highlighted URLs should be clickable and open a webbrowser.

The eMail-address should be clickable.

Steps to reproduce the issue

Open e.g. Legal (Rechtliche Hinweise) and try to click on one of the shown links. Nothing happens.

Technical details

Possible Fix

Additional context

Implement a proper Network Security Configuration

Current Implementation

Currently a Network Security Configuration is declared but it is empty.

Suggested Enhancement

Please implement a proper network security configuration for certificate pinning and setting up trusted Certificate Authorities.

Expected Benefits

Enhanced security and reduction of security vulnerabilities.

Create Notification Channel On App Start

Current Implementation

Currently the notification channel gets created, as soon as a notification is raised.

Suggested Enhancement

Create the notification channel on app startup.

Expected Benefits

If the notification channel is setup on app startup, then the user is able to see what types of notifications the app will post. In addition, the user can directly modify the settings based on personal preference. This would not be possible, if the notification channel was not created, because there was no notification yet. This could lead to incomprehensible bevhavior of the app in the eyes of the user.

We would be happy, if we could supply a PR 😁

Best Regards 🚀
Robert Jakobs & Tim Brüggenthies (@timbrueggenthies)

Text and Translation Contributions

Please see #332 for the new process about text and translation contributions

Dear contributers,

first of all, we are really happy to see the amazing support we are receiving from the community, thank you very much for that!
As we have encountered several issues and pull requests regarding translations and text improvements that we sadly had to reject, I am creating this pin thus adding it to the list of known issues as soon as possible.

Text and translation are not handled by the development team but by a dedicated stream within the project. Once they are finalized, they will replace them alltogether which should solve all related issues.
Therefore, to reduce efforts for everyone, we cannot accept PRs regarding texts and translations at the moment. Once the final texts and translations are available I will close this issue.

Thank you for understanding!
Best regards,
Marc

minSdkVersion of cwa

Your Question

In your source minSdkVersion 23 (Marshmallow) is used.

The market share of Lollipop is 8.5 % and Marshmallow is 10.7%
Why don't you follow the recommendation from Google?

Show the number of stored keys in the local database

Feature description

The app receives keys from other smartphones via bluetooth and stores the keys for the last 14 days (IIRC) in the local database. Even though the app doesn't know if multiple keys in the local db correspond to one single person in real life, it would be nice to have this number as it could be used as a rough indicator if the app is working at all and give some transparency how well it is performing.

Also it could be helpful by providing some feedback to the user how well he is keeping distance to other people eg. by looking at the counter before and after going to the supermarket.

The counter could be put into a debug submenu to prevent confusing users with less technical knowledge.

Problem and motivation

As a technically interested person I would like to gain some insignts und transparency how the app is performing.

Is this something you're interested in working on

No

Enhance Logging With Timber

Current Implementation

Currently the app uses the native android logging framework. To do that, every class holds a reference to a dedicated logging tag, which is given the logger, in case it is used. Example:

cwa-app-android/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/receiver/ExposureStateUpdateReceiver.kt

Line 38 in 43412bd

private val TAG: String? = ExposureStateUpdateReceiver::class.simpleName

Suggested Enhancement

There is a leightweight library called Timber, which automatically sets the logging tags to the calling class. We suggest to change the implementation for all logging calls towards Timber.

Expected Benefits

Timber optimizes the developer experience inseveral cases, eg. it automatically makes sure that the tag is at most 23 characters long (more details here). We could alsoget rid of the tag in every class that is currently hardcoded.

We would be happy if we could supply you with the corresponding PR 👍

Best Regards
Robert Jakobs & Tim Brüggenthies (@timbrueggenthies)

Micro G

Your Question

Hi, I have successfully build the app and now I am wondering how to use the app with micro g(https://microg.org). Can i use the regular app with some workaround, or do I have to wait for the libre App?

  • Source File:
  • Line(s):
  • Question:

Replacing Volley based network stack with a OkHttp based one.

Current Implementation

The current network stack is based on Volley using a requestQueue. For every request a complex class is defined which declares how the request should be parsed and some other boilerplate code. As far as i analyzed the code none of this requests is used in a asynchronous way. The WebRequestBuilder object do synchronize the requests so they can be used in a serial programming style using coroutines. Btw. a little bit misleading to call the functions e.g.: WebRequestBuilder.asyncGetKeyFilesFromServer because it is not a async call it is a synchronized call. ;-)

Suggested Enhancement

Use OkHttp to do the network calls with a small layer of serialization helper (Kind of OkHttp rest class to make it easy describe requests in a simple function call) for ProtoBuf and json (Why do you mix JSON and ProtoBuf is this really necessary?).
I use this approach in many projects and the OkHttp helper class will only take about 100 loc. Maybe a little bit more supporting both JSON and ProtBuf and implementing a general error classification system. Than the WebRequestBuilder could be a class which defines a function for each call. We do not need a class for every request anymore.
Sample for this OkHttp helper class is here: https://gitlab.com/timod/compose-playground/-/blob/master/app/src/main/java/de/appsonair/network/RestJsonClient.kt

Expected Benefits

  • Reducing lines of code (Reducing boilerplate code for request definitions).
  • Reducing complexity (We do not need to do asnychrouse calls and than synchronize them with suspendCoroutine continuation any more)
  • The refactoring will not change the interface ob the WebRequestBuilder

Introduction of a Dependency Injection Framework

Current Implementation

Right now, The code uses object Singletons and static methods to access shared data and execute code e.g. [1], [2] and in many more files.

This would make testing so hard as it will make the experience for developers and contributors unpleasant as they would spend more time in writing the mock code rather than testing their actual work. e.g. [3]

Suggested Enhancement

Introduction of a DI framework that will make writing a testable, maintainable and extensible code achievable. The selection of which framework to use is up to the maintainers.

Expected Benefits

These are the points on top of my head, I might be for sure missing some.

  • Embracing constructor injected objects rather than the current approach of using object singletons, as using Singletons is considered as anti-pattern.
  • Reducing the memory footprint of the application since the DI framework will take care of scoping of the objects, right now everything is declared object and once accessed it will be kept in memory even though it may not be needed anymore after it has served its purpose.
  • Instrumentation tests would be easier to write since the created test DI Graph will be responsible to provide the dependencies across all the application layers, right now it is impossible to provide a mock network or a Database for example
  • Increasing the test coverage point in the README would be easier to achieve as objects will be able to get instantiated through ctor injection hence providing fakes/mocks would be easier.

[DISCUSSION] Solution without closed source dependencys

EDIT: tl;dr: We are working an an open implementation of the Google/Apple protocol (I call it PPCP, which may be wrong). You can find and contribute to our repository here: https://github.com/theScrabi/CoraLibre-android-sdk

Dear Corona-Warn-App developpers we have read your code and documentation. We like your effort and your open mind for the community. As there is a small but willing comunity of people who refuse to use closed source Google or Apple dependency and so can not use the Corona-Warn-App, I'd like to discuss the posibility of a Google Free FLOSS solution.

What we know so far is that it's currently hardly possible to create the corona app without the use of the closed source GMS. This comes along with the nearby android exposure notification API, which implements the PPCP protocol. From what we can see in the code so far, the contact points between the android exposure notification API and the Corona-Warn-App are few as for the most common calls to the API a wrapper class is used. That's great! Also there are only about 14 code files that include parts from the GMS library. We therefor think it might be feasible to create a version of the app that does not require GSM services.

As statet in this issue the current maintainer have no plans to implement an own exposure notificatoin API, but might be open to support an alternative if available.
What we understand so far is that the part that runs the actual BLE contact tracing as well as providing a highlevel API for exposure likelihood and key handling would need to be reimplemented. We had several thoughts about how this might be possible.

  • As its offtenly staded that PPCP is close to DP-3T, I question if it is possible to reuse some parts of the prestandard DP-3T android sdk.
  • Prestandard DP-3T used forground services. Would it be possible to implement a GMS free non root version that would always display a notificatino while running the advertising and receiving service? Now i know this is not a good idea as people tend to close this notificaion, but was it techinically feacable to replace GMS by such a sollution?
  • A solution that would work on phones with root access such as Lineage-OS or /e/ could maybe be to introduce a privileged extentsion like the one used by the fdroid app. This might make it possible to expose functions to the Corona-Warn-App that would normally only be accessible for higher privileged applications. Maybe this could be a replacement for GMS.
  • One other way was to compleatly break loose from Smartphones and focuse on microcontroller based tracing beacons. For example on ESP32 or the CCC Card10. This way not only users with Lineage-OS or rooted smartphones could profit from an alternative implementation but also people who don't use a smartphone at all.

If a community based solution is possible what would be the next steps to go?

Internal Tracking ID: EXPOSUREAPP-5775

Do not log in production

Current Implementation

The release build contains Log-statements of all levels.

Suggested Enhancement

A production build should not contain any Log-statements, see for example the Android Prepare for release guide.

Log statements can be removed using ProGuard rules.

Expected Benefits

  • Compliance to Androids suggestions for release builds
  • Security: logs can contain sensitive data which should never be logged
  • Behaves like the iOS application will do, see corona-warn-app/cwa-app-ios#22

Typo in Onboarding Screen

Describe the bug

When starting the app, the app opens the dialog for requesting the permission for sending and receiving the users covid-19 risk status. The description says : "Corona-Wran-App...", instead of "Corona-Warn-App".

Steps to reproduce the issue

  1. Start the app for the first time
  2. Continue till the permission dialog

Technical details

  • Host Machine OS (Windows/Linux/Mac): Windows 10 Emulator With Android 28

Possible Fix

We already have a fix ready for a PR. We sadly miss authorization to publish our branch. We would be happy if we could supply you with the PR 👍 (and possibly also other enhancements 😁 )

Best Regards 🚀
Robert Jakobs & Tim Brüggenthies (@timbrueggenthies)

Split "submission" activity from main activity

Current Implementation

The current implementation has 2 activities:

  • onboarding
  • main activity

The onboarding activity is only shown on first start of the app. On the main class this state is checked and then either the main or the onboarding activity is started. This is perfectly good design, as the onboarding activity is mostly not used, so all fragments, images, text don't need to be loaded.

Suggested Enhancement

The submission of a positive COVID-19 case should be implemented as a separate activity. The transition to this should not be to hard. The fragments (in package "submission") should be removed from main activity and moved to a new activity.

Expected Benefits

Like the onboarding, the submission code is not needed in 99% of all cases and should not be loaded when app starts. This is not necessarily an important thing with today's devices, but there are some other things to imagine:

  • The submission of a positive COVID-19 result is a security/privacy sensitive task and involves scanning a QR-code or entering the teleTAN. The data involved by this should be protected. Therefore screenshots should be disabled (which also involves the preview image shown if you close app with home button - see issue #9). Pull request #41 tries to disallow screenshots by enabling/disabling them on enable/disable of the fragment. IMHO, a more clear approach would be to just disable screenshots for the submission activity. Screenshotting and app snapshot images should be allowed for all other parts (like main app showing actual status and onboarding). This seems very important to me. The enable/disable screenshotting is a setting for the activity's window and can't be enabled/disabled easily without hacks per fragment.
  • The submission interface could be started without starting the app by directly triggering the activity by an URI handler: It may be possible to encode the QR code key as an URI (that's my other suggestion - should I open separate issue?). In that case users can just open the default mobile's camera app, scan the barcode and the camera app will trigger the URI encoded into it. The app can then provide an URI handler that automatically starts the submission activity in the CWA Android app. This is much more easier to understand to most people, because when they see a QR/barcode, they intuitively scan it with the camera app (recent phones have QR code reader embedded into camera app) or their favourite QR code scanning app.

Internal Tracking ID: EXPOSUREAPP-18

[BSI][0.5.4-alpha] No Pinning

Rating: Medium

Description:
Certificate or public key pinning is when an app communicates with entites with specific TLS Certificates/Public Key. Any other communication is denied by the app, even if the TLS Certificate is signed by a trusted CA. This provides extra protection against Man in the Middle (MitM) attacks.

Android app does not implement this secure measure, and this increases the possibility of a MitM attack.

[UI] QR-Code scan

Current Implementation

The qr-code-scan-view has multiple ui issues

  • Flash button has a bad visibility compared to the x button
  • Qr code rectangle is not fully grey
  • Touch target of the buttons on the top is too small. Flash button only has 20dp. Google suggests 48dp.

cwa-qr-code

Suggested Enhancement

  • Use white or black for the flash button
  • Fill the qr code rectangle fully
  • Increase the touch targets of the buttons to 48dp

Expected Benefits

  • Flash button has a greater visibility
  • The qr code is rectangular
  • Buttons can be touched more easily

Optimization Of Edits Of SharedPreferences

Current Implementation

Currently there are many places in the code where you can see something like this:

with(getSharedPreferenceInstance().edit()) {
    putString(
        CoronaWarnApplication.getAppContext()
            .getString(R.string.preference_m_auth_code),
        value
    )
    commit()
}

See here

Suggested Enhancement

We could enhance it easily to this code:

getSharedPreferenceInstance().edit { 
    putString(CoronaWarnApplication.getAppContext()
        .getString(R.string.preference_m_auth_code),
        value
    )
}

The needed library is already included into the project - we are only using already available functionality. More info on KTX here 👍

Expected Benefits

These enhancements could reduce the code size /nesting, make it more readable and less verbose.

If you think this is valid, we would be very happy to supply a PR for this 🥇

Best Regards 🚀
Robert Jakobs & Tim Brüggenthies (@timbrueggenthies)

Code Coverage Badge & Changes

Current Implementation

There's no code coverage badge for the main branch and also no way to see the difference on pull requests.

Suggested Enhancement

Add a coverage badge to the README and setup a service to automatically post coverage changes to PRs.

Expected Benefits

Long term improved code coverage and therefore also increased trust from our users (which is important for this app to become a success).

Exceptions From Workers Get Reported As Toasts

Current Implementation

Currently, the app opens a Toast, if any worker encounters an exception of any type. This can be an issue if the app is in the background, and a worker gets into an error state. Then the user gets a Toast message, possibly completely out of context due to currently being in another app. Example:

cwa-app-android/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/worker/DiagnosisKeyRetrievalPeriodicWorker.kt

Line 37 in 43412bd

e.report(ExceptionCategory.JOB)

In addition to that, a technical message is shown, as the Throwable localizedmessage is used for the text.

Suggested Enhancement

If the user really needs to have information about these kind of errors, the user should get a notification with a human readable error text. This could be implemented by making use of the enum class ExceptionCategory, which is already in place. In our eyes it would only make sense to notify the user, if the user can change the situation to the good.

Expected Benefits

The user does not get disturbed during the usage of the mobile phone, especially when working in other apps. Also the user could decide if the error needs attention from the users side.

Best Regards
Robert Jakobs & Tim Brüggenthies (@timbrueggenthies)

Exposing secret keys from Android KeyStore is not possible

de.rki.coronawarnapp.util.security.SecurityHelper:64

    /**
     * Retrieves the Master Key from the Android KeyStore to use in SQLCipher
     */
    fun getDBPassword() = keyStore
        .getKey(masterKeyAlias, null)
        .toString()
        .toCharArray()

This doesn't work because the content of the secret key is not exposed.

The current return value of the function is:

android.security.keystore.AndroidKeyStoreSecretKey@{hash}

[BSI][05_23_cwa-app-android-master] network_security_config.xml Allows Cleartext Communication

Rating: High

Description:

The network_security_config.xml defines cleartext communication exceptions for two backend domains. This configuration opens up the possibility of
man-in-the-middle attacks.

Proof of Concept:

As can be seen below, 'cleartextTrafficPermitted' is set to 'true' and two exceptions are defined via 'includeSubdomains="true"'.

network_security_config.xml:

distribution-mock-cwa-server.apps.p006.otc.mcs-paas.io submission-cwa-server.apps.p006.otc.mcs-paas.io

Setting Notification Priority to PRIORITY_MAX

Current Implementation

Currently, the notification priority for notifications from the corona-warn app is set to PRIORITY_HIGH.

Suggested Enhancement

We should set the notification priority to PRIORITY_MAX. See pull request here: #203

Expected Benefits

Setting the notification priority to the highest possible value will ensure that on all android devices notifications from the corona-warn app have the highest priority and will be recognized.

[BSI][05_23_cwa-app-android-master] Key File Export Stored on External Storage

Rating: High

Description:

The app implements a storage helper which stores data in the external/sdcard directory of the device. This directory is a shared directory, accessible to all apps on the device and requires no special permissions to access in general. According to developer comments in the code, the storage location is also referenced as 'internal' storage, even though the implementation contradicts the comments.

All apps on the device can access this location and therefore access and manipulate the data stored in there. Depending on the criticallity of the files stored in the directory, this implementation should be considered dangerous.

Proof of Concept:
android_file_export_to_external_storage.txt

[BSI][0.5.4-alpha] Unencrypted Sensitive Data in External Storage

Rating: High

Description:

The App saves the diagnosis keys it receives on the device's external storage. Android's external storage is not protected by sandboxing, therefore any other app can access this data. There is no sign of encryption done by the app before persisting the keys on its external storage.

It is recommended to save sentivie data in an encrypted form, preferrably in the app's local storage directory.

Proof of Concept:
in Class de.rki.coronawarnapp.receiver.DiagnosisKeyDownloadReceiver:

setupReceiver(openDownloadIDs: AtomicReference<MutableList<Long>>, cancellableContinuation: CancellableContinuation<List<File>>):
	val receiver = DiagnosisKeyDownloadReceiver(CoronaWarnApplication.getAppContext().getExternalFilesDir(null), openDownloadIDs, cancellableContinuation)
	......
	receiver.register()

	
Additionally in TestForApiFragment.enterOtherKeys():
	val dir = File(File(requireContext().getExternalFilesDir(null), "key-export"), token ?: "")
	......
	googleFileList = KeyFileHelper.asyncCreateExportFiles(appleFiles, dir)

Fix dark-mode colors

Current Implementation

The app has a dark-mode theme but the background color of the TAN input is not changed so the fields are not visible.
Same for the option selection: the background makes it hard to read the font

Suggested Enhancement

Adding tanInputBackground to the night/colors and set a correct color.

Use colorGreyLight for dispatcher_card_header.

Expected Benefits

User can see the TAN input fields in dark-mode and is able to read the text inside the option cards:

After changes:
app_cards2
83351983-aafe8080-a348-11ea-94e9-a4012887781b

[SECURITY] The application implemented usesCleartextTraffic , cleartextTrafficPermitted, is set to "true", which will allow HTTP Traffic

name: "usesCleartextTraffic , cleartextTrafficPermitted, is set to "true""
about: Source Code in Git Repo
labels: bug

Describe the bug

As per the android documentation "android:usesCleartextTraffic"

Indicates whether the app intends to use cleartext network traffic, such as cleartext HTTP. 
When the attribute is set to "false", platform components (for example, HTTP and FTP stacks, DownloadManager, and MediaPlayer) will refuse the app's requests to use cleartext traffic. 
Third-party libraries are strongly encouraged to honor this setting as well. 
The key reason for avoiding cleartext traffic is the lack of confidentiality, authenticity, and protections against tampering; a network attacker can eavesdrop on transmitted data and also modify it without being detected.

As the usesCleartextTraffic is set to true it violates the above principle.

 <application
        android:name="de.rki.coronawarnapp.ExposureNotificationAppApplication"
        android:allowBackup="false"
        android:icon="@drawable/ic_app_launch_icon"
        android:label="@string/app_name"
        android:networkSecurityConfig="@xml/network_security_config"
        android:roundIcon="@drawable/ic_app_launch_icon"
        android:supportsRtl="true"
        android:theme="@style/AppTheme"
        android:usesCleartextTraffic="true">

Similarly for cleartextTrafficPermitted , the value is set true.

<?xml version="1.0" encoding="utf-8"?>
<network-security-config>
    <domain-config cleartextTrafficPermitted="true">
        <domain includeSubdomains="true">distribution-mock-cwa-server.apps.p006.otc.mcs-paas.io
        </domain>
        <domain includeSubdomains="true">submission-cwa-server.apps.p006.otc.mcs-paas.io</domain>
    </domain-config>
</network-security-config>

Expected behaviour

Steps to reproduce the issue

Location of the code using usesCleartextTraffic
https://github.com/corona-warn-app/cwa-app-android/blob/master/Corona-Warn-App/src/main/AndroidManifest.xml

Location of the code using cleartextTrafficPermitted
https://github.com/corona-warn-app/cwa-app-android/blob/master/Corona-Warn-App/src/main/res/xml/network_security_config.xml

Technical details

  • Host Machine OS (Windows/Linux/Mac): Android

Possible Fix

It is advised to set the values to false , instead of true.
cleartextTrafficPermitted="false", cleartextTrafficPermitted="false"

An example from the android documentation

For example, an app may want to ensure that all connections to secure.example.com are always done over HTTPS to protect sensitive traffic from hostile networks.

<?xml version="1.0" encoding="utf-8"?>
<network-security-config>
    <domain-config cleartextTrafficPermitted="false">
        <domain includeSubdomains="true">secure.example.com</domain>
    </domain-config>
</network-security-config>

Additional context

[0.8.5] App crashing immediately (file is not a database)

Describe the bug

The current master 73c97f9 (Version 0.8.5) is crashing immediately after opening the app

Expected behaviour

The app should not crash and show a error message or sth. similar

Steps to reproduce the issue

Build and run the current master

Technical details

Log output:

 --------- beginning of crash
2020-06-07 15:47:30.655 23244-23244/de.rki.coronawarnapp E/AndroidRuntime: FATAL EXCEPTION: main
    Process: de.rki.coronawarnapp, PID: 23244
    net.sqlcipher.database.SQLiteException: file is not a database: , while compiling: select count(*) from sqlite_master;
[REMOVED]

Possible Fix

Additional context

[BSI][0.5.4-alpha] App Snapshots Enabled

Rating: Low

Description:
When an app is sent to the background, Android takes a snapshot of the latest view of the application and saves it on the file system. If this snapshot contains sensitive data, a malicious app can obtain such data from the sanpshot.

The snapshot is saved outside of the app's sandbox, therefore any malicious app can access it only if it has root privileges or can bypass sandboxing limitations.

Proof of concept - see picture below:
android_background_snapshot

[BSI][05_23_cwa-app-android-master] AndroidManifest.xml Allows Cleartext Communication

Rating: High

Description:

The AndroidManifest.xml defines cleartext communication to be allowed. This configuration opens up the possibility of
man-in-the-middle attacks.

Proof of Concept:

As can be seen below, 'android:usesCleartextTraffic' is set to 'true'.

AndroidManifest.xml:

[...]

<application
    android:name="de.rki.coronawarnapp.CoronaWarnApplication"
    [...]
    android:usesCleartextTraffic="true">
    
[...]

</application>

Use adaptive icon

Current Implementation

The app icon is always rectangular
cwa-current-icon

Suggested Enhancement

Use adaptive icons

cwa-adaptive-icon

Expected Benefits

The app icon is rendered acording to the users launcher settings. Fallback is a round icon.

App crashes on startup due to missing backend urls (leads to uncaught okhttp exception)

Describe the bug

The app built from 16f016a crashes on startup.

Expected behaviour

It should not crash and instead show some kind of error message.

Steps to reproduce the issue

Build and run the app from commit 16f016a

Technical details

n/A

Possible Fix

The reason for this crash are the missing backend URLs. OkHTTP is expecting valid URLs starting with http:// or https://. Since the update check is invoked on startup, the app crashes immediately.

However, the uncaught exception is the real reason for the crash.

Additional context

Excerpt from Logcat:
java.lang.ExceptionInInitializerError at de.rki.coronawarnapp.service.applicationconfiguration.ApplicationConfigurationService.asyncRetrieveApplicationConfiguration(ApplicationConfigurationService.kt:9) at de.rki.coronawarnapp.update.UpdateChecker.checkIfUpdatesNeededFromServer(UpdateChecker.kt:122) at de.rki.coronawarnapp.update.UpdateChecker.checkForUpdate(UpdateChecker.kt:35
[...]
Caused by: java.lang.IllegalArgumentException: Expected URL scheme 'http' or 'https' but no colon was found at okhttp3.HttpUrl$Builder.parse$okhttp(HttpUrl.kt:1260)

Replace Try/Catch with CoroutineExceptionHandler for Exposure Notification Exceptions.

Current Implementation

The current Exposure Notification Client uses Try Catches for reporting Exceptions which is a hassle to implement correctly and error-prone.

Suggested Enhancement

We should use a dedicated CoroutineExceptionHandler that is able to catch Errors from dedicated threads so that we have a smooth exit scenario in the error cases.

Expected Benefits

A better UX by providing consistent crash scenarios.

[BSI][05_23_cwa-app-android-master] Root Detection only Implemented in Launcher Activity

Rating: Medium

Description:

The Android root detection check is solely implemented in the LauncherActivity of the app. Therefore, detection is only possible at the start of the app and can also be completely circumvented by directly calling the MainActivity as root user. Depending on the intention of implementing root detection, this implementation might be sufficient to warn users about the risk but should not be considered to be enough to prevent app usage on rooted devices.

Proof of Concept:

The following code snippet is an excerpt from de.rki.coronawarnapp.ui.LauncherActivity which shows the location, in which the check is implemented.

[...]

class LauncherActivity : AppCompatActivity() {
companion object {
private val TAG: String? = LauncherActivity::class.simpleName
}

override fun onCreate(savedInstanceState: Bundle?) {
    super.onCreate(savedInstanceState)
    val rootBeer = RootBeer(applicationContext)
    // if root is enabled the user isn't allowed to use the app
    if (rootBeer.isRooted) {
        Log.i(TAG, "Device is rooted")
        startRootedActivity()
    } else {
        Log.i(TAG, "Device is very probably not rooted")
        if (LocalData.isOnboarded()) {
            startMainActivity()
        } else {
            startOnboardingActivity()
        }
    }
}

[...]

The check can therefore be circumvented by directly calling

am start -n de.rki.coronawarnapp/de.rki.coronawarnapp.ui.main.MainActivity

as a root user on the device (as the activity is not exported and as such requires highest permissions).

Use android:inputType=textVisiblePassword for tan input edittext

Feature description

Currently, android:inputType=text is used for the tan input edittext. This has the consequence that the user's soft keyboard shows suggestions and may save the TAN to a dictionary.

With android:inputType=textVisiblePassword, no suggestions are shown and the keyboard shouldn't save it in its dictionary.

Problem and motivation

The autocompletion of the softkeyboard may alter the TAN the user has typed in.

Is this something you're interested in working on

No

[UI] Elements missing from images based on theme

Describe the bug

The images on the screen where a user can choose how to enter the TAN (QR-Code, manually, request the TAN) are missing some details based on the theme color, see pictures below.

The back-arrow is also not visible on the black theme, but this might be a seperate issue...

There might be other views with similar error, but i noticed it on this screen.

Expected behaviour

The pictures should show identical content, regardless of the theme.

Steps to reproduce the issue

If your phone has the option to change the system color from light to dark, do it. Open the app in both themes and see the different behaviour.

dark (2)_LI
light (2)_LI

Technical details

  • Smartphone with Android 10
  • Build from dev-branch 8dd2299

Possible Fix

Additional context

Internal Tracking ID: EXPOSUREAPP-1968

Usage of sqlite

Please describe in great details why you decided to use sqlite.

According to the documentation and the expected data a simple json file can be used.

The CCC demonstrated in very simple steps, that SQLite can be used via SQL Statements to execute code.

Why add this level of possible vulnerability to the software - when there is really no need to use a database?

Please also describe in great details - why a specific version "2.0.x" was chosen by the gradle build files.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.