diff --git a/auditors/apparmor/apparmor.go b/auditors/apparmor/apparmor.go index e794fabe..c8ab3fef 100644 --- a/auditors/apparmor/apparmor.go +++ b/auditors/apparmor/apparmor.go @@ -7,6 +7,7 @@ import ( "github.com/Shopify/kubeaudit" "github.com/Shopify/kubeaudit/pkg/fix" "github.com/Shopify/kubeaudit/pkg/k8s" + "github.com/Shopify/kubeaudit/pkg/override" ) const Name = "apparmor" @@ -14,8 +15,10 @@ const Name = "apparmor" const ( // AppArmorAnnotationMissing occurs when the apparmor annotation is missing AppArmorAnnotationMissing = "AppArmorAnnotationMissing" - // AppArmorDisabled occurs when the apparmor annotation is set to a bad value + // AppArmorDisabled occurs when the apparmor annotation is set to the unconfined value AppArmorDisabled = "AppArmorDisabled" + // AppArmorDisabled occurs when the apparmor annotation is set to a bad value + AppArmorBadValue = "AppArmorBadValue" // AppArmorInvalidAnnotation occurs when the apparmor annotation key refers to a container which doesn't exist. This will // prevent the manifest from being applied to a cluster with AppArmor enabled. AppArmorInvalidAnnotation = "AppArmorInvalidAnnotation" @@ -29,10 +32,14 @@ const ( // The profile specifying the runtime default. ProfileRuntimeDefault = "runtime/default" + // The profile specifying the unconfined profile. + ProfileUnconfined = "unconfined" // The prefix for specifying profiles loaded on the node. ProfileNamePrefix = "localhost/" ) +const OverrideLabel = "allow-disabled-apparmor" + // AppArmor implements Auditable type AppArmor struct{} @@ -46,8 +53,10 @@ func (a *AppArmor) Audit(resource k8s.Resource, _ []k8s.Resource) ([]*kubeaudit. var containerNames []string for _, container := range k8s.GetContainers(resource) { - containerNames = append(containerNames, container.Name) + containerName := container.Name + containerNames = append(containerNames, containerName) auditResult := auditContainer(container, resource) + auditResult = applyDisabledOverride(auditResult, containerName, resource) if auditResult != nil { auditResults = append(auditResults, auditResult) } @@ -80,9 +89,16 @@ func auditContainer(container *k8s.ContainerV1, resource k8s.Resource) *kubeaudi } if isAppArmorDisabled(containerAnnotation, annotations) { + var rule string + if isAppArmorUnconfined(containerAnnotation, annotations) { + rule = AppArmorDisabled + } else { + rule = AppArmorBadValue + } + return &kubeaudit.AuditResult{ Auditor: Name, - Rule: AppArmorDisabled, + Rule: rule, Message: fmt.Sprintf("AppArmor is disabled. The apparmor annotation should be set to '%s' or start with '%s'.", ProfileRuntimeDefault, ProfileNamePrefix), Severity: kubeaudit.Error, Metadata: kubeaudit.Metadata{ @@ -100,6 +116,13 @@ func auditContainer(container *k8s.ContainerV1, resource k8s.Resource) *kubeaudi return nil } +func applyDisabledOverride(auditResult *kubeaudit.AuditResult, containerName string, resource k8s.Resource) *kubeaudit.AuditResult { + if auditResult == nil || auditResult.Rule != AppArmorDisabled { + return auditResult + } + return override.ApplyOverride(auditResult, Name, containerName, resource, OverrideLabel) +} + func auditPodAnnotations(resource k8s.Resource, containerNames []string) []*kubeaudit.AuditResult { var auditResults []*kubeaudit.AuditResult for annotationKey, annotationValue := range k8s.GetAnnotations(resource) { @@ -136,6 +159,11 @@ func isAppArmorDisabled(apparmorAnnotation string, annotations map[string]string return !ok || profileName != ProfileRuntimeDefault && !strings.HasPrefix(profileName, ProfileNamePrefix) } +func isAppArmorUnconfined(apparmorAnnotation string, annotations map[string]string) bool { + profileName, ok := annotations[apparmorAnnotation] + return ok && profileName == ProfileUnconfined +} + func getContainerAnnotation(container *k8s.ContainerV1) string { return ContainerAnnotationKeyPrefix + container.Name } diff --git a/auditors/apparmor/apparmor_test.go b/auditors/apparmor/apparmor_test.go index a59d47b8..b02e9791 100644 --- a/auditors/apparmor/apparmor_test.go +++ b/auditors/apparmor/apparmor_test.go @@ -6,6 +6,7 @@ import ( "github.com/Shopify/kubeaudit/internal/test" "github.com/Shopify/kubeaudit/pkg/k8s" + "github.com/Shopify/kubeaudit/pkg/override" "github.com/stretchr/testify/assert" ) @@ -21,8 +22,12 @@ func TestAuditAppArmor(t *testing.T) { {"apparmor-annotation-missing.yml", []string{AppArmorAnnotationMissing}, true}, {"apparmor-annotation-init-container-enabled.yml", nil, true}, {"apparmor-annotation-init-container-missing.yml", []string{AppArmorAnnotationMissing}, true}, + {"apparmor-disabled.yml", []string{AppArmorDisabled}, true}, + {"apparmor-disabled-overriden.yml", []string{override.GetOverriddenResultName(AppArmorDisabled)}, true}, + {"apparmor-disabled-overriden-multiple.yml", []string{AppArmorAnnotationMissing, override.GetOverriddenResultName(AppArmorDisabled)}, true}, // These are invalid manifests so we should only test it in manifest mode as kubernetes will fail to apply it - {"apparmor-disabled.yml", []string{AppArmorDisabled}, false}, + {"apparmor-bad-value.yml", []string{AppArmorBadValue}, false}, + {"apparmor-bad-value-override.yml", []string{AppArmorBadValue}, false}, {"apparmor-invalid-annotation.yml", []string{AppArmorInvalidAnnotation}, false}, } diff --git a/auditors/apparmor/fixtures/apparmor-bad-value-override.yml b/auditors/apparmor/fixtures/apparmor-bad-value-override.yml new file mode 100644 index 00000000..1df0cdff --- /dev/null +++ b/auditors/apparmor/fixtures/apparmor-bad-value-override.yml @@ -0,0 +1,13 @@ +apiVersion: v1 +kind: Pod +metadata: + name: pod + namespace: apparmor-bad-value-override + annotations: + container.apparmor.security.beta.kubernetes.io/container: badval + labels: + container.audit.kubernetes.io/container.allow-disabled-apparmor: "SomeReason" +spec: + containers: + - name: container + image: scratch diff --git a/auditors/apparmor/fixtures/apparmor-bad-value.yml b/auditors/apparmor/fixtures/apparmor-bad-value.yml new file mode 100644 index 00000000..5c3703ee --- /dev/null +++ b/auditors/apparmor/fixtures/apparmor-bad-value.yml @@ -0,0 +1,11 @@ +apiVersion: v1 +kind: Pod +metadata: + name: pod + namespace: apparmor-bad-value + annotations: + container.apparmor.security.beta.kubernetes.io/container: badval +spec: + containers: + - name: container + image: scratch diff --git a/auditors/apparmor/fixtures/apparmor-disabled-overriden-multiple.yml b/auditors/apparmor/fixtures/apparmor-disabled-overriden-multiple.yml new file mode 100644 index 00000000..fe0496ff --- /dev/null +++ b/auditors/apparmor/fixtures/apparmor-disabled-overriden-multiple.yml @@ -0,0 +1,15 @@ +apiVersion: v1 +kind: Pod +metadata: + name: pod + namespace: apparmor-disabled-overriden-multiple + annotations: + container.apparmor.security.beta.kubernetes.io/container2: unconfined + labels: + container.audit.kubernetes.io/container2.allow-disabled-apparmor: "SomeReason" +spec: + containers: + - name: container + image: scratch + - name: container2 + image: scratch diff --git a/auditors/apparmor/fixtures/apparmor-disabled-overriden.yml b/auditors/apparmor/fixtures/apparmor-disabled-overriden.yml new file mode 100644 index 00000000..6afdb01c --- /dev/null +++ b/auditors/apparmor/fixtures/apparmor-disabled-overriden.yml @@ -0,0 +1,13 @@ +apiVersion: v1 +kind: Pod +metadata: + name: pod + namespace: apparmor-disabled-overriden + annotations: + container.apparmor.security.beta.kubernetes.io/container: unconfined + labels: + container.audit.kubernetes.io/container.allow-disabled-apparmor: "SomeReason" +spec: + containers: + - name: container + image: scratch diff --git a/auditors/apparmor/fixtures/apparmor-disabled.yml b/auditors/apparmor/fixtures/apparmor-disabled.yml index b2593ab1..ddebab64 100644 --- a/auditors/apparmor/fixtures/apparmor-disabled.yml +++ b/auditors/apparmor/fixtures/apparmor-disabled.yml @@ -4,7 +4,7 @@ metadata: name: pod namespace: apparmor-disabled annotations: - container.apparmor.security.beta.kubernetes.io/container: badval + container.apparmor.security.beta.kubernetes.io/container: unconfined spec: containers: - name: container diff --git a/docs/auditors/apparmor.md b/docs/auditors/apparmor.md index 12ac116f..5c7a644c 100644 --- a/docs/auditors/apparmor.md +++ b/docs/auditors/apparmor.md @@ -80,4 +80,28 @@ To learn more about AppArmor in Kubernetes, see https://kubernetes.io/docs/tutor ## Override Errors -Overrides are not currently supported for `apparmor`. +First, see the [Introduction to Override Errors](/README.md#override-errors). + +Override identifier for the `unconfined` apparmor profile value: `allow-disabled-apparmor` + +Container overrides have the form: +```yaml +container.audit.kubernetes.io/[container name].allow-disabled-apparmor: "SomeReason" +``` + +Example of resource with the `unconfined` apparmor profile overridden for a specific container: +```yaml +apiVersion: apps/v1 +kind: Deployment +spec: + template: + metadata: + annotations: + container.apparmor.security.beta.kubernetes.io/myContainer: unconfined + labels: + container.audit.kubernetes.io/myContainer.allow-disabled-apparmor: "SomeReason" + spec: + containers: + - name: myContainer + image: scratch +``` diff --git a/pkg/override/override.go b/pkg/override/override.go index 018ed951..71824234 100644 --- a/pkg/override/override.go +++ b/pkg/override/override.go @@ -38,7 +38,7 @@ func NewRedundantOverrideResult(auditorName, containerName, overrideReason, over } // ApplyOverride checks if hasOverride is true. If it is, it changes the severity of the audit result from error to -// warn, adds the override reason to the metadata and removes the pending fix +// info, adds the override reason to the metadata and removes the pending fix func ApplyOverride(auditResult *kubeaudit.AuditResult, auditorName, containerName string, resource k8s.Resource, overrideLabel string) *kubeaudit.AuditResult { hasOverride, overrideReason := GetContainerOverrideReason(containerName, resource, overrideLabel)