fix(proxy): stop emitting allowInsecure for domains with a valid cert - #34
Open
yarrrly wants to merge 1 commit into
Open
fix(proxy): stop emitting allowInsecure for domains with a valid cert#34yarrrly wants to merge 1 commit into
yarrrly wants to merge 1 commit into
Conversation
need_valid_ssl already yields False for the modes that are supposed to have a certificate, but the servernames branch overwrote it with True for every non-Reality domain. The guard never gates: re.split on an empty string returns [''], which is truthy, so the block runs for every domain. Xray-core has removed allowInsecure, so configs carrying it are now rejected. Refs hiddify/Hiddify-Manager#5438
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes hiddify/Hiddify-Manager#5438.
Xray-core removed
allowInsecure. In currentmainthe check is unconditional(
infra/conf/transport_security.go:361):The config is rejected as a whole, so a client bundling a recent core refuses the
subscription outright. #5438 reports it for Happ Plus, and the same core change
produced 2dust/v2rayN#8736, MHSanaei/3x-ui#3727 and
Openwrt-Passwall/openwrt-passwall2#999, all with the identical
The feature "allowInsecure" has been removederror.The panel puts the flag on every non-Reality TLS outbound, including domains it
classifies itself as needing a valid certificate.
sni_host_server_extractorcomputes the correct value and then discards it:
need_valid_sslis True for direct, cdn, worker, relay, auto_cdn_ip,old_xtls_direct and sub_link_only (
models/domain.py:136), so line 324 alreadyyields False for the domains that are supposed to have a certificate. The
condition below it reads as "this domain has servernames", but it never gates:
The block is entered for every domain, which makes line 324 dead code for
anything that is not Reality. The value then goes into
xrayjson.py:243(
allowInsecure),xray.py:209(allowInsecure=trueandinsecure=truein theURI),
clash.py:75,115(skip-cert-verify) andsingbox.py:216(insecure).This keeps line 324 as the source of truth, removes the
elsebranch, and makesthe condition test what it was written to test. Reality is still forced to False.
allow_insecureper domain mode, before and after:The workaround circulating in the #5438 thread only filters the empty split. That
leaves
cdnwith domain fronting on True, because that path takes theelsebranch on purpose. The reporter wrote in the thread that they were not going to
open a PR and preferred to leave the design to you; the diagnosis above is theirs.
There is a cost. A domain in a
need_valid_sslmode whose certificate is notCA-valid (ACME never issued, or a fronting SNI the CDN does not serve) now stops
connecting instead of silently skipping verification. On Xray those configs are
already rejected by the core, so nothing that works today gets worse. On sing-box
and mihomo, where
insecureandskip-cert-verifystill exist, such a setup doesbreak, and the operator has to fix the certificate or move the domain to a mode
that is not expected to have one.
fakekeepsallow_insecure = True, so its xray configs stay rejected by newcores. The replacement Xray points at is
pinnedPeerCertSha256/verifyPeerCertByName, which the panel could compute for the modes that cannot beCA-verified. That is a larger change and not part of this one.
Test
make testandmake lintare@echo skiphere and there is notests/directory, so there is nothing to extend. I checked the function by executing it
against each domain mode, which is where the table comes from.
On a running panel: with a
directdomain andcore_type = xray, open/<proxy_path>/<uuid>/xray/and look forallowInsecure. Before the change everyTLS outbound carries
"allowInsecure": true; after a panel restart they arefalse. Same for thevless://links in/<proxy_path>/<uuid>/sub/, whereallowInsecure=true&insecure=truedisappears from the query.