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

mbedTLS backend should use CURLOPT_SSL_VERIFYHOST to control CN checking #3376

Closed
rosenqui opened this issue Dec 15, 2018 · 5 comments
Closed

mbedTLS backend should use CURLOPT_SSL_VERIFYHOST to control CN checking #3376

rosenqui opened this issue Dec 15, 2018 · 5 comments
Labels

Comments

@rosenqui
Copy link

@rosenqui rosenqui commented Dec 15, 2018

operating system

All platforms for mbedTLS

curl/libcurl version

All versions

The certificate validation code for the mbedTLS backend should look at the CURLOPT_SSL_VERIFYHOST option via SSL_CONN_CONFIG(verifyhost) to control if CN checking is done as part of server certification validation.

As it stands now, it's impossible to validate the certificate but omit the hostname checks when using the mbedTLS backend. This is possible with other backends like OpenSSL.

https://github.com/curl/curl/blob/master/lib/vtls/mbedtls.c#L586

@bagder bagder added the SSL/TLS label Dec 17, 2018
bagder added a commit that referenced this issue Dec 17, 2018
Previously, VERIFYPEER would enable/disable all checks.

Fixes #3376
@bagder
Copy link
Member

@bagder bagder commented Dec 17, 2018

@rosenqui can you check if my fix works for you?

@rosenqui
Copy link
Author

@rosenqui rosenqui commented Dec 17, 2018

I should have time to try it out later today.

Thanks for the quick fix!

@rosenqui
Copy link
Author

@rosenqui rosenqui commented Dec 17, 2018

Fix looks good - I commented in #3382

@bagder bagder closed this in f097669 Dec 17, 2018
@bagder bagder reopened this Dec 18, 2018
@rosenqui
Copy link
Author

@rosenqui rosenqui commented Dec 18, 2018

This looks to be the simplest solution - same code as before with a check of verifyhost first.

  ret = mbedtls_ssl_get_verify_result(&BACKEND->ssl);

  if(!SSL_CONN_CONFIG(verifyhost))
    ret &= ~MBEDTLS_X509_BADCERT_CN_MISMATCH;   /* Ignore hostname errors if verifyhost is disabled */
  if(ret && SSL_CONN_CONFIG(verifypeer)) {
    if(ret & MBEDTLS_X509_BADCERT_EXPIRED)
      failf(data, "Cert verify failed: BADCERT_EXPIRED");

    if(ret & MBEDTLS_X509_BADCERT_REVOKED) {
      failf(data, "Cert verify failed: BADCERT_REVOKED");
      return CURLE_SSL_CACERT;
    }

    if (ret & MBEDTLS_X509_BADCERT_CN_MISMATCH)
      failf(data, "Cert verify failed: BADCERT_CN_MISMATCH");

    if(ret & MBEDTLS_X509_BADCERT_NOT_TRUSTED)
      failf(data, "Cert verify failed: BADCERT_NOT_TRUSTED");

    return CURLE_PEER_FAILED_VERIFICATION;
  }

  peercert = mbedtls_ssl_get_peer_cert(&BACKEND->ssl);

Turn off the MBEDTLS_X509_BADCERT_CN_MISMATCH error bit if verifyhost is disabled, but otherwise do the same as before. We don't want to fail if verifyhost is on and verifypeer is off since the latter is the master kill switch for validation.

bagder added a commit that referenced this issue Dec 19, 2018
Fix-by: Eric Rosenquist

Fixes #3376
@bagder
Copy link
Member

@bagder bagder commented Dec 19, 2018

Thanks, like #3390 ?

bagder added a commit that referenced this issue Dec 19, 2018
Fix-by: Eric Rosenquist

Fixes #3376
@bagder bagder closed this in 0b9fadf Dec 20, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Mar 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.