Implement SSL_CONF_cmd VerifyCAPath and VerifyCADir commands
#33
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ctz
reviewed
Jun 26, 2024
Most of these will require implementing more logic elsewhere before we can add a supported command.
cpu
force-pushed
the
cpu-ssl-conf-verify-bundles
branch
from
01c520a
to
34217a2
Compare
ctz
approved these changes
Jun 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This branch follows #29, #31 and #32 extending the existing
SSL_CONF_xxxAPI to support the "VerifyCAPath" and "VerifyCADir" sub-commands. Seeman 3 SSL_CONF_cmdfor more information. We have no support for the equivalent ofCAPathandCADir(a root store for just path building, but not verification?) and so these related commands are not implemented.We diverge slightly from the upstream here by re-using the existing
SSL_CTX_set_default_verify_fileandSSL_CTX_set_default_verify_dirlogic. Taking this approach requires the minimum amount of fuss to support these configuration commands, but means we error later in the case of the file/directory being bogus - at the time we build a connection and a root trust store. Relatedly, we don't have all the pieces needed to support setting these paths for aSSL, just aSSL_CTX, so we skip implementing the commands afterSSL_CONF_CTX_set_ssl- in practice Nginx only usesSSL_CONF_CTX_set_ssl_ctxso we can punt here for the time being.This is the last of the
SSL_CONF_cmdcommands we can support without more unrelated compat work, so I've tacked on a commit calling out some of the cmds that might be worth implementing in the future after that work is completed.Closes #22