From 092373511e1a1e5a8701e6237708312f6aa5c2da Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 26 Feb 2023 11:32:52 +1300 Subject: [PATCH 1/2] core: remove LocalPrivateKey method from network.Conn interface --- core/network/conn.go | 3 -- core/sec/insecure/insecure.go | 56 +++++++++------------------- core/sec/insecure/insecure_test.go | 7 ++-- p2p/net/connmgr/connmgr_test.go | 1 - p2p/net/mock/mock_conn.go | 5 --- p2p/net/swarm/swarm_conn.go | 5 --- p2p/security/noise/session.go | 4 -- p2p/security/noise/transport_test.go | 15 ++++---- p2p/security/tls/conn.go | 8 +--- p2p/security/tls/transport.go | 1 - p2p/security/tls/transport_test.go | 4 -- p2p/transport/quic/conn.go | 4 -- p2p/transport/quic/conn_test.go | 2 - p2p/transport/quic/listener.go | 1 - p2p/transport/quic/transport.go | 1 - 15 files changed, 31 insertions(+), 86 deletions(-) diff --git a/core/network/conn.go b/core/network/conn.go index 56031ab0c1..97168915f8 100644 --- a/core/network/conn.go +++ b/core/network/conn.go @@ -53,9 +53,6 @@ type ConnSecurity interface { // LocalPeer returns our peer ID LocalPeer() peer.ID - // LocalPrivateKey returns our private key - LocalPrivateKey() ic.PrivKey - // RemotePeer returns the peer ID of the remote peer. RemotePeer() peer.ID diff --git a/core/sec/insecure/insecure.go b/core/sec/insecure/insecure.go index 9b3664e202..9ed20f093e 100644 --- a/core/sec/insecure/insecure.go +++ b/core/sec/insecure/insecure.go @@ -40,9 +40,7 @@ type Transport struct { var _ sec.SecureTransport = &Transport{} -// NewWithIdentity constructs a new insecure transport. The provided private key -// is stored and returned from LocalPrivateKey to satisfy the -// SecureTransport interface, and the public key is sent to +// NewWithIdentity constructs a new insecure transport. The public key is sent to // remote peers. No security is provided. func NewWithIdentity(protocolID protocol.ID, id peer.ID, key ci.PrivKey) *Transport { return &Transport{ @@ -57,12 +55,6 @@ func (t *Transport) LocalPeer() peer.ID { return t.id } -// LocalPrivateKey returns the local private key. -// This key is used only for identity generation and provides no security. -func (t *Transport) LocalPrivateKey() ci.PrivKey { - return t.key -} - // SecureInbound *pretends to secure* an inbound connection to the given peer. // It sends the local peer's ID and public key, and receives the same from the remote peer. // No validation is performed as to the authenticity or ownership of the provided public key, @@ -70,19 +62,18 @@ func (t *Transport) LocalPrivateKey() ci.PrivKey { // // SecureInbound may fail if the remote peer sends an ID and public key that are inconsistent // with each other, or if a network error occurs during the ID exchange. -func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, error) { +func (t *Transport) SecureInbound(_ context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, error) { conn := &Conn{ - Conn: insecure, - local: t.id, - localPrivKey: t.key, + Conn: insecure, + local: t.id, + localPubKey: t.key.GetPublic(), } - err := conn.runHandshakeSync() - if err != nil { + if err := conn.runHandshakeSync(); err != nil { return nil, err } - if t.key != nil && p != "" && p != conn.remote { + if p != "" && p != conn.remote { return nil, fmt.Errorf("remote peer sent unexpected peer ID. expected=%s received=%s", p, conn.remote) } @@ -97,19 +88,18 @@ func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn, p peer // SecureOutbound may fail if the remote peer sends an ID and public key that are inconsistent // with each other, or if the ID sent by the remote peer does not match the one dialed. It may // also fail if a network error occurs during the ID exchange. -func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, error) { +func (t *Transport) SecureOutbound(_ context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, error) { conn := &Conn{ - Conn: insecure, - local: t.id, - localPrivKey: t.key, + Conn: insecure, + local: t.id, + localPubKey: t.key.GetPublic(), } - err := conn.runHandshakeSync() - if err != nil { + if err := conn.runHandshakeSync(); err != nil { return nil, err } - if t.key != nil && p != conn.remote { + if p != conn.remote { return nil, fmt.Errorf("remote peer sent unexpected peer ID. expected=%s received=%s", p, conn.remote) } @@ -117,19 +107,14 @@ func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p pee return conn, nil } -func (t *Transport) ID() protocol.ID { - return t.protocolID -} +func (t *Transport) ID() protocol.ID { return t.protocolID } // Conn is the connection type returned by the insecure transport. type Conn struct { net.Conn - local peer.ID - remote peer.ID - - localPrivKey ci.PrivKey - remotePubKey ci.PubKey + local, remote peer.ID + localPubKey, remotePubKey ci.PubKey } func makeExchangeMessage(pubkey ci.PubKey) (*pb.Exchange, error) { @@ -150,12 +135,12 @@ func makeExchangeMessage(pubkey ci.PubKey) (*pb.Exchange, error) { func (ic *Conn) runHandshakeSync() error { // If we were initialized without keys, behave as in plaintext/1.0.0 (do nothing) - if ic.localPrivKey == nil { + if ic.localPubKey == nil { return nil } // Generate an Exchange message - msg, err := makeExchangeMessage(ic.localPrivKey.GetPublic()) + msg, err := makeExchangeMessage(ic.localPubKey) if err != nil { return err } @@ -239,11 +224,6 @@ func (ic *Conn) RemotePublicKey() ci.PubKey { return ic.remotePubKey } -// LocalPrivateKey returns the private key for the local peer. -func (ic *Conn) LocalPrivateKey() ci.PrivKey { - return ic.localPrivKey -} - // ConnState returns the security connection's state information. func (ic *Conn) ConnState() network.ConnectionState { return network.ConnectionState{} diff --git a/core/sec/insecure/insecure_test.go b/core/sec/insecure/insecure_test.go index 8663f1d975..f6ceba4124 100644 --- a/core/sec/insecure/insecure_test.go +++ b/core/sec/insecure/insecure_test.go @@ -103,6 +103,7 @@ func connect(t *testing.T, clientTpt, serverTpt *Transport, clientExpectsID, ser // Check the peer IDs func testIDs(t *testing.T, clientTpt, serverTpt *Transport, clientConn, serverConn sec.SecureConn) { + t.Helper() require.Equal(t, clientConn.LocalPeer(), clientTpt.LocalPeer(), "Client Local Peer ID mismatch.") require.Equal(t, clientConn.RemotePeer(), serverTpt.LocalPeer(), "Client Remote Peer ID mismatch.") require.Equal(t, clientConn.LocalPeer(), serverConn.RemotePeer(), "Server Local Peer ID mismatch.") @@ -110,9 +111,9 @@ func testIDs(t *testing.T, clientTpt, serverTpt *Transport, clientConn, serverCo // Check the keys func testKeys(t *testing.T, clientTpt, serverTpt *Transport, clientConn, serverConn sec.SecureConn) { - sk := serverConn.LocalPrivateKey() - require.True(t, sk.Equals(serverTpt.LocalPrivateKey()), "private key mismatch") - require.True(t, sk.GetPublic().Equals(clientConn.RemotePublicKey()), "public key mismatch") + t.Helper() + require.True(t, clientConn.RemotePublicKey().Equals(serverTpt.key.GetPublic()), "client conn key mismatch") + require.True(t, serverConn.RemotePublicKey().Equals(clientTpt.key.GetPublic()), "server conn key mismatch") } // Check sending and receiving messages diff --git a/p2p/net/connmgr/connmgr_test.go b/p2p/net/connmgr/connmgr_test.go index 7468f671f8..69f82a395a 100644 --- a/p2p/net/connmgr/connmgr_test.go +++ b/p2p/net/connmgr/connmgr_test.go @@ -796,7 +796,6 @@ type mockConn struct { func (m mockConn) Close() error { panic("implement me") } func (m mockConn) LocalPeer() peer.ID { panic("implement me") } -func (m mockConn) LocalPrivateKey() crypto.PrivKey { panic("implement me") } func (m mockConn) RemotePeer() peer.ID { panic("implement me") } func (m mockConn) RemotePublicKey() crypto.PubKey { panic("implement me") } func (m mockConn) LocalMultiaddr() ma.Multiaddr { panic("implement me") } diff --git a/p2p/net/mock/mock_conn.go b/p2p/net/mock/mock_conn.go index b6fe8f2d1f..970fc2fc05 100644 --- a/p2p/net/mock/mock_conn.go +++ b/p2p/net/mock/mock_conn.go @@ -158,11 +158,6 @@ func (c *conn) LocalPeer() peer.ID { return c.local } -// LocalPrivateKey is the private key of the peer on our side. -func (c *conn) LocalPrivateKey() ic.PrivKey { - return c.localPrivKey -} - // RemoteMultiaddr is the Multiaddr on the remote side func (c *conn) RemoteMultiaddr() ma.Multiaddr { return c.remoteAddr diff --git a/p2p/net/swarm/swarm_conn.go b/p2p/net/swarm/swarm_conn.go index 406b100a57..8cdabc4194 100644 --- a/p2p/net/swarm/swarm_conn.go +++ b/p2p/net/swarm/swarm_conn.go @@ -168,11 +168,6 @@ func (c *Conn) RemotePeer() peer.ID { return c.conn.RemotePeer() } -// LocalPrivateKey is the public key of the peer on this side -func (c *Conn) LocalPrivateKey() ic.PrivKey { - return c.conn.LocalPrivateKey() -} - // RemotePublicKey is the public key of the peer on the remote side func (c *Conn) RemotePublicKey() ic.PubKey { return c.conn.RemotePublicKey() diff --git a/p2p/security/noise/session.go b/p2p/security/noise/session.go index 2f6c23c098..fa32ab8fa4 100644 --- a/p2p/security/noise/session.go +++ b/p2p/security/noise/session.go @@ -95,10 +95,6 @@ func (s *secureSession) LocalPeer() peer.ID { return s.localID } -func (s *secureSession) LocalPrivateKey() crypto.PrivKey { - return s.localKey -} - func (s *secureSession) LocalPublicKey() crypto.PubKey { return s.localKey.GetPublic() } diff --git a/p2p/security/noise/transport_test.go b/p2p/security/noise/transport_test.go index ada0cbfe01..42477f83ab 100644 --- a/p2p/security/noise/transport_test.go +++ b/p2p/security/noise/transport_test.go @@ -159,15 +159,16 @@ func TestKeys(t *testing.T) { defer initConn.Close() defer respConn.Close() - sk := respConn.LocalPrivateKey() - pk := sk.GetPublic() - - if !sk.Equals(respTransport.privateKey) { - t.Error("Private key Mismatch.") + pk1 := respConn.RemotePublicKey() + pk2 := initTransport.privateKey.GetPublic() + if !pk1.Equals(pk2) { + t.Errorf("Public key mismatch. expected %x got %x", pk1, pk2) } - if !pk.Equals(initConn.RemotePublicKey()) { - t.Errorf("Public key mismatch. expected %x got %x", pk, initConn.RemotePublicKey()) + pk3 := initConn.RemotePublicKey() + pk4 := respTransport.privateKey.GetPublic() + if !pk3.Equals(pk4) { + t.Errorf("Public key mismatch. expected %x got %x", pk3, pk4) } } diff --git a/p2p/security/tls/conn.go b/p2p/security/tls/conn.go index 3ebc7aefc5..143da3921c 100644 --- a/p2p/security/tls/conn.go +++ b/p2p/security/tls/conn.go @@ -12,9 +12,7 @@ import ( type conn struct { *tls.Conn - localPeer peer.ID - privKey ci.PrivKey - + localPeer peer.ID remotePeer peer.ID remotePubKey ci.PubKey connectionState network.ConnectionState @@ -26,10 +24,6 @@ func (c *conn) LocalPeer() peer.ID { return c.localPeer } -func (c *conn) LocalPrivateKey() ci.PrivKey { - return c.privKey -} - func (c *conn) RemotePeer() peer.ID { return c.remotePeer } diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go index b8c1dcfb14..7c28efe37e 100644 --- a/p2p/security/tls/transport.go +++ b/p2p/security/tls/transport.go @@ -168,7 +168,6 @@ func (t *Transport) setupConn(tlsConn *tls.Conn, remotePubKey ci.PubKey) (sec.Se return &conn{ Conn: tlsConn, localPeer: t.localPeer, - privKey: t.privKey, remotePeer: remotePeerID, remotePubKey: remotePubKey, connectionState: network.ConnectionState{ diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go index a30c742aa0..2d3c2d9706 100644 --- a/p2p/security/tls/transport_test.go +++ b/p2p/security/tls/transport_test.go @@ -111,8 +111,6 @@ func TestHandshakeSucceeds(t *testing.T) { 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") @@ -249,8 +247,6 @@ func TestHandshakeWithNextProtoSucceeds(t *testing.T) { 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") diff --git a/p2p/transport/quic/conn.go b/p2p/transport/quic/conn.go index 1154cf72d4..a2da81eb34 100644 --- a/p2p/transport/quic/conn.go +++ b/p2p/transport/quic/conn.go @@ -18,7 +18,6 @@ type conn struct { scope network.ConnManagementScope localPeer peer.ID - privKey ic.PrivKey localMultiaddr ma.Multiaddr remotePeerID peer.ID @@ -66,9 +65,6 @@ func (c *conn) AcceptStream() (network.MuxedStream, error) { // LocalPeer returns our peer ID func (c *conn) LocalPeer() peer.ID { return c.localPeer } -// LocalPrivateKey returns our private key -func (c *conn) LocalPrivateKey() ic.PrivKey { return c.privKey } - // RemotePeer returns the peer ID of the remote peer. func (c *conn) RemotePeer() peer.ID { return c.remotePeerID } diff --git a/p2p/transport/quic/conn_test.go b/p2p/transport/quic/conn_test.go index 6929458706..189a38dfc5 100644 --- a/p2p/transport/quic/conn_test.go +++ b/p2p/transport/quic/conn_test.go @@ -103,12 +103,10 @@ func testHandshake(t *testing.T, tc *connTestCase) { defer serverConn.Close() require.Equal(t, conn.LocalPeer(), clientID) - require.True(t, conn.LocalPrivateKey().Equals(clientKey), "local private key doesn't match") require.Equal(t, conn.RemotePeer(), serverID) require.True(t, conn.RemotePublicKey().Equals(serverKey.GetPublic()), "remote public key doesn't match") require.Equal(t, serverConn.LocalPeer(), serverID) - require.True(t, serverConn.LocalPrivateKey().Equals(serverKey), "local private key doesn't match") require.Equal(t, serverConn.RemotePeer(), clientID) require.True(t, serverConn.RemotePublicKey().Equals(clientKey.GetPublic()), "remote public key doesn't match") } diff --git a/p2p/transport/quic/listener.go b/p2p/transport/quic/listener.go index 017189cb69..73bb5026bd 100644 --- a/p2p/transport/quic/listener.go +++ b/p2p/transport/quic/listener.go @@ -133,7 +133,6 @@ func (l *listener) setupConnWithScope(qconn quic.Connection, connScope network.C scope: connScope, localPeer: l.localPeer, localMultiaddr: localMultiaddr, - privKey: l.privKey, remoteMultiaddr: remoteMultiaddr, remotePeerID: remotePeerID, remotePubKey: remotePubKey, diff --git a/p2p/transport/quic/transport.go b/p2p/transport/quic/transport.go index 5ccebf9367..2d68b660c5 100644 --- a/p2p/transport/quic/transport.go +++ b/p2p/transport/quic/transport.go @@ -153,7 +153,6 @@ func (t *transport) dialWithScope(ctx context.Context, raddr ma.Multiaddr, p pee quicConn: pconn, transport: t, scope: scope, - privKey: t.privKey, localPeer: t.localPeer, localMultiaddr: localMultiaddr, remotePubKey: remotePubKey, From 3b51517c383f5d9a4f53e8d957efc0e89777567b Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 3 Mar 2023 20:54:28 +1300 Subject: [PATCH 2/2] add Changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d53dcd4fb..012815d332 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ # [v0.27.0](https://github.com/libp2p/go-libp2p/releases/tag/v0.27.0) (unreleased) +### Breaking Changes + +* The `LocalPrivateKey` method was removed from the `network.Conn` interface. + # [v0.26.1](https://github.com/libp2p/go-libp2p/releases/tag/v0.26.1) This patch release fixes two bugs: