Skip to content
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

Determine if cleartextTraffic flag is required on Android #1060

Closed
IzzySoft opened this issue Jan 15, 2024 · 8 comments
Closed

Determine if cleartextTraffic flag is required on Android #1060

IzzySoft opened this issue Jan 15, 2024 · 8 comments
Assignees
Labels
fixed in upcoming release Indicates that an issue has been fixed, and will be released in the next version
Milestone

Comments

@IzzySoft
Copy link
Contributor

My updater (having been enhanced with additional security checks recently) just informed me:

repo/com.hjiangsu.thunder_36.apk declares flags: usesCleartextTraffic

which refers to your definition here. By default, that option is disabled, so it must have been enabled explicitly at one point for some reason. Is that reason still standing (if so, what is it?) – or can that line be removed? I don't think there's a Lemmy instance out there not secured by HTTPS/SSL, so I'd guess Thunder won't need to make any request using "insecure channels"?

Thanks in advance for clarification!

@hjiangsu
Copy link
Member

I managed to trace this down to this PR: #104.

The main reason for keeping this would be if a Lemmy post contains an external link to a HTTP site (or potentially a HTTP link which redirects to its HTTPS counterpart) I believe we might be able to remove this, but users may encounter this issue in the future (although it might be rare)

@micahmo
Copy link
Member

micahmo commented Jan 15, 2024

Is it possible that that error was specifically related to the fact that we were loading and rendering web page links in-app using WebView? And now that we mainly just support embedded or external browsers, we'd push the burden on them to enforce/redirect insecure requests?

@hjiangsu
Copy link
Member

Is it possible that that error was specifically related to the fact that we were loading and rendering web page links in-app using WebView?

I think that might've been the case. I'm not sure if this issue will still happen with the custom_tabs, or if custom_tabs understands to automatically redirect HTTP -> HTTPS

@micahmo
Copy link
Member

micahmo commented Jan 15, 2024

if custom_tabs understands to automatically redirect

My understanding is that custom tabs isn't doing anything except to invoke that feature in your default browser. It should be up to Firefox/Chrome/Safari to either redirect or show an appropriate warning.

I'd be tempted to take out this line, especially if it's a requirement for Izzy (or to avoid a bad label 😆), and see what happens.

@IzzySoft
Copy link
Contributor Author

It's not a strict requirement – this is just a "warning" that something might be not as it should. If there's a good reason to need the flag, I can put it on the allow-list for Thunder. So take your time to make sure you really don't need it before removing it. Nothing is "handed up" for display yet, currently it's just "internal handling" (to detect possible "dangerous" things and report them to the corresponding app authors to check). At some later point I might make this visible to visitors of the app's page on my website, but that's unlikely to happen very soon. First all checks need to be in place and confirmed working as intended, which will at least take until spring.

@hjiangsu
Copy link
Member

hjiangsu commented Jan 17, 2024

What we can do here is release a nightly build with this change to see if it affects anyone on the using those builds. From here, we'll get a good sense of whether or not usesCleartextTraffic is needed.

I don't expect to be releasing a general build anytime soon (~1 month) so that should give us enough data to go off of! I'll rename this issue to a more suitable title

@hjiangsu hjiangsu changed the title cleartextTraffic? Determine if cleartextTraffic flag is required on Android Jan 17, 2024
@hjiangsu hjiangsu self-assigned this Jan 26, 2024
@hjiangsu hjiangsu added the in-progress Indicates that an issue is currently being worked on label Jan 26, 2024
@hjiangsu hjiangsu added this to the v0.2.9 milestone Jan 26, 2024
@hjiangsu hjiangsu added fixed in upcoming release Indicates that an issue has been fixed, and will be released in the next version and removed in-progress Indicates that an issue is currently being worked on labels Jan 29, 2024
@hjiangsu
Copy link
Member

usesCleartextTraffic has now been removed from the pre-releases and the general releases with 0.2.9. I'll close this issue but feel free to comment on here if there is anything else that needs to be addressed!

@IzzySoft
Copy link
Contributor Author

Thanks! Confirmed it gone:

image

If you want to let that DEPENDENCY_INFO_BLOCK disappear as well:

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed in upcoming release Indicates that an issue has been fixed, and will be released in the next version
Projects
None yet
Development

No branches or pull requests

3 participants