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

quic: add support for TLS certificate templates & extra validation #2645

Closed
wants to merge 1 commit into from

Conversation

ivan4th
Copy link

@ivan4th ivan4th commented Nov 15, 2023

In some cases, it may be desirable to isolate some p2p networks, for example, test networks where nodes shouldn't be able to join the main network by accident. Talking about QUIC in particular, #1432 is probably not going to be implemented soon, so this PR is a stopgap solution, enabling custom TLS certificate templates for QUIC connections, and also a way to verify them. This way, the networks that need to be isolated might use a TLS extension (or something more hacky such as email, which is used in the test) to store a network-specific value that must be the same among the peers.

This is in no way a security feature, but rather just a safeguard against possible test network deployment mistakes.

I'm not entirely sure this approach is correct, e.g. it might be that connection gating should be used for the checks, but right now the underlying QUIC connection with its cert chain cannot be accessed from the gater, from what I see.

It might be also that instead of such a generic approach with cert templates and verification as exported transport options, we might just pass some "cookie" value which must be the same among peers as a QUIC transport option, if you think that is preferable, I can update this PR.

@marten-seemann
Copy link
Contributor

marten-seemann commented Nov 17, 2023

The problem is that we'd need something like this for every transport. This PR only covers QUIC (and TCP/TLS), but not TLS/Noise, nor WebSocket, WebTransport or WebRTC.

@ivan4th
Copy link
Author

ivan4th commented Nov 17, 2023

@marten-seemann Hmm, there's a way to pass TLS certs for WebSocket transport: transport/webtransport/
And also apparently there's something like that in webtransport:

// WithTLSClientConfig sets a custom tls.Config used for dialing.
// This option is most useful for setting a custom tls.Config.RootCAs certificate pool.
// When dialing a multiaddr that contains a /certhash component, this library will set InsecureSkipVerify and
// overwrite the VerifyPeerCertificate callback.
func WithTLSClientConfig(c *tls.Config) Option {
return func(t *transport) error {
t.tlsClientConf = c
return nil
}
}

With TCP, the goal of this PR is already achievable with Noise prologue, as we do in go-spacemesh: https://github.com/spacemeshos/go-spacemesh/blob/8b1a6ed6f83ed6d6bff1435151bbab3dd928994c/p2p/host.go#L209-L218 so I wanted a similar functionality for QUIC, but the go-libp2p code doesn't have enough extension points for that, so I tried to add some.
So we kind of already have an asymmetry here, with other transports providing some way to separate networks, but not QUIC.
Still, if we want some kind of common functionality here... Given that different encryption schemes are being used by different transports, and TLS certs do not apply to Noise-encrypted connections, what kind of facade should we have for this functionality? Maybe there should be something like NetworkCookie option for the Host that accepts byte sequence? (for TLS certs, we then can store it as an additional critical extension, or use it as an encryption key for P2P Peer ID TLS extension)

@ivan4th
Copy link
Author

ivan4th commented Nov 20, 2023

Ok, for now, I will proceed to add the NetworkCookie option for the Host that should work with any transport and will use the following semantics:

  • two peers can connect if they both have NetworkCookie specified and its argument is the same sequence of bytes, OR
  • none of these two peers have NetworkCookie specified.

The underlying mechanism will be a TLS extension in the case of TLS (perhaps extra data in the extension that is used for PeerID) and the prologue in the case of Noise.

The objective is primarily to prevent nodes from test networks from connecting to mainnets due to accidental misconfiguration. The supposed way to use this feature is by adding NetworkCookie for test networks so that each test network is uniquely identified by the byte sequence. NetworkCookie is not a replacement for a private network feature (which QUIC transport lacks).

Feel free to correct me if you don't consider this to be the right approach or if a better name for the option should be used.

@ivan4th
Copy link
Author

ivan4th commented Dec 1, 2023

@marten-seemann closing this in favor of #2658 which supports all transports

@ivan4th ivan4th closed this Dec 1, 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.

None yet

2 participants