SkyTubeTeam / SkyTube

Copylefted libre / open source YouTube player for Android
GNU General Public License v3.0
2.33k stars 322 forks source link

Question on permissions #1256

Closed IzzySoft closed 2 months ago

IzzySoft commented 5 months ago

On the latest release, my scanner just reported:

! repo/free.rm.skytube.extra_48.apk declares sensitive permission(s):
  android.permission.REQUEST_INSTALL_PACKAGES android.permission.READ_EXTERNAL_STORAGE*
! repo/free.rm.skytube.extra_48.apk contains signature block blobs: 0x504b4453 (DEPENDENCY_INFO_BLOCK; GOOGLE)

READ_EXTERNAL_STORAGE was implicitly granted because of WRITE_EXTERNAL_STORAGE. But what is REQUEST_INSTALL_PACKAGES used for here? The DEPENDENCY_INFO_BLOCK can easily be dealt with:

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!

drogga commented 5 months ago

@IzzySoft Regarding the REQUEST_INSTALL_PACKAGES: SkyTube has a check for updates, but only Extra can DL and install them, the Oss only shows that an update is available in the form of only a white changelog pop-up dialog (with an unclickable link at the bottom), hence the permission.

IzzySoft commented 5 months ago

@drogga thanks, I was afraid that would be the case. Is that updater (including the checks) opt-in or enabled by default? Does it clearly state where the updates are taken from, and that they'd bypass the additional scanning at the IoD repo?

drogga commented 5 months ago

Unfortunately it's enabled by default and you can't stop it from checking (you can also trigger the check from the About menu screen), but you can always choose to not DL (2 buttons are available in the prompt: Later & Update, but if you choose "Later", it will nag/ask the same on every launch), I'm pretty sure they are directly DLed from the release assets here, on GH.

IzzySoft commented 5 months ago

That still violates the IzzyOnDroid App Inclusion Criteria. Here's the relevant part:

[the app] must not download additional executable binary files (e.g. addons, auto-updates, etc.) without explicit user consent. Consent means it needs to be opt-in (it must not be harder to decline than to accept or presented in a way users are likely to press accept without reading) and structured in a way that clearly explains to users that they’re choosing to bypass the checks performed in this repo if they activate it.

Can that be adjusted? For an example, please see here (RiMusic implemented that exemplary).

drogga commented 5 months ago

It probably can, as long there's someone that can code... I'm not one of those and @gzsombor is hard to contact to and rarely responds to my mentions here, let's hope he sees this and handle it if he can and want's.

IzzySoft commented 5 months ago

Thanks! I've to raise the red flag here and set a reminder on my end to check up here. I'd have to remove it now as it violates the criteria, but will make the compromise to delay that for a while given the update itself has to be confirmed, but that's deep gray area in this context already as the implications are not made clear. People installing apps from a repo expect updates to come from the very same, with the very same extra precautionary scans and all.

drogga commented 5 months ago

F-Droid lists the Oss variants of the 2 repos (this and the Legacy), IDK if you intend to do the same and if they are problematic for your repo as well, or only the Extra(s).

IzzySoft commented 5 months ago

I just left Extra in as it adds some more features. And I usually do not add apps at IoD if they're already available (and updated) at F-Droid. Exceptions are possible of course; but all those using Extra will be extra disappointed.

So let's see what @gzsombor says. Or maybe one of the other contributors can help out, there are several which are quite active (maybe @comradekingu?)

gzsombor commented 5 months ago

@drogga thanks, I was afraid that would be the case. Is that updater (including the checks) opt-in or enabled by default? Does it clearly state where the updates are taken from, and that they'd bypass the additional scanning at the IoD repo?

It depends on how did you compile your app, if FLAVOR=="oss" or BUILD_TYPE=="snapshot" then it won't contact github (configured in SKYTUBE_UPDATES_URL) to check for the latest release - in other build configuration, it does it. If github returns a different version number as the latest release, the app will show a dialog about the upgrade, and the user can click on it to download it - and Android can install it.

IzzySoft commented 5 months ago

Thanks @gzsombor – but I don't compile (I don't even have an environment set up for that). The updater of the IzzyOnDroid repo fetches the APKs you provide, signed by you, from the releases here. Would it be possible providing a build of extra with one of the matching flavors?

gzsombor commented 5 months ago

Technically it is possible to create a new flavor, and release that one too everytime, but the current release process is very manual, so we need to improve that, before we could add another task to do it.

IzzySoft commented 4 months ago

Thanks for considering! Is there an (approximate) ETA when that could happen?

IzzySoft commented 3 months ago

Any news here?

drogga commented 3 months ago

The dude is nowhere to be found (MIA), I'm waiting for like 2 month for my PRs to be merged, he's here like every ~6 months or so for a day and that's it, no responses on tagging or even twitter/x.

Also this is a super low priority RN, because we have to watch in 360p now thanks to G./YT and the Dash is not an option in most cases (slow bandwidth speed - not client's fault and doesn't work on everything). The end of ad-less and free YT is near.
IzzySoft commented 2 months ago

Well, someone showed up and made some commits yesterday. Please note that the self-updater violates the inclusion criteria at IzzyOnDroid as well as at F-Droid (the only difference is that F-Droid does not actively check for such things and thus probably did not yet notice). So we cannot keep this state forever. If the updater is not removed or properly made opt-in ("properly" meaning with full information of the implications), we'll have to remove the apps from both repositories.

Also this is a super low priority RN

It's not the same low prio here – as when inclusion criteria are not enforced, they are useless.

drogga commented 2 months ago

"someone" :D

IzzySoft commented 2 months ago

(imitates a known female singer) :notes: "Somebody, huh! – somebody, huh! Some-body-who-looooves me…" :notes: :speak_no_evil: :dash:

IzzySoft commented 2 months ago

Ok, some "dude" made another update and the problematic permission is still there. It has been almost 3 months now since I've reported it, and I gave headroom to address the issue.

It was not addressed.

The updater is still there. So I'm sorry, but the app will be removed from IzzyOnDroid now, effective with the next sync around 6 pm UTC. It will of course be welcome back once this issue has been solved, so give me a ping then should that happen:

mnalis commented 2 months ago

Well, someone showed up and made some commits yesterday. Please note that the self-updater violates the inclusion criteria at IzzyOnDroid as well as at F-Droid (the only difference is that F-Droid does not actively check for such things and thus probably did not yet notice).

@IzzySoft Isn't the another difference that F-Droid rebuilds the package by themselves (hopefully with mentioned FLAVOR=="oss" ?), which should thus avoid including that GitHub auto-update code (if I understand https://github.com/SkyTubeTeam/SkyTube/issues/1256#issuecomment-2087313164 correctly?)

IzzySoft commented 2 months ago

@mnalis see above: yes, F-Droid serves the FOSS flavor (as you can see by the packageName there), and builds it themselves. So hopefully their build is not affected – or they just didn't notice it (they have no checks in place for that). Oof, the latter is the case – so be prepared for the app to be taken down there as well once they find out:

image

See: the permission is even explicitly listed there.

shuvashish76 commented 2 months ago

F-Droid serves the FOSS flavor (as you can see by the packageName there), and builds it themselves. So hopefully their build is not affected – or they just didn't notice it (they have no checks in place for that)

Installed slightly older version for testing. From my testing, for IzzyOnDroid update checker is enabled by default but it asks for downloader option. But for F-Droid : it doesn't notify for updates but still connects to api.github.com to show changes in latest version. I guess for FOSS flavor the "request install packages" permission is there for no reason at all.


For IzzyOnDroid:

Checking for updates without your knowledge or permission

The software must not download additional executable binary files (e.g. addons, auto-updates, etc.) without explicit user consent. Consent means it needs to be opt-in (it must not be harder to decline than to accept or presented in a way users are likely to press accept without reading) and structured in a way that clearly explains to users that they’re choosing to bypass F-Droid’s checks if they activate it.

image

Conclusion : IMO it needs Tracking AF only.


For F-Droid:

shuvashish76 commented 2 months ago

Request to developers : "New version available" dialog shown (IzzyOnDroid) everytime I restart the app even if I click "Later" which is annoying. Please add a "Never show this again" checkbox to this dialog.

drogga commented 2 months ago

I guess I can move the install packages perm. to only the Extra's manifest if you think it's pointless to be in the main Oss one (as it's not actually being in use), even tho I would actually like the Oss (+ Legacy) to be able to check and offer to DL an update as well, will this actually change/improve anything - IDK, but I certainly would like to be able to turn off the update checks and if enabled/on to definitely not nag and annoy.

Guys, spread the word that we need some help with the coding here if you want, because I can only do some basic things (out of necessity) and that's it, but someone will needs to be granted the Member/Collaborator permission(s) to manage the repos, instead of only relying to 1 person being rarely active here.

gzsombor commented 2 months ago

Yes, the OSS version doesn't attempt to download the APK - decided here, and only calls github API to display the release notes at most once IMHO. The code is already complex, due to the various scenarios/flavors that it needs to support, so I'm not surprised that you can't easily find out, what's going on.
So yes, the REQUEST_INSTALL_PACKAGES permission is unnecessary for the OSS flavor, which is easy to fix.

shuvashish76 commented 2 months ago

Here is how NewPipe handle the update checker behaviour : https://github.com/TeamNewPipe/NewPipe/discussions/10785#discussioncomment-8950428

even tho I would actually like the Oss (+ Legacy) to be able to check and offer to DL an update as well

@drogga As I see the foss flavor is built and signed by F-Droid so DL feature is meaningless here as cross updates is not possible.

drogga commented 2 months ago

Yeah, but I'm not using the F-Droid build, but the GH one, so I would benefit from this. As we know, using signature rotation or whatever was the term, the FD build can also be made to use the same signature/certificate by gzsombor.

gzsombor commented 2 months ago

Here is how NewPipe handle the update checker behaviour : TeamNewPipe/NewPipe#10785 (comment)

even tho I would actually like the Oss (+ Legacy) to be able to check and offer to DL an update as well

@drogga As I see the foss flavor is built and signed by F-Droid so DL feature is meaningless here as cross updates is not possible.

Yes, just to grow the general confusion, there is an Oss variant built for every release to Github too - https://github.com/SkyTubeTeam/SkyTube/releases/download/v2.989/SkyTube-Oss-2.989.apk - @drogga refers to that

gzsombor commented 2 months ago

Yeah, but I'm not using the F-Droid build, but the GH one, so I would benefit from this. As we know, using signature rotation or whatever was the term, the FD build can also be made to use the same signature/certificate by gzsombor.

Honestly, the less build that uses my certificate, the better for me - I'm a bit nervous to sign too many variants for which I don't want to take any responsibility

shuvashish76 commented 2 months ago

but I'm not using the F-Droid build, but the GH one, so I would benefit from this.

Sorry I didn't notice that. The general idea is : android apps should be maintained/updated by dedicated app stores, whether F-Droid/IzzyOnDroid clients or GitHub updates through Obtainium etc. not by the app itself, so my argument still holds. But I got your point, thanks.

drogga commented 2 months ago

Throwing a few things here to avoid a potential confusion that might arise if someone else/new reads this ticket and decides to comment.

It might and often does takes weeks for an update to be available on F-Droid, which is critical in situations when an app is no longer working and an emergency fix should be pushed, so users using their builds are left waiting. For other apps or the IzzySoft's repo, which uses the builds from here (untouched) and doesn't compile/build them - they are made available way faster, so if you are impatient, better start using the builds from GitHub instead.

In/on old devices with Android below 4.4 I have to use the Legacy builds (Oss in my case from GH) and there I can't use a browser to DL the updates, because the vers. of those are too old and can no longer properly load GitHub to let me DL the assets (even if they can, then I have to reboot after anyway, because the latest available browsers like Chrome & Firefox there are so heavy, that the devices can barely handle them and lag and barely work after launching them, unless rebooted and no, exiting, clearing recents and force stopping/closing doesn't help), so I have to DL them from a modern device and browser and transfer them over, while the last time I've used the Extra (a few years ago), I could DL the update directly from it (at least with the help of IDM/1DM, which might still be able to DL a file from GH from/with a direct raw link), but IDK if that's still the case, if this still works, then I would definitely benefit from the Oss (from GH) being able to do so too and prompt me for / offer me an update once, when available, but only once, with the option to reCheck for an update manually from the preferences.
IzzySoft commented 2 months ago

using signature rotation

Key rotation is not supported by F-Droid, they broke that earlier this year (but it is supported by IzzyOnDroid where the proper patches were applied instead, which did not break this: we just had an app with key rotation a few days ago and thus can confirm both by now – "both" meaning that it works with IoD but not with the official fdroidserver software used by F-Droid).

Key rotation is not supported by most F-Droid clients either (currently only by Neo Store – and with its next release also by Droid-ify).

So yes, the REQUEST_INSTALL_PACKAGES permission is unnecessary for the OSS flavor, which is easy to fix.

I'd strongly recommend that for the OSS build which doesn't need it anyway, to avoid triggering the removal there as well.

gzsombor commented 2 months ago

The REQUEST_INSTALL_PACKAGES right was removed from the OSS build, and a new version is released. In my opinion, the currently provided 2 (or with SkyTube Legacy 4) variant is more than enough:

shuvashish76 commented 2 months ago

The REQUEST_INSTALL_PACKAGES right was removed from the OSS build.

ThankYou please disable the update checker and unnecessary connection to GitHub to display the release notes by default as well.