Skip to content

Commit dfd950c

Browse files
raveningandrijapanicsb
ravening
authored and
andrijapanicsb
committed
Add protocol number support for security group rules (apache#3736)
Currently while creating ingress/egress rule for a security group, we can specify only TCP/UDP/ICMP. Sometimes we need to add rules for different protocol number or rules for all the above three mentioned protocols. In this new feature users can specify the protocol number or select "ALL" option which will apply rules for TCP/UDP/ICMP
1 parent f2708d6 commit dfd950c

5 files changed

Lines changed: 608 additions & 22 deletions

File tree

‎api/src/main/java/org/apache/cloudstack/api/command/user/securitygroup/AuthorizeSecurityGroupIngressCmd.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public class AuthorizeSecurityGroupIngressCmd extends BaseAsyncCmd {
5858
// ////////////// API parameters /////////////////////
5959
// ///////////////////////////////////////////////////
6060

61-
@Parameter(name = ApiConstants.PROTOCOL, type = CommandType.STRING, description = "TCP is default. UDP is the other supported protocol")
61+
@Parameter(name = ApiConstants.PROTOCOL, type = CommandType.STRING, description = "the protocol for the ACL rule. Valid values are TCP/UDP/ICMP/ALL or valid protocol number (see /etc/protocols). ALL is default.")
6262
private String protocol;
6363

6464
@Parameter(name = ApiConstants.START_PORT, type = CommandType.INTEGER, description = "start port for this ingress rule")

‎scripts/vm/network/security_group.py‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,7 +1032,18 @@ def add_network_rules(vm_name, vm_id, vm_ip, vm_ip6, signature, seqno, vmMac, ru
10321032
action = "ACCEPT"
10331033
direction = "-s"
10341034

1035-
range = str(start) + ':' + str(end)
1035+
if start == 0 and end == 0:
1036+
dport = ""
1037+
else:
1038+
dport = " --dport " + str(start) + ":" + str(end)
1039+
1040+
if protocol != 'all' and protocol != 'icmp' and protocol != 'tcp' and protocol != 'udp':
1041+
protocol_all = " -p " + protocol
1042+
protocol_state = " "
1043+
else:
1044+
protocol_all = " -p " + protocol + " -m " + protocol
1045+
protocol_state = " -m state --state NEW "
1046+
10361047
if 'icmp' == protocol:
10371048
range = str(start) + '/' + str(end)
10381049
if start == -1:
@@ -1041,16 +1052,16 @@ def add_network_rules(vm_name, vm_id, vm_ip, vm_ip6, signature, seqno, vmMac, ru
10411052
for ip in rule['ipv4']:
10421053
if protocol == 'all':
10431054
execute('iptables -I ' + vmchain + ' -m state --state NEW ' + direction + ' ' + ip + ' -j ' + action)
1044-
elif protocol != 'icmp':
1045-
execute('iptables -I ' + vmchain + ' -p ' + protocol + ' -m ' + protocol + ' --dport ' + range + ' -m state --state NEW ' + direction + ' ' + ip + ' -j ' + action)
1055+
elif protocol == 'icmp':
1056+
execute("iptables -I " + vmchain + " -p icmp --icmp-type " + range + " " + direction + " " + ip + " -j " + action)
10461057
else:
1047-
execute('iptables -I ' + vmchain + ' -p icmp --icmp-type ' + range + ' ' + direction + ' ' + ip + ' -j ' + action)
1058+
execute("iptables -I " + vmchain + protocol_all + dport + protocol_state + direction + " " + ip + " -j "+ action)
10481059

10491060
for ip in rule['ipv6']:
10501061
if protocol == 'all':
10511062
execute('ip6tables -I ' + vmchain + ' -m state --state NEW ' + direction + ' ' + ip + ' -j ' + action)
10521063
elif 'icmp' != protocol:
1053-
execute('ip6tables -I ' + vmchain + ' -p ' + protocol + ' -m ' + protocol + ' --dport ' + range + ' -m state --state NEW ' + direction + ' ' + ip + ' -j ' + action)
1064+
execute("ip6tables -I " + vmchain + protocol_all + dport + protocol_state + direction + " " + ip + " -j "+ action)
10541065
else:
10551066
# ip6tables does not allow '--icmpv6-type any', allowing all ICMPv6 is done by not allowing a specific type
10561067
if range == 'any':

‎server/src/main/java/com/cloud/network/security/SecurityGroupManagerImpl.java‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
5151
import org.apache.cloudstack.utils.identity.ManagementServerNode;
5252
import org.apache.commons.codec.digest.DigestUtils;
53+
import org.apache.commons.lang.StringUtils;
5354
import org.apache.log4j.Logger;
5455

5556
import com.cloud.agent.AgentManager;
@@ -617,10 +618,27 @@ private List<SecurityGroupRuleVO> authorizeSecurityGroupRule(final Long security
617618
}
618619
}
619620

620-
if (!NetUtils.isValidSecurityGroupProto(protocol)) {
621-
throw new InvalidParameterValueException("Invalid protocol " + protocol);
621+
//Validate Protocol
622+
protocol = protocol.trim().toLowerCase();
623+
//Check if protocol is a number
624+
if(StringUtils.isNumeric(protocol)){
625+
int protoNumber = Integer.parseInt(protocol);
626+
// Deal with ICMP(protocol number 1) specially because it need to be paired with icmp type and code
627+
if (protoNumber == 1) {
628+
protocol = "icmp";
629+
icmpCode = -1;
630+
icmpType = -1;
631+
} else if(protoNumber < 0 || protoNumber > 255){
632+
throw new InvalidParameterValueException("Invalid protocol number: " + protoNumber);
633+
}
634+
} else {
635+
//Protocol is not number
636+
//Check for valid protocol strings
637+
if (!NetUtils.isValidSecurityGroupProto(protocol)) {
638+
throw new InvalidParameterValueException("Invalid protocol " + protocol);
639+
}
622640
}
623-
if ("icmp".equalsIgnoreCase(protocol)) {
641+
if (protocol.equals(NetUtils.ICMP_PROTO)) {
624642
if ((icmpType == null) || (icmpCode == null)) {
625643
throw new InvalidParameterValueException("Invalid ICMP type/code specified, icmpType = " + icmpType + ", icmpCode = " + icmpCode);
626644
}
@@ -641,7 +659,7 @@ private List<SecurityGroupRuleVO> authorizeSecurityGroupRule(final Long security
641659
}
642660
startPortOrType = 0;
643661
endPortOrCode = 0;
644-
} else {
662+
} else if (protocol.equals(NetUtils.TCP_PROTO) || protocol.equals(NetUtils.UDP_PROTO)) {
645663
if ((startPort == null) || (endPort == null)) {
646664
throw new InvalidParameterValueException("Invalid port range specified, startPort = " + startPort + ", endPort = " + endPort);
647665
}
@@ -660,10 +678,13 @@ private List<SecurityGroupRuleVO> authorizeSecurityGroupRule(final Long security
660678
}
661679
startPortOrType = startPort;
662680
endPortOrCode = endPort;
681+
} else {
682+
// in 4.6, the start port and end port are ignored in definition of ProtocolAclRule
683+
// see core/src/com/cloud/agent/resource/virtualnetwork/facade/SetNetworkAclConfigItem.java
684+
startPortOrType = 0;
685+
endPortOrCode = 0;
663686
}
664687

665-
protocol = protocol.toLowerCase();
666-
667688
List<SecurityGroupVO> authorizedGroups = new ArrayList<SecurityGroupVO>();
668689
if (groupList != null) {
669690
Collection userGroupCollection = groupList.values();

0 commit comments

Comments
 (0)