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

WiFiClientSecure verify can't match Cert Name (FIX provided) #2978

Closed
J6B opened this issue Feb 17, 2017 · 2 comments
Closed

WiFiClientSecure verify can't match Cert Name (FIX provided) #2978

J6B opened this issue Feb 17, 2017 · 2 comments

Comments

@J6B
Copy link

J6B commented Feb 17, 2017

During my HTTPS client tests, I found a bug into verify (now into _verifyDN) function of WiFiClientSecure.cpp

I have a webserver with a certificate with a CN that contains capital letters like : sub.TOTO.com
(from a private PKI it's easy to get one like this)

The problem is that domain_name is convert into String and then lowered.
But common_name got from certificate is not lowered so they will never match.

FIX :

bool WiFiClientSecure::_verifyDN(const char* domain_name)
{
DEBUGV("domain name: '%s'\r\n", (domain_name)?domain_name:"(null)");
String domain_name_str(domain_name);
domain_name_str.toLowerCase();

const char* san = NULL;
int i = 0;
while ((san = ssl_get_cert_subject_alt_dnsname(*_ssl, i)) != NULL) {
    if (matchName(String(san), domain_name_str)) {
        return true;
    }
    DEBUGV("SAN %d: '%s', no match\r\n", i, san);
    ++i;
}
const char* common_name = ssl_get_cert_dn(*_ssl, SSL_X509_CERT_COMMON_NAME);
String common_name_str(common_name);
common_name_str.toLowerCase();
if (common_name && matchName(common_name_str, domain_name_str)) {
    return true;
}
DEBUGV("CN: '%s', no match\r\n", (common_name)?common_name:"(null)");

return false;

}

@dandandandandandandan
Copy link

I tested this fix with previously working servers and it still works with them. (Since most of the SSL ciphers are not supported by the ESP8266 it is not that easy to find a server for testing, i could connect to twitter.com, api.github.com and paypal.com but not much else)

@igrr igrr added this to the 2.4.0 milestone May 6, 2017
@igrr igrr added this to Pending in 2.4.0 May 6, 2017
@igrr igrr moved this from Pending to To Do in 2.4.0 May 15, 2017
igrr added a commit that referenced this issue May 21, 2017
Some websites have certificates with uppercase letters in CN. This change
makes _verifyDN function accept such certificates by converting all names
to lower case before comparing them.

Resolves #2978
@igrr igrr moved this from To Do to In Progress in 2.4.0 May 21, 2017
@igrr
Copy link
Member

igrr commented May 22, 2017

PR #3271 includes this fix.

@igrr igrr closed this as completed in f6d232f May 22, 2017
@igrr igrr reopened this May 22, 2017
@igrr igrr moved this from In Progress to Done in 2.4.0 May 22, 2017
@igrr igrr closed this as completed Jan 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
2.4.0
Done
Development

No branches or pull requests

3 participants