Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question on permissions #262

Closed
IzzySoft opened this issue Jan 30, 2024 · 7 comments
Closed

Question on permissions #262

IzzySoft opened this issue Jan 30, 2024 · 7 comments

Comments

@IzzySoft
Copy link

On a check for apps with sensitive permissions in my repository, I just found that SongTube requests

  • MANAGE_EXTERNAL_STORAGE
  • READ_EXTERNAL_STORAGE
  • REQUEST_INSTALL_PACKAGES

Could you please clarify what those are needed for? Does SongTube offer playing local media as well (which would explain the first two) – and what apps does it want to install? Thanks in advance!

@Artx-II
Copy link
Member

Artx-II commented Jan 31, 2024

Sure,

MANAGE_EXTERNAL_STORAGE is only and exclusively requested when editing a song's artwork or tags, since write permission is required to do this action.

READ_EXTERNAL_STORAGE is used to fetch all songs from the device.

REQUEST_INSTALL_PACKAGES is needed for the in-app updates.

If this clarifies everything feel free to close this issue, otherwise, just let me know.

@IzzySoft
Copy link
Author

Thanks @Artx-II – added the first two to the allow list. As for the updater: is it opt-in or opt-out? Is explained where the APK is downloaded from? Are the implications clearly presented (e.g. they might bypass the extra screening in my repo¹)?

¹ no offense meant, but none of us is perfect – mistakes happen to the best of us. Wouldn't be the first time my scanner alerted of something that "slipped through" unintentionally.

@Artx-II
Copy link
Member

Artx-II commented Feb 1, 2024

When a new update is detected the app opens up a dialog with information like new version number, changes, etc... and a button to download and install the latest APK. Everything is extracted from this repository latest release and it's optional, you can simply choose to ignore and close it.

As for an explanation from where the update information and APK comes from, is not clarified in the app, I guess I should do that.

Here is the file from where the check happens and the information is extracted:
https://github.com/SongTube/SongTube-App/blob/development/lib/internal/models/update/update_manger.dart

And here is the dialog that pops up when a new update is detected:
https://github.com/SongTube/SongTube-App/blob/development/lib/ui/components/app_update_dialog.dart

That's all, let me know if I missed something!

@IzzySoft
Copy link
Author

IzzySoft commented Feb 1, 2024

Hm, OK – that would not meet my repo's (or F-Droid's at that) inclusion criteria.

I guess I should do that.

If I might suggest for that:

  • have an option in settings whether the app should update itself, initialized with NULL. With this option, add the details (where the APK would be fetched from, plus the implications I mentioned above) in its description
  • on start of the app, if the option is NULL ask if it should be used or not (showing those details for an "informed consent"), then remember the choice
  • now the other part can stay as-is, but update check is only performed when enabled

Those having installed from file (e.g. from releases here) can/will then choose to have the app perform the updates, those using e.g. my F-Droid repo can choose not to if they prefer the additional screening over "getting the update a few hours earlier".

That makes the updater checker opt-in, and thus meet the inclusion criteria. Functionality stays available, everyone happy (hopefully).

How does that sound to you? No "ultimatum" here, just trying to find the best way to deal with it.

@Artx-II
Copy link
Member

Artx-II commented Feb 1, 2024

That sounds reasonable, actually, I also think that's how it should be.

I made a new issue at #263 to start working on it.

@Artx-II Artx-II closed this as completed Feb 1, 2024
@IzzySoft
Copy link
Author

IzzySoft commented Feb 1, 2024

Thanks! I'll add REQUEST_INSTALL_PACKAGES to the allow-list when that's implemented then.

@IzzySoft
Copy link
Author

OK, added to the allow list. One more thing that popped in today:

! repo/com.artxdev.songtube_7000.apk contains signature block blobs: 0x504b4453 (DEPENDENCY_INFO_BLOCK; GOOGLE)

Easy to avoid:

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants