Neutron Security Group Rule: Validate RemoteIPPrefix and EthernetType

testservices/errors.go

@@ -128,6 +128,18 @@ func NewInvalidDirectionSecurityGroupError(direction string) *ServerError {
 	return serverErrorf(400, "Invalid input for direction. Reason: %s is not ingress or egress.", direction)
 }
 
+func NewSecurityGroupRuleInvalidEthernetType(ethernetType string) *ServerError {
+	return serverErrorf(400, "Invalid input for ethertype. Reason: %s is not '', 'IPv4' or 'IPv6'.", ethernetType)
+}
+
+func NewSecurityGroupRuleParameterConflict(param1 string, value1 string, param2 string, value2 string) *ServerError {
+	return serverErrorf(400, "Conflicting value %s %s for %s %s", param1, value1, param2, value2)
+}
+
+func NewSecurityGroupRuleInvalidCIDR(cidr string) *ServerError {
+	return serverErrorf(400, "Invalid CIDR %s given as IP prefix.", cidr)
+}
+
 func NewServerBelongsToGroupError(serverId, groupId string) *ServerError {
 	return serverErrorf(409, "Server %q already belongs to group %s", serverId, groupId)
 }

testservices/neutronmodel/neutronmodel.go

@@ -4,6 +4,7 @@
 package neutronmodel
 
 import (
+	"net"
 	"strconv"
 	"sync"
 
@@ -320,8 +321,24 @@ func (n *NeutronModel) AddSecurityGroupRule(ruleId string, rule neutron.RuleInfo
 	if rule.IPProtocol != "" {
 		newrule.IPProtocol = &rule.IPProtocol
 	}
-	if rule.EthernetType != "" {
-		newrule.EthernetType = rule.EthernetType
+	switch rule.EthernetType {
+		case "":
+			// Neutron assumes IPv4 if no EthernetType is specified
+			newrule.EthernetType = "IPv4"
+		case "IPv4", "IPv6":
+			newrule.EthernetType = rule.EthernetType
+		default:
+			return testservices.NewSecurityGroupRuleInvalidEthernetType(rule.EthernetType)
+	}
+	if (newrule.RemoteIPPrefix != "") {
+		ip, _, err := net.ParseCIDR(newrule.RemoteIPPrefix)
+		if (err != nil) {
+			return testservices.NewSecurityGroupRuleInvalidCIDR(rule.RemoteIPPrefix)
+		}
+		if ((newrule.EthernetType == "IPv4" && ip.To4() == nil) ||
+		    (newrule.EthernetType == "IPv6" && ip.To4() != nil)) {
+			return testservices.NewSecurityGroupRuleParameterConflict("ethertype", newrule.EthernetType, "CIDR", newrule.RemoteIPPrefix)
+		}
 	}
 	if group.TenantId != "" {
 		newrule.TenantId = group.TenantId

testservices/neutronservice/service_http.go

@@ -475,6 +475,12 @@ func (n *Neutron) handleSecurityGroupRules(w http.ResponseWriter, r *http.Reques
 			return err
 		}
 		inrule := req.Rule
+		// set default EthernetType for correct comparison
+		// TODO: we should probably have a neutronmodel func to parse and set default values
+		//       and/or move the duplicate check there
+		if (inrule.EthernetType == "") {
+			inrule.EthernetType = "IPv4"
+		}
 		group, err := n.securityGroup(inrule.ParentGroupId)
 		if err != nil {
 			return err // TODO: should be a 4XX error with details
@@ -485,7 +491,8 @@ func (n *Neutron) handleSecurityGroupRules(w http.ResponseWriter, r *http.Reques
 			if r.IPProtocol != nil && *r.IPProtocol == inrule.IPProtocol &&
 				r.PortRangeMax != nil && *r.PortRangeMax == inrule.PortRangeMax &&
 				r.PortRangeMin != nil && *r.PortRangeMin == inrule.PortRangeMin &&
-				r.RemoteIPPrefix != "" && r.RemoteIPPrefix == inrule.RemoteIPPrefix {
+				r.RemoteIPPrefix != "" && r.RemoteIPPrefix == inrule.RemoteIPPrefix &&
+				r.EthernetType != "" && r.EthernetType == inrule.EthernetType {
 				// TODO: Use a proper helper and sane error type
 				return &errorResponse{
 					http.StatusBadRequest,

testservices/neutronservice/service_http_test.go

@@ -533,6 +533,15 @@ func (s *NeutronHTTPSuite) TestAddSecurityGroupRule(c *gc.C) {
 		IPProtocol:     "tcp",
 		RemoteIPPrefix: "5.4.3.2/1",
 	}
+	riIngress6 := neutron.RuleInfoV2{
+		ParentGroupId:  "1",
+		Direction:      "ingress",
+		PortRangeMax:   22,
+		PortRangeMin:   22,
+		IPProtocol:     "tcp",
+		RemoteIPPrefix: "2001:db8:42::/64",
+		EthernetType:   "IPv6",
+	}
 	rule1 := neutron.SecurityGroupRuleV2{
 		Id:             "1",
 		ParentGroupId:  group1.Id,
@@ -559,6 +568,15 @@ func (s *NeutronHTTPSuite) TestAddSecurityGroupRule(c *gc.C) {
 		PortRangeMin:  &riEgress.PortRangeMin,
 		IPProtocol:    &riEgress.IPProtocol,
 	}
+	rule6 := neutron.SecurityGroupRuleV2{
+		Id:             "5",
+		ParentGroupId:  group1.Id,
+		Direction:      riIngress6.Direction,
+		PortRangeMax:   &riIngress6.PortRangeMax,
+		PortRangeMin:   &riIngress6.PortRangeMin,
+		IPProtocol:     &riIngress6.IPProtocol,
+		RemoteIPPrefix: riIngress6.RemoteIPPrefix,
+	}
 	ok := s.service.hasSecurityGroupRule(group1.Id, rule1.Id)
 	c.Assert(ok, gc.Equals, false)
 	ok = s.service.hasSecurityGroupRule(group2.Id, rule2.Id)
@@ -584,12 +602,15 @@ func (s *NeutronHTTPSuite) TestAddSecurityGroupRule(c *gc.C) {
 	// Attempt to create duplicate rule should fail
 	resp, err = s.jsonRequest("POST", neutron.ApiSecurityGroupRulesV2, req, nil)
 	c.Assert(resp.StatusCode, gc.Equals, http.StatusBadRequest)
-	defer s.service.removeSecurityGroupRule(rule1.Id)
+	err = s.service.removeSecurityGroupRule(rule1.Id)
+	c.Assert(err, gc.IsNil)
 	// Attempt to create rule with all fields but RemoteIPPrefix identical should pass
 	req.Rule = riIngress2
 	resp, err = s.jsonRequest("POST", neutron.ApiSecurityGroupRulesV2, req, nil)
 	c.Assert(err, gc.IsNil)
 	c.Assert(resp.StatusCode, gc.Equals, http.StatusCreated)
+	err = s.service.removeSecurityGroupRule(rule2.Id)
+	c.Assert(err, gc.IsNil)
 	assertJSON(c, resp, &expected)
 	c.Assert(expected.Rule.Id, gc.Equals, rule2.Id)
 	c.Assert(expected.Rule.ParentGroupId, gc.Equals, rule2.ParentGroupId)
@@ -598,16 +619,30 @@ func (s *NeutronHTTPSuite) TestAddSecurityGroupRule(c *gc.C) {
 	c.Assert(*expected.Rule.IPProtocol, gc.Equals, *rule2.IPProtocol)
 	c.Assert(expected.Rule.Direction, gc.Equals, rule2.Direction)
 	c.Assert(expected.Rule.RemoteIPPrefix, gc.Equals, rule2.RemoteIPPrefix)
-	defer s.service.removeSecurityGroupRule(rule2.Id)
 	req.Rule = riEgress
 	resp, err = s.jsonRequest("POST", neutron.ApiSecurityGroupRulesV2, req, nil)
 	c.Assert(err, gc.IsNil)
 	c.Assert(resp.StatusCode, gc.Equals, http.StatusCreated)
+	err = s.service.removeSecurityGroupRule(rule3.Id)
+	c.Assert(err, gc.IsNil)
 	assertJSON(c, resp, &expected)
 	c.Assert(expected.Rule.Id, gc.Equals, rule3.Id)
 	c.Assert(expected.Rule.ParentGroupId, gc.Equals, rule3.ParentGroupId)
-	err = s.service.removeSecurityGroupRule(rule3.Id)
+	// Attempt to create rule with IPv6 RemoteIPPrefix without specifying EthernetType, should fail
+	req.Rule = riIngress6
+	req.Rule.EthernetType = ""
+	resp, err = s.jsonRequest("POST", neutron.ApiSecurityGroupRulesV2, req, nil)
+	c.Assert(resp.StatusCode, gc.Equals, http.StatusBadRequest)
+	// Attempt to create rule with IPv6 RemoteIPPrefix with correct EthernetType, should pass
+	req.Rule = riIngress6
+	resp, err = s.jsonRequest("POST", neutron.ApiSecurityGroupRulesV2, req, nil)
+	c.Assert(err, gc.IsNil)
+	c.Assert(resp.StatusCode, gc.Equals, http.StatusCreated)
+	err = s.service.removeSecurityGroupRule(rule6.Id)
 	c.Assert(err, gc.IsNil)
+	assertJSON(c, resp, &expected)
+	c.Assert(expected.Rule.Id, gc.Equals, rule6.Id)
+	c.Assert(expected.Rule.ParentGroupId, gc.Equals, rule6.ParentGroupId)
 }
 
 func (s *NeutronHTTPSuite) TestDeleteSecurityGroupRule(c *gc.C) {

testservices/neutronservice/service_test.go

@@ -326,6 +326,7 @@ func (s *NeutronSuite) TestAddSecurityGroupRuleUpdatesParent(c *gc.C) {
 		ParentGroupId: group.Id,
 		Direction:     "egress",
 		TenantId:      s.service.TenantId,
+		EthernetType:  "IPv4",
 	}
 	s.ensureNoRule(c, rule)
 	err := s.service.addSecurityGroupRule(rule.Id, ri)