From cacd2a4016b0146c246902164f316a9cf115ddcd Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 29 Jul 2019 08:26:25 +0700 Subject: [PATCH 1/2] use the handshake logic from go-libp2p-tls --- p2p/transport/quic/conn_test.go | 79 +++---------- p2p/transport/quic/crypto.go | 123 -------------------- p2p/transport/quic/libp2pquic_suite_test.go | 6 + p2p/transport/quic/listener.go | 22 +++- p2p/transport/quic/listener_test.go | 1 + p2p/transport/quic/transport.go | 45 +++---- 6 files changed, 57 insertions(+), 219 deletions(-) delete mode 100644 p2p/transport/quic/crypto.go diff --git a/p2p/transport/quic/conn_test.go b/p2p/transport/quic/conn_test.go index 8b86af207..ef2430783 100644 --- a/p2p/transport/quic/conn_test.go +++ b/p2p/transport/quic/conn_test.go @@ -4,10 +4,9 @@ import ( "bytes" "context" "crypto/rand" - "crypto/rsa" - "crypto/tls" - "crypto/x509" + "fmt" "io/ioutil" + mrand "math/rand" "time" ic "github.com/libp2p/go-libp2p-core/crypto" @@ -26,12 +25,26 @@ var _ = Describe("Connection", func() { ) createPeer := func() (peer.ID, ic.PrivKey) { - key, err := rsa.GenerateKey(rand.Reader, 1024) - Expect(err).ToNot(HaveOccurred()) - priv, err := ic.UnmarshalRsaPrivateKey(x509.MarshalPKCS1PrivateKey(key)) + var priv ic.PrivKey + var err error + switch mrand.Int() % 4 { + case 0: + fmt.Fprintf(GinkgoWriter, " using an ECDSA key: ") + priv, _, err = ic.GenerateECDSAKeyPair(rand.Reader) + case 1: + fmt.Fprintf(GinkgoWriter, " using an RSA key: ") + priv, _, err = ic.GenerateRSAKeyPair(1024, rand.Reader) + case 2: + fmt.Fprintf(GinkgoWriter, " using an Ed25519 key: ") + priv, _, err = ic.GenerateEd25519Key(rand.Reader) + case 3: + fmt.Fprintf(GinkgoWriter, " using an secp256k1 key: ") + priv, _, err = ic.GenerateSecp256k1Key(rand.Reader) + } Expect(err).ToNot(HaveOccurred()) id, err := peer.IDFromPrivateKey(priv) Expect(err).ToNot(HaveOccurred()) + fmt.Fprintln(GinkgoWriter, id.Pretty()) return id, priv } @@ -52,11 +65,6 @@ var _ = Describe("Connection", func() { return <-addrChan, connChan } - // modify the cert chain such that verificiation will fail - invalidateCertChain := func(tlsConf *tls.Config) { - tlsConf.Certificates[0].Certificate = [][]byte{tlsConf.Certificates[0].Certificate[0]} - } - BeforeEach(func() { serverID, serverKey = createPeer() clientID, clientKey = createPeer() @@ -141,55 +149,6 @@ var _ = Describe("Connection", func() { Consistently(serverConnChan).ShouldNot(Receive()) }) - It("fails if the client presents an invalid cert chain", func() { - serverTransport, err := NewTransport(serverKey) - Expect(err).ToNot(HaveOccurred()) - serverAddr, serverConnChan := runServer(serverTransport, "/ip4/127.0.0.1/udp/0/quic") - - clientTransport, err := NewTransport(clientKey) - invalidateCertChain(clientTransport.(*transport).tlsConf) - Expect(err).ToNot(HaveOccurred()) - conn, err := clientTransport.Dial(context.Background(), serverAddr, serverID) - Expect(err).ToNot(HaveOccurred()) - Eventually(func() bool { return conn.IsClosed() }).Should(BeTrue()) - Consistently(serverConnChan).ShouldNot(Receive()) - }) - - It("fails if the server presents an invalid cert chain", func() { - serverTransport, err := NewTransport(serverKey) - invalidateCertChain(serverTransport.(*transport).tlsConf) - Expect(err).ToNot(HaveOccurred()) - serverAddr, serverConnChan := runServer(serverTransport, "/ip4/127.0.0.1/udp/0/quic") - - clientTransport, err := NewTransport(clientKey) - Expect(err).ToNot(HaveOccurred()) - _, err = clientTransport.Dial(context.Background(), serverAddr, serverID) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("CRYPTO_ERROR")) - Consistently(serverConnChan).ShouldNot(Receive()) - }) - - It("keeps accepting connections after a failed connection attempt", func() { - serverTransport, err := NewTransport(serverKey) - Expect(err).ToNot(HaveOccurred()) - serverAddr, serverConnChan := runServer(serverTransport, "/ip4/127.0.0.1/udp/0/quic") - - // first dial with an invalid cert chain - clientTransport1, err := NewTransport(clientKey) - invalidateCertChain(clientTransport1.(*transport).tlsConf) - Expect(err).ToNot(HaveOccurred()) - _, err = clientTransport1.Dial(context.Background(), serverAddr, serverID) - Expect(err).ToNot(HaveOccurred()) - Consistently(serverConnChan).ShouldNot(Receive()) - - // then dial with a valid client - clientTransport2, err := NewTransport(clientKey) - Expect(err).ToNot(HaveOccurred()) - _, err = clientTransport2.Dial(context.Background(), serverAddr, serverID) - Expect(err).ToNot(HaveOccurred()) - Eventually(serverConnChan).Should(Receive()) - }) - It("dials to two servers at the same time", func() { serverID2, serverKey2 := createPeer() diff --git a/p2p/transport/quic/crypto.go b/p2p/transport/quic/crypto.go deleted file mode 100644 index 2ce698680..000000000 --- a/p2p/transport/quic/crypto.go +++ /dev/null @@ -1,123 +0,0 @@ -package libp2pquic - -import ( - "crypto/ecdsa" - "crypto/elliptic" - "crypto/rand" - "crypto/tls" - "crypto/x509" - "errors" - "math/big" - "time" - - "github.com/gogo/protobuf/proto" - ic "github.com/libp2p/go-libp2p-core/crypto" - pb "github.com/libp2p/go-libp2p-core/crypto/pb" -) - -// mint certificate selection is broken. -const hostname = "quic.ipfs" - -const alpn string = "libp2p" - -const certValidityPeriod = 180 * 24 * time.Hour - -func generateConfig(privKey ic.PrivKey) (*tls.Config, error) { - key, hostCert, err := keyToCertificate(privKey) - if err != nil { - return nil, err - } - // The ephemeral key used just for a couple of connections (or a limited time). - ephemeralKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - if err != nil { - return nil, err - } - // Sign the ephemeral key using the host key. - // This is the only time that the host's private key of the peer is needed. - // Note that this step could be done asynchronously, such that a running node doesn't need access its private key at all. - certTemplate := &x509.Certificate{ - DNSNames: []string{hostname}, - SerialNumber: big.NewInt(1), - NotBefore: time.Now().Add(-24 * time.Hour), - NotAfter: time.Now().Add(certValidityPeriod), - } - certDER, err := x509.CreateCertificate(rand.Reader, certTemplate, hostCert, ephemeralKey.Public(), key) - if err != nil { - return nil, err - } - cert, err := x509.ParseCertificate(certDER) - if err != nil { - return nil, err - } - return &tls.Config{ - ServerName: hostname, - InsecureSkipVerify: true, // This is not insecure here. We will verify the cert chain ourselves. - ClientAuth: tls.RequireAnyClientCert, - Certificates: []tls.Certificate{{ - Certificate: [][]byte{cert.Raw, hostCert.Raw}, - PrivateKey: ephemeralKey, - }}, - NextProtos: []string{alpn}, - }, nil -} - -func getRemotePubKey(chain []*x509.Certificate) (ic.PubKey, error) { - if len(chain) != 2 { - return nil, errors.New("expected 2 certificates in the chain") - } - pool := x509.NewCertPool() - pool.AddCert(chain[1]) - if _, err := chain[0].Verify(x509.VerifyOptions{Roots: pool}); err != nil { - return nil, err - } - remotePubKey, err := x509.MarshalPKIXPublicKey(chain[1].PublicKey) - if err != nil { - return nil, err - } - return ic.UnmarshalRsaPublicKey(remotePubKey) -} - -func keyToCertificate(sk ic.PrivKey) (interface{}, *x509.Certificate, error) { - sn, err := rand.Int(rand.Reader, big.NewInt(1<<62)) - if err != nil { - return nil, nil, err - } - tmpl := &x509.Certificate{ - SerialNumber: sn, - NotBefore: time.Now().Add(-24 * time.Hour), - NotAfter: time.Now().Add(certValidityPeriod), - IsCA: true, - BasicConstraintsValid: true, - } - - var publicKey, privateKey interface{} - keyBytes, err := sk.Bytes() - if err != nil { - return nil, nil, err - } - pbmes := new(pb.PrivateKey) - if err := proto.Unmarshal(keyBytes, pbmes); err != nil { - return nil, nil, err - } - switch pbmes.GetType() { - case pb.KeyType_RSA: - k, err := x509.ParsePKCS1PrivateKey(pbmes.GetData()) - if err != nil { - return nil, nil, err - } - publicKey = &k.PublicKey - privateKey = k - // TODO: add support for ECDSA - default: - return nil, nil, errors.New("unsupported key type for TLS") - } - certDER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, publicKey, privateKey) - if err != nil { - return nil, nil, err - } - cert, err := x509.ParseCertificate(certDER) - if err != nil { - return nil, nil, err - } - return privateKey, cert, nil -} diff --git a/p2p/transport/quic/libp2pquic_suite_test.go b/p2p/transport/quic/libp2pquic_suite_test.go index 9675f7499..2bb5d416c 100644 --- a/p2p/transport/quic/libp2pquic_suite_test.go +++ b/p2p/transport/quic/libp2pquic_suite_test.go @@ -1,6 +1,8 @@ package libp2pquic import ( + mrand "math/rand" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -11,3 +13,7 @@ func TestLibp2pQuicTransport(t *testing.T) { RegisterFailHandler(Fail) RunSpecs(t, "libp2p QUIC Transport Suite") } + +var _ = BeforeSuite(func() { + mrand.Seed(GinkgoRandomSeed()) +}) diff --git a/p2p/transport/quic/listener.go b/p2p/transport/quic/listener.go index 2e39854dd..0d1b83532 100644 --- a/p2p/transport/quic/listener.go +++ b/p2p/transport/quic/listener.go @@ -8,14 +8,13 @@ import ( ic "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/peer" tpt "github.com/libp2p/go-libp2p-core/transport" + p2ptls "github.com/libp2p/go-libp2p-tls" quic "github.com/lucas-clemente/quic-go" ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr-net" ) -var quicListenAddr = quic.ListenAddr - // A listener listens for QUIC connections. type listener struct { quicListener quic.Listener @@ -28,7 +27,16 @@ type listener struct { var _ tpt.Listener = &listener{} -func newListener(addr ma.Multiaddr, transport tpt.Transport, localPeer peer.ID, key ic.PrivKey, tlsConf *tls.Config) (tpt.Listener, error) { +func newListener(addr ma.Multiaddr, transport tpt.Transport, localPeer peer.ID, key ic.PrivKey, identity *p2ptls.Identity) (tpt.Listener, error) { + var tlsConf tls.Config + tlsConf.GetConfigForClient = func(_ *tls.ClientHelloInfo) (*tls.Config, error) { + // return a tls.Config that verifies the peer's certificate chain. + // Note that since we have no way of associating an incoming QUIC connection with + // the peer ID calculated here, we don't actually receive the peer's public key + // from the key chan. + conf, _ := identity.ConfigForAny() + return conf, nil + } lnet, host, err := manet.DialArgs(addr) if err != nil { return nil, err @@ -41,7 +49,7 @@ func newListener(addr ma.Multiaddr, transport tpt.Transport, localPeer peer.ID, if err != nil { return nil, err } - ln, err := quic.Listen(conn, tlsConf, quicConfig) + ln, err := quic.Listen(conn, &tlsConf, quicConfig) if err != nil { return nil, err } @@ -75,7 +83,11 @@ func (l *listener) Accept() (tpt.CapableConn, error) { } func (l *listener) setupConn(sess quic.Session) (tpt.CapableConn, error) { - remotePubKey, err := getRemotePubKey(sess.ConnectionState().PeerCertificates) + // The tls.Config used to establish this connection already verified the certificate chain. + // Since we don't have any way of knowing which tls.Config was used though, + // we have to re-determine the peer's identity here. + // Therefore, this is expected to never fail. + remotePubKey, err := p2ptls.PubKeyFromCertChain(sess.ConnectionState().PeerCertificates) if err != nil { return nil, err } diff --git a/p2p/transport/quic/listener_test.go b/p2p/transport/quic/listener_test.go index bf0435f1a..44d1c4ab1 100644 --- a/p2p/transport/quic/listener_test.go +++ b/p2p/transport/quic/listener_test.go @@ -30,6 +30,7 @@ var _ = Describe("Listener", func() { Context("listening on the right address", func() { It("returns the address it is listening on", func() { localAddr, err := ma.NewMultiaddr("/ip4/127.0.0.1/udp/0/quic") + Expect(err).ToNot(HaveOccurred()) ln, err := t.Listen(localAddr) Expect(err).ToNot(HaveOccurred()) netAddr := ln.Addr() diff --git a/p2p/transport/quic/transport.go b/p2p/transport/quic/transport.go index ebf571200..e6fc143e2 100644 --- a/p2p/transport/quic/transport.go +++ b/p2p/transport/quic/transport.go @@ -2,9 +2,6 @@ package libp2pquic import ( "context" - "crypto/tls" - "crypto/x509" - "errors" "fmt" "net" "sync" @@ -12,6 +9,7 @@ import ( ic "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/peer" tpt "github.com/libp2p/go-libp2p-core/transport" + p2ptls "github.com/libp2p/go-libp2p-tls" quic "github.com/lucas-clemente/quic-go" ma "github.com/multiformats/go-multiaddr" @@ -74,7 +72,7 @@ func (c *connManager) createConn(network, host string) (net.PacketConn, error) { type transport struct { privKey ic.PrivKey localPeer peer.ID - tlsConf *tls.Config + identity *p2ptls.Identity connManager *connManager } @@ -86,7 +84,7 @@ func NewTransport(key ic.PrivKey) (tpt.Transport, error) { if err != nil { return nil, err } - tlsConf, err := generateConfig(key) + identity, err := p2ptls.NewIdentity(key) if err != nil { return nil, err } @@ -94,7 +92,7 @@ func NewTransport(key ic.PrivKey) (tpt.Transport, error) { return &transport{ privKey: key, localPeer: localPeer, - tlsConf: tlsConf, + identity: identity, connManager: &connManager{}, }, nil } @@ -113,30 +111,7 @@ func (t *transport) Dial(ctx context.Context, raddr ma.Multiaddr, p peer.ID) (tp if err != nil { return nil, err } - var remotePubKey ic.PubKey - tlsConf := t.tlsConf.Clone() - // 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. - tlsConf.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 - } - var err error - remotePubKey, err = getRemotePubKey(chain) - if err != nil { - return err - } - if !p.MatchesPublicKey(remotePubKey) { - return errors.New("peer IDs don't match") - } - return nil - } + tlsConf, keyCh := t.identity.ConfigForPeer(p) sess, err := quic.DialContext(ctx, pconn, addr, host, tlsConf, quicConfig) if err != nil { return nil, err @@ -145,6 +120,14 @@ func (t *transport) Dial(ctx context.Context, raddr ma.Multiaddr, p peer.ID) (tp if err != nil { return nil, err } + + // Should be ready by this point, don't block. + var remotePubKey ic.PubKey + select { + case remotePubKey = <-keyCh: + default: + } + return &conn{ sess: sess, transport: t, @@ -164,7 +147,7 @@ func (t *transport) CanDial(addr ma.Multiaddr) bool { // Listen listens for new QUIC connections on the passed multiaddr. func (t *transport) Listen(addr ma.Multiaddr) (tpt.Listener, error) { - return newListener(addr, t, t.localPeer, t.privKey, t.tlsConf) + return newListener(addr, t, t.localPeer, t.privKey, t.identity) } // Proxy returns true if this transport proxies. From 5d493b46b352c6927077ddc21cdc8b2d316d89da Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 1 Aug 2019 09:20:19 +0700 Subject: [PATCH 2/2] add an error check that we actually received the peer's public key --- p2p/transport/quic/transport.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/p2p/transport/quic/transport.go b/p2p/transport/quic/transport.go index e6fc143e2..5b217087b 100644 --- a/p2p/transport/quic/transport.go +++ b/p2p/transport/quic/transport.go @@ -2,6 +2,7 @@ package libp2pquic import ( "context" + "errors" "fmt" "net" "sync" @@ -127,6 +128,9 @@ func (t *transport) Dial(ctx context.Context, raddr ma.Multiaddr, p peer.ID) (tp case remotePubKey = <-keyCh: default: } + if remotePubKey == nil { + return nil, errors.New("go-libp2p-quic-transport BUG: expected remote pub key to be set") + } return &conn{ sess: sess,