Skip to content

[v1.13] bugtool: Avoid sensitive data in envoy config dump #32966

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Jun 8, 2024
Merged

[v1.13] bugtool: Avoid sensitive data in envoy config dump #32966

merged 2 commits into from
Jun 8, 2024

Conversation

sayboras
Copy link
Member

@sayboras sayboras commented Jun 7, 2024

No description provided.

@maintainer-s-little-helper maintainer-s-little-helper bot added backport/1.13 kind/backports This PR provides functionality previously merged into master. labels Jun 7, 2024
@sayboras sayboras force-pushed the pr/tammach/envoy-bugtool-1.13 branch from acc0201 to c08c3b1 Compare June 7, 2024 16:03
@sayboras
Copy link
Member Author

sayboras commented Jun 7, 2024

/test-backport-1.13

@sayboras sayboras marked this pull request as ready for review June 7, 2024 16:06
@sayboras sayboras requested a review from a team as a code owner June 7, 2024 16:06
@sayboras sayboras force-pushed the pr/tammach/envoy-bugtool-1.13 branch from c08c3b1 to 89379db Compare June 7, 2024 16:10
@sayboras
Copy link
Member Author

sayboras commented Jun 7, 2024

/test-backport-1.13

@sayboras sayboras force-pushed the pr/tammach/envoy-bugtool-1.13 branch from 89379db to 621417f Compare June 7, 2024 16:19
@sayboras
Copy link
Member Author

sayboras commented Jun 7, 2024 •
edited by maintainer-s-little-helper bot
Loading

/test-backport-1.13

Job 'Cilium-PR-K8s-1.26-kernel-net-next' failed:

Click to show.

Test Name

K8sAgentPolicyTest Multi-node policy test with L7 policy using connectivity-check to check datapath

Failure Output

FAIL: cannot install connectivity-check

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.26-kernel-net-next/1375/

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.26-kernel-net-next so I can create one.

Then please upload the Jenkins artifacts to that issue.

@aanm
Copy link
Member

aanm commented Jun 7, 2024

/test-runtime

@qmonnet
Copy link
Member

qmonnet commented Jun 7, 2024 •
edited
Loading 

20:03:21  FAIL: Failed to run privileged unit tests
20:03:21  Expected command: bash -c 'sudo make -C /home/vagrant/go/src/github.com/cilium/cilium/ tests-privileged NO_COLOR=1 | ts "[%H:%M:%S]"; exit "${PIPESTATUS[0]}"' 
20:03:21  To succeed, but it failed:
20:03:21  Exitcode: 2 
20:03:21  Err: Process exited with status 2
20:03:21  Stdout:
20:03:21   	 [19:03:05] make: Entering directory '/home/vagrant/go/src/github.com/cilium/cilium'
20:03:21  	 [19:03:13] PRIVILEGED_TESTS=true PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/home/vagrant/go/src/github.com/cilium/cilium/bpf CGO_ENABLED=0 go test  -mod=vendor -vet=all -tags=osusergo -ldflags "-X github.com/cilium/cilium/pkg/kvstore.consulDummyAddress=https://consul:8443/ -X github.com/cilium/cilium/pkg/kvstore.etcdDummyAddress=http://etcd:4002/ -X github.com/cilium/cilium/pkg/datapath.DatapathSHA256=e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" \
20:03:21  	 [19:03:13] 	./... -test.v -timeout 600s  -json -covermode=count -coverprofile=coverage.out | tparse -follow
20:03:21  	 [19:03:19] make: Leaving directory '/home/vagrant/go/src/github.com/cilium/cilium'
20:03:21  	 
20:03:21  Stderr:
20:03:21   	 bugtool/cmd/mask.go:8:2: package slices is not in GOROOT (/usr/local/go/src/slices)
20:03:21  	 no parseable events: Make sure to run go test with -json flag
20:03:21  	 make: *** [Makefile:115: tests-privileged] Error 1

I read this as the Go version being too old to use if slices.Contains(...) in mask.go.

sayboras added 2 commits June 7, 2024 21:38
@sayboras @qmonnet
bugtool: Add json masking function
a30e981 
[upstream commit 568dbc5]

This commit is to add a generic json field masking based on the field
name.

[ Quentin: Replaced the use of slices.Contains(), unsupported with the
  Go version used with v1.13. ]

Signed-off-by: Tam Mach <tam.mach@cilium.io>
Signed-off-by: Quentin Monnet <qmo@qmon.net>
@sayboras @qmonnet
bugtool: Add post-processing masking function for Envoy
e869c11 
[upstream commit 48a9976]

This commit is to explicitly mask the below fields from Envoy config
dump:

- api_key (used in kafka L7 policy)
- TLSContext (used in Cilium NetworkPolicy)

One round of scanning on existing Cilium protobuf was done.

Related docs: https://www.envoyproxy.io/docs/envoy/latest/operations/admin#get--config_dump

Signed-off-by: Tam Mach <tam.mach@cilium.io>
@qmonnet qmonnet force-pushed the pr/tammach/envoy-bugtool-1.13 branch from 621417f to e869c11 Compare June 7, 2024 20:40
@qmonnet
Copy link
Member

qmonnet commented Jun 7, 2024

I added the following change to the first commit:

diff --git a/bugtool/cmd/mask.go b/bugtool/cmd/mask.go
index 4a6a32961d03..77847895d503 100644
--- a/bugtool/cmd/mask.go
+++ b/bugtool/cmd/mask.go
@@ -5,7 +5,6 @@ package cmd
 
 import (
        "encoding/json"
-       "slices"
 )
 
 const (
@@ -34,9 +33,18 @@ func maskFields(b []byte, fieldNames []string) ([]byte, error) {
        return json.MarshalIndent(data, "", ident)
 }
 
+func contains(names []string, name string) bool {
+       for _, n := range names {
+               if n == name {
+                       return true
+               }
+       }
+       return false
+}
+
 func mask(data map[string]interface{}, fieldNames []string) {
        for k, v := range data {
-               if slices.Contains(fieldNames, k) {
+               if contains(fieldNames, k) {
                        data[k] = redacted
                        continue
                }

@qmonnet
Copy link
Member

qmonnet commented Jun 7, 2024 •
edited by maintainer-s-little-helper bot
Loading

/test-backport-1.13

Job 'Cilium-PR-K8s-1.21-kernel-4.19' hit: #30802 (90.53% similarity)

@maintainer-s-little-helper maintainer-s-little-helper bot mentioned this pull request Jun 7, 2024
Closed
@qmonnet
Copy link
Member

qmonnet commented Jun 7, 2024

/test-1.21-4.19

@sayboras
Copy link
Member Author

sayboras commented Jun 8, 2024

/test-1.19-4.19

@michi-covalent michi-covalent merged commit bf9a1ae into v1.13 Jun 8, 2024
@michi-covalent michi-covalent deleted the pr/tammach/envoy-bugtool-1.13 branch June 8, 2024 03:49
@michi-covalent michi-covalent restored the pr/tammach/envoy-bugtool-1.13 branch June 8, 2024 03:50
@michi-covalent michi-covalent deleted the pr/tammach/envoy-bugtool-1.13 branch June 8, 2024 03:50
@michi-covalent michi-covalent mentioned this pull request Jun 8, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qmonnet qmonnet qmonnet approved these changes

@michi-covalent michi-covalent michi-covalent approved these changes

Assignees
No one assigned
Labels
kind/backports This PR provides functionality previously merged into master.
Projects
No open projects
cilium v1.13.17
Status: Released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sayboras @aanm @qmonnet @michi-covalent