- Notifications
You must be signed in to change notification settings - Fork78
Add hostname validation feature#23
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
schmidtw commentedMar 28, 2017
@francisbrosnan This change looks like a solid & needed improvement to eliminate a discovered "man in the middle" attack vector. Do you have any concerns merging it? |
francisbrosnan commentedApr 5, 2017
Hello Selvam, Sorry for the delay. I was looking for a moment to thank you for the work and effort done (you have even created a regression test) and also to send you some notes to review some issues that the patch has. Main concern is copyright assignment: accepting the patch as it is will create us problems with some From a technical perspective, there are several issues with the naming (ie. functions like hostmatch or inet_pton4 will clash with other's code for sure given noPoll's library nature and C lack of namespaces). Code convention is another issue (under this subject there is work to do). Besides, there are several code sections that will require checking/valgrinding to ensure we don't introduce problems inside base code that can be triggered with carefully crafted input...that I'm sure you have tested but I need to review them. I'll try to review all these things and reply you as soon as possible, |
Fixed build on Mac.
As per openssl wiki pagehttps://wiki.openssl.org/index.php/Hostname_validation, openssl version prior to 1.0.2 did not perform hostname validation. So users of nopoll should handle the hostname validation part during conn handshake in the client end.
This code can help users to enable/disable hostname validation feature based on the options they are providing during the tls connect. As we could see in the computer security this man in the middle attack is most common security threats faced by the current world, So it is better to enable hostname validation by default and we did the same here. And also user may not be aware of this hostname validation is not handled in their openssl in case if they are using the openssl version < 1.0.2.