Repository: Azure/aad-pod-identity
Pull request #842 (2020-10-19 21:58:58)

chore: disable aad-pod-identity by default for kubenet
<!-- Thank you for helping AAD Pod Identity with a pull request! Please make sure you read the [contributing guidelines](https://github.com/Azure/aad-pod-identity/blob/master/CONTRIBUTING.md). -->

**Reason for Change**:
<!-- What does this PR improve or fix in AAD Pod Identity? Why is it needed? -->


**Requirements**

- [x] squashed commits
- [x] included documentation
- [x] added unit tests and e2e tests (if applicable). See [test standard](https://github.com/Azure/aad-pod-identity/blob/master/CONTRIBUTING.md#test-standard) for more details.
- [x] ran `make precommit`

**Issue Fixed**:
<!-- If this PR fixes GitHub issue 1234, add "Fixes #1234" to the next line. -->

**Please answer the following questions with yes/no**:

Does this change contain code from or inspired by another project? If so, did you notify the maintainers and provide attribution?

Some of the docs contain snippets from https://blog.aquasec.com/dns-spoofing-kubernetes-clusters


**Notes for Reviewers**:


codecov-io: # [Codecov](https://codecov.io/gh/Azure/aad-pod-identity/pull/842?src=pr&el=h1) Report
> Merging [#842](https://codecov.io/gh/Azure/aad-pod-identity/pull/842?src=pr&el=desc) into [master](https://codecov.io/gh/Azure/aad-pod-identity/commit/3598f7bf7abc54982f7acebc51e1701b0a296cab?el=desc) will **decrease** coverage by `0.09%`.
> The diff coverage is `60.00%`.


```diff
@@            Coverage Diff             @@
##           master     #842      +/-   ##
==========================================
- Coverage   36.29%   36.19%   -0.10%     
==========================================
  Files          25       25              
  Lines        2838     2843       +5     
==========================================
- Hits         1030     1029       -1     
- Misses       1732     1735       +3     
- Partials       76       79       +3     
```


Patch:
diff --git a/.pipelines/deploy-aks-cluster.yml b/.pipelines/deploy-aks-cluster.yml
index a8b71dd2..4824b50f 100644
--- a/.pipelines/deploy-aks-cluster.yml
+++ b/.pipelines/deploy-aks-cluster.yml
@@ -31,6 +31,7 @@ steps:
           --max-pods ${MAX_PODS} \
           --service-principal $(AZURE_CLIENT_ID) \
           --client-secret $(AZURE_CLIENT_SECRET) \
+          --network-plugin azure \
           --generate-ssh-keys > /dev/null
       fi
 
diff --git a/cmd/nmi/main.go b/cmd/nmi/main.go
index 3a1dbb57..4bf53dd5 100644
--- a/cmd/nmi/main.go
+++ b/cmd/nmi/main.go
@@ -12,6 +12,7 @@ import (
 	"github.com/Azure/aad-pod-identity/pkg/nmi"
 	server "github.com/Azure/aad-pod-identity/pkg/nmi/server"
 	"github.com/Azure/aad-pod-identity/pkg/probes"
+	"github.com/Azure/aad-pod-identity/pkg/utils"
 	"github.com/Azure/aad-pod-identity/version"
 
 	"github.com/spf13/pflag"
@@ -47,6 +48,8 @@ var (
 	metadataHeaderRequired             = pflag.Bool("metadata-header-required", false, "Metadata header required for querying Azure Instance Metadata service")
 	prometheusPort                     = pflag.String("prometheus-port", "9090", "Prometheus port for metrics")
 	operationMode                      = pflag.String("operation-mode", "standard", "NMI operation mode")
+	allowNetworkPluginKubenet          = pflag.Bool("allow-network-plugin-kubenet", false, "Allow running aad-pod-identity in cluster with kubenet")
+	kubeletConfig                      = pflag.String("kubelet-config", "/etc/default/kubelet", "Path to kubelet default config")
 )
 
 func main() {
@@ -69,6 +72,15 @@ func main() {
 		version.PrintVersionAndExit()
 	}
 
+	// check if the cni is kubenet from the --network-plugin defined in kubelet config
+	isKubenet, err := utils.IsKubenetCNI(*kubeletConfig)
+	if err != nil {
+		klog.Fatalf("failed to check if CNI plugin is kubenet, error: %+v", err)
+	}
+	if !*allowNetworkPluginKubenet && isKubenet {
+		klog.Fatalf("AAD Pod Identity is not supported for Kubenet. Review https://azure.github.io/aad-pod-identity/docs/configure/aad_pod_identity_on_kubenet/ for more details.")
+	}
+
 	klog.Infof("starting nmi process. Version: %v. Build date: %v.", version.NMIVersion, version.BuildDate)
 
 	if *enableProfile {
diff --git a/manifest_staging/charts/aad-pod-identity/README.md b/manifest_staging/charts/aad-pod-identity/README.md
index aa2bb2b8..66169990 100755
--- a/manifest_staging/charts/aad-pod-identity/README.md
+++ b/manifest_staging/charts/aad-pod-identity/README.md
@@ -238,6 +238,7 @@ The following tables list the configurable parameters of the aad-pod-identity ch
 | `nmi.retryAttemptsForCreated`             | Override number of retries in NMI to find assigned identity in CREATED state                                                                                                                                                                                                                                                  | If not provided, default is  `16`                              |
 | `nmi.retryAttemptsForAssigned`            | Override number of retries in NMI to find assigned identity in ASSIGNED state                                                                                                                                                                                                                                                 | If not provided, default is  `4`                               |
 | `nmi.findIdentityRetryIntervalInSeconds`  | Override retry interval to find assigned identities in seconds                                                                                                                                                                                                                                                                | If not provided, default is  `5`                               |
+| `nmi.allowNetworkPluginKubenet`           | Allow running aad-pod-identity in cluster with kubenet                                                                                                                                                                                                                                                                        | `false`                                                        |
 | `rbac.enabled`                            | Create and use RBAC for all aad-pod-identity resources                                                                                                                                                                                                                                                                        | `true`                                                         |
 | `rbac.allowAccessToSecrets`               | NMI requires permissions to get secrets when service principal (type: 1) is used in AzureIdentity. If using only MSI (type: 0) in AzureIdentity, secret get permission can be disabled by setting this to false.                                                                                                              | `true`                                                         |
 | `azureIdentities`                         | List of azure identities and azure identity bindings resources to create                                                                                                                                                                                                                                                      | `[]`                                                           |
diff --git a/manifest_staging/charts/aad-pod-identity/templates/nmi-daemonset.yaml b/manifest_staging/charts/aad-pod-identity/templates/nmi-daemonset.yaml
index 716072db..554e1952 100644
--- a/manifest_staging/charts/aad-pod-identity/templates/nmi-daemonset.yaml
+++ b/manifest_staging/charts/aad-pod-identity/templates/nmi-daemonset.yaml
@@ -42,6 +42,9 @@ spec:
           path: /run/xtables.lock
           type: FileOrCreate
         name: iptableslock
+      - name: kubelet-config
+        hostPath:
+          path: /etc/default/kubelet
       containers:
       - name: nmi
         image: "{{ .Values.image.repository }}/{{ .Values.nmi.image }}:{{ .Values.nmi.tag }}"
@@ -93,6 +96,9 @@ spec:
           {{- if eq .Values.operationMode "managed" }}
           - --forceNamespaced
           {{- end }}
+          {{- if .Values.nmi.allowNetworkPluginKubenet }}
+          - --allow-network-plugin-kubenet={{ .Values.nmi.allowNetworkPluginKubenet }}
+          {{- end }}
         env:
           {{- if semverCompare "<= 1.6.1-0" .Values.nmi.tag }}
           - name: HOST_IP
@@ -122,6 +128,9 @@ spec:
         volumeMounts:
         - mountPath: /run/xtables.lock
           name: iptableslock
+        - name: kubelet-config
+          mountPath: /etc/default/kubelet
+          readOnly: true
         livenessProbe:
           httpGet:
             path: /healthz
diff --git a/manifest_staging/charts/aad-pod-identity/values.yaml b/manifest_staging/charts/aad-pod-identity/values.yaml
index ac713d8d..9d37657d 100644
--- a/manifest_staging/charts/aad-pod-identity/values.yaml
+++ b/manifest_staging/charts/aad-pod-identity/values.yaml
@@ -184,6 +184,10 @@ nmi:
   # default is false
   metadataHeaderRequired: ""
 
+  # enable running aad-pod-identity on clusters with kubenet
+  # default is false
+  allowNetworkPluginKubenet: false
+
 rbac:
   enabled: true
   # NMI requires permissions to get secrets when service principal (type: 1) is used in AzureIdentity.
diff --git a/manifest_staging/deploy/infra/deployment-rbac.yaml b/manifest_staging/deploy/infra/deployment-rbac.yaml
index 73ee957c..c967d419 100644
--- a/manifest_staging/deploy/infra/deployment-rbac.yaml
+++ b/manifest_staging/deploy/infra/deployment-rbac.yaml
@@ -120,6 +120,9 @@ spec:
           path: /run/xtables.lock
           type: FileOrCreate
         name: iptableslock
+      - name: kubelet-config
+        hostPath:
+          path: /etc/default/kubelet
       containers:
       - name: nmi
         image: "mcr.microsoft.com/oss/azure/aad-pod-identity/nmi:v1.6.3"
@@ -151,6 +154,9 @@ spec:
         volumeMounts:
         - mountPath: /run/xtables.lock
           name: iptableslock
+        - name: kubelet-config
+          mountPath: /etc/default/kubelet
+          readOnly: true
         livenessProbe:
           httpGet:
             path: /healthz
diff --git a/manifest_staging/deploy/infra/deployment.yaml b/manifest_staging/deploy/infra/deployment.yaml
index 136c05eb..07619190 100644
--- a/manifest_staging/deploy/infra/deployment.yaml
+++ b/manifest_staging/deploy/infra/deployment.yaml
@@ -76,6 +76,9 @@ spec:
           path: /run/xtables.lock
           type: FileOrCreate
         name: iptableslock
+      - name: kubelet-config
+        hostPath:
+          path: /etc/default/kubelet
       containers:
       - name: nmi
         image: "mcr.microsoft.com/oss/azure/aad-pod-identity/nmi:v1.6.3"
@@ -107,6 +110,9 @@ spec:
         volumeMounts:
         - mountPath: /run/xtables.lock
           name: iptableslock
+        - name: kubelet-config
+          mountPath: /etc/default/kubelet
+          readOnly: true
         livenessProbe:
           httpGet:
             path: /healthz
diff --git a/manifest_staging/deploy/infra/managed-mode-deployment.yaml b/manifest_staging/deploy/infra/managed-mode-deployment.yaml
index a862e4c3..39d95a75 100644
--- a/manifest_staging/deploy/infra/managed-mode-deployment.yaml
+++ b/manifest_staging/deploy/infra/managed-mode-deployment.yaml
@@ -105,6 +105,9 @@ spec:
           path: /run/xtables.lock
           type: FileOrCreate
         name: iptableslock
+      - name: kubelet-config
+        hostPath:
+          path: /etc/default/kubelet
       containers:
       - name: nmi
         image: "mcr.microsoft.com/oss/azure/aad-pod-identity/nmi:v1.6.3"
@@ -138,6 +141,9 @@ spec:
         volumeMounts:
         - mountPath: /run/xtables.lock
           name: iptableslock
+        - name: kubelet-config
+          mountPath: /etc/default/kubelet
+          readOnly: true
         livenessProbe:
           httpGet:
             path: /healthz
diff --git a/manifest_staging/deploy/infra/noazurejson/deployment-rbac.yaml b/manifest_staging/deploy/infra/noazurejson/deployment-rbac.yaml
index 24c50929..06cf1f38 100644
--- a/manifest_staging/deploy/infra/noazurejson/deployment-rbac.yaml
+++ b/manifest_staging/deploy/infra/noazurejson/deployment-rbac.yaml
@@ -118,6 +118,9 @@ spec:
           path: /run/xtables.lock
           type: FileOrCreate
         name: iptableslock
+      - name: kubelet-config
+        hostPath:
+          path: /etc/default/kubelet
       containers:
       - name: nmi
         image: "mcr.microsoft.com/oss/azure/aad-pod-identity/nmi:v1.6.3"
@@ -149,6 +152,9 @@ spec:
         volumeMounts:
         - mountPath: /run/xtables.lock
           name: iptableslock
+        - name: kubelet-config
+          mountPath: /etc/default/kubelet
+          readOnly: true
         livenessProbe:
           httpGet:
             path: /healthz
diff --git a/manifest_staging/deploy/infra/noazurejson/deployment.yaml b/manifest_staging/deploy/infra/noazurejson/deployment.yaml
index 46d480e1..c2fdbeae 100644
--- a/manifest_staging/deploy/infra/noazurejson/deployment.yaml
+++ b/manifest_staging/deploy/infra/noazurejson/deployment.yaml
@@ -74,6 +74,9 @@ spec:
           path: /run/xtables.lock
           type: FileOrCreate
         name: iptableslock
+      - name: kubelet-config
+        hostPath:
+          path: /etc/default/kubelet
       containers:
       - name: nmi
         image: "mcr.microsoft.com/oss/azure/aad-pod-identity/nmi:v1.6.3"
@@ -105,6 +108,9 @@ spec:
         volumeMounts:
         - mountPath: /run/xtables.lock
           name: iptableslock
+        - name: kubelet-config
+          mountPath: /etc/default/kubelet
+          readOnly: true
         livenessProbe:
           httpGet:
             path: /healthz
diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go
index bae9284e..150809ea 100644
--- a/pkg/utils/utils.go
+++ b/pkg/utils/utils.go
@@ -2,7 +2,9 @@ package utils
 
 import (
 	"fmt"
+	"io/ioutil"
 	"regexp"
+	"strings"
 )
 
 // RedactClientID redacts client id
@@ -24,3 +26,13 @@ func ValidateResourceID(resourceID string) error {
 	}
 	return nil
 }
+
+func IsKubenetCNI(kubeletConfig string) (bool, error) {
+	var isKubenet bool
+	// read the kubelet config
+	bytes, err := ioutil.ReadFile(kubeletConfig)
+	if err != nil {
+		return isKubenet, err
+	}
+	return strings.Contains(string(bytes), "--network-plugin=kubenet"), nil
+}
diff --git a/website/content/en/docs/Configure/aad_pod_identity_on_kubenet.md b/website/content/en/docs/Configure/aad_pod_identity_on_kubenet.md
new file mode 100644
index 00000000..5e5d79f4
--- /dev/null
+++ b/website/content/en/docs/Configure/aad_pod_identity_on_kubenet.md
@@ -0,0 +1,37 @@
+---
+title: "Deploy AAD Pod Identity in a Cluster with Kubenet"
+linkTitle: "Deploy AAD Pod Identity in a Cluster with Kubenet"
+weight: 2
+description: >
+  AAD Pod Identity is disabled by default on Clusters with Kubenet starting from release v1.7. 
+---
+
+> Starting from 1.7 release
+
+## Introduction
+
+AAD Pod Identity is disabled by default on clusters with Kubenet network plugin. The NMI pods will fail to run with error `AAD Pod Identity is not supported for Kubenet`.
+
+## Why this change?
+
+Kubenet network plugin is susceptible to ARP spoofing. This makes it possible for pods to impersonate as a pod with access to an identity. Using `CAP_NET_RAW` capability the attacker pod could then request token as a pod it's impersonating.
+
+Network plugins like Azure CNI, Calico, Cilium prevents ARP Spoofing.
+
+## Mitigation steps to take before running clusters with Kubenet
+
+The recommended steps to take before configuring AAD Pod Identity to run on clusters with Kubenet network plugin
+
+- Add a [`securityContext`](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/) that drops the `NET_RAW` capability by default in your applications.
+    ```yaml
+    securityContext:
+      capabilities:
+        drop:
+        - NET_RAW
+    ```
+  
+  This shouldn’t affect most applications, since it's only needed for applications that do deep networking inspection/manipulation. Dropping this capability will make sure even if your application code got compromised, the attacker could not perform such network-based attacks on your cluster.
+ 
+## How to run AAD Pod Identity on clusters with Kubenet
+
+Set the `--allow-network-plugin-kubenet=true` arg in the NMI container to continue running on clusters with Kubenet.
\ No newline at end of file


Test patch:
diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go
index 22172687..f0243274 100644
--- a/pkg/utils/utils_test.go
+++ b/pkg/utils/utils_test.go
@@ -1,6 +1,8 @@
 package utils
 
 import (
+	"io/ioutil"
+	"os"
 	"testing"
 )
 
@@ -60,3 +62,53 @@ func TestIsValidResourceID(t *testing.T) {
 		})
 	}
 }
+
+func TestIsKubenetCNI(t *testing.T) {
+	tests := []struct {
+		name                 string
+		kubeletConfig        string
+		expectedIsKubenetCNI bool
+	}{
+		{
+			name: "network plugin cni",
+			kubeletConfig: `KUBELET_FLAGS=--address=0.0.0.0 --anonymous-auth=false --authentication-token-webhook=true --authorization-mode=Webhook --azure-container-registry-config=/etc/kubernetes/azure.json --cgroups-per-qos=true --client-ca-file=/etc/kubernetes/certs/ca.crt --cloud-config=/etc/kubernetes/azure.json --cloud-provider=azure --cluster-dns=10.0.0.10 --cluster-domain=cluster.local --dynamic-config-dir=/var/lib/kubelet --enforce-node-allocatable=pods --event-qps=0 --eviction-hard=memory.available<750Mi,nodefs.available<10%,nodefs.inodesFree<5% --feature-gates=RotateKubeletServerCertificate=true --image-gc-high-threshold=85 --image-gc-low-threshold=80 --image-pull-progress-deadline=30m --keep-terminated-pod-volumes=false --kube-reserved=cpu=100m,memory=1638Mi --kubeconfig=/var/lib/kubelet/kubeconfig --max-pods=110 --network-plugin=cni --node-status-update-frequency=10s --non-masquerade-cidr=0.0.0.0/0 --pod-infra-container-image=mcr.microsoft.com/oss/kubernetes/pause:1.3.1 --pod-manifest-path=/etc/kubernetes/manifests --pod-max-pids=-1 --protect-kernel-defaults=true --read-only-port=0 --rotate-certificates=false --streaming-connection-idle-timeout=4h --tls-cert-file=/etc/kubernetes/certs/kubeletserver.crt --tls-cipher-suites=TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_RSA_WITH_AES_256_GCM_SHA384,TLS_RSA_WITH_AES_128_GCM_SHA256 --tls-private-key-file=/etc/kubernetes/certs/kubeletserver.key
+KUBELET_REGISTER_SCHEDULABLE=true
+NETWORK_POLICY=
+
+KUBELET_NODE_LABELS=kubernetes.azure.com/role=agent,agentpool=agentpool,storageprofile=managed,storagetier=Premium_LRS,kubernetes.azure.com/cluster=MC_aks1016_c00_southcentralus,kubernetes.azure.com/mode=system,kubernetes.azure.com/node-image-version=AKSUbuntu-1604-2020.09.30`,
+			expectedIsKubenetCNI: false,
+		},
+		{
+			name: "network plugin kubenet",
+			kubeletConfig: `KUBELET_FLAGS=--address=0.0.0.0 --anonymous-auth=false --authentication-token-webhook=true --authorization-mode=Webhook --azure-container-registry-config=/etc/kubernetes/azure.json --cgroups-per-qos=true --client-ca-file=/etc/kubernetes/certs/ca.crt --cloud-config=/etc/kubernetes/azure.json --cloud-provider=azure --cluster-dns=10.0.0.10 --cluster-domain=cluster.local --dynamic-config-dir=/var/lib/kubelet --enforce-node-allocatable=pods --event-qps=0 --eviction-hard=memory.available<750Mi,nodefs.available<10%,nodefs.inodesFree<5% --feature-gates=RotateKubeletServerCertificate=true --image-gc-high-threshold=85 --image-gc-low-threshold=80 --image-pull-progress-deadline=30m --keep-terminated-pod-volumes=false --kube-reserved=cpu=100m,memory=1638Mi --kubeconfig=/var/lib/kubelet/kubeconfig --max-pods=110 --network-plugin=kubenet --node-status-update-frequency=10s --non-masquerade-cidr=0.0.0.0/0 --pod-infra-container-image=mcr.microsoft.com/oss/kubernetes/pause:1.3.1 --pod-manifest-path=/etc/kubernetes/manifests --pod-max-pids=-1 --protect-kernel-defaults=true --read-only-port=0 --rotate-certificates=false --streaming-connection-idle-timeout=4h --tls-cert-file=/etc/kubernetes/certs/kubeletserver.crt --tls-cipher-suites=TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_RSA_WITH_AES_256_GCM_SHA384,TLS_RSA_WITH_AES_128_GCM_SHA256 --tls-private-key-file=/etc/kubernetes/certs/kubeletserver.key
+KUBELET_REGISTER_SCHEDULABLE=true
+NETWORK_POLICY=
+
+KUBELET_NODE_LABELS=kubernetes.azure.com/role=agent,agentpool=agentpool,storageprofile=managed,storagetier=Premium_LRS,kubernetes.azure.com/cluster=MC_aks1016_c00_southcentralus,kubernetes.azure.com/mode=system,kubernetes.azure.com/node-image-version=AKSUbuntu-1604-2020.09.30`,
+			expectedIsKubenetCNI: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			tmpFile, err := ioutil.TempFile("", "ut")
+			if err != nil {
+				t.Fatalf("expected err to be nil, got: %+v", err)
+			}
+			defer os.Remove(tmpFile.Name())
+
+			_, err = tmpFile.Write([]byte(test.kubeletConfig))
+			if err != nil {
+				t.Fatalf("expected err to be nil, got: %+v", err)
+			}
+
+			isKubenet, err := IsKubenetCNI(tmpFile.Name())
+			if err != nil {
+				t.Fatalf("expected err to be nil, got: %+v", err)
+			}
+			if isKubenet != test.expectedIsKubenetCNI {
+				t.Fatalf("expected kubenet CNI to be %v, got: %v", test.expectedIsKubenetCNI, isKubenet)
+			}
+		})
+	}
+}
diff --git a/test/e2e/cluster_configs/vmas.json b/test/e2e/cluster_configs/vmas.json
index e6328841..530859fb 100644
--- a/test/e2e/cluster_configs/vmas.json
+++ b/test/e2e/cluster_configs/vmas.json
@@ -13,7 +13,8 @@
                 ],
                 "kubeletConfig": {
                     "--max-pods": "${MAX_PODS}"
-                }
+                },
+                "networkPlugin": "azure"
             }
         },
         "masterProfile": {
diff --git a/test/e2e/cluster_configs/vmss.json b/test/e2e/cluster_configs/vmss.json
index 928c01c9..3db237e3 100644
--- a/test/e2e/cluster_configs/vmss.json
+++ b/test/e2e/cluster_configs/vmss.json
@@ -13,7 +13,8 @@
                 ],
                 "kubeletConfig": {
                     "--max-pods": "${MAX_PODS}"
-                }
+                },
+                "networkPlugin": "azure"
             }
         },
         "masterProfile": {