Changeset 233919 in webkit


Ignore:
Timestamp:
Jul 18, 2018 11:35:16 AM (6 years ago)
Author:
Basuke Suzuki
Message:

[Curl] Disable CURLOPT_SSL_VERIFYPEER and CURLOPT_SSL_VERIFYHOST specified by setAllowsAnyHTTPSCertificate.
​https://bugs.webkit.org/show_bug.cgi?id=187611

Reviewed by Fujii Hironori.

Current interface for TLS certificate validation for Curl port are as follows:

  • WEBCORE_EXPORT void setHostAllowsAnyHTTPSCertificate(const String&);
  • bool isAllowedHTTPSCertificateHost(const String&);
  • bool canIgnoredHTTPSCertificate(const String&, const Vector<CertificateInfo::Certificate>&);

First one registers a host to be ignored for any certificate check. Once it is registered, no
further certificate validation check is executed.
Second one checks the host is registered in the list above.
Third one is weird. The method signature implies it checks the certificate for the host and detect
whether we can ignore this certificate for the host, but actually it just check only the host and
register the certificate into the vector. Then in the next request for the host, the certificate
will be checked with the previously stored certificate.

It's hard to understand, but in short,

  • We can register a host as an exception for any TLS certificate validation.
  • But only certificate arrived first is ignored, not any certificates for the host (which is rare, but possible for mis configured web cluster).

This behavior is incomplete. To ignore any certificates of the host, these two methods are enough:

  • void allowAnyHTTPSCertificatesForHost(const String&)
  • bool canIgnoreAnyHTTPSCertificatesForHost(const String&)

No new tests. Covered by existing tests.

  • platform/network/curl/CertificateInfo.h:
  • platform/network/curl/CurlContext.cpp:

(WebCore::CurlHandle::enableSSLForHost): Ignore TLS verification for registered host.

  • platform/network/curl/CurlSSLHandle.cpp:

(WebCore::CurlSSLHandle::allowAnyHTTPSCertificatesForHost): Added.
(WebCore::CurlSSLHandle::canIgnoreAnyHTTPSCertificatesForHost const): Ditto.
(WebCore::CurlSSLHandle::setClientCertificateInfo): Separate lock.
(WebCore::CurlSSLHandle::getSSLClientCertificate const): Ditto and add const.
(WebCore::CurlSSLHandle::setHostAllowsAnyHTTPSCertificate): Deleted.
(WebCore::CurlSSLHandle::isAllowedHTTPSCertificateHost): Deleted.
(WebCore::CurlSSLHandle::canIgnoredHTTPSCertificate): Deleted.
(WebCore::CurlSSLHandle::getSSLClientCertificate): Deleted.

  • platform/network/curl/CurlSSLHandle.h:
  • platform/network/curl/CurlSSLVerifier.cpp:

(WebCore::CurlSSLVerifier::CurlSSLVerifier):
(WebCore::CurlSSLVerifier::collectInfo): Renamed from verify.
(WebCore::CurlSSLVerifier::verifyCallback):
(WebCore::CurlSSLVerifier::verify): Renamed to collectInfo.

  • platform/network/curl/CurlSSLVerifier.h:
  • platform/network/curl/ResourceHandleCurl.cpp:

(WebCore::ResourceHandle::setHostAllowsAnyHTTPSCertificate): Rename calling method.

Location:
trunk/Source/WebCore
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r233916 r233919  
     12018-07-18  Basuke Suzuki  <Basuke.Suzuki@sony.com>
     2
     3        [Curl] Disable CURLOPT_SSL_VERIFYPEER and CURLOPT_SSL_VERIFYHOST specified by setAllowsAnyHTTPSCertificate.
     4        https://bugs.webkit.org/show_bug.cgi?id=187611
     5
     6        Reviewed by Fujii Hironori.
     7
     8        Current interface for TLS certificate validation for Curl port are as follows:
     9
     10        - WEBCORE_EXPORT void setHostAllowsAnyHTTPSCertificate(const String&);
     11        - bool isAllowedHTTPSCertificateHost(const String&);
     12        - bool canIgnoredHTTPSCertificate(const String&, const Vector<CertificateInfo::Certificate>&);
     13
     14        First one registers a host to be ignored for any certificate check. Once it is registered, no
     15        further certificate validation check is executed.
     16        Second one checks the host is registered in the list above.
     17        Third one is weird. The method signature implies it checks the certificate for the host and detect
     18        whether we can ignore this certificate for the host, but actually it  just check only the host and
     19        register the certificate into the vector. Then in the next request for the host, the certificate
     20        will be checked with the previously stored certificate.
     21
     22        It's hard to understand, but in short,
     23        - We can register a host as an exception for any TLS certificate validation.
     24        - But only certificate arrived first is ignored, not any certificates for the host
     25          (which is rare, but possible for mis configured web cluster).
     26
     27        This behavior is incomplete. To ignore any certificates of the host, these two methods are enough:
     28
     29        - void allowAnyHTTPSCertificatesForHost(const String&)
     30        - bool canIgnoreAnyHTTPSCertificatesForHost(const String&)
     31
     32        No new tests. Covered by existing tests.
     33
     34        * platform/network/curl/CertificateInfo.h:
     35        * platform/network/curl/CurlContext.cpp:
     36        (WebCore::CurlHandle::enableSSLForHost): Ignore TLS verification for registered host.
     37        * platform/network/curl/CurlSSLHandle.cpp:
     38        (WebCore::CurlSSLHandle::allowAnyHTTPSCertificatesForHost): Added.
     39        (WebCore::CurlSSLHandle::canIgnoreAnyHTTPSCertificatesForHost const): Ditto.
     40        (WebCore::CurlSSLHandle::setClientCertificateInfo): Separate lock.
     41        (WebCore::CurlSSLHandle::getSSLClientCertificate const): Ditto and add const.
     42        (WebCore::CurlSSLHandle::setHostAllowsAnyHTTPSCertificate): Deleted.
     43        (WebCore::CurlSSLHandle::isAllowedHTTPSCertificateHost): Deleted.
     44        (WebCore::CurlSSLHandle::canIgnoredHTTPSCertificate): Deleted.
     45        (WebCore::CurlSSLHandle::getSSLClientCertificate): Deleted.
     46        * platform/network/curl/CurlSSLHandle.h:
     47        * platform/network/curl/CurlSSLVerifier.cpp:
     48        (WebCore::CurlSSLVerifier::CurlSSLVerifier):
     49        (WebCore::CurlSSLVerifier::collectInfo): Renamed from verify.
     50        (WebCore::CurlSSLVerifier::verifyCallback):
     51        (WebCore::CurlSSLVerifier::verify): Renamed to collectInfo.
     52        * platform/network/curl/CurlSSLVerifier.h:
     53        * platform/network/curl/ResourceHandleCurl.cpp:
     54        (WebCore::ResourceHandle::setHostAllowsAnyHTTPSCertificate): Rename calling method.
     55
    1562018-07-18  Myles C. Maxfield  <mmaxfield@apple.com>
    257

  • trunk/Source/WebCore/platform/network/curl/CertificateInfo.h

    r233481 r233919  
    3333class CertificateInfo {
    3434public:
    35     using Certificate = Vector<char>;
     35    using Certificate = Vector<uint8_t>;
    3636
    3737    CertificateInfo() = default;

  • trunk/Source/WebCore/platform/network/curl/CurlContext.cpp

    r233803 r233919  
    302302    }
    303303
    304     if (sslHandle.shouldIgnoreSSLErrors()) {
     304    if (sslHandle.canIgnoreAnyHTTPSCertificatesForHost(host) || sslHandle.shouldIgnoreSSLErrors()) {
    305305        setSslVerifyPeer(CurlHandle::VerifyPeer::Disable);
    306306        setSslVerifyHost(CurlHandle::VerifyHost::LooseNameCheck);

  • trunk/Source/WebCore/platform/network/curl/CurlSSLHandle.cpp

    r233481 r233919  
    9393}
    9494
    95 void CurlSSLHandle::setHostAllowsAnyHTTPSCertificate(const String& hostName)
     95void CurlSSLHandle::allowAnyHTTPSCertificatesForHost(const String& host)
    9696{
    97     LockHolder mutex(m_mutex);
     97    LockHolder mutex(m_allowedHostsLock);
    9898
    99     m_allowedHosts.set(hostName, Vector<CertificateInfo::Certificate> { });
     99    m_allowedHosts.addVoid(host);
    100100}
    101101
    102 bool CurlSSLHandle::isAllowedHTTPSCertificateHost(const String& hostName)
     102bool CurlSSLHandle::canIgnoreAnyHTTPSCertificatesForHost(const String& host) const
    103103{
    104     LockHolder mutex(m_mutex);
     104    LockHolder mutex(m_allowedHostsLock);
    105105
    106     auto it = m_allowedHosts.find(hostName);
    107     return (it != m_allowedHosts.end());
    108 }
    109 
    110 bool CurlSSLHandle::canIgnoredHTTPSCertificate(const String& hostName, const Vector<CertificateInfo::Certificate>& certificates)
    111 {
    112     LockHolder mutex(m_mutex);
    113 
    114     auto found = m_allowedHosts.find(hostName);
    115     if (found == m_allowedHosts.end())
    116         return false;
    117 
    118     auto& value = found->value;
    119     if (value.isEmpty()) {
    120         value = certificates;
    121         return true;
    122     }
    123 
    124     return std::equal(certificates.begin(), certificates.end(), value.begin());
     106    return m_allowedHosts.contains(host);
    125107}
    126108
    127109void CurlSSLHandle::setClientCertificateInfo(const String& hostName, const String& certificate, const String& key)
    128110{
    129     LockHolder mutex(m_mutex);
     111    LockHolder mutex(m_allowedClientHostsLock);
    130112
    131113    m_allowedClientHosts.set(hostName, ClientCertificate { certificate, key });
    132114}
    133115
    134 std::optional<CurlSSLHandle::ClientCertificate> CurlSSLHandle::getSSLClientCertificate(const String& hostName)
     116std::optional<CurlSSLHandle::ClientCertificate> CurlSSLHandle::getSSLClientCertificate(const String& hostName) const
    135117{
    136     LockHolder mutex(m_mutex);
     118    LockHolder mutex(m_allowedClientHostsLock);
    137119
    138120    auto it = m_allowedClientHosts.find(hostName);

  • trunk/Source/WebCore/platform/network/curl/CurlSSLHandle.h

    r233481 r233919  
    3030#include <openssl/crypto.h>
    3131#include <wtf/HashMap.h>
     32#include <wtf/HashSet.h>
    3233#include <wtf/NeverDestroyed.h>
    33 #include <wtf/Noncopyable.h>
    3434#include <wtf/Variant.h>
    3535#include <wtf/text/StringHash.h>
    … …  
    7070    WEBCORE_EXPORT void clearCACertInfo();
    7171
    72     WEBCORE_EXPORT void setHostAllowsAnyHTTPSCertificate(const String&);
    73     bool isAllowedHTTPSCertificateHost(const String&);
    74     bool canIgnoredHTTPSCertificate(const String&, const Vector<CertificateInfo::Certificate>&);
     72    WEBCORE_EXPORT void allowAnyHTTPSCertificatesForHost(const String& host);
     73    bool canIgnoreAnyHTTPSCertificatesForHost(const String&) const;
    7574
    7675    WEBCORE_EXPORT void setClientCertificateInfo(const String&, const String&, const String&);
    77     std::optional<ClientCertificate> getSSLClientCertificate(const String&);
     76    std::optional<ClientCertificate> getSSLClientCertificate(const String&) const;
    7877
    7978private:
    … …  
    114113    bool m_ignoreSSLErrors { false };
    115114
    116     Lock m_mutex;
    117     HashMap<String, Vector<CertificateInfo::Certificate>, ASCIICaseInsensitiveHash> m_allowedHosts;
     115    mutable Lock m_allowedHostsLock;
     116    mutable Lock m_allowedClientHostsLock;
     117    HashSet<String, ASCIICaseInsensitiveHash> m_allowedHosts;
    118118    HashMap<String, ClientCertificate, ASCIICaseInsensitiveHash> m_allowedClientHosts;
    119119};

  • trunk/Source/WebCore/platform/network/curl/CurlSSLVerifier.cpp

    r233803 r233919  
    4848
    4949#if defined(LIBRESSL_VERSION_NUMBER)
    50     if (auto* data = WTF::get_if<Vector<char>>(sslHandle.getCACertInfo()))
    51         SSL_CTX_load_verify_mem(ctx, static_cast<void*>(const_cast<char*>(data->data())), data->size());
     50    if (auto data = WTF::get_if<CertificateInfo::Certificate>(sslHandle.getCACertInfo()))
     51        SSL_CTX_load_verify_mem(ctx, static_cast<void*>(const_cast<uint8_t*>(data->data())), data->size());
    5252#endif
    5353
    … …  
    6363}
    6464
    65 bool CurlSSLVerifier::verify(X509StoreCTX* ctx)
    66 {
    67     // whether the verification of the certificate in question was passed (preverify_ok=1) or not (preverify_ok=0)
     65void CurlSSLVerifier::collectInfo(X509StoreCTX* ctx)
     66{
    6867    m_certificateInfo = CertificateInfo { X509_STORE_CTX_get_error(ctx), pemDataFromCtx(ctx) };
    6968
    70     auto error = m_certificateInfo.verificationError();
    71     if (!error)
    72         return 1;
    73 
    74     m_sslErrors = static_cast<int>(convertToSSLCertificateFlags(error));
    75 
    76 #if PLATFORM(WIN)
    77     bool ok = CurlContext::singleton().sslHandle().isAllowedHTTPSCertificateHost(m_curlHandle.url().host().toString());
    78 #else
    79     bool ok = CurlContext::singleton().sslHandle().canIgnoredHTTPSCertificate(m_curlHandle.url().host().toString(), m_certificateInfo.certificateChain());
    80 #endif
    81 
    82     if (ok) {
    83         // if the host and the certificate are stored for the current handle that means is enabled,
    84         // so don't need to curl verifies the authenticity of the peer's certificate
    85         m_curlHandle.setSslVerifyPeer(CurlHandle::VerifyPeer::Disable);
    86     }
    87 
    88     return ok;
    89 }
    90 
    91 int CurlSSLVerifier::verifyCallback(int ok, X509StoreCTX* ctx)
     69    if (auto error = m_certificateInfo.verificationError())
     70        m_sslErrors = static_cast<int>(convertToSSLCertificateFlags(error));
     71}
     72
     73int CurlSSLVerifier::verifyCallback(int preverified, X509StoreCTX* ctx)
    9274{
    9375    auto ssl = static_cast<SSL*>(X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()));
    … …  
    9577    auto verifier = static_cast<CurlSSLVerifier*>(SSL_CTX_get_app_data(sslCtx));
    9678
    97     // whether the verification of the certificate in question was passed (preverify_ok=1) or not (preverify_ok=0)
    98     return verifier->verify(ctx);
     79    verifier->collectInfo(ctx);
     80    // whether the verification of the certificate in question was passed (preverified=1) or not (preverified=0)
     81    return preverified;
    9982}
    10083

  • trunk/Source/WebCore/platform/network/curl/CurlSSLVerifier.h

    r233803 r233919  
    6464    CertificateInfo m_certificateInfo;
    6565
    66     bool verify(X509StoreCTX*);
     66    void collectInfo(X509StoreCTX*);
    6767};
    6868

  • trunk/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp

    r233122 r233919  
    170170    ASSERT(isMainThread());
    171171
    172     CurlContext::singleton().sslHandle().setHostAllowsAnyHTTPSCertificate(host);
     172    CurlContext::singleton().sslHandle().allowAnyHTTPSCertificatesForHost(host);
    173173}
    174174
Note: See TracChangeset for help on using the changeset viewer.