Null Pointer Deref on address config

sundbry commented 9 months ago

Hi @kmott, I was giving your module a try, and ran into this crash:

{"level":"info","ts":1700038283.9390812,"msg":"using provided configuration","config_file":"/etc/caddy/Caddyfile","config_adapter":"caddyfile"}
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x15f696b]

goroutine 1 [running]:
github.com/mywordpress-io/caddy-vault-storage.(*Storage).UnmarshalCaddyfile(0xc0005880b0, 0xc0003b8f60)
        /go/pkg/mod/github.com/mywordpress-io/caddy-vault-storage@v0.0.0-20230517195640-dcd95157cf03/main.go:50 +0xeb
github.com/caddyserver/caddy/v2/caddyconfig/caddyfile.UnmarshalModule(0x7fce8cdc95b8?, {0xc0005164c8, 0x13})
        /go/pkg/mod/github.com/caddyserver/caddy/v2@v2.7.5/caddyconfig/caddyfile/adapter.go:141 +0x1b4
github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile.parseOptStorage(0xc0003b8f30, {0xc0003b8d80?, 0xc0003cc560?})
        /go/pkg/mod/github.com/caddyserver/caddy/v2@v2.7.5/caddyconfig/httpcaddyfile/options.go:180 +0xb7
github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile.ServerType.evaluateGlobalOptionsBlock({}, {0xc0003d2180?, 0x2, 0x2}, 0x0?)
        /go/pkg/mod/github.com/caddyserver/caddy/v2@v2.7.5/caddyconfig/httpcaddyfile/httptype.go:363 +0x1d5
github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile.ServerType.Setup({}, {0xc00048ca80, 0x2, 0xc0003b8e10?}, 0xc0003b8d80)
        /go/pkg/mod/github.com/caddyserver/caddy/v2@v2.7.5/caddyconfig/httpcaddyfile/httptype.go:79 +0x2ab
github.com/caddyserver/caddy/v2/caddyconfig/caddyfile.Adapter.Adapt({{0x1eb1ec0?, 0x2bacbc0?}}, {0xc0003f6000, 0x29d, 0x29e}, 0xc0003e6120?)
        /go/pkg/mod/github.com/caddyserver/caddy/v2@v2.7.5/caddyconfig/caddyfile/adapter.go:50 +0x12d
github.com/caddyserver/caddy/v2/cmd.loadConfigWithLogger(0xc00032aa80, {0x7ffe6a1a2bd9, 0x14}, {0x7ffe6a1a2bf8, 0x9})
        /go/pkg/mod/github.com/caddyserver/caddy/v2@v2.7.5/cmd/main.go:166 +0x6cd
github.com/caddyserver/caddy/v2/cmd.LoadConfig({0x7ffe6a1a2bd9, 0x14}, {0x7ffe6a1a2bf8, 0x9})
        /go/pkg/mod/github.com/caddyserver/caddy/v2@v2.7.5/cmd/main.go:98 +0x45
github.com/caddyserver/caddy/v2/cmd.cmdRun({0x0?})
        /go/pkg/mod/github.com/caddyserver/caddy/v2@v2.7.5/cmd/commandfuncs.go:214 +0x593
github.com/caddyserver/caddy/v2/cmd.init.1.func2.WrapCommandFuncForCobra.func1(0xc000005200, {0x19cb1d6?, 0x4?, 0x19cb1b2?})
        /go/pkg/mod/github.com/caddyserver/caddy/v2@v2.7.5/cmd/cobra.go:137 +0x2f
github.com/spf13/cobra.(*Command).execute(0xc000005200, {0xc000511d80, 0x4, 0x4})
        /go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:940 +0x87c
github.com/spf13/cobra.(*Command).ExecuteC(0x2b5b300)
        /go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:1068 +0x3a5
github.com/spf13/cobra.(*Command).Execute(...)
        /go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:992
github.com/caddyserver/caddy/v2/cmd.Main()
        /go/pkg/mod/github.com/caddyserver/caddy/v2@v2.7.5/cmd/main.go:66 +0x5b
main.main()
        /usr/src/caddy/cmd/caddylb/main.go:40 +0xf

My config looks like this,

{
  storage vault "https://vault.mydomain.net:8201" {
    token "redacted"
    secrets_path "kv"
    path_prefix "caddy"
  }
}
mydomain.com {
  ...
}

I am not sure why it's failing to parse that URL.

sundbry commented 9 months ago

I was able to hack the code up enough to verify that the address parameter is not nil. It looks to me like the problem is the ParseURL function returns a reference to a URL from the stack rather than a value here, but I am not ordinarily a go programmer so I'm not entirely sure if that's valid or not.

kmott commented 9 months ago

Great catch @sundbry! I am not even sure how I had this working initially (it's been running in production since May)--I am guessing it's because I am only using the JSON config for my Caddy deployments, so I never caught this specific error when using Caddyfile.

I have 2 MR's listed below that I think fix the problem and do a bunch of cleanup that was overdue. I also added some additional tests, bumped to Caddy v2.75, and refactored some interfaces.

If the MR(s) look okay, I will cut a release for them tomorrow, and your xcaddy builds should work again when it pulls in this plugin.

sundbry commented 9 months ago

@kmott #4 Looks good to me! Nice job on the test coverage.

kmott commented 9 months ago

FYI @sundbry new tagged version is available v0.1.0, and should fix the issues you had. Thank you again for reporting the issue!

mywordpress-io / caddy-vault-storage

Null Pointer Deref on address config #1