search

ihciah / shadow-tls

A proxy to expose real tls handshake to the firewall
https://www.ihcblog.com/a-better-tls-obfs-proxy/
MIT License
2.3k stars 265 forks source link

Feat: Add config file support #90

Closed erfan-khadem closed 1 year ago

erfan-khadem commented 1 year ago

Closes https://github.com/ihciah/shadow-tls/issues/76

ihciah commented 1 year ago

Thanks for your contribution! Cound you add some optional markers for fields like fastopen or disable_nodelay? It's better to make json field optional with the same default value if it is optional in command line parameter. Also, it is better to use toml than json. Toml is better for human reading and editing.

erfan-khadem commented 1 year ago

For sure toml is easier to read, but json tools are ubiquitous (like jq) and people who want to make shell scripts to make setting up the VPN easier are more important IMO. Also many existing tools use json, so non technical people who have learned to setup other tools will also have an easier time.

In any case, if it is a hard requirement, sure. I will do it. It won't take much more than changing from serde_json to toml and changing a line or two.

ihciah commented 1 year ago

For sure toml is easier to read, but json tools are ubiquitous (like jq) and people who want to make shell scripts to make setting up the VPN easier are more important IMO. Also many existing tools use json, so non technical people who have learned to setup other tools will also have an easier time.

In any case, if it is a hard requirement, sure. I will do it. It won't take much more than changing from serde_json to toml and changing a line or two.

It's ok to use json :) Using toml is just a suggestion. It can also be implemented as pre-read the first non-space char and using json or toml.

I think the optional's default value should be the same as in clap. So maybe only one default false function need to be defined and applied to all bool parameters(like fastopen, which should be false by default): fastopen, v3, strict, disable_nodelay, and others?

Thanks!

erfan-khadem commented 1 year ago

It's ok to use json :) Using toml is just a suggestion. It can also be implemented as pre-read the first non-space char and using json or toml.

Using more than one standard is just going to complicate stuff. I was thinking of the same thing (detecting the config type by using the file extension) but I decided against it. You know, KISS.

I think the optional's default value should be the same as in clap. So maybe only one default false function need to be defined and applied to all bool parameters(like fastopen, which should be false by default): fastopen, v3, strict, disable_nodelay, and others?

You're right. what the hell was I thinking while writing that code :/ On the other hand though, can we start to change to more secure defaults? I know it should happen on a major release, but wouldn't it be better if we do it sooner?

ihciah commented 1 year ago

Using more than one standard is just going to complicate stuff. I was thinking of the same thing (detecting the config type by using the file extension) but I decided against it. You know, KISS.

Sure, json only is ok.

On the other hand though, can we start to change to more secure defaults? I know it should happen on a major release, but wouldn't it be better if we do it sooner?

If we can defend against some active attack easily, even we may not enable it, the attackers tend to not attack it. With the current default settings, users are free to use some domains with tls1.2 only.

There's a plan that support udp in next major version(tcp is compatiable). The current default works well, so I don't think there is a need to rush to release a new major version. Except for incompatiable changes, It should come with some new features.

erfan-khadem commented 1 year ago

There's a plan that support udp in next major version(tcp is compatiable). The current default works well, so I don't think there is a need to rush to release a new major version. Except for incompatiable changes, It should come with some new features.

Can you explain this plan a little bit more to me? Or create a roadmap so that everybody can see where we are heading. RN I am very busy with university exams, but once I have a little bit of free time I would love to work on this project.

ihciah commented 1 year ago

There's a plan that support udp in next major version(tcp is compatiable). The current default works well, so I don't think there is a need to rush to release a new major version. Except for incompatiable changes, It should come with some new features.

Can you explain this plan a little bit more to me? Or create a roadmap so that everybody can see where we are heading. RN I am very busy with university exams, but once I have a little bit of free time I would love to work on this project.

Thanks! The udp support plan is a little bit ambigious now. The protocol is like what here in tcp. The traffic will look like QUIC, and the identity is hidden in SCID. We can try to make it clear later.