diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b87a552..c7daf9be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ NOTE: Add new changes BELOW THIS COMMENT. ### Fixed +- Searching for persistent clients by CIDR in the `POST /clients/search HTTP API`. + - Rules with the `client` modifier not working ([#7708]). - The search form not working in the query log ([#7704]). diff --git a/internal/client/index.go b/internal/client/index.go index 876d2566..5e750a94 100644 --- a/internal/client/index.go +++ b/internal/client/index.go @@ -283,15 +283,13 @@ func (ci *index) findByIP(ip netip.Addr) (c *Persistent, found bool) { // identifier. func (ci *index) findByCIDR(subnet netip.Prefix) (c *Persistent, ok bool) { var uid UID - ci.subnetToUID.Range(func(pref netip.Prefix, id UID) (cont bool) { + for pref, id := range ci.subnetToUID.Range { if subnet == pref { uid, ok = id, true - return false + break } - - return true - }) + } if ok { return ci.uidToClient[uid], true diff --git a/internal/client/persistent.go b/internal/client/persistent.go index a3f970cc..35e51fef 100644 --- a/internal/client/persistent.go +++ b/internal/client/persistent.go @@ -256,7 +256,7 @@ func ValidateClientID(id string) (err error) { return nil } -// IDs returns a list of ClientIDs containing at least one element. +// IDs returns a list of client identifiers containing at least one element. func (c *Persistent) IDs() (ids []string) { ids = make([]string, 0, c.IDsLen()) diff --git a/internal/client/storage.go b/internal/client/storage.go index c9478482..e347b295 100644 --- a/internal/client/storage.go +++ b/internal/client/storage.go @@ -440,22 +440,8 @@ func (s *Storage) Add(ctx context.Context, p *Persistent) (err error) { return nil } -// FindByName finds persistent client by name. And returns its shallow copy. -func (s *Storage) FindByName(name string) (p *Persistent, ok bool) { - s.mu.Lock() - defer s.mu.Unlock() - - p, ok = s.index.findByName(name) - if ok { - return p.ShallowClone(), ok - } - - return nil, false -} - -// FindParams represents the parameters for searching a client. -// -// TODO(s.chzhen): Add a UID field. +// FindParams represents the parameters for searching a client. At least one +// field must be non-empty. type FindParams struct { // ClientID is a unique identifier for the client used in DoH, DoT, and DoQ // DNS queries. @@ -469,15 +455,24 @@ type FindParams struct { // MAC is the physical hardware address used as a client search parameter. MAC net.HardwareAddr + + // UID is the unique ID of persistent client used as a search parameter. + // + // TODO(s.chzhen): Use this. + UID UID } // ParseFindParams parses a string representation of the search parameter into // typed search parameters. +// +// TODO(s.chzhen): Add support for UID. func ParseFindParams(id string) (params *FindParams) { params = &FindParams{} isClientID := true addr, err := netip.ParseAddr(id) + // Even if id can be parsed as an IP address, it may be a MAC address. So + // do not return prematurely, continue parsing. if err == nil { params.RemoteIP = addr isClientID = false @@ -502,16 +497,15 @@ func ParseFindParams(id string) (params *FindParams) { return params } -// FindParams represents the parameters for searching a client. At least one +// Find represents the parameters for searching a client. At least one // field must be non-empty. -func (s *Storage) FindParams(params *FindParams) (p *Persistent, ok bool) { +func (s *Storage) Find(params *FindParams) (p *Persistent, ok bool) { s.mu.Lock() defer s.mu.Unlock() if params.ClientID != "" { p, ok = s.index.findByClientID(params.ClientID) } - if ok { return p.ShallowClone(), true } @@ -519,7 +513,6 @@ func (s *Storage) FindParams(params *FindParams) (p *Persistent, ok bool) { if params.RemoteIP != (netip.Addr{}) { p, ok = s.findByIP(params.RemoteIP) } - if ok { return p.ShallowClone(), true } @@ -527,7 +520,6 @@ func (s *Storage) FindParams(params *FindParams) (p *Persistent, ok bool) { if params.Subnet != (netip.Prefix{}) { p, ok = s.index.findByCIDR(params.Subnet) } - if ok { return p.ShallowClone(), true } @@ -535,7 +527,6 @@ func (s *Storage) FindParams(params *FindParams) (p *Persistent, ok bool) { if params.MAC != nil { p, ok = s.index.findByMAC(params.MAC) } - if ok { return p.ShallowClone(), true } @@ -559,32 +550,6 @@ func (s *Storage) findByIP(addr netip.Addr) (p *Persistent, ok bool) { return nil, false } -// Find finds persistent client by string representation of the ClientID, IP -// address, or MAC. And returns its shallow copy. -// -// TODO(s.chzhen): !! Replace with [Storage.FindParams]. -func (s *Storage) Find(id string) (p *Persistent, ok bool) { - s.mu.Lock() - defer s.mu.Unlock() - - p, ok = s.index.find(id) - if ok { - return p.ShallowClone(), ok - } - - ip, err := netip.ParseAddr(id) - if err != nil { - return nil, false - } - - foundMAC := s.dhcp.MACByIP(ip) - if foundMAC != nil { - return s.FindByMAC(foundMAC) - } - - return nil, false -} - // FindLoose is like [Storage.Find] but it also tries to find a persistent // client by IP address without zone. It strips the IPv6 zone index from the // stored IP addresses before comparing, because querylog entries don't have it. @@ -592,6 +557,8 @@ func (s *Storage) Find(id string) (p *Persistent, ok bool) { // // Note that multiple clients can have the same IP address with different zones. // Therefore, the result of this method is indeterminate. +// +// TODO(s.chzhen): Consider accepting [FindParams]. func (s *Storage) FindLoose(ip netip.Addr, id string) (p *Persistent, ok bool) { s.mu.Lock() defer s.mu.Unlock() @@ -609,17 +576,6 @@ func (s *Storage) FindLoose(ip netip.Addr, id string) (p *Persistent, ok bool) { return nil, false } -// FindByMAC finds persistent client by MAC and returns its shallow copy. s.mu -// is expected to be locked. -func (s *Storage) FindByMAC(mac net.HardwareAddr) (p *Persistent, ok bool) { - p, ok = s.index.findByMAC(mac) - if ok { - return p.ShallowClone(), ok - } - - return nil, false -} - // RemoveByName removes persistent client information. ok is false if no such // client exists by that name. func (s *Storage) RemoveByName(ctx context.Context, name string) (ok bool) { diff --git a/internal/client/storage_internal_test.go b/internal/client/storage_internal_test.go new file mode 100644 index 00000000..a462521c --- /dev/null +++ b/internal/client/storage_internal_test.go @@ -0,0 +1,28 @@ +package client + +import "net" + +// FindByName finds persistent client by name. And returns its shallow copy. +// It is currently only used in tests. +func (s *Storage) FindByName(name string) (p *Persistent, ok bool) { + s.mu.Lock() + defer s.mu.Unlock() + + p, ok = s.index.findByName(name) + if ok { + return p.ShallowClone(), ok + } + + return nil, false +} + +// FindByMAC finds persistent client by MAC and returns its shallow copy. s.mu +// is expected to be locked. It is currently only used in tests. +func (s *Storage) FindByMAC(mac net.HardwareAddr) (p *Persistent, ok bool) { + p, ok = s.index.findByMAC(mac) + if ok { + return p.ShallowClone(), ok + } + + return nil, false +} diff --git a/internal/client/storage_test.go b/internal/client/storage_test.go index 8f8a2443..97e55680 100644 --- a/internal/client/storage_test.go +++ b/internal/client/storage_test.go @@ -519,7 +519,7 @@ func TestClientsDHCP(t *testing.T) { }) require.NoError(t, err) - prsCli, ok := storage.FindParams(client.ParseFindParams(prsCliIP.String())) + prsCli, ok := storage.Find(client.ParseFindParams(prsCliIP.String())) require.True(t, ok) assert.Equal(t, prsCliName, prsCli.Name) @@ -950,7 +950,7 @@ func TestStorage_Find(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { for _, id := range tc.ids { - c, ok := s.FindParams(client.ParseFindParams(id)) + c, ok := s.Find(client.ParseFindParams(id)) require.True(t, ok) assert.Equal(t, tc.want, c) @@ -959,7 +959,7 @@ func TestStorage_Find(t *testing.T) { } t.Run("not_found", func(t *testing.T) { - _, ok := s.FindParams(client.ParseFindParams(cliIPNone)) + _, ok := s.Find(client.ParseFindParams(cliIPNone)) assert.False(t, ok) }) } diff --git a/internal/home/clients.go b/internal/home/clients.go index 6d8139ad..336c4dc2 100644 --- a/internal/home/clients.go +++ b/internal/home/clients.go @@ -364,7 +364,7 @@ func (clients *clientsContainer) shouldCountClient(ids []string) (y bool) { defer clients.lock.Unlock() for _, id := range ids { - client, ok := clients.storage.FindParams(client.ParseFindParams(id)) + client, ok := clients.storage.Find(client.ParseFindParams(id)) if ok { return !client.IgnoreStatistics } diff --git a/internal/home/clientshttp.go b/internal/home/clientshttp.go index 8cce5a21..f8b24097 100644 --- a/internal/home/clientshttp.go +++ b/internal/home/clientshttp.go @@ -446,8 +446,9 @@ func (clients *clientsContainer) handleFindClient(w http.ResponseWriter, r *http // findClient returns available information about a client by idStr from the // client's storage or access settings. cj is guaranteed to be non-nil. func (clients *clientsContainer) findClient(idStr string) (cj *clientJSON) { - ip, _ := netip.ParseAddr(idStr) - c, ok := clients.storage.FindParams(client.ParseFindParams(idStr)) + params := client.ParseFindParams(idStr) + ip := params.RemoteIP + c, ok := clients.storage.Find(params) if !ok { return clients.findRuntime(ip, idStr) }