Trust user-added CAs on Android

[gnprice]

This is a configuration change that was requested a few times over the years for zulip-mobile, as https://github.com/zulip/zulip-mobile/issues/3312, before we got a contribution implementing it as https://github.com/zulip/zulip-mobile/pull/5493 . We should do the same thing here.

The implementation for zulip-mobile (see https://github.com/zulip/zulip-mobile/commit/85c3a716a7bbf00c936beaf34bc84a0e6bf5c885) is entirely at the Android metadata layer, so we should be able to pretty much just copy-paste that.

[gnprice]

This feature is in the new beta release v0.0.9. @shrizza please give it a try — I'd be glad to have confirmation of whether it works for you.

[gnprice]

Seems like that version doesn't yet work — but there is hope that a small further tweak to the manifest will make it work. Details in chat: https://chat.zulip.org/#narrow/stream/48-mobile/topic/flutter.3A.20user-added.20certs/near/1716835

[gnprice]

Rereading the thread, it looks like the next step is to make that small further tweak to our manifest, and then put that in the next beta release so @shrizza can try it out. I think we have another beta release (v0.0.15) coming up this week, so I'll aim to include this there.

(The last round of this was just before I went travelling for part of a week and then got covid; I guess by the time I was back at full speed, this had fallen off my radar.)

[gnprice]

I guess by the time I was back at full speed, this had fallen off my radar

Oho, partly due to this GitHub bug/gap/misfeature:

So when we originally marked the issue fixed, that caused its "status" to become "Done" in the GitHub "project" we've been using to manage tasks for the Flutter beta app. But then when I reopened it a few days later, that didn't cause the "status" to change back — the project still saw it as "Done", until I happened to notice just last week.

As a result, the issue was absent from the project board we've been watching. So the "fallen off my radar" metaphor was unusually apt here — this issue literally did not appear on the display we were regularly checking to spot tasks we should handle next.

Anyway, glad we've now caught it.

[shrizza]

I appreciate the proactive follow-up, and look forward to the next release.

[gnprice]

Well, unfortunately it looks like I'm going to have to revert the take-2 fix I made as #671.

After that PR, in a release build on Android, the app crashes at startup with SIGSEGV and a stack trace like:

#00 pc 00000000006b39e8  /apex/com.android.art/lib64/libart.so (art::JNI<false>::GetStringChars(_JNIEnv*, _jstring*, unsigned char*)+232) (BuildId: 7ece79c15d80914c83e60c9e93ac1684)
#01 pc 00000000000ddbd8  /system/lib64/libandroid_runtime.so (android::android_content_XmlBlock_nativeGetAttributeIndex(_JNIEnv*, _jobject*, long, _jstring*, _jstring*)+72) (BuildId: 2c0c526c96b2f0f37ce26c968792b5fe)
#02 pc 0000000000331474  /data/misc/apexdata/com.android.art/dalvik-cache/arm64/boot.oat (art_jni_trampoline+116)
#03 pc 00000000008bc6d4  /data/misc/apexdata/com.android.art/dalvik-cache/arm64/boot.oat (android.content.res.XmlBlock$Parser.getAttributeBooleanValue+84)
#04 pc 00000000005ba754  /apex/com.android.art/lib64/libart.so (nterp_helper+7636) (BuildId: 7ece79c15d80914c83e60c9e93ac1684)
#05 pc 000000000021525e  /data/app/~~OdBMxcLjRbZre3jUFwqhtA==/com.zulip.flutter-Ivlz_Plsw-9nCNlt4y7GhA==/base.apk (y4.a.c+58)
#06 pc 000000000033b680  /apex/com.android.art/lib64/libart.so (art_quick_invoke_static_stub+640) (BuildId: 7ece79c15d80914c83e60c9e93ac1684)
#07 pc 0000000000511714  /apex/com.android.art/lib64/libart.so (bool art::interpreter::DoCall<false>(art::ArtMethod*, art::Thread*, art::ShadowFrame&, art::Instruction const*, unsigned short, bool, art::JValue*)+2364) (BuildId: 7ece79c15d80914c83e60c9e93ac1684)
#08 pc 000000000049136c  /apex/com.android.art/lib64/libart.so (void art::interpreter::ExecuteSwitchImplCpp<false>(art::interpreter::SwitchImplContext*)+1892) (BuildId: 7ece79c15d80914c83e60c9e93ac1684)
#09 pc 00000000003545d8  /apex/com.android.art/lib64/libart.so (ExecuteSwitchImplAsm+8) (BuildId: 7ece79c15d80914c83e60c9e93ac1684)
#10 pc 0000000000215534  /data/app/~~OdBMxcLjRbZre3jUFwqhtA==/com.zulip.flutter-Ivlz_Plsw-9nCNlt4y7GhA==/base.apk (y4.a.e+0)
#11 pc 000000000036e6ec  /apex/com.android.art/lib64/libart.so (art::interpreter::Execute(art::Thread*, art::CodeItemDataAccessor const&, art::ShadowFrame&, art::JValue, bool, bool) (.__uniq.112435418011751916792819755956732575238.llvm.9545667076320299271)+232) (BuildId: 7ece79c15d80914c83e60c9e93ac1684)
#12 pc 000000000036dfe4  /apex/com.android.art/lib64/libart.so (artQuickToInterpreterBridge+964) (BuildId: 7ece79c15d80914c83e60c9e93ac1684)
#13 pc 0000000000351f68  /apex/com.android.art/lib64/libart.so (art_quick_to_interpreter_bridge+88) (BuildId: 7ece79c15d80914c83e60c9e93ac1684)
#14 pc 00000000005b8a18  /apex/com.android.art/lib64/libart.so (nterp_helper+152) (BuildId: 7ece79c15d80914c83e60c9e93ac1684)
#15 pc 00000000002161a6  /data/app/~~OdBMxcLjRbZre3jUFwqhtA==/com.zulip.flutter-Ivlz_Plsw-9nCNlt4y7GhA==/base.apk (y4.f.t+66)
#16 pc 00000000005b98d4  /apex/com.android.art/lib64/libart.so (nterp_helper+3924) (BuildId: 7ece79c15d80914c83e60c9e93ac1684)
#17 pc 000000000021614a  /data/app/~~OdBMxcLjRbZre3jUFwqhtA==/com.zulip.flutter-Ivlz_Plsw-9nCNlt4y7GhA==/base.apk (y4.f.s+10)
#18 pc 00000000005b98d4  /apex/com.android.art/lib64/libart.so (nterp_helper+3924) (BuildId: 7ece79c15d80914c83e60c9e93ac1684)
#19 pc 000000000017f720  /data/app/~~OdBMxcLjRbZre3jUFwqhtA==/com.zulip.flutter-Ivlz_Plsw-9nCNlt4y7GhA==/base.apk (io.flutter.embedding.engine.d.<init>+56)
#20 pc 00000000005b98d4  /apex/com.android.art/lib64/libart.so (nterp_helper+3924) (BuildId: 7ece79c15d80914c83e60c9e93ac1684)
#21 pc 0000000000174fa4  /data/app/~~OdBMxcLjRbZre3jUFwqhtA==/com.zulip.flutter-Ivlz_Plsw-9nCNlt4y7GhA==/base.apk (io.flutter.embedding.android.h.K+344)
#22 pc 00000000005b98d4  /apex/com.android.art/lib64/libart.so (nterp_helper+3924) (BuildId: 7ece79c15d80914c83e60c9e93ac1684)
#23 pc 00000000001746a2  /data/app/~~OdBMxcLjRbZre3jUFwqhtA==/com.zulip.flutter-Ivlz_Plsw-9nCNlt4y7GhA==/base.apk (io.flutter.embedding.android.h.s+14)
#24 pc 00000000005b98d4  /apex/com.android.art/lib64/libart.so (nterp_helper+3924) (BuildId: 7ece79c15d80914c83e60c9e93ac1684)
#25 pc 00000000001759ee  /data/app/~~OdBMxcLjRbZre3jUFwqhtA==/com.zulip.flutter-Ivlz_Plsw-9nCNlt4y7GhA==/base.apk (io.flutter.embedding.android.g.onCreate+26)
#26 pc 00000000008e83ec  /data/misc/apexdata/com.android.art/dalvik-cache/arm64/boot.oat (android.app.Activity.performCreate+924)
#27 pc 000000000064b5a0  /data/misc/apexdata/com.android.art/dalvik-cache/arm64/boot.oat (android.app.Instrumentation.callActivityOnCreate+80)
#28 pc 000000000072cf94  /data/misc/apexdata/com.android.art/dalvik-cache/arm64/boot.oat (android.app.ActivityThread.performLaunchActivity+2708)
[… more frames …]

I didn't catch this earlier because it doesn't happen in debug mode.

Not sure what the issue is. Those top few frames sound like it's parsing some XML. Seems likely to be a bug in Flutter (or in Android) one way or another — in an APK, these XML files have been converted by the Android build system into its own binary format, so there really shouldn't be any parse errors after that stage.

In any case, we'll need to debug that before we can reland that change.

[gnprice]

@rajveermalviya, would you be up for investigating this after the small notification tweaks that are currently on your list?

[rajveermalviya]

would you be up for investigating this after the small notification tweaks that are currently on your list?

@gnprice Sure, I'll self-assign it and take a at it look later.

[gnprice]

Here's an update on the state of this issue. Thanks to @rajveermalviya's investigation, we've learned:

• The strategy we'd been trying above unfortunately isn't going to work — the Flutter feature that we thought was there, isn't actually there.

  That strategy was to use an Android "network security config" XML file, just like the one we'd used on zulip-mobile; it looked like there was a Flutter feature to read that config and apply it to Dart's HTTP library. But that feature doesn't actually exist. The chat thread where we figured that out is here.

• There's an alternative strategy that should work: instead of the pure-Dart HTTP implementation that's in the Dart standard library, we could switch to package:cronet_http on Android. That uses Cronet, an HTTP client from Chromium. See chat thread.

• Using Cronet (and perhaps package:cupertino_http for an analogous choice on iOS?) may be a good idea independent of this issue. But it's going to be nontrivial to do:

  • By default package:cronet_http depends on Google Play Services for the Cronet library it uses.
  • Some people use the Zulip app without Google Play Services, and we'd like it to keep working for them. There's a build-time flag we can use in order to bundle Cronet into the app instead.
  • But that bundled Cronet adds around 10 MB to the app's download size. That's a bigger size increase than I would feel great doing for all our users who don't need it (because they do have Play Services).
  • So we'd need to do something like build two different variants, with and without a bundled Cronet.

Given the greater complexity that it turns out will be involved in order to implement this, this isn't an issue we'll be able to get to right now. I still hope we'll be able to do this, but there are a number of other features we'll prioritize building first.

[rajveermalviya]

I think closing this was unintended, reopening. (It happened because of the ... fix #461. line in commit message.)

[gnprice]

Shoot, yes — thanks @rajveermalviya for catching that!

In that commit message I even used one of my usual circumlocutions for avoiding exactly this situation: "would be a fix for #nnn", instead of "would fix #nnn". But then a few sentences later when I wrote "a future effort to fix #461", I didn't notice that the magic phrase was appearing again and needed to be avoided again.