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

Support simultaneous verification of CSR and Challenge Password #115

Closed
wants to merge 3 commits into from

Conversation

directionless
Copy link
Contributor

@directionless directionless commented Nov 6, 2020

Add a WithCombinedVerifier option, and associated interface, to verify the CSR and challenge password together. This allows the password to be a signature on the csr.

Additionally cleanup the logic around verification. This preserves the existing precedence, but should be more clear.

Question for the maintainers: I think it would be valuable in allowing something to modify the CSR before signing. I think there's an obvious hook for it, right here. What do y'all think? Should that be a different interface? Should this become ValidateAndRewrite ?

Add a `WithCombinedVerifier` option, and associated interface, to verify the CSR and challenge password together. This allows the password to be a signature on the csr.

Additionally cleanup the logic around verification. This preserves the existing precedence, but should be more clear.
@directionless
Copy link
Contributor Author

These days, bolt fails tests with -race. So there's some bitrot in the test suite.

@jessepeterson
Copy link
Member

We changed the server service significantly in #113. By wrapping challenge password verification as a chained CSRSigner we can now do both at the same time. Does this address what you were attempting here?

Regarding your second request — modification of the CSR — what do you mean by this? Do you mean doing things to resulting certificate that aren't explicitly set in the CSR? I.e. setting add'l/different attributes and such? That sort of thing I would push towards the CA which, in #113's case, has now been moved into the depot pacakge. But in any case curious what you were thinking here; maybe an example would help.

@jessepeterson
Copy link
Member

I think your first concern is now possible under #113 and your second concern is spoken to in issues like #103. Feel free to re-open this or another PR if not though. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants