From 64d9f161fe6c476fc7a335187527e98d3c668c1c Mon Sep 17 00:00:00 2001 From: Patrick Pelletier Date: Wed, 27 Feb 2013 17:16:27 -0800 Subject: [PATCH] use iSECPartners code to validate hostname in certificate The problem is that if you go to a website whose certificate does not match its hostname, it should fail. Try this in a web browser for https://www.kegel.com/ for example. Your web browser will say the certificate is for *.pair.com, not for www.kegel.com, and won't let you visit it without clicking through a bunch of scary warnings. However, prior to this commit, https-client was happy to fetch https://www.kegel.com/ without complaining. That is bad. Now, with this commit, it will properly complain, which is good: pelletier@chives:~/src/libevent/sample$ ./https-client https://www.kegel.com/ Got 'MatchNotFound' for hostname 'www.kegel.com' and certificate: /C=US/postalCode=15203/ST=Pennsylvania/L=Pittsburgh/street=Suite 210/street=2403 Sidney Street/O=pair Networks, Inc./OU=Provided by pair Networks, Inc./OU=PairWildcardSSL $250,000/CN=*.pair.com some request failed - no idea which one though! error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed ppelletier@chives:~/src/libevent/sample$ It will still succeed for sites with an exactly-matching certificate, such as https://github.com/ and that is also good! However, the problem is that the iSECPartners code doesn't handle wildcards, which means we reject https://ip.appspot.com/ even though it is perfectly legitimate, because we don't understand the wildcard: ppelletier@chives:~/src/libevent/sample$ ./https-client https://ip.appspot.com/ Got 'MatchNotFound' for hostname 'ip.appspot.com' and certificate: /C=US/ST=California/L=Mountain View/O=Google Inc/CN=*.appspot.com some request failed - no idea which one though! error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed ppelletier@chives:~/src/libevent/sample$ So, we need to fix this. In other words, "to be continued..." --- sample/https-client.c | 80 +++++++++++++ sample/include.am | 5 +- sample/openssl_hostname_validation.c | 167 +++++++++++++++++++++++++++ sample/openssl_hostname_validation.h | 56 +++++++++ 4 files changed, 307 insertions(+), 1 deletion(-) create mode 100644 sample/openssl_hostname_validation.c create mode 100644 sample/openssl_hostname_validation.h diff --git a/sample/https-client.c b/sample/https-client.c index e19a791e..d6e5d880 100644 --- a/sample/https-client.c +++ b/sample/https-client.c @@ -36,6 +36,8 @@ #include #include +#include "openssl_hostname_validation.h" + static struct event_base *base; static void @@ -110,6 +112,61 @@ die_openssl(const char *func) exit(1); } +/* See http://archives.seul.org/libevent/users/Jan-2013/msg00039.html */ +static int cert_verify_callback(X509_STORE_CTX *x509_ctx, void *arg) +{ + char cert_str[256]; + const char *host = (const char *) arg; + const char *res_str = "X509_verify_cert failed"; + HostnameValidationResult res = Error; + + /* This is the function that OpenSSL would call if we hadn't called + * SSL_CTX_set_cert_verify_callback(). Therefore, we are "wrapping" + * the default functionality, rather than replacing it. */ + int ok_so_far = X509_verify_cert(x509_ctx); + + X509 *server_cert = X509_STORE_CTX_get_current_cert(x509_ctx); + + if (ok_so_far) { + res = validate_hostname(host, server_cert); + + switch (res) { + case MatchFound: + res_str = "MatchFound"; + break; + case MatchNotFound: + res_str = "MatchNotFound"; + break; + case NoSANPresent: + res_str = "NoSANPresent"; + break; + case MalformedCertificate: + res_str = "MalformedCertificate"; + break; + case Error: + res_str = "Error"; + break; + default: + res_str = "WTF!"; + break; + } + } + + X509_NAME_oneline(X509_get_subject_name (server_cert), + cert_str, sizeof (cert_str)); + + if (res == MatchFound) { + printf("https server '%s' has this certificate, " + "which looks good to me:\n%s\n", + host, cert_str); + return 1; + } else { + printf("Got '%s' for hostname '%s' and certificate:\n%s\n", + res_str, host, cert_str); + return 0; + } +} + int main(int argc, char **argv) { @@ -188,7 +245,30 @@ main(int argc, char **argv) "/etc/ssl/certs/ca-certificates.crt", NULL)) die_openssl("SSL_CTX_load_verify_locations"); + /* Ask OpenSSL to verify the server certificate. Note that this + * does NOT include verifying that the hostname is correct. + * So, by itself, this means anyone with any legitimate + * CA-issued certificate for any website, can impersonate any + * other website in the world. This is not good. See "The + * Most Dangerous Code in the World" article at + * https://crypto.stanford.edu/~dabo/pubs/abstracts/ssl-client-bugs.html + */ SSL_CTX_set_verify(ssl_ctx, SSL_VERIFY_PEER, NULL); + /* This is how we solve the problem mentioned in the previous + * comment. We "wrap" OpenSSL's validation routine in our + * own routine, which also validates the hostname by calling + * the code provided by iSECPartners. Note that even though + * the "Everything You've Always Wanted to Know About + * Certificate Validation With OpenSSL (But Were Afraid to + * Ask)" paper from iSECPartners says very explicitly not to + * call SSL_CTX_set_cert_verify_callback (at the bottom of + * page 2), what we're doing here is safe because our + * cert_verify_callback() calls X509_verify_cert(), which is + * OpenSSL's built-in routine which would have been called if + * we hadn't set the callback. Therefore, we're just + * "wrapping" OpenSSL's routine, not replacing it. */ + SSL_CTX_set_cert_verify_callback (ssl_ctx, cert_verify_callback, + (void *) host); // Create event base base = event_base_new(); diff --git a/sample/include.am b/sample/include.am index 91d28352..de0a53dc 100644 --- a/sample/include.am +++ b/sample/include.am @@ -19,9 +19,12 @@ sample_le_proxy_LDADD = libevent.la libevent_openssl.la ${OPENSSL_LIBS} ${OPENSS sample_le_proxy_INCLUDES = $(OPENSSL_INCS) SAMPLES += sample/https-client -sample_https_client_SOURCES = sample/https-client.c +sample_https_client_SOURCES = \ + sample/https-client.c \ + sample/openssl_hostname_validation.c sample_https_client_LDADD = libevent.la libevent_openssl.la ${OPENSSL_LIBS} ${OPENSSL_LIBADD} sample_https_client_INCLUDES = $(OPENSSL_INCS) +noinst_HEADERS += sample/openssl_hostname_validation.h endif noinst_PROGRAMS += $(SAMPLES) diff --git a/sample/openssl_hostname_validation.c b/sample/openssl_hostname_validation.c new file mode 100644 index 00000000..94f0c0eb --- /dev/null +++ b/sample/openssl_hostname_validation.c @@ -0,0 +1,167 @@ +/* Obtained from: https://github.com/iSECPartners/ssl-conservatory */ + +/* +Copyright (C) 2012, iSEC Partners. + +Permission is hereby granted, free of charge, to any person obtaining a copy of +this software and associated documentation files (the "Software"), to deal in +the Software without restriction, including without limitation the rights to +use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies +of the Software, and to permit persons to whom the Software is furnished to do +so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. + */ + +/* + * Helper functions to perform basic hostname validation using OpenSSL. + * + * Please read "everything-you-wanted-to-know-about-openssl.pdf" before + * attempting to use this code. This whitepaper describes how the code works, + * how it should be used, and what its limitations are. + * + * Author: Alban Diquet + * License: See LICENSE + * + */ + + +#include +#include + +#include "openssl_hostname_validation.h" + + +#define HOSTNAME_MAX_SIZE 255 + +/** +* Tries to find a match for hostname in the certificate's Common Name field. +* +* Returns MatchFound if a match was found. +* Returns MatchNotFound if no matches were found. +* Returns MalformedCertificate if the Common Name had a NUL character embedded in it. +* Returns Error if the Common Name could not be extracted. +*/ +static HostnameValidationResult matches_common_name(const char *hostname, const X509 *server_cert) { + int common_name_loc = -1; + X509_NAME_ENTRY *common_name_entry = NULL; + ASN1_STRING *common_name_asn1 = NULL; + char *common_name_str = NULL; + + // Find the position of the CN field in the Subject field of the certificate + common_name_loc = X509_NAME_get_index_by_NID(X509_get_subject_name((X509 *) server_cert), NID_commonName, -1); + if (common_name_loc < 0) { + return Error; + } + + // Extract the CN field + common_name_entry = X509_NAME_get_entry(X509_get_subject_name((X509 *) server_cert), common_name_loc); + if (common_name_entry == NULL) { + return Error; + } + + // Convert the CN field to a C string + common_name_asn1 = X509_NAME_ENTRY_get_data(common_name_entry); + if (common_name_asn1 == NULL) { + return Error; + } + common_name_str = (char *) ASN1_STRING_data(common_name_asn1); + + // Make sure there isn't an embedded NUL character in the CN + if (ASN1_STRING_length(common_name_asn1) != strlen(common_name_str)) { + return MalformedCertificate; + } + + // Compare expected hostname with the CN + if (strcasecmp(hostname, common_name_str) == 0) { + return MatchFound; + } + else { + return MatchNotFound; + } +} + + +/** +* Tries to find a match for hostname in the certificate's Subject Alternative Name extension. +* +* Returns MatchFound if a match was found. +* Returns MatchNotFound if no matches were found. +* Returns MalformedCertificate if any of the hostnames had a NUL character embedded in it. +* Returns NoSANPresent if the SAN extension was not present in the certificate. +*/ +static HostnameValidationResult matches_subject_alternative_name(const char *hostname, const X509 *server_cert) { + HostnameValidationResult result = MatchNotFound; + int i; + int san_names_nb = -1; + STACK_OF(GENERAL_NAME) *san_names = NULL; + + // Try to extract the names within the SAN extension from the certificate + san_names = X509_get_ext_d2i((X509 *) server_cert, NID_subject_alt_name, NULL, NULL); + if (san_names == NULL) { + return NoSANPresent; + } + san_names_nb = sk_GENERAL_NAME_num(san_names); + + // Check each name within the extension + for (i=0; itype == GEN_DNS) { + // Current name is a DNS name, let's check it + char *dns_name = (char *) ASN1_STRING_data(current_name->d.dNSName); + + // Make sure there isn't an embedded NUL character in the DNS name + if (ASN1_STRING_length(current_name->d.dNSName) != strlen(dns_name)) { + result = MalformedCertificate; + break; + } + else { // Compare expected hostname with the DNS name + if (strcasecmp(hostname, dns_name) == 0) { + result = MatchFound; + break; + } + } + } + } + sk_GENERAL_NAME_pop_free(san_names, GENERAL_NAME_free); + + return result; +} + + +/** +* Validates the server's identity by looking for the expected hostname in the +* server's certificate. As described in RFC 6125, it first tries to find a match +* in the Subject Alternative Name extension. If the extension is not present in +* the certificate, it checks the Common Name instead. +* +* Returns MatchFound if a match was found. +* Returns MatchNotFound if no matches were found. +* Returns MalformedCertificate if any of the hostnames had a NUL character embedded in it. +* Returns Error if there was an error. +*/ +HostnameValidationResult validate_hostname(const char *hostname, const X509 *server_cert) { + HostnameValidationResult result; + + if((hostname == NULL) || (server_cert == NULL)) + return Error; + + // First try the Subject Alternative Names extension + result = matches_subject_alternative_name(hostname, server_cert); + if (result == NoSANPresent) { + // Extension was not found: try the Common Name + result = matches_common_name(hostname, server_cert); + } + + return result; +} diff --git a/sample/openssl_hostname_validation.h b/sample/openssl_hostname_validation.h new file mode 100644 index 00000000..54aa1c43 --- /dev/null +++ b/sample/openssl_hostname_validation.h @@ -0,0 +1,56 @@ +/* Obtained from: https://github.com/iSECPartners/ssl-conservatory */ + +/* +Copyright (C) 2012, iSEC Partners. + +Permission is hereby granted, free of charge, to any person obtaining a copy of +this software and associated documentation files (the "Software"), to deal in +the Software without restriction, including without limitation the rights to +use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies +of the Software, and to permit persons to whom the Software is furnished to do +so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. + */ + +/* + * Helper functions to perform basic hostname validation using OpenSSL. + * + * Please read "everything-you-wanted-to-know-about-openssl.pdf" before + * attempting to use this code. This whitepaper describes how the code works, + * how it should be used, and what its limitations are. + * + * Author: Alban Diquet + * License: See LICENSE + * + */ + +typedef enum { + MatchFound, + MatchNotFound, + NoSANPresent, + MalformedCertificate, + Error +} HostnameValidationResult; + +/** +* Validates the server's identity by looking for the expected hostname in the +* server's certificate. As described in RFC 6125, it first tries to find a match +* in the Subject Alternative Name extension. If the extension is not present in +* the certificate, it checks the Common Name instead. +* +* Returns MatchFound if a match was found. +* Returns MatchNotFound if no matches were found. +* Returns MalformedCertificate if any of the hostnames had a NUL character embedded in it. +* Returns Error if there was an error. +*/ +HostnameValidationResult validate_hostname(const char *hostname, const X509 *server_cert);