zelenyhleb / tortoise

Android music player
Apache License 2.0
14 stars 1 forks source link

Certificate issue #36

Open IzzySoft opened 8 months ago

IzzySoft commented 8 months ago

A scan (see here for details and background) just revealed the APKs at your releases are signed using a debug key. As that has security implications, may I ask you to please switch to a proper release key, and provide the corresponding APK signed with it? Thanks in advance!

IzzySoft commented 7 months ago

@zelenyhleb any word?

zelenyhleb commented 7 months ago

Hello @IzzySoft! TBH, I was really surprised (and pleased at the same time) about such attention to this forgotten project. I've checked out your GitHub account and IzzyOnDroid website and I am very impressed by your venture!

I've left Android development a while ago, but decided to please you with fixing this security issue. New release with signed apks is already published. It uses already published pre-release version with no further additions, so there was no reason to update source code somehow.

Please take a look and feel free to contact me here if signed apk is still somehow not applicable for your case.

IzzySoft commented 7 months ago

Such a heartwarming response :star_struck: Thanks! Yes, both sites you've checked are kind of my "crown jewels" (next to my eBook server). I'm currently working hard on improving quality, security and transparency for my F-Droid repo – which also caused me to reach out to you here.

Signature looks good now, but versionCode was decreased:

package: name='ru.krivocraft.tortoise' versionCode='1' versionName='0.4.1'

The previous version had

package: name='ru.krivocraft.kbmp' versionCode='20' versionName='0.3.8'

Would it be possible to give the new release a versionCode higher than 20? Not that it matters for updates (which will require an uninstall anyway), but that way it would show up on top as newest release.

Btw, here's what my updater logged:

$ iod repo get ru.krivocraft.kbmp
ru.krivocraft.kbmp: looking for 'https://api.github.com/repos/zelenyhleb/tortoise/releases'
ru.krivocraft.kbmp: checking tag 'v0.4.0'
ru.krivocraft.kbmp: lastRelNo set to 'v0.4.0', checking for files
ru.krivocraft.kbmp: Upstream file date (2024-02-19 15:51) is newer than ours (2020-02-11 01:00).
ru.krivocraft.kbmp: returning ['v0.4.0','https://github.com/zelenyhleb/tortoise/releases/download/v0.4.0/tortoise.apk',1708354277]
ru.krivocraft.kbmp: 0.3.8/v0.4.0, https://github.com/zelenyhleb/tortoise/releases: https://github.com/zelenyhleb/tortoise/releases/download/v0.4.0/tortoise.apk
- Grabbing update for ru.krivocraft.kbmp: OK
- Checking 'repo/ru.krivocraft.kbmp_1.apk' for libraries and malware …
- Checking the app's AndroidManifest.xml …
! repo/ru.krivocraft.kbmp_1.apk declares sensitive permission(s): android.permission.READ_EXTERNAL_STORAGE
! repo/ru.krivocraft.kbmp_1.apk contains signature block blobs: 0x504b4453 (DEPENDENCY_INFO_BLOCK; GOOGLE)
ru.krivocraft.kbmp: check if repo contains FUNDING.yml
ru.krivocraft.kbmp: looking for 'https://api.github.com/repos/zelenyhleb/tortoise/contents/.github'
ru.krivocraft.kbmp: Github reports "Not Found" for https://api.github.com/repos/zelenyhleb/tortoise/contents/.github
ru.krivocraft.kbmp: looking for 'https://api.github.com/repos/zelenyhleb/tortoise/contents/'
ru.krivocraft.kbmp: looking for 'https://api.github.com/repos/zelenyhleb/.github/contents/'
ru.krivocraft.kbmp: Github reports "Not Found" for https://api.github.com/repos/zelenyhleb/.github/contents/
ru.krivocraft.kbmp: no FUNDING.yml detected.
ru.krivocraft.kbmp: no Fastlane configured, skipping Fastlane check.

The storage permission is clear (I'll just set that here now – and I guess the write permission would be for saving playlists). But should you increase versionCode and sign again, could we have that DEPENDENCY_INFO_BLOCK removed? Easy to achieve:

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 a lot once more!!!

zelenyhleb commented 7 months ago

Thank you for detailed instructions. It helped a lot, because I remember nearly nothing about android apps development =). I tried to follow it (#37). Release was updated as well.

Please take a look.

IzzySoft commented 7 months ago

Thank you for detailed instructions. It helped a lot

Glad to read! :star_struck:

Please take a look.

That looks fine!

ru.krivocraft.kbmp: 0.3.8/v0.4.0, https://github.com/zelenyhleb/tortoise/releases: https://github.com/zelenyhleb/tortoise/releases/download/v0.4.0/tortoise.apk
- Grabbing update for ru.krivocraft.kbmp: OK
- Checking 'repo/ru.krivocraft.kbmp_21.apk' for libraries and malware …
- Checking the app's AndroidManifest.xml …
! repo/ru.krivocraft.kbmp_21.apk declares sensitive permission(s): android.permission.READ_EXTERNAL_STORAGE

Now pinning the new cert… done. Updating the index… Oops… Did you change the packageName? It's now ru.krivocraft.tortoise, that makes it a new app basically. But when if not now, as folks need to uninstall and reinstall anyway. OK, so I made this a new app, added a hint to switch over in the "old" ones description, and set a marker to remove the "old" one in a month.

All done then – unless you want the other permissions annotated as well:

image

If not, you can simply close this issue then. The changes will go live with the next sync around 7 pm UTC – and until the kbmp is removed look like this:

image

Thanks a lot once more!

zelenyhleb commented 7 months ago

Ok, so I am closing this. Thank you for your involvement!

IzzySoft commented 7 months ago

I have to thank you!

IzzySoft commented 1 month ago

@zelenyhleb what happened now? Signing key changed again, so your update was rejected.

v0.4.1:

Signer #1 certificate DN: C=RU, ST=Saint-Petersburg, L=Saint-Petersburg, O=Nikifor Fedorov, OU=Nikifor Fedorov, CN=Nikifor Fedorov
Signer #1 certificate SHA-256 digest: 3c1d521bd2048b649466b8cc08859aa2d81dd80acc13ef777c42bc03a1807fe2
Signer #1 certificate SHA-1 digest: 0e4ee38de3b8b4ab745c6edbbea05064e538848c
Signer #1 certificate MD5 digest: a90b5e14ae27b0a023f28a5bb4873e6a
Signer #1 key algorithm: RSA
Signer #1 key size (bits): 2048
Signer #1 public key SHA-256 digest: 8a2d9be2550a4cd2542dbe087c6b9988106fa511c57d8fc6f3d77c39772cd280
Signer #1 public key SHA-1 digest: e89d9b38cb5fcfb343d8e86ed56d44fc79c6d955
Signer #1 public key MD5 digest: f85c7cd058df3add3d3959c8b9d21186

v0.5.2:

Signer #1 certificate DN: C=RU, ST=Saint-Petersburg, L=Saint-Petersburg, CN=Nikifor Fedorov
Signer #1 certificate SHA-256 digest: 03dd0662eeb9911221c9fdc6eb560bb9e47cfbbd06eb4ba317cd2a63fd5dd444
Signer #1 certificate SHA-1 digest: 579e201d4d371c333573cc3ebfab7b0dd5be5c0d
Signer #1 certificate MD5 digest: b73d7f0e8a66d3074512bff1d763876a
Signer #1 key algorithm: RSA
Signer #1 key size (bits): 2048
Signer #1 public key SHA-256 digest: 02b52bfb374d1d65ce9593a815356216e1d76f2ff786e43deb9ea756da766dbd
Signer #1 public key SHA-1 digest: f3c55ce61832620e96e134ad3cfb7de94a5e5e42
Signer #1 public key MD5 digest: 3121ee2089a392eab88f5f7c4f7749cb
zelenyhleb commented 1 month ago

@IzzySoft Yes, the key has changed because I completely lost all data from my old computer including all keys and certificates I had, which was harmful. For now it's impossible to restore them. What would you advise me to resolve the issue?

IzzySoft commented 1 month ago

Well, we need to "confirm it's really you":

While waiting for that answer, let me continue here: above list gives you some measures to take for such cases. Best, create yourself a GPG key and start signing your commits: it's rather unlikely someone got hold of your Github account and your GPG keys at the same time. Making backups of your keystore would also be a good idea :wink:

IzzySoft commented 1 month ago

OK, I received your "other answer" – and yes, sometimes one has to be creative :rofl:

Added the new hash to AllowedAPKSigningKeys for Tortoise. And may I strongly suggest to take some measures, like signing your commits and keeping a backup of your keystore? How to keep your key safe and what measures to take for the event of loss? gives you some background and additional options.

IzzySoft commented 1 month ago

PS: I also added a release note here: "the signing key was changed, so you need to uninstall and re-install in order to update." This is possible thanks to Fastlane structures being established on our end. Would be easier to have them on your end, so you can maintain description, screenshots etc. and keep them updated together with the app. If you wish, I can send you a "starter kit" with what is set up here via PR – and you then can use the IzzyOnDroid Fastlane Documentation as guide to build upon that. Just let me know if you want to have it.

image