From 82f9c87252708a75ebf6d0f204ed438f636681c0 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sat, 24 Nov 2018 15:33:50 +0700 Subject: [PATCH 01/51] initial commit --- p2p/security/tls/conn.go | 37 +++++ p2p/security/tls/crypto.go | 140 +++++++++++++++++++ p2p/security/tls/libp2p_tls_suite_test.go | 13 ++ p2p/security/tls/transport.go | 81 +++++++++++ p2p/security/tls/transport_test.go | 161 ++++++++++++++++++++++ 5 files changed, 432 insertions(+) create mode 100644 p2p/security/tls/conn.go create mode 100644 p2p/security/tls/crypto.go create mode 100644 p2p/security/tls/libp2p_tls_suite_test.go create mode 100644 p2p/security/tls/transport.go create mode 100644 p2p/security/tls/transport_test.go diff --git a/p2p/security/tls/conn.go b/p2p/security/tls/conn.go new file mode 100644 index 0000000000..f4eb600089 --- /dev/null +++ b/p2p/security/tls/conn.go @@ -0,0 +1,37 @@ +package libp2ptls + +import ( + "crypto/tls" + + cs "github.com/libp2p/go-conn-security" + ic "github.com/libp2p/go-libp2p-crypto" + peer "github.com/libp2p/go-libp2p-peer" +) + +type conn struct { + *tls.Conn + + localPeer peer.ID + privKey ic.PrivKey + + remotePeer peer.ID + remotePubKey ic.PubKey +} + +var _ cs.Conn = &conn{} + +func (c *conn) LocalPeer() peer.ID { + return c.localPeer +} + +func (c *conn) LocalPrivateKey() ic.PrivKey { + return c.privKey +} + +func (c *conn) RemotePeer() peer.ID { + return c.remotePeer +} + +func (c *conn) RemotePublicKey() ic.PubKey { + return c.remotePubKey +} diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go new file mode 100644 index 0000000000..1a6812a18e --- /dev/null +++ b/p2p/security/tls/crypto.go @@ -0,0 +1,140 @@ +package libp2ptls + +import ( + "crypto/rand" + "crypto/tls" + "crypto/x509" + "errors" + "math/big" + "time" + + "github.com/gogo/protobuf/proto" + ic "github.com/libp2p/go-libp2p-crypto" + pb "github.com/libp2p/go-libp2p-crypto/pb" + peer "github.com/libp2p/go-libp2p-peer" +) + +// Identity is used to secure connections +type Identity struct { + *tls.Config +} + +// NewIdentity creates a new identity +func NewIdentity(privKey ic.PrivKey) (*Identity, error) { + conf, err := generateConfig(privKey) + if err != nil { + return nil, err + } + return &Identity{conf}, nil +} + +// ConfigForPeer creates a new tls.Config that verifies the peers certificate chain. +// It should be used to create a new tls.Config before dialing. +func (i *Identity) ConfigForPeer(remote peer.ID) *tls.Config { + // 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() + // 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 { + 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 + } + if !remote.MatchesPublicKey(pubKey) { + return errors.New("peer IDs don't match") + } + return nil + } + return conf +} + +// KeyFromChain takes a chain of x509.Certificates and returns the peer's public key. +func KeyFromChain(chain []*x509.Certificate) (ic.PubKey, error) { + return getRemotePubKey(chain) +} + +const certValidityPeriod = 180 * 24 * time.Hour + +func generateConfig(privKey ic.PrivKey) (*tls.Config, error) { + key, cert, err := keyToCertificate(privKey) + if err != nil { + return nil, err + } + return &tls.Config{ + InsecureSkipVerify: true, // This is not insecure here. We will verify the cert chain ourselves. + ClientAuth: tls.RequireAnyClientCert, + Certificates: []tls.Certificate{{ + Certificate: [][]byte{cert.Raw}, + PrivateKey: key, + }}, + }, nil +} + +func getRemotePubKey(chain []*x509.Certificate) (ic.PubKey, error) { + if len(chain) != 1 { + return nil, errors.New("expected one certificates in the chain") + } + pool := x509.NewCertPool() + pool.AddCert(chain[0]) + if _, err := chain[0].Verify(x509.VerifyOptions{Roots: pool}); err != nil { + return nil, err + } + remotePubKey, err := x509.MarshalPKIXPublicKey(chain[0].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), + } + + 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/security/tls/libp2p_tls_suite_test.go b/p2p/security/tls/libp2p_tls_suite_test.go new file mode 100644 index 0000000000..3303ed3030 --- /dev/null +++ b/p2p/security/tls/libp2p_tls_suite_test.go @@ -0,0 +1,13 @@ +package libp2ptls + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestLibp2pTLS(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "libp2p TLS Suite") +} diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go new file mode 100644 index 0000000000..16941bc059 --- /dev/null +++ b/p2p/security/tls/transport.go @@ -0,0 +1,81 @@ +package libp2ptls + +import ( + "context" + "crypto/tls" + "net" + + cs "github.com/libp2p/go-conn-security" + ci "github.com/libp2p/go-libp2p-crypto" + peer "github.com/libp2p/go-libp2p-peer" +) + +// ID is the protocol ID (used when negotiating with multistream) +const ID = "/tls/1.0.0" + +// Transport constructs secure communication sessions for a peer. +type Transport struct { + identity *Identity + + localPeer peer.ID + privKey ci.PrivKey +} + +// New creates a TLS encrypted transport +func New(key ci.PrivKey) (*Transport, error) { + id, err := peer.IDFromPrivateKey(key) + if err != nil { + return nil, err + } + identity, err := NewIdentity(key) + if err != nil { + return nil, err + } + return &Transport{ + identity: identity, + localPeer: id, + privKey: key, + }, nil +} + +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) { + serv := tls.Server(insecure, t.identity.Config) + // TODO: use the ctx + // see https://github.com/golang/go/issues/18482 + if err := serv.Handshake(); err != nil { + return nil, err + } + return t.setupConn(serv) +} + +// SecureOutbound runs the TLS handshake as a client. +func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p peer.ID) (cs.Conn, error) { + cl := tls.Client(insecure, t.identity.ConfigForPeer(p)) + // TODO: use the ctx + // see https://github.com/golang/go/issues/18482 + if err := cl.Handshake(); err != nil { + return nil, err + } + return t.setupConn(cl) +} + +func (t *Transport) setupConn(tlsConn *tls.Conn) (cs.Conn, error) { + remotePubKey, err := KeyFromChain(tlsConn.ConnectionState().PeerCertificates) + if err != nil { + return nil, err + } + remotePeerID, err := peer.IDFromPublicKey(remotePubKey) + if err != nil { + return nil, err + } + return &conn{ + Conn: tlsConn, + localPeer: t.localPeer, + privKey: t.privKey, + remotePeer: remotePeerID, + remotePubKey: remotePubKey, + }, nil +} diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go new file mode 100644 index 0000000000..1a55433e13 --- /dev/null +++ b/p2p/security/tls/transport_test.go @@ -0,0 +1,161 @@ +package libp2ptls + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "net" + + cs "github.com/libp2p/go-conn-security" + ic "github.com/libp2p/go-libp2p-crypto" + peer "github.com/libp2p/go-libp2p-peer" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("Transport", func() { + var ( + serverKey, clientKey ic.PrivKey + serverID, clientID peer.ID + ) + + createPeer := func() (peer.ID, ic.PrivKey) { + key, err := rsa.GenerateKey(rand.Reader, 1024) + Expect(err).ToNot(HaveOccurred()) + priv, err := ic.UnmarshalRsaPrivateKey(x509.MarshalPKCS1PrivateKey(key)) + Expect(err).ToNot(HaveOccurred()) + id, err := peer.IDFromPrivateKey(priv) + Expect(err).ToNot(HaveOccurred()) + return id, priv + } + + connect := func() (net.Conn, net.Conn) { + ln, err := net.Listen("tcp", "localhost:0") + Expect(err).ToNot(HaveOccurred()) + defer ln.Close() + serverConnChan := make(chan net.Conn) + go func() { + defer GinkgoRecover() + conn, err := ln.Accept() + Expect(err).ToNot(HaveOccurred()) + serverConnChan <- conn + }() + conn, err := net.Dial("tcp", ln.Addr().String()) + Expect(err).ToNot(HaveOccurred()) + return conn, <-serverConnChan + } + + // modify the cert chain such that verificiation will fail + invalidateCertChain := func(identity *Identity) { + key, err := rsa.GenerateKey(rand.Reader, 1024) + Expect(err).ToNot(HaveOccurred()) + identity.Config.Certificates[0].PrivateKey = key + } + + BeforeEach(func() { + serverID, serverKey = createPeer() + clientID, clientKey = createPeer() + }) + + It("handshakes", func() { + clientTransport, err := New(clientKey) + Expect(err).ToNot(HaveOccurred()) + serverTransport, err := New(serverKey) + Expect(err).ToNot(HaveOccurred()) + + clientInsecureConn, serverInsecureConn := connect() + + serverConnChan := make(chan cs.Conn) + go func() { + defer GinkgoRecover() + serverConn, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) + Expect(err).ToNot(HaveOccurred()) + serverConnChan <- serverConn + }() + clientConn, err := clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) + Expect(err).ToNot(HaveOccurred()) + var serverConn cs.Conn + Eventually(serverConnChan).Should(Receive(&serverConn)) + defer clientConn.Close() + defer serverConn.Close() + Expect(clientConn.LocalPeer()).To(Equal(clientID)) + Expect(serverConn.LocalPeer()).To(Equal(serverID)) + Expect(clientConn.LocalPrivateKey()).To(Equal(clientKey)) + Expect(serverConn.LocalPrivateKey()).To(Equal(serverKey)) + Expect(clientConn.RemotePeer()).To(Equal(serverID)) + Expect(serverConn.RemotePeer()).To(Equal(clientID)) + Expect(clientConn.RemotePublicKey()).To(Equal(serverKey.GetPublic())) + Expect(serverConn.RemotePublicKey()).To(Equal(clientKey.GetPublic())) + // exchange some data + _, err = serverConn.Write([]byte("foobar")) + Expect(err).ToNot(HaveOccurred()) + b := make([]byte, 6) + _, err = clientConn.Read(b) + Expect(err).ToNot(HaveOccurred()) + Expect(string(b)).To(Equal("foobar")) + }) + + It("fails if the peer ID doesn't match", func() { + thirdPartyID, _ := createPeer() + + serverTransport, err := New(serverKey) + Expect(err).ToNot(HaveOccurred()) + clientTransport, err := New(clientKey) + Expect(err).ToNot(HaveOccurred()) + + clientInsecureConn, serverInsecureConn := connect() + + go func() { + defer GinkgoRecover() + _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("tls: bad certificate")) + }() + // dial, but expect the wrong peer ID + _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, thirdPartyID) + Expect(err).To(MatchError("peer IDs don't match")) + }) + + It("fails if the client presents an invalid cert chain", func() { + serverTransport, err := New(serverKey) + Expect(err).ToNot(HaveOccurred()) + clientTransport, err := New(clientKey) + Expect(err).ToNot(HaveOccurred()) + invalidateCertChain(clientTransport.identity) + + clientInsecureConn, serverInsecureConn := connect() + + go func() { + defer GinkgoRecover() + _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("crypto/rsa: verification error")) + }() + + _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("tls: bad certificate")) + }) + + It("fails if the server presents an invalid cert chain", func() { + serverTransport, err := New(serverKey) + Expect(err).ToNot(HaveOccurred()) + invalidateCertChain(serverTransport.identity) + clientTransport, err := New(clientKey) + Expect(err).ToNot(HaveOccurred()) + + clientInsecureConn, serverInsecureConn := connect() + + go func() { + defer GinkgoRecover() + _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("tls: bad certificate")) + }() + + _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("crypto/rsa: verification error")) + }) +}) From b70779f12c0760d1718c98f7faa7af2afca5b3c1 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sat, 24 Nov 2018 15:34:45 +0700 Subject: [PATCH 02/51] add a license --- p2p/security/tls/LICENSE.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 p2p/security/tls/LICENSE.md diff --git a/p2p/security/tls/LICENSE.md b/p2p/security/tls/LICENSE.md new file mode 100644 index 0000000000..a94e82cceb --- /dev/null +++ b/p2p/security/tls/LICENSE.md @@ -0,0 +1,7 @@ +Copyright 2018 Marten Seemann + +Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. From 09e1e2ad8c68f9433447423e4d17c5b8fea2ce08 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sat, 24 Nov 2018 16:41:08 +0700 Subject: [PATCH 03/51] fix handshake tests --- p2p/security/tls/transport_test.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index 1a55433e13..5e50868c95 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -106,15 +106,18 @@ var _ = Describe("Transport", func() { clientInsecureConn, serverInsecureConn := connect() + done := make(chan struct{}) go func() { defer GinkgoRecover() _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("tls: bad certificate")) + close(done) }() // dial, but expect the wrong peer ID _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, thirdPartyID) Expect(err).To(MatchError("peer IDs don't match")) + Eventually(done).Should(BeClosed()) }) It("fails if the client presents an invalid cert chain", func() { @@ -126,16 +129,19 @@ var _ = Describe("Transport", func() { clientInsecureConn, serverInsecureConn := connect() + done := make(chan struct{}) go func() { defer GinkgoRecover() _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("crypto/rsa: verification error")) + close(done) }() _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("tls: bad certificate")) + Eventually(done).Should(BeClosed()) }) It("fails if the server presents an invalid cert chain", func() { @@ -147,15 +153,18 @@ var _ = Describe("Transport", func() { clientInsecureConn, serverInsecureConn := connect() + done := make(chan struct{}) go func() { defer GinkgoRecover() _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("tls: bad certificate")) + // TLS returns a weird error here: "remote error: tls: unexpected message" + close(done) }() _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("crypto/rsa: verification error")) + Eventually(done).Should(BeClosed()) }) }) From 63843cc17e2d4d4dcb105cf8f846c657125fdae5 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 28 Nov 2018 14:06:54 +0700 Subject: [PATCH 04/51] close the underlying connection when the context is canceled --- p2p/security/tls/transport.go | 32 +++++++++++++++++++++--- p2p/security/tls/transport_test.go | 40 ++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index 16941bc059..40181f9a8a 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -43,8 +43,20 @@ 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) { serv := tls.Server(insecure, t.identity.Config) - // TODO: use the ctx - // see https://github.com/golang/go/issues/18482 + + // There's no way to pass a context to tls.Conn.Handshake(). + // See https://github.com/golang/go/issues/18482. + // Close the connection instead. + done := make(chan struct{}) + defer close(done) + go func() { + select { + case <-done: + case <-ctx.Done(): + insecure.Close() + } + }() + if err := serv.Handshake(); err != nil { return nil, err } @@ -54,8 +66,20 @@ func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn) (cs.Co // SecureOutbound runs the TLS handshake as a client. func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p peer.ID) (cs.Conn, error) { cl := tls.Client(insecure, t.identity.ConfigForPeer(p)) - // TODO: use the ctx - // see https://github.com/golang/go/issues/18482 + + // There's no way to pass a context to tls.Conn.Handshake(). + // See https://github.com/golang/go/issues/18482. + // Close the connection instead. + done := make(chan struct{}) + defer close(done) + go func() { + select { + case <-done: + case <-ctx.Done(): + insecure.Close() + } + }() + if err := cl.Handshake(); err != nil { return nil, err } diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index 5e50868c95..94b61ea3a6 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -96,6 +96,46 @@ var _ = Describe("Transport", func() { Expect(string(b)).To(Equal("foobar")) }) + It("fails when the context of the outgoing connection is canceled", func() { + clientTransport, err := New(clientKey) + Expect(err).ToNot(HaveOccurred()) + serverTransport, err := New(serverKey) + Expect(err).ToNot(HaveOccurred()) + + clientInsecureConn, serverInsecureConn := connect() + + go func() { + defer GinkgoRecover() + _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) + Expect(err).To(HaveOccurred()) + }() + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err = clientTransport.SecureOutbound(ctx, clientInsecureConn, serverID) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("use of closed network connection")) + }) + + It("fails when the context of the incoming connection is canceled", func() { + clientTransport, err := New(clientKey) + Expect(err).ToNot(HaveOccurred()) + serverTransport, err := New(serverKey) + Expect(err).ToNot(HaveOccurred()) + + clientInsecureConn, serverInsecureConn := connect() + + go func() { + defer GinkgoRecover() + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err := serverTransport.SecureInbound(ctx, serverInsecureConn) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("use of closed network connection")) + }() + _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) + Expect(err).To(HaveOccurred()) + }) + It("fails if the peer ID doesn't match", func() { thirdPartyID, _ := createPeer() From 1c0f10c904dd928dc069cccd434e8dbaf8011a61 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 29 Nov 2018 09:36:22 +0700 Subject: [PATCH 05/51] return the context cancelation error --- p2p/security/tls/transport.go | 44 ++++++++++++++++-------------- p2p/security/tls/transport_test.go | 6 ++-- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index 40181f9a8a..6bd5ba4400 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -43,30 +43,23 @@ 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) { serv := tls.Server(insecure, t.identity.Config) - - // There's no way to pass a context to tls.Conn.Handshake(). - // See https://github.com/golang/go/issues/18482. - // Close the connection instead. - done := make(chan struct{}) - defer close(done) - go func() { - select { - case <-done: - case <-ctx.Done(): - insecure.Close() - } - }() - - if err := serv.Handshake(); err != nil { - return nil, err - } - return t.setupConn(serv) + return t.handshake(ctx, insecure, serv) } // SecureOutbound runs the TLS handshake as a client. func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p peer.ID) (cs.Conn, error) { cl := tls.Client(insecure, t.identity.ConfigForPeer(p)) + return t.handshake(ctx, insecure, cl) +} +func (t *Transport) handshake( + ctx context.Context, + // in Go 1.10, we need to close the underlying net.Conn + // in Go 1.11 this was fixed, and tls.Conn.Close() works as well + insecure net.Conn, + tlsConn *tls.Conn, +) (cs.Conn, error) { + errChan := make(chan error, 2) // There's no way to pass a context to tls.Conn.Handshake(). // See https://github.com/golang/go/issues/18482. // Close the connection instead. @@ -76,14 +69,23 @@ func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p pee select { case <-done: case <-ctx.Done(): + errChan <- ctx.Err() insecure.Close() } }() - if err := cl.Handshake(); err != nil { - return nil, err + if err := tlsConn.Handshake(); err != nil { + // if the context was canceled, return the context error + errChan <- err + return nil, <-errChan + } + conn, err := t.setupConn(tlsConn) + if err != nil { + // if the context was canceled, return the context error + errChan <- err + return nil, <-errChan } - return t.setupConn(cl) + return conn, nil } func (t *Transport) setupConn(tlsConn *tls.Conn) (cs.Conn, error) { diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index 94b61ea3a6..bc4d5b3813 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -112,8 +112,7 @@ var _ = Describe("Transport", func() { ctx, cancel := context.WithCancel(context.Background()) cancel() _, err = clientTransport.SecureOutbound(ctx, clientInsecureConn, serverID) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("use of closed network connection")) + Expect(err).To(MatchError(context.Canceled)) }) It("fails when the context of the incoming connection is canceled", func() { @@ -129,8 +128,7 @@ var _ = Describe("Transport", func() { ctx, cancel := context.WithCancel(context.Background()) cancel() _, err := serverTransport.SecureInbound(ctx, serverInsecureConn) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("use of closed network connection")) + Expect(err).To(MatchError(context.Canceled)) }() _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) Expect(err).To(HaveOccurred()) From eaf15fd98bfcbc788b0508c6ec8dcac8e1827c7e Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 29 Nov 2018 20:38:57 +0700 Subject: [PATCH 06/51] simplify returning of context cancellation errors --- p2p/security/tls/transport.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index 6bd5ba4400..5a75296f1a 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -59,7 +59,6 @@ func (t *Transport) handshake( insecure net.Conn, tlsConn *tls.Conn, ) (cs.Conn, error) { - errChan := make(chan error, 2) // There's no way to pass a context to tls.Conn.Handshake(). // See https://github.com/golang/go/issues/18482. // Close the connection instead. @@ -69,21 +68,24 @@ func (t *Transport) handshake( select { case <-done: case <-ctx.Done(): - errChan <- ctx.Err() insecure.Close() } }() if err := tlsConn.Handshake(); err != nil { // if the context was canceled, return the context error - errChan <- err - return nil, <-errChan + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } + return nil, err } conn, err := t.setupConn(tlsConn) if err != nil { // if the context was canceled, return the context error - errChan <- err - return nil, <-errChan + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } + return nil, err } return conn, nil } From 955b8056cb4ace524eac46ca500a07cf64a43445 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 11 Jan 2019 13:51:35 +0700 Subject: [PATCH 07/51] add support for ECDSA keys --- p2p/security/tls/crypto.go | 19 ++++++++- p2p/security/tls/libp2p_tls_suite_test.go | 5 +++ p2p/security/tls/transport_test.go | 49 ++++++++++++++++++----- 3 files changed, 61 insertions(+), 12 deletions(-) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index 1a6812a18e..583fe39c14 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "crypto/x509" "errors" + "fmt" "math/big" "time" @@ -93,7 +94,14 @@ func getRemotePubKey(chain []*x509.Certificate) (ic.PubKey, error) { if err != nil { return nil, err } - return ic.UnmarshalRsaPublicKey(remotePubKey) + switch chain[0].PublicKeyAlgorithm { + case x509.RSA: + return ic.UnmarshalRsaPublicKey(remotePubKey) + case x509.ECDSA: + return ic.UnmarshalECDSAPublicKey(remotePubKey) + default: + return nil, fmt.Errorf("unexpected public key algorithm: %d", chain[0].PublicKeyAlgorithm) + } } func keyToCertificate(sk ic.PrivKey) (interface{}, *x509.Certificate, error) { @@ -124,7 +132,14 @@ func keyToCertificate(sk ic.PrivKey) (interface{}, *x509.Certificate, error) { } publicKey = &k.PublicKey privateKey = k - // TODO: add support for ECDSA + case pb.KeyType_ECDSA: + k, err := x509.ParseECPrivateKey(pbmes.GetData()) + if err != nil { + return nil, nil, err + } + publicKey = &k.PublicKey + privateKey = k + // TODO: add support for Ed25519 default: return nil, nil, errors.New("unsupported key type for TLS") } diff --git a/p2p/security/tls/libp2p_tls_suite_test.go b/p2p/security/tls/libp2p_tls_suite_test.go index 3303ed3030..e0e6785862 100644 --- a/p2p/security/tls/libp2p_tls_suite_test.go +++ b/p2p/security/tls/libp2p_tls_suite_test.go @@ -1,6 +1,7 @@ package libp2ptls import ( + mrand "math/rand" "testing" . "github.com/onsi/ginkgo" @@ -11,3 +12,7 @@ func TestLibp2pTLS(t *testing.T) { RegisterFailHandler(Fail) RunSpecs(t, "libp2p TLS Suite") } + +var _ = BeforeSuite(func() { + mrand.Seed(GinkgoRandomSeed()) +}) diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index bc4d5b3813..d06326d90f 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -2,9 +2,12 @@ package libp2ptls import ( "context" + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" "crypto/rsa" - "crypto/x509" + "fmt" + mrand "math/rand" "net" cs "github.com/libp2p/go-conn-security" @@ -21,10 +24,18 @@ var _ = Describe("Transport", 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)) - Expect(err).ToNot(HaveOccurred()) + var priv ic.PrivKey + if mrand.Int()%2 == 0 { + fmt.Fprintln(GinkgoWriter, " using an ECDSA key") + var err error + priv, _, err = ic.GenerateECDSAKeyPair(rand.Reader) + Expect(err).ToNot(HaveOccurred()) + } else { + fmt.Fprintln(GinkgoWriter, " using an RSA key") + var err error + priv, _, err = ic.GenerateRSAKeyPair(1024, rand.Reader) + Expect(err).ToNot(HaveOccurred()) + } id, err := peer.IDFromPrivateKey(priv) Expect(err).ToNot(HaveOccurred()) return id, priv @@ -48,13 +59,24 @@ var _ = Describe("Transport", func() { // modify the cert chain such that verificiation will fail invalidateCertChain := func(identity *Identity) { - key, err := rsa.GenerateKey(rand.Reader, 1024) - Expect(err).ToNot(HaveOccurred()) - identity.Config.Certificates[0].PrivateKey = key + 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 + case *ecdsa.PrivateKey: + key, err := ecdsa.GenerateKey(elliptic.P224(), rand.Reader) + Expect(err).ToNot(HaveOccurred()) + identity.Config.Certificates[0].PrivateKey = key + default: + Fail("unexpected private key type") + } } BeforeEach(func() { + fmt.Fprintf(GinkgoWriter, "Initializing a server") serverID, serverKey = createPeer() + fmt.Fprintf(GinkgoWriter, "Initializing a client") clientID, clientKey = createPeer() }) @@ -135,6 +157,7 @@ var _ = Describe("Transport", func() { }) It("fails if the peer ID doesn't match", func() { + fmt.Fprintf(GinkgoWriter, "Creating another peer") thirdPartyID, _ := createPeer() serverTransport, err := New(serverKey) @@ -172,7 +195,10 @@ var _ = Describe("Transport", func() { defer GinkgoRecover() _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("crypto/rsa: verification error")) + Expect(err.Error()).To(Or( + ContainSubstring("crypto/rsa: verification error"), + ContainSubstring("ECDSA verification failure"), + )) close(done) }() @@ -202,7 +228,10 @@ var _ = Describe("Transport", func() { _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("crypto/rsa: verification error")) + Expect(err.Error()).To(Or( + ContainSubstring("crypto/rsa: verification error"), + ContainSubstring("ECDSA verification failure"), + )) Eventually(done).Should(BeClosed()) }) }) From 10b3e2a26565ffcc709bd22017b77b00aaab6b6a Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 10 Feb 2019 15:06:26 +0800 Subject: [PATCH 08/51] avoid using interface{} when generating certificates --- p2p/security/tls/crypto.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index 583fe39c14..23178fa9ab 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -1,6 +1,7 @@ package libp2ptls import ( + "crypto" "crypto/rand" "crypto/tls" "crypto/x509" @@ -104,7 +105,7 @@ func getRemotePubKey(chain []*x509.Certificate) (ic.PubKey, error) { } } -func keyToCertificate(sk ic.PrivKey) (interface{}, *x509.Certificate, error) { +func keyToCertificate(sk ic.PrivKey) (crypto.PrivateKey, *x509.Certificate, error) { sn, err := rand.Int(rand.Reader, big.NewInt(1<<62)) if err != nil { return nil, nil, err @@ -115,7 +116,8 @@ func keyToCertificate(sk ic.PrivKey) (interface{}, *x509.Certificate, error) { NotAfter: time.Now().Add(certValidityPeriod), } - var publicKey, privateKey interface{} + var privateKey crypto.PrivateKey + var publicKey crypto.PublicKey keyBytes, err := sk.Bytes() if err != nil { return nil, nil, err From 9f8a3248114d079de1f851ed9bbfaa3b69710a03 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 17 Feb 2019 11:15:42 +0800 Subject: [PATCH 09/51] remove unneeded marshaling / unmarshaling when generating cert chain --- p2p/security/tls/crypto.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index 23178fa9ab..eb817eef35 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -10,7 +10,6 @@ import ( "math/big" "time" - "github.com/gogo/protobuf/proto" ic "github.com/libp2p/go-libp2p-crypto" pb "github.com/libp2p/go-libp2p-crypto/pb" peer "github.com/libp2p/go-libp2p-peer" @@ -118,24 +117,20 @@ func keyToCertificate(sk ic.PrivKey) (crypto.PrivateKey, *x509.Certificate, erro var privateKey crypto.PrivateKey var publicKey crypto.PublicKey - keyBytes, err := sk.Bytes() + raw, err := sk.Raw() 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() { + switch sk.Type() { case pb.KeyType_RSA: - k, err := x509.ParsePKCS1PrivateKey(pbmes.GetData()) + k, err := x509.ParsePKCS1PrivateKey(raw) if err != nil { return nil, nil, err } publicKey = &k.PublicKey privateKey = k case pb.KeyType_ECDSA: - k, err := x509.ParseECPrivateKey(pbmes.GetData()) + k, err := x509.ParseECPrivateKey(raw) if err != nil { return nil, nil, err } From 5ecc2f97d3d4f75522bdcdcf12ef3778b6363ec3 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 10 Feb 2019 11:12:10 +0800 Subject: [PATCH 10/51] drop support for Go 1.10 TLS 1.3 will require Go >= 1.12, so we don't need any fixes that were specific to Go 1.10 any more. --- p2p/security/tls/transport.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index 5a75296f1a..3f25989210 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -43,21 +43,16 @@ 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) { serv := tls.Server(insecure, t.identity.Config) - return t.handshake(ctx, insecure, serv) + return t.handshake(ctx, serv) } // SecureOutbound runs the TLS handshake as a client. func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p peer.ID) (cs.Conn, error) { cl := tls.Client(insecure, t.identity.ConfigForPeer(p)) - return t.handshake(ctx, insecure, cl) + return t.handshake(ctx, cl) } -func (t *Transport) handshake( - ctx context.Context, - // in Go 1.10, we need to close the underlying net.Conn - // in Go 1.11 this was fixed, and tls.Conn.Close() works as well - insecure net.Conn, - tlsConn *tls.Conn, +func (t *Transport) handshake(ctx context.Context, tlsConn *tls.Conn, ) (cs.Conn, error) { // There's no way to pass a context to tls.Conn.Handshake(). // See https://github.com/golang/go/issues/18482. @@ -68,7 +63,7 @@ func (t *Transport) handshake( select { case <-done: case <-ctx.Done(): - insecure.Close() + tlsConn.Close() } }() From 490981871c7882d07e589a3f466ac8dd6f2f0675 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 10 Feb 2019 15:25:29 +0800 Subject: [PATCH 11/51] improve logging in tests --- p2p/security/tls/transport_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index d06326d90f..c3d92196d2 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -26,18 +26,19 @@ var _ = Describe("Transport", func() { createPeer := func() (peer.ID, ic.PrivKey) { var priv ic.PrivKey if mrand.Int()%2 == 0 { - fmt.Fprintln(GinkgoWriter, " using an ECDSA key") + fmt.Fprintf(GinkgoWriter, " using an ECDSA key: ") var err error priv, _, err = ic.GenerateECDSAKeyPair(rand.Reader) Expect(err).ToNot(HaveOccurred()) } else { - fmt.Fprintln(GinkgoWriter, " using an RSA key") + fmt.Fprintf(GinkgoWriter, " using an RSA key: ") var err error priv, _, err = ic.GenerateRSAKeyPair(1024, rand.Reader) Expect(err).ToNot(HaveOccurred()) } id, err := peer.IDFromPrivateKey(priv) Expect(err).ToNot(HaveOccurred()) + fmt.Fprintln(GinkgoWriter, id.Pretty()) return id, priv } From 0b45a8d2fb671404be4248f55226e4eefb4e97a1 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 10 Feb 2019 16:23:43 +0800 Subject: [PATCH 12/51] make sure to close the connection if the context is already canceled --- p2p/security/tls/transport.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index 3f25989210..835e8f61e4 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -57,6 +57,11 @@ func (t *Transport) handshake(ctx context.Context, tlsConn *tls.Conn, // There's no way to pass a context to tls.Conn.Handshake(). // See https://github.com/golang/go/issues/18482. // Close the connection instead. + select { + case <-ctx.Done(): + tlsConn.Close() + default: + } done := make(chan struct{}) defer close(done) go func() { From 1c09b025c071bc26803c041add92f8b8f1e02d78 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 10 Feb 2019 15:51:59 +0800 Subject: [PATCH 13/51] switch to TLS 1.3 TLS 1.3 support was recently made opt-in in Go 1.12, so we need to explicitly enable it. --- p2p/security/tls/crypto.go | 1 + p2p/security/tls/transport.go | 13 +++++++++++++ p2p/security/tls/transport_test.go | 23 ++++++++--------------- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index eb817eef35..eb1b7a12e0 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -72,6 +72,7 @@ func generateConfig(privKey ic.PrivKey) (*tls.Config, error) { return nil, err } return &tls.Config{ + MinVersion: tls.VersionTLS13, InsecureSkipVerify: true, // This is not insecure here. We will verify the cert chain ourselves. ClientAuth: tls.RequireAnyClientCert, Certificates: []tls.Certificate{{ diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index 835e8f61e4..db820cf0a5 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -4,12 +4,19 @@ import ( "context" "crypto/tls" "net" + "os" cs "github.com/libp2p/go-conn-security" ci "github.com/libp2p/go-libp2p-crypto" peer "github.com/libp2p/go-libp2p-peer" ) +// TLS 1.3 is opt-in in Go 1.12 +// Activate it by setting the tls13 GODEBUG flag. +func init() { + os.Setenv("GODEBUG", os.Getenv("GODEBUG")+",tls13=1") +} + // ID is the protocol ID (used when negotiating with multistream) const ID = "/tls/1.0.0" @@ -47,6 +54,12 @@ func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn) (cs.Co } // SecureOutbound runs the TLS handshake as a client. +// Note that SecureOutbound will not return an error if the server doesn't +// accept the certificate. This is due to the fact that in TLS 1.3, the client +// sends its certificate and the ClientFinished in the same flight, and can send +// application data immediately afterwards. +// 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) { cl := tls.Client(insecure, t.identity.ConfigForPeer(p)) return t.handshake(ctx, cl) diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index c3d92196d2..3e281e3e1b 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -66,7 +66,7 @@ var _ = Describe("Transport", func() { Expect(err).ToNot(HaveOccurred()) identity.Config.Certificates[0].PrivateKey = key case *ecdsa.PrivateKey: - key, err := ecdsa.GenerateKey(elliptic.P224(), rand.Reader) + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) Expect(err).ToNot(HaveOccurred()) identity.Config.Certificates[0].PrivateKey = key default: @@ -195,17 +195,14 @@ var _ = Describe("Transport", func() { go func() { defer GinkgoRecover() _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(Or( - ContainSubstring("crypto/rsa: verification error"), - ContainSubstring("ECDSA verification failure"), - )) + Expect(err).To(MatchError("tls: invalid certificate signature")) close(done) }() - _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("tls: bad certificate")) + conn, err := clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) + Expect(err).ToNot(HaveOccurred()) + _, err = conn.Read([]byte{0}) + Expect(err).To(MatchError("remote error: tls: error decrypting message")) Eventually(done).Should(BeClosed()) }) @@ -223,16 +220,12 @@ var _ = Describe("Transport", func() { defer GinkgoRecover() _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) Expect(err).To(HaveOccurred()) - // TLS returns a weird error here: "remote error: tls: unexpected message" + Expect(err.Error()).To(ContainSubstring("remote error: tls:")) close(done) }() _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(Or( - ContainSubstring("crypto/rsa: verification error"), - ContainSubstring("ECDSA verification failure"), - )) + Expect(err).To(MatchError("tls: invalid certificate signature")) Eventually(done).Should(BeClosed()) }) }) From f799512a9738a19b917b4fdd54e25715bacf2908 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 21 Feb 2019 16:20:12 +0800 Subject: [PATCH 14/51] derive and save the client's pub key in tls.Config.VerifyPeerCertificate --- p2p/security/tls/crypto.go | 65 +++++++---- p2p/security/tls/transport.go | 60 +++++++--- p2p/security/tls/transport_test.go | 182 ++++++++++++++++++++--------- 3 files changed, 218 insertions(+), 89 deletions(-) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index eb1b7a12e0..272bb49d60 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "math/big" + "net" "time" ic "github.com/libp2p/go-libp2p-crypto" @@ -15,17 +16,54 @@ import ( peer "github.com/libp2p/go-libp2p-peer" ) +const certValidityPeriod = 180 * 24 * time.Hour + // Identity is used to secure connections type Identity struct { *tls.Config } // NewIdentity creates a new identity -func NewIdentity(privKey ic.PrivKey) (*Identity, error) { - conf, err := generateConfig(privKey) +func NewIdentity( + privKey ic.PrivKey, + verifiedPeerCallback func(net.Conn, ic.PubKey), +) (*Identity, error) { + key, cert, err := keyToCertificate(privKey) if err != nil { return nil, err } + conf := &tls.Config{ + MinVersion: tls.VersionTLS13, + InsecureSkipVerify: true, // This is not insecure here. We will verify the cert chain ourselves. + ClientAuth: tls.RequireAnyClientCert, + Certificates: []tls.Certificate{{ + 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 } @@ -64,24 +102,7 @@ func KeyFromChain(chain []*x509.Certificate) (ic.PubKey, error) { return getRemotePubKey(chain) } -const certValidityPeriod = 180 * 24 * time.Hour - -func generateConfig(privKey ic.PrivKey) (*tls.Config, error) { - key, cert, err := keyToCertificate(privKey) - if err != nil { - return nil, err - } - return &tls.Config{ - MinVersion: tls.VersionTLS13, - InsecureSkipVerify: true, // This is not insecure here. We will verify the cert chain ourselves. - ClientAuth: tls.RequireAnyClientCert, - Certificates: []tls.Certificate{{ - Certificate: [][]byte{cert.Raw}, - PrivateKey: key, - }}, - }, nil -} - +// getRemotePubKey derives the remote's public key from the certificate chain. func getRemotePubKey(chain []*x509.Certificate) (ic.PubKey, error) { if len(chain) != 1 { return nil, errors.New("expected one certificates in the chain") @@ -89,7 +110,9 @@ func getRemotePubKey(chain []*x509.Certificate) (ic.PubKey, error) { pool := x509.NewCertPool() pool.AddCert(chain[0]) if _, err := chain[0].Verify(x509.VerifyOptions{Roots: pool}); err != nil { - return nil, err + // If we return an x509 error here, it will be sent on the wire. + // Wrap the error to avoid that. + return nil, fmt.Errorf("certificate verification failed: %s", err) } remotePubKey, err := x509.MarshalPKIXPublicKey(chain[0].PublicKey) if err != nil { diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index db820cf0a5..751a461487 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -5,9 +5,11 @@ import ( "crypto/tls" "net" "os" + "sync" cs "github.com/libp2p/go-conn-security" ci "github.com/libp2p/go-libp2p-crypto" + ic "github.com/libp2p/go-libp2p-crypto" peer "github.com/libp2p/go-libp2p-peer" ) @@ -26,6 +28,9 @@ type Transport struct { localPeer peer.ID privKey ci.PrivKey + + incomingMutex sync.Mutex + incoming map[net.Conn]ic.PubKey } // New creates a TLS encrypted transport @@ -34,23 +39,32 @@ func New(key ci.PrivKey) (*Transport, error) { if err != nil { return nil, err } - identity, err := NewIdentity(key) + t := &Transport{ + localPeer: id, + privKey: key, + incoming: make(map[net.Conn]ic.PubKey), + } + identity, err := NewIdentity(key, t.verifiedPeer) if err != nil { return nil, err } - return &Transport{ - identity: identity, - localPeer: id, - privKey: key, - }, nil + t.identity = identity + return t, nil } 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.incomingMutex.Lock() + // only contains this connection if we successfully derived the client's key + delete(t.incoming, insecure) + t.incomingMutex.Unlock() + }() + serv := tls.Server(insecure, t.identity.Config) - return t.handshake(ctx, serv) + return t.handshake(ctx, insecure, serv) } // SecureOutbound runs the TLS handshake as a client. @@ -62,10 +76,13 @@ func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn) (cs.Co // notice this after 1 RTT when calling Read. func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p peer.ID) (cs.Conn, error) { cl := tls.Client(insecure, t.identity.ConfigForPeer(p)) - return t.handshake(ctx, cl) + return t.handshake(ctx, insecure, cl) } -func (t *Transport) handshake(ctx context.Context, tlsConn *tls.Conn, +func (t *Transport) handshake( + ctx context.Context, + insecure net.Conn, + tlsConn *tls.Conn, ) (cs.Conn, error) { // There's no way to pass a context to tls.Conn.Handshake(). // See https://github.com/golang/go/issues/18482. @@ -92,7 +109,7 @@ func (t *Transport) handshake(ctx context.Context, tlsConn *tls.Conn, } return nil, err } - conn, err := t.setupConn(tlsConn) + conn, err := t.setupConn(insecure, tlsConn) if err != nil { // if the context was canceled, return the context error if ctxErr := ctx.Err(); ctxErr != nil { @@ -103,10 +120,25 @@ func (t *Transport) handshake(ctx context.Context, tlsConn *tls.Conn, return conn, nil } -func (t *Transport) setupConn(tlsConn *tls.Conn) (cs.Conn, error) { - remotePubKey, err := KeyFromChain(tlsConn.ConnectionState().PeerCertificates) - if err != nil { - return nil, err +func (t *Transport) verifiedPeer(conn net.Conn, pubKey ic.PubKey) { + t.incomingMutex.Lock() + t.incoming[conn] = pubKey + t.incomingMutex.Unlock() +} + +func (t *Transport) setupConn(insecure net.Conn, tlsConn *tls.Conn) (cs.Conn, error) { + t.incomingMutex.Lock() + remotePubKey := t.incoming[insecure] + t.incomingMutex.Unlock() + + // This case only occurs for the client. + // Servers already determined the client's key in the VerifyPeerCertificate callback. + if remotePubKey == nil { + var err error + remotePubKey, err = KeyFromChain(tlsConn.ConnectionState().PeerCertificates) + if err != nil { + return nil, err + } } remotePeerID, err := peer.IDFromPublicKey(remotePubKey) if err != nil { diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index 3e281e3e1b..c10cbdd187 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -6,9 +6,15 @@ import ( "crypto/elliptic" "crypto/rand" "crypto/rsa" + "crypto/tls" + "crypto/x509" "fmt" + "math/big" mrand "math/rand" "net" + "time" + + "github.com/onsi/gomega/gbytes" cs "github.com/libp2p/go-conn-security" ic "github.com/libp2p/go-libp2p-crypto" @@ -17,6 +23,12 @@ import ( . "github.com/onsi/gomega" ) +type transform struct { + name string + apply func(*Identity) + remoteErr string // the error that the side validating the chain gets +} + var _ = Describe("Transport", func() { var ( serverKey, clientKey ic.PrivKey @@ -58,22 +70,6 @@ var _ = Describe("Transport", func() { return conn, <-serverConnChan } - // modify the cert chain such that verificiation will fail - invalidateCertChain := func(identity *Identity) { - 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 - case *ecdsa.PrivateKey: - key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - Expect(err).ToNot(HaveOccurred()) - identity.Config.Certificates[0].PrivateKey = key - default: - Fail("unexpected private key type") - } - } - BeforeEach(func() { fmt.Fprintf(GinkgoWriter, "Initializing a server") serverID, serverKey = createPeer() @@ -182,50 +178,128 @@ var _ = Describe("Transport", func() { Eventually(done).Should(BeClosed()) }) - It("fails if the client presents an invalid cert chain", func() { - serverTransport, err := New(serverKey) - Expect(err).ToNot(HaveOccurred()) - clientTransport, err := New(clientKey) - Expect(err).ToNot(HaveOccurred()) - invalidateCertChain(clientTransport.identity) + Context("invalid certificates", func() { + invalidateCertChain := func(identity *Identity) { + 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 + case *ecdsa.PrivateKey: + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + Expect(err).ToNot(HaveOccurred()) + identity.Config.Certificates[0].PrivateKey = key + default: + Fail("unexpected private key type") + } + } - clientInsecureConn, serverInsecureConn := connect() + twoCerts := func(identity *Identity) { + tmpl := &x509.Certificate{SerialNumber: big.NewInt(1)} + key1, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + Expect(err).ToNot(HaveOccurred()) + key2, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + Expect(err).ToNot(HaveOccurred()) + cert1DER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key1.Public(), key1) + Expect(err).ToNot(HaveOccurred()) + cert1, err := x509.ParseCertificate(cert1DER) + Expect(err).ToNot(HaveOccurred()) + cert2DER, err := x509.CreateCertificate(rand.Reader, tmpl, cert1, key2.Public(), key2) + Expect(err).ToNot(HaveOccurred()) + identity.Config.Certificates = []tls.Certificate{{ + Certificate: [][]byte{cert2DER, cert1DER}, + PrivateKey: key2, + }} + } - done := make(chan struct{}) - go func() { - defer GinkgoRecover() - _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) - Expect(err).To(MatchError("tls: invalid certificate signature")) - close(done) - }() + expiredCert := func(identity *Identity) { + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(-time.Minute), + } + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + Expect(err).ToNot(HaveOccurred()) + cert, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key.Public(), key) + Expect(err).ToNot(HaveOccurred()) + identity.Config.Certificates = []tls.Certificate{{ + Certificate: [][]byte{cert}, + PrivateKey: key, + }} + } - conn, err := clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) - Expect(err).ToNot(HaveOccurred()) - _, err = conn.Read([]byte{0}) - Expect(err).To(MatchError("remote error: tls: error decrypting message")) - Eventually(done).Should(BeClosed()) - }) + transforms := []transform{ + { + name: "private key used in the TLS handshake doesn't match the public key in the cert", + apply: invalidateCertChain, + remoteErr: "tls: invalid certificate signature", + }, + { + name: "certificate chain contains 2 certs", + apply: twoCerts, + remoteErr: "expected one certificates in the chain", + }, + { + name: "cert is expired", + apply: expiredCert, + remoteErr: "certificate verification failed: x509: certificate has expired or is not yet valid", + }, + } - It("fails if the server presents an invalid cert chain", func() { - serverTransport, err := New(serverKey) - Expect(err).ToNot(HaveOccurred()) - invalidateCertChain(serverTransport.identity) - clientTransport, err := New(clientKey) - Expect(err).ToNot(HaveOccurred()) + for i := range transforms { + t := transforms[i] - clientInsecureConn, serverInsecureConn := connect() + It(fmt.Sprintf("fails if the client presents an invalid cert: %s", t.name), func() { + serverTransport, err := New(serverKey) + Expect(err).ToNot(HaveOccurred()) + clientTransport, err := New(clientKey) + Expect(err).ToNot(HaveOccurred()) + t.apply(clientTransport.identity) - done := make(chan struct{}) - go func() { - defer GinkgoRecover() - _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("remote error: tls:")) - close(done) - }() + clientInsecureConn, serverInsecureConn := connect() - _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) - Expect(err).To(MatchError("tls: invalid certificate signature")) - Eventually(done).Should(BeClosed()) + done := make(chan struct{}) + go func() { + defer GinkgoRecover() + _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) + Expect(err).To(MatchError(t.remoteErr)) + close(done) + }() + + conn, err := clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) + Expect(err).ToNot(HaveOccurred()) + _, err = gbytes.TimeoutReader(conn, time.Second).Read([]byte{0}) + Expect(err).To(Or( + // if the certificate's public key doesn't match the private key used for signing + MatchError("remote error: tls: error decrypting message"), + // all other errors + MatchError("remote error: tls: bad certificate"), + )) + Eventually(done).Should(BeClosed()) + }) + + It(fmt.Sprintf("fails if the server presents an invalid cert: %s", t.name), func() { + serverTransport, err := New(serverKey) + Expect(err).ToNot(HaveOccurred()) + t.apply(serverTransport.identity) + clientTransport, err := New(clientKey) + Expect(err).ToNot(HaveOccurred()) + + clientInsecureConn, serverInsecureConn := connect() + + done := make(chan struct{}) + go func() { + defer GinkgoRecover() + _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("remote error: tls:")) + close(done) + }() + + _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) + Expect(err).To(MatchError(t.remoteErr)) + Eventually(done).Should(BeClosed()) + }) + } }) }) From 5677418dda1692ecdbb43968242f03cbe7c6f578 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 21 Feb 2019 16:33:00 +0800 Subject: [PATCH 15/51] derive and save the server's pub key in tls.Config.VerifyPeerCertificate --- p2p/security/tls/crypto.go | 11 ++++---- p2p/security/tls/transport.go | 48 +++++++++++++++++------------------ 2 files changed, 29 insertions(+), 30 deletions(-) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index 272bb49d60..3b6b425e48 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -69,7 +69,10 @@ func NewIdentity( // ConfigForPeer creates a new tls.Config that verifies the peers certificate chain. // It should be used to create a new tls.Config before dialing. -func (i *Identity) ConfigForPeer(remote peer.ID) *tls.Config { +func (i *Identity) ConfigForPeer( + remote peer.ID, + verifiedPeerCallback func(ic.PubKey), +) *tls.Config { // 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. @@ -92,16 +95,12 @@ func (i *Identity) ConfigForPeer(remote peer.ID) *tls.Config { if !remote.MatchesPublicKey(pubKey) { return errors.New("peer IDs don't match") } + verifiedPeerCallback(pubKey) return nil } return conf } -// KeyFromChain takes a chain of x509.Certificates and returns the peer's public key. -func KeyFromChain(chain []*x509.Certificate) (ic.PubKey, error) { - return getRemotePubKey(chain) -} - // getRemotePubKey derives the remote's public key from the certificate chain. func getRemotePubKey(chain []*x509.Certificate) (ic.PubKey, error) { if len(chain) != 1 { diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index 751a461487..dd57b261e8 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -3,6 +3,7 @@ package libp2ptls import ( "context" "crypto/tls" + "errors" "net" "os" "sync" @@ -29,8 +30,8 @@ type Transport struct { localPeer peer.ID privKey ci.PrivKey - incomingMutex sync.Mutex - incoming map[net.Conn]ic.PubKey + activeMutex sync.Mutex + active map[net.Conn]ic.PubKey } // New creates a TLS encrypted transport @@ -42,9 +43,14 @@ func New(key ci.PrivKey) (*Transport, error) { t := &Transport{ localPeer: id, privKey: key, - incoming: make(map[net.Conn]ic.PubKey), + active: make(map[net.Conn]ic.PubKey), } - identity, err := NewIdentity(key, t.verifiedPeer) + + identity, err := NewIdentity(key, func(conn net.Conn, pubKey ic.PubKey) { + t.activeMutex.Lock() + t.active[conn] = pubKey + t.activeMutex.Unlock() + }) if err != nil { return nil, err } @@ -57,10 +63,10 @@ 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.incomingMutex.Lock() + t.activeMutex.Lock() // only contains this connection if we successfully derived the client's key - delete(t.incoming, insecure) - t.incomingMutex.Unlock() + delete(t.active, insecure) + t.activeMutex.Unlock() }() serv := tls.Server(insecure, t.identity.Config) @@ -75,7 +81,12 @@ 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) { - cl := tls.Client(insecure, t.identity.ConfigForPeer(p)) + 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) } @@ -120,26 +131,15 @@ func (t *Transport) handshake( return conn, nil } -func (t *Transport) verifiedPeer(conn net.Conn, pubKey ic.PubKey) { - t.incomingMutex.Lock() - t.incoming[conn] = pubKey - t.incomingMutex.Unlock() -} - func (t *Transport) setupConn(insecure net.Conn, tlsConn *tls.Conn) (cs.Conn, error) { - t.incomingMutex.Lock() - remotePubKey := t.incoming[insecure] - t.incomingMutex.Unlock() + t.activeMutex.Lock() + remotePubKey := t.active[insecure] + t.activeMutex.Unlock() - // This case only occurs for the client. - // Servers already determined the client's key in the VerifyPeerCertificate callback. if remotePubKey == nil { - var err error - remotePubKey, err = KeyFromChain(tlsConn.ConnectionState().PeerCertificates) - if err != nil { - return nil, err - } + return nil, errors.New("go-libp2p-tls BUG: expected remote pub key to be set") } + remotePeerID, err := peer.IDFromPublicKey(remotePubKey) if err != nil { return nil, err From 92fedfe742a462d368bc98d1ecf730be2833f48e Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Wed, 27 Feb 2019 12:31:03 -0800 Subject: [PATCH 16/51] make peer verification use a channel --- p2p/security/tls/crypto.go | 83 +++++++++++++----------------- p2p/security/tls/transport.go | 49 ++++++------------ p2p/security/tls/transport_test.go | 10 ++-- 3 files changed, 57 insertions(+), 85 deletions(-) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index 3b6b425e48..0c31bf5d52 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" "math/big" - "net" "time" ic "github.com/libp2p/go-libp2p-crypto" @@ -20,66 +19,55 @@ 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{ - MinVersion: tls.VersionTLS13, - InsecureSkipVerify: true, // This is not insecure here. We will verify the cert chain ourselves. - ClientAuth: tls.RequireAnyClientCert, - Certificates: []tls.Certificate{{ - 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 + 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, + Certificates: []tls.Certificate{{ + Certificate: [][]byte{cert.Raw}, + PrivateKey: key, + }}, + 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. diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index dd57b261e8..6301f8897c 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/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") } diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index c10cbdd187..a02debe8c9 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/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, }} From ebc4872cb93b36f0834121d45043de7373e60cda Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 28 Feb 2019 09:16:27 +0900 Subject: [PATCH 17/51] fix duplicate import --- p2p/security/tls/conn.go | 10 +++++----- p2p/security/tls/transport.go | 5 ++--- p2p/security/tls/transport_test.go | 12 ++++++------ 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/p2p/security/tls/conn.go b/p2p/security/tls/conn.go index f4eb600089..d5450b1c6d 100644 --- a/p2p/security/tls/conn.go +++ b/p2p/security/tls/conn.go @@ -4,7 +4,7 @@ import ( "crypto/tls" cs "github.com/libp2p/go-conn-security" - ic "github.com/libp2p/go-libp2p-crypto" + ci "github.com/libp2p/go-libp2p-crypto" peer "github.com/libp2p/go-libp2p-peer" ) @@ -12,10 +12,10 @@ type conn struct { *tls.Conn localPeer peer.ID - privKey ic.PrivKey + privKey ci.PrivKey remotePeer peer.ID - remotePubKey ic.PubKey + remotePubKey ci.PubKey } var _ cs.Conn = &conn{} @@ -24,7 +24,7 @@ func (c *conn) LocalPeer() peer.ID { return c.localPeer } -func (c *conn) LocalPrivateKey() ic.PrivKey { +func (c *conn) LocalPrivateKey() ci.PrivKey { return c.privKey } @@ -32,6 +32,6 @@ func (c *conn) RemotePeer() peer.ID { return c.remotePeer } -func (c *conn) RemotePublicKey() ic.PubKey { +func (c *conn) RemotePublicKey() ci.PubKey { return c.remotePubKey } diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index 6301f8897c..8b3e132f6b 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -9,7 +9,6 @@ import ( cs "github.com/libp2p/go-conn-security" ci "github.com/libp2p/go-libp2p-crypto" - ic "github.com/libp2p/go-libp2p-crypto" peer "github.com/libp2p/go-libp2p-peer" ) @@ -101,7 +100,7 @@ func (t *Transport) handshake( } // Should be ready by this point, don't block. - var remotePubKey ic.PubKey + var remotePubKey ci.PubKey select { case remotePubKey = <-keyCh: default: @@ -118,7 +117,7 @@ func (t *Transport) handshake( return conn, nil } -func (t *Transport) setupConn(tlsConn *tls.Conn, remotePubKey ic.PubKey) (cs.Conn, error) { +func (t *Transport) setupConn(tlsConn *tls.Conn, remotePubKey ci.PubKey) (cs.Conn, error) { if remotePubKey == nil { return nil, errors.New("go-libp2p-tls BUG: expected remote pub key to be set") } diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index a02debe8c9..02076eed92 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -17,7 +17,7 @@ import ( "github.com/onsi/gomega/gbytes" cs "github.com/libp2p/go-conn-security" - ic "github.com/libp2p/go-libp2p-crypto" + ci "github.com/libp2p/go-libp2p-crypto" peer "github.com/libp2p/go-libp2p-peer" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -31,21 +31,21 @@ type transform struct { var _ = Describe("Transport", func() { var ( - serverKey, clientKey ic.PrivKey + serverKey, clientKey ci.PrivKey serverID, clientID peer.ID ) - createPeer := func() (peer.ID, ic.PrivKey) { - var priv ic.PrivKey + createPeer := func() (peer.ID, ci.PrivKey) { + var priv ci.PrivKey if mrand.Int()%2 == 0 { fmt.Fprintf(GinkgoWriter, " using an ECDSA key: ") var err error - priv, _, err = ic.GenerateECDSAKeyPair(rand.Reader) + priv, _, err = ci.GenerateECDSAKeyPair(rand.Reader) Expect(err).ToNot(HaveOccurred()) } else { fmt.Fprintf(GinkgoWriter, " using an RSA key: ") var err error - priv, _, err = ic.GenerateRSAKeyPair(1024, rand.Reader) + priv, _, err = ci.GenerateRSAKeyPair(1024, rand.Reader) Expect(err).ToNot(HaveOccurred()) } id, err := peer.IDFromPrivateKey(priv) From 7d129c2a6f2766db042f79a358aee76095fd5989 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 20 Feb 2019 17:44:39 +0800 Subject: [PATCH 18/51] add an example server and client --- p2p/security/tls/example/README.md | 6 ++ p2p/security/tls/example/client/main.go | 68 +++++++++++++++++++++++ p2p/security/tls/example/server/main.go | 73 +++++++++++++++++++++++++ 3 files changed, 147 insertions(+) create mode 100644 p2p/security/tls/example/README.md create mode 100644 p2p/security/tls/example/client/main.go create mode 100644 p2p/security/tls/example/server/main.go diff --git a/p2p/security/tls/example/README.md b/p2p/security/tls/example/README.md new file mode 100644 index 0000000000..dff0f66885 --- /dev/null +++ b/p2p/security/tls/example/README.md @@ -0,0 +1,6 @@ +# TLS handshake example + +Run +```bash +go run example/server/main.go +``` diff --git a/p2p/security/tls/example/client/main.go b/p2p/security/tls/example/client/main.go new file mode 100644 index 0000000000..7e8ffca5da --- /dev/null +++ b/p2p/security/tls/example/client/main.go @@ -0,0 +1,68 @@ +package main + +import ( + "context" + "crypto/rand" + "flag" + "fmt" + "io/ioutil" + "net" + "time" + + ic "github.com/libp2p/go-libp2p-crypto" + peer "github.com/libp2p/go-libp2p-peer" + libp2ptls "github.com/libp2p/go-libp2p-tls" +) + +func main() { + if err := startClient(); err != nil { + panic(err) + } +} + +func startClient() error { + port := flag.Int("p", 5533, "port") + peerIDString := flag.String("id", "", "peer ID") + flag.Parse() + + peerID, err := peer.IDB58Decode(*peerIDString) + if err != nil { + return err + } + + priv, _, err := ic.GenerateECDSAKeyPair(rand.Reader) + if err != nil { + return err + } + id, err := peer.IDFromPrivateKey(priv) + if err != nil { + return err + } + fmt.Printf("Generated new peer with an ECDSA key. Peer ID: %s\n", id.Pretty()) + tp, err := libp2ptls.New(priv) + if err != nil { + return err + } + + remoteAddr := fmt.Sprintf("localhost:%d", *port) + fmt.Printf("Dialing %s\n", remoteAddr) + conn, err := net.Dial("tcp", remoteAddr) + if err != nil { + return err + } + fmt.Printf("Dialed raw connection to %s\n", conn.RemoteAddr()) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + sconn, err := tp.SecureOutbound(ctx, conn, peerID) + if err != nil { + return err + } + fmt.Printf("Authenticated server: %s\n", sconn.RemotePeer().Pretty()) + data, err := ioutil.ReadAll(sconn) + if err != nil { + return err + } + fmt.Printf("Received message from server: %s\n", string(data)) + return nil +} diff --git a/p2p/security/tls/example/server/main.go b/p2p/security/tls/example/server/main.go new file mode 100644 index 0000000000..d8371658b3 --- /dev/null +++ b/p2p/security/tls/example/server/main.go @@ -0,0 +1,73 @@ +package main + +import ( + "context" + "crypto/rand" + "flag" + "fmt" + "net" + "time" + + ic "github.com/libp2p/go-libp2p-crypto" + peer "github.com/libp2p/go-libp2p-peer" + libp2ptls "github.com/libp2p/go-libp2p-tls" +) + +func main() { + if err := startServer(); err != nil { + panic(err) + } +} + +func startServer() error { + port := flag.Int("p", 5533, "port") + flag.Parse() + + priv, _, err := ic.GenerateECDSAKeyPair(rand.Reader) + if err != nil { + return err + } + id, err := peer.IDFromPrivateKey(priv) + if err != nil { + return err + } + fmt.Printf("Generated new peer with an ECDSA key. Peer ID: %s\n", id.Pretty()) + tp, err := libp2ptls.New(priv) + if err != nil { + return err + } + + ln, err := net.Listen("tcp", fmt.Sprintf("localhost:%d", *port)) + if err != nil { + return err + } + fmt.Printf("Listening for new connections on %s\n", ln.Addr()) + fmt.Printf("Now run the following command in a separate terminal:\n") + fmt.Printf("\tgo run example/client/main.go -p %d -id %s\n", *port, id.Pretty()) + + for { + conn, err := ln.Accept() + if err != nil { + return err + } + fmt.Printf("Accepted raw connection from %s\n", conn.RemoteAddr()) + go func() { + if err := handleConn(tp, conn); err != nil { + fmt.Printf("Error handling connection from %s: %s\n", conn.RemoteAddr(), err) + } + }() + } +} + +func handleConn(tp *libp2ptls.Transport, conn net.Conn) error { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + sconn, err := tp.SecureInbound(ctx, conn) + if err != nil { + return err + } + fmt.Printf("Authenticated client: %s\n", sconn.RemotePeer().Pretty()) + fmt.Fprintf(sconn, "Hello client!") + fmt.Printf("Closing connection to %s\n", conn.RemoteAddr()) + return sconn.Close() +} From e4b8bb72f3dba471ad8e115ce8bdf7389685890c Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 20 Feb 2019 18:10:23 +0800 Subject: [PATCH 19/51] add a command line flag to set the key type --- p2p/security/tls/example/client/main.go | 25 ++++++++++++++++++++++--- p2p/security/tls/example/server/main.go | 23 +++++++++++++++++++++-- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/p2p/security/tls/example/client/main.go b/p2p/security/tls/example/client/main.go index 7e8ffca5da..678ece6ca6 100644 --- a/p2p/security/tls/example/client/main.go +++ b/p2p/security/tls/example/client/main.go @@ -23,22 +23,41 @@ func main() { func startClient() error { port := flag.Int("p", 5533, "port") peerIDString := flag.String("id", "", "peer ID") + keyType := flag.String("key", "ecdsa", "rsa, ecdsa, ed25519 or secp256k1") flag.Parse() - peerID, err := peer.IDB58Decode(*peerIDString) + var priv ic.PrivKey + var err error + switch *keyType { + case "rsa": + fmt.Printf("Generated new peer with an RSA key.") + priv, _, err = ic.GenerateRSAKeyPair(2048, rand.Reader) + case "ecdsa": + fmt.Printf("Generated new peer with an ECDSA key.") + priv, _, err = ic.GenerateECDSAKeyPair(rand.Reader) + case "ed25519": + fmt.Printf("Generated new peer with an Ed25519 key.") + priv, _, err = ic.GenerateEd25519Key(rand.Reader) + case "secp256k1": + fmt.Printf("Generated new peer with an Secp256k1 key.") + priv, _, err = ic.GenerateSecp256k1Key(rand.Reader) + default: + return fmt.Errorf("unknown key type: %s", *keyType) + } if err != nil { return err } - priv, _, err := ic.GenerateECDSAKeyPair(rand.Reader) + peerID, err := peer.IDB58Decode(*peerIDString) if err != nil { return err } + id, err := peer.IDFromPrivateKey(priv) if err != nil { return err } - fmt.Printf("Generated new peer with an ECDSA key. Peer ID: %s\n", id.Pretty()) + fmt.Printf(" Peer ID: %s\n", id.Pretty()) tp, err := libp2ptls.New(priv) if err != nil { return err diff --git a/p2p/security/tls/example/server/main.go b/p2p/security/tls/example/server/main.go index d8371658b3..9988ec08b2 100644 --- a/p2p/security/tls/example/server/main.go +++ b/p2p/security/tls/example/server/main.go @@ -21,17 +21,36 @@ func main() { func startServer() error { port := flag.Int("p", 5533, "port") + keyType := flag.String("key", "ecdsa", "rsa, ecdsa, ed25519 or secp256k1") flag.Parse() - priv, _, err := ic.GenerateECDSAKeyPair(rand.Reader) + var priv ic.PrivKey + var err error + switch *keyType { + case "rsa": + fmt.Printf("Generated new peer with an RSA key.") + priv, _, err = ic.GenerateRSAKeyPair(2048, rand.Reader) + case "ecdsa": + fmt.Printf("Generated new peer with an ECDSA key.") + priv, _, err = ic.GenerateECDSAKeyPair(rand.Reader) + case "ed25519": + fmt.Printf("Generated new peer with an Ed25519 key.") + priv, _, err = ic.GenerateEd25519Key(rand.Reader) + case "secp256k1": + fmt.Printf("Generated new peer with an Secp256k1 key.") + priv, _, err = ic.GenerateSecp256k1Key(rand.Reader) + default: + return fmt.Errorf("unknown key type: %s", *keyType) + } if err != nil { return err } + id, err := peer.IDFromPrivateKey(priv) if err != nil { return err } - fmt.Printf("Generated new peer with an ECDSA key. Peer ID: %s\n", id.Pretty()) + fmt.Printf(" Peer ID: %s\n", id.Pretty()) tp, err := libp2ptls.New(priv) if err != nil { return err From e996c4ac2a879f5cf2887c30cb220934dc6ca508 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 21 Feb 2019 17:04:59 +0800 Subject: [PATCH 20/51] rename example to cmd, move to a single .go file --- p2p/security/tls/{example => cmd}/README.md | 2 +- p2p/security/tls/cmd/tlsdiag.go | 33 +++++++++++++++++++ .../client/main.go => cmd/tlsdiag/client.go} | 31 ++--------------- p2p/security/tls/cmd/tlsdiag/key.go | 28 ++++++++++++++++ .../server/main.go => cmd/tlsdiag/server.go} | 33 +++---------------- 5 files changed, 69 insertions(+), 58 deletions(-) rename p2p/security/tls/{example => cmd}/README.md (57%) create mode 100644 p2p/security/tls/cmd/tlsdiag.go rename p2p/security/tls/{example/client/main.go => cmd/tlsdiag/client.go} (61%) create mode 100644 p2p/security/tls/cmd/tlsdiag/key.go rename p2p/security/tls/{example/server/main.go => cmd/tlsdiag/server.go} (62%) diff --git a/p2p/security/tls/example/README.md b/p2p/security/tls/cmd/README.md similarity index 57% rename from p2p/security/tls/example/README.md rename to p2p/security/tls/cmd/README.md index dff0f66885..d0efa12ce6 100644 --- a/p2p/security/tls/example/README.md +++ b/p2p/security/tls/cmd/README.md @@ -2,5 +2,5 @@ Run ```bash -go run example/server/main.go +go run cmd/tlsdiag.go server ``` diff --git a/p2p/security/tls/cmd/tlsdiag.go b/p2p/security/tls/cmd/tlsdiag.go new file mode 100644 index 0000000000..73aef5fe60 --- /dev/null +++ b/p2p/security/tls/cmd/tlsdiag.go @@ -0,0 +1,33 @@ +package main + +import ( + "fmt" + "os" + + "github.com/libp2p/go-libp2p-tls/cmd/cmdimpl" +) + +func main() { + if len(os.Args) <= 1 { + fmt.Println("missing argument: client / server") + return + } + + role := os.Args[1] + // remove the role argument from os.Args + os.Args = append([]string{os.Args[0]}, os.Args[2:]...) + + var err error + switch role { + case "client": + err = cmdimpl.StartClient() + case "server": + err = cmdimpl.StartServer() + default: + fmt.Println("invalid argument. Expected client / server") + return + } + if err != nil { + panic(err) + } +} diff --git a/p2p/security/tls/example/client/main.go b/p2p/security/tls/cmd/tlsdiag/client.go similarity index 61% rename from p2p/security/tls/example/client/main.go rename to p2p/security/tls/cmd/tlsdiag/client.go index 678ece6ca6..9de3347846 100644 --- a/p2p/security/tls/example/client/main.go +++ b/p2p/security/tls/cmd/tlsdiag/client.go @@ -1,49 +1,24 @@ -package main +package cmdimpl import ( "context" - "crypto/rand" "flag" "fmt" "io/ioutil" "net" "time" - ic "github.com/libp2p/go-libp2p-crypto" peer "github.com/libp2p/go-libp2p-peer" libp2ptls "github.com/libp2p/go-libp2p-tls" ) -func main() { - if err := startClient(); err != nil { - panic(err) - } -} - -func startClient() error { +func StartClient() error { port := flag.Int("p", 5533, "port") peerIDString := flag.String("id", "", "peer ID") keyType := flag.String("key", "ecdsa", "rsa, ecdsa, ed25519 or secp256k1") flag.Parse() - var priv ic.PrivKey - var err error - switch *keyType { - case "rsa": - fmt.Printf("Generated new peer with an RSA key.") - priv, _, err = ic.GenerateRSAKeyPair(2048, rand.Reader) - case "ecdsa": - fmt.Printf("Generated new peer with an ECDSA key.") - priv, _, err = ic.GenerateECDSAKeyPair(rand.Reader) - case "ed25519": - fmt.Printf("Generated new peer with an Ed25519 key.") - priv, _, err = ic.GenerateEd25519Key(rand.Reader) - case "secp256k1": - fmt.Printf("Generated new peer with an Secp256k1 key.") - priv, _, err = ic.GenerateSecp256k1Key(rand.Reader) - default: - return fmt.Errorf("unknown key type: %s", *keyType) - } + priv, err := generateKey(*keyType) if err != nil { return err } diff --git a/p2p/security/tls/cmd/tlsdiag/key.go b/p2p/security/tls/cmd/tlsdiag/key.go new file mode 100644 index 0000000000..fc2ad8a788 --- /dev/null +++ b/p2p/security/tls/cmd/tlsdiag/key.go @@ -0,0 +1,28 @@ +package cmdimpl + +import ( + "crypto/rand" + "fmt" + + ic "github.com/libp2p/go-libp2p-crypto" +) + +func generateKey(keyType string) (priv ic.PrivKey, err error) { + switch keyType { + case "rsa": + fmt.Printf("Generated new peer with an RSA key.") + priv, _, err = ic.GenerateRSAKeyPair(2048, rand.Reader) + case "ecdsa": + fmt.Printf("Generated new peer with an ECDSA key.") + priv, _, err = ic.GenerateECDSAKeyPair(rand.Reader) + case "ed25519": + fmt.Printf("Generated new peer with an Ed25519 key.") + priv, _, err = ic.GenerateEd25519Key(rand.Reader) + case "secp256k1": + fmt.Printf("Generated new peer with an Secp256k1 key.") + priv, _, err = ic.GenerateSecp256k1Key(rand.Reader) + default: + return nil, fmt.Errorf("unknown key type: %s", keyType) + } + return +} diff --git a/p2p/security/tls/example/server/main.go b/p2p/security/tls/cmd/tlsdiag/server.go similarity index 62% rename from p2p/security/tls/example/server/main.go rename to p2p/security/tls/cmd/tlsdiag/server.go index 9988ec08b2..b192c44bba 100644 --- a/p2p/security/tls/example/server/main.go +++ b/p2p/security/tls/cmd/tlsdiag/server.go @@ -1,47 +1,22 @@ -package main +package cmdimpl import ( "context" - "crypto/rand" "flag" "fmt" "net" "time" - ic "github.com/libp2p/go-libp2p-crypto" peer "github.com/libp2p/go-libp2p-peer" libp2ptls "github.com/libp2p/go-libp2p-tls" ) -func main() { - if err := startServer(); err != nil { - panic(err) - } -} - -func startServer() error { +func StartServer() error { port := flag.Int("p", 5533, "port") keyType := flag.String("key", "ecdsa", "rsa, ecdsa, ed25519 or secp256k1") flag.Parse() - var priv ic.PrivKey - var err error - switch *keyType { - case "rsa": - fmt.Printf("Generated new peer with an RSA key.") - priv, _, err = ic.GenerateRSAKeyPair(2048, rand.Reader) - case "ecdsa": - fmt.Printf("Generated new peer with an ECDSA key.") - priv, _, err = ic.GenerateECDSAKeyPair(rand.Reader) - case "ed25519": - fmt.Printf("Generated new peer with an Ed25519 key.") - priv, _, err = ic.GenerateEd25519Key(rand.Reader) - case "secp256k1": - fmt.Printf("Generated new peer with an Secp256k1 key.") - priv, _, err = ic.GenerateSecp256k1Key(rand.Reader) - default: - return fmt.Errorf("unknown key type: %s", *keyType) - } + priv, err := generateKey(*keyType) if err != nil { return err } @@ -62,7 +37,7 @@ func startServer() error { } fmt.Printf("Listening for new connections on %s\n", ln.Addr()) fmt.Printf("Now run the following command in a separate terminal:\n") - fmt.Printf("\tgo run example/client/main.go -p %d -id %s\n", *port, id.Pretty()) + fmt.Printf("\tgo run cmd/tlsdiag.go client -p %d -id %s\n", *port, id.Pretty()) for { conn, err := ln.Accept() From 20005517f5cfde5875bbbba888ec11bf667bc3a5 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 28 Feb 2019 09:44:58 +0900 Subject: [PATCH 21/51] fix package name of tlsdiag --- p2p/security/tls/cmd/tlsdiag.go | 6 +++--- p2p/security/tls/cmd/tlsdiag/client.go | 2 +- p2p/security/tls/cmd/tlsdiag/key.go | 2 +- p2p/security/tls/cmd/tlsdiag/server.go | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/p2p/security/tls/cmd/tlsdiag.go b/p2p/security/tls/cmd/tlsdiag.go index 73aef5fe60..4aa2f2f09c 100644 --- a/p2p/security/tls/cmd/tlsdiag.go +++ b/p2p/security/tls/cmd/tlsdiag.go @@ -4,7 +4,7 @@ import ( "fmt" "os" - "github.com/libp2p/go-libp2p-tls/cmd/cmdimpl" + "github.com/libp2p/go-libp2p-tls/cmd/tlsdiag" ) func main() { @@ -20,9 +20,9 @@ func main() { var err error switch role { case "client": - err = cmdimpl.StartClient() + err = tlsdiag.StartClient() case "server": - err = cmdimpl.StartServer() + err = tlsdiag.StartServer() default: fmt.Println("invalid argument. Expected client / server") return diff --git a/p2p/security/tls/cmd/tlsdiag/client.go b/p2p/security/tls/cmd/tlsdiag/client.go index 9de3347846..21e4d18098 100644 --- a/p2p/security/tls/cmd/tlsdiag/client.go +++ b/p2p/security/tls/cmd/tlsdiag/client.go @@ -1,4 +1,4 @@ -package cmdimpl +package tlsdiag import ( "context" diff --git a/p2p/security/tls/cmd/tlsdiag/key.go b/p2p/security/tls/cmd/tlsdiag/key.go index fc2ad8a788..038974225a 100644 --- a/p2p/security/tls/cmd/tlsdiag/key.go +++ b/p2p/security/tls/cmd/tlsdiag/key.go @@ -1,4 +1,4 @@ -package cmdimpl +package tlsdiag import ( "crypto/rand" diff --git a/p2p/security/tls/cmd/tlsdiag/server.go b/p2p/security/tls/cmd/tlsdiag/server.go index b192c44bba..0a895baf12 100644 --- a/p2p/security/tls/cmd/tlsdiag/server.go +++ b/p2p/security/tls/cmd/tlsdiag/server.go @@ -1,4 +1,4 @@ -package cmdimpl +package tlsdiag import ( "context" From c6c1a3447084cacae3684fe14ada5b0fafc218e0 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 20 Feb 2019 18:21:46 +0800 Subject: [PATCH 22/51] implement the new handshake --- p2p/security/tls/crypto.go | 138 ++++++++++++++---------- p2p/security/tls/extension.go | 23 ++++ p2p/security/tls/extension_test.go | 19 ++++ p2p/security/tls/transport_test.go | 163 +++++++++++++++++++++++++---- 4 files changed, 270 insertions(+), 73 deletions(-) create mode 100644 p2p/security/tls/extension.go create mode 100644 p2p/security/tls/extension_test.go diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index 0c31bf5d52..2b9cd52482 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -1,21 +1,31 @@ package libp2ptls import ( - "crypto" + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" "crypto/tls" "crypto/x509" + "crypto/x509/pkix" + "encoding/asn1" "errors" "fmt" "math/big" "time" + crypto "github.com/libp2p/go-libp2p-crypto" ic "github.com/libp2p/go-libp2p-crypto" - pb "github.com/libp2p/go-libp2p-crypto/pb" peer "github.com/libp2p/go-libp2p-peer" ) -const certValidityPeriod = 180 * 24 * time.Hour +const certValidityPeriod = 100 * 365 * 24 * time.Hour // ~100 years + +var extensionID = getPrefixedExtensionID([]int{1, 1}) + +type signedKey struct { + PubKey []byte + Signature []byte +} // Identity is used to secure connections type Identity struct { @@ -24,7 +34,7 @@ type Identity struct { // NewIdentity creates a new identity func NewIdentity(privKey ic.PrivKey) (*Identity, error) { - key, cert, err := keyToCertificate(privKey) + cert, err := keyToCertificate(privKey) if err != nil { return nil, err } @@ -33,10 +43,7 @@ func NewIdentity(privKey ic.PrivKey) (*Identity, error) { MinVersion: tls.VersionTLS13, InsecureSkipVerify: true, // This is not insecure here. We will verify the cert chain ourselves. ClientAuth: tls.RequireAnyClientCert, - Certificates: []tls.Certificate{{ - Certificate: [][]byte{cert.Raw}, - PrivateKey: key, - }}, + Certificates: []tls.Certificate{*cert}, VerifyPeerCertificate: func(_ [][]byte, _ [][]*x509.Certificate) error { panic("tls config not specialized for peer") }, @@ -95,70 +102,95 @@ func getRemotePubKey(chain []*x509.Certificate) (ic.PubKey, error) { if len(chain) != 1 { return nil, errors.New("expected one certificates in the chain") } + cert := chain[0] pool := x509.NewCertPool() - pool.AddCert(chain[0]) - if _, err := chain[0].Verify(x509.VerifyOptions{Roots: pool}); err != nil { + pool.AddCert(cert) + if _, err := cert.Verify(x509.VerifyOptions{Roots: pool}); err != nil { // If we return an x509 error here, it will be sent on the wire. // Wrap the error to avoid that. return nil, fmt.Errorf("certificate verification failed: %s", err) } - remotePubKey, err := x509.MarshalPKIXPublicKey(chain[0].PublicKey) + + var found bool + var keyExt pkix.Extension + // find the libp2p key extension, skipping all unknown extensions + for _, ext := range cert.Extensions { + if extensionIDEqual(ext.Id, extensionID) { + keyExt = ext + found = true + break + } + } + if !found { + return nil, errors.New("expected certificate to contain the key extension") + } + var sk signedKey + if _, err := asn1.Unmarshal(keyExt.Value, &sk); err != nil { + return nil, fmt.Errorf("unmarshalling signed certificate failed: %s", err) + } + pubKey, err := crypto.UnmarshalPublicKey(sk.PubKey) + if err != nil { + return nil, fmt.Errorf("unmarshalling public key failed: %s", err) + } + certKeyPub, err := x509.MarshalPKIXPublicKey(cert.PublicKey) if err != nil { return nil, err } - switch chain[0].PublicKeyAlgorithm { - case x509.RSA: - return ic.UnmarshalRsaPublicKey(remotePubKey) - case x509.ECDSA: - return ic.UnmarshalECDSAPublicKey(remotePubKey) - default: - return nil, fmt.Errorf("unexpected public key algorithm: %d", chain[0].PublicKeyAlgorithm) + valid, err := pubKey.Verify(certKeyPub, sk.Signature) + if err != nil { + return nil, fmt.Errorf("signature verification failed: %s", err) } + if !valid { + return nil, errors.New("signature invalid") + } + return pubKey, nil } -func keyToCertificate(sk ic.PrivKey) (crypto.PrivateKey, *x509.Certificate, error) { - sn, err := rand.Int(rand.Reader, big.NewInt(1<<62)) +func keyToCertificate(sk ic.PrivKey) (*tls.Certificate, error) { + certKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil { - return nil, nil, err - } - tmpl := &x509.Certificate{ - SerialNumber: sn, - NotBefore: time.Now().Add(-24 * time.Hour), - NotAfter: time.Now().Add(certValidityPeriod), + return nil, err } - var privateKey crypto.PrivateKey - var publicKey crypto.PublicKey - raw, err := sk.Raw() + keyBytes, err := crypto.MarshalPublicKey(sk.GetPublic()) if err != nil { - return nil, nil, err + return nil, err } - switch sk.Type() { - case pb.KeyType_RSA: - k, err := x509.ParsePKCS1PrivateKey(raw) - if err != nil { - return nil, nil, err - } - publicKey = &k.PublicKey - privateKey = k - case pb.KeyType_ECDSA: - k, err := x509.ParseECPrivateKey(raw) - if err != nil { - return nil, nil, err - } - publicKey = &k.PublicKey - privateKey = k - // TODO: add support for Ed25519 - default: - return nil, nil, errors.New("unsupported key type for TLS") + certKeyPub, err := x509.MarshalPKIXPublicKey(certKey.Public()) + if err != nil { + return nil, err + } + signature, err := sk.Sign(certKeyPub) + if err != nil { + return nil, err + } + value, err := asn1.Marshal(signedKey{ + PubKey: keyBytes, + Signature: signature, + }) + if err != nil { + return nil, err } - certDER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, publicKey, privateKey) + + sn, err := rand.Int(rand.Reader, big.NewInt(1<<62)) if err != nil { - return nil, nil, err + return nil, err + } + tmpl := &x509.Certificate{ + SerialNumber: sn, + NotBefore: time.Time{}, + NotAfter: time.Now().Add(certValidityPeriod), + // after calling CreateCertificate, these will end up in Certificate.Extensions + ExtraExtensions: []pkix.Extension{ + {Id: extensionID, Value: value}, + }, } - cert, err := x509.ParseCertificate(certDER) + certDER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, certKey.Public(), certKey) if err != nil { - return nil, nil, err + return nil, err } - return privateKey, cert, nil + return &tls.Certificate{ + Certificate: [][]byte{certDER}, + PrivateKey: certKey, + }, nil } diff --git a/p2p/security/tls/extension.go b/p2p/security/tls/extension.go new file mode 100644 index 0000000000..33c106d928 --- /dev/null +++ b/p2p/security/tls/extension.go @@ -0,0 +1,23 @@ +package libp2ptls + +// TODO: get an assigment for a valid OID +var extensionPrefix = []int{1, 3, 6, 1, 4, 1, 123456789} + +// getPrefixedExtensionID returns an Object Identifier +// that can be used in x509 Certificates. +func getPrefixedExtensionID(suffix []int) []int { + return append(extensionPrefix, suffix...) +} + +// extensionIDEqual compares two extension IDs. +func extensionIDEqual(a, b []int) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/p2p/security/tls/extension_test.go b/p2p/security/tls/extension_test.go new file mode 100644 index 0000000000..425e8f4a3c --- /dev/null +++ b/p2p/security/tls/extension_test.go @@ -0,0 +1,19 @@ +package libp2ptls + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("Extensions", func() { + It("generates a prefixed extension ID", func() { + Expect(getPrefixedExtensionID([]int{13, 37})).To(Equal([]int{1, 3, 6, 1, 4, 1, 123456789, 13, 37})) + }) + + It("compares extension IDs", func() { + Expect(extensionIDEqual([]int{1, 2, 3, 4}, []int{1, 2, 3, 4})).To(BeTrue()) + Expect(extensionIDEqual([]int{1, 2, 3, 4}, []int{1, 2, 3})).To(BeFalse()) + Expect(extensionIDEqual([]int{1, 2, 3}, []int{1, 2, 3, 4})).To(BeFalse()) + Expect(extensionIDEqual([]int{1, 2, 3, 4}, []int{4, 3, 2, 1})).To(BeFalse()) + }) +}) diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index 02076eed92..62a8f1cd15 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -2,12 +2,15 @@ package libp2ptls import ( "context" + "crypto" "crypto/ecdsa" "crypto/elliptic" "crypto/rand" "crypto/rsa" "crypto/tls" "crypto/x509" + "crypto/x509/pkix" + "encoding/asn1" "fmt" "math/big" mrand "math/rand" @@ -15,6 +18,7 @@ import ( "time" "github.com/onsi/gomega/gbytes" + "github.com/onsi/gomega/types" cs "github.com/libp2p/go-conn-security" ci "github.com/libp2p/go-libp2p-crypto" @@ -26,7 +30,7 @@ import ( type transform struct { name string apply func(*Identity) - remoteErr string // the error that the side validating the chain gets + remoteErr types.GomegaMatcher // the error that the side validating the chain gets } var _ = Describe("Transport", func() { @@ -37,17 +41,22 @@ var _ = Describe("Transport", func() { createPeer := func() (peer.ID, ci.PrivKey) { var priv ci.PrivKey - if mrand.Int()%2 == 0 { + var err error + switch mrand.Int() % 4 { + case 0: fmt.Fprintf(GinkgoWriter, " using an ECDSA key: ") - var err error priv, _, err = ci.GenerateECDSAKeyPair(rand.Reader) - Expect(err).ToNot(HaveOccurred()) - } else { + case 1: fmt.Fprintf(GinkgoWriter, " using an RSA key: ") - var err error priv, _, err = ci.GenerateRSAKeyPair(1024, rand.Reader) - Expect(err).ToNot(HaveOccurred()) + case 2: + fmt.Fprintf(GinkgoWriter, " using an Ed25519 key: ") + priv, _, err = ci.GenerateEd25519Key(rand.Reader) + case 3: + fmt.Fprintf(GinkgoWriter, " using an Ed25519 key: ") + priv, _, err = ci.GenerateSecp256k1Key(rand.Reader) } + Expect(err).ToNot(HaveOccurred()) id, err := peer.IDFromPrivateKey(priv) Expect(err).ToNot(HaveOccurred()) fmt.Fprintln(GinkgoWriter, id.Pretty()) @@ -212,37 +221,149 @@ var _ = Describe("Transport", func() { }} } + getCertWithKey := func(key crypto.Signer, tmpl *x509.Certificate) tls.Certificate { + cert, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key.Public(), key) + Expect(err).ToNot(HaveOccurred()) + return tls.Certificate{ + Certificate: [][]byte{cert}, + PrivateKey: key, + } + } + + getCert := func(tmpl *x509.Certificate) tls.Certificate { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + Expect(err).ToNot(HaveOccurred()) + return getCertWithKey(key, tmpl) + } + expiredCert := func(identity *Identity) { - tmpl := &x509.Certificate{ + cert := getCert(&x509.Certificate{ SerialNumber: big.NewInt(1), NotBefore: time.Now().Add(-time.Hour), NotAfter: time.Now().Add(-time.Minute), - } - key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + }) + identity.config.Certificates = []tls.Certificate{cert} + } + + noKeyExtension := func(identity *Identity) { + cert := getCert(&x509.Certificate{ + SerialNumber: big.NewInt(1), + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + }) + identity.config.Certificates = []tls.Certificate{cert} + } + + unparseableKeyExtension := func(identity *Identity) { + cert := getCert(&x509.Certificate{ + SerialNumber: big.NewInt(1), + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + ExtraExtensions: []pkix.Extension{ + {Id: extensionID, Value: []byte("foobar")}, + }, + }) + identity.config.Certificates = []tls.Certificate{cert} + } + + unparseableKey := func(identity *Identity) { + data, err := asn1.Marshal(signedKey{PubKey: []byte("foobar")}) Expect(err).ToNot(HaveOccurred()) - cert, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key.Public(), key) + cert := getCert(&x509.Certificate{ + SerialNumber: big.NewInt(1), + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + ExtraExtensions: []pkix.Extension{ + {Id: extensionID, Value: data}, + }, + }) + identity.config.Certificates = []tls.Certificate{cert} + } + + tooShortSignature := func(identity *Identity) { + key, _, err := ci.GenerateSecp256k1Key(rand.Reader) Expect(err).ToNot(HaveOccurred()) - identity.config.Certificates = []tls.Certificate{{ - Certificate: [][]byte{cert}, - PrivateKey: key, - }} + keyBytes, err := key.GetPublic().Bytes() + Expect(err).ToNot(HaveOccurred()) + data, err := asn1.Marshal(signedKey{ + PubKey: keyBytes, + Signature: []byte("foobar"), + }) + Expect(err).ToNot(HaveOccurred()) + cert := getCert(&x509.Certificate{ + SerialNumber: big.NewInt(1), + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + ExtraExtensions: []pkix.Extension{ + {Id: extensionID, Value: data}, + }, + }) + identity.config.Certificates = []tls.Certificate{cert} + } + + invalidSignature := func(identity *Identity) { + key, _, err := ci.GenerateSecp256k1Key(rand.Reader) + Expect(err).ToNot(HaveOccurred()) + keyBytes, err := key.GetPublic().Bytes() + Expect(err).ToNot(HaveOccurred()) + signature, err := key.Sign([]byte("foobar")) + Expect(err).ToNot(HaveOccurred()) + data, err := asn1.Marshal(signedKey{ + PubKey: keyBytes, + Signature: signature, + }) + Expect(err).ToNot(HaveOccurred()) + cert := getCert(&x509.Certificate{ + SerialNumber: big.NewInt(1), + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + ExtraExtensions: []pkix.Extension{ + {Id: extensionID, Value: data}, + }, + }) + identity.config.Certificates = []tls.Certificate{cert} } transforms := []transform{ { name: "private key used in the TLS handshake doesn't match the public key in the cert", apply: invalidateCertChain, - remoteErr: "tls: invalid certificate signature", + remoteErr: Equal("tls: invalid certificate signature"), }, { name: "certificate chain contains 2 certs", apply: twoCerts, - remoteErr: "expected one certificates in the chain", + remoteErr: Equal("expected one certificates in the chain"), }, { name: "cert is expired", apply: expiredCert, - remoteErr: "certificate verification failed: x509: certificate has expired or is not yet valid", + remoteErr: Equal("certificate verification failed: x509: certificate has expired or is not yet valid"), + }, + { + name: "cert doesn't have the key extension", + apply: noKeyExtension, + remoteErr: Equal("expected certificate to contain the key extension"), + }, + { + name: "key extension not parseable", + apply: unparseableKeyExtension, + remoteErr: ContainSubstring("asn1"), + }, + { + name: "key protobuf not parseable", + apply: unparseableKey, + remoteErr: ContainSubstring("unmarshalling public key failed: proto:"), + }, + { + name: "signature is malformed", + apply: tooShortSignature, + remoteErr: ContainSubstring("signature verification failed:"), + }, + { + name: "signature is invalid", + apply: invalidSignature, + remoteErr: Equal("signature invalid"), }, } @@ -262,7 +383,8 @@ var _ = Describe("Transport", func() { go func() { defer GinkgoRecover() _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) - Expect(err).To(MatchError(t.remoteErr)) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(t.remoteErr) close(done) }() @@ -297,7 +419,8 @@ var _ = Describe("Transport", func() { }() _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) - Expect(err).To(MatchError(t.remoteErr)) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(t.remoteErr) Eventually(done).Should(BeClosed()) }) } From afcc2e4cffb400cbb1dfa2a9e98c03a9cc603cd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Kripalani?= Date: Mon, 11 Mar 2019 23:06:32 +0900 Subject: [PATCH 23/51] fix logging when using secp256k1 key in tests Co-Authored-By: marten-seemann --- p2p/security/tls/transport_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index 62a8f1cd15..82944560e4 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -53,7 +53,7 @@ var _ = Describe("Transport", func() { fmt.Fprintf(GinkgoWriter, " using an Ed25519 key: ") priv, _, err = ci.GenerateEd25519Key(rand.Reader) case 3: - fmt.Fprintf(GinkgoWriter, " using an Ed25519 key: ") + fmt.Fprintf(GinkgoWriter, " using an secp256k1 key: ") priv, _, err = ci.GenerateSecp256k1Key(rand.Reader) } Expect(err).ToNot(HaveOccurred()) From a2bf05d8819975bf957d7c6c5e367011f668da85 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 14 Mar 2019 09:46:12 +0900 Subject: [PATCH 24/51] use the new Protocol Labs PEN for the certificate extension --- p2p/security/tls/extension.go | 3 +-- p2p/security/tls/extension_test.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/p2p/security/tls/extension.go b/p2p/security/tls/extension.go index 33c106d928..9472c77e83 100644 --- a/p2p/security/tls/extension.go +++ b/p2p/security/tls/extension.go @@ -1,7 +1,6 @@ package libp2ptls -// TODO: get an assigment for a valid OID -var extensionPrefix = []int{1, 3, 6, 1, 4, 1, 123456789} +var extensionPrefix = []int{1, 3, 6, 1, 4, 1, 53594} // getPrefixedExtensionID returns an Object Identifier // that can be used in x509 Certificates. diff --git a/p2p/security/tls/extension_test.go b/p2p/security/tls/extension_test.go index 425e8f4a3c..5a7ef47756 100644 --- a/p2p/security/tls/extension_test.go +++ b/p2p/security/tls/extension_test.go @@ -7,7 +7,7 @@ import ( var _ = Describe("Extensions", func() { It("generates a prefixed extension ID", func() { - Expect(getPrefixedExtensionID([]int{13, 37})).To(Equal([]int{1, 3, 6, 1, 4, 1, 123456789, 13, 37})) + Expect(getPrefixedExtensionID([]int{13, 37})).To(Equal([]int{1, 3, 6, 1, 4, 1, 53594, 13, 37})) }) It("compares extension IDs", func() { From a49a4b1ccf801bbccaabf4ec410b418279050d8a Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 11 Mar 2019 15:01:47 +0900 Subject: [PATCH 25/51] use ChaCha if one of the peers doesn't have AES hardware support --- p2p/security/tls/crypto.go | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index 2b9cd52482..c0ac657ec8 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -13,6 +13,8 @@ import ( "math/big" "time" + "golang.org/x/sys/cpu" + crypto "github.com/libp2p/go-libp2p-crypto" ic "github.com/libp2p/go-libp2p-crypto" peer "github.com/libp2p/go-libp2p-peer" @@ -40,10 +42,11 @@ func NewIdentity(privKey ic.PrivKey) (*Identity, error) { } 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, - Certificates: []tls.Certificate{*cert}, + MinVersion: tls.VersionTLS13, + PreferServerCipherSuites: preferServerCipherSuites(), + InsecureSkipVerify: true, // This is not insecure here. We will verify the cert chain ourselves. + ClientAuth: tls.RequireAnyClientCert, + Certificates: []tls.Certificate{*cert}, VerifyPeerCertificate: func(_ [][]byte, _ [][]*x509.Certificate) error { panic("tls config not specialized for peer") }, @@ -194,3 +197,25 @@ func keyToCertificate(sk ic.PrivKey) (*tls.Certificate, error) { PrivateKey: certKey, }, nil } + +// We want nodes without AES hardware (e.g. ARM) support to always use ChaCha. +// Only if both nodes have AES hardware support (e.g. x86), AES should be used. +// x86->x86: AES, ARM->x86: ChaCha, x86->ARM: ChaCha and ARM->ARM: Chacha +// This function returns true if we don't have AES hardware support, and false otherwise. +// Thus, ARM servers will always use their own cipher suite preferences (ChaCha first), +// and x86 servers will aways use the client's cipher suite preferences. +func preferServerCipherSuites() bool { + // Copied from the Go TLS implementation. + + // Check the cpu flags for each platform that has optimized GCM implementations. + // Worst case, these variables will just all be false. + var ( + hasGCMAsmAMD64 = cpu.X86.HasAES && cpu.X86.HasPCLMULQDQ + hasGCMAsmARM64 = cpu.ARM64.HasAES && cpu.ARM64.HasPMULL + // Keep in sync with crypto/aes/cipher_s390x.go. + hasGCMAsmS390X = cpu.S390X.HasAES && cpu.S390X.HasAESCBC && cpu.S390X.HasAESCTR && (cpu.S390X.HasGHASH || cpu.S390X.HasAESGCM) + + hasGCMAsm = hasGCMAsmAMD64 || hasGCMAsmARM64 || hasGCMAsmS390X + ) + return !hasGCMAsm +} From 2b073e1ebf1659955c45b29744d705b2705fc82c Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 14 Mar 2019 11:19:39 +0900 Subject: [PATCH 26/51] use a prefix when signing the public key --- p2p/security/tls/crypto.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index c0ac657ec8..c991e33b4b 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -21,6 +21,7 @@ import ( ) const certValidityPeriod = 100 * 365 * 24 * time.Hour // ~100 years +const certificatePrefix = "libp2p-tls-handshake:" var extensionID = getPrefixedExtensionID([]int{1, 1}) @@ -139,7 +140,7 @@ func getRemotePubKey(chain []*x509.Certificate) (ic.PubKey, error) { if err != nil { return nil, err } - valid, err := pubKey.Verify(certKeyPub, sk.Signature) + valid, err := pubKey.Verify(append([]byte(certificatePrefix), certKeyPub...), sk.Signature) if err != nil { return nil, fmt.Errorf("signature verification failed: %s", err) } @@ -163,7 +164,7 @@ func keyToCertificate(sk ic.PrivKey) (*tls.Certificate, error) { if err != nil { return nil, err } - signature, err := sk.Sign(certKeyPub) + signature, err := sk.Sign(append([]byte(certificatePrefix), certKeyPub...)) if err != nil { return nil, err } From 2684cc16ea673c374a00415bbc71ffb5cc2b8cc3 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 7 Apr 2019 14:36:06 +0900 Subject: [PATCH 27/51] disable session tickets When resuming a session using session tickets, no certificate chain is presented, and the callbacks needed to verify the peer identity would not be called. --- p2p/security/tls/crypto.go | 1 + 1 file changed, 1 insertion(+) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index c991e33b4b..50c9d71a43 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -51,6 +51,7 @@ func NewIdentity(privKey ic.PrivKey) (*Identity, error) { VerifyPeerCertificate: func(_ [][]byte, _ [][]*x509.Certificate) error { panic("tls config not specialized for peer") }, + SessionTicketsDisabled: true, }, }, nil } From c37e733d403f0261be74d9fb8d9f8fc42368397e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Kripalani?= Date: Mon, 27 May 2019 12:00:30 +0100 Subject: [PATCH 28/51] migrate to consolidated types. (#30) --- p2p/security/tls/cmd/tlsdiag/client.go | 2 +- p2p/security/tls/cmd/tlsdiag/key.go | 2 +- p2p/security/tls/cmd/tlsdiag/server.go | 2 +- p2p/security/tls/conn.go | 8 ++++---- p2p/security/tls/crypto.go | 8 ++++---- p2p/security/tls/transport.go | 16 ++++++++-------- p2p/security/tls/transport_test.go | 11 ++++++----- 7 files changed, 25 insertions(+), 24 deletions(-) diff --git a/p2p/security/tls/cmd/tlsdiag/client.go b/p2p/security/tls/cmd/tlsdiag/client.go index 21e4d18098..8b0fc46fb0 100644 --- a/p2p/security/tls/cmd/tlsdiag/client.go +++ b/p2p/security/tls/cmd/tlsdiag/client.go @@ -8,7 +8,7 @@ import ( "net" "time" - peer "github.com/libp2p/go-libp2p-peer" + "github.com/libp2p/go-libp2p-core/peer" libp2ptls "github.com/libp2p/go-libp2p-tls" ) diff --git a/p2p/security/tls/cmd/tlsdiag/key.go b/p2p/security/tls/cmd/tlsdiag/key.go index 038974225a..557a485c35 100644 --- a/p2p/security/tls/cmd/tlsdiag/key.go +++ b/p2p/security/tls/cmd/tlsdiag/key.go @@ -4,7 +4,7 @@ import ( "crypto/rand" "fmt" - ic "github.com/libp2p/go-libp2p-crypto" + ic "github.com/libp2p/go-libp2p-core/crypto" ) func generateKey(keyType string) (priv ic.PrivKey, err error) { diff --git a/p2p/security/tls/cmd/tlsdiag/server.go b/p2p/security/tls/cmd/tlsdiag/server.go index 0a895baf12..290af13d63 100644 --- a/p2p/security/tls/cmd/tlsdiag/server.go +++ b/p2p/security/tls/cmd/tlsdiag/server.go @@ -7,7 +7,7 @@ import ( "net" "time" - peer "github.com/libp2p/go-libp2p-peer" + "github.com/libp2p/go-libp2p-core/peer" libp2ptls "github.com/libp2p/go-libp2p-tls" ) diff --git a/p2p/security/tls/conn.go b/p2p/security/tls/conn.go index d5450b1c6d..cf32fa459d 100644 --- a/p2p/security/tls/conn.go +++ b/p2p/security/tls/conn.go @@ -3,9 +3,9 @@ package libp2ptls import ( "crypto/tls" - cs "github.com/libp2p/go-conn-security" - ci "github.com/libp2p/go-libp2p-crypto" - peer "github.com/libp2p/go-libp2p-peer" + ci "github.com/libp2p/go-libp2p-core/crypto" + "github.com/libp2p/go-libp2p-core/peer" + "github.com/libp2p/go-libp2p-core/sec" ) type conn struct { @@ -18,7 +18,7 @@ type conn struct { remotePubKey ci.PubKey } -var _ cs.Conn = &conn{} +var _ sec.SecureConn = &conn{} func (c *conn) LocalPeer() peer.ID { return c.localPeer diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index 50c9d71a43..917c65a666 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -13,11 +13,11 @@ import ( "math/big" "time" + crypto "github.com/libp2p/go-libp2p-crypto" "golang.org/x/sys/cpu" - crypto "github.com/libp2p/go-libp2p-crypto" - ic "github.com/libp2p/go-libp2p-crypto" - peer "github.com/libp2p/go-libp2p-peer" + ic "github.com/libp2p/go-libp2p-core/crypto" + "github.com/libp2p/go-libp2p-core/peer" ) const certValidityPeriod = 100 * 365 * 24 * time.Hour // ~100 years @@ -133,7 +133,7 @@ func getRemotePubKey(chain []*x509.Certificate) (ic.PubKey, error) { if _, err := asn1.Unmarshal(keyExt.Value, &sk); err != nil { return nil, fmt.Errorf("unmarshalling signed certificate failed: %s", err) } - pubKey, err := crypto.UnmarshalPublicKey(sk.PubKey) + pubKey, err := ic.UnmarshalPublicKey(sk.PubKey) if err != nil { return nil, fmt.Errorf("unmarshalling public key failed: %s", err) } diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index 8b3e132f6b..2dc4257a3e 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -7,9 +7,9 @@ import ( "net" "os" - cs "github.com/libp2p/go-conn-security" - ci "github.com/libp2p/go-libp2p-crypto" - peer "github.com/libp2p/go-libp2p-peer" + ci "github.com/libp2p/go-libp2p-core/crypto" + "github.com/libp2p/go-libp2p-core/peer" + "github.com/libp2p/go-libp2p-core/sec" ) // TLS 1.3 is opt-in in Go 1.12 @@ -48,10 +48,10 @@ func New(key ci.PrivKey) (*Transport, error) { return t, nil } -var _ cs.Transport = &Transport{} +var _ sec.SecureTransport = &Transport{} // SecureInbound runs the TLS handshake as a server. -func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn) (cs.Conn, error) { +func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn) (sec.SecureConn, error) { config, keyCh := t.identity.ConfigForAny() return t.handshake(ctx, tls.Server(insecure, config), keyCh) } @@ -63,7 +63,7 @@ func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn) (cs.Co // application data immediately afterwards. // 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) { +func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, error) { config, keyCh := t.identity.ConfigForPeer(p) return t.handshake(ctx, tls.Client(insecure, config), keyCh) } @@ -72,7 +72,7 @@ func (t *Transport) handshake( ctx context.Context, tlsConn *tls.Conn, keyCh <-chan ci.PubKey, -) (cs.Conn, error) { +) (sec.SecureConn, error) { // There's no way to pass a context to tls.Conn.Handshake(). // See https://github.com/golang/go/issues/18482. // Close the connection instead. @@ -117,7 +117,7 @@ func (t *Transport) handshake( return conn, nil } -func (t *Transport) setupConn(tlsConn *tls.Conn, remotePubKey ci.PubKey) (cs.Conn, error) { +func (t *Transport) setupConn(tlsConn *tls.Conn, remotePubKey ci.PubKey) (sec.SecureConn, error) { if remotePubKey == nil { return nil, errors.New("go-libp2p-tls BUG: expected remote pub key to be set") } diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index 82944560e4..94f2c215f0 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -20,9 +20,10 @@ import ( "github.com/onsi/gomega/gbytes" "github.com/onsi/gomega/types" - cs "github.com/libp2p/go-conn-security" - ci "github.com/libp2p/go-libp2p-crypto" - peer "github.com/libp2p/go-libp2p-peer" + ci "github.com/libp2p/go-libp2p-core/crypto" + "github.com/libp2p/go-libp2p-core/peer" + "github.com/libp2p/go-libp2p-core/sec" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) @@ -94,7 +95,7 @@ var _ = Describe("Transport", func() { clientInsecureConn, serverInsecureConn := connect() - serverConnChan := make(chan cs.Conn) + serverConnChan := make(chan sec.SecureConn) go func() { defer GinkgoRecover() serverConn, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) @@ -103,7 +104,7 @@ var _ = Describe("Transport", func() { }() clientConn, err := clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) Expect(err).ToNot(HaveOccurred()) - var serverConn cs.Conn + var serverConn sec.SecureConn Eventually(serverConnChan).Should(Receive(&serverConn)) defer clientConn.Close() defer serverConn.Close() From 250af2033927b8a0c299f793b71c2c3470d5049e Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 28 Jul 2019 06:40:50 +0900 Subject: [PATCH 29/51] don't use deprecated go-libp2p-crypto.MarshalPublicKey --- p2p/security/tls/crypto.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index 917c65a666..b3693b6af7 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -13,7 +13,6 @@ import ( "math/big" "time" - crypto "github.com/libp2p/go-libp2p-crypto" "golang.org/x/sys/cpu" ic "github.com/libp2p/go-libp2p-core/crypto" @@ -157,7 +156,7 @@ func keyToCertificate(sk ic.PrivKey) (*tls.Certificate, error) { return nil, err } - keyBytes, err := crypto.MarshalPublicKey(sk.GetPublic()) + keyBytes, err := ic.MarshalPublicKey(sk.GetPublic()) if err != nil { return nil, err } From 08736ac1f3ea870124ee33dbc6452324c22e5993 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 28 Jul 2019 06:49:08 +0900 Subject: [PATCH 30/51] set an ALPN value in the tls.Config --- p2p/security/tls/crypto.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index b3693b6af7..a0a52cea72 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -21,6 +21,7 @@ import ( const certValidityPeriod = 100 * 365 * 24 * time.Hour // ~100 years const certificatePrefix = "libp2p-tls-handshake:" +const alpn string = "libp2p" var extensionID = getPrefixedExtensionID([]int{1, 1}) @@ -50,6 +51,7 @@ func NewIdentity(privKey ic.PrivKey) (*Identity, error) { VerifyPeerCertificate: func(_ [][]byte, _ [][]*x509.Certificate) error { panic("tls config not specialized for peer") }, + NextProtos: []string{alpn}, SessionTicketsDisabled: true, }, }, nil From f7ede3724980911b3480c65da94fca88f0200fdd Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 29 Jul 2019 08:17:03 +0700 Subject: [PATCH 31/51] expose the function to derive the peer's public key from the cert chain --- p2p/security/tls/crypto.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index b3693b6af7..42f0b2e0e5 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -66,9 +66,7 @@ func (i *Identity) ConfigForAny() (*tls.Config, <-chan ic.PubKey) { // // 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, -) (*tls.Config, <-chan ic.PubKey) { +func (i *Identity) ConfigForPeer(remote peer.ID) (*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. @@ -88,7 +86,7 @@ func (i *Identity) ConfigForPeer( chain[i] = cert } - pubKey, err := getRemotePubKey(chain) + pubKey, err := PubKeyFromCertChain(chain) if err != nil { return err } @@ -101,8 +99,8 @@ func (i *Identity) ConfigForPeer( return conf, keyCh } -// getRemotePubKey derives the remote's public key from the certificate chain. -func getRemotePubKey(chain []*x509.Certificate) (ic.PubKey, error) { +// PubKeyFromCertChain verifies the certificate chain and extract the remote's public key. +func PubKeyFromCertChain(chain []*x509.Certificate) (ic.PubKey, error) { if len(chain) != 1 { return nil, errors.New("expected one certificates in the chain") } From ea13d7a1e9fdd0828d6fe086323f2e7e5e78369c Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 1 Aug 2019 09:18:41 +0700 Subject: [PATCH 32/51] make the error check for not receiving a public key more explicit --- p2p/security/tls/transport.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index 2dc4257a3e..c563d0937b 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -105,6 +105,9 @@ func (t *Transport) handshake( case remotePubKey = <-keyCh: default: } + if remotePubKey == nil { + return nil, errors.New("go-libp2p-tls BUG: expected remote pub key to be set") + } conn, err := t.setupConn(tlsConn, remotePubKey) if err != nil { @@ -118,10 +121,6 @@ func (t *Transport) handshake( } func (t *Transport) setupConn(tlsConn *tls.Conn, remotePubKey ci.PubKey) (sec.SecureConn, error) { - if remotePubKey == nil { - return nil, errors.New("go-libp2p-tls BUG: expected remote pub key to be set") - } - remotePeerID, err := peer.IDFromPublicKey(remotePubKey) if err != nil { return nil, err From 0328485c9f9d652c560c8500f3a5e09676d38918 Mon Sep 17 00:00:00 2001 From: Hector Sanjuan Date: Fri, 1 Nov 2019 21:52:28 +0100 Subject: [PATCH 33/51] Fix: Connection Closed after handshake The context-cancelled watchdog goroutine may start running way after the handshake has finished and the associated context has been cancelled (by the executeDial() function in go-libp2p-swarm usuaully). This results in the connection being closed right after being stablished. --- p2p/security/tls/transport.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index 2dc4257a3e..e081becbf0 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -6,6 +6,7 @@ import ( "errors" "net" "os" + "sync" ci "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/peer" @@ -81,9 +82,19 @@ func (t *Transport) handshake( tlsConn.Close() default: } + done := make(chan struct{}) + var wg sync.WaitGroup + + // Ensure that we do not return before + // either being done or having a context + // cancellation. + defer wg.Wait() defer close(done) + + wg.Add(1) go func() { + defer wg.Done() select { case <-done: case <-ctx.Done(): From 3b381d806f9ca6abda37e0668a5bcc67e9078018 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 24 Nov 2019 18:07:58 +0700 Subject: [PATCH 34/51] close the underlying connection when the handshake fails --- p2p/security/tls/transport.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index 453f7451e7..214853cb0f 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -54,7 +54,11 @@ var _ sec.SecureTransport = &Transport{} // SecureInbound runs the TLS handshake as a server. func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn) (sec.SecureConn, error) { config, keyCh := t.identity.ConfigForAny() - return t.handshake(ctx, tls.Server(insecure, config), keyCh) + cs, err := t.handshake(ctx, tls.Server(insecure, config), keyCh) + if err != nil { + insecure.Close() + } + return cs, err } // SecureOutbound runs the TLS handshake as a client. @@ -66,7 +70,11 @@ func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn) (sec.S // notice this after 1 RTT when calling Read. func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, error) { config, keyCh := t.identity.ConfigForPeer(p) - return t.handshake(ctx, tls.Client(insecure, config), keyCh) + cs, err := t.handshake(ctx, tls.Client(insecure, config), keyCh) + if err != nil { + insecure.Close() + } + return cs, err } func (t *Transport) handshake( From 04be62c1c8cb67e85cfc89947be906f8a7d11657 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 6 Dec 2019 21:22:27 -0500 Subject: [PATCH 35/51] chore: update deps And bump minimum key to 2048. --- p2p/security/tls/transport_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index 94f2c215f0..62e9b0e73a 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -49,7 +49,7 @@ var _ = Describe("Transport", func() { priv, _, err = ci.GenerateECDSAKeyPair(rand.Reader) case 1: fmt.Fprintf(GinkgoWriter, " using an RSA key: ") - priv, _, err = ci.GenerateRSAKeyPair(1024, rand.Reader) + priv, _, err = ci.GenerateRSAKeyPair(2048, rand.Reader) case 2: fmt.Fprintf(GinkgoWriter, " using an Ed25519 key: ") priv, _, err = ci.GenerateEd25519Key(rand.Reader) @@ -192,7 +192,7 @@ var _ = Describe("Transport", func() { invalidateCertChain := func(identity *Identity) { switch identity.config.Certificates[0].PrivateKey.(type) { case *rsa.PrivateKey: - key, err := rsa.GenerateKey(rand.Reader, 1024) + key, err := rsa.GenerateKey(rand.Reader, 2048) Expect(err).ToNot(HaveOccurred()) identity.config.Certificates[0].PrivateKey = key case *ecdsa.PrivateKey: From 1ca135881c276ac4919d585c78ec074fe75f6155 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 13 Mar 2020 09:40:51 +0700 Subject: [PATCH 36/51] update to Go 1.14 --- p2p/security/tls/transport_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index 62e9b0e73a..901c85e1ac 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -327,9 +327,12 @@ var _ = Describe("Transport", func() { transforms := []transform{ { - name: "private key used in the TLS handshake doesn't match the public key in the cert", - apply: invalidateCertChain, - remoteErr: Equal("tls: invalid certificate signature"), + name: "private key used in the TLS handshake doesn't match the public key in the cert", + apply: invalidateCertChain, + remoteErr: Or( + Equal("tls: invalid signature by the client certificate: ECDSA verification failure"), + Equal("tls: invalid signature by the server certificate: ECDSA verification failure"), + ), }, { name: "certificate chain contains 2 certs", @@ -339,7 +342,7 @@ var _ = Describe("Transport", func() { { name: "cert is expired", apply: expiredCert, - remoteErr: Equal("certificate verification failed: x509: certificate has expired or is not yet valid"), + remoteErr: ContainSubstring("certificate has expired or is not yet valid"), }, { name: "cert doesn't have the key extension", From 4fa1ab45d6e809cd56d2e5bf1fb5873310067b6f Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 21 Apr 2020 14:46:32 +0700 Subject: [PATCH 37/51] improve the error message returned when peer verification fails --- p2p/security/tls/crypto.go | 6 +++++- p2p/security/tls/transport_test.go | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index 14e1db02b5..e6d6d5fdab 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -93,7 +93,11 @@ func (i *Identity) ConfigForPeer(remote peer.ID) (*tls.Config, <-chan ic.PubKey) return err } if remote != "" && !remote.MatchesPublicKey(pubKey) { - return errors.New("peer IDs don't match") + peerID, err := peer.IDFromPublicKey(pubKey) + if err != nil { + peerID = peer.ID(fmt.Sprintf("(not determined: %s)", err.Error())) + } + return fmt.Errorf("peer IDs don't match: expected %s, got %s", remote, peerID) } keyCh <- pubKey return nil diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index 901c85e1ac..2b5be8f2a9 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -184,7 +184,8 @@ var _ = Describe("Transport", func() { }() // dial, but expect the wrong peer ID _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, thirdPartyID) - Expect(err).To(MatchError("peer IDs don't match")) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("peer IDs don't match")) Eventually(done).Should(BeClosed()) }) From 8aa3448da2933d91cb1fb3f89d9d0da62d7b74a6 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 30 Jul 2020 12:25:36 +0700 Subject: [PATCH 38/51] remove setting of the TLS 1.3 GODEBUG flag --- p2p/security/tls/transport.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index 214853cb0f..1a02f786e5 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -5,7 +5,6 @@ import ( "crypto/tls" "errors" "net" - "os" "sync" ci "github.com/libp2p/go-libp2p-core/crypto" @@ -13,12 +12,6 @@ import ( "github.com/libp2p/go-libp2p-core/sec" ) -// TLS 1.3 is opt-in in Go 1.12 -// Activate it by setting the tls13 GODEBUG flag. -func init() { - os.Setenv("GODEBUG", os.Getenv("GODEBUG")+",tls13=1") -} - // ID is the protocol ID (used when negotiating with multistream) const ID = "/tls/1.0.0" From aaa62b9eed6206ec7d87e4091f36d8a24bb48f0c Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 30 Mar 2021 11:20:09 +0700 Subject: [PATCH 39/51] fix usage of deprecated peer.IDB58Decode --- p2p/security/tls/cmd/tlsdiag/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/security/tls/cmd/tlsdiag/client.go b/p2p/security/tls/cmd/tlsdiag/client.go index 8b0fc46fb0..9bf2e69c51 100644 --- a/p2p/security/tls/cmd/tlsdiag/client.go +++ b/p2p/security/tls/cmd/tlsdiag/client.go @@ -23,7 +23,7 @@ func StartClient() error { return err } - peerID, err := peer.IDB58Decode(*peerIDString) + peerID, err := peer.Decode(*peerIDString) if err != nil { return err } From aa3fa7d6938a71365c343aef4f30ce993631d9a0 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 23 Jul 2021 12:09:01 +0200 Subject: [PATCH 40/51] fix deprecated call to key.Bytes --- p2p/security/tls/transport_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index 2b5be8f2a9..776c45eb24 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -285,7 +285,7 @@ var _ = Describe("Transport", func() { tooShortSignature := func(identity *Identity) { key, _, err := ci.GenerateSecp256k1Key(rand.Reader) Expect(err).ToNot(HaveOccurred()) - keyBytes, err := key.GetPublic().Bytes() + keyBytes, err := ci.MarshalPublicKey(key.GetPublic()) Expect(err).ToNot(HaveOccurred()) data, err := asn1.Marshal(signedKey{ PubKey: keyBytes, @@ -306,7 +306,7 @@ var _ = Describe("Transport", func() { invalidSignature := func(identity *Identity) { key, _, err := ci.GenerateSecp256k1Key(rand.Reader) Expect(err).ToNot(HaveOccurred()) - keyBytes, err := key.GetPublic().Bytes() + keyBytes, err := ci.MarshalPublicKey(key.GetPublic()) Expect(err).ToNot(HaveOccurred()) signature, err := key.Sign([]byte("foobar")) Expect(err).ToNot(HaveOccurred()) From 1e7a4d7b350ce4ab9f607e1cc5fd58f30145c0c3 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sat, 7 Aug 2021 11:43:15 +0100 Subject: [PATCH 41/51] fix: don't fail the handshake when the libp2p extension is critical --- p2p/security/tls/crypto.go | 22 ++++++--- p2p/security/tls/transport_test.go | 79 +++++++++++++++++------------- 2 files changed, 60 insertions(+), 41 deletions(-) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index e6d6d5fdab..5a2fcf4add 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -24,6 +24,7 @@ const certificatePrefix = "libp2p-tls-handshake:" const alpn string = "libp2p" var extensionID = getPrefixedExtensionID([]int{1, 1}) +var extensionCritical bool // so we can mark the extension critical in tests type signedKey struct { PubKey []byte @@ -113,12 +114,6 @@ func PubKeyFromCertChain(chain []*x509.Certificate) (ic.PubKey, error) { cert := chain[0] pool := x509.NewCertPool() pool.AddCert(cert) - if _, err := cert.Verify(x509.VerifyOptions{Roots: pool}); err != nil { - // If we return an x509 error here, it will be sent on the wire. - // Wrap the error to avoid that. - return nil, fmt.Errorf("certificate verification failed: %s", err) - } - var found bool var keyExt pkix.Extension // find the libp2p key extension, skipping all unknown extensions @@ -126,12 +121,25 @@ func PubKeyFromCertChain(chain []*x509.Certificate) (ic.PubKey, error) { if extensionIDEqual(ext.Id, extensionID) { keyExt = ext found = true + for i, oident := range cert.UnhandledCriticalExtensions { + if oident.Equal(ext.Id) { + // delete the extension from UnhandledCriticalExtensions + cert.UnhandledCriticalExtensions = append(cert.UnhandledCriticalExtensions[:i], cert.UnhandledCriticalExtensions[i+1:]...) + break + } + } break } } if !found { return nil, errors.New("expected certificate to contain the key extension") } + if _, err := cert.Verify(x509.VerifyOptions{Roots: pool}); err != nil { + // If we return an x509 error here, it will be sent on the wire. + // Wrap the error to avoid that. + return nil, fmt.Errorf("certificate verification failed: %s", err) + } + var sk signedKey if _, err := asn1.Unmarshal(keyExt.Value, &sk); err != nil { return nil, fmt.Errorf("unmarshalling signed certificate failed: %s", err) @@ -190,7 +198,7 @@ func keyToCertificate(sk ic.PrivKey) (*tls.Certificate, error) { NotAfter: time.Now().Add(certValidityPeriod), // after calling CreateCertificate, these will end up in Certificate.Extensions ExtraExtensions: []pkix.Extension{ - {Id: extensionID, Value: value}, + {Id: extensionID, Critical: extensionCritical, Value: value}, }, } certDER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, certKey.Public(), certKey) diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index 776c45eb24..a68164f4a7 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -87,42 +87,50 @@ var _ = Describe("Transport", func() { clientID, clientKey = createPeer() }) - It("handshakes", func() { - clientTransport, err := New(clientKey) - Expect(err).ToNot(HaveOccurred()) - serverTransport, err := New(serverKey) - Expect(err).ToNot(HaveOccurred()) + Context("successful handshakes", func() { + for _, critical := range []bool{true, false} { + crit := critical - clientInsecureConn, serverInsecureConn := connect() + It(fmt.Sprintf("handshakes, extension critical: %t", crit), func() { + extensionCritical = crit + defer func() { extensionCritical = false }() + clientTransport, err := New(clientKey) + Expect(err).ToNot(HaveOccurred()) + serverTransport, err := New(serverKey) + Expect(err).ToNot(HaveOccurred()) - serverConnChan := make(chan sec.SecureConn) - go func() { - defer GinkgoRecover() - serverConn, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) - Expect(err).ToNot(HaveOccurred()) - serverConnChan <- serverConn - }() - clientConn, err := clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) - Expect(err).ToNot(HaveOccurred()) - var serverConn sec.SecureConn - Eventually(serverConnChan).Should(Receive(&serverConn)) - defer clientConn.Close() - defer serverConn.Close() - Expect(clientConn.LocalPeer()).To(Equal(clientID)) - Expect(serverConn.LocalPeer()).To(Equal(serverID)) - Expect(clientConn.LocalPrivateKey()).To(Equal(clientKey)) - Expect(serverConn.LocalPrivateKey()).To(Equal(serverKey)) - Expect(clientConn.RemotePeer()).To(Equal(serverID)) - Expect(serverConn.RemotePeer()).To(Equal(clientID)) - Expect(clientConn.RemotePublicKey()).To(Equal(serverKey.GetPublic())) - Expect(serverConn.RemotePublicKey()).To(Equal(clientKey.GetPublic())) - // exchange some data - _, err = serverConn.Write([]byte("foobar")) - Expect(err).ToNot(HaveOccurred()) - b := make([]byte, 6) - _, err = clientConn.Read(b) - Expect(err).ToNot(HaveOccurred()) - Expect(string(b)).To(Equal("foobar")) + clientInsecureConn, serverInsecureConn := connect() + + serverConnChan := make(chan sec.SecureConn) + go func() { + defer GinkgoRecover() + serverConn, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) + Expect(err).ToNot(HaveOccurred()) + serverConnChan <- serverConn + }() + clientConn, err := clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) + Expect(err).ToNot(HaveOccurred()) + var serverConn sec.SecureConn + Eventually(serverConnChan).Should(Receive(&serverConn)) + defer clientConn.Close() + defer serverConn.Close() + Expect(clientConn.LocalPeer()).To(Equal(clientID)) + Expect(serverConn.LocalPeer()).To(Equal(serverID)) + Expect(clientConn.LocalPrivateKey()).To(Equal(clientKey)) + Expect(serverConn.LocalPrivateKey()).To(Equal(serverKey)) + Expect(clientConn.RemotePeer()).To(Equal(serverID)) + Expect(serverConn.RemotePeer()).To(Equal(clientID)) + Expect(clientConn.RemotePublicKey()).To(Equal(serverKey.GetPublic())) + Expect(serverConn.RemotePublicKey()).To(Equal(clientKey.GetPublic())) + // exchange some data + _, err = serverConn.Write([]byte("foobar")) + Expect(err).ToNot(HaveOccurred()) + b := make([]byte, 6) + _, err = clientConn.Read(b) + Expect(err).ToNot(HaveOccurred()) + Expect(string(b)).To(Equal("foobar")) + }) + } }) It("fails when the context of the outgoing connection is canceled", func() { @@ -243,6 +251,9 @@ var _ = Describe("Transport", func() { SerialNumber: big.NewInt(1), NotBefore: time.Now().Add(-time.Hour), NotAfter: time.Now().Add(-time.Minute), + ExtraExtensions: []pkix.Extension{ + {Id: extensionID, Value: []byte("foobar")}, + }, }) identity.config.Certificates = []tls.Certificate{cert} } From 958fc8e5b1e677bf929864f4a0dda02fbee8adf1 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 5 Sep 2021 12:28:09 +0100 Subject: [PATCH 42/51] fix keys used for generating cert chain in tests --- p2p/security/tls/transport_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index a68164f4a7..c75be5766b 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -223,7 +223,7 @@ var _ = Describe("Transport", func() { Expect(err).ToNot(HaveOccurred()) cert1, err := x509.ParseCertificate(cert1DER) Expect(err).ToNot(HaveOccurred()) - cert2DER, err := x509.CreateCertificate(rand.Reader, tmpl, cert1, key2.Public(), key2) + cert2DER, err := x509.CreateCertificate(rand.Reader, tmpl, cert1, key2.Public(), key1) Expect(err).ToNot(HaveOccurred()) identity.config.Certificates = []tls.Certificate{{ Certificate: [][]byte{cert2DER, cert1DER}, From 2bf69fb8d4a45a3a97850f34969ade1c58e75735 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 5 Sep 2021 15:37:54 +0100 Subject: [PATCH 43/51] add the peer ID to SecureInbound --- p2p/security/tls/cmd/tlsdiag/server.go | 2 +- p2p/security/tls/crypto.go | 5 -- p2p/security/tls/transport.go | 5 +- p2p/security/tls/transport_test.go | 116 ++++++++++++++++++------- 4 files changed, 90 insertions(+), 38 deletions(-) diff --git a/p2p/security/tls/cmd/tlsdiag/server.go b/p2p/security/tls/cmd/tlsdiag/server.go index 290af13d63..6934f10343 100644 --- a/p2p/security/tls/cmd/tlsdiag/server.go +++ b/p2p/security/tls/cmd/tlsdiag/server.go @@ -56,7 +56,7 @@ func StartServer() error { func handleConn(tp *libp2ptls.Transport, conn net.Conn) error { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - sconn, err := tp.SecureInbound(ctx, conn) + sconn, err := tp.SecureInbound(ctx, conn, "") if err != nil { return err } diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index 5a2fcf4add..61db27db71 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -58,11 +58,6 @@ func NewIdentity(privKey ic.PrivKey) (*Identity, error) { }, nil } -// ConfigForAny is a short-hand for ConfigForPeer(""). -func (i *Identity) ConfigForAny() (*tls.Config, <-chan ic.PubKey) { - return i.ConfigForPeer("") -} - // 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. diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index 1a02f786e5..6be3c79a79 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -45,8 +45,9 @@ func New(key ci.PrivKey) (*Transport, error) { var _ sec.SecureTransport = &Transport{} // SecureInbound runs the TLS handshake as a server. -func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn) (sec.SecureConn, error) { - config, keyCh := t.identity.ConfigForAny() +// If p is empty, connections from any peer are accepted. +func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, error) { + config, keyCh := t.identity.ConfigForPeer(p) cs, err := t.handshake(ctx, tls.Server(insecure, config), keyCh) if err != nil { insecure.Close() diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index c75be5766b..c85c64ea55 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -17,15 +17,14 @@ import ( "net" "time" - "github.com/onsi/gomega/gbytes" - "github.com/onsi/gomega/types" - ci "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/peer" "github.com/libp2p/go-libp2p-core/sec" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/onsi/gomega/gbytes" + "github.com/onsi/gomega/types" ) type transform struct { @@ -104,7 +103,7 @@ var _ = Describe("Transport", func() { serverConnChan := make(chan sec.SecureConn) go func() { defer GinkgoRecover() - serverConn, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) + serverConn, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, "") Expect(err).ToNot(HaveOccurred()) serverConnChan <- serverConn }() @@ -120,8 +119,8 @@ var _ = Describe("Transport", func() { Expect(serverConn.LocalPrivateKey()).To(Equal(serverKey)) Expect(clientConn.RemotePeer()).To(Equal(serverID)) Expect(serverConn.RemotePeer()).To(Equal(clientID)) - Expect(clientConn.RemotePublicKey()).To(Equal(serverKey.GetPublic())) - Expect(serverConn.RemotePublicKey()).To(Equal(clientKey.GetPublic())) + Expect(ci.KeyEqual(clientConn.RemotePublicKey(), serverKey.GetPublic())).To(BeTrue()) + Expect(ci.KeyEqual(serverConn.RemotePublicKey(), clientKey.GetPublic())).To(BeTrue()) // exchange some data _, err = serverConn.Write([]byte("foobar")) Expect(err).ToNot(HaveOccurred()) @@ -143,7 +142,7 @@ var _ = Describe("Transport", func() { go func() { defer GinkgoRecover() - _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) + _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, "") Expect(err).To(HaveOccurred()) }() ctx, cancel := context.WithCancel(context.Background()) @@ -164,37 +163,94 @@ var _ = Describe("Transport", func() { defer GinkgoRecover() ctx, cancel := context.WithCancel(context.Background()) cancel() - _, err := serverTransport.SecureInbound(ctx, serverInsecureConn) + _, err := serverTransport.SecureInbound(ctx, serverInsecureConn, "") Expect(err).To(MatchError(context.Canceled)) }() _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) Expect(err).To(HaveOccurred()) }) - It("fails if the peer ID doesn't match", func() { - fmt.Fprintf(GinkgoWriter, "Creating another peer") - thirdPartyID, _ := createPeer() + Context("peer ID checks", func() { + It("succeeds when the server checks the client's ID", func() { + serverTransport, err := New(serverKey) + Expect(err).ToNot(HaveOccurred()) + clientTransport, err := New(clientKey) + Expect(err).ToNot(HaveOccurred()) - serverTransport, err := New(serverKey) - Expect(err).ToNot(HaveOccurred()) - clientTransport, err := New(clientKey) - Expect(err).ToNot(HaveOccurred()) + clientInsecureConn, serverInsecureConn := connect() + done := make(chan struct{}) + go func() { + defer GinkgoRecover() + defer close(done) + conn, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, clientID) + Expect(err).ToNot(HaveOccurred()) + Expect(conn.RemotePeer()).To(Equal(clientID)) + b := make([]byte, 6) + _, err = conn.Read(b) + Expect(err).ToNot(HaveOccurred()) + Expect(string(b)).To(Equal("foobar")) + }() + conn, err := clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) + Expect(err).ToNot(HaveOccurred()) + defer conn.Close() + _, err = conn.Write([]byte("foobar")) + Expect(err).ToNot(HaveOccurred()) + Expect(conn.RemotePeer()).To(Equal(serverID)) + Eventually(done).Should(BeClosed()) + }) - clientInsecureConn, serverInsecureConn := connect() + It("fails if the peer ID doesn't match, for outgoing connections", func() { + fmt.Fprintf(GinkgoWriter, "Creating another peer") + thirdPartyID, _ := createPeer() - done := make(chan struct{}) - go func() { - defer GinkgoRecover() - _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) + serverTransport, err := New(serverKey) + Expect(err).ToNot(HaveOccurred()) + clientTransport, err := New(clientKey) + Expect(err).ToNot(HaveOccurred()) + + clientInsecureConn, serverInsecureConn := connect() + + done := make(chan struct{}) + go func() { + defer GinkgoRecover() + defer close(done) + _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, "") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("tls: bad certificate")) + }() + // dial, but expect the wrong peer ID + _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, thirdPartyID) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("tls: bad certificate")) - close(done) - }() - // dial, but expect the wrong peer ID - _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, thirdPartyID) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("peer IDs don't match")) - Eventually(done).Should(BeClosed()) + Expect(err.Error()).To(ContainSubstring("peer IDs don't match")) + Eventually(done).Should(BeClosed()) + }) + + It("fails if the peer ID doesn't match, for incoming connections", func() { + fmt.Fprintf(GinkgoWriter, "Creating another peer") + thirdPartyID, _ := createPeer() + + serverTransport, err := New(serverKey) + Expect(err).ToNot(HaveOccurred()) + clientTransport, err := New(clientKey) + Expect(err).ToNot(HaveOccurred()) + + clientInsecureConn, serverInsecureConn := connect() + + done := make(chan struct{}) + go func() { + defer GinkgoRecover() + defer close(done) + conn, err := clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) + Expect(err).ToNot(HaveOccurred()) + _, err = conn.Read([]byte{0}) + Expect(err.Error()).To(ContainSubstring("tls: bad certificate")) + }() + // accept connection, but expect the wrong peer ID + _, err = serverTransport.SecureInbound(context.Background(), serverInsecureConn, thirdPartyID) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("peer IDs don't match")) + Eventually(done).Should(BeClosed()) + }) }) Context("invalid certificates", func() { @@ -398,7 +454,7 @@ var _ = Describe("Transport", func() { done := make(chan struct{}) go func() { defer GinkgoRecover() - _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) + _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, "") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(t.remoteErr) close(done) @@ -428,7 +484,7 @@ var _ = Describe("Transport", func() { done := make(chan struct{}) go func() { defer GinkgoRecover() - _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn) + _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, "") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("remote error: tls:")) close(done) From 1b09f16b4216e23a7aa2303bcfe0c68686410a76 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 9 Nov 2021 11:31:16 +0000 Subject: [PATCH 44/51] set a random certificate issuer According to RFC3280, the issuer field must not be empty. --- p2p/security/tls/crypto.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index 61db27db71..c893cba0ec 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -183,7 +183,12 @@ func keyToCertificate(sk ic.PrivKey) (*tls.Certificate, error) { return nil, err } - sn, err := rand.Int(rand.Reader, big.NewInt(1<<62)) + bigNum := big.NewInt(1 << 62) + sn, err := rand.Int(rand.Reader, bigNum) + if err != nil { + return nil, err + } + subjectSN, err := rand.Int(rand.Reader, bigNum) if err != nil { return nil, err } @@ -191,6 +196,9 @@ func keyToCertificate(sk ic.PrivKey) (*tls.Certificate, error) { SerialNumber: sn, NotBefore: time.Time{}, NotAfter: time.Now().Add(certValidityPeriod), + // According to RFC 3280, the issuer field must be set, + // see https://datatracker.ietf.org/doc/html/rfc3280#section-4.1.2.4. + Subject: pkix.Name{SerialNumber: subjectSN.String()}, // after calling CreateCertificate, these will end up in Certificate.Extensions ExtraExtensions: []pkix.Extension{ {Id: extensionID, Critical: extensionCritical, Value: value}, From 433e650113d84915cb4672b99b9a5fbe2dd7c6da Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 22 Nov 2021 11:15:13 +0400 Subject: [PATCH 45/51] set an actual NotBefore time on the certificate --- p2p/security/tls/crypto.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index c893cba0ec..f3502aa740 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -194,7 +194,7 @@ func keyToCertificate(sk ic.PrivKey) (*tls.Certificate, error) { } tmpl := &x509.Certificate{ SerialNumber: sn, - NotBefore: time.Time{}, + NotBefore: time.Now().Add(-time.Hour), NotAfter: time.Now().Add(certValidityPeriod), // According to RFC 3280, the issuer field must be set, // see https://datatracker.ietf.org/doc/html/rfc3280#section-4.1.2.4. From 414ea4c9848ca5f06cdde96cbe20affd7bf953a9 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 4 Jan 2022 11:38:11 +0400 Subject: [PATCH 46/51] migrate the extension tests away from Ginkgo --- p2p/security/tls/extension_test.go | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/p2p/security/tls/extension_test.go b/p2p/security/tls/extension_test.go index 5a7ef47756..f50695a248 100644 --- a/p2p/security/tls/extension_test.go +++ b/p2p/security/tls/extension_test.go @@ -1,19 +1,18 @@ package libp2ptls import ( - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" + "testing" + + "github.com/stretchr/testify/require" ) -var _ = Describe("Extensions", func() { - It("generates a prefixed extension ID", func() { - Expect(getPrefixedExtensionID([]int{13, 37})).To(Equal([]int{1, 3, 6, 1, 4, 1, 53594, 13, 37})) - }) +func TestExtensionGenerating(t *testing.T) { + require.Equal(t, getPrefixedExtensionID([]int{13, 37}), []int{1, 3, 6, 1, 4, 1, 53594, 13, 37}) +} - It("compares extension IDs", func() { - Expect(extensionIDEqual([]int{1, 2, 3, 4}, []int{1, 2, 3, 4})).To(BeTrue()) - Expect(extensionIDEqual([]int{1, 2, 3, 4}, []int{1, 2, 3})).To(BeFalse()) - Expect(extensionIDEqual([]int{1, 2, 3}, []int{1, 2, 3, 4})).To(BeFalse()) - Expect(extensionIDEqual([]int{1, 2, 3, 4}, []int{4, 3, 2, 1})).To(BeFalse()) - }) -}) +func TestExtensionComparison(t *testing.T) { + require.True(t, extensionIDEqual([]int{1, 2, 3, 4}, []int{1, 2, 3, 4})) + require.False(t, extensionIDEqual([]int{1, 2, 3, 4}, []int{1, 2, 3})) + require.False(t, extensionIDEqual([]int{1, 2, 3}, []int{1, 2, 3, 4})) + require.False(t, extensionIDEqual([]int{1, 2, 3, 4}, []int{4, 3, 2, 1})) +} From 07909fc5452c1a6126b04ece8e1ba3e7d2d3069b Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 4 Jan 2022 13:00:44 +0400 Subject: [PATCH 47/51] migrate the transport tests away from Ginkgo --- p2p/security/tls/transport_test.go | 843 +++++++++++++++-------------- 1 file changed, 427 insertions(+), 416 deletions(-) diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index c85c64ea55..8106f815a7 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -15,486 +15,497 @@ import ( "math/big" mrand "math/rand" "net" + "testing" "time" - ci "github.com/libp2p/go-libp2p-core/crypto" + ic "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/peer" "github.com/libp2p/go-libp2p-core/sec" - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - "github.com/onsi/gomega/gbytes" - "github.com/onsi/gomega/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -type transform struct { - name string - apply func(*Identity) - remoteErr types.GomegaMatcher // the error that the side validating the chain gets +func createPeer(t *testing.T) (peer.ID, ic.PrivKey) { + var priv ic.PrivKey + var err error + switch mrand.Int() % 4 { + case 0: + priv, _, err = ic.GenerateECDSAKeyPair(rand.Reader) + case 1: + priv, _, err = ic.GenerateRSAKeyPair(2048, rand.Reader) + case 2: + priv, _, err = ic.GenerateEd25519Key(rand.Reader) + case 3: + priv, _, err = ic.GenerateSecp256k1Key(rand.Reader) + } + require.NoError(t, err) + id, err := peer.IDFromPrivateKey(priv) + require.NoError(t, err) + t.Logf("using a %s key: %s", priv.Type(), id.Pretty()) + return id, priv } -var _ = Describe("Transport", func() { - var ( - serverKey, clientKey ci.PrivKey - serverID, clientID peer.ID - ) - - createPeer := func() (peer.ID, ci.PrivKey) { - var priv ci.PrivKey - var err error - switch mrand.Int() % 4 { - case 0: - fmt.Fprintf(GinkgoWriter, " using an ECDSA key: ") - priv, _, err = ci.GenerateECDSAKeyPair(rand.Reader) - case 1: - fmt.Fprintf(GinkgoWriter, " using an RSA key: ") - priv, _, err = ci.GenerateRSAKeyPair(2048, rand.Reader) - case 2: - fmt.Fprintf(GinkgoWriter, " using an Ed25519 key: ") - priv, _, err = ci.GenerateEd25519Key(rand.Reader) - case 3: - fmt.Fprintf(GinkgoWriter, " using an secp256k1 key: ") - priv, _, err = ci.GenerateSecp256k1Key(rand.Reader) - } - Expect(err).ToNot(HaveOccurred()) - id, err := peer.IDFromPrivateKey(priv) - Expect(err).ToNot(HaveOccurred()) - fmt.Fprintln(GinkgoWriter, id.Pretty()) - return id, priv - } +func connect(t *testing.T) (net.Conn, net.Conn) { + ln, err := net.Listen("tcp", "localhost:0") + require.NoError(t, err) + defer ln.Close() + serverConnChan := make(chan net.Conn) + go func() { + conn, err := ln.Accept() + assert.NoError(t, err) + serverConnChan <- conn + }() + conn, err := net.Dial("tcp", ln.Addr().String()) + require.NoError(t, err) + return conn, <-serverConnChan +} + +func TestHandshakeSucceeds(t *testing.T) { + clientID, clientKey := createPeer(t) + serverID, serverKey := createPeer(t) + + handshake := func(t *testing.T) { + clientTransport, err := New(clientKey) + require.NoError(t, err) + serverTransport, err := New(serverKey) + require.NoError(t, err) - connect := func() (net.Conn, net.Conn) { - ln, err := net.Listen("tcp", "localhost:0") - Expect(err).ToNot(HaveOccurred()) - defer ln.Close() - serverConnChan := make(chan net.Conn) + clientInsecureConn, serverInsecureConn := connect(t) + + serverConnChan := make(chan sec.SecureConn) go func() { - defer GinkgoRecover() - conn, err := ln.Accept() - Expect(err).ToNot(HaveOccurred()) - serverConnChan <- conn + serverConn, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, "") + require.NoError(t, err) + serverConnChan <- serverConn }() - conn, err := net.Dial("tcp", ln.Addr().String()) - Expect(err).ToNot(HaveOccurred()) - return conn, <-serverConnChan + + clientConn, err := clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) + require.NoError(t, err) + defer clientConn.Close() + + var serverConn sec.SecureConn + select { + case serverConn = <-serverConnChan: + case <-time.After(250 * time.Millisecond): + t.Fatal("expected the server to accept a connection") + } + defer serverConn.Close() + + require.Equal(t, clientConn.LocalPeer(), clientID) + require.Equal(t, serverConn.LocalPeer(), serverID) + require.True(t, clientConn.LocalPrivateKey().Equals(clientKey), "client private key mismatch") + require.True(t, serverConn.LocalPrivateKey().Equals(serverKey), "server private key mismatch") + require.Equal(t, clientConn.RemotePeer(), serverID) + require.Equal(t, serverConn.RemotePeer(), clientID) + require.True(t, clientConn.RemotePublicKey().Equals(serverKey.GetPublic()), "server public key mismatch") + require.True(t, serverConn.RemotePublicKey().Equals(clientKey.GetPublic()), "client public key mismatch") + // exchange some data + _, err = serverConn.Write([]byte("foobar")) + require.NoError(t, err) + b := make([]byte, 6) + _, err = clientConn.Read(b) + require.NoError(t, err) + require.Equal(t, string(b), "foobar") } - BeforeEach(func() { - fmt.Fprintf(GinkgoWriter, "Initializing a server") - serverID, serverKey = createPeer() - fmt.Fprintf(GinkgoWriter, "Initializing a client") - clientID, clientKey = createPeer() + t.Run("with extension not critical", func(t *testing.T) { + handshake(t) }) - Context("successful handshakes", func() { - for _, critical := range []bool{true, false} { - crit := critical - - It(fmt.Sprintf("handshakes, extension critical: %t", crit), func() { - extensionCritical = crit - defer func() { extensionCritical = false }() - clientTransport, err := New(clientKey) - Expect(err).ToNot(HaveOccurred()) - serverTransport, err := New(serverKey) - Expect(err).ToNot(HaveOccurred()) - - clientInsecureConn, serverInsecureConn := connect() - - serverConnChan := make(chan sec.SecureConn) - go func() { - defer GinkgoRecover() - serverConn, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, "") - Expect(err).ToNot(HaveOccurred()) - serverConnChan <- serverConn - }() - clientConn, err := clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) - Expect(err).ToNot(HaveOccurred()) - var serverConn sec.SecureConn - Eventually(serverConnChan).Should(Receive(&serverConn)) - defer clientConn.Close() - defer serverConn.Close() - Expect(clientConn.LocalPeer()).To(Equal(clientID)) - Expect(serverConn.LocalPeer()).To(Equal(serverID)) - Expect(clientConn.LocalPrivateKey()).To(Equal(clientKey)) - Expect(serverConn.LocalPrivateKey()).To(Equal(serverKey)) - Expect(clientConn.RemotePeer()).To(Equal(serverID)) - Expect(serverConn.RemotePeer()).To(Equal(clientID)) - Expect(ci.KeyEqual(clientConn.RemotePublicKey(), serverKey.GetPublic())).To(BeTrue()) - Expect(ci.KeyEqual(serverConn.RemotePublicKey(), clientKey.GetPublic())).To(BeTrue()) - // exchange some data - _, err = serverConn.Write([]byte("foobar")) - Expect(err).ToNot(HaveOccurred()) - b := make([]byte, 6) - _, err = clientConn.Read(b) - Expect(err).ToNot(HaveOccurred()) - Expect(string(b)).To(Equal("foobar")) - }) - } + t.Run("with extension critical", func(t *testing.T) { + extensionCritical = true + t.Cleanup(func() { extensionCritical = false }) + + handshake(t) }) +} - It("fails when the context of the outgoing connection is canceled", func() { - clientTransport, err := New(clientKey) - Expect(err).ToNot(HaveOccurred()) - serverTransport, err := New(serverKey) - Expect(err).ToNot(HaveOccurred()) +func TestHandshakeConnectionCancelations(t *testing.T) { + _, clientKey := createPeer(t) + serverID, serverKey := createPeer(t) + + clientTransport, err := New(clientKey) + require.NoError(t, err) + serverTransport, err := New(serverKey) + require.NoError(t, err) - clientInsecureConn, serverInsecureConn := connect() + t.Run("cancel outgoing connection", func(t *testing.T) { + clientInsecureConn, serverInsecureConn := connect(t) + errChan := make(chan error) go func() { - defer GinkgoRecover() _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, "") - Expect(err).To(HaveOccurred()) + errChan <- err }() ctx, cancel := context.WithCancel(context.Background()) cancel() _, err = clientTransport.SecureOutbound(ctx, clientInsecureConn, serverID) - Expect(err).To(MatchError(context.Canceled)) + require.ErrorIs(t, err, context.Canceled) + require.Error(t, <-errChan) }) - It("fails when the context of the incoming connection is canceled", func() { - clientTransport, err := New(clientKey) - Expect(err).ToNot(HaveOccurred()) - serverTransport, err := New(serverKey) - Expect(err).ToNot(HaveOccurred()) - - clientInsecureConn, serverInsecureConn := connect() + t.Run("cancel incoming connection", func(t *testing.T) { + clientInsecureConn, serverInsecureConn := connect(t) + errChan := make(chan error) go func() { - defer GinkgoRecover() ctx, cancel := context.WithCancel(context.Background()) cancel() _, err := serverTransport.SecureInbound(ctx, serverInsecureConn, "") - Expect(err).To(MatchError(context.Canceled)) + errChan <- err }() _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) - Expect(err).To(HaveOccurred()) + require.Error(t, err) + require.ErrorIs(t, <-errChan, context.Canceled) }) +} - Context("peer ID checks", func() { - It("succeeds when the server checks the client's ID", func() { - serverTransport, err := New(serverKey) - Expect(err).ToNot(HaveOccurred()) - clientTransport, err := New(clientKey) - Expect(err).ToNot(HaveOccurred()) - - clientInsecureConn, serverInsecureConn := connect() - done := make(chan struct{}) - go func() { - defer GinkgoRecover() - defer close(done) - conn, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, clientID) - Expect(err).ToNot(HaveOccurred()) - Expect(conn.RemotePeer()).To(Equal(clientID)) - b := make([]byte, 6) - _, err = conn.Read(b) - Expect(err).ToNot(HaveOccurred()) - Expect(string(b)).To(Equal("foobar")) - }() - conn, err := clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) - Expect(err).ToNot(HaveOccurred()) - defer conn.Close() - _, err = conn.Write([]byte("foobar")) - Expect(err).ToNot(HaveOccurred()) - Expect(conn.RemotePeer()).To(Equal(serverID)) - Eventually(done).Should(BeClosed()) - }) +func TestPeerIDMismatch(t *testing.T) { + _, clientKey := createPeer(t) + serverID, serverKey := createPeer(t) - It("fails if the peer ID doesn't match, for outgoing connections", func() { - fmt.Fprintf(GinkgoWriter, "Creating another peer") - thirdPartyID, _ := createPeer() + serverTransport, err := New(serverKey) + require.NoError(t, err) + clientTransport, err := New(clientKey) + require.NoError(t, err) - serverTransport, err := New(serverKey) - Expect(err).ToNot(HaveOccurred()) - clientTransport, err := New(clientKey) - Expect(err).ToNot(HaveOccurred()) + t.Run("for outgoing connections", func(t *testing.T) { + clientInsecureConn, serverInsecureConn := connect(t) - clientInsecureConn, serverInsecureConn := connect() - - done := make(chan struct{}) - go func() { - defer GinkgoRecover() - defer close(done) - _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, "") - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("tls: bad certificate")) - }() - // dial, but expect the wrong peer ID - _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, thirdPartyID) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("peer IDs don't match")) - Eventually(done).Should(BeClosed()) - }) - - It("fails if the peer ID doesn't match, for incoming connections", func() { - fmt.Fprintf(GinkgoWriter, "Creating another peer") - thirdPartyID, _ := createPeer() + errChan := make(chan error) + go func() { + _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, "") + errChan <- err + }() - serverTransport, err := New(serverKey) - Expect(err).ToNot(HaveOccurred()) - clientTransport, err := New(clientKey) - Expect(err).ToNot(HaveOccurred()) + // dial, but expect the wrong peer ID + thirdPartyID, _ := createPeer(t) + _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, thirdPartyID) + require.Error(t, err) + require.Contains(t, err.Error(), "peer IDs don't match") + + var serverErr error + select { + case serverErr = <-errChan: + case <-time.After(250 * time.Millisecond): + t.Fatal("expected handshake to return on the server side") + } + require.Error(t, serverErr) + require.Contains(t, serverErr.Error(), "tls: bad certificate") + }) - clientInsecureConn, serverInsecureConn := connect() + t.Run("for incoming connections", func(t *testing.T) { + clientInsecureConn, serverInsecureConn := connect(t) - done := make(chan struct{}) - go func() { - defer GinkgoRecover() - defer close(done) - conn, err := clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) - Expect(err).ToNot(HaveOccurred()) - _, err = conn.Read([]byte{0}) - Expect(err.Error()).To(ContainSubstring("tls: bad certificate")) - }() - // accept connection, but expect the wrong peer ID - _, err = serverTransport.SecureInbound(context.Background(), serverInsecureConn, thirdPartyID) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("peer IDs don't match")) - Eventually(done).Should(BeClosed()) - }) - }) + errChan := make(chan error) + go func() { + thirdPartyID, _ := createPeer(t) + // expect the wrong peer ID + _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, thirdPartyID) + errChan <- err + }() - Context("invalid certificates", func() { - invalidateCertChain := func(identity *Identity) { - switch identity.config.Certificates[0].PrivateKey.(type) { - case *rsa.PrivateKey: - key, err := rsa.GenerateKey(rand.Reader, 2048) - Expect(err).ToNot(HaveOccurred()) - 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 - default: - Fail("unexpected private key type") - } + conn, err := clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) + require.NoError(t, err) + _, err = conn.Read([]byte{0}) + require.Error(t, err) + require.Contains(t, err.Error(), "tls: bad certificate") + + var serverErr error + select { + case serverErr = <-errChan: + case <-time.After(250 * time.Millisecond): + t.Fatal("expected handshake to return on the server side") } + require.Error(t, serverErr) + require.Contains(t, serverErr.Error(), "peer IDs don't match") + }) +} - twoCerts := func(identity *Identity) { - tmpl := &x509.Certificate{SerialNumber: big.NewInt(1)} - key1, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - Expect(err).ToNot(HaveOccurred()) - key2, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - Expect(err).ToNot(HaveOccurred()) - cert1DER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key1.Public(), key1) - Expect(err).ToNot(HaveOccurred()) - cert1, err := x509.ParseCertificate(cert1DER) - Expect(err).ToNot(HaveOccurred()) - cert2DER, err := x509.CreateCertificate(rand.Reader, tmpl, cert1, key2.Public(), key1) - Expect(err).ToNot(HaveOccurred()) - identity.config.Certificates = []tls.Certificate{{ - Certificate: [][]byte{cert2DER, cert1DER}, - PrivateKey: key2, - }} - } +func TestInvalidCerts(t *testing.T) { + _, clientKey := createPeer(t) + serverID, serverKey := createPeer(t) - getCertWithKey := func(key crypto.Signer, tmpl *x509.Certificate) tls.Certificate { - cert, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key.Public(), key) - Expect(err).ToNot(HaveOccurred()) - return tls.Certificate{ - Certificate: [][]byte{cert}, - PrivateKey: key, - } - } + type transform struct { + name string + apply func(*Identity) + checkErr func(*testing.T, error) // the error that the side validating the chain gets + } - getCert := func(tmpl *x509.Certificate) tls.Certificate { + invalidateCertChain := func(identity *Identity) { + switch identity.config.Certificates[0].PrivateKey.(type) { + case *rsa.PrivateKey: + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + identity.config.Certificates[0].PrivateKey = key + case *ecdsa.PrivateKey: key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - Expect(err).ToNot(HaveOccurred()) - return getCertWithKey(key, tmpl) + require.NoError(t, err) + identity.config.Certificates[0].PrivateKey = key + default: + t.Fatal("unexpected private key type") } + } - expiredCert := func(identity *Identity) { - cert := getCert(&x509.Certificate{ - SerialNumber: big.NewInt(1), - NotBefore: time.Now().Add(-time.Hour), - NotAfter: time.Now().Add(-time.Minute), - ExtraExtensions: []pkix.Extension{ - {Id: extensionID, Value: []byte("foobar")}, - }, - }) - identity.config.Certificates = []tls.Certificate{cert} - } + twoCerts := func(identity *Identity) { + tmpl := &x509.Certificate{SerialNumber: big.NewInt(1)} + key1, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + key2, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + cert1DER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key1.Public(), key1) + require.NoError(t, err) + cert1, err := x509.ParseCertificate(cert1DER) + require.NoError(t, err) + cert2DER, err := x509.CreateCertificate(rand.Reader, tmpl, cert1, key2.Public(), key1) + require.NoError(t, err) + identity.config.Certificates = []tls.Certificate{{ + Certificate: [][]byte{cert2DER, cert1DER}, + PrivateKey: key2, + }} + } - noKeyExtension := func(identity *Identity) { - cert := getCert(&x509.Certificate{ - SerialNumber: big.NewInt(1), - NotBefore: time.Now().Add(-time.Hour), - NotAfter: time.Now().Add(time.Hour), - }) - identity.config.Certificates = []tls.Certificate{cert} + getCertWithKey := func(key crypto.Signer, tmpl *x509.Certificate) tls.Certificate { + cert, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key.Public(), key) + require.NoError(t, err) + return tls.Certificate{ + Certificate: [][]byte{cert}, + PrivateKey: key, } + } - unparseableKeyExtension := func(identity *Identity) { - cert := getCert(&x509.Certificate{ - SerialNumber: big.NewInt(1), - NotBefore: time.Now().Add(-time.Hour), - NotAfter: time.Now().Add(time.Hour), - ExtraExtensions: []pkix.Extension{ - {Id: extensionID, Value: []byte("foobar")}, - }, - }) - identity.config.Certificates = []tls.Certificate{cert} - } + getCert := func(tmpl *x509.Certificate) tls.Certificate { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + return getCertWithKey(key, tmpl) + } - unparseableKey := func(identity *Identity) { - data, err := asn1.Marshal(signedKey{PubKey: []byte("foobar")}) - Expect(err).ToNot(HaveOccurred()) - cert := getCert(&x509.Certificate{ - SerialNumber: big.NewInt(1), - NotBefore: time.Now().Add(-time.Hour), - NotAfter: time.Now().Add(time.Hour), - ExtraExtensions: []pkix.Extension{ - {Id: extensionID, Value: data}, - }, - }) - identity.config.Certificates = []tls.Certificate{cert} - } + expiredCert := func(identity *Identity) { + cert := getCert(&x509.Certificate{ + SerialNumber: big.NewInt(1), + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(-time.Minute), + ExtraExtensions: []pkix.Extension{ + {Id: extensionID, Value: []byte("foobar")}, + }, + }) + identity.config.Certificates = []tls.Certificate{cert} + } - tooShortSignature := func(identity *Identity) { - key, _, err := ci.GenerateSecp256k1Key(rand.Reader) - Expect(err).ToNot(HaveOccurred()) - keyBytes, err := ci.MarshalPublicKey(key.GetPublic()) - Expect(err).ToNot(HaveOccurred()) - data, err := asn1.Marshal(signedKey{ - PubKey: keyBytes, - Signature: []byte("foobar"), - }) - Expect(err).ToNot(HaveOccurred()) - cert := getCert(&x509.Certificate{ - SerialNumber: big.NewInt(1), - NotBefore: time.Now().Add(-time.Hour), - NotAfter: time.Now().Add(time.Hour), - ExtraExtensions: []pkix.Extension{ - {Id: extensionID, Value: data}, - }, - }) - identity.config.Certificates = []tls.Certificate{cert} - } + noKeyExtension := func(identity *Identity) { + cert := getCert(&x509.Certificate{ + SerialNumber: big.NewInt(1), + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + }) + identity.config.Certificates = []tls.Certificate{cert} + } - invalidSignature := func(identity *Identity) { - key, _, err := ci.GenerateSecp256k1Key(rand.Reader) - Expect(err).ToNot(HaveOccurred()) - keyBytes, err := ci.MarshalPublicKey(key.GetPublic()) - Expect(err).ToNot(HaveOccurred()) - signature, err := key.Sign([]byte("foobar")) - Expect(err).ToNot(HaveOccurred()) - data, err := asn1.Marshal(signedKey{ - PubKey: keyBytes, - Signature: signature, - }) - Expect(err).ToNot(HaveOccurred()) - cert := getCert(&x509.Certificate{ - SerialNumber: big.NewInt(1), - NotBefore: time.Now().Add(-time.Hour), - NotAfter: time.Now().Add(time.Hour), - ExtraExtensions: []pkix.Extension{ - {Id: extensionID, Value: data}, - }, - }) - identity.config.Certificates = []tls.Certificate{cert} - } + unparseableKeyExtension := func(identity *Identity) { + cert := getCert(&x509.Certificate{ + SerialNumber: big.NewInt(1), + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + ExtraExtensions: []pkix.Extension{ + {Id: extensionID, Value: []byte("foobar")}, + }, + }) + identity.config.Certificates = []tls.Certificate{cert} + } - transforms := []transform{ - { - name: "private key used in the TLS handshake doesn't match the public key in the cert", - apply: invalidateCertChain, - remoteErr: Or( - Equal("tls: invalid signature by the client certificate: ECDSA verification failure"), - Equal("tls: invalid signature by the server certificate: ECDSA verification failure"), - ), + unparseableKey := func(identity *Identity) { + data, err := asn1.Marshal(signedKey{PubKey: []byte("foobar")}) + require.NoError(t, err) + cert := getCert(&x509.Certificate{ + SerialNumber: big.NewInt(1), + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + ExtraExtensions: []pkix.Extension{ + {Id: extensionID, Value: data}, }, - { - name: "certificate chain contains 2 certs", - apply: twoCerts, - remoteErr: Equal("expected one certificates in the chain"), + }) + identity.config.Certificates = []tls.Certificate{cert} + } + + tooShortSignature := func(identity *Identity) { + key, _, err := ic.GenerateSecp256k1Key(rand.Reader) + require.NoError(t, err) + keyBytes, err := ic.MarshalPublicKey(key.GetPublic()) + require.NoError(t, err) + data, err := asn1.Marshal(signedKey{ + PubKey: keyBytes, + Signature: []byte("foobar"), + }) + require.NoError(t, err) + cert := getCert(&x509.Certificate{ + SerialNumber: big.NewInt(1), + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + ExtraExtensions: []pkix.Extension{ + {Id: extensionID, Value: data}, }, - { - name: "cert is expired", - apply: expiredCert, - remoteErr: ContainSubstring("certificate has expired or is not yet valid"), + }) + identity.config.Certificates = []tls.Certificate{cert} + } + + invalidSignature := func(identity *Identity) { + key, _, err := ic.GenerateSecp256k1Key(rand.Reader) + require.NoError(t, err) + keyBytes, err := ic.MarshalPublicKey(key.GetPublic()) + require.NoError(t, err) + signature, err := key.Sign([]byte("foobar")) + require.NoError(t, err) + data, err := asn1.Marshal(signedKey{ + PubKey: keyBytes, + Signature: signature, + }) + require.NoError(t, err) + cert := getCert(&x509.Certificate{ + SerialNumber: big.NewInt(1), + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + ExtraExtensions: []pkix.Extension{ + {Id: extensionID, Value: data}, }, - { - name: "cert doesn't have the key extension", - apply: noKeyExtension, - remoteErr: Equal("expected certificate to contain the key extension"), + }) + identity.config.Certificates = []tls.Certificate{cert} + } + + transforms := []transform{ + { + name: "private key used in the TLS handshake doesn't match the public key in the cert", + apply: invalidateCertChain, + checkErr: func(t *testing.T, err error) { + if err.Error() != "tls: invalid signature by the client certificate: ECDSA verification failure" && + err.Error() != "tls: invalid signature by the server certificate: ECDSA verification failure" { + t.Fatalf("unexpected error message: %s", err) + } }, - { - name: "key extension not parseable", - apply: unparseableKeyExtension, - remoteErr: ContainSubstring("asn1"), + }, + { + name: "certificate chain contains 2 certs", + apply: twoCerts, + checkErr: func(t *testing.T, err error) { + require.EqualError(t, err, "expected one certificates in the chain") }, - { - name: "key protobuf not parseable", - apply: unparseableKey, - remoteErr: ContainSubstring("unmarshalling public key failed: proto:"), + }, + { + name: "cert is expired", + apply: expiredCert, + checkErr: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "certificate has expired or is not yet valid") }, - { - name: "signature is malformed", - apply: tooShortSignature, - remoteErr: ContainSubstring("signature verification failed:"), + }, + { + name: "cert doesn't have the key extension", + apply: noKeyExtension, + checkErr: func(t *testing.T, err error) { + require.EqualError(t, err, "expected certificate to contain the key extension") }, - { - name: "signature is invalid", - apply: invalidSignature, - remoteErr: Equal("signature invalid"), + }, + { + name: "key extension not parseable", + apply: unparseableKeyExtension, + checkErr: func(t *testing.T, err error) { require.Contains(t, err.Error(), "asn1") }, + }, + { + name: "key protobuf not parseable", + apply: unparseableKey, + checkErr: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "unmarshalling public key failed: proto:") }, - } + }, + { + name: "signature is malformed", + apply: tooShortSignature, + checkErr: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "signature verification failed:") + }, + }, + { + name: "signature is invalid", + apply: invalidSignature, + checkErr: func(t *testing.T, err error) { + require.Contains(t, err.Error(), "signature invalid") + }, + }, + } - for i := range transforms { - t := transforms[i] - - It(fmt.Sprintf("fails if the client presents an invalid cert: %s", t.name), func() { - serverTransport, err := New(serverKey) - Expect(err).ToNot(HaveOccurred()) - clientTransport, err := New(clientKey) - Expect(err).ToNot(HaveOccurred()) - t.apply(clientTransport.identity) - - clientInsecureConn, serverInsecureConn := connect() - - done := make(chan struct{}) - go func() { - defer GinkgoRecover() - _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, "") - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(t.remoteErr) - close(done) - }() - - conn, err := clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) - Expect(err).ToNot(HaveOccurred()) - _, err = gbytes.TimeoutReader(conn, time.Second).Read([]byte{0}) - Expect(err).To(Or( - // if the certificate's public key doesn't match the private key used for signing - MatchError("remote error: tls: error decrypting message"), - // all other errors - MatchError("remote error: tls: bad certificate"), - )) - Eventually(done).Should(BeClosed()) - }) - - It(fmt.Sprintf("fails if the server presents an invalid cert: %s", t.name), func() { - serverTransport, err := New(serverKey) - Expect(err).ToNot(HaveOccurred()) - t.apply(serverTransport.identity) - clientTransport, err := New(clientKey) - Expect(err).ToNot(HaveOccurred()) - - clientInsecureConn, serverInsecureConn := connect() - - done := make(chan struct{}) - go func() { - defer GinkgoRecover() - _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, "") - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("remote error: tls:")) - close(done) - }() - - _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(t.remoteErr) - Eventually(done).Should(BeClosed()) - }) - } - }) -}) + for i := range transforms { + tr := transforms[i] + + t.Run(fmt.Sprintf("client offending: %s", tr.name), func(t *testing.T) { + serverTransport, err := New(serverKey) + require.NoError(t, err) + clientTransport, err := New(clientKey) + require.NoError(t, err) + tr.apply(clientTransport.identity) + + clientInsecureConn, serverInsecureConn := connect(t) + + serverErrChan := make(chan error) + go func() { + _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, "") + serverErrChan <- err + }() + + conn, err := clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) + require.NoError(t, err) + clientErrChan := make(chan error) + go func() { + _, err := conn.Read([]byte{0}) + clientErrChan <- err + }() + var clientErr error + select { + case clientErr = <-clientErrChan: + case <-time.After(250 * time.Millisecond): + t.Fatal("expected the server handshake to return") + } + require.Error(t, clientErr) + if clientErr.Error() != "remote error: tls: error decrypting message" && + clientErr.Error() != "remote error: tls: bad certificate" { + t.Fatalf("unexpected error: %s", err.Error()) + } + + var serverErr error + select { + case serverErr = <-serverErrChan: + case <-time.After(250 * time.Millisecond): + t.Fatal("expected the server handshake to return") + } + require.Error(t, serverErr) + tr.checkErr(t, serverErr) + }) + + t.Run(fmt.Sprintf("server offending: %s", tr.name), func(t *testing.T) { + serverTransport, err := New(serverKey) + require.NoError(t, err) + tr.apply(serverTransport.identity) + clientTransport, err := New(clientKey) + require.NoError(t, err) + + clientInsecureConn, serverInsecureConn := connect(t) + + errChan := make(chan error) + go func() { + _, err := serverTransport.SecureInbound(context.Background(), serverInsecureConn, "") + errChan <- err + }() + + _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) + require.Error(t, err) + tr.checkErr(t, err) + + var serverErr error + select { + case serverErr = <-errChan: + case <-time.After(250 * time.Millisecond): + t.Fatal("expected the server handshake to return") + } + require.Error(t, serverErr) + require.Contains(t, serverErr.Error(), "remote error: tls:") + }) + } +} From 5ffe478c081790bff512d6ea072e339140d4b7dd Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 4 Jan 2022 13:01:16 +0400 Subject: [PATCH 48/51] remove the Ginkgo test suite --- p2p/security/tls/libp2p_tls_suite_test.go | 18 ------------------ 1 file changed, 18 deletions(-) delete mode 100644 p2p/security/tls/libp2p_tls_suite_test.go diff --git a/p2p/security/tls/libp2p_tls_suite_test.go b/p2p/security/tls/libp2p_tls_suite_test.go deleted file mode 100644 index e0e6785862..0000000000 --- a/p2p/security/tls/libp2p_tls_suite_test.go +++ /dev/null @@ -1,18 +0,0 @@ -package libp2ptls - -import ( - mrand "math/rand" - "testing" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" -) - -func TestLibp2pTLS(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "libp2p TLS Suite") -} - -var _ = BeforeSuite(func() { - mrand.Seed(GinkgoRandomSeed()) -}) From 7ee67dd8d4873ee34c0d7da281023ab9801d1e0f Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 10 Apr 2022 14:30:15 +0100 Subject: [PATCH 49/51] use tls.Conn.HandshakeContext instead of tls.Conn.Handshake (#106) * use tls.Conn.HandshakeContext instead of tls.Conn.Handshake * make sure that crypto/tls picks up the handshake ctx cancelation in tests --- p2p/security/tls/transport.go | 51 ++---------------------------- p2p/security/tls/transport_test.go | 15 ++++++++- 2 files changed, 17 insertions(+), 49 deletions(-) diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index 6be3c79a79..85e75a1587 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -5,7 +5,6 @@ import ( "crypto/tls" "errors" "net" - "sync" ci "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/peer" @@ -71,44 +70,8 @@ func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p pee return cs, err } -func (t *Transport) handshake( - ctx context.Context, - tlsConn *tls.Conn, - keyCh <-chan ci.PubKey, -) (sec.SecureConn, error) { - // There's no way to pass a context to tls.Conn.Handshake(). - // See https://github.com/golang/go/issues/18482. - // Close the connection instead. - select { - case <-ctx.Done(): - tlsConn.Close() - default: - } - - done := make(chan struct{}) - var wg sync.WaitGroup - - // Ensure that we do not return before - // either being done or having a context - // cancellation. - defer wg.Wait() - defer close(done) - - wg.Add(1) - go func() { - defer wg.Done() - select { - case <-done: - case <-ctx.Done(): - tlsConn.Close() - } - }() - - if err := tlsConn.Handshake(); err != nil { - // if the context was canceled, return the context error - if ctxErr := ctx.Err(); ctxErr != nil { - return nil, ctxErr - } +func (t *Transport) handshake(ctx context.Context, tlsConn *tls.Conn, keyCh <-chan ci.PubKey) (sec.SecureConn, error) { + if err := tlsConn.HandshakeContext(ctx); err != nil { return nil, err } @@ -122,15 +85,7 @@ func (t *Transport) handshake( return nil, errors.New("go-libp2p-tls BUG: expected remote pub key to be set") } - conn, err := t.setupConn(tlsConn, remotePubKey) - if err != nil { - // if the context was canceled, return the context error - if ctxErr := ctx.Err(); ctxErr != nil { - return nil, ctxErr - } - return nil, err - } - return conn, nil + return t.setupConn(tlsConn, remotePubKey) } func (t *Transport) setupConn(tlsConn *tls.Conn, remotePubKey ci.PubKey) (sec.SecureConn, error) { diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index 8106f815a7..a290ed6fdc 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -121,6 +121,19 @@ func TestHandshakeSucceeds(t *testing.T) { }) } +// crypto/tls' cancellation logic works by spinning up a separate Go routine that watches the ctx. +// If the ctx is canceled, it kills the handshake. +// We need to make sure that the handshake doesn't complete before that Go routine picks up the cancellation. +type delayedConn struct { + net.Conn + delay time.Duration +} + +func (c *delayedConn) Read(b []byte) (int, error) { + time.Sleep(c.delay) + return c.Conn.Read(b) +} + func TestHandshakeConnectionCancelations(t *testing.T) { _, clientKey := createPeer(t) serverID, serverKey := createPeer(t) @@ -152,7 +165,7 @@ func TestHandshakeConnectionCancelations(t *testing.T) { go func() { ctx, cancel := context.WithCancel(context.Background()) cancel() - _, err := serverTransport.SecureInbound(ctx, serverInsecureConn, "") + _, err := serverTransport.SecureInbound(ctx, &delayedConn{Conn: serverInsecureConn, delay: 5 * time.Millisecond}, "") errChan <- err }() _, err = clientTransport.SecureOutbound(context.Background(), clientInsecureConn, serverID) From 6d7a3968cdf65f589e0638a9d8542cc3dbfdbaee Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Tue, 19 Apr 2022 11:42:05 +0200 Subject: [PATCH 50/51] feat: catch panics in TLS negotiation Part of https://github.com/libp2p/go-libp2p/issues/1389 --- p2p/security/tls/crypto.go | 12 +++++++++++- p2p/security/tls/transport.go | 13 ++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/p2p/security/tls/crypto.go b/p2p/security/tls/crypto.go index f3502aa740..24a06939ee 100644 --- a/p2p/security/tls/crypto.go +++ b/p2p/security/tls/crypto.go @@ -11,6 +11,8 @@ import ( "errors" "fmt" "math/big" + "os" + "runtime/debug" "time" "golang.org/x/sys/cpu" @@ -72,7 +74,15 @@ func (i *Identity) ConfigForPeer(remote peer.ID) (*tls.Config, <-chan ic.PubKey) 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 { + conf.VerifyPeerCertificate = func(rawCerts [][]byte, _ [][]*x509.Certificate) (err error) { + defer func() { + if rerr := recover(); rerr != nil { + fmt.Fprintf(os.Stderr, "panic when processing peer certificate in TLS handshake: %s\n%s\n", rerr, debug.Stack()) + err = fmt.Errorf("panic when processing peer certificate in TLS handshake: %s", rerr) + + } + }() + defer close(keyCh) chain := make([]*x509.Certificate, len(rawCerts)) diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index 85e75a1587..fcdbd3c676 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -4,7 +4,10 @@ import ( "context" "crypto/tls" "errors" + "fmt" "net" + "os" + "runtime/debug" ci "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/peer" @@ -70,7 +73,15 @@ func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p pee return cs, err } -func (t *Transport) handshake(ctx context.Context, tlsConn *tls.Conn, keyCh <-chan ci.PubKey) (sec.SecureConn, error) { +func (t *Transport) handshake(ctx context.Context, tlsConn *tls.Conn, keyCh <-chan ci.PubKey) (_sconn sec.SecureConn, err error) { + defer func() { + if rerr := recover(); rerr != nil { + fmt.Fprintf(os.Stderr, "panic in TLS handshake: %s\n%s\n", rerr, debug.Stack()) + err = fmt.Errorf("panic in TLS handshake: %s", rerr) + + } + }() + if err := tlsConn.HandshakeContext(ctx); err != nil { return nil, err } From 27cfd3f4edee0f702fac02311cc29a7506859159 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 27 Apr 2022 12:00:11 +0200 Subject: [PATCH 51/51] switch from github.com/libp2p/go-libp2p-tls to p2p/security/tls --- defaults.go | 2 +- go.mod | 4 ++-- p2p/security/tls/cmd/tlsdiag.go | 2 +- p2p/security/tls/cmd/tlsdiag/client.go | 3 ++- p2p/security/tls/cmd/tlsdiag/server.go | 3 ++- p2p/security/tls/transport.go | 2 +- p2p/transport/quic/listener.go | 4 ++-- p2p/transport/quic/transport.go | 4 ++-- 8 files changed, 13 insertions(+), 11 deletions(-) diff --git a/defaults.go b/defaults.go index 759955a933..7cca89a6cf 100644 --- a/defaults.go +++ b/defaults.go @@ -8,6 +8,7 @@ import ( "github.com/libp2p/go-libp2p/p2p/muxer/yamux" "github.com/libp2p/go-libp2p/p2p/net/connmgr" "github.com/libp2p/go-libp2p/p2p/security/noise" + tls "github.com/libp2p/go-libp2p/p2p/security/tls" quic "github.com/libp2p/go-libp2p/p2p/transport/quic" "github.com/libp2p/go-libp2p/p2p/transport/tcp" ws "github.com/libp2p/go-libp2p/p2p/transport/websocket" @@ -16,7 +17,6 @@ import ( "github.com/libp2p/go-libp2p-peerstore/pstoremem" rcmgr "github.com/libp2p/go-libp2p-resource-manager" - tls "github.com/libp2p/go-libp2p-tls" "github.com/multiformats/go-multiaddr" ) diff --git a/go.mod b/go.mod index 77aa407581..edef69c5d0 100644 --- a/go.mod +++ b/go.mod @@ -26,7 +26,6 @@ require ( github.com/libp2p/go-libp2p-pnet v0.2.0 github.com/libp2p/go-libp2p-resource-manager v0.2.1 github.com/libp2p/go-libp2p-testing v0.9.2 - github.com/libp2p/go-libp2p-tls v0.4.1 github.com/libp2p/go-mplex v0.7.0 github.com/libp2p/go-msgio v0.2.0 github.com/libp2p/go-netroute v0.2.0 @@ -52,6 +51,7 @@ require ( github.com/whyrusleeping/multiaddr-filter v0.0.0-20160516205228-e903e4adabd7 golang.org/x/crypto v0.0.0-20220411220226-7b82a4e95df4 golang.org/x/sync v0.0.0-20210220032951-036812b2e83c + golang.org/x/sys v0.0.0-20220412211240-33da011f77ad ) require ( @@ -82,6 +82,7 @@ require ( github.com/libp2p/go-libp2p-blankhost v0.3.0 // indirect github.com/libp2p/go-libp2p-quic-transport v0.17.0 // indirect github.com/libp2p/go-libp2p-swarm v0.10.2 // indirect + github.com/libp2p/go-libp2p-tls v0.4.1 // indirect github.com/libp2p/go-libp2p-transport-upgrader v0.7.1 // indirect github.com/libp2p/go-libp2p-yamux v0.9.1 // indirect github.com/libp2p/go-nat v0.1.0 // indirect @@ -117,7 +118,6 @@ require ( go.uber.org/zap v1.21.0 // indirect golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 // indirect golang.org/x/net v0.0.0-20220418201149-a630d4f3e7a2 // indirect - golang.org/x/sys v0.0.0-20220412211240-33da011f77ad // indirect golang.org/x/tools v0.1.10 // indirect golang.org/x/xerrors v0.0.0-20220411194840-2f41105eb62f // indirect google.golang.org/grpc v1.45.0 // indirect diff --git a/p2p/security/tls/cmd/tlsdiag.go b/p2p/security/tls/cmd/tlsdiag.go index 4aa2f2f09c..d6f7bac674 100644 --- a/p2p/security/tls/cmd/tlsdiag.go +++ b/p2p/security/tls/cmd/tlsdiag.go @@ -4,7 +4,7 @@ import ( "fmt" "os" - "github.com/libp2p/go-libp2p-tls/cmd/tlsdiag" + "github.com/libp2p/go-libp2p/p2p/security/tls/cmd/tlsdiag" ) func main() { diff --git a/p2p/security/tls/cmd/tlsdiag/client.go b/p2p/security/tls/cmd/tlsdiag/client.go index 9bf2e69c51..b902f509f7 100644 --- a/p2p/security/tls/cmd/tlsdiag/client.go +++ b/p2p/security/tls/cmd/tlsdiag/client.go @@ -8,8 +8,9 @@ import ( "net" "time" + libp2ptls "github.com/libp2p/go-libp2p/p2p/security/tls" + "github.com/libp2p/go-libp2p-core/peer" - libp2ptls "github.com/libp2p/go-libp2p-tls" ) func StartClient() error { diff --git a/p2p/security/tls/cmd/tlsdiag/server.go b/p2p/security/tls/cmd/tlsdiag/server.go index 6934f10343..5f85bc7299 100644 --- a/p2p/security/tls/cmd/tlsdiag/server.go +++ b/p2p/security/tls/cmd/tlsdiag/server.go @@ -7,8 +7,9 @@ import ( "net" "time" + libp2ptls "github.com/libp2p/go-libp2p/p2p/security/tls" + "github.com/libp2p/go-libp2p-core/peer" - libp2ptls "github.com/libp2p/go-libp2p-tls" ) func StartServer() error { diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index fcdbd3c676..f8911fc960 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -93,7 +93,7 @@ func (t *Transport) handshake(ctx context.Context, tlsConn *tls.Conn, keyCh <-ch default: } if remotePubKey == nil { - return nil, errors.New("go-libp2p-tls BUG: expected remote pub key to be set") + return nil, errors.New("go-libp2p tls BUG: expected remote pub key to be set") } return t.setupConn(tlsConn, remotePubKey) diff --git a/p2p/transport/quic/listener.go b/p2p/transport/quic/listener.go index d6a20d22a4..293fa0314a 100644 --- a/p2p/transport/quic/listener.go +++ b/p2p/transport/quic/listener.go @@ -5,13 +5,13 @@ import ( "crypto/tls" "net" + p2ptls "github.com/libp2p/go-libp2p/p2p/security/tls" + ic "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/network" "github.com/libp2p/go-libp2p-core/peer" tpt "github.com/libp2p/go-libp2p-core/transport" - p2ptls "github.com/libp2p/go-libp2p-tls" - "github.com/lucas-clemente/quic-go" ma "github.com/multiformats/go-multiaddr" ) diff --git a/p2p/transport/quic/transport.go b/p2p/transport/quic/transport.go index fbc8abc75b..a36b3e58cb 100644 --- a/p2p/transport/quic/transport.go +++ b/p2p/transport/quic/transport.go @@ -12,6 +12,8 @@ import ( "golang.org/x/crypto/hkdf" + p2ptls "github.com/libp2p/go-libp2p/p2p/security/tls" + "github.com/libp2p/go-libp2p-core/connmgr" ic "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/network" @@ -19,8 +21,6 @@ import ( "github.com/libp2p/go-libp2p-core/pnet" tpt "github.com/libp2p/go-libp2p-core/transport" - p2ptls "github.com/libp2p/go-libp2p-tls" - ma "github.com/multiformats/go-multiaddr" mafmt "github.com/multiformats/go-multiaddr-fmt" manet "github.com/multiformats/go-multiaddr/net"