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

fix: check peers certificate when using https transport #41

Merged
merged 1 commit into from
Sep 28, 2020

Conversation

fmoessbauer
Copy link
Contributor

@fmoessbauer fmoessbauer commented Sep 27, 2020

Previously, the peers certificate identity was not checked.
By that, man-in-the-middle attacks where possible by using
self signed certificates.

The fix removes the CURLOPT_SSL_VERIFYPEER=0 configuration,
so that the CURL default is used and the certificate is properly
checked.

closes #42

Previously, the peers certificate identity was not checked.
By that, man-in-the-middle attacks where possible by using
self signed certificates.

The fix removes the CURLOPT_SSL_VERIFYPEER=0 configuration,
so that the CURL default is used and the certificate is properly
checked.
@offa offa self-assigned this Sep 28, 2020
@offa offa added this to the next milestone Sep 28, 2020
@offa offa added the security label Sep 28, 2020
@offa
Copy link
Owner

offa commented Sep 28, 2020

The fact that the option needs to be changed on several places indicates the need of some additional refactoring. Also it's a bit misleading, that a method enableSsl() actually disables certificate checking. 😕

Thanks for reporting and fixing this issue. I'll create another release containing the fix.

@offa offa merged commit 7c5ba9e into offa:master Sep 28, 2020
@fmoessbauer
Copy link
Contributor Author

Also it's a bit misleading, that a method enableSsl() actually disables certificate checking. 😕

Yes, I thought exactly the same. I just stumbled upon that by searching for possible performance improvements (like reduction of allocations ...)

@offa
Copy link
Owner

offa commented Sep 28, 2020

PRs are always welcome! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Certificate authenticity is not checked in HTTPS transport
2 participants