-
Notifications
You must be signed in to change notification settings - Fork 238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Trust user-added CAs on Android #461
Comments
This is copied from the corresponding config in zulip-mobile, which was added in the following commit: zulip/zulip-mobile@85c3a71 Unlike in zulip-mobile, though, we don't need any adjustments in debug (or profile) builds, because we don't use the `android:usesCleartextTraffic` attribute which this would clobber. (React Native debug builds reach out to the development host over cleartext HTTP in order to support hot updates; but the Flutter development tools have the host contact the app instead, using the Dart VM protocol. It's also not clear that any Dart libraries respect the `android:usesCleartextTraffic` flag anyway.) Fixes: zulip#461
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. |
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: |
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.) |
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. |
I appreciate the proactive follow-up, and look forward to the next release. |
The previous attempt at this (cffb112, zulip#474) set the configuration used by the HTTP implementation from the Android SDK, and by some third-party HTTP implementations. But in the bulk of the app's code, we use the HTTP implementation from the Dart standard library, and by default that does not consult the Android configuration. Happily, Flutter has an opt-in feature to apply the same configuration to the Dart standard library: https://chat.zulip.org/#narrow/stream/48-mobile/topic/flutter.3A.20user-added.20certs/near/1716845 So use that. Fixes: zulip#461
The previous attempt at this (cffb112, #474) set the configuration used by the HTTP implementation from the Android SDK, and by some third-party HTTP implementations. But in the bulk of the app's code, we use the HTTP implementation from the Dart standard library, and by default that does not consult the Android configuration. Happily, Flutter has an opt-in feature to apply the same configuration to the Dart standard library: https://chat.zulip.org/#narrow/stream/48-mobile/topic/flutter.3A.20user-added.20certs/near/1716845 So use that. Fixes: #461
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:
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. |
This reverts commit 9b2a5f2. That change caused a mysterious crash at startup, in release mode only: #461 (comment) Reverting while we debug.
@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. |
Here's an update on the state of this issue. Thanks to @rajveermalviya's investigation, we've learned:
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. |
This was introduced in cffb112 / zulip#474 because we thought it would be a fix for zulip#461. As discussed on that issue thread, though, it turns out this file doesn't have an effect on the Dart HTTP implementation, which we use. Since it looks like it's doing something but in fact it isn't, cut it out to avoid confusion. It'll still be there in the Git history if we later want to use it as part of a future effort to fix zulip#461.
This was introduced in cffb112 / zulip#474 because we thought it would be a fix for zulip#461. As discussed on that issue thread, though, it turns out this file doesn't have an effect on the Dart HTTP implementation, which we use. Since it looks like it's doing something but in fact it isn't, cut it out to avoid confusion. It'll still be there in the Git history if we later want to use it as part of a future effort to fix zulip#461.
This was introduced in cffb112 / zulip#474 because we thought it would be a fix for zulip#461. As discussed on that issue thread, though, it turns out this file doesn't have an effect on the Dart HTTP implementation, which we use. Since it looks like it's doing something but in fact it isn't, cut it out to avoid confusion. It'll still be there in the Git history if we later want to use it as part of a future effort to fix zulip#461.
This is a configuration change that was requested a few times over the years for zulip-mobile, as zulip/zulip-mobile#3312, before we got a contribution implementing it as zulip/zulip-mobile#5493 . We should do the same thing here.
The implementation for zulip-mobile (see zulip/zulip-mobile@85c3a71) is entirely at the Android metadata layer, so we should be able to pretty much just copy-paste that.
The text was updated successfully, but these errors were encountered: