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

use a different set of default transports when PSK is enabled #1921

Merged
merged 1 commit into from
Dec 2, 2022

Conversation

RobotSail
Copy link
Contributor

Problem

The current behavior in go-libp2p is that the transports are provided through fx 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 of cfg.PSK == nil.

Signed-off-by: Oleg 97077423+RobotSail@users.noreply.github.com

Signed-off-by: Oleg <97077423+RobotSail@users.noreply.github.com>
@p-shahi p-shahi added this to the Best Effort Track milestone Dec 1, 2022
Copy link
Contributor

@marten-seemann marten-seemann left a 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.

@RobotSail
Copy link
Contributor Author

@marten-seemann Is there any transport that supports PSK which we could use? The PSK is important to our use-case in particular.

@marten-seemann
Copy link
Contributor

TCP and WebSocket support PSK. Would you mind describing your use case?

@RobotSail
Copy link
Contributor Author

RobotSail commented Dec 1, 2022

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.
The reason for this PR is so that we can initialize go-libp2p-relay-circuit with a PSK and be able to properly handle peer-to-peer circuits while maintaining privacy.

@marten-seemann
Copy link
Contributor

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.
What PSK allows you is to conveniently separate your nodes from other libp2p / IPFS nodes, but at the heavy cost of double-encryption and limiting your transports to TCP and WebSocket.

@RobotSail
Copy link
Contributor Author

@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.

@marten-seemann
Copy link
Contributor

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.

@marten-seemann
Copy link
Contributor

There’s an issue for this PR from a while ago: #1249.
For some reason, GitHub doesn’t allow me to link it to this PR.

Copy link
Contributor

@marten-seemann marten-seemann left a 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.

@marten-seemann marten-seemann changed the title Creates a PSK-friendly Default Option use a different set of default transports when PSK is enabled Dec 2, 2022
@marten-seemann marten-seemann linked an issue Dec 2, 2022 that may be closed by this pull request
@marten-seemann marten-seemann merged commit 2cc4de5 into libp2p:master Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

PrivateNetwork option broken by making QUIC a default
3 participants