diachedelic / capacitor-blob-writer

Capacitor plugin to write binary data to the filesystem
MIT License
132 stars 17 forks source link

net::ERR_CLEARTEXT_NOT_PERMITTED on Android #20

Closed diachedelic closed 2 years ago

diachedelic commented 4 years ago

Manifests as a "TypeError: Failed to fetch" during writeFile.

See https://stackoverflow.com/questions/54752716/why-am-i-seeing-neterr-cleartext-not-permitted-errors-after-upgrading-to-cordo

Breaks BlobWriter for many, many Android devices (especially newer ones).

axis7818 commented 4 years ago

I just ran into this. I didn't want to allow http for the entire app, so here is my fix if it helps others:

  1. Create android/app/src/main/res/xml/network_security_config.xml
<?xml version="1.0" encoding="utf-8"?>
<network-security-config>
  <domain-config cleartextTrafficPermitted="true">
    <domain includeSubdomains="true">localhost</domain>
  </domain-config>
</network-security-config>
  1. specify the network security config in android/app/src/main/AndroidManifest.xml
<?xml version="1.0" encoding="utf-8"?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android" package="com.worldspinner.portraits">

  <application
    . . . 
    android:networkSecurityConfig="@xml/network_security_config">

    . . . 

  </application>

  . . .

</manifest>
diachedelic commented 4 years ago

There may be a way to trust a self-signed localhost certificate, but it looks like a pain and potentially impacts the security of the whole app: https://stackoverflow.com/questions/8693991/java-ignore-expired-ssl-certificate/8694377#8694377. Edit: also https://stackoverflow.com/a/21936109/502846

I think users of this plugin will just have to use the configuration you have provided @axis7818 , I will update the docs soon.

diachedelic commented 4 years ago

This issue has possibly reoccurred on at least one device - all I see in the log is "TypeError: Failed to fetch" so it may be a different issue.

Chrome Mobile WebView 66.0.3359 Pixel 3 XL Android 9

KevinKelchen commented 3 years ago

I was able to reproduce the net::ERR_CLEARTEXT_NOT_PERMITTED issue when using Capacitor's live reload feature.

With live reload an ionic serve runs on the development machine and is exposed to the mobile device over a local WiFi network using a dynamic, local IP address. That local IP is used to serve the front-end app in the WebView. Since it's an IP address and not localhost it doesn't work as expected.

What happens in the app is the splash screen doesn't hide. If I background the app and then bring it back into the foreground, the splash screen hides and you can see the error message.

If you add a <domain> to the config for the IP address in addition to localhost, then it works as expected. That's a manual step though that's likely cause for confusion and lost time during the development workflow.

It appears that Capacitor live reload automatically adds android:usesCleartextTraffic="true" to the application element of android/capacitor-cordova-android-plugins/src/main/AndroidManifest.xml so that the WebView traffic itself is permitted using an IP address. It would seem that this is being trounced by the manual configuration recommended by capacitor-blob-writer in network_security_config. See https://developer.android.com/guide/topics/manifest/application-element where it discusses android:usesCleartextTraffic: "This flag is ignored on Android 7.0 (API level 24) and above if an Android Network Security Config is present."

For us live reload working well on Android is important not only for the shortened feedback loop for front-end changes but also because that's our workaround for getting TypeScript source maps working with the Chrome dev tools during remote WebView debugging.

diachedelic commented 3 years ago

@KevinKelchen Do you see then net::ERR_CLEARTEXT_NOT_PERMITTED error in response to a BlobWriter call, or somewhere else?

Is there a way to further configure the network_security_config such that it behaves like android:usesCleartextTraffic="true"?

KevinKelchen commented 3 years ago

Thank you for responding! 😀

@KevinKelchen Do you see then net::ERR_CLEARTEXT_NOT_PERMITTED error in response to a BlobWriter call, or somewhere else?

When I described the net::ERR_CLEARTEXT_NOT_PERMITTED occurring and talked about the splash screen not hiding and such, that was upon application launch and BlobWriter is not being used at that point. What's messed up is the WebView's ability to load data from the ionic serve running on the development machine at an IP address such as http://192.168.0.104:8100.

As mentioned above, this is because the presence of a android:networkSecurityConfig overrides Ionic's dynamic addition of android:usesCleartextTraffic that they add for facilitating live reload from an external IP address. The android:networkSecurityConfig only allows clear text over the localhost domain. That continues to work fine for the WebView when not using live reload or in production when the front-end app is served at a hostname of localhost, but during live reload it's an IP address that can potentially change often. The android:networkSecurityConfig's specification of a domain like localhost makes it act like an allow list and therefore blocks any http traffic not explicitly in android:networkSecurityConfig such as the IP address.

Is there a way to further configure the network_security_config such that it behaves like android:usesCleartextTraffic="true"?

I think you might be able to with \<base-config cleartextTrafficPermitted="true">. However, we wouldn't want that in a production/release build while we would want the localhost configuration for BlobWriter always. Since BlobWriter may have to be configured using android:networkSecurityConfig and we always want it to work, and since that overrides android:usesCleartextTraffic, then perhaps one workaround could be to have separate android:networkSecurityConfig files for debug and release builds. The debug build would allow anything and the release build would only allow localhost. A downside is that in debug mode while developing there could be a clear text-related issue you wouldn't catch until the app is built in release mode and you try it out. Probably not very common to happen but still something to note.

It'd be more preferable if it was tied to just a live reload scenario like if the Ionic CLI could add the IP to the existing android:networkSecurityConfig if one exists as a temporary change while the live reload is in progress similar to how it temporary modifies other files during live reload to add android:usesCleartextTraffic and such.

diachedelic commented 3 years ago

I like the last suggestion, as it is possible the user has configured a network_security_config for other purposes than BlobWriter.

What's messed up is the WebView's ability to load data from the ionic serve running on the development machine

That's odd, because I use HMR all the time, though not via the Ionic CLI command. Perhaps this is different to Live Reload? I just set server.url in capacitor.config.js to my IP address and away I go. (I do however run a patched version of Capacitor which lets me access the filesystem via URLs, see ionic-team/capacitor#3433.)

KevinKelchen commented 3 years ago

Thank you for the crazy-fast reply! 😀

That's odd, because I use HMR all the time, though not via the Ionic CLI command. Perhaps this is different to Live Reload? I just set server.url in capacitor.config.js to my IP address and away I go. (I do however run a patched version of Capacitor which lets me access the filesystem via URLs, see ionic-team/capacitor#3433.)

Interesting. I would think if the Android WebView's server.url is an IP address with the android:networkSecurityConfig configured as mentioned in the README that your IP address over http would be blocked because it's clear text. 🤔 I'm not sure if the Capacitor patch would affect that or not.

diachedelic commented 3 years ago

Yes, I don't think the patch would affect that. However, I did just test plain old server.url on my Android running Chrome v88, and the app loaded OK.

Screen Shot 2021-02-04 at 11 15 28 am

Perhaps Ionic Live Reload uses XHR to transfer the file or something? I do not set usesCleartextTraffic in my AndroidManifest.xml.

KevinKelchen commented 3 years ago

Thanks for the reply and for trying that out and sharing the information!

Huh--interesting. If you haven't checked, I suppose it's possible that the final/built version of the AndroidManifest.xml could have somehow gotten android:usesCleartextTraffic added to it. To check, open the app's AndroidManifest.xml and then click the Merged Manifest button/tab.

It's also possible that your front-end dev server/tooling is configured differently somehow. I'm using the Ionic CLI which calls into the Angular CLI, FWIW.

diachedelic commented 3 years ago

Hmm no sign of usesCleartextTraffic in the merged manifest. I use Vue CLI for my HMR. I can only assume Ionic CLI is doing some proxying and it's going wrong there?

On 4 Feb 2021, at 12:06 pm, Kevin Kelchen notifications@github.com wrote:

Thanks for the reply and for trying that out and sharing the information!

Huh--interesting. If you haven't checked, I suppose it's possible that the final/built version of the AndroidManifest.xml could have somehow gotten android:usesCleartextTraffic added to it. To check, open the app's AndroidManifest.xml and then click the Merged Manifest button/tab.

It's also possible that your front-end dev server/tooling is configured differently somehow. I'm using the Ionic CLI which calls into the Angular CLI, FWIW.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/diachedelic/capacitor-blob-writer/issues/20#issuecomment-772979856, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACLZQE6VZXKMGO5TN6LCGLS5IBZVANCNFSM4PXH7EUQ.

diachedelic commented 2 years ago

I just ran into Kevin's issue. I have no idea why it only started happening now. It does appear that BlobWriter's network security config breaks Capacitor's live reload (as the network address of the web server is not listed as a <domain>).

KevinKelchen commented 2 years ago

Glad at least that I'm not the only one who bumped into this issue, @diachedelic! 😅

Do you think there might security concerns by allowing cleartext traffic to all domains?

FWIW, due to potential security concerns, I will describe the workaround we used in our app. It's basically what I described above:

I think you might be able to with \<base-config cleartextTrafficPermitted="true">. However, we wouldn't want that in a production/release build while we would want the localhost configuration for BlobWriter always. Since BlobWriter may have to be configured using android:networkSecurityConfig and we always want it to work, and since that overrides android:usesCleartextTraffic, then perhaps one workaround could be to have separate android:networkSecurityConfig files for debug and release builds. The debug build would allow anything and the release build would only allow localhost. A downside is that in debug mode while developing there could be a clear text-related issue you wouldn't catch until the app is built in release mode and you try it out. Probably not very common to happen but still something to note.

It's hard to believe it's been over a year since that comment, but so far we've had no issues with our workaround.

Workaround

Final thoughts

If this workaround doesn't become the official recommended configuration, that's probably ok. I at least wanted to provide the workaround as an option for anyone who might be leery of allowing cleartext traffic to all domains. 🙂

Thanks for all that you do to maintain this awesome and much-needed plugin, @diachedelic! 😀