improve peer verification

[marten-seemann]

Fixes #15. Fixes #16.

This PR fixes two issues by

• always verifying the peer's certificate chain and deriving the libp2p public key in the tls.Config.VerifyPeerCertificate callback
• saving the derived libp2p public key along the net.Conn. This is necessary since in the VerifyPeerCertificate callback the server has no idea which connection it is currently verifying.

Note that this PR will make it harder to reuse the code with QUIC, since in QUIC, there's no such thing as a (unique) net.Conn (as a client, you could reuse the same UDP connection to run multiple dials, and a server only listens on a single net.Conn). I suggest taking one step at a time and dealing with these issues later.

[Stebalien]

This sort of works but it's kind of messy and I'm pretty sure it's leaking connections. Instead of stashing the key in a global map, it may be simpler to use a one-off channel. That is:

1. Implement a Config() (*tls.Config, <-chan ic.PubKey) function on Identity. This generates a one-off config object and a one-off channel that returns the peer's key on successful authentication.
2. Change ConfigForPeer(peer.ID) *tls.Config to ConfigForPeer(peer.ID) (*tls.Config, <-chan ic.PubKey).
3. Make t.handshake take a <-chan ic.PublicKey` channel.

[Stebalien]

Won't this leak the conn?

[marten-seemann]

It is, but only because I forgot the defer statement that cleans up the map, as in SecureInbound.

[marten-seemann]

I agree that it's kind of messy, although maybe not messier than calculating the peer ID multiple times during the handshake.

1. Implement a Config() (*tls.Config, <-chan ic.PubKey) function on Identity. This generates a one-off config object and a one-off channel that returns the peer's key on successful authentication.

I'm not sure how this should work for a tls.Server. The server listens on a TCP connection, so it has to use the same tls.Config for every connection. The only way to generate a new tls.Config for every connection is by using the tls.Config.GetConfigForClient callback (which I'm doing in the PR). However, this callback has to return a tls.Config, so we can't put any metadata in there.

[Stebalien]

@marten-seemann can you generate a new config here: https://github.com/libp2p/go-libp2p-tls/pull/17/files#diff-0fecf8d8a1c3b52ef05e62d6a1bf6effR72?

[Stebalien]

Something like: #18 (needs testing).

[marten-seemann]

Thanks for the PR, @Stebalien. I don't think this needs additional tests, this should all be covered by the existing tests already.

[Stebalien]

LGTM modulo the duplicate import.

(note: I'm not married to the channel method, I'm just not a fan of mutable shared state)

[Stebalien]

Duplicate import.

[marten-seemann]

Why is this even allowed in Go... Fixed.

[marten-seemann]

Instead of a <-chan ci.PubKey we could just use a ci.PubKey. It doesn't make a difference, since we're doing a non-blocking read from that channel after the handshake anyway.

[marten-seemann]

On the other hand, the chan makes it immediately obvious that the ci.PubKey can't be accessed immediately, but is something that will be provided in the future. I'll merge the PR as it is for now, we can still change this later.

[Stebalien]

It doesn't make a difference, since we're doing a non-blocking read from that channel after the handshake anyway.

Yeah... It should technically be safe but only technically.