LagradOst / QuickNovel

Android app for downloading novels
MIT License
1.04k stars 62 forks source link

Question on permissions #223

Closed IzzySoft closed 1 month ago

IzzySoft commented 7 months ago

My scanner got a few new features in January, and now reported on today's release:

! repo/com.lagradost.quicknovel_54.apk declares sensitive permission(s):
 android.permission.READ_EXTERNAL_STORAGE android.permission.REQUEST_INSTALL_PACKAGES

While the storage one is easily explained, I wonder what apps a novel reader wants to install?

image

Oh, if you wonder about that DEPENDENCY_INFO_BLOCK, that's easily eliminated:

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.

LagradOst commented 7 months ago

Qn uses android.permission.REQUEST_INSTALL_PACKAGES because it has a built in updater within the app (and that is the only use), it may be removed but I dont trust android not fucking it up.

IzzySoft commented 7 months ago

Ummm… Is the self-updater opt-in? And does it give proper explanation of the implications? From the IzzyOnDroid App Inclusion Criteria:

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.

(basically the same rule as F-Droid.org has). So while ideally it is not present at all, you need not necessarily to remove it altogether, but having it opt-in with the proper explanations would suffice.

LagradOst commented 7 months ago

Ummm… Is the self-updater opt-in? And does it give proper explanation of the implications? From the IzzyOnDroid App Inclusion Criteria:

If search update check is enabled (can be disabled/enabled in settings) it will search the github releases every startup for an update, and if it finds one it will show the user a dialog with the following options: update, cancel, dont_show_again. So the update is optional but it searches by default. It will never update without user consent.

LagradOst commented 7 months ago

While you are already here, the same goes for cs3 (https://apt.izzysoft.de/fdroid/index/apk/com.lagradost.cloudstream3) and cs3 disabled acra by default

IzzySoft commented 7 months ago

If search update check is enabled (can be disabled/enabled in settings) it will search the github releases every startup for an update, and if it finds one it will show the user a dialog with the following options: update, cancel, dont_show_again. So the update is optional but it searches by default. It will never update without user consent.

Hm, the check should also be opt-in. Let me give you two example screenshots from an app that recently implemented this:

01_enable_update_check 02_update_action

The first popup occurs when no choice was made yet (e.g. right after the first start following the initial installation or update to the version that introduced this, ad by default the value for that setting is NULL). Both buttons use the very same design, so no "nudging". The second popup of course never appears if the check was disabled. It's still not ideal as it forgets to mention the implications (such updates would bypass the additional checks performed in the resp. F-Droid repo).

While you are already here

Thanks! I removed the Tracking AF right now (effective with the next sync around 7 pm UTC, as usual) and configured the scanner to not report it. As for "the same goes" – yes, then please carry over the same solution from here. It was not yet triggered for CS3 as most likely there was no update yet since those new features got added to my scanner. I didn't yet run a full scan on existing APKs as I still have more than enough to follow up by incoming updates :see_no_evil:

IzzySoft commented 7 months ago

The warning popped up again today, so let me show you the new "screenshot 1" I just received:

01_enable_update_check

IzzySoft commented 3 months ago

@LagradOst sorry to be a bother, but I just received the warning once more:

! repo/com.lagradost.quicknovel_55.apk declares sensitive permission(s): android.permission.REQUEST_INSTALL_PACKAGES
! repo/com.lagradost.quicknovel_55.apk contains signature block blobs: 0x504b4453 (DEPENDENCY_INFO_BLOCK; GOOGLE)

So may I ask you again if the update check is disabled by default?

As for DEPENDENCY_INFO_BLOCK, that can easily be avoided with a minor adjustment to your build.gradle:

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. More details can be found e.g. here: Ramping up security: additional APK checks are in place with the IzzyOnDroid repo.

LagradOst commented 3 months ago

DEPENDENCY_INFO_BLOCK will be removed, just forgot. I also want to say that update check will not be disabled by default as github is the primary platform to download and update this app from, and this will not change.

However if you compile this app yourself I might be able to include a build config to change or remove the build in github updater on FDroid/IzzyOnDroid builds. Is there any environment variables or similar I can read in the build step to indicate that the computer is building the fdroid ver?

IzzySoft commented 3 months ago

DEPENDENCY_INFO_BLOCK will be removed, just forgot.

Thanks!

I also want to say that update check will not be disabled by default

That's sad. Those downloads would bypass the extra checks implemented in the repo here.

However if you compile this app yourself

I don't; the IzzyOnDroid updater just fetches the APKs the developers provide. But if you could establish such a flavor, maybe you could also provide the resulting APK – in addition to the current one? That would also make direct updates possible, as that build then would be signed by the same key.

LagradOst commented 3 months ago

I don't; the IzzyOnDroid updater just fetches the APKs the developers provide. But if you could establish such a flavor, maybe you could also provide the resulting APK – in addition to the current one? That would also make direct updates possible, as that build then would be signed by the same key.

Bruh that makes no sense. If IzzyOnDroid only grabs the APK from this github repo then why is inapp updater wrong, they both serve the exact same APK in the end.

IzzySoft commented 3 months ago

Because there are additional checks in place here. Folks get the results presented or, in the worst case, the APK is held back from being published.

I didn't say "in-app updater is wrong". It should be the conscious decision of those using the app. If I install an app from F-Droid or IzzyOnDroid, I expect the updates to come from there. People might think the update offered would be downloaded from the repo, and they might not be aware that the scans have been skipped.

Further, as it reaches out to Github by default, I might have to add the NonFreeNet anti-feature. Not so if it's opt-in.

IzzySoft commented 1 month ago

@LagradOst the updater is still there, the permission still requested. It's half a year now, so we need to solve this. If we can't, we'll have to remove the app from IoD: a policy that's not enforced is useless. So which way to go here?

(in order of my preference; removing the app I count as last resort if no choice is left).

I'll set myself a reminder there now to check back in about 4 weeks. By then we must have decided, sorry.

LagradOst commented 1 month ago

Read https://github.com/recloudstream/cloudstream/issues/1215. Remove QuickNovel and Cloud Stream 3, if the updated being turned off is critical, as github is the primary and only official source for both apps.

IzzySoft commented 1 month ago

OK, as you wish. A specific build flavor would have done the job, proper hints too. A pity.

IzzySoft commented 1 month ago

Both apps are removed now, effective with the next sync around 6 pm UTC. Well, almost exactly at their 3rd anniversary at IoD… Should you change your mind one day, just ring my bell.

All the best then!