Fix: only allow to install security group rules from master node
Change-Id: Iee1fb85417872dc7f6a88e33ca994277a9ede048
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSecurityGroupHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSecurityGroupHandler.java
index 2308015..11b22fd 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSecurityGroupHandler.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSecurityGroupHandler.java
@@ -119,6 +119,8 @@
private static final boolean USE_SECURITY_GROUP = false;
+ private static final int VM_IP_PREFIX = 32;
+
@Property(name = "useSecurityGroup", boolValue = USE_SECURITY_GROUP,
label = "Apply OpenStack security group rule for VM traffic")
private boolean useSecurityGroup = USE_SECURITY_GROUP;
@@ -357,28 +359,32 @@
return;
}
- selectors.forEach(selector -> {
- osFlowRuleService.setRule(appId,
- instPort.deviceId(),
- selector,
- DefaultTrafficTreatment.builder().transition(JUMP_TABLE).build(),
- PRIORITY_ACL_RULE,
- ACL_TABLE,
- install);
- });
+ selectors.forEach(selector ->
+ osFlowRuleService.setRule(appId,
+ instPort.deviceId(),
+ selector,
+ DefaultTrafficTreatment.builder().transition(JUMP_TABLE).build(),
+ PRIORITY_ACL_RULE,
+ ACL_TABLE,
+ install));
}
/**
* Sets connection tracking rule using OVS extension commands.
- * It is not so graceful, but I don't want to make it more general because it is going to be used
- * only here. The following is the usage of the function.
+ * It is not so graceful, but I don't want to make it more general because
+ * it is going to be used only here.
+ * The following is the usage of the function.
*
* @param deviceId Device ID
- * @param ctState ctState: please use RulePopulatorUtil.computeCtStateFlag() to build the value
- * @param ctMask crMask: please use RulePopulatorUtil.computeCtMaskFlag() to build the value
+ * @param ctState ctState: please use RulePopulatorUtil.computeCtStateFlag()
+ * to build the value
+ * @param ctMask crMask: please use RulePopulatorUtil.computeCtMaskFlag()
+ * to build the value
* @param commit CT_COMMIT for commit action, CT_NO_COMMIT otherwise
- * @param recircTable table number for recirculation after CT actions. CT_NO_RECIRC with no recirculation
- * @param action Additional actions. ACTION_DROP, ACTION_NONE, GOTO_XXX_TABLE are supported.
+ * @param recircTable table number for recirculation after CT actions.
+ * CT_NO_RECIRC with no recirculation
+ * @param action Additional actions. ACTION_DROP, ACTION_NONE,
+ * GOTO_XXX_TABLE are supported.
* @param priority priority value for the rule
* @param install true for insertion, false for removal
*/
@@ -443,7 +449,8 @@
* @param sgId security group id
* @return set of ip addresses
*/
- private Set<InstancePort> getRemoteInstPorts(String tenantId, String sgId, boolean install) {
+ private Set<InstancePort> getRemoteInstPorts(String tenantId,
+ String sgId, boolean install) {
Set<InstancePort> remoteInstPorts;
Set<Port> removedPorts = Sets.newConcurrentHashSet();
@@ -466,7 +473,7 @@
Ip4Address vmIp,
IpPrefix remoteIp,
Port port) {
- if (remoteIp != null && remoteIp.equals(IpPrefix.valueOf(vmIp, 32))) {
+ if (remoteIp != null && remoteIp.equals(IpPrefix.valueOf(vmIp, VM_IP_PREFIX))) {
// do nothing if the remote IP is my IP
return null;
}
@@ -481,25 +488,24 @@
Map<TpPort, TpPort> portRangeMatchMap =
buildPortRangeMatches(sgRule.getPortRangeMin(),
sgRule.getPortRangeMax());
- portRangeMatchMap.entrySet().forEach(entry -> {
+ portRangeMatchMap.forEach((key, value) -> {
- if (sgRule.getProtocol().toUpperCase().equals(PROTO_TCP)) {
- if (sgRule.getDirection().toUpperCase().equals(EGRESS)) {
- sBuilder.matchTcpSrcMasked(entry.getKey(), entry.getValue());
- } else {
- sBuilder.matchTcpDstMasked(entry.getKey(), entry.getValue());
- }
- } else if (sgRule.getProtocol().toUpperCase().equals(PROTO_UDP)) {
- if (sgRule.getDirection().toUpperCase().equals(EGRESS)) {
- sBuilder.matchUdpSrcMasked(entry.getKey(), entry.getValue());
- } else {
- sBuilder.matchUdpDstMasked(entry.getKey(), entry.getValue());
- }
- }
-
- selectorSet.add(sBuilder.build());
+ if (sgRule.getProtocol().toUpperCase().equals(PROTO_TCP)) {
+ if (sgRule.getDirection().toUpperCase().equals(EGRESS)) {
+ sBuilder.matchTcpSrcMasked(key, value);
+ } else {
+ sBuilder.matchTcpDstMasked(key, value);
}
- );
+ } else if (sgRule.getProtocol().toUpperCase().equals(PROTO_UDP)) {
+ if (sgRule.getDirection().toUpperCase().equals(EGRESS)) {
+ sBuilder.matchUdpSrcMasked(key, value);
+ } else {
+ sBuilder.matchUdpDstMasked(key, value);
+ }
+ }
+
+ selectorSet.add(sBuilder.build());
+ });
} else {
selectorSet.add(sBuilder.build());
}
@@ -538,9 +544,9 @@
String direction,
Ip4Address vmIp) {
if (direction.toUpperCase().equals(EGRESS)) {
- sBuilder.matchIPSrc(IpPrefix.valueOf(vmIp, 32));
+ sBuilder.matchIPSrc(IpPrefix.valueOf(vmIp, VM_IP_PREFIX));
} else {
- sBuilder.matchIPDst(IpPrefix.valueOf(vmIp, 32));
+ sBuilder.matchIPDst(IpPrefix.valueOf(vmIp, VM_IP_PREFIX));
}
}
@@ -657,21 +663,21 @@
}
private int binLower(String binStr, int bits) {
- String outBin = binStr.substring(0, 16 - bits);
+ StringBuilder outBin = new StringBuilder(binStr.substring(0, 16 - bits));
for (int i = 0; i < bits; i++) {
- outBin += "0";
+ outBin.append("0");
}
- return Integer.parseInt(outBin, 2);
+ return Integer.parseInt(outBin.toString(), 2);
}
private int binHigher(String binStr, int bits) {
- String outBin = binStr.substring(0, 16 - bits);
+ StringBuilder outBin = new StringBuilder(binStr.substring(0, 16 - bits));
for (int i = 0; i < bits; i++) {
- outBin += "1";
+ outBin.append("1");
}
- return Integer.parseInt(outBin, 2);
+ return Integer.parseInt(outBin.toString(), 2);
}
private int testMasks(String binStr, int start, int end) {
@@ -740,11 +746,8 @@
@Override
public boolean isRelevant(InstancePortEvent event) {
- InstancePort instPort = event.subject();
- if (!useSecurityGroup) {
- return false;
- }
- return mastershipService.isLocalMaster(instPort.deviceId());
+ return useSecurityGroup &&
+ mastershipService.isLocalMaster(event.subject().deviceId());
}
@Override
@@ -779,10 +782,9 @@
log.debug("Instance port detected/updated MAC:{} IP:{}",
instPort.macAddress(),
instPort.ipAddress());
- eventExecutor.execute(() -> {
- setSecurityGroupRules(instPort,
- osNetService.port(event.subject().portId()), true);
- });
+ eventExecutor.execute(() ->
+ setSecurityGroupRules(instPort,
+ osNetService.port(event.subject().portId()), true));
}
}
@@ -819,21 +821,27 @@
}
}
- private class InternalOpenstackNetworkListener implements OpenstackNetworkListener {
+ private class InternalOpenstackNetworkListener
+ implements OpenstackNetworkListener {
@Override
public boolean isRelevant(OpenstackNetworkEvent event) {
if (event.port() == null || Strings.isNullOrEmpty(event.port().getId())) {
return false;
}
+
if (event.securityGroupId() == null ||
securityGroupService.securityGroup(event.securityGroupId()) == null) {
return false;
}
- if (instancePortService.instancePort(event.port().getId()) == null) {
+
+ InstancePort instPort = instancePortService.instancePort(event.port().getId());
+
+ if (instPort == null) {
return false;
}
- return useSecurityGroup;
+
+ return useSecurityGroup && mastershipService.isLocalMaster(instPort.deviceId());
}
@Override
@@ -869,7 +877,8 @@
}
}
- private class InternalSecurityGroupListener implements OpenstackSecurityGroupListener {
+ private class InternalSecurityGroupListener
+ implements OpenstackSecurityGroupListener {
@Override
public boolean isRelevant(OpenstackSecurityGroupEvent event) {
@@ -885,20 +894,20 @@
public void event(OpenstackSecurityGroupEvent event) {
switch (event.type()) {
case OPENSTACK_SECURITY_GROUP_RULE_CREATED:
- SecurityGroupRule securityGroupRuleToAdd = event.securityGroupRule();
+ SecurityGroupRule sgRuleToAdd = event.securityGroupRule();
eventExecutor.execute(() -> {
- securityGroupRuleAdded(securityGroupRuleToAdd);
+ securityGroupRuleAdded(sgRuleToAdd);
log.info("Applied new security group rule {} to ports",
- securityGroupRuleToAdd.getId());
+ sgRuleToAdd.getId());
});
break;
case OPENSTACK_SECURITY_GROUP_RULE_REMOVED:
- SecurityGroupRule securityGroupRuleToRemove = event.securityGroupRule();
+ SecurityGroupRule sgRuleToRemove = event.securityGroupRule();
eventExecutor.execute(() -> {
- securityGroupRuleRemoved(securityGroupRuleToRemove);
+ securityGroupRuleRemoved(sgRuleToRemove);
log.info("Removed security group rule {} from ports",
- securityGroupRuleToRemove.getId());
+ sgRuleToRemove.getId());
});
break;
case OPENSTACK_SECURITY_GROUP_REMOVED: