From 8fd055bc60f1aacfa0157c7cb538e69726033ab4 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Wed, 9 Dec 2020 07:28:00 +0000 Subject: [PATCH 1/4] Model SecretInterface from k8s.io/client-go/kubernetes/typed/core/v1 --- ql/src/go.qll | 1 + ql/src/semmle/go/frameworks/K8sIoClientGo.qll | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 ql/src/semmle/go/frameworks/K8sIoClientGo.qll diff --git a/ql/src/go.qll b/ql/src/go.qll index 47126d338c5..d4cd5b7e31f 100644 --- a/ql/src/go.qll +++ b/ql/src/go.qll @@ -38,6 +38,7 @@ import semmle.go.frameworks.Gin import semmle.go.frameworks.Glog import semmle.go.frameworks.GoRestfulHttp import semmle.go.frameworks.K8sIoApimachineryPkgRuntime +import semmle.go.frameworks.K8sIoClientGo import semmle.go.frameworks.Logrus import semmle.go.frameworks.Macaron import semmle.go.frameworks.Mux diff --git a/ql/src/semmle/go/frameworks/K8sIoClientGo.qll b/ql/src/semmle/go/frameworks/K8sIoClientGo.qll new file mode 100644 index 00000000000..38048aed95c --- /dev/null +++ b/ql/src/semmle/go/frameworks/K8sIoClientGo.qll @@ -0,0 +1,29 @@ +/** Provides models of commonly used functions in the `k8s.io/client-go/kubernetes/typed/core/v1` package. */ + +import go + +/** + * Provides models of commonly used functions in the `k8s.io/client-go/kubernetes/typed/core/v1` + * package. + */ +module K8sIoClientGo { + /** Gets the package name. */ + bindingset[result] + string packagePath() { result = package("k8s.io/client-go", "kubernetes/typed/core/v1") } + + /** + * A model of `SecretInterface` methods that are sources of secret data. + */ + private class SecretInterfaceSourceMethod extends Method { + SecretInterfaceSourceMethod() { + this.implements(packagePath(), "SecretInterface", ["Get", "List", "Patch"]) + } + } + + /** + * A model of `SecretInterface` as a source of secret data. + */ + class SecretInterfaceSource extends DataFlow::Node { + SecretInterfaceSource() { this = any(SecretInterfaceSourceMethod g).getACall().getResult(0) } + } +} From 6ca2e0e38e8a0343fde685854bc538a96d7cbd6a Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 1 Dec 2020 13:12:14 +0000 Subject: [PATCH 2/4] Add SecretInterface as source for cleartext logging query --- change-notes/2020-12-09-clear-text-logging-source.md | 2 ++ ql/src/semmle/go/security/CleartextLoggingCustomizations.qll | 4 ++++ 2 files changed, 6 insertions(+) create mode 100644 change-notes/2020-12-09-clear-text-logging-source.md diff --git a/change-notes/2020-12-09-clear-text-logging-source.md b/change-notes/2020-12-09-clear-text-logging-source.md new file mode 100644 index 00000000000..7ba2113ba79 --- /dev/null +++ b/change-notes/2020-12-09-clear-text-logging-source.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* The query "Clear-text logging of sensitive information" has been improved to recognize `SecretInterface` from `k8s.io/client-go/kubernetes/typed/core/v1` as a source of sensitive data, which may lead to more alerts. diff --git a/ql/src/semmle/go/security/CleartextLoggingCustomizations.qll b/ql/src/semmle/go/security/CleartextLoggingCustomizations.qll index 9f21fa67eb4..1a6af789aaf 100644 --- a/ql/src/semmle/go/security/CleartextLoggingCustomizations.qll +++ b/ql/src/semmle/go/security/CleartextLoggingCustomizations.qll @@ -184,6 +184,10 @@ module CleartextLogging { override string describe() { result = "HTTP request headers" } } + private class KubernetesSecretInterfaceSource extends Source, K8sIoClientGo::SecretInterfaceSource { + override string describe() { result = "Kubernetes Secret" } + } + /** * The first element of a split by ' ' or ':', often sanitizing a username/password pair * or the "Method value" syntax used in the HTTP Authorization header. From 676ca529b5186d218b4e7c9496a47b35cb3b0b67 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Wed, 9 Dec 2020 12:22:48 +0000 Subject: [PATCH 3/4] Add tests --- .../SecretInterfaceSource.expected | 0 .../K8sIoClientGo/SecretInterfaceSource.ql | 17 ++++ .../semmle/go/frameworks/K8sIoClientGo/go.mod | 16 ++++ .../go/frameworks/K8sIoClientGo/main.go | 83 +++++++++++++++++++ .../kubernetes/typed/core/v1/stub.go | 24 ++++++ .../K8sIoClientGo/vendor/modules.txt | 25 ++++++ 6 files changed, 165 insertions(+) create mode 100644 ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/SecretInterfaceSource.expected create mode 100644 ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/SecretInterfaceSource.ql create mode 100644 ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/go.mod create mode 100644 ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/main.go create mode 100644 ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/vendor/k8s.io/client-go/kubernetes/typed/core/v1/stub.go create mode 100644 ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/vendor/modules.txt diff --git a/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/SecretInterfaceSource.expected b/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/SecretInterfaceSource.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/SecretInterfaceSource.ql b/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/SecretInterfaceSource.ql new file mode 100644 index 00000000000..be59ef4371c --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/SecretInterfaceSource.ql @@ -0,0 +1,17 @@ +import go +import TestUtilities.InlineExpectationsTest + +class K8sIoApimachineryPkgRuntimeTest extends InlineExpectationsTest { + K8sIoApimachineryPkgRuntimeTest() { this = "KsIoClientGoTest" } + + override string getARelevantTag() { result = "KsIoClientGo" } + + override predicate hasActualResult(string file, int line, string element, string tag, string value) { + exists(K8sIoClientGo::SecretInterfaceSource source | + source.hasLocationInfo(file, line, _, _, _) and + element = source.toString() and + value = "" and + tag = "KsIoClientGo" + ) + } +} diff --git a/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/go.mod b/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/go.mod new file mode 100644 index 00000000000..19dc26f1753 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/go.mod @@ -0,0 +1,16 @@ +module codeql-go-tests/frameworks/K8sIoClientGo + +go 1.14 + +require ( + github.com/github/depstubber v0.0.0-20201022140002-ee3b8f2acc53 // indirect + github.com/google/gofuzz v1.2.0 // indirect + golang.org/x/crypto v0.0.0-20201208171446-5f87f3452ae9 // indirect + golang.org/x/net v0.0.0-20201207224615-747e23833adb // indirect + golang.org/x/oauth2 v0.0.0-20201208152858-08078c50e5b5 // indirect + golang.org/x/time v0.0.0-20201208040808-7e3f01d25324 // indirect + k8s.io/client-go v0.19.0 + k8s.io/utils v0.0.0-20201110183641-67b214c5f920 // indirect +) + +replace k8s.io/apimachinery => k8s.io/apimachinery v0.19.0 diff --git a/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/main.go b/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/main.go new file mode 100644 index 00000000000..530248f51bd --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/main.go @@ -0,0 +1,83 @@ +package main + +import ( + context "context" + + "k8s.io/client-go/kubernetes/typed/core/v1" +) + +//go:generate depstubber -vendor k8s.io/client-go/kubernetes/typed/core/v1 SecretInterface + +func main() { + var s mySecretInterface + var t core.SecretInterface + var ctx context.Context + var secret interface{} + var opts interface{} + var listOpts interface{} + var name string + var pt interface{} + var data []byte + + use(s.Create(ctx, secret, opts)) + use(t.Create(ctx, secret, opts)) + use(s.Update(ctx, secret, opts)) + use(t.Update(ctx, secret, opts)) + use(s.Delete(ctx, name, opts)) + use(t.Delete(ctx, name, opts)) + use(s.DeleteCollection(ctx, opts, listOpts)) + use(t.DeleteCollection(ctx, opts, listOpts)) + use(s.Get(ctx, name, opts)) // $KsIoClientGo + use(t.Get(ctx, name, opts)) // $KsIoClientGo + use(s.List(ctx, opts)) // $KsIoClientGo + use(t.List(ctx, opts)) // $KsIoClientGo + use(s.Watch(ctx, opts)) + use(t.Watch(ctx, opts)) + use(s.Patch(ctx, name, pt, data, opts)) // $KsIoClientGo + use(t.Patch(ctx, name, pt, data, opts)) // $KsIoClientGo +} + +func use(arg ...interface{}) {} + +type mySecretInterface struct { +} + +// func (m mySecretInterface) Create(ctx context.Context, secret *v1.Secret, opts metav1.CreateOptions) (*v1.Secret, error) { +func (m mySecretInterface) Create(ctx context.Context, secret interface{}, opts interface{}) (interface{}, error) { + return nil, nil +} + +// func (m mySecretInterface) Update(ctx context.Context, secret *v1.Secret, opts metav1.UpdateOptions) (*v1.Secret, error) { +func (m mySecretInterface) Update(ctx context.Context, secret interface{}, opts interface{}) (interface{}, error) { + return nil, nil +} + +// func (m mySecretInterface) Delete(ctx context.Context, name string, opts metav1.DeleteOptions) error { +func (m mySecretInterface) Delete(ctx context.Context, name string, opts interface{}) error { + return nil +} + +// func (m mySecretInterface) DeleteCollection(ctx context.Context, opts metav1.DeleteOptions, listOpts metav1.ListOptions) error { +func (m mySecretInterface) DeleteCollection(ctx context.Context, opts interface{}, listOpts interface{}) error { + return nil +} + +// func (m mySecretInterface) Get(ctx context.Context, name string, opts metav1.GetOptions) (*v1.Secret, error) { +func (m mySecretInterface) Get(ctx context.Context, name string, opts interface{}) (interface{}, error) { + return nil, nil +} + +// func (m mySecretInterface) List(ctx context.Context, opts metav1.ListOptions) (*v1.SecretList, error) { +func (m mySecretInterface) List(ctx context.Context, opts interface{}) (interface{}, error) { + return nil, nil +} + +// func (m mySecretInterface) Watch(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { +func (m mySecretInterface) Watch(ctx context.Context, opts interface{}) (interface{}, error) { + return nil, nil +} + +// func (m mySecretInterface) Patch(ctx context.Context, name string, pt types.PatchType, data []byte, opts metav1.PatchOptions, subresources ...string) (result *v1.Secret, err error) { +func (m mySecretInterface) Patch(ctx context.Context, name string, pt interface{}, data []byte, opts interface{}, subresources ...string) (result interface{}, err error) { + return nil, nil +} diff --git a/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/vendor/k8s.io/client-go/kubernetes/typed/core/v1/stub.go b/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/vendor/k8s.io/client-go/kubernetes/typed/core/v1/stub.go new file mode 100644 index 00000000000..bfe2cd72cd5 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/vendor/k8s.io/client-go/kubernetes/typed/core/v1/stub.go @@ -0,0 +1,24 @@ +// Code generated by depstubber. DO NOT EDIT. +// This is a simple stub for k8s.io/client-go/kubernetes/typed/core/v1, strictly for use in testing. + +// See the LICENSE file for information about the licensing of the original library. +// Source: k8s.io/client-go/kubernetes/typed/core/v1 (exports: SecretInterface; functions: ) + +// Package core is a stub of k8s.io/client-go/kubernetes/typed/core/v1, generated by depstubber. +package core + +import ( + context "context" + time "time" +) + +type SecretInterface interface { + Create(_ context.Context, _ interface{}, _ interface{}) (interface{}, error) + Delete(_ context.Context, _ string, _ interface{}) error + DeleteCollection(_ context.Context, _ interface{}, _ interface{}) error + Get(_ context.Context, _ string, _ interface{}) (interface{}, error) + List(_ context.Context, _ interface{}) (interface{}, error) + Patch(_ context.Context, _ string, _ interface{}, _ []byte, _ interface{}, _ ...string) (interface{}, error) + Update(_ context.Context, _ interface{}, _ interface{}) (interface{}, error) + Watch(_ context.Context, _ interface{}) (interface{}, error) +} diff --git a/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/vendor/modules.txt b/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/vendor/modules.txt new file mode 100644 index 00000000000..7c2af624559 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/vendor/modules.txt @@ -0,0 +1,25 @@ +# github.com/github/depstubber v0.0.0-20201022140002-ee3b8f2acc53 +## explicit +github.com/github/depstubber +# github.com/google/gofuzz v1.2.0 +## explicit +github.com/google/gofuzz +# golang.org/x/crypto v0.0.0-20201208171446-5f87f3452ae9 +## explicit +golang.org/x/crypto +# golang.org/x/net v0.0.0-20201207224615-747e23833adb +## explicit +golang.org/x/net +# golang.org/x/oauth2 v0.0.0-20201208152858-08078c50e5b5 +## explicit +golang.org/x/oauth2 +# golang.org/x/time v0.0.0-20201208040808-7e3f01d25324 +## explicit +golang.org/x/time +# k8s.io/client-go v0.19.0 +## explicit +k8s.io/client-go +# k8s.io/utils v0.0.0-20201110183641-67b214c5f920 +## explicit +k8s.io/utils +# k8s.io/apimachinery => k8s.io/apimachinery v0.19.0 From 0980a50627c3ccef831a0dac1b851fe46a14f8d6 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Wed, 9 Dec 2020 12:23:39 +0000 Subject: [PATCH 4/4] Remove erroneous import from stub --- .../vendor/k8s.io/client-go/kubernetes/typed/core/v1/stub.go | 1 - 1 file changed, 1 deletion(-) diff --git a/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/vendor/k8s.io/client-go/kubernetes/typed/core/v1/stub.go b/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/vendor/k8s.io/client-go/kubernetes/typed/core/v1/stub.go index bfe2cd72cd5..82793a87110 100644 --- a/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/vendor/k8s.io/client-go/kubernetes/typed/core/v1/stub.go +++ b/ql/test/library-tests/semmle/go/frameworks/K8sIoClientGo/vendor/k8s.io/client-go/kubernetes/typed/core/v1/stub.go @@ -9,7 +9,6 @@ package core import ( context "context" - time "time" ) type SecretInterface interface {