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

Revert "openssl: Don't add verify locations when verifypeer==0" #2451

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@malhotrag
Contributor

malhotrag commented Apr 3, 2018

This reverts commit dc85437.

libcurl (with the OpenSSL backend) performs server certificate verification
even if verifypeer == 0 and the verification result is available using
CURLINFO_SSL_VERIFYRESULT. The commit that is being reverted caused the
CURLINFO_SSL_VERIFYRESULT to not have useful information for the
verifypeer == 0 use case (it would always have
X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY).

Revert "openssl: Don't add verify locations when verifypeer==0"
This reverts commit dc85437.

libcurl (with the OpenSSL backend) performs server certificate verification
even if verifypeer == 0 and the verification result is available using
CURLINFO_SSL_VERIFYRESULT. The commit that is being reverted caused the
CURLINFO_SSL_VERIFYRESULT to not have useful information for the
verifypeer == 0 use case (it would always have
X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY).
@malhotrag

This comment has been minimized.

Contributor

malhotrag commented Apr 3, 2018

See comment on #2290 for more details

@bagder

This comment has been minimized.

Member

bagder commented Apr 4, 2018

In the mean time, it seems the macos libressl build uses libressl 2.7.2 that causes a build error if this gets reverted. We should probably try to merge #2447 first then...

@malhotrag

This comment has been minimized.

Contributor

malhotrag commented Apr 4, 2018

Sorry, I'm a little confused by the comment. Are you saying the revert causes the build failure? As I understand it, the build failure is an existing issue that #2447 is attempting to fix.

I also noticed a per-existing fuzzer failure. https://travis-ci.org/curl/curl/jobs/361624340
Is this something that needs more investigation? I could take a look if required.

@bagder

This comment has been minimized.

Member

bagder commented Apr 6, 2018

The fuzz fail was fixed already in 82dfdac

@bagder

This comment has been minimized.

Member

bagder commented Apr 6, 2018

Thanks for this!

@bagder bagder closed this in 2536e24 Apr 6, 2018

@lock lock bot locked as resolved and limited conversation to collaborators Jul 5, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.