From 2c12d5f860cef4504e5cca17741d00b944e30699 Mon Sep 17 00:00:00 2001 From: Andrey Meshkov Date: Tue, 8 Sep 2020 13:56:45 +0300 Subject: [PATCH] Improve the clients/find API response --- AGHTechDoc.md | 12 +++--- dnsforward/access.go | 5 ++- dnsforward/access_test.go | 73 +++++++++++++++++++++++++++++++++++ dnsforward/dnsforward_test.go | 52 ------------------------- home/clients.go | 3 +- home/clients_http.go | 26 ++++++------- home/dns.go | 2 +- openapi/openapi.yaml | 10 +++-- 8 files changed, 103 insertions(+), 80 deletions(-) create mode 100644 dnsforward/access_test.go diff --git a/AGHTechDoc.md b/AGHTechDoc.md index 196515a5..0ac594ef 100644 --- a/AGHTechDoc.md +++ b/AGHTechDoc.md @@ -940,7 +940,7 @@ Error response (Client not found): ### API: Find clients by IP This method returns the list of clients (manual and auto-clients) matching the IP list. -For auto-clients only `name`, `ids`, `whois_info`, `disallowed` fields are set. Other fields are empty. +For auto-clients only `name`, `ids`, `whois_info`, `disallowed`, and `disallowed_rule` fields are set. Other fields are empty. Request: @@ -967,17 +967,15 @@ Response: ... } - "disallowed": "..." + "disallowed": false, + "disallowed_rule": "..." } } ... ] -`disallowed`: -* "": IP is allowed -* not "", e.g. "127.0.0.0/24" - IP is disallowed by "disallowed IP list", and the string contains the matched rule (IP or CIDR) -* "not-in-allowed-list" - IP is disallowed by "allowed IP list" - +* `disallowed` - whether the client's IP is blocked or not. +* `disallowed_rule` - the rule due to which the client is disallowed. If `disallowed` is `true`, and this string is empty - it means that the client IP is disallowed by the "allowed IP list", i.e. it is not included in allowed. ## DNS general settings diff --git a/dnsforward/access.go b/dnsforward/access.go index 1a56dc84..dbe60de5 100644 --- a/dnsforward/access.go +++ b/dnsforward/access.go @@ -80,6 +80,9 @@ func processIPCIDRArray(dst *map[string]bool, dstIPNet *[]net.IPNet, src []strin } // IsBlockedIP - return TRUE if this client should be blocked +// Returns the item from the "disallowedClients" list that lead to blocking IP. +// If it returns TRUE and an empty string, it means that the "allowedClients" is not empty, +// but the ip does not belong to it. func (a *accessCtx) IsBlockedIP(ip string) (bool, string) { a.lock.Lock() defer a.lock.Unlock() @@ -99,7 +102,7 @@ func (a *accessCtx) IsBlockedIP(ip string) (bool, string) { } } - return true, "not-in-allowed-list" + return true, "" } _, ok := a.disallowedClients[ip] diff --git a/dnsforward/access_test.go b/dnsforward/access_test.go new file mode 100644 index 00000000..b760d554 --- /dev/null +++ b/dnsforward/access_test.go @@ -0,0 +1,73 @@ +package dnsforward + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsBlockedIPAllowed(t *testing.T) { + a := &accessCtx{} + assert.True(t, a.Init([]string{"1.1.1.1", "2.2.0.0/16"}, nil, nil) == nil) + + disallowed, disallowedRule := a.IsBlockedIP("1.1.1.1") + assert.False(t, disallowed) + assert.Equal(t, "", disallowedRule) + + disallowed, disallowedRule = a.IsBlockedIP("1.1.1.2") + assert.True(t, disallowed) + assert.Equal(t, "", disallowedRule) + + disallowed, disallowedRule = a.IsBlockedIP("2.2.1.1") + assert.False(t, disallowed) + assert.Equal(t, "", disallowedRule) + + disallowed, disallowedRule = a.IsBlockedIP("2.3.1.1") + assert.True(t, disallowed) + assert.Equal(t, "", disallowedRule) +} + +func TestIsBlockedIPDisallowed(t *testing.T) { + a := &accessCtx{} + assert.True(t, a.Init(nil, []string{"1.1.1.1", "2.2.0.0/16"}, nil) == nil) + + disallowed, disallowedRule := a.IsBlockedIP("1.1.1.1") + assert.True(t, disallowed) + assert.Equal(t, "1.1.1.1", disallowedRule) + + disallowed, disallowedRule = a.IsBlockedIP("1.1.1.2") + assert.False(t, disallowed) + assert.Equal(t, "", disallowedRule) + + disallowed, disallowedRule = a.IsBlockedIP("2.2.1.1") + assert.True(t, disallowed) + assert.Equal(t, "2.2.0.0/16", disallowedRule) + + disallowed, disallowedRule = a.IsBlockedIP("2.3.1.1") + assert.False(t, disallowed) + assert.Equal(t, "", disallowedRule) +} + +func TestIsBlockedIPBlockedDomain(t *testing.T) { + a := &accessCtx{} + assert.True(t, a.Init(nil, nil, []string{"host1", + "host2", + "*.host.com", + "||host3.com^", + }) == nil) + + // match by "host2.com" + assert.True(t, a.IsBlockedDomain("host1")) + assert.True(t, a.IsBlockedDomain("host2")) + assert.True(t, !a.IsBlockedDomain("host3")) + + // match by wildcard "*.host.com" + assert.True(t, !a.IsBlockedDomain("host.com")) + assert.True(t, a.IsBlockedDomain("asdf.host.com")) + assert.True(t, a.IsBlockedDomain("qwer.asdf.host.com")) + assert.True(t, !a.IsBlockedDomain("asdf.zhost.com")) + + // match by wildcard "||host3.com^" + assert.True(t, a.IsBlockedDomain("host3.com")) + assert.True(t, a.IsBlockedDomain("asdf.host3.com")) +} diff --git a/dnsforward/dnsforward_test.go b/dnsforward/dnsforward_test.go index 6099fed0..605b52a9 100644 --- a/dnsforward/dnsforward_test.go +++ b/dnsforward/dnsforward_test.go @@ -874,58 +874,6 @@ func publicKey(priv interface{}) interface{} { } } -func TestIsBlockedIPAllowed(t *testing.T) { - a := &accessCtx{} - assert.True(t, a.Init([]string{"1.1.1.1", "2.2.0.0/16"}, nil, nil) == nil) - - disallowed, _ := a.IsBlockedIP("1.1.1.1") - assert.False(t, disallowed) - disallowed, _ = a.IsBlockedIP("1.1.1.2") - assert.True(t, disallowed) - disallowed, _ = a.IsBlockedIP("2.2.1.1") - assert.False(t, disallowed) - disallowed, _ = a.IsBlockedIP("2.3.1.1") - assert.True(t, disallowed) -} - -func TestIsBlockedIPDisallowed(t *testing.T) { - a := &accessCtx{} - assert.True(t, a.Init(nil, []string{"1.1.1.1", "2.2.0.0/16"}, nil) == nil) - - disallowed, _ := a.IsBlockedIP("1.1.1.1") - assert.True(t, disallowed) - disallowed, _ = a.IsBlockedIP("1.1.1.2") - assert.False(t, disallowed) - disallowed, _ = a.IsBlockedIP("2.2.1.1") - assert.True(t, disallowed) - disallowed, _ = a.IsBlockedIP("2.3.1.1") - assert.False(t, disallowed) -} - -func TestIsBlockedIPBlockedDomain(t *testing.T) { - a := &accessCtx{} - assert.True(t, a.Init(nil, nil, []string{"host1", - "host2", - "*.host.com", - "||host3.com^", - }) == nil) - - // match by "host2.com" - assert.True(t, a.IsBlockedDomain("host1")) - assert.True(t, a.IsBlockedDomain("host2")) - assert.True(t, !a.IsBlockedDomain("host3")) - - // match by wildcard "*.host.com" - assert.True(t, !a.IsBlockedDomain("host.com")) - assert.True(t, a.IsBlockedDomain("asdf.host.com")) - assert.True(t, a.IsBlockedDomain("qwer.asdf.host.com")) - assert.True(t, !a.IsBlockedDomain("asdf.zhost.com")) - - // match by wildcard "||host3.com^" - assert.True(t, a.IsBlockedDomain("host3.com")) - assert.True(t, a.IsBlockedDomain("asdf.host3.com")) -} - func TestValidateUpstream(t *testing.T) { invalidUpstreams := []string{"1.2.3.4.5", "123.3.7m", diff --git a/home/clients.go b/home/clients.go index 81ee9bec..6a1de852 100644 --- a/home/clients.go +++ b/home/clients.go @@ -80,7 +80,8 @@ type clientsContainer struct { // dhcpServer is used for looking up clients IP addresses by MAC addresses dhcpServer *dhcpd.Server - DNSServer *dnsforward.Server + // dnsServer is used for checking clients IP status access list status + dnsServer *dnsforward.Server autoHosts *util.AutoHosts // get entries from system hosts-files diff --git a/home/clients_http.go b/home/clients_http.go index 40130d42..42fa6f2a 100644 --- a/home/clients_http.go +++ b/home/clients_http.go @@ -24,10 +24,14 @@ type clientJSON struct { WhoisInfo map[string]interface{} `json:"whois_info"` - // * "": IP is allowed - // * not "", e.g. "127.0.0.0/24" - IP is disallowed by "disallowed IP list", and the string contains the matched rule (IP or CIDR) - // * "not-in-allowed-list" - IP is disallowed by "allowed IP list" - Disallowed string `json:"disallowed"` + // Disallowed - if true -- client's IP is not disallowed + // Otherwise, it is blocked. + Disallowed bool `json:"disallowed"` + + // DisallowedRule - the rule due to which the client is disallowed + // If Disallowed is true, and this string is empty - it means that the client IP + // is disallowed by the "allowed IP list", i.e. it is not included in allowed. + DisallowedRule string `json:"disallowed_rule"` } type clientHostJSON struct { @@ -45,7 +49,7 @@ type clientListJSON struct { } // respond with information about configured clients -func (clients *clientsContainer) handleGetClients(w http.ResponseWriter, r *http.Request) { +func (clients *clientsContainer) handleGetClients(w http.ResponseWriter, _ *http.Request) { data := clientListJSON{} clients.lock.Lock() @@ -257,20 +261,12 @@ func (clients *clientsContainer) handleFindClient(w http.ResponseWriter, r *http } cj := clientHostToJSON(ip, ch) - disallowed, disallowedRule := clients.DNSServer.IsBlockedIP(ip) - if disallowed { - cj.Disallowed = disallowedRule - } - + cj.Disallowed, cj.DisallowedRule = clients.dnsServer.IsBlockedIP(ip) el[ip] = cj } else { cj := clientToJSON(&c) - disallowed, disallowedRule := clients.DNSServer.IsBlockedIP(ip) - if disallowed { - cj.Disallowed = disallowedRule - } - + cj.Disallowed, cj.DisallowedRule = clients.dnsServer.IsBlockedIP(ip) el[ip] = cj } diff --git a/home/dns.go b/home/dns.go index e44c1ff1..c28ebd1f 100644 --- a/home/dns.go +++ b/home/dns.go @@ -70,7 +70,7 @@ func initDNSServer() error { p.DHCPServer = Context.dhcpServer } Context.dnsServer = dnsforward.NewServer(p) - Context.clients.DNSServer = Context.dnsServer + Context.clients.dnsServer = Context.dnsServer dnsConfig := generateServerConfig() err = Context.dnsServer.Prepare(&dnsConfig) if err != nil { diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index b9be01c3..52791130 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -1796,11 +1796,15 @@ components: items: $ref: "#/components/schemas/WhoisInfo" disallowed: + type: boolean description: > - * "": IP is allowed - * not "", e.g. "127.0.0.0/24" - IP is disallowed by "disallowed IP list", and the string contains the matched rule (IP or CIDR) - * "not-in-allowed-list" - IP is disallowed by "allowed IP list" + Whether the client's IP is blocked or not. + disallowed_rule: type: string + description: > + The rule due to which the client is disallowed. + If `disallowed` is `true`, and this string is empty - it means that + the client IP is disallowed by the "allowed IP list", i.e. it is not included in allowed list. WhoisInfo: type: object