make peer verification use a channel

crypto.go

@@ -8,7 +8,6 @@ import (
 	"errors"
 	"fmt"
 	"math/big"
-	"net"
 	"time"
 
 	ic "github.com/libp2p/go-libp2p-crypto"
@@ -20,19 +19,17 @@ const certValidityPeriod = 180 * 24 * time.Hour
 
 // Identity is used to secure connections
 type Identity struct {
-	*tls.Config
+	config tls.Config
 }
 
 // NewIdentity creates a new identity
-func NewIdentity(
-	privKey ic.PrivKey,
-	verifiedPeerCallback func(net.Conn, ic.PubKey),
-) (*Identity, error) {
+func NewIdentity(privKey ic.PrivKey) (*Identity, error) {
 	key, cert, err := keyToCertificate(privKey)
 	if err != nil {
 		return nil, err
 	}
-	conf := &tls.Config{
+	return &Identity{
+		config: tls.Config{
 			MinVersion:         tls.VersionTLS13,
 			InsecureSkipVerify: true, // This is not insecure here. We will verify the cert chain ourselves.
 			ClientAuth:         tls.RequireAnyClientCert,
@@ -40,46 +37,37 @@ func NewIdentity(
 				Certificate: [][]byte{cert.Raw},
 				PrivateKey:  key,
 			}},
-	}
-	// When receiving the ClientHello, create a new tls.Config.
-	// This new config has a VerifyPeerCertificate set, which calls the verifiedPeerCallback
-	// when we derived the remote's public key from its certificate chain.
-	conf.GetConfigForClient = func(ch *tls.ClientHelloInfo) (*tls.Config, error) {
-		c := conf.Clone()
-		c.VerifyPeerCertificate = func(rawCerts [][]byte, _ [][]*x509.Certificate) error {
-			chain := make([]*x509.Certificate, len(rawCerts))
-			for i := 0; i < len(rawCerts); i++ {
-				cert, err := x509.ParseCertificate(rawCerts[i])
-				if err != nil {
-					return err
-				}
-				chain[i] = cert
-			}
-			pubKey, err := getRemotePubKey(chain)
-			if err != nil {
-				return err
-			}
-			verifiedPeerCallback(ch.Conn, pubKey)
-			return nil
-		}
-		return c, nil
-	}
-	return &Identity{conf}, nil
+			VerifyPeerCertificate: func(_ [][]byte, _ [][]*x509.Certificate) error {
+				panic("tls config not specialized for peer")
+			},
+		},
+	}, nil
+}
+
+// ConfigForAny is a short-hand for ConfigForPeer("").
+func (i *Identity) ConfigForAny() (*tls.Config, <-chan ic.PubKey) {
+	return i.ConfigForPeer("")
 }
 
-// ConfigForPeer creates a new tls.Config that verifies the peers certificate chain.
-// It should be used to create a new tls.Config before dialing.
+// ConfigForPeer creates a new single-use tls.Config that verifies the peer's
+// certificate chain and returns the peer's public key via the channel. If the
+// peer ID is empty, the returned config will accept any peer.
+//
+// It should be used to create a new tls.Config before securing either an
+// incoming or outgoing connection.
 func (i *Identity) ConfigForPeer(
 	remote peer.ID,
-	verifiedPeerCallback func(ic.PubKey),
-) *tls.Config {
+) (*tls.Config, <-chan ic.PubKey) {
+	keyCh := make(chan ic.PubKey, 1)
 	// We need to check the peer ID in the VerifyPeerCertificate callback.
 	// The tls.Config it is also used for listening, and we might also have concurrent dials.
 	// Clone it so we can check for the specific peer ID we're dialing here.
-	conf := i.Config.Clone()
+	conf := i.config.Clone()
 	// We're using InsecureSkipVerify, so the verifiedChains parameter will always be empty.
 	// We need to parse the certificates ourselves from the raw certs.
 	conf.VerifyPeerCertificate = func(rawCerts [][]byte, _ [][]*x509.Certificate) error {
+		defer close(keyCh)
+
 		chain := make([]*x509.Certificate, len(rawCerts))
 		for i := 0; i < len(rawCerts); i++ {
 			cert, err := x509.ParseCertificate(rawCerts[i])
@@ -88,17 +76,18 @@ func (i *Identity) ConfigForPeer(
 			}
 			chain[i] = cert
 		}
+
 		pubKey, err := getRemotePubKey(chain)
 		if err != nil {
 			return err
 		}
-		if !remote.MatchesPublicKey(pubKey) {
+		if remote != "" && !remote.MatchesPublicKey(pubKey) {
 			return errors.New("peer IDs don't match")
 		}
-		verifiedPeerCallback(pubKey)
+		keyCh <- pubKey
 		return nil
 	}
-	return conf
+	return conf, keyCh
 }
 
 // getRemotePubKey derives the remote's public key from the certificate chain.

transport.go

@@ -6,7 +6,6 @@ import (
 	"errors"
 	"net"
 	"os"
-	"sync"
 
 	cs "github.com/libp2p/go-conn-security"
 	ci "github.com/libp2p/go-libp2p-crypto"
@@ -29,9 +28,6 @@ type Transport struct {
 
 	localPeer peer.ID
 	privKey   ci.PrivKey
-
-	activeMutex sync.Mutex
-	active      map[net.Conn]ic.PubKey
 }
 
 // New creates a TLS encrypted transport
@@ -43,14 +39,9 @@ func New(key ci.PrivKey) (*Transport, error) {
 	t := &Transport{
 		localPeer: id,
 		privKey:   key,
-		active:    make(map[net.Conn]ic.PubKey),
 	}
 
-	identity, err := NewIdentity(key, func(conn net.Conn, pubKey ic.PubKey) {
-		t.activeMutex.Lock()
-		t.active[conn] = pubKey
-		t.activeMutex.Unlock()
-	})
+	identity, err := NewIdentity(key)
 	if err != nil {
 		return nil, err
 	}
@@ -62,15 +53,8 @@ var _ cs.Transport = &Transport{}
 
 // SecureInbound runs the TLS handshake as a server.
 func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn) (cs.Conn, error) {
-	defer func() {
-		t.activeMutex.Lock()
-		// only contains this connection if we successfully derived the client's key
-		delete(t.active, insecure)
-		t.activeMutex.Unlock()
-	}()
-
-	serv := tls.Server(insecure, t.identity.Config)
-	return t.handshake(ctx, insecure, serv)
+	config, keyCh := t.identity.ConfigForAny()
+	return t.handshake(ctx, tls.Server(insecure, config), keyCh)
 }
 
 // SecureOutbound runs the TLS handshake as a client.
@@ -81,19 +65,14 @@ func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn) (cs.Co
 // If the handshake fails, the server will close the connection. The client will
 // notice this after 1 RTT when calling Read.
 func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p peer.ID) (cs.Conn, error) {
-	verifiedCallback := func(pubKey ic.PubKey) {
-		t.activeMutex.Lock()
-		t.active[insecure] = pubKey
-		t.activeMutex.Unlock()
-	}
-	cl := tls.Client(insecure, t.identity.ConfigForPeer(p, verifiedCallback))
-	return t.handshake(ctx, insecure, cl)
+	config, keyCh := t.identity.ConfigForPeer(p)
+	return t.handshake(ctx, tls.Client(insecure, config), keyCh)
 }
 
 func (t *Transport) handshake(
 	ctx context.Context,
-	insecure net.Conn,
 	tlsConn *tls.Conn,
+	keyCh <-chan ci.PubKey,
 ) (cs.Conn, error) {
 	// There's no way to pass a context to tls.Conn.Handshake().
 	// See https://github.com/golang/go/issues/18482.
@@ -120,7 +99,15 @@ func (t *Transport) handshake(
 		}
 		return nil, err
 	}
-	conn, err := t.setupConn(insecure, tlsConn)
+
+	// Should be ready by this point, don't block.
+	var remotePubKey ic.PubKey
+	select {
+	case remotePubKey = <-keyCh:
+	default:
+	}
+
+	conn, err := t.setupConn(tlsConn, remotePubKey)
 	if err != nil {
 		// if the context was canceled, return the context error
 		if ctxErr := ctx.Err(); ctxErr != nil {
@@ -131,11 +118,7 @@ func (t *Transport) handshake(
 	return conn, nil
 }
 
-func (t *Transport) setupConn(insecure net.Conn, tlsConn *tls.Conn) (cs.Conn, error) {
-	t.activeMutex.Lock()
-	remotePubKey := t.active[insecure]
-	t.activeMutex.Unlock()
-
+func (t *Transport) setupConn(tlsConn *tls.Conn, remotePubKey ic.PubKey) (cs.Conn, error) {
 	if remotePubKey == nil {
 		return nil, errors.New("go-libp2p-tls BUG: expected remote pub key to be set")
 	}

transport_test.go

@@ -180,15 +180,15 @@ var _ = Describe("Transport", func() {
 
 	Context("invalid certificates", func() {
 		invalidateCertChain := func(identity *Identity) {
-			switch identity.Config.Certificates[0].PrivateKey.(type) {
+			switch identity.config.Certificates[0].PrivateKey.(type) {
 			case *rsa.PrivateKey:
 				key, err := rsa.GenerateKey(rand.Reader, 1024)
 				Expect(err).ToNot(HaveOccurred())
-				identity.Config.Certificates[0].PrivateKey = key
+				identity.config.Certificates[0].PrivateKey = key
 			case *ecdsa.PrivateKey:
 				key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
 				Expect(err).ToNot(HaveOccurred())
-				identity.Config.Certificates[0].PrivateKey = key
+				identity.config.Certificates[0].PrivateKey = key
 			default:
 				Fail("unexpected private key type")
 			}
@@ -206,7 +206,7 @@ var _ = Describe("Transport", func() {
 			Expect(err).ToNot(HaveOccurred())
 			cert2DER, err := x509.CreateCertificate(rand.Reader, tmpl, cert1, key2.Public(), key2)
 			Expect(err).ToNot(HaveOccurred())
-			identity.Config.Certificates = []tls.Certificate{{
+			identity.config.Certificates = []tls.Certificate{{
 				Certificate: [][]byte{cert2DER, cert1DER},
 				PrivateKey:  key2,
 			}}
@@ -222,7 +222,7 @@ var _ = Describe("Transport", func() {
 			Expect(err).ToNot(HaveOccurred())
 			cert, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key.Public(), key)
 			Expect(err).ToNot(HaveOccurred())
-			identity.Config.Certificates = []tls.Certificate{{
+			identity.config.Certificates = []tls.Certificate{{
 				Certificate: [][]byte{cert},
 				PrivateKey:  key,
 			}}