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

fix: hong kong locale does not always mean china #1397

Merged
merged 1 commit into from Jul 24, 2023

Conversation

sgammon
Copy link
Contributor

@sgammon sgammon commented Jul 21, 2023

hello, thank you so much for this project 👍🏻

a previous commit by @92hackers set this conditional to return as true when the observed windows locale contains zh-hk, and then applies GOPROXY setting based on such detection.

the flag is supposed to Set GOPROXY for people in China (source code). if this flag is truthy, the proxy goproxy.cn is used, which would, in effect, redirect HK users traffic.

@@ -452,7 +452,7 @@ func isInChinaWindows() bool {
return false
}
// Check if output contains `zh-cn;`
return strings.Contains(out, "zh-cn;") || strings.Contains(out, "zh-hk;")
return strings.Contains(out, "zh-cn;")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change

@alexobviously
Copy link

alexobviously commented Jul 21, 2023

maintainers should observe that the user who changed this originally is a chinese troll, who basically does nothing but submit PRs to projects to undermine the independence of countries and regions that china claims (e.g. saadeghi/daisyui#2150)

@sgammon
Copy link
Contributor Author

sgammon commented Jul 21, 2023

@alexobviously good notes thank you for adding, i do think such behavior would be unexpected for users in hk without other context

a previous commit set this conditional to return as `true` when
the detected windows locale `zh-hk` is detected, and then applies
`GOPROXY` setting based on such detection.

the flag is supposed to _Set GOPROXY for people in China_. if this
flag is truthy, the proxy `goproxy.cn` is used, which would, in
effect, redirect HK users traffic.
@@ -466,7 +466,7 @@ func isInChina() bool {
}
if strings.HasPrefix(out, prefix) {
out = out[len(prefix):]
return strings.HasPrefix(out, "zh_CN") || strings.HasPrefix(out, "zh_HK")
return strings.HasPrefix(out, "zh_CN")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

second pointed out by anonymous help 👍🏻 thank you

@sgammon
Copy link
Contributor Author

sgammon commented Jul 21, 2023

for perspective, while hong kong is part of china in an administrative capacity, this pr addresses dependency traffic, not politics; we are the makers of pkgst and other tools which help secure software supply chains, which is why this caught my eye after a friend sent it along.

while this redirection of traffic doesn't constitute a vulnerability on its own, it could certainly be a delivery mechanism for injecting software into a supply chain or at least for identifying users when they otherwise wouldn't have made a choice to be visible.

the goproxy.cn functionality isn't removed, in fact it is still in place behind a flag and auto-proxy as before. i have no evidence that goproxy.cn is anything but a great dependency proxy like we are and this pr should not reflect on them.

@sgammon
Copy link
Contributor Author

sgammon commented Jul 21, 2023

if you were physically in hong kong, or abroad with your windows locale set to hong kong, you would expect your traffic to be redirected @mcg-anon ? please, this pr is not about politics.

@sgammon
Copy link
Contributor Author

sgammon commented Jul 21, 2023

@mcg-anon cloudflare has a chinese subsidiary network with in-country dns resolution https://www.cloudflare.com/china-network/

cloudflare does not "sniff" traffic; providers are compelled to operate within the legal framework of their host jurisdiction. this is no different than any chinese CDN or any CDN anywhere.

locale is not necessarily a representation of physical location, either, which is why this behavior might be confusing. a windows locale set to hong kong anywhere in the world would see this behavior if i understand the code correctly.

@sgammon
Copy link
Contributor Author

sgammon commented Jul 21, 2023

@mcg-anon it cannot, because cloudflare in front of proxy.golang.org is end-to-end TLS verified. then again, the PR does not install cloudflare, it removes unexpected use of another proxy.

cloudflare also has nodes local to hong kong, so my intent there is to cover users who might be affected by this change.

(as in, they cannot rewrite dependencies without breaking signatures or hashes)

@sgammon
Copy link
Contributor Author

sgammon commented Jul 21, 2023

if this boils down to trust in one operator or another, you're right, people should have a choice. my advocacy of cloudflare here is in the light of presenting non-chinese options for users who may not be located in china. it should be up to users whether their traffic is proxied.

@sgammon
Copy link
Contributor Author

sgammon commented Jul 21, 2023

@mcg-anon fine, nobody has to use it -- as i said, it's up to the user to trust or not trust networks. end-to-end TLS and dependency hashing or signing (a-la sigstore) are great ideas and people should use them. that is irrelevant to this pr, though.

@sgammon sgammon changed the title fix: hong kong is not in china fix: hong kong locale does not always mean china Jul 21, 2023
@sgammon
Copy link
Contributor Author

sgammon commented Jul 21, 2023

@mcg-anon i've updated the pr title in order to try to be less inflammatory, i hope you see this (in addition to the code change not installing a preferred provider) as an olive branch

@alex-kinokon
Copy link

alex-kinokon commented Jul 21, 2023

@mcg-anon Legally speaking Hong Kong is considered extraterritorial by China, so you are wrong even on the technical level. The Great Firewall only applies to mainland China and not Hong Kong, so there’s no need to apply the proxy for the latter.

@sgammon
Copy link
Contributor Author

sgammon commented Jul 21, 2023

@mcg-anon this is the important point with regard to the pr:

The Great Firewall only applies to mainland China and not Hong Kong, so there’s no need to apply the proxy for the latter.

@alex-kinokon
Copy link

@mcg-anon Look, Hong Kong is a constitutionally separate jurisdiction with an entirely different legal system where most Chinese laws don’t apply. Facebook, Twitter, Google and most foreign websites are accessible in Hong Kong but not in mainland China. Even the Great Firewall heavily censors Internet traffic from Hong Kong, so I’m not sure why you want them to share a network rule?

@alex-kinokon
Copy link

Geopolitics aside, using locale to determine proxy rules doesn’t seem like a good idea to me. Plenty of developers whose first language is not English prefer to use their computer in en-us for the better DX, and even if you have zh-cn as your preferred locale, it’s entirely possible that you live outside China (e.g. Singapore or Malaysia).

@alex-kinokon
Copy link

Just like Bavaria in Germany.

I already explained to you in painstaking details why these two are not even remotely comparable, twice, and you just pretended you didn’t hear any of that. Go read up what people wrote and address their points before further commenting.

@sgammon
Copy link
Contributor Author

sgammon commented Jul 21, 2023

@mcg-anon

You have to make sure that people in Hong Kong use the correct Chinese proxy even when they have their locale set to en-us.

You do not have to, because

The Great Firewall only applies to mainland China and not Hong Kong, so there’s no need to apply the proxy for the latter.

If Hong Kong users want to use such a proxy,

the goproxy.cn functionality isn't removed, in fact it is still in place behind a flag and auto-proxy as before.

@vinicioslc
Copy link

need many taiwanese guys in conversation

@codecov-commenter
Copy link

Codecov Report

Patch coverage has no change and project coverage change: -0.02 ⚠️

Comparison is base (74fdb39) 90.28% compared to head (c46d95f) 90.27%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1397      +/-   ##
==========================================
- Coverage   90.28%   90.27%   -0.02%     
==========================================
  Files          24       24              
  Lines        9240     9240              
==========================================
- Hits         8342     8341       -1     
- Misses        730      731       +1     
  Partials      168      168              

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@xushiwei xushiwei merged commit 104daa8 into goplus:main Jul 24, 2023
15 checks passed
@sgammon
Copy link
Contributor Author

sgammon commented Jul 24, 2023

thank you @xushiwei

@qiukeren
Copy link
Contributor

qiukeren commented Jul 24, 2023

if you do not let the politicals be politicals,

this issue must be like this:

A vietnam troll undermine the independence of hmong, Khmerkrom,Việt Nam Cộng Hòa, FULRO, that Vietnam claims.
A brazil troll undermine the independence of Paraná, Santa Catarina and Rio Grande that brazil claims and need many Paraná, Santa Catarina and Rio Grande guys in conversation.
A canada troll undermine the independence of quebec that canada claims.
A england troll undermine the independence of north ireland and scotland that england claims.
A thailand troll undermine the independence of Barisan Revolusi Nasional that thailand claims.
A japan troll undermine the independence of Ryukyu islands that japan claims.
A french troll undermine the independence of corsica that french claims.
A ukraine troll undermine the independence of Donetsk ugansk Zaporizhzhia Kherson that ukraine claims.
A Germany troll undermine the independence of Bavaria that Germany claims.
A ______ troll undermine the independence of ______ that ____ claims, need many _____ guys in conversation

just Solve the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants