Backups should not store sensitive data

Luna712 commented 1 year ago

Steps to reproduce

Backup and then look at backup

Expected behavior

Sensitive data should not be in there

Actual behavior

Sensitive data is there

For example, "opensubtitles_account_3/open_subtitles_user":"{\"user\":\"<real_username>\",\"pass\":\"<real_password>\",\"access_token\":\"<real_access_token>\"}"

From what I could tell, this is meant to be avoided by this, but it does not seem to work properly: https://github.com/recloudstream/cloudstream/blob/8b73c35e43ecdd1f8658b3ee75552ded132aaa22/app/src/main/java/com/lagradost/cloudstream3/utils/BackupUtils.kt#L51-L69

Additionally we should probably add lockPin to be excluded from that as well. simkl_user maybe also, even though that only stores username and avatar, it can't be used on restore without reauthorization, especially if you restore on a different device. And another issue is when restoring it should only try to enable the plugins from restored repositories that you had enabled before, the only option seems to be none and do all needed knes manually again, or do all of them and remove ones you don't want again.

Also, automatic backups don't seem to always work on my phone. It does on TV and even emulator but on my phone, set to 3 hours and last backup was two days ago. Subscription worker doesn't seem to be triggered either. I notice subscriptions being updated on emulator on PC, but on phone, I never, as if the schedulers just don't work on the phone very often.

Cloudstream version and commit hash

4.2.1 8b73c35

Android version

Android 13

Logcat

No response

Other details

No response

Acknowledgements

[X] I have searched the existing issues and this is a new ticket, NOT a duplicate or related to another open issue.
[X] I have written a short but informative title.
[X] I have updated the app to pre-release version Latest.
[X] If related to a provider, I have checked the site and it works, but not the app.
[X] I will fill out all of the requested information in this form.

recloudstream[bot] commented 1 year ago

Hello Luna712. Please do not report any provider bugs here. This repository does not contain any providers. Please find the appropriate repository and report your issue there or join the discord.

Found provider name: Loklok

fire-light42 commented 1 year ago

Ye, this seams like a major bug. However it would be pointless to not store the lockPin as lockPin is only the key to view the rest of the unencrypted data. If you really wanted that, then you would need to encrypt all keys associated with an account with that pin, but that too would be useless as you could brute force 10000 combinations quite easily. So while I agree that opensubs + simkl and stuff should be removed, we cant remove the pin.
What api is your phone? the worker does not work on some lower API vers.

Luna712 commented 1 year ago

Ye, this seams like a major bug. However it would be pointless to not store the lockPin as lockPin is only the key to view the rest of the unencrypted data. If you really wanted that, then you would need to encrypt all keys associated with an account with that pin, but that too would be useless as you could brute force 10000 combinations quite easily. So while I agree that opensubs + simkl and stuff should be removed, we cant remove the pin.

Yeah It would not make much sense now that I think of it, not storing it only introduces another way to bypass the PIN, so nevermind on that part

What api is your phone? the worker does not work on some lower API vers.

API 33

@fire-light42

CranberrySoup commented 1 year ago

Please do not touch the backup system overly much, my PR reworks a lot there, including fixing sensitive keys.

Luna712 commented 1 year ago

Please do not touch the backup system overly much, my PR reworks a lot there, including fixing this.

@CranberrySoup

Your PR was the reason I created this instead of trying to fix it myself for this exact reason, I did not know it fixed the issue though, sorry about that.

recloudstream / cloudstream