From 7db934488ea271dea050d8c03c3d7b5ae559118f Mon Sep 17 00:00:00 2001 From: ehmry Date: Sun, 2 Oct 2022 06:35:43 -0500 Subject: [PATCH] Reimplement AddPeer and RemovePeer for admin socket (#951) * Reimplement AddPeer and RemovePeer for admin socket Fix #950 * Disconnect the peer on `removePeer` Co-authored-by: Neil Alexander --- src/admin/addpeer.go | 12 +++++ src/admin/admin.go | 28 +++++++++++ src/admin/removepeer.go | 12 +++++ src/core/api.go | 102 +++++++++++++++------------------------- src/core/core.go | 4 +- src/core/link.go | 10 ++-- src/core/options.go | 2 +- 7 files changed, 97 insertions(+), 73 deletions(-) create mode 100644 src/admin/addpeer.go create mode 100644 src/admin/removepeer.go diff --git a/src/admin/addpeer.go b/src/admin/addpeer.go new file mode 100644 index 00000000..f5e37ee9 --- /dev/null +++ b/src/admin/addpeer.go @@ -0,0 +1,12 @@ +package admin + +type AddPeerRequest struct { + Uri string `json:"uri"` + Sintf string `json:"interface,omitempty"` +} + +type AddPeerResponse struct{} + +func (a *AdminSocket) addPeerHandler(req *AddPeerRequest, res *AddPeerResponse) error { + return a.core.AddPeer(req.Uri, req.Sintf) +} diff --git a/src/admin/admin.go b/src/admin/admin.go index ca1d95ef..7f5f467e 100644 --- a/src/admin/admin.go +++ b/src/admin/admin.go @@ -174,6 +174,34 @@ func (a *AdminSocket) SetupAdminHandlers() { return res, nil }, ) + _ = a.AddHandler( + "addPeer", "Add a peer to the peer list", []string{"uri", "interface"}, + func(in json.RawMessage) (interface{}, error) { + req := &AddPeerRequest{} + res := &AddPeerResponse{} + if err := json.Unmarshal(in, &req); err != nil { + return nil, err + } + if err := a.addPeerHandler(req, res); err != nil { + return nil, err + } + return res, nil + }, + ) + _ = a.AddHandler( + "removePeer", "Remove a peer from the peer list", []string{"uri", "interface"}, + func(in json.RawMessage) (interface{}, error) { + req := &RemovePeerRequest{} + res := &RemovePeerResponse{} + if err := json.Unmarshal(in, &req); err != nil { + return nil, err + } + if err := a.removePeerHandler(req, res); err != nil { + return nil, err + } + return res, nil + }, + ) //_ = a.AddHandler("getNodeInfo", []string{"key"}, t.proto.nodeinfo.nodeInfoAdminHandler) //_ = a.AddHandler("debug_remoteGetSelf", []string{"key"}, t.proto.getSelfHandler) //_ = a.AddHandler("debug_remoteGetPeers", []string{"key"}, t.proto.getPeersHandler) diff --git a/src/admin/removepeer.go b/src/admin/removepeer.go new file mode 100644 index 00000000..145b8524 --- /dev/null +++ b/src/admin/removepeer.go @@ -0,0 +1,12 @@ +package admin + +type RemovePeerRequest struct { + Uri string `json:"uri"` + Sintf string `json:"interface,omitempty"` +} + +type RemovePeerResponse struct{} + +func (a *AdminSocket) removePeerHandler(req *RemovePeerRequest, res *RemovePeerResponse) error { + return a.core.RemovePeer(req.Uri, req.Sintf) +} diff --git a/src/core/api.go b/src/core/api.go index a67ecf3f..75d9d7b3 100644 --- a/src/core/api.go +++ b/src/core/api.go @@ -181,78 +181,49 @@ func (c *Core) SetLogger(log util.Logger) { } // AddPeer adds a peer. This should be specified in the peer URI format, e.g.: -// tcp://a.b.c.d:e -// socks://a.b.c.d:e/f.g.h.i:j +// +// tcp://a.b.c.d:e +// socks://a.b.c.d:e/f.g.h.i:j +// // This adds the peer to the peer list, so that they will be called again if the // connection drops. -/* -func (c *Core) AddPeer(addr string, sintf string) error { - if err := c.CallPeer(addr, sintf); err != nil { - // TODO: We maybe want this to write the peer to the persistent - // configuration even if a connection attempt fails, but first we'll need to - // move the code to check the peer URI so that we don't deliberately save a - // peer with a known bad URI. Loading peers from config should really do the - // same thing too but I don't think that happens today +func (c *Core) AddPeer(uri string, sourceInterface string) error { + u, err := url.Parse(uri) + if err != nil { return err } - c.config.Mutex.Lock() - defer c.config.Mutex.Unlock() - if sintf == "" { - for _, peer := range c.config.Current.Peers { - if peer == addr { - return errors.New("peer already added") - } - } - c.config.Current.Peers = append(c.config.Current.Peers, addr) - } else { - if _, ok := c.config.Current.InterfacePeers[sintf]; ok { - for _, peer := range c.config.Current.InterfacePeers[sintf] { - if peer == addr { - return errors.New("peer already added") - } - } - } - if _, ok := c.config.Current.InterfacePeers[sintf]; !ok { - c.config.Current.InterfacePeers[sintf] = []string{addr} - } else { - c.config.Current.InterfacePeers[sintf] = append(c.config.Current.InterfacePeers[sintf], addr) - } + info, err := c.links.call(u, sourceInterface) + if err != nil { + return err } - return nil -} -*/ - -/* -func (c *Core) RemovePeer(addr string, sintf string) error { - if sintf == "" { - for i, peer := range c.config.Current.Peers { - if peer == addr { - c.config.Current.Peers = append(c.config.Current.Peers[:i], c.config.Current.Peers[i+1:]...) - break - } - } - } else if _, ok := c.config.Current.InterfacePeers[sintf]; ok { - for i, peer := range c.config.Current.InterfacePeers[sintf] { - if peer == addr { - c.config.Current.InterfacePeers[sintf] = append(c.config.Current.InterfacePeers[sintf][:i], c.config.Current.InterfacePeers[sintf][i+1:]...) - break - } - } - } - - panic("TODO") // Get the net.Conn to this peer (if any) and close it - c.peers.Act(nil, func() { - ports := c.peers.ports - for _, peer := range ports { - if addr == peer.intf.name() { - c.peers._removePeer(peer) - } - } + phony.Block(c, func() { + c.config._peers[Peer{uri, sourceInterface}] = &info }) - return nil } -*/ + +// RemovePeer removes a peer. The peer should be specified in URI format, see AddPeer. +// The peer is not disconnected immediately. +func (c *Core) RemovePeer(uri string, sourceInterface string) error { + var err error + phony.Block(c, func() { + peer := Peer{uri, sourceInterface} + linkInfo, ok := c.config._peers[peer] + if !ok { + err = fmt.Errorf("peer not configured") + return + } + if ok && linkInfo != nil { + c.links.Act(nil, func() { + if link := c.links._links[*linkInfo]; link != nil { + _ = link.close() + } + }) + } + delete(c.config._peers, peer) + }) + return err +} // CallPeer calls a peer once. This should be specified in the peer URI format, // e.g.: @@ -263,7 +234,8 @@ func (c *Core) RemovePeer(addr string, sintf string) error { // This does not add the peer to the peer list, so if the connection drops, the // peer will not be called again automatically. func (c *Core) CallPeer(u *url.URL, sintf string) error { - return c.links.call(u, sintf) + _, err := c.links.call(u, sintf) + return err } func (c *Core) PublicKey() ed25519.PublicKey { diff --git a/src/core/core.go b/src/core/core.go index 5cfcc7a2..b8315416 100644 --- a/src/core/core.go +++ b/src/core/core.go @@ -36,7 +36,7 @@ type Core struct { log util.Logger addPeerTimer *time.Timer config struct { - _peers map[Peer]struct{} // configurable after startup + _peers map[Peer]*linkInfo // configurable after startup _listeners map[ListenAddress]struct{} // configurable after startup nodeinfo NodeInfo // immutable after startup nodeinfoPrivacy NodeInfoPrivacy // immutable after startup @@ -66,7 +66,7 @@ func New(secret ed25519.PrivateKey, logger util.Logger, opts ...SetupOption) (*C if c.PacketConn, err = iwe.NewPacketConn(c.secret); err != nil { return nil, fmt.Errorf("error creating encryption: %w", err) } - c.config._peers = map[Peer]struct{}{} + c.config._peers = map[Peer]*linkInfo{} c.config._listeners = map[ListenAddress]struct{}{} c.config._allowedPublicKeys = map[[32]byte]struct{}{} for _, opt := range opts { diff --git a/src/core/link.go b/src/core/link.go index 26e44669..963963a4 100644 --- a/src/core/link.go +++ b/src/core/link.go @@ -108,10 +108,10 @@ func (l *links) isConnectedTo(info linkInfo) bool { return isConnected } -func (l *links) call(u *url.URL, sintf string) error { +func (l *links) call(u *url.URL, sintf string) (linkInfo, error) { info := linkInfoFor(u.Scheme, sintf, u.Host) if l.isConnectedTo(info) { - return nil + return info, nil } options := linkOptions{ pinnedEd25519Keys: map[keyArray]struct{}{}, @@ -119,7 +119,7 @@ func (l *links) call(u *url.URL, sintf string) error { for _, pubkey := range u.Query()["key"] { sigPub, err := hex.DecodeString(pubkey) if err != nil { - return fmt.Errorf("pinned key contains invalid hex characters") + return info, fmt.Errorf("pinned key contains invalid hex characters") } var sigPubKey keyArray copy(sigPubKey[:], sigPub) @@ -172,9 +172,9 @@ func (l *links) call(u *url.URL, sintf string) error { }() default: - return errors.New("unknown call scheme: " + u.Scheme) + return info, errors.New("unknown call scheme: " + u.Scheme) } - return nil + return info, nil } func (l *links) listen(u *url.URL, sintf string) (*Listener, error) { diff --git a/src/core/options.go b/src/core/options.go index 8009aa3b..66aa16ce 100644 --- a/src/core/options.go +++ b/src/core/options.go @@ -7,7 +7,7 @@ import ( func (c *Core) _applyOption(opt SetupOption) { switch v := opt.(type) { case Peer: - c.config._peers[v] = struct{}{} + c.config._peers[v] = nil case ListenAddress: c.config._listeners[v] = struct{}{} case NodeInfo: