^ in MONGO_SERVER_URL password causes null pointer panic

[bradbeck]

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

[concaf]

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]

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]

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]

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