Skip to content

Commit

Permalink
Allow one Service ACL to bind to multiple services (#1576)
Browse files Browse the repository at this point in the history 
* [caclmgrd] Also ignore IP protocol if found in rule; we will only use our predefined protocols
  • Loading branch information
@jleveque @lguohan
jleveque authored and lguohan committed Apr 11, 2018
1 parent d54b9ef commit 7b36744
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 84 deletions.
4 changes: 2 additions & 2 deletions dockers/docker-snmp-sv2/snmpd-config-updater
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ class ConfigUpdater(object):
if table_data["type"] != self.ACL_TABLE_TYPE_CTRLPLANE:
continue

# Ignore non-SSH service ACLs
if table_data["service"] != self.ACL_SERVICE_SNMP:
# Ignore non-SNMP service ACLs
if self.ACL_SERVICE_SNMP not in table_data["services"]:
continue

acl_rules = {}
Expand Down
138 changes: 67 additions & 71 deletions files/image_config/caclmgrd/caclmgrd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,80 +123,76 @@ class ControlPlaneAclManager(object):
if table_data["type"] != self.ACL_TABLE_TYPE_CTRLPLANE:
continue

acl_service = table_data["service"]
acl_services = table_data["services"]

if acl_service not in self.ACL_SERVICES:
log_warning("Ignoring control plane ACL '{}' with unrecognized service '{}'"
.format(table_name, acl_service))
continue

log_info("Translating ACL rules for control plane ACL '{}' (service: '{}')"
.format(table_name, acl_service))

# Obtain default IP protocol(s) and destination port(s) for this service
ip_protocols = self.ACL_SERVICES[acl_service]["ip_protocols"]
dst_ports = self.ACL_SERVICES[acl_service]["dst_ports"]

acl_rules = {}

for ((rule_table_name, rule_id), rule_props) in self._rules_db_info.iteritems():
if rule_table_name == table_name:
acl_rules[rule_props["PRIORITY"]] = rule_props

# For each ACL rule in this table (in descending order of priority)
for priority in sorted(acl_rules.iterkeys(), reverse=True):
rule_props = acl_rules[priority]

if "PACKET_ACTION" not in rule_props:
log_error("ACL rule does not contain PACKET_ACTION property")
for acl_service in acl_services:
if acl_service not in self.ACL_SERVICES:
log_warning("Ignoring control plane ACL '{}' with unrecognized service '{}'"
.format(table_name, acl_service))
continue

# If the rule contains an IP protocol, we will use it.
# Otherwise, we will apply the rule to the default
# protocol(s) for this ACL service
if "IP_PROTOCOL" in rule_props:
ip_protocols = [rule_props["IP_PROTOCOL"]]

for ip_protocol in ip_protocols:
for dst_port in dst_ports:
rule_cmd = "iptables -A INPUT -p {}".format(ip_protocol)

if "SRC_IP" in rule_props and rule_props["SRC_IP"]:
rule_cmd += " -s {}".format(rule_props["SRC_IP"])

rule_cmd += " --dport {}".format(dst_port)

# If there are TCP flags present, append them
if "TCP_FLAGS" in rule_props and rule_props["TCP_FLAGS"]:
tcp_flags = int(rule_props["TCP_FLAGS"], 16)

if tcp_flags > 0:
rule_cmd += " --tcp-flags "

if tcp_flags & 0x01:
rule_cmd += "FIN,"
if tcp_flags & 0x02:
rule_cmd += "SYN,"
if tcp_flags & 0x04:
rule_cmd += "RST,"
if tcp_flags & 0x08:
rule_cmd += "PSH,"
if tcp_flags & 0x10:
rule_cmd += "ACK,"
if tcp_flags & 0x20:
rule_cmd += "URG,"
if tcp_flags & 0x40:
rule_cmd += "ECE,"
if tcp_flags & 0x80:
rule_cmd += "CWR,"

# Delete the trailing comma
rule_cmd = rule_cmd[:-1]

# Append the packet action as the jump target
rule_cmd += " -j {}".format(rule_props["PACKET_ACTION"])

iptables_cmds.append(rule_cmd)
log_info("Translating ACL rules for control plane ACL '{}' (service: '{}')"
.format(table_name, acl_service))

# Obtain default IP protocol(s) and destination port(s) for this service
ip_protocols = self.ACL_SERVICES[acl_service]["ip_protocols"]
dst_ports = self.ACL_SERVICES[acl_service]["dst_ports"]

acl_rules = {}

for ((rule_table_name, rule_id), rule_props) in self._rules_db_info.iteritems():
if rule_table_name == table_name:
acl_rules[rule_props["PRIORITY"]] = rule_props

# For each ACL rule in this table (in descending order of priority)
for priority in sorted(acl_rules.iterkeys(), reverse=True):
rule_props = acl_rules[priority]

if "PACKET_ACTION" not in rule_props:
log_error("ACL rule does not contain PACKET_ACTION property")
continue

# Apply the rule to the default protocol(s) for this ACL service
for ip_protocol in ip_protocols:
for dst_port in dst_ports:
rule_cmd = "iptables -A INPUT -p {}".format(ip_protocol)

if "SRC_IP" in rule_props and rule_props["SRC_IP"]:
rule_cmd += " -s {}".format(rule_props["SRC_IP"])

rule_cmd += " --dport {}".format(dst_port)

# If there are TCP flags present, append them
if "TCP_FLAGS" in rule_props and rule_props["TCP_FLAGS"]:
tcp_flags = int(rule_props["TCP_FLAGS"], 16)

if tcp_flags > 0:
rule_cmd += " --tcp-flags "

if tcp_flags & 0x01:
rule_cmd += "FIN,"
if tcp_flags & 0x02:
rule_cmd += "SYN,"
if tcp_flags & 0x04:
rule_cmd += "RST,"
if tcp_flags & 0x08:
rule_cmd += "PSH,"
if tcp_flags & 0x10:
rule_cmd += "ACK,"
if tcp_flags & 0x20:
rule_cmd += "URG,"
if tcp_flags & 0x40:
rule_cmd += "ECE,"
if tcp_flags & 0x80:
rule_cmd += "CWR,"

# Delete the trailing comma
rule_cmd = rule_cmd[:-1]

# Append the packet action as the jump target
rule_cmd += " -j {}".format(rule_props["PACKET_ACTION"])

iptables_cmds.append(rule_cmd)

return iptables_cmds

Expand Down
2 changes: 1 addition & 1 deletion files/image_config/ssh/sshd-config-updater
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class ConfigUpdater(object):
continue

# Ignore non-SSH service ACLs
if table_data["service"] != self.ACL_SERVICE_SSH:
if self.ACL_SERVICE_SSH not in table_data["services"]:
continue

acl_rules = {}
Expand Down
24 changes: 19 additions & 5 deletions src/sonic-config-engine/minigraph.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ def parse_dpg(dpg, hname):
aclattach = aclintf.find(str(QName(ns, "AttachTo"))).text.split(';')
acl_intfs = []
is_mirror = False

# TODO: Ensure that acl_intfs will only ever contain front-panel interfaces (e.g.,
# maybe we should explicity ignore management and loopback interfaces?) because we
# decide an ACL is a Control Plane ACL if acl_intfs is empty below.
for member in aclattach:
member = member.strip()
if pcs.has_key(member):
Expand All @@ -209,12 +213,22 @@ def parse_dpg(dpg, hname):
# This ACL has no interfaces to attach to -- consider this a control plane ACL
try:
aclservice = aclintf.find(str(QName(ns, "Type"))).text
acls[aclname] = {'policy_desc': aclname,
'ports': acl_intfs,
'type': 'CTRLPLANE',
'service': aclservice if aclservice is not None else 'UNKNOWN'}

# If we already have an ACL with this name and this ACL is bound to a different service,
# append the service to our list of services
if aclname in acls:
if acls[aclname]['type'] != 'CTRLPLANE':
print >> sys.stderr, "Warning: ACL '%s' type mismatch. Not updating ACL." % aclname
elif acls[aclname]['services'] == aclservice:
print >> sys.stderr, "Warning: ACL '%s' already contains service '%s'. Not updating ACL." % (aclname, aclservice)
else:
acls[aclname]['services'].append(aclservice)
else:
acls[aclname] = {'policy_desc': aclname,
'type': 'CTRLPLANE',
'services': [aclservice]}
except:
print >> sys.stderr, "Warning: Ingore Control Plane ACL %s without type" % aclname
print >> sys.stderr, "Warning: Ignoring Control Plane ACL %s without type" % aclname

return intfs, lo_intfs, mgmt_intf, vlans, vlan_members, pcs, acls
return None, None, None, None, None, None, None
Expand Down
10 changes: 10 additions & 0 deletions src/sonic-config-engine/tests/t0-sample-graph.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,16 @@
<InAcl>SSH_ACL</InAcl>
<Type>SSH</Type>
</AclInterface>
<AclInterface>
<AttachTo>SSH</AttachTo>
<InAcl>ROUTER-PROTECT</InAcl>
<Type>SSH</Type>
</AclInterface>
<AclInterface>
<AttachTo>SNMP</AttachTo>
<InAcl>ROUTER-PROTECT</InAcl>
<Type>SNMP</Type>
</AclInterface>
<AclInterface>
<AttachTo>NTP</AttachTo>
<InAcl>NTP_ACL</InAcl>
Expand Down
10 changes: 6 additions & 4 deletions src/sonic-config-engine/tests/test_cfggen.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,13 @@ def test_render_template(self):
def test_minigraph_acl(self):
argument = '-m "' + self.sample_graph_t0 + '" -p "' + self.port_config + '" -v ACL_TABLE'
output = self.run_script(argument, True)
self.assertEqual(output.strip(), "Warning: Ingore Control Plane ACL NTP_ACL without type\n"
"{'SSH_ACL': {'type': 'CTRLPLANE', 'policy_desc': 'SSH_ACL', 'service': 'SSH', 'ports': []},"
" 'SNMP_ACL': {'type': 'CTRLPLANE', 'policy_desc': 'SNMP_ACL', 'service': 'SNMP', 'ports': []},"
self.assertEqual(output.strip(), "Warning: Ignoring Control Plane ACL NTP_ACL without type\n"
"{'SSH_ACL': {'services': ['SSH'], 'type': 'CTRLPLANE', 'policy_desc': 'SSH_ACL'},"
" 'SNMP_ACL': {'services': ['SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'SNMP_ACL'},"
" 'DATAACL': {'type': 'L3', 'policy_desc': 'DATAACL', 'ports': ['Ethernet112', 'Ethernet116', 'Ethernet120', 'Ethernet124']},"
" 'NTP_ACL': {'type': 'CTRLPLANE', 'policy_desc': 'NTP_ACL', 'service': 'NTP', 'ports': []}}")
" 'NTP_ACL': {'services': ['NTP'], 'type': 'CTRLPLANE', 'policy_desc': 'NTP_ACL'},"
" 'ROUTER_PROTECT': {'services': ['SSH', 'SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'ROUTER_PROTECT'}}")

def test_minigraph_everflow(self):
argument = '-m "' + self.sample_graph_t0 + '" -p "' + self.port_config + '" -v MIRROR_SESSION'
output = self.run_script(argument)
Expand Down
2 changes: 1 addition & 1 deletion src/sonic-utilities
Submodule sonic-utilities updated from 3f2d7b to 5e476d

0 comments on commit 7b36744

Please sign in to comment.