Kwesi/net 326 bug client ac ls (#2462)

* feat(NET-326): return 200 [] instead of 500 when there are not network acls

* fix(NET-326): implement allow/deny client acl functions

* fix(NET-326): implement extclient acl update

* fix(NET-326): kame fixes, send peer updates

controllers/ext_client.go

@@ -423,6 +423,7 @@ func updateExtClient(w http.ResponseWriter, r *http.Request) {
 
 	var update models.CustomExtClient
 	var oldExtClient models.ExtClient
+	var sendPeerUpdate bool
 	err := json.NewDecoder(r.Body).Decode(&update)
 	if err != nil {
 		logger.Log(0, r.Header.Get("user"), "error decoding request body: ",
@@ -478,9 +479,15 @@ func updateExtClient(w http.ResponseWriter, r *http.Request) {
 			logger.Log(0, "failed to associate client", update.ClientID, "to user", oldExtClient.OwnerID)
 		}
 	}
+	if len(update.DeniedACLs) != len(oldExtClient.DeniedACLs) {
+		sendPeerUpdate = true
+		logic.SetClientACLs(&oldExtClient, update.DeniedACLs)
+	}
 	// == END PRO ==
 
-	var changedEnabled = (update.Enabled != oldExtClient.Enabled) // indicates there was a change in enablement
+	if update.Enabled != oldExtClient.Enabled {
+		sendPeerUpdate = true
+	}
 	// extra var need as logic.Update changes oldExtClient
 	currentClient := oldExtClient
 	newclient, err := logic.UpdateExtClient(&oldExtClient, &update)
@@ -492,7 +499,7 @@ func updateExtClient(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	logger.Log(0, r.Header.Get("user"), "updated ext client", update.ClientID)
-	if changedEnabled { // need to send a peer update to the ingress node as enablement of one of it's clients has changed
+	if sendPeerUpdate { // need to send a peer update to the ingress node as enablement of one of it's clients has changed
 		if ingressNode, err := logic.GetNodeByID(newclient.IngressGatewayID); err == nil {
 			if err = mq.PublishPeerUpdate(); err != nil {
 				logger.Log(1, "error setting ext peers on", ingressNode.ID.String(), ":", err.Error())

ee/logic/ext_acls.go

@@ -7,11 +7,11 @@ func DenyClientNode(ec *models.ExtClient, clientOrNodeID string) (ok bool) {
 	if ec == nil || len(clientOrNodeID) == 0 {
 		return
 	}
-	if ec.ACLs == nil {
-		ec.ACLs = map[string]struct{}{}
+	if ec.DeniedACLs == nil {
+		ec.DeniedACLs = map[string]struct{}{}
 	}
 	ok = true
-	ec.ACLs[clientOrNodeID] = struct{}{}
+	ec.DeniedACLs[clientOrNodeID] = struct{}{}
 	return
 }
 
@@ -20,22 +20,22 @@ func IsClientNodeAllowed(ec *models.ExtClient, clientOrNodeID string) bool {
 	if ec == nil || len(clientOrNodeID) == 0 {
 		return false
 	}
-	if ec.ACLs == nil {
+	if ec.DeniedACLs == nil {
 		return true
 	}
-	_, ok := ec.ACLs[clientOrNodeID]
-	return ok
+	_, ok := ec.DeniedACLs[clientOrNodeID]
+	return !ok
 }
 
 // RemoveDeniedNodeFromClient - removes a node id from set of denied nodes
 func RemoveDeniedNodeFromClient(ec *models.ExtClient, clientOrNodeID string) bool {
-	if ec.ACLs == nil {
+	if ec.DeniedACLs == nil {
 		return true
 	}
-	_, ok := ec.ACLs[clientOrNodeID]
+	_, ok := ec.DeniedACLs[clientOrNodeID]
 	if !ok {
 		return false
 	}
-	delete(ec.ACLs, clientOrNodeID)
+	delete(ec.DeniedACLs, clientOrNodeID)
 	return true
 }

logic/clients.go

@@ -10,11 +10,17 @@ import (
 
 var (
 	// DenyClientNodeAccess - function to handle adding a node to an ext client's denied node set
-	DenyClientNodeAccess = func(ec *models.ExtClient, clientOrNodeID string) bool { return true }
+	DenyClientNodeAccess = func(ec *models.ExtClient, clientOrNodeID string) bool {
+		return true
+	}
 	// IsClientNodeAllowed - function to check if an ext client's denied node set contains a node ID
-	IsClientNodeAllowed = func(ec *models.ExtClient, clientOrNodeID string) bool { return true }
+	IsClientNodeAllowed = func(ec *models.ExtClient, clientOrNodeID string) bool {
+		return true
+	}
 	// AllowClientNodeAccess - function to handle removing a node ID from ext client's denied nodes, thus allowing it
-	AllowClientNodeAccess = func(ec *models.ExtClient, clientOrNodeID string) bool { return true }
+	AllowClientNodeAccess = func(ec *models.ExtClient, clientOrNodeID string) bool {
+		return true
+	}
 )
 
 // SetClientDefaultACLs - set's a client's default ACLs based on network and nodes in network
@@ -34,6 +40,8 @@ func SetClientDefaultACLs(ec *models.ExtClient) error {
 		currNode := networkNodes[i]
 		if network.DefaultACL == "no" || currNode.DefaultACL == "no" {
 			DenyClientNodeAccess(ec, currNode.ID.String())
+		} else {
+			AllowClientNodeAccess(ec, currNode.ID.String())
 		}
 	}
 	return nil
@@ -44,7 +52,7 @@ func SetClientACLs(ec *models.ExtClient, newACLs map[string]struct{}) {
 	if ec == nil || newACLs == nil || !isEE {
 		return
 	}
-	ec.ACLs = newACLs
+	ec.DeniedACLs = newACLs
 }
 
 // IsClientNodeAllowedByID - checks if a given ext client ID + nodeID are allowed

logic/extpeers.go

@@ -3,6 +3,7 @@ package logic
 import (
 	"encoding/json"
 	"fmt"
+	"reflect"
 	"sync"
 	"time"
 
@@ -94,6 +95,9 @@ func GetNetworkExtClients(network string) ([]models.ExtClient, error) {
 	}
 	records, err := database.FetchRecords(database.EXT_CLIENT_TABLE_NAME)
 	if err != nil {
+		if database.IsEmptyRecord(err) {
+			return extclients, nil
+		}
 		return extclients, err
 	}
 	for _, value := range records {
@@ -231,6 +235,9 @@ func UpdateExtClient(old *models.ExtClient, update *models.CustomExtClient) (*mo
 	if update.ExtraAllowedIPs != nil && StringDifference(old.ExtraAllowedIPs, update.ExtraAllowedIPs) != nil {
 		new.ExtraAllowedIPs = update.ExtraAllowedIPs
 	}
+	if update.DeniedACLs != nil && !reflect.DeepEqual(old.DeniedACLs, update.DeniedACLs) {
+		new.DeniedACLs = update.DeniedACLs
+	}
 	return new, CreateExtClient(new)
 }
 

logic/hosts.go

@@ -409,6 +409,15 @@ func DissasociateNodeFromHost(n *models.Node, h *models.Host) error {
 	} else {
 		h.Nodes = RemoveStringSlice(h.Nodes, index)
 	}
+	go func() {
+		if servercfg.Is_EE {
+			if clients, err := GetNetworkExtClients(n.Network); err != nil {
+				for i := range clients {
+					AllowClientNodeAccess(&clients[i], n.ID.String())
+				}
+			}
+		}
+	}()
 	if err := deleteNodeByID(n); err != nil {
 		return err
 	}

logic/peers.go

@@ -199,7 +199,7 @@ func GetPeerUpdateForHost(network string, host *models.Host, allNodes []models.N
 			}
 			if node.IsIngressGateway || node.IsEgressGateway {
 				if peer.IsIngressGateway {
-					_, extPeerIDAndAddrs, err := getExtPeers(&peer)
+					_, extPeerIDAndAddrs, err := getExtPeers(&peer, &node)
 					if err == nil {
 						for _, extPeerIdAndAddr := range extPeerIDAndAddrs {
 							extPeerIdAndAddr := extPeerIdAndAddr
@@ -322,7 +322,7 @@ func GetPeerUpdateForHost(network string, host *models.Host, allNodes []models.N
 		var extPeers []wgtypes.PeerConfig
 		var extPeerIDAndAddrs []models.IDandAddr
 		if node.IsIngressGateway {
-			extPeers, extPeerIDAndAddrs, err = getExtPeers(&node)
+			extPeers, extPeerIDAndAddrs, err = getExtPeers(&node, &node)
 			if err == nil {
 				for _, extPeerIdAndAddr := range extPeerIDAndAddrs {
 					extPeerIdAndAddr := extPeerIdAndAddr
@@ -463,7 +463,7 @@ func GetProxyListenPort(host *models.Host) int {
 	return proxyPort
 }
 
-func getExtPeers(node *models.Node) ([]wgtypes.PeerConfig, []models.IDandAddr, error) {
+func getExtPeers(node, peer *models.Node) ([]wgtypes.PeerConfig, []models.IDandAddr, error) {
 	var peers []wgtypes.PeerConfig
 	var idsAndAddr []models.IDandAddr
 	extPeers, err := GetNetworkExtClients(node.Network)
@@ -476,6 +476,9 @@ func getExtPeers(node *models.Node) ([]wgtypes.PeerConfig, []models.IDandAddr, e
 	}
 	for _, extPeer := range extPeers {
 		extPeer := extPeer
+		if !IsClientNodeAllowed(&extPeer, peer.ID.String()) {
+			continue
+		}
 		pubkey, err := wgtypes.ParseKey(extPeer.PublicKey)
 		if err != nil {
 			logger.Log(1, "error parsing ext pub key:", err.Error())
@@ -598,7 +601,7 @@ func GetAllowedIPs(node, peer *models.Node, metrics *models.Metrics) []net.IPNet
 
 	// handle ingress gateway peers
 	if peer.IsIngressGateway {
-		extPeers, _, err := getExtPeers(peer)
+		extPeers, _, err := getExtPeers(peer, node)
 		if err != nil {
 			logger.Log(2, "could not retrieve ext peers for ", peer.ID.String(), err.Error())
 		}
@@ -766,7 +769,7 @@ func filterNodeMapForClientACLs(publicKey, network string, nodePeerMap map[strin
 	}
 	for k := range nodePeerMap {
 		currNodePeer := nodePeerMap[k]
-		if _, ok := client.ACLs[currNodePeer.ID]; ok {
+		if _, ok := client.DeniedACLs[currNodePeer.ID]; ok {
 			delete(nodePeerMap, k)
 		}
 	}

models/extclient.go

@@ -15,14 +15,15 @@ type ExtClient struct {
 	LastModified           int64               `json:"lastmodified" bson:"lastmodified"`
 	Enabled                bool                `json:"enabled" bson:"enabled"`
 	OwnerID                string              `json:"ownerid" bson:"ownerid"`
-	ACLs                   map[string]struct{} `json:"acls,omitempty" bson:"acls,omitempty"`
+	DeniedACLs             map[string]struct{} `json:"deniednodeacls" bson:"acls,omitempty"`
 }
 
 // CustomExtClient - struct for CustomExtClient params
 type CustomExtClient struct {
-	ClientID        string   `json:"clientid,omitempty"`
-	PublicKey       string   `json:"publickey,omitempty"`
-	DNS             string   `json:"dns,omitempty"`
-	ExtraAllowedIPs []string `json:"extraallowedips,omitempty"`
-	Enabled         bool     `json:"enabled,omitempty"`
+	ClientID        string              `json:"clientid,omitempty"`
+	PublicKey       string              `json:"publickey,omitempty"`
+	DNS             string              `json:"dns,omitempty"`
+	ExtraAllowedIPs []string            `json:"extraallowedips,omitempty"`
+	Enabled         bool                `json:"enabled,omitempty"`
+	DeniedACLs      map[string]struct{} `json:"deniednodeacls" bson:"acls,omitempty"`
 }