Check loopback and link-local multicast endpoints

Previously we just disallowed link-local (unicast). This disallows loopback and link-local multicast.
lavalamp · Aug 19, 2015 · 86f4535 · 86f4535
1 parent b5a4a54
commit 86f4535
Show file tree

Hide file tree

Showing 7 changed files with 47 additions and 16 deletions.
diff --git a/api/swagger-spec/v1.json b/api/swagger-spec/v1.json
@@ -11230,7 +11230,7 @@
     "properties": {
      "ip": {
       "type": "string",
-      "description": "IP address of the endpoint"
+      "description": "IP address of the endpoint; may not be loopback (127.0.0.0/8), link-local (169.254.0.0/16), or link-local multicast ((224.0.0.0/24)"
      },
      "targetRef": {
       "$ref": "v1.ObjectReference",

diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go
@@ -264,8 +264,9 @@ func (s *APIServer) Run(_ []string) error {
 	s.verifyClusterIPFlags()
 
 	// If advertise-address is not specified, use bind-address. If bind-address
-	// is also unset (or 0.0.0.0), setDefaults() in pkg/master/master.go will
-	// do the right thing and use the host's default interface.
+	// is not usable (unset, 0.0.0.0, or loopback), setDefaults() in
+	// pkg/master/master.go will do the right thing and use the host's default
+	// interface.
 	if s.AdvertiseAddress == nil || s.AdvertiseAddress.IsUnspecified() {
 		s.AdvertiseAddress = s.BindAddress
 	}

diff --git a/docs/user-guide/services.md b/docs/user-guide/services.md
@@ -195,6 +195,9 @@ created. You can manually map the service to your own specific endpoints:
 }
 ```
 
+NOTE: Endpoint IPs may not be loopback (127.0.0.0/8), link-local
+(169.254.0.0/16), or link-local multicast ((224.0.0.0/24).
+
 Accessing a `Service` without a selector works the same as if it had selector.
 The traffic will be routed to endpoints defined by the user (`1.2.3.4:80` in
 this example).

diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go
@@ -1273,7 +1273,7 @@ type EndpointSubset struct {
 type EndpointAddress struct {
 	// The IP of this endpoint.
 	// TODO: This should allow hostname or IP, see #4447.
-	IP string `json:"ip" description:"IP address of the endpoint"`
+	IP string `json:"ip" description:"IP address of the endpoint; may not be loopback (127.0.0.0/8), link-local (169.254.0.0/16), or link-local multicast ((224.0.0.0/24)"`
 
 	// Optional: The kubernetes object related to the entry point.
 	TargetRef *ObjectReference `json:"targetRef,omitempty" description:"reference to object providing the endpoint"`

diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go
@@ -1734,24 +1734,25 @@ func validateEndpointSubsets(subsets []api.EndpointSubset) errs.ValidationErrorL
 	return allErrs
 }
 
-var linkLocalNet *net.IPNet
-
 func validateEndpointAddress(address *api.EndpointAddress) errs.ValidationErrorList {
-	if linkLocalNet == nil {
-		var err error
-		_, linkLocalNet, err = net.ParseCIDR("169.254.0.0/16")
-		if err != nil {
-			glog.Errorf("Failed to parse link-local CIDR: %v", err)
-		}
-	}
-
 	allErrs := errs.ValidationErrorList{}
 	if !util.IsValidIPv4(address.IP) {
 		allErrs = append(allErrs, errs.NewFieldInvalid("ip", address.IP, "invalid IPv4 address"))
+		return allErrs
 	}
-	if linkLocalNet.Contains(net.ParseIP(address.IP)) {
+	// We disallow some IPs as endpoints.  Specifically, loopback addresses are
+	// nonsensical and link-local addresses tend to be used for node-centric
+	// purposes (e.g. metadata service).
+	ip := net.ParseIP(address.IP)
+	if ip.IsLoopback() {
+		allErrs = append(allErrs, errs.NewFieldInvalid("ip", address.IP, "may not be in the loopback range (127.0.0.0/8)"))
+	}
+	if ip.IsLinkLocalUnicast() {
 		allErrs = append(allErrs, errs.NewFieldInvalid("ip", address.IP, "may not be in the link-local range (169.254.0.0/16)"))
 	}
+	if ip.IsLinkLocalMulticast() {
+		allErrs = append(allErrs, errs.NewFieldInvalid("ip", address.IP, "may not be in the link-local multicast range (224.0.0.0/24)"))
+	}
 	return allErrs
 }
 

diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go
@@ -3948,6 +3948,19 @@ func TestValidateEndpoints(t *testing.T) {
 			},
 			errorType: "FieldValueRequired",
 		},
+		"Address is loopback": {
+			endpoints: api.Endpoints{
+				ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"},
+				Subsets: []api.EndpointSubset{
+					{
+						Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}},
+						Ports:     []api.EndpointPort{{Name: "p", Port: 93, Protocol: "TCP"}},
+					},
+				},
+			},
+			errorType:   "FieldValueInvalid",
+			errorDetail: "loopback",
+		},
 		"Address is link-local": {
 			endpoints: api.Endpoints{
 				ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"},
@@ -3961,6 +3974,19 @@ func TestValidateEndpoints(t *testing.T) {
 			errorType:   "FieldValueInvalid",
 			errorDetail: "link-local",
 		},
+		"Address is link-local multicast": {
+			endpoints: api.Endpoints{
+				ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"},
+				Subsets: []api.EndpointSubset{
+					{
+						Addresses: []api.EndpointAddress{{IP: "224.0.0.1"}},
+						Ports:     []api.EndpointPort{{Name: "p", Port: 93, Protocol: "TCP"}},
+					},
+				},
+			},
+			errorType:   "FieldValueInvalid",
+			errorDetail: "link-local multicast",
+		},
 	}
 
 	for k, v := range errorCases {

diff --git a/pkg/master/master.go b/pkg/master/master.go
@@ -286,7 +286,7 @@ func setDefaults(c *Config) {
 	if c.CacheTimeout == 0 {
 		c.CacheTimeout = 5 * time.Second
 	}
-	for c.PublicAddress == nil || c.PublicAddress.IsUnspecified() {
+	for c.PublicAddress == nil || c.PublicAddress.IsUnspecified() || c.PublicAddress.IsLoopback() {
 		// TODO: This should be done in the caller and just require a
 		// valid value to be passed in.
 		hostIP, err := util.ChooseHostInterface()