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

Add support for custom ALPN #9529

Closed
wants to merge 1 commit into from
Closed

Add support for custom ALPN #9529

wants to merge 1 commit into from

Conversation

misery
Copy link

@misery misery commented May 6, 2024

Use "bep-relay" as default but allow custom ALPNs if the relay server is used for something unrelated to "bep-relay".

Purpose

This allows to use the perfect syncthing-relay for another usage for different applications without to have conflicts with syncthing application itself and custom application.

Testing

Used "testutil" of strelaysrv and saw that it shows an error in log output if the ALPN does not match. Also used wireshark to see the TLS alert "No application protocol".

Authorship

André Klitzing aklitzing@gmail.com

Use "bep-relay" as default but allow custom ALPNs if the relay server
is used for something unrelated to "bep-relay".
@calmh
Copy link
Member

calmh commented May 16, 2024

I'm not sure we want to build general relay infrastructure for non-Syncthing use cases though? I mean, the proposed diff is trivial and harmless for sure, but I question the expansion of scope it implies...

@misery
Copy link
Author

misery commented May 16, 2024

Well, maybe syncthing will have benefit, too. The developer community could grow with it.

But I doubt that there will be a big impact because of this.

@misery
Copy link
Author

misery commented Jun 4, 2024

@calmh Did you think about it? :-)

@calmh
Copy link
Member

calmh commented Jun 4, 2024

Yeah. Given the unclear implications and the fact that this is just a debug print that doesn't actually prevent you from doing what you want, this is a no from me.

@calmh calmh closed this Jun 4, 2024
@misery
Copy link
Author

misery commented Jun 4, 2024

It will prevent the client connection as the "NextProtos" in tlsConfig will force it.

It -alpn is defined the testutil cannot connect.

$ ./testutil -test
2024/06/04 14:05:13 main.go:48: ID: LO46K72-PQQ6RGH-CNRKKMS-YGVKCPM-WO5HCCJ-HIOHWS4-DFSZZ2A-IGFSVAD
2024/06/04 14:05:13 main.go:115: FAIL: getting invitation: remote error: tls: no application protocol

@calmh
Copy link
Member

calmh commented Jun 4, 2024

Ah, my bad. Nonetheless.

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

2 participants