Skip to content

feat: Add support for using the original destination port for SNI proxy connections on Linux.#1029

Merged
zh-jq merged 1 commit intobytedance:masterfrom
SinghNanak:master
Feb 26, 2026
Merged

feat: Add support for using the original destination port for SNI proxy connections on Linux.#1029
zh-jq merged 1 commit intobytedance:masterfrom
SinghNanak:master

Conversation

@SinghNanak
Copy link
Contributor

I wanted to enforce SNI filtering transparently, I tried tcp_tproxy but it doesn't do that. It can be done with sni_proxy but it doesn't use the original port when connecting from Proxy --> Server. I have implemented "use_original_port", if its true cap_net_bind_service is required. I have tried to make sure it won't break any existing configs.

Usage

server:
  - name: my_transparent_proxy
    type: sni_proxy
    listen: "[::]:8443"
    escaper: sni_router
    auditor: default
    use_original_port: true

As sni_proxy supports both http and tls, I have redirected port 80 and 443 to a single listening port and when the proxy connects to upstream it uses the original ports like 80 for http and 443 for https.
If this functionality could be achieved previously kindly let me know.

I used following config previously.

server:
  - name: sni
    escaper: sni_router
    type: sni_proxy
    listen:
      address: "[::]:443"
  - name: port80
    type: plain_tcp_port
    listen: "[::]:80"
    server: sni

@CLAassistant
Copy link

CLAassistant commented Feb 24, 2026

CLA assistant check
All committers have signed the CLA.

}

#[cfg(target_os = "linux")]
pub fn original_dst(&self) -> io::Result<SocketAddr> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This won't be needed as you have already enabled the transparent socket option.

I would prefer to add a listen_transparent config option (which is linux specific) to sni_proxy server, and it will only enable the transparent socket option leaving other code unchanged.


#[inline]
pub(crate) fn server_port(&self) -> u16 {
self.cc_info.server_addr().port()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the same method as used in tcp_tproxy server, so no need to change this.

@zh-jq
Copy link
Collaborator

zh-jq commented Feb 24, 2026

One another way is to add a transparent_tcp_port server, which will set the transparent socket option by default, it can be chained in front of both tcp_tproxy server and sni_proxy server.

Copy link
Collaborator

@zh-jq zh-jq left a comment

Choose a reason for hiding this comment

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

Please squash all changes into one so I can rebase and merge this PR.

pub(crate) client_tcp_portmap: ProtocolPortMap,
pub(crate) extra_metrics_tags: Option<Arc<MetricTagMap>>,
pub(crate) allowed_sites: Option<HostMatch<Arc<SniHostConfig>>>,
pub(crate) listen_transparent: bool,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
pub(crate) listen_transparent: bool,
#[cfg(target_os = "linux")]
listen_transparent: bool,

Comment on lines +133 to +138

#[cfg(not(any(target_os = "linux", target_os = "android")))]
pub fn tcp_sock_incoming_cpu(&self) -> Option<usize> {
None
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

unrelated changes

}
Ok(())
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

unrelated changes

Comment on lines +83 to +84


Copy link
Collaborator

Choose a reason for hiding this comment

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

unrelated changes

client_tcp_portmap: ProtocolPortMap::tcp_client(),
extra_metrics_tags: None,
allowed_sites: None,
listen_transparent: false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

this also need the cfg guard

self.allowed_sites = Some(allowed_sites);
Ok(())
}
"listen_transparent" => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above

zh-jq
zh-jq previously approved these changes Feb 26, 2026
@codecov
Copy link

codecov bot commented Feb 26, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.68%. Comparing base (1560923) to head (70bb784).
⚠️ Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
g3proxy/src/config/server/sni_proxy/mod.rs 50.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1029      +/-   ##
==========================================
+ Coverage   67.66%   67.68%   +0.02%     
==========================================
  Files        1432     1432              
  Lines      136748   136756       +8     
==========================================
+ Hits        92527    92563      +36     
+ Misses      44221    44193      -28     
Flag Coverage Δ
g3bench 19.74% <ø> (ø)
g3keymess 20.76% <ø> (-0.01%) ⬇️
g3mkcert 26.73% <ø> (ø)
g3proxy 49.90% <50.00%> (+0.03%) ⬆️
g3statsd 5.58% <ø> (ø)
lib 57.86% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zh-jq
Copy link
Collaborator

zh-jq commented Feb 26, 2026

@SinghNanak clippy failed, please fix it as suggested.

@SinghNanak
Copy link
Contributor Author

@zh-jq Could you please re-review?

@zh-jq zh-jq merged commit d196f19 into bytedance:master Feb 26, 2026
119 of 120 checks passed
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.

3 participants