-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
use a different set of default transports when PSK is enabled #1921
Conversation
Signed-off-by: Oleg <97077423+RobotSail@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a bigger problem here, namely that most transports don't support PSK at all. We just made it explicit for QUIC, but not for the others (see #1432 for context).
The correct fix would be to throw an error when constructing transports that don't support PSK, when a PSK is configured. Unfortunately, that's not a trivial change.
The even more correct fix would be to get rid of PSK, as it's clearly not useful if it can't be used with most of our transports.
@marten-seemann Is there any transport that supports PSK which we could use? The PSK is important to our use-case in particular. |
TCP and WebSocket support PSK. Would you mind describing your use case? |
We're building an IPFS Operator to run in Kubernetes. One of our key features is orchestrating large-scale private networks, but we're using the libp2p circuit relay in order to allow individual IPFS nodes to be dialable by each other due to the amount of network abstractions that take place in a cloud environment. |
I'm not sure I understand how this helps maintain privacy. There's no such thing as unencrypted libp2p connections, so all connections are private anyway. |
@marten-seemann We're doing this so that enterprise users can create their own private IPFS networks that would run at scale but without advertising their CIDs to the global DHT. I think it would be fine if the transports are limited to TCP & WebSocket, but we haven't been able to try running it at scale yet to actually see what the drawbacks would be. |
Thank you for the explanation. I think this should be part of a larger discussion around PSK, and I’ve opened libp2p/specs#489 for that. |
There’s an issue for this PR from a while ago: #1249. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, this PR is fine, and an improvement independent from the larger discussion about the future of pnet.
Problem
The current behavior in
go-libp2p
is that the transports are provided throughfx
by default when no other options are specified.QUIC is listed in the defaults but does not support PSK usage, so creating a new app will always result in an error.
The way that libp2p is currently bootstrapped does not allow for dynamically enabling/disabling transports, so we cannot disable QUIC once it's been provided as a value group.
Solution
This PR adds a new default transport option that gets activated when
cfg.Transports == nil && cfg.PSK != nil
. The new option is mutually exclusive with the current one on the condition ofcfg.PSK == nil
.Signed-off-by: Oleg 97077423+RobotSail@users.noreply.github.com