Akylas / alpimaps

Offline map app iOS/Android
https://www.akylas.fr
MIT License
68 stars 1 forks source link

Question on permissions #15

Open IzzySoft opened 6 months ago

IzzySoft commented 6 months ago

Well, guess you're waiting for this one already with the new release still hot, so I can't disappoint you, right?

! repo/akylas.alpi.maps_251.apk declares sensitive permission(s):
 android.permission.ACCESS_COARSE_LOCATION android.permission.ACCESS_FINE_LOCATION
 android.permission.MANAGE_EXTERNAL_STORAGE android.permission.READ_EXTERNAL_STORAGE
 android.permission.CAMERA android.permission.READ_PHONE_STATE

got most of them covered already (I hope):

      android.permission.ACCESS_COARSE_LOCATION: needed for navigation
      android.permission.ACCESS_FINE_LOCATION: needed for navigation
      android.permission.MANAGE_EXTERNAL_STORAGE: needed to access data generated off-device e.g. using the alpimaps_data_generator
      android.permission.READ_EXTERNAL_STORAGE: needed to access data generated off-device e.g. using the alpimaps_data_generator

which leaves CAMERA and READ_PHONE_STATE. I've searched the code, but not being an Android dev (and especially having no ideas about Svelte and TypeScript), I had no success there. Can you once more help me out, please?

image

And in case I didn't yet mention that DEPENDENCY_INFO_BLOCK, which is only useful to Google but to nobody else:

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.

farfromrefug commented 6 months ago

@IzzySoft You are so fast :D Yes i was kind of waiting for it :D READ_PHONE_STATE i already added to code to remove it (must have been after the release). I also found out it came from androidx (core i think) .... CAMERA I actually use this because there is a peak finder view in the app (AR view of summits). It is done through a webview but i still need camera permission to use camera in the webview

IzzySoft commented 6 months ago

You are so fast :D Yes i was kind of waiting for it :D

Hehe, thought so :rofl:

READ_PHONE_STATE i already added to code to remove it (must have been after the release). I also found out it came from androidx (core i think) ....

Wouldn't surprise me. Most documentations recommend that to detect incoming calls and stop your activity while the call is in progress. Google made it especially easy this way, though there's a less privacy-sensitive way to achieve the very same, not even needing any special permission IIRC (hint: register for the AudioFocusChanged broadcast and check if it switched to a call :man_shrugging:).

As it's already on your removal-list, I'll simply leave it open for now and the warning should go away with the next release then.

peak finder view in the app (AR view of summits)

Ah, that's why I couldn't find it, I used the wrong keywords… Yes, that totally makes sense, adding it right away and syncing up the changes. Should be visible in a minute or so … done.

What do you think about that DEPENDENCY_INFO_BLOCK? It's an opaque block nobody but Google can decipher, so it shouldn't be in a FOSS build IMHO. Currently it's just shown "neutrally" with my repo; but we've run tests/POCs and found one can easily hide almost anything in such blobs. If I understood our security expert correctly, she could take your APK, add some payload into such a BLOB – and signature verification would still signal an "all fine" (I'll make sure to get a proper explanation from her as soon as my desk shows some of its own structure again – to much work on it currently :see_no_evil:).

farfromrefug commented 6 months ago

@IzzySoft i already disabled dependenciesInfo for apk builds in my other apps. Will do the same here. I

IzzySoft commented 6 months ago

Thanks!

IzzySoft commented 4 months ago

Oof, I see we have another issue there:

$ keytool -printcert -jarfile ../repo/akylas.alpi.maps_254.apk
Signer #1:

Certificate #1:
Owner: CN=Appcelerator Titanium, OU=Nolan's Gang, O=Appcelerator, L=Mountain View, ST=CA, C=US
Issuer: CN=Appcelerator Titanium, OU=Nolan's Gang, O=Appcelerator, L=Mountain View, ST=CA, C=US
Serial number: 4a1786e2
Valid from: Sat May 23 07:17:22 CEST 2009 until: Wed Jan 14 06:17:22 CET 2995
Certificate fingerprints:
     SHA1: CC:E3:7F:08:FA:03:9C:88:07:BC:CB:AB:7B:88:61:F4:75:9D:47:9F
     SHA256: D6:D2:96:21:13:2D:4A:F2:9B:A1:42:AC:F0:A6:E5:87:DA:AF:B3:8A:61:D5:15:CA:52:8A:D6:D2:B0:5B:A4:85
Signature algorithm name: MD5withRSA (disabled)
Subject Public Key Algorithm: 1024-bit RSA key (weak)
Version: 1

Warning:
The certificate uses the MD5withRSA signature algorithm which is considered a security risk and is disabled.
The certificate uses a 1024-bit RSA key which is considered a security risk. This key size will be disabled in a future update.

AFAIK there's no way to "upgrade" the certificate, you'd need a new key. I wonder what your app at PlayStore is signed with? This might become an issue at some point: a disabled algorithm, plus a too small RSA key – and most likely is a security issue already now (or else keytool would not complain).

Do you have any plans for a "key upgrade" already? With minSdk at Android 5, key rotation is not possible (unless you want to move minSdk up to 9). Or maybe 9+ would have it seemless with rotation, but < 9 would need to uninstall/reinstall then.

farfromrefug commented 4 months ago

@IzzySoft that key is so old you are right. It was one I started using by mistake on play store. And like a dummy I started using it for fdroid. What other solution i have than create a new one ? (Which would break for existing user, thankfully not many other than me

IzzySoft commented 4 months ago

Apart from key rotation (which I only know from theory), I'm not aware of any alternative to an "abrupt change". Currently it's only keytool giving a warning (neither androguard nor apksigner complain), and as the app is not a "security core app" (as e.g. a password store or banking app would be), it's probably nothing you need to "fix this week", but you should be prepared. As you can see, MD5withRSAalready is declared "disabled" by keytool, so a corresponding policy might come up at play soon. fdroidserver (which I use in my repo) doesn't complain yet either, as it uses apksigner for verification – but that might also change one day.

I'd probably not wait until the last Android-8 devices are "phased out" to be "on the safe side" with key rotation (which was introduced with Android-9), that would take years at best. But if you need some weeks to decide, I'd say that should be fine.

thankfully not many other than me

I wouldn't be too sure about that. From the logs of the last 2 weeks:

# egrep 'GET .*akylas.alpi.maps.*apk' access_log |wc -l
54

So 54 downloads of the APK in 2 weeks (just from my primary, not counting the mirrors), or about 2 a day. Only 1 of those has "F-Droid" as user agent, though, so I cannot tell how many of those downloads mean (unique) installations. Especially as UAs like "Ktor client" or "okhttp/5.0.0-alpha.9" could be anything (e.g. a 3rd party F-Droid client).