search

smswithoutborders / SMSWithoutBorders-App-Android

SWOB relies heavily on the security of the messages it transmits. We accomplish this by using tools like Android Apps which posses well tested tools for security
GNU General Public License v3.0
102 stars 5 forks source link

Question on permissions #101

Open IzzySoft opened 2 weeks ago

IzzySoft commented 2 weeks ago

The scanner here just reported:

! repo/com.afkanerd.sw0b_55.apk declares sensitive permission(s): android.permission.CAMERA android.permission.READ_PHONE_STATE

Could you please clarify what those permissions are needed for? Thanks in advance!

IzzySoft commented 2 weeks ago

Oof. Just catching up with the reports (I was AFK for 10 days), and there's another one. Looks like you've added non-free dependencies:

Are those really needed? Any chance to remove them again, or provide a FOSS flavor without them?

sherlockwisdom commented 2 weeks ago

Yes I believe I can remove those, should push an update in a few hours - should replace with https://github.com/altcha-org/altcha

sherlockwisdom commented 2 weeks ago

@IzzySoft

The Camera permissions are the scanning QR codes to add Gateway clients into the app. The implementation was broken and I didn't go ahead with implementing the QR codes but it has been available in earlier versions of the app.

The READ_PHONE_STATE permission is needed to get the mobile network code for the user to recommend a Gateway client nearest to them and default the nearest (cheapest) Gateway client in the app. This can be seen in the screen showing the available Gateway clients

IzzySoft commented 2 weeks ago

Thanks for both! Added the two perms to the app's green list now (with explanation) and look forward to the two non-free libs to be gone then.

IzzySoft commented 2 weeks ago

PS: Looks like I forgot to ask for the storage permissions?

image

sherlockwisdom commented 1 week ago

@IzzySoft

Looking into the Merged manifest, I believe the READ_EXTERNAL_STORAGE permission is implied from its dependency on the Whisper's System Curve25519.

Here's a copy of the logs for the merge manifest file and a snippet of the section detailing the IMPLIED manifest.

uses-permission#android.permission.WRITE_EXTERNAL_STORAGE IMPLIED from /Users/M1PRO/StudioProjects/SMSWithoutBorders-App-Android/app/src/main/AndroidManifest.xml:2:1-127:12 reason: org.whispersystems.curve25519 has a targetSdkVersion < 4 uses-permission#android.permission.READ_EXTERNAL_STORAGE IMPLIED from /Users/M1PRO/StudioProjects/SMSWithoutBorders-App-Android/app/src/main/AndroidManifest.xml:2:1-127:12 reason: org.whispersystems.curve25519 requested WRITE_EXTERNAL_STORAGE

manifest-merger-release-report.txt

IzzySoft commented 1 week ago

Now that's funny. I've searched their repo and found no mention of WRITE_EXTERNAL_STORAGE. Hm, but yeah, targetSdk=4 implies that, so no mention needed. Strange they use such a low TargetSDK. But as that repo is archived, there's no chance for a change.

What do you want me to put as reason? Would it be OK to write "required by the curve25519-java library the app uses"?

Scanning my library definitions for a possible alternative I only found ECC-25519 – which is archived as well and no longer maintained. Strange, there should be some up-to-date ones? OK, 49 matches here in case you're looking for a replacement that's not archived (but hopefully still maintained). No pressure, just a hint to accept or not :wink:

Btw, another way to get rid of those permissions (your app doesn't target such a low version after all) would be to Removing Unwanted Manifest Permissions With tools:node – but that would need testing to ensure that library does not "feel offended"…