httpcaddyfile: Support client auth verifiers

[zachgalvin]

This PR adds a case statement to the Caddyfile adapter that parses the TLS section to allow client auth verifiers. Closes #6018

[CLAassistant]

All committers have signed the CLA.

[francislavoie]

Thanks, LGTM!

Please make sure to sign the CLA so that we can accept the code 👍

You may need to add the email you used to commit to your GitHub account, or edit the commit to use your GitHub account's email & force push.

[zachgalvin]

Awesome! Yeah, I had to update the email on my commit. Should be good to go now though!

[francislavoie]

Actually I think the JSON part needs to be like:

cp.ClientAuthentication.VerifiersRaw = append(
	cp.ClientAuthentication.VerifiersRaw,
	caddyconfig.JSONModuleObject(unm, "name", vType, h.warnings),
)

[zachgalvin]

How come we want to add the "name" key with the module ID as the value to the marshalled value? Is that used else where in the code? I can test that out though. I think I would remove the following right above that too since we will marshal it in the JSONModuleObject function, so no need to Marshall before that:

bytes, err := json.Marshal(ccv)
if err != nil {
	 return nil, err
}

[francislavoie]

That's how Caddy can decode JSON config and know which module to decode the config into. It needs to match the namespace declared in the JSON tag on the VerifiersRaw struct field.

[zachgalvin]

I may be misunderstanding here, but isn't all of that handled in the UnmarshalModule() method? We create a new module in that method, and then we parse the config with that module in that method, so I'm not sure why we would need the name of the module as key value pair in the map in VerifiersRaw later on.

I also tested that out, and I got the following error:
Error: loading initial config: loading new config: loading http app module: provision http: server srv0: setting up TLS connection policies: loading client cert verifiers: position 0: loading module 'revocation': decoding module config: tls.client_auth.revocation: json: unknown field "name"

[francislavoie]

The job of the Caddyfile adapter is to produce a valid JSON config, ultimately. So the module name needs to exist somewhere in the JSON config for it to be able to be loaded at runtime.

The Caddyfile adapter produces JSON, then that JSON is loaded to run. The config structs produced by Caddyfile unmarshaling are not used to run Caddy, only to produce a JSON config.

My code snippet might have been wrong, but plenty of other places that accept modules as input use this pattern. I don't have a verifier plugin on hand to test this.

[francislavoie]

Ah sorry, I misread VerifiersRaw, it has inline_key=verifier so I think "name" -> "verifier". So in the JSON, the config should look something like:

"verifiers": [
	{
		"verifier": "revocation",
		... the rest of that module's config
	},
	{
		"verifier": "another_imaginary_verifier_module",
		... the rest of that module's config
	}
]

[mohammed90]

If you add caddyfile_adapt test under caddytest/integration/caddyfile_adapt, you'll have feedback on the gap between the implementation of this PR and the expected JSON.

[mholt]

This change is looking good! Let me know when you're ready to merge.

[zachgalvin]

Alright, can you take another look? That makes sense, so I updated it to add the verifier key with the name of the module as the value, and it worked! I also dumped the JSON config to confirm that the verifier key matched the struct tag and your example above.

[francislavoie]

Linter is failing, need to run gci, apparently.

[zachgalvin]

I ran the following gci command based on the failed pipelines output:
gci write caddyconfig/httpcaddyfile/builtins.go --skip-generated -s standard -s default -s "prefix(github.com/caddy
server/caddy/v2/cmd)" -s "prefix(github.com/caddyserver/caddy)" --custom-order

It removed and then added an empty line, so not sure why that was an issue, but looks like it fixed it!

[mholt]

The diff was whitespace (unnecessary indentation). Looking good now!