tls/http: add certificate chain setters #1125
Merged
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
maximilianfridrich
force-pushed
the
tls_cert_chain_setters
branch
2 times, most recently
from
17fe5ad
to
4cc622b
Compare
alfredh
reviewed
May 23, 2024
| @@ -169,6 +169,9 @@ int http_client_set_cert(struct http_cli *cli, const char *path); | |||
| int http_client_set_certpem(struct http_cli *cli, const char *pem); | |||
| int http_client_set_key(struct http_cli *cli, const char *path); | |||
| int http_client_set_keypem(struct http_cli *cli, const char *pem); | |||
| int http_client_use_chain(struct http_cli *cli, const char *path); | |||
| int http_client_use_chainpem(struct http_cli *cli, const char *chain, | |||
| int len_chain); | |||
There was a problem hiding this comment.
Please use size_t len_chain which is consistant with the API ...
There was a problem hiding this comment.
I wanted to prevent the cast from size_t to int when calling BIO_new_mem_buf, but consistency makes sense. Done.
alfredh
reviewed
May 23, 2024
src/http/client.c
Outdated
| if (!cli || !cli->tls) | ||
| return EINVAL; | ||
|
|
||
| err = tls_set_certificate_chain_pem(cli->tls, chain, len_chain); |
There was a problem hiding this comment.
use C99 style declare and assign here ... int err = ...
alfredh
reviewed
May 23, 2024
src/tls/openssl/tls.c
Outdated
| X509 *cert = NULL; | ||
| X509 *leaf_cert = NULL; | ||
| int err = ENOMEM; | ||
| long ok = 0; |
There was a problem hiding this comment.
Is it possible to move the declaration of some of these variables further down, to the assignment ?
|
In general it looks good. some minor comments ... |
maximilianfridrich
force-pushed
the
tls_cert_chain_setters
branch
from
4cc622b
to
fcbb412
Compare
|
Thank you for the review! @alfredh |
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.
The current setters (e.g.
tls_set_certificate) only allow single certificates to be set. Now the whole chain can be set.