NO-ob / LoliSnatcher_Droid

A booru client with support for batch downloading
GNU Affero General Public License v3.0
371 stars 23 forks source link

cleartext traffic? #245

Open IzzySoft opened 4 months ago

IzzySoft commented 4 months ago

My scanner got a few additional checks in January, and on today's update reported:

! repo/com.noaisu.loliSnatcher_1187.apk declares flag(s): usesCleartextTraffic
! repo/com.noaisu.loliSnatcher_1187.apk declares sensitive permission(s): android.permission.READ_EXTERNAL_STORAGE

Can you please clarify what those two are needed for? Are there still sites around that do not offer HTTPS nowadays, making cleartext traffic (aka "plain HTTP") necessary?

Taking a closer look at the package details:

image

OK, Read + Write for external storage. If I knew what those are for, I might be able to put them on the "allow list".

Ah, in case you wonder about that DEPENDENCY_INFO_BLOCK, that's easy to get rid of if you update 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.

Thanks in advance!

NANI-SORE commented 4 months ago

I think we saw a few old sites that still use http, but it was like a year ago and I can't give any examples right now. We'll probably remove the usesCleartextTraffic in the next release if there aren't any active sites.

Read/write to external storage iirc is to store settings file on older android versions and download images + check if they were already downloaded. They are used from the start of the project 4+ years ago, when I wasn't even on the project and I didn't ever really bother to go through our permissions to check if we need to rethink usage of some of them

@NO-ob Maybe you have info/thoughts on this?

IzzySoft commented 4 months ago

We'll probably remove the usesCleartextTraffic in the next release if there aren't any active sites.

:+1: Thanks! If you can't, it can be put to the "allow-list" with a proper explanation (like "needed for some sites like Lollipops and SnickerSneakers" (the two names are just placeholders). I just wanted to make sure if it wasn't simply forgotten from some tests, and that if it's needed the use cases are made transparent.

to store settings file on older android versions

Maybe an app-internal directory would fit those? Or if we speak of "a single user-defined location", SAF (Storage Access Framework) would suffice? For a single, non-sensitive directory, SAF should be fine. One needs at least Android 5 to run your app, so SAF should be available (it just becomes tricky when media directories or the Downloads/ come into play, or when you need to access the "root" directory of external storage (i.e. /sdcard itself). Then it is a mess if you want to support Android 5-14 with all the exceptions and special cases).

NANI-SORE commented 4 months ago

I'll see during the week if I can find examples of sites we may need usesCleartextTraffic for.

Maybe an app-internal directory would fit those?

We already do, but there was a bit of a mess involving older devices when we were switching to internal directory and had to support loading from both internal and external folders to avoid losing settings. Right now we only use internal folder. And we already use SAF for media in case user sets custom folder, but by default we use Pictures folder

IzzySoft commented 4 months ago

I'll see during the week if I can find examples of sites we may need usesCleartextTraffic for.

:+1: thanks!

had to support loading from both internal and external folders to avoid losing settings

A one-time "path migration" maybe to fix that up? Now that you no longer support Android < 5, SAF is available on all clients.

but by default we use Pictures folder

Oof. OK, that complicates things with SAF IIRC (needing the MEDIA_STORAGE permissions on some, EXTERNAL_STORAGE on others – and not sure if even MANAGE on newer ones…

OK, let me know when you came to a conclusion. If there are clear reasons, that shouldn't be a stopper. Those permissions can be put to the allow-list accompanied with the proper reasons.

NO-ob commented 4 months ago

Hi Izzy thankyou for hosting our app on your repo.

HTTP is needed for people who are self hosting boorus as they may not be using SSL certificates if its only available on their local network, we also use HTTP for the syncing feature which allows users to send data between two devices. There is a warning on the sync page about using it on public wifi.

Regarding the read/write external that may be able to be removed as were using SAF, it think its left over from before SAF was being used. Ill have to take a look after work later though. afaik the permission doesn't actually do anything on android versions newer than 10 or 11

IzzySoft commented 4 months ago

Thanks for the explanations! I've added usesCleartextTraffic to the allow-list with your explanation ("needed for people who are self hosting boorus in their local network".

Storage: Leaving that open then for now (so updates will raise warnings until it's gone), waiting for it to disappear – or for you to state it's still needed and giving me some background I can add as reason.


While I'm here, and no offense meant, may I raise the question on why you gave the app a rather, how to put it, doubt-raising name? I just got a complaint by someone using my repo and felt very offended that I host "such apps". Turned out (just as I thought) they just saw the name and the screenshots, no more, but they didn't even dare taking a closer look out of fear.

I don't think their "expectations of content" would be met by looking at your app (would I think so, it would certainly not be in my repo, as that should be safe for everyone to use without taking offense). Maybe some proper hints in the app description would help people not even getting such ideas (changing the name would help, too, if you would consider that – but given this is a long-standing project I'm not sure if you'd count that an option).

I'm hesitant simply slapping the NSFW anti-feature on the app just going by the name and by "Hentai" being mentioned in the Readme. If you'd say that would be OK, folks feeling offended by stumbling upon it while browsing the app list in their clients could simply hide it via that. Giving some clarification would definitely help I'd say. What do you think?

NO-ob commented 4 months ago

Regarding the read/write external permissions we should have this tested by the end of the week to see if it can be removed.

Regarding making changes to appease someone who is scared/offended by a few letters on their screen, I think that is quite stupid and a waste of time. Some people will always choose to get offended by anything. Also what do you mean by proper hints in description?

I'm not sure the NSFW tag is appropriate because the app will only show NSFW content if the user decides to add NSFW boorus

IzzySoft commented 4 months ago

we should have this tested

I definitely agree with that! And should it turn out to be needed, I can put it to the "allow-list" with the proper explanation.

Some people will always choose to get offended by anything

Eh, c'mon. To those familiar with the topic the name is "pulling their noses". It's at least a bit provocative to them, don't you agree? And now that I learned what the term is used for in some places, I was looking as well: is it meant that way? It's not denied (nor confirmed). Which raises the suspicion it might be that way to not trigger any "shutdown actions" or the likes (I saw you had some experience with "NSWF labeling" already, probably because of that). Not saying it is that way. But it's a "nagging thought".

what do you mean by proper hints in description?

I've just learned the name, together with the icon of the app… Well, let me quote as it's a "public record" anyway:

it's called "lolisnatcher" and the icon is a blushing child eating a popsicle which is pretty suggestive in itself that there might be more going on than just art.

So while I agree with you it's impossible to "appease everyone" (there will always be people not liking this or that) – I also agree with the voice saying it's "suggestive" and, as I understand it, easy to misinterpret if you're familiar with the topic (which I am not; my contacts with any kind of manga are rather very few and occasional. Believe it or not, I've crossed the 50 years ago but only a few years ago learned what Hentai is even).

I have no clue what a good (and short) hint could look like (my brain kicks in saying: "it's not what you think – it's just a generic image board browser"; but the first part is not an "ideal intro" to write down I guess).

I'm not sure the NSFW tag is appropriate because the app will only show NSFW content if the user decides to add NSFW boorus

If none of those are selected by default, but one has to actively "tick" it to be shown: No, then NSFW is not justified. Only if NSFW categories are "pushed on your nose", and "offensive content" is shown by default.

IzzySoft commented 4 months ago

what do you mean by proper hints in description?

If you mean it that way, maybe something like: "Manga to me is like sweets to a child. So like a child would snatch a lolly out of your hands when offered, I snatch the manga from the server where it is available. Which is why I called this app „LoliSnatcher Droid“."

NANI-SORE commented 4 months ago

You are making it worse...

IzzySoft commented 4 months ago

???

NANI-SORE commented 4 months ago

You propose to explain the use of the word "loli" by mentioning children. This will only lead to people misinterpreting it even more

IzzySoft commented 4 months ago

Ah; but I only interpreted the icon there (a child enjoying some sweets). OK, pick something else then; you know best what the name is supposed to mean. I cannot advise in an area I'm not familiar with, sorry.

NO-ob commented 4 months ago

About the external storage permissions they dont do anything in version newer than android 10 afaik as the functionality of those was moved to MANAGE_EXTERNAL_STORAGE, were not using that because it's a pain to get the app on the playstore with that permission but it makes the app more convenient for users

For android < 10 with read/write external:

We store user settings in a directory in the users main storage. I dont really want to remove the permissions because this method of storing the users files is much more convenient on the user end than using the private data directory.

Forcing the private data directory means on those older android versions that can use the read/write external perms that all app settings and the database will now be deleted on app reinstall which is something that happens on newer android versions but not the older ones.

If a user wants to downgrade to an older apk they have to export their settings before uninstalling and then import again when reinstalling. If they have read/write permissions they dont need to do these steps which they may forget to do and lose their configs and favourites database.

It also allows users to share settings between different app builds such as between the github and google play versions of the app by default instead of having to manually set their storage directory. And because it's in their external storage by default they have easy access to their configs and favourites database via usb without first needing to export it from the private data directory

We haven't had any user complaints about the directory being created in their external storage so I dont think it's right to just remove it when the app functions better when it has access to it

IzzySoft commented 4 months ago

export their settings

That's a fully sufficient explanation on the permission, thanks! Added that for WRITE_EXTERNAL_STORAGE as well, including the example that one might wish to transfer settings to another device. So it's added to the allow-list now and won't trigger any warnings anymore.

For the "naming question": As obviously even my innocent explanation got it wrong, could you tell me the story behind the name of the app, maybe? Thanks in advance!