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.

Sign up for GitHub

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

Jump to bottom

wolfssl: if CURLOPT_CAINFO_BLOB is set, ignore the CA files #11884

Closed
wants to merge 1 commit into from
Closed

wolfssl: if CURLOPT_CAINFO_BLOB is set, ignore the CA files #11884

wants to merge 1 commit into from

Conversation

bagder
Copy link
Member

@bagder bagder commented Sep 19, 2023

Ref: #11883
Reported-by: Michael Osipov

@bagder bagder added the TLS label Sep 19, 2023
@michael-o
Copy link
Contributor

Wouldn't it make sense to move conn_config->CAfile and conn_config->CApath to separate vars to have consistency with OpenSSL provider?

@michael-o
Copy link
Contributor

michael-o commented Sep 19, 2023 •
edited

Stencil:

curl/lib/vtls/openssl.c

Lines 3013 to 3018 in becd1e8

const struct curl_blob *ca_info_blob = conn_config->ca_info_blob;
const char * const ssl_cafile =
/* CURLOPT_CAINFO_BLOB overrides CURLOPT_CAINFO */
(ca_info_blob ? NULL : conn_config->CAfile);
const char * const ssl_capath = conn_config->CApath;
const char * const ssl_crlfile = ssl_config->primary.CRLfile;

@bagder
Copy link
Member Author

bagder commented Sep 19, 2023

Wouldn't it make sense to move conn_config->CAfile and conn_config->CApath to separate vars to have consistency with OpenSSL provider?

I wanted to just fix the issue, not rearrange the code to make it look more similar to openssl.c.

@michael-o
Copy link
Contributor

michael-o commented Sep 19, 2023 •
edited

Wouldn't it make sense to move conn_config->CAfile and conn_config->CApath to separate vars to have consistency with OpenSSL provider?

I wanted to just fix the issue, not rearrange the code to make it look more similar to openssl.c.

I see, it still looks weird compared to OpenSSL although the behavior is almost identical, but not the same.

@bagder
Copy link
Member Author

bagder commented Sep 19, 2023

If my PR and mission had been to align the look of the two source files, then the changes would have addressed that. Now I'm addressing the mentioned issues and then I think it is out of scope to clean up unrelated areas.

@bagder bagder closed this in 8420fef Sep 19, 2023
@bagder bagder deleted the bagder/wolfssl-ca-blob branch September 19, 2023 11:00
@michael-o michael-o mentioned this pull request Sep 20, 2023
Closed
ptitSeb pushed a commit to wasix-org/curl that referenced this pull request Sep 25, 2023
@bagder @ptitSeb
wolfssl: if CURLOPT_CAINFO_BLOB is set, ignore the CA files
f6f3078 
Ref: curl#11883
Reported-by: Michael Osipov
Closes curl#11884
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
TLS
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@bagder @michael-o