XayahSuSuSu / Android-DataBackup

DataBackup for Android 7.0+
https://DataBackupOfficial.github.io
GNU General Public License v3.0
3.88k stars 147 forks source link

Question on permissions #257

Open IzzySoft opened 4 months ago

IzzySoft commented 4 months ago

I just received a report from the IoD scanners:

! repo/com.xayah.databackup_3411063.apk declares flag(s): usesCleartextTraffic
! repo/com.xayah.databackup_3411063.apk declares sensitive permission(s):
  android.permission.QUERY_ALL_PACKAGES
  android.permission.READ_EXTERNAL_STORAGE
  android.permission.MANAGE_EXTERNAL_STORAGE
! repo/com.xayah.databackup_3411063.apk contains signature block blobs: 0x504b4453 (DEPENDENCY_INFO_BLOCK; GOOGLE)

Could you please clarify? android.permission.QUERY_ALL_PACKAGES is clear for a backup app of course – but what cleartext connections are needed? I guess the storage should be clear as well (needed to access the data to be backed up/restored), but a confirmation is welcome.

As for DEPENDENCY_INFO_BLOCK, that can easily be avoided with a tiny adjustment in 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.

Thanks in advance!

IzzySoft commented 4 months ago

Btw, looks like the package name has changed:

2024-07-01 03:34:47,961 WARNING: com.xayah.databackup_3411063.apk (com.xayah.databackup.foss) has no metadata!

Shall I move to the new one? that would mean everyone had to uninstall/reinstall of course. Updater here is pinned to /v7a-foss-release/i.

XayahSuSuSu commented 4 months ago
  1. android.permission.QUERY_ALL_PACKAGES is for querying packages
  2. usesCleartextTraffic is needed by http connection, which is used by cloud functions(WevDAV)
  3. Storage permissions are for writing/reading
  4. Will check and test this DEPENDENCY_INFO_BLOCK with your suggetsions, thanks!
XayahSuSuSu commented 4 months ago

Btw, looks like the package name has changed:

2024-07-01 03:34:47,961 WARNING: com.xayah.databackup_3411063.apk (com.xayah.databackup.foss) has no metadata!

Shall I move to the new one? that would mean everyone had to uninstall/reinstall of course. Updater here is pinned to /v7a-foss-release/i.

Yeah we need to migrate to com.xayah.databackup.foss

XayahSuSuSu commented 4 months ago

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

android {
    dependenciesInfo {
        // Disables dependency metadata when building APKs.
        includeInApk = false
        // Disables dependency metadata when building Android App Bundles.
        includeInBundle = false
    }
}

I've added it, can you have a test? DataBackup-2.0.1-arm64-v8a-foss-release.zip

IzzySoft commented 4 months ago

which is used by cloud functions(WevDAV)

Local WebDAV I assume? Because in the "open net" it should always be SSL.

Package and storage permissions are clear then (the former I already had added to the green list). Done so for the storage perms as well then.

we need to migrate to com.xayah.databackup.foss

OK, will do so now so the new package should show up with the next sync around 6 pm UTC.

Thanks for disabling the blob – I've sent the file through the scanner and it didn't complain.

XayahSuSuSu commented 4 months ago

Local WebDAV I assume? Because in the "open net" it should always be SSL.

Not only local, we also support remote server

IzzySoft commented 4 months ago

Not only local, we also support remote server

I meant cleartextTraffic in this context. Cleartext connections to the open internet should at least raise a proper warning due to increased risk of MITM etc. Do you check that? The backups can contain quite sensitive details after all.

XayahSuSuSu commented 4 months ago

Not only local, we also support remote server

I meant cleartextTraffic in this context. Cleartext connections to the open internet should at least raise a proper warning due to increased risk of MITM etc. Do you check that? The backups can contain quite sensitive details after all.

Nope, there's no any check for that, maybe I can add a warning about it.

IzzySoft commented 4 months ago

maybe I can add a warning about it.

That would be good I'd say. Meanwhile, I'll add the flag to the "green list":

usesCleartextTraffic: intended for connections within the local network

should match I guess?

XayahSuSuSu commented 4 months ago

maybe I can add a warning about it.

That would be good I'd say. Meanwhile, I'll add the flag to the "green list":

usesCleartextTraffic: intended for connections within the local network

should match I guess?

Yes!