cmeng-git / xmail

XryptoMail (email client for Android)
Apache License 2.0
15 stars 1 forks source link

Question on permissions #6

Closed IzzySoft closed 6 days ago

IzzySoft commented 1 month ago

First, the release build seems to be missing at the latest tag, there's only a debug APK available. Maybe you just forgot the other one?

Next, the scanners here yelled at me:

! repo/org.atalk.xryptomail_19101.apk declares sensitive permission(s):
  android.permission.REQUEST_INSTALL_PACKAGES android.permission.ACCESS_MEDIA_LOCATION
  android.permission.READ_CONTACTS android.permission.READ_EXTERNAL_STORAGE
  android.permission.READ_MEDIA_AUDIO android.permission.READ_MEDIA_VIDEO
  android.permission.READ_MEDIA_IMAGES android.permission.WRITE_CONTACTS
! 2024-07-20 19:42:13,831 WARNING: repo/org.atalk.xryptomail_19101.apk: debuggable or testOnly set in AndroidManifest.xml
! 2024-07-20 19:42:35,476 WARNING: "org.atalk.xryptomail_19101.apk" is signed by a key that is not allowed:
  5bb77141be2d21ffaf4a2896b1e4e5f0ef1f69d5ecec51cfb93f018f3bd8ae61

Well, the last two messages are caused by the debug APK that was picked as the release one was missing. But could you please clarify the permissions? Thanks in advance!

cmeng-git commented 1 month ago

First, the release build seems to be missing at the latest tag, there's only a debug APK available. Maybe you just forgot the other one? For the official xmail release, the user should refer to android playstore for installation. The debug version in the github is for xmail developer and testers.

Note: There will not be any release version in github; as any release version in this repository will have different signature from the one in playstore, and they are not compatible.

android.permission.REQUEST_INSTALL_PACKAGES This is only enabled and used for the debug apk release; to support debug apk auto update.

android.permission.WRITE_CONTACTS android.permission.READ_CONTACTS Required by app. These are same permission required, and get included as in K9.

android.permission.READ_EXTERNAL_STORAGE android.permission.ACCESS_MEDIA_LOCATION android.permission.READ_MEDIA_AUDIO android.permission.READ_MEDIA_VIDEO These permissions are based on android device API. You can refer to PermissionActivity.java file for detail.

IzzySoft commented 1 month ago

There will not be any release version in github

There was a release version before, or the app would not be in the IzzyOnDroid repo. The above scan was triggered by the updater there (first message), and integration failed (the other 2 messages) as the signing key did not match (debug APK was the only one left). Previous releases were not signed by Google, but by:

DN: CN=cmeng, OU=atalk.org, O=atalk.org, L=singapore, ST=singapore, C=65

The app was listed here since last August in case you wonder.

any release version in this repository will have different signature from the one in playstore, and they are not compatible.

That the signature here is not compatible with the one in PlayStore is fully acceptable. Many folks who use F-Droid and the IzzyOnDroid repo don't use PlayStore anyway (some even run Google-free devices with no GMS or PlayStore installed).

So would you consider providing the release build again? Debug builds/Debug keys are not accepted in the IzzyOnDroid repo, so otherwise I had to freeze or remove the app as it could no longer be updated.

Thanks for considering!

cmeng-git commented 1 month ago

I had placed the release apk in the github release directory. Please note this apk signature is different from the one in android playstore. They cannot be installed over each other.

IzzySoft commented 1 month ago

Thank you! The signature matches the one of the APK present here from the previous release, so it can be installed over that. I've re-enabled the daily checks for new releases of your app again, so it will show up with the next run later today (around 6 pm UTC).

For the permissions, I've now set up the following explanations, if you could please confirm them:

android.permission.READ_CONTACTS: required to pick contacts as recipients
android.permission.READ_EXTERNAL_STORAGE: required to read files for attachments
android.permission.READ_MEDIA_AUDIO: needed on newer Android versions to access audio files for attachments
android.permission.READ_MEDIA_VIDEO: needed on newer Android versions to access video files for attachments
android.permission.READ_MEDIA_IMAGES: needed on newer Android versions to access image files for attachments

Open questions remain towards android.permission.ACCESS_MEDIA_LOCATION and android.permission.WRITE_CONTACTS. What are those used for? For the latter I could imagine one can pick addresses from a mail to add them to existing contacts, or create new contacts from. But the location details from inside images? Guess that's needed if you want to attach an image without those metadata stripped, as without it Android would give you a "stripped" version of the photo? (The idea behind this is to make the use of (sensitive) permissions transparent to those thinking about installing the app. This was introduced to the IzzyOnDroid repo at the beginning of this year, and reported to me at updates – which is why it showed up for your app only now, with its first update after that)

cmeng-git commented 1 month ago

Some of the permissions are included due to last minute upgrade to support API-34; and was based on other app test result.

android.permission.ACCESS_MEDIA_LOCATION This permission was added after upgraded to support API-34; based on test outcome on my other two app i.e. aTalk and Hymnchtv. Believe this is not required for Cryptomail. But need further test to confirm this.

Also the below 3 permissions are also included based on test, which are required for aTalk and Hymnchtv to work properly on API-34 and Huawei HarmonyOS. May be these are also no required for XryptoMail, but I need to do detail test and source review to verify these.

android.permission.WRITE_CONTACTS This permission was inherited from earlier K9 fork in 2012; and it is commented "to mark a contact as contacted". I suspect this permission is not further required. However I need to do a detailed source review before removing it.

IzzySoft commented 1 month ago

Thanks for the details! So I'll leave the one remaining permission (ACCESS_MEDIA_LOCATION) open for now as reminder while patiently waiting for your test results. Might ask for an update whenever a new release still has it then – hope that's OK with you?

cmeng-git commented 1 month ago

My apology that v5.1.0 release is not working and crashes after enter account info.

Found that with the latest SDK build environment setup, both the shrinkResources and minifyEnabled must be set to false for release build; else xmail crashes in Moshi with cannot serialize abstract class MessagingControllerCommands$PendingDelete.

Just release version 5.1.1 which has resolved the issues and updated all the permissions use. You may want to remove v5.1.0 from your repository; use the latest 5.1.1 instead.

IzzySoft commented 1 month ago

Thanks for the notice, @cmeng-git! v5.1.1 will replace v5.1.0 with the next sync around 6 pm UTC. I've just triggered the update manually to make sure the replacement will happen.

Btw: would you consider to pick a badge to link to your app at IzzyOnDroid from your Readme, maybe next to the PlayStore badge?

IzzySoft commented 1 month ago

Oof, another problem now: the latest release introduces Crashlytics to the app – which especially with a mail program, is a no-go concerning privacy and also a proprietary component.

Could you please remove that again? Or provide a FOSS-flavor without it at least? Thanks in advance!

If you're looking for alternatives, please see a.o. the analytics section in my snippet here. For now I've added the Tracking and NonFreeDep anti-features to your app's listing at IzzyOnDroid in the hope you can solve the issue with the next release. As "personal/sensitive details" (as mails often have them) and "tracking" are a no-go combination violating the IzzyOnDroid App Inclusion Policy, I'd otherwise need to remove it from the repo again, which would be a pity:

if the app processes sensitive data (e.g. health data, passwords), is intended to improve security/privacy, has root permissions, or is targeted at children, it must have no ATS (Advertizing and Tracking Services) elements at all.

cmeng-git commented 1 month ago

Oof, another problem now: the latest release introduces Crashlytics to the app – which especially with a mail program, is a no-go concerning privacy and also a proprietary component.

This is not new and already included in the v5.0.3. Actually I do not want to spent too much effort on XryptoMail development, except update to meet Playstore new requirements. The reason why I am not in favour of release it into Fdroid.

At the moment, I do not have the capacity to support you for the change request.

cmeng-git commented 1 month ago

Just found that the specified statement in build.gradle below is not used in xryptomail. So will remove it in the next release.

implementation 'com.google.firebase:firebase-crashlytics-buildtools:3.0.2'

IzzySoft commented 6 days ago

Thanks again then! I just cross-checked with the latest release and would say we can close this issue as solved – everything seems to be addressed.