Skip to content
Snippets Groups Projects
Commit a2bffd82 authored by Alex Reimers's avatar Alex Reimers
Browse files

FL-82 - Firewall.java - Protect the firewall rules with synchronization.

parent 2b6b0b3c
No related branches found
No related tags found
No related merge requests found
......@@ -40,7 +40,7 @@ public class Firewall implements IFirewallService, IOFMessageListener, IFloodlig
protected IStorageSourceService storageSource;
protected IRestApiService restApi;
protected static Logger logger;
protected ArrayList<FirewallRule> rules;
protected List<FirewallRule> rules; // protected by synchornized
protected boolean enabled;
protected int subnet_mask = IPv4.toIPv4Address("255.255.255.0");
......@@ -119,19 +119,7 @@ public class Firewall implements IFirewallService, IOFMessageListener, IFloodlig
l.add(IRestApiService.class);
return l;
}
public IFloodlightProviderService getFloodlightProvider() {
return floodlightProvider;
}
public void setFloodlightProvider(IFloodlightProviderService floodlightProvider) {
this.floodlightProvider = floodlightProvider;
}
public void setStorageSource(IStorageSourceService storageSource) {
this.storageSource = storageSource;
}
/**
* Reads the rules from the storage and creates a sorted arraylist of FirewallRule
* from them.
......@@ -223,14 +211,6 @@ public class Firewall implements IFirewallService, IOFMessageListener, IFloodlig
return l;
}
/**
* used for debugging and unittests
* @return the number of rules
*/
public int countRules() {
return this.rules.size();
}
@Override
public void init(FloodlightModuleContext context)
throws FloodlightModuleException {
......@@ -254,12 +234,14 @@ public class Firewall implements IFirewallService, IOFMessageListener, IFloodlig
// storage, create table and read rules
storageSource.createTable(TABLE_NAME, null);
storageSource.setTablePrimaryKeyName(TABLE_NAME, COLUMN_RULEID);
this.rules = readRulesFromStorage();
synchronized (rules) {
this.rules = readRulesFromStorage();
}
}
@Override
public Command receive(IOFSwitch sw, OFMessage msg, FloodlightContext cntx) {
if (this.enabled == false) return Command.CONTINUE;
if (!this.enabled) return Command.CONTINUE;
switch (msg.getType()) {
case PACKET_IN:
......@@ -284,7 +266,7 @@ public class Firewall implements IFirewallService, IOFMessageListener, IFloodlig
public void enableFirewall() {
// check if the firewall module is not listening for events, if not, then start listening (enable it)
List<IOFMessageListener> listeners = floodlightProvider.getListeners().get(OFType.PACKET_IN);
if (listeners != null && listeners.contains(this) == false) {
if ((listeners != null) && (!listeners.contains(this))) {
// enable firewall, i.e. listen for packet-in events
floodlightProvider.addOFMessageListener(OFType.PACKET_IN, this);
}
......@@ -295,7 +277,7 @@ public class Firewall implements IFirewallService, IOFMessageListener, IFloodlig
public void disableFirewall() {
// check if the firewall module is listening for events, if yes, then remove it from listeners (disable it)
List<IOFMessageListener> listeners = floodlightProvider.getListeners().get(OFType.PACKET_IN);
if (listeners != null && listeners.contains(this) == true) {
if ((listeners != null) && (listeners.contains(this))) {
// disable firewall, i.e. stop listening for packet-in events
floodlightProvider.removeOFMessageListener(OFType.PACKET_IN, this);
}
......@@ -336,7 +318,7 @@ public class Firewall implements IFirewallService, IOFMessageListener, IFloodlig
}
@Override
public void addRule(FirewallRule rule) {
public synchronized void addRule(FirewallRule rule) {
rule.ruleid = rule.genID();
int i = 0;
// locate the position of the new rule in the sorted arraylist
......@@ -376,7 +358,7 @@ public class Firewall implements IFirewallService, IOFMessageListener, IFloodlig
}
@Override
public void deleteRule(int ruleid) {
public synchronized void deleteRule(int ruleid) {
Iterator<FirewallRule> iter = this.rules.iterator();
while (iter.hasNext()) {
FirewallRule r = iter.next();
......@@ -419,18 +401,19 @@ public class Firewall implements IFirewallService, IOFMessageListener, IFloodlig
Ethernet eth = IFloodlightProviderService.bcStore.get(cntx, IFloodlightProviderService.CONTEXT_PI_PAYLOAD);
WildcardsPair wildcards = new WildcardsPair();
Iterator<FirewallRule> iter = this.rules.iterator();
FirewallRule rule = null;
// iterate through list to find a matching firewall rule
while (iter.hasNext()) {
// get next rule from list
rule = iter.next();
// check if rule matches
if (rule.matchesFlow(sw.getId(), pi.getInPort(), eth, wildcards) == true) {
matched_rule = rule;
break;
synchronized (rules) {
Iterator<FirewallRule> iter = this.rules.iterator();
FirewallRule rule = null;
// iterate through list to find a matching firewall rule
while (iter.hasNext()) {
// get next rule from list
rule = iter.next();
// check if rule matches
if (rule.matchesFlow(sw.getId(), pi.getInPort(), eth, wildcards) == true) {
matched_rule = rule;
break;
}
}
}
......
......@@ -215,7 +215,7 @@ public class FirewallTest extends FloodlightTestCase {
firewall.receive(sw, this.packetIn, cntx);
verify(sw);
assertEquals(0, firewall.countRules());
assertEquals(0, firewall.rules.size());
IRoutingDecision decision = IRoutingDecision.rtStore.get(cntx, IRoutingDecision.CONTEXT_DECISION);
// no rules to match, so firewall should deny
......@@ -272,7 +272,7 @@ public class FirewallTest extends FloodlightTestCase {
firewall.receive(sw, this.packetIn, cntx);
verify(sw);
assertEquals(1, firewall.countRules());
assertEquals(1, firewall.rules.size());
IRoutingDecision decision = IRoutingDecision.rtStore.get(cntx, IRoutingDecision.CONTEXT_DECISION);
assertNull(decision);
......@@ -335,7 +335,7 @@ public class FirewallTest extends FloodlightTestCase {
rule.priority = 2;
firewall.addRule(rule);
assertEquals(2, firewall.countRules());
assertEquals(2, firewall.rules.size());
// packet destined to TCP port 80 - should be allowed
......@@ -374,7 +374,7 @@ public class FirewallTest extends FloodlightTestCase {
// broadcast-ARP traffic should be allowed
IRoutingDecision decision = IRoutingDecision.rtStore.get(cntx, IRoutingDecision.CONTEXT_DECISION);
assertEquals(decision.getRoutingAction(), IRoutingDecision.RoutingAction.FORWARD_OR_FLOOD);
assertEquals(IRoutingDecision.RoutingAction.MULTICAST, decision.getRoutingAction());
// clear decision
IRoutingDecision.rtStore.remove(cntx, IRoutingDecision.CONTEXT_DECISION);
......@@ -408,7 +408,7 @@ public class FirewallTest extends FloodlightTestCase {
// broadcast traffic should be allowed
IRoutingDecision decision = IRoutingDecision.rtStore.get(cntx, IRoutingDecision.CONTEXT_DECISION);
assertEquals(decision.getRoutingAction(), IRoutingDecision.RoutingAction.FORWARD_OR_FLOOD);
assertEquals(IRoutingDecision.RoutingAction.MULTICAST, decision.getRoutingAction());
}
@Test
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment