-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
client_auth: wire up leaf verifier Caddyfile #6772
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Mohammed Al Sahaf <[email protected]>
if !strings.HasPrefix(modName, "load_") { | ||
return d.Err("expected a leaf certificate loader module name prefixed with `load_`") | ||
} | ||
modName = strings.TrimPrefix(modName, "load_") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@francislavoie, what do you think of this from Caddyfile UX perspective?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need load_
at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought I should future-proof it in case other parameters are added besides the loaders. I thought the namespace will be clobbered if I don't distinguish them with a prefix or nest them into a block. I don't know...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The module name is already prefixed by tls.leaf_cert_loader.
. Are there other "kinds" (?) of modules that go in this namespace that aren't loading?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be like verifier leaf file <filename>
etc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm saying what if leaf
has more parameters in the future, not the loader modules. Anyways, looking at what VerifyClientCertificate
does, I can't imagine any new behavioral parameters that may be added in the future.
The format verifier leaf file <filename>
doesn`t work because it blocks the use multiple loaders, which is currently possible. In other words, this is now possible:
verifier leaf {
file file-1.pem file-2.pem
file more-cert.pem
folder /path/to/cert/repo
}
The one-liner disallows this use.
Closes #6771