Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Handle unexpected security exception when determining network capabilities (DEV) #1986

Merged
merged 2 commits into from
Dec 28, 2020

Conversation

d4rken
Copy link
Member

@d4rken d4rken commented Dec 22, 2020

There are a few devices that seem to throw a SecurityException when we try to determine the network state.
We do have NETWORK_STATE permission, so it's not clear why we get the exception here.
Mostly likely user modifications made to the device, or incorrectly implemented ROMs.

In any case we can guard this and handle it gracefully instead of crashing.

The stack trace is

java.lang.SecurityException: 
  at android.os.Parcel.createExceptionOrNull (Parcel.java:2373)
  at android.os.Parcel.createException (Parcel.java:2357)
  at android.os.Parcel.readException (Parcel.java:2340)
  at android.os.Parcel.readException (Parcel.java:2282)
  at android.net.IConnectivityManager$Stub$Proxy.getNetworkCapabilities (IConnectivityManager.java:2456)
  at android.net.ConnectivityManager.getNetworkCapabilities (ConnectivityManager.java:1385)
  at de.rki.coronawarnapp.util.network.NetworkStateProvider.access$getCurrentState$p (NetworkStateProvider.kt:2)
  at de.rki.coronawarnapp.util.network.NetworkStateProvider$networkState$1.invokeSuspend (NetworkStateProvider.kt:5)
  at de.rki.coronawarnapp.util.network.NetworkStateProvider$networkState$1.invoke (NetworkStateProvider.kt:2)
  at kotlinx.coroutines.flow.CallbackFlowBuilder.collectTo (Builders.kt:5)
  at kotlinx.coroutines.flow.internal.ChannelFlow$collectToFun$1.invokeSuspend (ChannelFlow.kt:2)
  at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith (ContinuationImpl.kt:3)
  at kotlinx.coroutines.DispatchedTask.run (DispatchedTask.kt:15)
  at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely (CoroutineScheduler.kt:1)
  at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run (CoroutineScheduler.kt:13)
Caused by: android.os.RemoteException: 
  at android.app.AppOpsManager.checkPackage (AppOpsManager.java:7698)
  at com.android.server.ConnectivityService.getNetworkCapabilities (ConnectivityService.java:1668)
  at android.net.IConnectivityManager$Stub.onTransact (IConnectivityManager.java:978)
  at android.os.Binder.execTransactInternal (Binder.java:1154)
  at android.os.Binder.execTransact (Binder.java:1123)

@d4rken d4rken added the maintainers Tag pull requests created by maintainers label Dec 22, 2020
@d4rken d4rken added this to the 1.11.0 milestone Dec 22, 2020
@d4rken d4rken requested a review from a team December 22, 2020 17:13
@vaubaehn
Copy link
Contributor

Hi Matthias,
due to lack of available time I'm not up-to-date on code changes, so please forgive in advance if my following question doesn't fit here.
At what time or times in the code runtime the network state is queried? Does the exception occur when it's about to determine whether wifi is on or off to start key download/provide diagnosis keys? If the answer is yes, from my observations (Play Store reports) this might be a specific Android 6 issue, thus being API level dependent. Does this match with your observations?
(My CWA updates only once per day, the daily check, since CWA 1.7.1, despite being on wifi nearly 99% of time)

In any case, I wish healthy, peaceful and relaxing Xmas days to all of you 🎄

Copy link
Contributor

@ralfgehrer ralfgehrer left a comment

Choose a reason for hiding this comment

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

LGTM.

Side Note only remotely related:
We also had some devices that have been on a non-metererd connection - nevertheless the check capabilities?.hasCapability(NET_CAPABILITY_NOT_METERED) was wrongly false in this provider.

@ralfgehrer ralfgehrer self-assigned this Dec 23, 2020
@d4rken
Copy link
Member Author

d4rken commented Dec 23, 2020

@vaubaehn

At what time or times in the code runtime the network state is queried?
Does the exception occur when it's about to determine whether wifi is on or off to start key download/provide diagnosis keys?
If the answer is yes, from my observations (Play Store reports) this might be a specific Android 6 issue, thus being API level dependent. Does this match with your observations?

val isMeteredConnection = networkStateProvider.networkState.first().isMeteredConnection

When the regular download task syncs the pkgs.
So yes it would match the symptoms you are describing, but it doesn't match my observations.
This specific stacktrace is something I only see on Android 11, and only a few reports.
But Google Play Error Reports are only a small picture usually, in my experience it catches less errors than actual in app error tracking.

(My CWA updates only once per day, the daily check, since CWA 1.7.1, despite being on wifi nearly 99% of time)

🤔 If you can have the device in WIFI for almost a day, you could take a bugreport:
https://developer.android.com/studio/debug/bug-report
and mail it to me (matthias.urhahn@sap.com) I'd take a look through the contained logcat and try to spot any CWA related exceptions.
Note that the bug report may contain sensitive information from other apps (e.g. received SMS).

In any case, I wish healthy, peaceful and relaxing Xmas days to all of you christmas_tree

❤️
Happy holidays 🎄

@d4rken
Copy link
Member Author

d4rken commented Dec 23, 2020

@vaubaehn Is your device a Samsung S6?

@sonarcloud
Copy link

sonarcloud bot commented Dec 28, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

62.5% 62.5% Coverage
0.0% 0.0% Duplication

@d4rken d4rken merged commit fa1bbc6 into release/1.11.x Dec 28, 2020
@d4rken d4rken deleted the fix/security-exception branch December 28, 2020 16:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
maintainers Tag pull requests created by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants