Repository: kubernetes-sigs/security-profiles-operator
Pull request #2111 (2024-02-06 09:38:34)

AppArmor Recorder Improvements
#### What type of PR is this?

/kind bug
/kind cleanup

#### What this PR does / why we need it:

This PR fixes a bunch of nits and smaller issues in the AppArmor recorder implementation. I'm working with @0xmilkmix on more substantial improvements, but I figured it'd be useful to file this early as an easy-to-review PR that gets small adjacent fixes out of the way. :)

#### Which issue(s) this PR fixes:

None

#### Does this PR have test?

N/A

#### Special notes for your reviewer:

#### Does this PR introduce a user-facing change?

```release-note
NONE
```


k8s-ci-robot: Hi @mhils. Thanks for your PR.

I'm waiting for a [kubernetes-sigs](https://github.com/orgs/kubernetes-sigs/people) member to verify that this patch is reasonable to test. If it is, they should reply with `/ok-to-test` on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should [join the org](https://git.k8s.io/community/community-membership.md#member) to skip this step.

Once the patch is verified, the new status will be reflected by the `ok-to-test` label.

I understand the commands that are listed [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fsecurity-profiles-operator).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>

k8s-ci-robot: Welcome @mhils! <br><br>It looks like this is your first PR to <a href='https://github.com/kubernetes-sigs/security-profiles-operator'>kubernetes-sigs/security-profiles-operator</a> 🎉. Please refer to our [pull request process documentation](https://git.k8s.io/community/contributors/guide/pull-requests.md) to help your PR have a smooth ride to approval. <br><br>You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. [Here is the bot commands documentation](https://go.k8s.io/bot-commands). <br><br>You can also check if kubernetes-sigs/security-profiles-operator has [its own contribution guidelines](https://github.com/kubernetes-sigs/security-profiles-operator/tree/master/CONTRIBUTING.md). <br><br>You may want to refer to our [testing guide](https://git.k8s.io/community/contributors/devel/sig-testing/testing.md) if you run into trouble with your tests not passing. <br><br>If you are having difficulty getting your pull request seen, please follow the [recommended escalation practices](https://github.com/kubernetes/community/blob/master/contributors/guide/pull-requests.md#why-is-my-pull-request-not-getting-reviewed). Also, for tips and tricks in the contribution process you may want to read the [Kubernetes contributor cheat sheet](https://git.k8s.io/community/contributors/guide/contributor-cheatsheet/README.md). We want to make sure your contribution gets all the attention it needs! <br><br>Thank you, and welcome to Kubernetes. :smiley:
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/security-profiles-operator/pull/2111#" title="Author self-approved">mhils</a>*
**Once this PR has been reviewed and has the lgtm label**, please assign [ccojocar](https://github.com/ccojocar) for approval. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fsecurity-profiles-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes-sigs/security-profiles-operator/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["ccojocar"]} -->
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/security-profiles-operator/pull/2111#" title="Author self-approved">mhils</a>*, *<a href="https://github.com/kubernetes-sigs/security-profiles-operator/pull/2111#pullrequestreview-1864717858" title="Approved">saschagrunert</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fsecurity-profiles-operator).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/security-profiles-operator/blob/main/OWNERS)~~ [saschagrunert]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
k8s-ci-robot: New changes are detected. LGTM label has been removed.

Patch:
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 8c0204888..694c435cf 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -37,7 +37,7 @@ jobs:
           authToken: '${{ secrets.CACHIX_AUTH_TOKEN }}'
           pushFilter: security-profiles-operator
       - run: make nix
-      - uses: actions/upload-artifact@26f96dfa697d77e81fd5907df203aa23a56210a8 # v4.3.0
+      - uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 # v4.3.1
         with:
           name: build
           path: build.tar.gz
@@ -64,7 +64,7 @@ jobs:
             https://github.com/kubernetes-sigs/bom/releases/download/${{ env.BOM_VERSION }}/bom-amd64-linux
           sudo chmod +x /usr/bin/bom
       - run: make nix-spoc
-      - uses: actions/upload-artifact@26f96dfa697d77e81fd5907df203aa23a56210a8 # v4.3.0
+      - uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 # v4.3.1
         with:
           name: spoc
           path: |
diff --git a/.github/workflows/helm-chart-package.yaml b/.github/workflows/helm-chart-package.yaml
index d07dddcaf..8726d7113 100644
--- a/.github/workflows/helm-chart-package.yaml
+++ b/.github/workflows/helm-chart-package.yaml
@@ -28,7 +28,7 @@ jobs:
           [ ! -f "${tgz_path}" ] && echo "failed to find helm chart from 'helm package' stdout" && exit 1
           echo "helm_tgz_path=${tgz_path}" >> $GITHUB_OUTPUT
           echo "helm_tgz_name=${tgz_path##*/}" >> $GITHUB_OUTPUT
-      - uses: actions/upload-artifact@26f96dfa697d77e81fd5907df203aa23a56210a8 # v4.3.0
+      - uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 # v4.3.1
         with:
           name: ${{ steps.package.outputs.helm_tgz_name}}
           path: ${{ steps.package.outputs.helm_tgz_path}}
diff --git a/.github/workflows/osff.yml b/.github/workflows/osff.yml
index c00352a72..abc38eab8 100644
--- a/.github/workflows/osff.yml
+++ b/.github/workflows/osff.yml
@@ -55,7 +55,7 @@ jobs:
       # Upload the results as artifacts (optional). Commenting out will disable uploads of run results in SARIF
       # format to the repository Actions tab.
       - name: "Upload artifact"
-        uses: actions/upload-artifact@26f96dfa697d77e81fd5907df203aa23a56210a8 # v4.3.0
+        uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 # v4.3.1
         with:
           name: SARIF file
           path: results.sarif
diff --git a/internal/pkg/cli/recorder/consts.go b/internal/pkg/cli/recorder/consts.go
index 501bae7bf..0610228d1 100644
--- a/internal/pkg/cli/recorder/consts.go
+++ b/internal/pkg/cli/recorder/consts.go
@@ -51,7 +51,7 @@ const (
 	TypeApparmor Type = "apparmor"
 
 	// TypeRawAppArmor is the type indicating that we should record a raw
-	// seccomp JSON profile.
+	// apparmor JSON profile.
 	TypeRawAppArmor Type = "raw-apparmor"
 )
 
diff --git a/internal/pkg/daemon/bpfrecorder/apparmor_filters.go b/internal/pkg/daemon/bpfrecorder/apparmor_filters.go
index d231a882c..277a150bf 100644
--- a/internal/pkg/daemon/bpfrecorder/apparmor_filters.go
+++ b/internal/pkg/daemon/bpfrecorder/apparmor_filters.go
@@ -38,9 +38,13 @@ var knownLibrariesPrefixes = []string{
 var knownReadPrefixes = []string{
 	"/dev/random",
 	"/dev/urandom",
+	"/etc/ld.so.cache",
+	"/etc/ld.so.conf",
+	"/etc/ld.so.conf.d/",
 	"/etc/locale/",
 	"/etc/locale.alias/",
 	"/etc/localtime",
+	"/sys/kernel/mm/transparent_hugepage/hpage_pmd_size",
 	"/usr/share/locale/",
 }
 
diff --git a/internal/pkg/daemon/bpfrecorder/bpfrecorder.go b/internal/pkg/daemon/bpfrecorder/bpfrecorder.go
index 80467bf6c..ea1ddba13 100644
--- a/internal/pkg/daemon/bpfrecorder/bpfrecorder.go
+++ b/internal/pkg/daemon/bpfrecorder/bpfrecorder.go
@@ -81,6 +81,9 @@ const (
 	sockDgram           int           = 2
 	sockRaw             int           = 3
 	sockTypeMask        int           = 0xF
+	fdStdin             uint64        = 0
+	fdStdout            uint64        = 1
+	fdStderr            uint64        = 2
 )
 
 // BpfRecorder is the main structure of this package.
@@ -118,7 +121,7 @@ type BpfRecorder struct {
 
 type bpfAppArmorEvent struct {
 	Pid   uint32
-	Mtns  uint32
+	Mntns uint32
 	Type  uint8
 	Flags uint64
 	Fd    uint64
@@ -767,11 +770,11 @@ func fileDataToString(data *[pathMax]uint8) string {
 
 func (b *BpfRecorder) handleMmapEvent(fileEvent *bpfAppArmorEvent) {
 	path, ok := b.fdMapping[fileEvent.Fd]
-	if int(fileEvent.Fd) < 0 {
+	if int32(fileEvent.Fd) < 0 {
 		return
 	}
 	if !ok {
-		b.logger.Info(fmt.Sprintf("Got event for unknown fd: %d", fileEvent.Fd))
+		b.logger.Info(fmt.Sprintf("Got mmap event for unknown fd: %d", fileEvent.Fd))
 		return
 	}
 	for i, file := range b.recordedFiles {
@@ -791,12 +794,15 @@ func (b *BpfRecorder) handleMmapEvent(fileEvent *bpfAppArmorEvent) {
 }
 
 func (b *BpfRecorder) handleWrite(fileEvent *bpfAppArmorEvent) {
-	if int(fileEvent.Fd) < 0 {
+	if int32(fileEvent.Fd) < 0 {
 		return
 	}
 	path, ok := b.fdMapping[fileEvent.Fd]
 	if !ok {
-		b.logger.Info(fmt.Sprintf("Got event for unknown fd: %d", fileEvent.Fd))
+		isStdoutStderr := fileEvent.Fd == fdStdout || fileEvent.Fd == fdStderr
+		if !isStdoutStderr {
+			b.logger.Info(fmt.Sprintf("Got write event for unknown fd: %d", fileEvent.Fd))
+		}
 		return
 	}
 	for i, file := range b.recordedFiles {
@@ -808,12 +814,15 @@ func (b *BpfRecorder) handleWrite(fileEvent *bpfAppArmorEvent) {
 }
 
 func (b *BpfRecorder) handleRead(fileEvent *bpfAppArmorEvent) {
-	if int(fileEvent.Fd) < 0 {
+	if int32(fileEvent.Fd) < 0 {
 		return
 	}
 	path, ok := b.fdMapping[fileEvent.Fd]
 	if !ok {
-		b.logger.Info(fmt.Sprintf("Got event for unknown fd: %d", fileEvent.Fd))
+		isStdin := fileEvent.Fd == fdStdin
+		if !isStdin {
+			b.logger.Info(fmt.Sprintf("Got read event for unknown fd: %d", fileEvent.Fd))
+		}
 		return
 	}
 	for i, file := range b.recordedFiles {
@@ -833,7 +842,7 @@ func (b *BpfRecorder) handleAppArmorFileEvents(fileEvent *bpfAppArmorEvent) {
 		var fileEv bpfAppArmorFileEvent
 		fileEv.Filename = fileDataToString(&fileEvent.Data)
 		fileEv.Flags = fileEvent.Flags
-		if int(fileEvent.Fd) < 0 {
+		if int32(fileEvent.Fd) < 0 {
 			fileEv.GotError = true
 		}
 		b.recordedFiles = append(b.recordedFiles, fileEv)


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 68d44aed1..c2e6a364b 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -50,7 +50,7 @@ jobs:
       - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v3.5.2
       - run: make image
       - run: podman save -o image.tar security-profiles-operator
-      - uses: actions/upload-artifact@26f96dfa697d77e81fd5907df203aa23a56210a8
+      - uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3
         with:
           name: image
           path: image.tar