atticuscornett / AtmosWeather

Atmos Weather is a lightweight weather app for receiving alerts and forecasts in the US.
https://atticuscornett.github.io/AtmosWeather/
GNU General Public License v3.0
23 stars 4 forks source link

Question on permissions #23

Closed IzzySoft closed 4 months ago

IzzySoft commented 5 months ago

My scanner just reported:

! repo/io.atticusc.atmosweather_2010000.apk declares sensitive permission(s):
  android.permission.ACCESS_COARSE_LOCATION android.permission.ACCESS_FINE_LOCATION
  android.permission.ACCESS_BACKGROUND_LOCATION android.permission.READ_EXTERNAL_STORAGE*
! repo/io.atticusc.atmosweather_2010000.apk contains signature block blobs: 0x504b4453 (DEPENDENCY_INFO_BLOCK; GOOGLE)

While location permissions are clear (weather at the current location), what is read/write storage needed for? And checking the other permissions, what (exact) alarms does a weather app schedule?

As for DEPENDENCY_INFO_BLOCK, that's easily avoided:

android {
    dependenciesInfo {
        // Disables dependency metadata when building APKs.
        includeInApk = false
        // Disables dependency metadata when building Android App Bundles.
        includeInBundle = false
    }
}

For some background: that BLOB is supposed to be just a binary representation of your app's dependency tree. But as it's encrypted with a public key belonging to Google, only Google can read it – and nobody else can even verify what it really contains.

Thanks in advance!

atticuscornett commented 5 months ago

As for the read/write permissions, I don't know how that slipped in - perhaps it is a default configuration with CapacitorJS? I will investigate soon.

As for the scheduling exact alarms permission, Atmos Weather uses AlarmManager to run periodic calls to the NWS API for alerts. (See #16) It must use the exact alarm permission to ensure the alerts are received in a reasonable timeframe. (I have observed over 15 minutes late alerts when testing using inexact alarms.)

Thanks for letting me know about the DEPENDENCY_INFO_BLOCK issue. I will change the build config accordingly.

Thank you again for your interest in Atmos Weather - it will probably take me a few days to get around to fixing these issues, as I will be busy the next few days - but I will get on this ASAP!

IzzySoft commented 5 months ago

perhaps it is a default configuration with CapacitorJS?

That rings some bell – IIRC there was such thing with INTERNET as well (which in case of AW of course is needed anyway), so this could well be. I see a * appended to android.permission.READ_EXTERNAL_STORAGE*, which means that was granted implicitly because of the presence of WRITE. So if both are not needed, a viable approach could be to remove WRITE, see e.g. Removing Unwanted Manifest Permissions With tools:node.

Atmos Weather uses AlarmManager to run periodic calls to the NWS API for alerts.

Thanks! Added that. Though it's not marked a "sensitive permission", good to make that transparent for those wondering/suspicious and thus build trust.

Thanks for letting me know about the DEPENDENCY_INFO_BLOCK issue. I will change the build config accordingly.

Thank you!

it will probably take me a few days to get around to fixing these issues, as I will be busy the next few days

Sure thing – and no worries. I understand it's nothing done at a coffee break. Thanks for taking care – and I hope my hints are helpful!

atticuscornett commented 4 months ago

Sorry for the delay - I have been out of town the past few days and only now have gotten home and had the time to look at this.

Upon looking through the commits, I cannot find when the android.permission.WRITE_EXTERNAL_STORAGE permission was added to the manifest - it predates the Android code even being in the repository (due to a misconfigured .gitignore over two years ago). Early in Atmos Weather development, I had not yet decided on a persistent storage solution for Android and may have been doing something strange with file storage very early in development. I somehow let the permission linger in the manifest unnoticed - I don't think anything is using it.

TL;DR - The WRITE_EXTERNAL_STORAGE permission can likely be removed from the manifest entirely - if not I will try using tools:node. Will update you after I have tested sufficiently.

DEPENDENCY_INFO_BLOCK has been addressed as of 1c09b5f.

As soon as everything is tested and good to go, I will release an update - it should hopefully be out in the next few days.

Additionally, this conversation has brought to my attention that (outside of IzzyOnDroid) there is no explanation for what permissions the app will need and why. Although a more transparent permission setup process has been in the plans for a while, I will also make an Android privacy statement accessible on the website or repo and in-app in case any other users have the same questions as you do.

Thank you for your suggestions and your patience!

IzzySoft commented 4 months ago

Thanks a lot, @atticuscornett! Looking forward to the next release then :star_struck: If you want me to fill in the other gaps then as well, please just give me a ping with a link to the privacy policy listing them and I'll copy them over:

image

I guess starting at boot is needed so the periodic calls can be rescheduled (IIRC, Android would delete the alarms on shutdown), ACCESS_NETWORK_STATE to check whether a network connection is available before trying to contact any server, INTERNET is clearly needed to obtain weather information and alerts – and POST_NOTIFICATIONS probably for a permanent notification in order to keep the app running? Just wonder there's no FOREGROUND_SERVICE needed… The DYNAMIC_RECEIVER_NOT_EXPORTED_PERMISSION is a bogus permission anyway (needed by AndroidX) and storage will be gone with the next release, so these two can be safely ignored concerning documentation.

atticuscornett commented 4 months ago

I have been testing the new version for the past couple of days and it seems that removing the WRITE_EXTERNAL_STORAGE permission has not caused any issues. I have just released Atmos Weather v2.1.1 (https://github.com/atticuscornett/AtmosWeather/releases/tag/v2.1.1) which should address all your original concerns.

I have also updated the app's privacy statement to include information about the Android permissions used by the app. The privacy statement is viewable at https://atticuscornett.github.io/AtmosWeather/privacy.html and should hopefully answer any further questions you or anyone else has about permissions.

If I missed anything or you have any questions, let me know! Thanks!

IzzySoft commented 4 months ago

Thanks! Filled the gaps in the package details at IzzyOnDroid from that. Only one missing is ACCESS_NETWORK_STATE which is rather obvious I guess (making sure you can reach out instead of try-and-fail).

IzzySoft commented 4 months ago

PS: Great privacy statement with good transparency, while still being compact enough to not be skipped for "TL;DR" :stuck_out_tongue_winking_eye: – thanks a lot!

atticuscornett commented 4 months ago

Only one missing is ACCESS_NETWORK_STATE which is rather obvious I guess (making sure you can reach out instead of try-and-fail).

ACCESS_NETWORK_STATE is under the "Network Permissions" header in the "Android Permissions" section of the privacy statement.

"ACCESS_NETWORK_STATE is used to check if the device is connected to the internet and optimize background usage based on whether the device is using mobile data or Wi-Fi."

So yes, it is used to check whether the device is connected before making calls to the NWS API. It is also used to optimize when and how to get the user's location for battery usage. If the user is on WiFi, location is checked less often and with lower accuracy since they are not moving.

PS: Great privacy statement with good transparency, while still being compact enough to not be skipped for "TL;DR" 😜 – thanks a lot!

Glad you liked it! Thanks!

IzzySoft commented 4 months ago

ACCESS_NETWORK_STATE is under the "Network Permissions" header

Gna, I missed it :see_no_evil: Thanks! Added.