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
proposal: crypto/x509: add Context to VerifyOptions #61576
Comments
To me it is confusing to cancel code that do not block on IO. |
Is there any reason for not doing it this (common) way instead? func (c *Certificate) VerifyContext(ctx context.Context, opts VerifyOptions) (chains [][]*Certificate, err error) { Is this going to involve goroutine creation for platform verifiers? Or is this not going to be used there? |
I don't have a clear memory of why we didn't consider VerifyContext, but perhaps @FiloSottile does. I cannot come up with any strong reason against it versus using the VerifyOptions, and it clearly does have significant precedent. For platform verifiers, I don't think we want to spawn goroutines and return early, since the work will still be happening, which may be expensive, and could not reasonably be cancelled. As with a couple of features of the verifiers, at least initially contexts would only be a native verifier only feature. Since contexts are not currently wired into cryptography code (and it seems unlikely we'll do that, at least at this point) the context will only be consulted between chain building operations (i.e. each time we consider a new certificate for addition to an in process chain, essentially at each invocation in a DFS). |
My understanding is that |
Nope, not sure why we went with VerifyOptions.Context instead of VerifyContext. Maybe just because it was there and we're used to adding things to it. VerifyContext sounds straightforwardly like the right choice, though. |
Currently, there is no way to cancel or put a deadline on certificate verification operations, which due to the graph nature of chain building (and the associated signature verifications) can be somewhat expensive.
I propose we add a context.Context to the VerifyOptions struct, which would then be checked during path building. This can then be populated during TLS handshakes using the existing context.
cc @FiloSottile
The text was updated successfully, but these errors were encountered: