github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-30 06:54:01 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #54177 from itowlson/azure-lb-sg-restrict-dest-address

Browse Source 
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Restrict Azure NSG rules to allow external access only to load balancer IP

**What this PR does / why we need it**: On Azure, we create NSG (Network Security Group) rules on the vnet to allow external clients to access services exposed as type LoadBalancer.  At the moment, these rules have a destination of `Any`, which means that they will permit requests on the opened port to any IP within the vnet.  This PR restricts the security rules so that they admit external access only to the load balancer IP.

**Which issue this PR fixes**: None in upstream - reported as https://github.com/Azure/acs-engine/issues/1619 

**Special notes for your reviewer**: None

**Release note**: 

```release-note
Azure NSG rules for services exposed via external load balancer 
now limit the destination IP address to the relevant front end load 
balancer IP.
```
This commit is contained in:
Kubernetes Submit Queue 2017-11-08 18:27:09 -08:00 committed by GitHub
commit 93ef57a11c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 67 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

105
pkg/cloudprovider/providers/azure/azure_loadbalancer.go
View File

@ -134,42 +134,6 @@ func (az *Cloud) EnsureLoadBalancer(clusterName string, service *v1.Service, nod
serviceName := getServiceName(service)
glog.V(5).Infof("ensure(%s): START clusterName=%q lbName=%q", serviceName, clusterName, lbName)
az.operationPollRateLimiter.Accept()
glog.V(10).Infof("SecurityGroupsClient.Get(%q): start", az.SecurityGroupName)
sg, err := az.SecurityGroupsClient.Get(az.ResourceGroup, az.SecurityGroupName, "")
glog.V(10).Infof("SecurityGroupsClient.Get(%q): end", az.SecurityGroupName)
if err != nil {
return nil, err
}
sg, sgNeedsUpdate, err := az.reconcileSecurityGroup(sg, clusterName, service, true /* wantLb */)
if err != nil {
return nil, err
}
if sgNeedsUpdate {
glog.V(3).Infof("ensure(%s): sg(%s) - updating", serviceName, *sg.Name)
// azure-sdk-for-go introduced contraint validation which breaks the updating here if we don't set these
// to nil. This is a workaround until https://github.com/Azure/go-autorest/issues/112 is fixed
sg.SecurityGroupPropertiesFormat.NetworkInterfaces = nil
sg.SecurityGroupPropertiesFormat.Subnets = nil
az.operationPollRateLimiter.Accept()
glog.V(10).Infof("SecurityGroupsClient.CreateOrUpdate(%q): start", *sg.Name)
respChan, errChan := az.SecurityGroupsClient.CreateOrUpdate(az.ResourceGroup, *sg.Name, sg, nil)
resp := <-respChan
err := <-errChan
glog.V(10).Infof("SecurityGroupsClient.CreateOrUpdate(%q): end", *sg.Name)
if az.CloudProviderBackoff && shouldRetryAPIRequest(resp.Response, err) {
glog.V(2).Infof("ensure(%s) backing off: sg(%s) - updating", serviceName, *sg.Name)
retryErr := az.CreateOrUpdateSGWithRetry(sg)
if retryErr != nil {
glog.V(2).Infof("ensure(%s) abort backoff: sg(%s) - updating", serviceName, *sg.Name)
return nil, retryErr
}
}
if err != nil {
return nil, err
}
}
lb, existsLb, err := az.getAzureLoadBalancer(lbName)
if err != nil {
return nil, err
@ -257,6 +221,50 @@ func (az *Cloud) EnsureLoadBalancer(clusterName string, service *v1.Service, nod
}
}
var lbStatus *v1.LoadBalancerStatus
if lbIP == nil {
lbStatus, exists, err := az.GetLoadBalancer(clusterName, service)
if err != nil {
return nil, err
}
if !exists {
return nil, fmt.Errorf("ensure(%s): lb(%s) - failed to get back load balancer", serviceName, lbName)
}
lbIP = &lbStatus.Ingress[0].IP
}
az.operationPollRateLimiter.Accept()
glog.V(10).Infof("SecurityGroupsClient.Get(%q): start", az.SecurityGroupName)
sg, err := az.SecurityGroupsClient.Get(az.ResourceGroup, az.SecurityGroupName, "")
glog.V(10).Infof("SecurityGroupsClient.Get(%q): end", az.SecurityGroupName)
if err != nil {
return nil, err
}
sg, sgNeedsUpdate, err := az.reconcileSecurityGroup(sg, clusterName, service, lbIP, true /* wantLb */)
if err != nil {
return nil, err
}
if sgNeedsUpdate {
glog.V(3).Infof("ensure(%s): sg(%s) - updating", serviceName, *sg.Name)
az.operationPollRateLimiter.Accept()
glog.V(10).Infof("SecurityGroupsClient.CreateOrUpdate(%q): start", *sg.Name)
respChan, errChan := az.SecurityGroupsClient.CreateOrUpdate(az.ResourceGroup, *sg.Name, sg, nil)
resp := <-respChan
err := <-errChan
glog.V(10).Infof("SecurityGroupsClient.CreateOrUpdate(%q): end", *sg.Name)
if az.CloudProviderBackoff && shouldRetryAPIRequest(resp.Response, err) {
glog.V(2).Infof("ensure(%s) backing off: sg(%s) - updating", serviceName, *sg.Name)
retryErr := az.CreateOrUpdateSGWithRetry(sg)
if retryErr != nil {
glog.V(2).Infof("ensure(%s) abort backoff: sg(%s) - updating", serviceName, *sg.Name)
return nil, retryErr
}
}
if err != nil {
return nil, err
}
}
// Add the machines to the backend pool if they're not already
lbBackendName := getBackendPoolName(clusterName)
lbBackendPoolID := az.getBackendPoolID(lbName, lbBackendName)
@ -280,6 +288,10 @@ func (az *Cloud) EnsureLoadBalancer(clusterName string, service *v1.Service, nod
glog.V(2).Infof("ensure(%s): lb(%s) finished", serviceName, lbName)
if lbStatus != nil {
return lbStatus, nil
}
if lbIP == nil {
lbStatus, exists, err := az.GetLoadBalancer(clusterName, service)
if err != nil {
@ -325,16 +337,12 @@ func (az *Cloud) EnsureLoadBalancerDeleted(clusterName string, service *v1.Servi
return err
}
if existsSg {
reconciledSg, sgNeedsUpdate, reconcileErr := az.reconcileSecurityGroup(sg, clusterName, service, false /* wantLb */)
reconciledSg, sgNeedsUpdate, reconcileErr := az.reconcileSecurityGroup(sg, clusterName, service, nil, false /* wantLb */)
if reconcileErr != nil {
return reconcileErr
}
if sgNeedsUpdate {
glog.V(3).Infof("delete(%s): sg(%s) - updating", serviceName, az.SecurityGroupName)
// azure-sdk-for-go introduced contraint validation which breaks the updating here if we don't set these
// to nil. This is a workaround until https://github.com/Azure/go-autorest/issues/112 is fixed
sg.SecurityGroupPropertiesFormat.NetworkInterfaces = nil
sg.SecurityGroupPropertiesFormat.Subnets = nil
az.operationPollRateLimiter.Accept()
glog.V(10).Infof("SecurityGroupsClient.CreateOrUpdate(%q): start", *reconciledSg.Name)
respChan, errChan := az.SecurityGroupsClient.CreateOrUpdate(az.ResourceGroup, *reconciledSg.Name, reconciledSg, nil)
@ -784,7 +792,7 @@ func (az *Cloud) reconcileLoadBalancer(lb network.LoadBalancer, fipConfiguration
// This reconciles the Network Security Group similar to how the LB is reconciled.
// This entails adding required, missing SecurityRules and removing stale rules.
func (az *Cloud) reconcileSecurityGroup(sg network.SecurityGroup, clusterName string, service *v1.Service, wantLb bool) (network.SecurityGroup, bool, error) {
func (az *Cloud) reconcileSecurityGroup(sg network.SecurityGroup, clusterName string, service *v1.Service, lbIP *string, wantLb bool) (network.SecurityGroup, bool, error) {
serviceName := getServiceName(service)
var ports []v1.ServicePort
if wantLb {
@ -793,6 +801,17 @@ func (az *Cloud) reconcileSecurityGroup(sg network.SecurityGroup, clusterName st
ports = []v1.ServicePort{}
}
destinationIPAddress := ""
if wantLb {
if lbIP == nil {
return sg, false, fmt.Errorf("No load balancer IP for setting up security rules for service %s", service.Name)
}
destinationIPAddress = *lbIP
}
if destinationIPAddress == "" {
destinationIPAddress = "*"
}
sourceRanges, err := serviceapi.GetLoadBalancerSourceRanges(service)
if err != nil {
return sg, false, err
@ -824,7 +843,7 @@ func (az *Cloud) reconcileSecurityGroup(sg network.SecurityGroup, clusterName st
SourcePortRange: to.StringPtr("*"),
DestinationPortRange: to.StringPtr(strconv.Itoa(int(port.Port))),
SourceAddressPrefix: to.StringPtr(sourceAddressPrefixes[j]),
DestinationAddressPrefix: to.StringPtr("*"),
DestinationAddressPrefix: to.StringPtr(destinationIPAddress),
Access: network.SecurityRuleAccessAllow,
Direction: network.SecurityRuleDirectionInbound,
},

10
pkg/cloudprovider/providers/azure/azure_test.go
View File

@ -299,7 +299,7 @@ func TestReconcileSecurityGroupNewServiceAddsPort(t *testing.T) {
sg := getTestSecurityGroup()
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &svc1, true)
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &svc1, to.StringPtr("192.168.0.0"), true)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
@ -313,7 +313,7 @@ func TestReconcileSecurityGroupNewInternalServiceAddsPort(t *testing.T) {
sg := getTestSecurityGroup()
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &svc1, true)
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &svc1, to.StringPtr("192.168.0.0"), true)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
@ -329,7 +329,7 @@ func TestReconcileSecurityGroupRemoveService(t *testing.T) {
validateSecurityGroup(t, sg, service1, service2)
az := getTestCloud()
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &service1, false)
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &service1, nil, false)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
@ -344,7 +344,7 @@ func TestReconcileSecurityGroupRemoveServiceRemovesPort(t *testing.T) {
sg := getTestSecurityGroup(svc)
svcUpdated := getTestService("servicea", v1.ProtocolTCP, 80)
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &svcUpdated, true)
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &svcUpdated, to.StringPtr("192.168.0.0"), true)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
@ -361,7 +361,7 @@ func TestReconcileSecurityWithSourceRanges(t *testing.T) {
}
sg := getTestSecurityGroup(svc)
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &svc, true)
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &svc, to.StringPtr("192.168.0.0"), true)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}