MyHush / SilentDragonAndroid

Android companion app for SilentDragon
https://www.myhush.org
GNU General Public License v3.0
5 stars 12 forks source link

Prevent cleartext traffic downgrade #71

Closed leto closed 3 years ago

leto commented 4 years ago

TOB-ZEC-005 from https://github.com/trailofbits/publications/blob/master/reviews/zecwallet.pdf

change the android: usesCleartextTraffic setting to false . This would
prevent any regressions to unencrypted web protocols.
leto commented 4 years ago

We use android sdk version 28 which defaults this to true so we are good:

android:usesCleartextTraffic
Indicates whether the app intends to use cleartext network traffic, such as cleartext HTTP. The default value for apps that target API level 27 or lower is "true". Apps that target API level 28 or higher default to "false".

as per https://developer.android.com/guide/topics/manifest/application-element

leto commented 4 years ago

This is actually set to true in app/src/main/AndroidManifest.xml

leto commented 4 years ago

My guess is that this is needed to allow SDA to pair to a local wallet on the local LAN without needing a valid SSL cert, which would be prevent average users from being able to pair their mobile app. Changing this setting to false needs to see how that affects pairing mobile apps locally. When the wormhole is used, that always uses TLS so that use case won't be affected by this.

jahway603 commented 3 years ago

I am unable to pair SDA with SDL on the same local network after changing the "android: usesCleartextTraffic setting to false" and have attached the following screen shot of the error.

d36b8774-5201-4507-8ff9-a161aec90c3a_2

jahway603 commented 3 years ago

Was curious if checking "Disallow routing over external service/internet" under Settings would change anything but same error message whether it is or is not checked.

leto commented 3 years ago

@jahway603 this is some weird bullshit error. We don't actually use HTTP but because of some deprecated nonsense in the codebase, it was required or hard to change. Everything keeps changing at the android layer

jahway603 commented 3 years ago

@leto Yeah, reading about this had to do with HTTP vs HTTPS. I was trying to determine which was being used for "wss://wormhole.myhush.org:443", but was not sure.

jahway603 commented 3 years ago

@leto Just stumbled on this [ https://developer.android.com/training/articles/security-config ] and am looking at an example of it now. This might be the solution to this :+1:

leto commented 3 years ago

@jahway603 ws is plain websockets, which is HTTP plus extra headers, where as wss is secure websockets, which is HTTPS with extra headers. I remember vaguely that I could not fix this issue, and also, it did not actually cause any privacy issues.

It might be something to do with the fact that we require the ability to talk to SSL certs which are not signed by a CA, and at least it used to be the fact, that we had to use the "sometimes http" policy to be able to use that.

If you want to support really old Android devices, the situation is more annoying. I will let you have fun with it :sweat_smile:

jahway603 commented 3 years ago

@leto Good to know the difference between wss and ws. Yeah, this is a funky issue and, after further looking into Android's Network Security Configuration, this config is only supported in Android 7+, so would only be a solution if we increased our minSdkVersion to that or higher. Additonally, this NCC Group article discusses how to bypass it. Since we are not using HTTP and only using HTTPS then I am closing this bug.

leto commented 3 years ago

@jahway603 agreed, thanks for verifying things. We never use HTTP and this is basically a false positive caused by automated software scanners and the fact that Android API is funky as hell