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

feat: add client alpn extension #42

Closed
wants to merge 3 commits into from
Closed

Conversation

WoLuo-dev
Copy link
Contributor

In most of time, when you send client hello to server while viewing website, client will set alpn. This PR get alpn from args, and set alpn in tls_config.

@ihciah
Copy link
Owner

ihciah commented Nov 16, 2022

Thanks for your PR!
I think maybe you can make it more generic by adding a config for finger print anti-detection(I wrote some thought about it in my blog post for reference). Except for alpn, there are many other tls extensions which can be part of finger print...

@WoLuo-dev
Copy link
Contributor Author

Thanks for your PR! I think maybe you can make it more generic by adding a config for finger print anti-detection(I wrote some thought about it in my blog post for reference). Except for alpn, there are many other tls extensions which can be part of finger print...

I read your blog and I agree with your opinion. But it might be kind of hard to make rustls's fingerprint totally same as some browser. Not only because we don't have library like utls ,but also rustls seems just have 9 Cipher Suites , but most browser have more.

So I think it's impossible to use rustls as utls now. This should be done by other threads sometime. Adding alpn is what we can do, and may be necessary for some handshake servers(well, That's why this pr is created).

src/client.rs Outdated
.with_safe_defaults()
.with_root_certificates(root_store)
.with_no_client_auth();
// Set ALPN
if alpn.len() != 0 {
tls_config.alpn_protocols = vec![alpn.as_bytes().to_vec()];
Copy link
Owner

Choose a reason for hiding this comment

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

I suggest put something like TLSConfig(with impl Default) in ShadowTlsClient::new, and put alpns inside it so we can extend it easily.
Also, since there can be multiple ALPN, maybe something like Vec<Vec<u8>>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a good idea to put all tls extension config in a struct, but i don't understand about impl Default.
After we create a struct and put config in it, ShadowTlsClient::new may use this struct as a parameter, so why we need default value?

Copy link
Contributor Author

@WoLuo-dev WoLuo-dev Dec 22, 2022

Choose a reason for hiding this comment

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

About multiple ALPN, it's easy to change it into Vec<Vec<u8>>, but i have no idea about how to get a vector from args. I need some help or change the way get config.

@ihciah
Copy link
Owner

ihciah commented Jan 18, 2023

Closed because I manually rebased onto the master and pushed it.
Thanks!

@ihciah ihciah closed this Jan 18, 2023
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.

2 participants