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

openssl: if verifypeer is not requested, skip the CA loading #7892

Closed
wants to merge 1 commit into from

Conversation

bagder
Copy link
Member

@bagder bagder commented Oct 22, 2021

It was previously done mostly to show a match/non-match in the verbose
output even when verification was not asked for. This change skips the
loading of the CA certs unless verifypeer is set to save memory and CPU.

It was previously done mostly to show a match/non-match in the verbose
output even when verification was not asked for. This change skips the
loading of the CA certs unless verifypeer is set to save memory and CPU.
@bagder bagder added the TLS label Oct 22, 2021
@bagder bagder closed this in 83393b1 Oct 22, 2021
@bagder bagder deleted the bagder/openssl-verify-skip branch October 22, 2021 14:16
@sjadhavar
Copy link
Contributor

This commit is causing the problem in our application that uses libcurl.

Please see the old comment for more details on this.
#2290 (comment)

IMO, we should revert this commit.

@bagder
Copy link
Member Author

bagder commented Mar 16, 2022

I think a start would be to file an issue explaining the problem.

@pschlan
Copy link
Contributor

pschlan commented May 30, 2022

Interesting (and glad) to see this has been re-added after my previous commit has been reverted. After my PR in #2290, I've added a note in https://github.com/pschlan/curl/blob/master/docs/libcurl/opts/CURLOPT_SSL_VERIFYPEER.3#L61-L68 to document the expensive loading of the ca file despite VERIFYPEER being disabled. Should we remove this note now?

@bagder
Copy link
Member Author

bagder commented May 30, 2022

Oh nice catch @pschlan, yes we should. You wanna do it?

pschlan added a commit to pschlan/curl that referenced this pull request Oct 30, 2022
This note became obsolete since PR curl#7892 (see also discussion
in the PR comments).
pschlan added a commit to pschlan/curl that referenced this pull request Oct 30, 2022
This note became obsolete since PR curl#7892 (see also discussion
in the PR comments).
bagder pushed a commit that referenced this pull request Oct 30, 2022
This note became obsolete since PR #7892 (see also discussion in the PR
comments).

Closes #9832
jay added a commit to jay/curl that referenced this pull request Jan 5, 2023
This commit restores the behavior of CURLSSLOPT_NATIVE_CA so that it
does not override CURLOPT_CAINFO / CURLOPT_CAPATH, or the hardcoded
default locations. Instead the CA store can be used at the same time.

---

This behavior was originally added over two years ago in abbc5d6
(curl#5585) but then 83393b1 (curl#7892) broke it two months ago, I assume
inadvertently.

The CURLSSLOPT_NATIVE_CA feature is marked experimental and likely
rarely used.

Ref: curl#5585
Ref: curl#7892
Ref: https://curl.se/mail/lib-2023-01/0019.html

Closes #xxxx
jay added a commit to jay/curl that referenced this pull request Jan 11, 2023
.. and remove 'experimental' designation from CURLSSLOPT_NATIVE_CA.

This commit restores the behavior of CURLSSLOPT_NATIVE_CA so that it
does not override CURLOPT_CAINFO / CURLOPT_CAPATH, or the hardcoded
default locations. Instead the CA store can be used at the same time.

---

This behavior was originally added over two years ago in abbc5d6
(curl#5585) but then 83393b1 (curl#7892) broke it two months ago, I assume
inadvertently.

The CURLSSLOPT_NATIVE_CA feature was marked experimental and likely
rarely used.

Ref: curl#5585
Ref: curl#7892
Ref: https://curl.se/mail/lib-2023-01/0019.html

Closes #xxxx
jay added a commit that referenced this pull request Jan 17, 2023
.. and remove 'experimental' designation from CURLSSLOPT_NATIVE_CA.

This commit restores the behavior of CURLSSLOPT_NATIVE_CA so that it
does not override CURLOPT_CAINFO / CURLOPT_CAPATH, or the hardcoded
default locations. Instead the native Windows CA store can be used at
the same time.

---

This behavior was originally added over two years ago in abbc5d6
(#5585) but then 83393b1 (#7892) broke it over a year ago, I assume
inadvertently.

The CURLSSLOPT_NATIVE_CA feature was marked experimental and likely
rarely used.

Ref: #5585
Ref: #7892
Ref: https://curl.se/mail/lib-2023-01/0019.html

Closes #10244
bch pushed a commit to bch/curl that referenced this pull request Jul 19, 2023
.. and remove 'experimental' designation from CURLSSLOPT_NATIVE_CA.

This commit restores the behavior of CURLSSLOPT_NATIVE_CA so that it
does not override CURLOPT_CAINFO / CURLOPT_CAPATH, or the hardcoded
default locations. Instead the native Windows CA store can be used at
the same time.

---

This behavior was originally added over two years ago in abbc5d6
(curl#5585) but then 83393b1 (curl#7892) broke it over a year ago, I assume
inadvertently.

The CURLSSLOPT_NATIVE_CA feature was marked experimental and likely
rarely used.

Ref: curl#5585
Ref: curl#7892
Ref: https://curl.se/mail/lib-2023-01/0019.html

Closes curl#10244
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants