Skip to content
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.

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

^ in MONGO_SERVER_URL password causes null pointer panic #1114

Open
bradbeck opened this issue May 6, 2024 · 4 comments
Open

^ in MONGO_SERVER_URL password causes null pointer panic #1114

bradbeck opened this issue May 6, 2024 · 4 comments
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@bradbeck
Copy link
Contributor

bradbeck commented May 6, 2024

Expected Behavior

The user should be able to have a MongoDB password that contains a ^ as part of MONGO_SERVER_URL without causing a panic.

^ seems to be valid without encoding when using mongosh and is not listed as one of the characters that is required to be encoded in the MongoDB documentation.

Actual Behavior

Having a password as part of MONGO_SERVER_URL that contains ^ causes a null pointer access panic when attempting to store payloads.

Steps to Reproduce the Problem

  1. Configure Chains to use MongoDB for attestation storage for TaskRuns

  2. Include a ^ in the MongoDB password used in MONGO_SERVER_URL

  3. Run a TaskRun

  4. Chains will panic:

     ...
     06T13:46:37.350Z","logger":"watcher","caller":"sharedmain/main.go:283","msg":"Starting configuration manager...","commit":"ebcd9c2"}
     {"level":"error","ts":"2024-05-06T13:46:37.351Z","logger":"watcher","caller":"taskrun/controller.go:59","msg":"open collection mongo://tekton-chains/bar?id_field=name: failed to dial default Mongo server at \"mongodb://tekton:foo^bar@mongodb-0.mongodb-svc.mongodb.svc.cluster.local:27017/?authSource=admin&replicaSet=mongodb\": parse \"mongodb://tekton:foo^bar@mongodb-0.mongodb-svc.mongodb.svc.cluster.local:27017/?authSource=admin&replicaSet=mongodb\": net/url: invalid userinfo","commit":"ebcd9c2","stacktrace":"github.com/tektoncd/chains/pkg/reconciler/taskrun.NewController.func1.1\n\t/go/src/github.com/tektoncd/chains/pkg/reconciler/taskrun/controller.go:59\nknative.dev/pkg/configmap.(*UntypedStore).OnConfigChanged\n\t/go/src/github.com/tektoncd/chains/vendor/knative.dev/pkg/configmap/store.go:159\nknative.dev/pkg/configmap.(*ManualWatcher).OnChange\n\t/go/src/github.com/tektoncd/chains/vendor/knative.dev/pkg/configmap/manual_watcher.go:72\nknative.dev/pkg/configmap/informer.(*InformedWatcher).addConfigMapEvent\n\t/go/src/github.com/tektoncd/chains/vendor/knative.dev/pkg/configmap/informer/informed_watcher.go:220\nknative.dev/pkg/configmap/informer.(*syncedCallback).Call\n\t/go/src/github.com/tektoncd/chains/vendor/knative.dev/pkg/configmap/informer/synced_callback.go:94\nknative.dev/pkg/configmap/informer.(*InformedWatcher).Start.func1\n\t/go/src/github.com/tektoncd/chains/vendor/knative.dev/pkg/configmap/informer/informed_watcher.go:158\nk8s.io/client-go/tools/cache.ResourceEventHandlerFuncs.OnAdd\n\t/go/src/github.com/tektoncd/chains/vendor/k8s.io/client-go/tools/cache/controller.go:239\nk8s.io/client-go/tools/cache.(*processorListener).run.func1\n\t/go/src/github.com/tektoncd/chains/vendor/k8s.io/client-go/tools/cache/shared_informer.go:974\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1\n\t/go/src/github.com/tektoncd/chains/vendor/k8s.io/apimachinery/pkg/util/wait/backoff.go:226\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil\n\t/go/src/github.com/tektoncd/chains/vendor/k8s.io/apimachinery/pkg/util/wait/backoff.go:227\nk8s.io/apimachinery/pkg/util/wait.JitterUntil\n\t/go/src/github.com/tektoncd/chains/vendor/k8s.io/apimachinery/pkg/util/wait/backoff.go:204\nk8s.io/apimachinery/pkg/util/wait.Until\n\t/go/src/github.com/tektoncd/chains/vendor/k8s.io/apimachinery/pkg/util/wait/backoff.go:161\nk8s.io/client-go/tools/cache.(*processorListener).run\n\t/go/src/github.com/tektoncd/chains/vendor/k8s.io/client-go/tools/cache/shared_informer.go:968\nk8s.io/apimachinery/pkg/util/wait.(*Group).Start.func1\n\t/go/src/github.com/tektoncd/chains/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:72"}
     {"level":"error","ts":"2024-05-06T13:46:37.351Z","logger":"watcher","caller":"pipelinerun/controller.go:64","msg":"open collection mongo://tekton-chains/bar?id_field=name: failed to dial default Mongo server at \"mongodb://tekton:foo^bar@mongodb-0.mongodb-svc.mongodb.svc.cluster.local:27017/?authSource=admin&replicaSet=mongodb\": parse \"mongodb://tekton:foo^bar@mongodb-0.mongodb-svc.mongodb.svc.cluster.local:27017/?authSource=admin&replicaSet=mongodb\": net/url: invalid userinfo","commit":"ebcd9c2","stacktrace":"github.com/tektoncd/chains/pkg/reconciler/pipelinerun.NewController.func1.1\n\t/go/src/github.com/tektoncd/chains/pkg/reconciler/pipelinerun/controller.go:64\nknative.dev/pkg/configmap.(*UntypedStore).OnConfigChanged\n\t/go/src/github.com/tektoncd/chains/vendor/knative.dev/pkg/configmap/store.go:159\nknative.dev/pkg/configmap.(*ManualWatcher).OnChange\n\t/go/src/github.com/tektoncd/chains/vendor/knative.dev/pkg/configmap/manual_watcher.go:72\nknative.dev/pkg/configmap/informer.(*InformedWatcher).addConfigMapEvent\n\t/go/src/github.com/tektoncd/chains/vendor/knative.dev/pkg/configmap/informer/informed_watcher.go:220\nknative.dev/pkg/configmap/informer.(*syncedCallback).Call\n\t/go/src/github.com/tektoncd/chains/vendor/knative.dev/pkg/configmap/informer/synced_callback.go:94\nknative.dev/pkg/configmap/informer.(*InformedWatcher).Start.func1\n\t/go/src/github.com/tektoncd/chains/vendor/knative.dev/pkg/configmap/informer/informed_watcher.go:158\nk8s.io/client-go/tools/cache.ResourceEventHandlerFuncs.OnAdd\n\t/go/src/github.com/tektoncd/chains/vendor/k8s.io/client-go/tools/cache/controller.go:239\nk8s.io/client-go/tools/cache.(*processorListener).run.func1\n\t/go/src/github.com/tektoncd/chains/vendor/k8s.io/client-go/tools/cache/shared_informer.go:974\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1\n\t/go/src/github.com/tektoncd/chains/vendor/k8s.io/apimachinery/pkg/util/wait/backoff.go:226\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil\n\t/go/src/github.com/tektoncd/chains/vendor/k8s.io/apimachinery/pkg/util/wait/backoff.go:227\nk8s.io/apimachinery/pkg/util/wait.JitterUntil\n\t/go/src/github.com/tektoncd/chains/vendor/k8s.io/apimachinery/pkg/util/wait/backoff.go:204\nk8s.io/apimachinery/pkg/util/wait.Until\n\t/go/src/github.com/tektoncd/chains/vendor/k8s.io/apimachinery/pkg/util/wait/backoff.go:161\nk8s.io/client-go/tools/cache.(*processorListener).run\n\t/go/src/github.com/tektoncd/chains/vendor/k8s.io/client-go/tools/cache/shared_informer.go:968\nk8s.io/apimachinery/pkg/util/wait.(*Group).Start.func1\n\t/go/src/github.com/tektoncd/chains/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:72"}
     ...
     {"level":"info","ts":"2024-05-06T13:51:01.221Z","logger":"watcher","caller":"chains/signing.go:171","msg":"Signing object with kms","commit":"ebcd9c2","knative.dev/controller":"github.com.tektoncd.chains.pkg.reconciler.taskrun.Reconciler","knative.dev/kind":"tekton.dev.TaskRun","knative.dev/traceid":"de0bfbdd-d388-4c1e-aad0-00ab0cbebef8","knative.dev/key":"default/hello-pr-hello"}
     panic: runtime error: invalid memory address or nil pointer dereference
     [signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x262b814]
     
     goroutine 154 [running]:
     github.com/tektoncd/chains/pkg/chains.(*ObjectSigner).Sign(0x40004031c0, {0x38b1d88, 0x4000ee8a80}, {0x38e71e0, 0x4000e9c580})
         /go/src/github.com/tektoncd/chains/pkg/chains/signing.go:195 +0xdd4
     github.com/tektoncd/chains/pkg/reconciler/taskrun.(*Reconciler).FinalizeKind(0x40005b8ac0, {0x38b1d88, 0x4000ee8a80}, 0x4000ee0580)
         /go/src/github.com/tektoncd/chains/pkg/reconciler/taskrun/taskrun.go:67 +0x18c
     github.com/tektoncd/chains/pkg/reconciler/taskrun.(*Reconciler).ReconcileKind(0x0?, {0x38b1d88?, 0x4000ee8a80?}, 0x0?)
         /go/src/github.com/tektoncd/chains/pkg/reconciler/taskrun/taskrun.go:45 +0x24
     github.com/tektoncd/pipeline/pkg/client/injection/reconciler/pipeline/v1/taskrun.(*reconcilerImpl).Reconcile(0x4000685b80, {0x38b1d88, 0x4000ee89c0}, {0x400007dde8, 0x16})
         /go/src/github.com/tektoncd/chains/vendor/github.com/tektoncd/pipeline/pkg/client/injection/reconciler/pipeline/v1/taskrun/reconciler.go:236 +0x438
     knative.dev/pkg/controller.(*Impl).processNextWorkItem(0x40001159e0)
         /go/src/github.com/tektoncd/chains/vendor/knative.dev/pkg/controller/controller.go:542 +0x37c
     knative.dev/pkg/controller.(*Impl).RunContext.func3()
         /go/src/github.com/tektoncd/chains/vendor/knative.dev/pkg/controller/controller.go:491 +0x5c
     created by knative.dev/pkg/controller.(*Impl).RunContext
         /go/src/github.com/tektoncd/chains/vendor/knative.dev/pkg/controller/controller.go:489 +0x2b0
    
    

Additional Info

  • Kubernetes version:

    $ kubectl version
    Client Version: v1.30.0
    Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
    Server Version: v1.28.3+k3s2
    WARNING: version difference between client (1.30) and server (1.28) exceeds the supported minor version skew of +/-1
    
  • Tekton Pipeline version:

    $ tkn version
    Client version: 0.36.0
    Chains version: v0.20.1
    Pipeline version: v0.56.2
    
@bradbeck bradbeck added the kind/bug Categorizes issue or PR as related to a bug. label May 6, 2024
@concaf
Copy link
Contributor

concaf commented May 6, 2024

the panic should be fixed by #1113, specifically these lines in signing.go

-				b := o.Backends[backend]
+				logger.Infof("signable storage backends: %v", signableType.StorageBackend(cfg))
+				logger.Infof("o.Backends(): %v", o.Backends)
+
+				b, ok := o.Backends[backend]
+				if !ok {
+					backendErr := fmt.Errorf("could not find backend '%s' in configured backends (%v) while trying sign: %s/%s", backend, maps.Keys(o.Backends), tektonObj.GetKindName(), tektonObj.GetName())
+					logger.Error(backendErr)
+					merr = multierror.Append(merr, backendErr)
+					continue
+				}
+

however, it will not fix the underlying issue (^ in password)

@concaf
Copy link
Contributor

concaf commented May 6, 2024

that said, chains doesn't really parse MONGO_SERVER_URL in any way, so i suspect this might be coming from https://github.com/google/go-cloud/tree/master/docstore/mongodocstore

@bradbeck
Copy link
Contributor Author

bradbeck commented May 9, 2024

It appears that there may be an issue with go.mongodb.org/mongo-driver v1.13.1. In local testing this version of the driver appears to fail in this way. v1.12.0, v1.13.2, v1.14.0 and v1.15.0 do not appear to fail in this way, at least based on my testing.

@bradbeck
Copy link
Contributor Author

bradbeck commented May 9, 2024

The following assumes there is a MongoDB server running locally with a tekton-chains user and a tekton-chains database with a bar collection defined.

package main

import (
	"context"
	"fmt"

	"go.mongodb.org/mongo-driver/bson"
	"go.mongodb.org/mongo-driver/mongo"
	"go.mongodb.org/mongo-driver/mongo/options"
)

func main() {
	uri := "mongodb://tekton-chains:foo^bar@localhost:27017/?authSource=admin"
	client, err := mongo.Connect(context.TODO(), options.Client().ApplyURI(uri))
	if err != nil {
		panic(err)
	}

	defer func() {
		if err := client.Disconnect(context.TODO()); err != nil {
			panic(err)
		}
	}()

	names, err := client.Database("tekton-chains").ListCollectionNames(context.TODO(), bson.D{})
	if err != nil {
		panic(err)
	}
	for i, name := range names {
		fmt.Printf("%d: %s\n", i, name)
	}
}
$ go get go.mongodb.org/mongo-driver@v1.13.1
...
$ go run main.go                            
panic: parse "mongodb://tekton-chains:foo^bar@localhost:27017/?authSource=admin": net/url: invalid userinfo

goroutine 1 [running]:
main.main()
        /Users/bradbeck/github/bradbeck/mongo-client/main.go:16 +0x214
exit status 2

$ go get go.mongodb.org/mongo-driver@v1.13.2
go: upgraded go.mongodb.org/mongo-driver v1.13.1 => v1.13.2
$ go run main.go   
0: bar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

2 participants