Autofix Network Policy (#155)

* autofix initialized

* add new setfields function

* Fix the object creation

* add ErrorMissingDefaultDenyIngressAndEgressNetworkPolicy

* add tests for functionality check

* add tests for ingress and egress

* Delete profile.out

* fix the root manifest bug

* Merge branch 'autofixnetworkPolicy' of github.com:Shopify/kubeaudit into autofixnetworkPolicy

* refactor code according to suggestions

* refactir code again and removed redundant write function

* add test for both fixed and extra Resources

* add beta2 test for autofix

* get rid of dead code

cmd/autofix.go

@@ -6,6 +6,9 @@ import (
 
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
+	k8sRuntime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/kubernetes/scheme"
 )
 
 // The fix function does not preserve comments (because kubernetes resources do not support comments) so we convert
@@ -18,7 +21,7 @@ func autofix(*cobra.Command, []string) {
 
 	resources, err := getKubeResourcesManifest(rootConfig.manifest)
 
-	fixedResources := fix(resources)
+	fixedResources, extraResources := fix(resources)
 
 	tmpFixedFile, err := ioutil.TempFile("", "kubeaudit_autofix_fixed")
 	if err != nil {
@@ -35,11 +38,14 @@ func autofix(*cobra.Command, []string) {
 		log.Error(err)
 	}
 	defer os.Remove(finalFile.Name())
+	if err != nil {
+		log.Error(err)
+	}
 
 	splitResources, toAppend, err := splitYamlResources(rootConfig.manifest, finalFile.Name())
 
 	for index := range fixedResources {
-		err = writeSingleResourceManifestFile(fixedResources[index], tmpFixedFile.Name())
+		err = WriteToFile(fixedResources[index], tmpFixedFile.Name(), false)
 		if err != nil {
 			log.Error(err)
 		}
@@ -57,6 +63,20 @@ func autofix(*cobra.Command, []string) {
 		}
 		toAppend = true
 	}
+	for index := range extraResources {
+		info, _ := k8sRuntime.SerializerInfoForMediaType(scheme.Codecs.SupportedMediaTypes(), "application/yaml")
+		groupVersion := schema.GroupVersion{Group: extraResources[index].GetObjectKind().GroupVersionKind().Group, Version: extraResources[index].GetObjectKind().GroupVersionKind().Version}
+		encoder := scheme.Codecs.EncoderForVersion(info.Serializer, groupVersion)
+		fixedData, err := k8sRuntime.Encode(encoder, extraResources[index])
+		if err != nil {
+			log.Error(err)
+		}
+		err = writeManifestFile(fixedData, finalFile.Name(), toAppend)
+		if err != nil {
+			log.Error(err)
+		}
+		toAppend = true
+	}
 
 	finalData, err := ioutil.ReadFile(finalFile.Name())
 	if err != nil {

cmd/autofix_test.go

@@ -13,27 +13,108 @@ import (
 func TestFixV1(t *testing.T) {
 	file := "../fixtures/autofix_v1.yml"
 	fileFixed := "../fixtures/autofix-fixed_v1.yml"
+	rootConfig.manifest = file
 	assert := assert.New(t)
 	resources, err := getKubeResourcesManifest(file)
 	assert.Nil(err)
-	fixedResources := fix(resources)
+	fixedResources, _ := fix(resources)
 	correctlyFixedResources, err := getKubeResourcesManifest(fileFixed)
 	assert.Nil(err)
 	assertEqualWorkloads(assert, correctlyFixedResources, fixedResources)
 }
 
+func TestAllResourcesFixV1(t *testing.T) {
+	file := "../fixtures/autofix-all-resources_v1.yml"
+	fileFixedResources := "../fixtures/autofix-fixed_v1.yml"
+	fileExtraResources := "../fixtures/autofix-extra-resources-fixed_v1.yml"
+	rootConfig.manifest = file
+	assert := assert.New(t)
+	resources, err := getKubeResourcesManifest(file)
+	assert.Nil(err)
+	fixedResources, extraResources := fix(resources)
+	correctlyFixedResources, err := getKubeResourcesManifest(fileFixedResources)
+	assert.Nil(err)
+	correctlyFixedExtraResources, err := getKubeResourcesManifest(fileExtraResources)
+	assertEqualWorkloads(assert, correctlyFixedResources, fixedResources)
+	assertEqualWorkloads(assert, correctlyFixedExtraResources, extraResources)
+}
+
+func TestExtraResourcesFixV1(t *testing.T) {
+	file := "../fixtures/autofix-extra-resources_v1.yml"
+	fileFixed := "../fixtures/autofix-extra-resources-fixed_v1.yml"
+	rootConfig.manifest = file
+	assert := assert.New(t)
+	resources, err := getKubeResourcesManifest(file)
+	assert.Nil(err)
+	_, extraResources := fix(resources)
+	correctlyFixedResources, err := getKubeResourcesManifest(fileFixed)
+	assert.Nil(err)
+	assertEqualWorkloads(assert, correctlyFixedResources, extraResources)
+}
+
+func TestExtraResourcesEgressFixV1(t *testing.T) {
+	file := "../fixtures/autofix-extra-resources-egress_v1.yml"
+	fileFixed := "../fixtures/autofix-extra-resources-egress-fixed_v1.yml"
+	rootConfig.manifest = file
+	assert := assert.New(t)
+	resources, err := getKubeResourcesManifest(file)
+	assert.Nil(err)
+	_, extraResources := fix(resources)
+	correctlyFixedResources, err := getKubeResourcesManifest(fileFixed)
+	assert.Nil(err)
+	assertEqualWorkloads(assert, correctlyFixedResources, extraResources)
+
+}
+
+func TestExtraResourcesIngressFixV1(t *testing.T) {
+	file := "../fixtures/autofix-extra-resources-ingress_v1.yml"
+	fileFixed := "../fixtures/autofix-extra-resources-ingress-fixed_v1.yml"
+	rootConfig.manifest = file
+	assert := assert.New(t)
+	resources, err := getKubeResourcesManifest(file)
+	assert.Nil(err)
+	_, extraResources := fix(resources)
+	correctlyFixedResources, err := getKubeResourcesManifest(fileFixed)
+	assert.Nil(err)
+	assertEqualWorkloads(assert, correctlyFixedResources, extraResources)
+}
+
 func TestFixV1Beta1(t *testing.T) {
 	file := "../fixtures/autofix_v1beta1.yml"
 	fileFixed := "../fixtures/autofix-fixed_v1beta1.yml"
 	assert := assert.New(t)
 	resources, err := getKubeResourcesManifest(file)
 	assert.Nil(err)
-	fixedResources := fix(resources)
+	fixedResources, _ := fix(resources)
 	correctlyFixedResources, err := getKubeResourcesManifest(fileFixed)
 	assert.Nil(err)
 	assertEqualWorkloads(assert, correctlyFixedResources, fixedResources)
 }
 
+func TestFixV1Beta2(t *testing.T) {
+	origFilename := "../fixtures/autofix-all-resources_v1.yml"
+	expectedFilename := "../fixtures/autofix-all-resources-fixed_v1.yml"
+	assert := assert.New(t)
+
+	// Copy original yaml to a temp file because autofix modifies the input file
+	tmpFile, err := ioutil.TempFile("", "kubeaudit_autofix_test")
+	tmpFilename := tmpFile.Name()
+	assert.Nil(err)
+	defer os.Remove(tmpFilename)
+	origFile, err := os.Open(origFilename)
+	assert.Nil(err)
+	_, err = io.Copy(tmpFile, origFile)
+	assert.Nil(err)
+	tmpFile.Close()
+	origFile.Close()
+
+	rootConfig.manifest = tmpFilename
+	autofix(nil, nil)
+
+	assert.True(compareTextFiles(expectedFilename, tmpFilename))
+
+}
+
 func TestPreserveComments(t *testing.T) {
 	origFilename := "../fixtures/autofix_v1.yml"
 	expectedFilename := "../fixtures/autofix-fixed_v1.yml"

cmd/autofix_util.go

@@ -13,12 +13,13 @@ func getAuditFunctions() []interface{} {
 	return []interface{}{
 		auditAllowPrivilegeEscalation, auditReadOnlyRootFS, auditRunAsNonRoot,
 		auditAutomountServiceAccountToken, auditPrivileged, auditCapabilities,
-		auditAppArmor, auditSeccomp,
+		auditAppArmor, auditSeccomp, auditNetworkPolicies,
 	}
 }
 
 func fixPotentialSecurityIssue(resource Resource, result Result) Resource {
 	resource = prepareResourceForFix(resource, result)
+
 	for _, occurrence := range result.Occurrences {
 		switch occurrence.id {
 		case ErrorAllowPrivilegeEscalationNil, ErrorAllowPrivilegeEscalationTrue:
@@ -42,6 +43,8 @@ func fixPotentialSecurityIssue(resource Resource, result Result) Resource {
 		case ErrorSeccompAnnotationMissing, ErrorSeccompDeprecated, ErrorSeccompDeprecatedPod, ErrorSeccompDisabled,
 			ErrorSeccompDisabledPod:
 			resource = fixSeccomp(resource)
+		case ErrorMissingDefaultDenyIngressNetworkPolicy, ErrorMissingDefaultDenyEgressNetworkPolicy, ErrorMissingDefaultDenyIngressAndEgressNetworkPolicy:
+			resource = fixNetworkPolicy(resource, occurrence)
 		}
 	}
 	return resource
@@ -78,15 +81,23 @@ func prepareResourceForFix(resource Resource, result Result) Resource {
 	return resource
 }
 
-func fix(resources []Resource) (fixedResources []Resource) {
+func fix(resources []Resource) (fixedResources []Resource, extraResources []Resource) {
 	for _, resource := range resources {
 		if !IsSupportedResourceType(resource) {
 			fixedResources = append(fixedResources, resource)
 			continue
 		}
 		results := mergeAuditFunctions(getAuditFunctions())(resource)
 		for _, result := range results {
-			resource = fixPotentialSecurityIssue(resource, result)
+			if IsNamespaceType(resource) {
+				extraResource := fixPotentialSecurityIssue(resource, result)
+				// If return resource from fixPotentialSecurityIssue is Namespace type then we don't have to add extra resources for it.
+				if !IsNamespaceType(extraResource) {
+					extraResources = append(extraResources, extraResource)
+				}
+			} else {
+				resource = fixPotentialSecurityIssue(resource, result)
+			}
 		}
 		fixedResources = append(fixedResources, resource)
 	}

cmd/cronjob_test.go

@@ -14,7 +14,7 @@ func TestCronjobV1(t *testing.T) {
 	assert := assert.New(t)
 	resources, err := getKubeResourcesManifest(file)
 	assert.Nil(err)
-	fixedResources := fix(resources)
+	fixedResources, _ := fix(resources)
 	correctlyFixedResources, err := getKubeResourcesManifest(fileFixed)
 	assert.Nil(err)
 	assert.Nil(deep.Equal(correctlyFixedResources, fixedResources))

cmd/errors.go

@@ -83,6 +83,8 @@ const (
 	ErrorSeccompDeprecated
 	// InfoImageCorrect occurs when an image tag is correct.
 	InfoImageCorrect
+	// ErrorMissingDefaultDenyIngressAndEgressNetworkPolicy missing a default deny egress and default deny egress NetworkPolicy
+	ErrorMissingDefaultDenyIngressAndEgressNetworkPolicy
 	// ErrorMissingDefaultDenyEgressNetworkPolicy occurs when a namespace is missing a default deny egress NetworkPolicy
 	ErrorMissingDefaultDenyEgressNetworkPolicy
 	// ErrorMissingDefaultDenyEgressNetworkPolicy occurs when a namespace is missing a default deny ingress NetworkPolicy

cmd/k8sruntime_util.go

@@ -4,6 +4,7 @@ import (
 	"io/ioutil"
 	"os"
 
+	networking "k8s.io/api/networking/v1"
 	k8sRuntime "k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -48,6 +49,18 @@ func setContainers(resource Resource, containers []ContainerV1) Resource {
 	return resource
 }
 
+func setNetworkPolicyFields(nsName string, policyList []string) Resource {
+	var np NetworkPolicyV1
+	np.Kind = "NetworkPolicy"
+	np.APIVersion = "networking.k8s.io/v1"
+	np.ObjectMeta.Namespace = nsName
+	np.ObjectMeta.Name = "default-deny"
+	for _, policy := range policyList {
+		np.Spec.PolicyTypes = append(np.Spec.PolicyTypes, networking.PolicyType(policy))
+	}
+	return np.DeepCopyObject()
+}
+
 func disableDSA(resource Resource) Resource {
 	switch t := resource.(type) {
 	case *CronJobV1Beta1:

cmd/networkPolicies.go

@@ -95,23 +95,28 @@ func checkNamespaceNetworkPolicies(netPols *NetworkPolicyListV1, result *Result)
 			}
 		}
 	}
-
-	if !hasDenyAllIngressRule {
+	if !hasDenyAllEgressRule && !hasDenyAllIngressRule {
+		occ := Occurrence{
+			container: "",
+			id:        ErrorMissingDefaultDenyIngressAndEgressNetworkPolicy,
+			kind:      Error,
+			message:   "Namespace is missing a default deny ingress and default deny egress NetworkPolicy",
+		}
+		result.Occurrences = append(result.Occurrences, occ)
+	} else if !hasDenyAllIngressRule {
 		occ := Occurrence{
 			container: "",
 			id:        ErrorMissingDefaultDenyIngressNetworkPolicy,
 			kind:      Error,
-			message:   "Namespace is missing a default deny egress NetworkPolicy",
+			message:   "Namespace is missing a default deny ingress NetworkPolicy",
 		}
 		result.Occurrences = append(result.Occurrences, occ)
-	}
-
-	if !hasDenyAllEgressRule {
+	} else if !hasDenyAllEgressRule {
 		occ := Occurrence{
 			container: "",
 			id:        ErrorMissingDefaultDenyEgressNetworkPolicy,
 			kind:      Error,
-			message:   "Namespace is missing a default deny ingress NetworkPolicy",
+			message:   "Namespace is missing a default deny egress NetworkPolicy",
 		}
 		result.Occurrences = append(result.Occurrences, occ)
 
@@ -142,7 +147,9 @@ func getNetworkPoliciesResources(namespace string) (netPolList *NetworkPolicyLis
 		for _, resource := range resources {
 			switch kubeType := resource.(type) {
 			case *NetworkPolicyV1:
-				netPolList.Items = append(netPolList.Items, *kubeType)
+				if kubeType.ObjectMeta.Namespace == namespace {
+					netPolList.Items = append(netPolList.Items, *kubeType)
+				}
 			}
 		}
 

cmd/networkPolicies_fixes.go

@@ -0,0 +1,16 @@
+package cmd
+
+func fixNetworkPolicy(resource Resource, occurrence Occurrence) Resource {
+	var obj Resource
+	nsName := getNamespaceName(resource)
+	if occurrence.id == ErrorMissingDefaultDenyIngressNetworkPolicy {
+		obj = setNetworkPolicyFields(nsName, []string{"Ingress"})
+	}
+	if occurrence.id == ErrorMissingDefaultDenyEgressNetworkPolicy {
+		obj = setNetworkPolicyFields(nsName, []string{"Egress"})
+	}
+	if occurrence.id == ErrorMissingDefaultDenyIngressAndEgressNetworkPolicy {
+		obj = setNetworkPolicyFields(nsName, []string{"Ingress", "Egress"})
+	}
+	return obj
+}

cmd/networkPolicies_test.go

@@ -3,7 +3,7 @@ package cmd
 import "testing"
 
 func TestNamespaceMissingDefaulDenyNetPol(t *testing.T) {
-	runAuditTest(t, "namespace_missing_default_deny_netpol.yml", auditNetworkPolicies, []int{ErrorMissingDefaultDenyIngressNetworkPolicy, ErrorMissingDefaultDenyEgressNetworkPolicy})
+	runAuditTest(t, "namespace_missing_default_deny_netpol.yml", auditNetworkPolicies, []int{ErrorMissingDefaultDenyIngressAndEgressNetworkPolicy})
 }
 
 func TestNamespaceMissingDefaultDenyEgressNetPol(t *testing.T) {

cmd/test_util.go

@@ -171,6 +171,20 @@ func compareTextFiles(file1, file2 string) bool {
 			return false
 		}
 	}
+	f1stat, err := f1.Stat()
+	if err != nil {
+		return false
+	}
+
+	f2stat, err := f2.Stat()
+	if err != nil {
+		return false
+	}
+
+	if f1stat.Size() != f2stat.Size() {
+		fmt.Printf("File sizes don't match")
+		return false
+	}
 	return true
 }
 

cmd/types.go

@@ -115,3 +115,13 @@ func IsSupportedResourceType(obj Resource) bool {
 		return false
 	}
 }
+
+// IsNamespaceType returns true if obj is of NamespaceV1 type
+func IsNamespaceType(obj Resource) bool {
+	switch obj.(type) {
+	case *NamespaceV1:
+		return true
+	default:
+		return false
+	}
+}

cmd/util.go

@@ -223,14 +223,6 @@ func writeManifestFile(decoded []byte, filename string, toAppend bool) error {
 	return nil
 }
 
-func writeSingleResourceManifestFile(decoded Resource, filename string) error {
-	if err := WriteToFile(decoded, filename, false); err != nil {
-		log.Error(err)
-		return err
-	}
-	return nil
-}
-
 func containerNamesUniq(resource Resource) bool {
 	names := make(map[string]bool)
 	for _, container := range getContainers(resource) {