Implementation of MS-Root check for WinInet HTTP client

examples/cpp/SampleCpp/main.cpp

@@ -299,6 +299,11 @@ int main()
     printf("LogManager::Initialize in direct\n");
     printf("Teardown time: %d\n", int(config[CFG_INT_MAX_TEARDOWN_TIME]) );
     config[CFG_INT_SDK_MODE] = SdkModeTypes::SdkModeTypes_CS;
+
+    // Code snippet showing how to perform MS Root certificate check for v10 end-point.
+    // Most other nd-points are Baltimore CA-rooted, but this one is MS CA-rooted.
+    config["http"]["msRootCheck"] = true;
+    config[CFG_STR_COLLECTOR_URL] = "https://v10.events.data.microsoft.com/OneCollector/1.0/";
     logger = LogManager::Initialize(API_KEY);
 
     logPiiMark();   // Direct upload

lib/api/LogManagerImpl.cpp

@@ -216,6 +216,11 @@ namespace ARIASDK_NS_BEGIN
         if (m_httpClient == nullptr)
         {
             m_httpClient.reset(HttpClientFactory::Create());
+#ifdef HAVE_MAT_WININET_HTTP_CLIENT
+            /* Allow WinInet HTTP client to obtain its parent ILogManager configuration */
+            HttpClient_WinInet* client = static_cast<HttpClient_WinInet *>(m_httpClient.get());
+            client->SetParentLogManager(this);
+#endif
         }
         else
         {

lib/config/RuntimeConfig_Default.hpp

@@ -52,6 +52,9 @@ namespace ARIASDK_NS_BEGIN {
 #else
                 { "compress",            false }
 #endif
+            ,
+                /* Optional parameter to require Microsoft Root CA */
+                { "msRootCheck",         false }
             }
         },
         { "tpm",

lib/http/HttpClientFactory.cpp

@@ -9,7 +9,8 @@
 #if defined(MATSDK_PAL_WIN32)
   #ifdef _WINRT_DLL
     #include "http/HttpClient_WinRt.hpp"
-  #else
+  #endif
+  #ifdef HAVE_MAT_WININET_HTTP_CLIENT
     #include "http/HttpClient_WinInet.hpp"
   #endif
 #elif defined(MATSDK_PAL_CPP11)
@@ -32,8 +33,8 @@ namespace ARIASDK_NS_BEGIN {
         LOG_TRACE("Creating HttpClient_WinRt");
         return new HttpClient_WinRt();
     }
-
-#else
+#endif
+#ifdef HAVE_MAT_WININET_HTTP_CLIENT
     IHttpClient* HttpClientFactory::Create() {
         LOG_TRACE("Creating HttpClient_WinInet");
         return new HttpClient_WinInet();

lib/http/HttpClientFactory.hpp

@@ -20,6 +20,12 @@ private:
 
 } ARIASDK_NS_END
 
+// TODO: [maxgolov] - remove this once there is a better way to pass HTTP client configuration
+#if defined(MATSDK_PAL_WIN32) && !defined(_WINRT_DLL)
+#define HAVE_MAT_WININET_HTTP_CLIENT
+#include "http/HttpClient_WinInet.hpp"
+#endif
+
 #endif // HAVE_MAT_DEFAULT_HTTP_CLIENT
 
 #endif // HTTPCLIENTFACTORY_HPP

lib/http/HttpClient_WinInet.cpp

@@ -61,6 +61,53 @@ class WinInetRequestWrapper
         }
     }
 
+    /**
+     * Verify that the server end-point certificate is MS-Rooted
+     */
+    bool isMsRootCert()
+    {
+        // Pointer to certificate chain obtained via InternetQueryOption :
+        // Ref. https://blogs.msdn.microsoft.com/alejacma/2012/01/18/how-to-use-internet_option_server_cert_chain_context-with-internetqueryoption-in-c/
+        PCCERT_CHAIN_CONTEXT pCertCtx = nullptr;
+        DWORD dwCertChainContextSize = sizeof(PCCERT_CHAIN_CONTEXT);
+        // Proceed to process the result if API call succeeds. That option is available in MSIE 8.x+ since Windows 7.1 and Win Server 2008 R2.
+        // In case if API call fails, then proceed without cert validation. This behavior is identical to default old behavior to avoid
+        // regressions for downlevel OS.
+        if (::InternetQueryOption(m_hWinInetRequest, INTERNET_OPTION_SERVER_CERT_CHAIN_CONTEXT, (LPVOID)&pCertCtx, &dwCertChainContextSize))
+        {
+            CERT_CHAIN_POLICY_STATUS pps = {0};
+            pps.cbSize = sizeof(pps);
+            // Verify that the cert chain roots up to the Microsoft application root at top level
+            CERT_CHAIN_POLICY_PARA policyPara = {0};
+            policyPara.cbSize = sizeof(policyPara);
+            policyPara.dwFlags = MICROSOFT_ROOT_CERT_CHAIN_POLICY_CHECK_APPLICATION_ROOT_FLAG;
+            policyPara.pvExtraPolicyPara = nullptr;
+
+            BOOL policyChecked = CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_MICROSOFT_ROOT, pCertCtx, &policyPara, &pps);
+            if (pCertCtx != nullptr)
+            {
+                CertFreeCertificateChain(pCertCtx);
+            }
+            // Unable to verify the chain
+            if (!policyChecked)
+            {
+                LOG_WARN("CertVerifyCertificateChainPolicy() failed: unable to verify");
+                return false;
+            }
+            // Non-MS rooted cert chaine
+            if (pps.dwError != ERROR_SUCCESS)
+            {
+                LOG_WARN("CertVerifyCertificateChainPolicy() failed: invalid root CA - %d", pps.dwError);
+                return false;
+            }
+        }
+        else
+        {
+            LOG_WARN("InternetQueryOption() failed to obtain cert chain");
+        }
+        return true;
+    }
+
     void send(IHttpResponseCallback* callback)
     {
         m_appCallback = callback;
@@ -110,6 +157,18 @@ class WinInetRequestWrapper
             return;
         }
 
+        /* Perform optional MS Root certificate check for certain end-point URLs */
+        if (m_parent.m_logManager != nullptr)
+        {
+            auto config = m_parent.m_logManager->GetLogConfiguration();
+            bool isMsRootCheckReqd = config["http"]["msRootCheck"];
+            if (isMsRootCheckReqd && !isMsRootCert())
+            {
+                onRequestComplete(ERROR_INTERNET_SEC_INVALID_CERT);
+                return;
+            }
+        }
+
         ::InternetSetStatusCallback(m_hWinInetRequest, &WinInetRequestWrapper::winInetCallback);
 
         std::ostringstream os;
@@ -299,7 +358,8 @@ class WinInetRequestWrapper
 
 unsigned HttpClient_WinInet::s_nextRequestId = 0;
 
-HttpClient_WinInet::HttpClient_WinInet()
+HttpClient_WinInet::HttpClient_WinInet() :
+    m_logManager(nullptr)
 {
     m_hInternet = ::InternetOpen(NULL, INTERNET_OPEN_TYPE_PRECONFIG, NULL, NULL, INTERNET_FLAG_ASYNC);
 }
@@ -371,6 +431,12 @@ void HttpClient_WinInet::CancelAllRequests()
     }
 };
 
+/* This internal helper method may optionally be called shortly after HTTP client creation */
+void HttpClient_WinInet::SetParentLogManager(ILogManager* logManager)
+{
+    m_logManager = logManager;
+}
+
 } ARIASDK_NS_END
 
 #endif // HAVE_MAT_DEFAULT_HTTP_CLIENT

lib/http/HttpClient_WinInet.hpp

@@ -7,6 +7,8 @@
 #include "IHttpClient.hpp"
 #include "pal/PAL.hpp"
 
+#include "ILogManager.hpp"
+
 namespace ARIASDK_NS_BEGIN {
 
 #ifndef _WININET_
@@ -17,6 +19,7 @@ class WinInetRequestWrapper;
 
 class HttpClient_WinInet : public IHttpClient {
   public:
+    // Common IHttpClient methods
     HttpClient_WinInet();
     virtual ~HttpClient_WinInet();
     virtual IHttpRequest* CreateRequest() override;
@@ -24,6 +27,17 @@ class HttpClient_WinInet : public IHttpClient {
     virtual void CancelRequestAsync(std::string const& id) override;
     virtual void CancelAllRequests() override;
 
+    // Methods unique to WinInet implementation.
+
+    // Pass a pointer to ILogManager in order to obtain config parameters, e.g.
+    // to check whether MS-Root cert validation is required or not. This logic
+    // is currently unique to Win32 / WinInet. Long-term strategy should be
+    // to expose a cross-platform HTTP client configuration object. That way we
+    // would not need to couple a concrete client implementation with the ILogManager.
+    // Current approach is taken to minimuze the code churn of a critical product
+    // release.
+    void SetParentLogManager(ILogManager* logManager);
+
   protected:
     void erase(std::string const& id);
 
@@ -33,6 +47,10 @@ class HttpClient_WinInet : public IHttpClient {
     std::map<std::string, WinInetRequestWrapper*>                    m_requests;
     static unsigned                                                  s_nextRequestId;
 
+    // TODO: [maxgolov] - allow the client to have its own configuration object.
+    // Currently we anchor to owner ILogManager configuration object.
+    // HTTP client lifecycle is managed by its owner LM.
+    ILogManager*                                                     m_logManager;
     friend class WinInetRequestWrapper;
 };
 

tests/functests/APITest.cpp

@@ -27,6 +27,10 @@
 
 #include "CorrelationVector.hpp"
 
+#include "http/HttpClientFactory.hpp"
+
+#include <list>
+
 using namespace MAT;
 
 LOGMANAGER_INSTANCE
@@ -76,6 +80,25 @@ public:
         resetOnLogX();
     }
 
+    void reset()
+    {
+        netChanged = false;
+        eps = 0;
+        numLogged0 = 0;
+        numLogged = 0;
+        numSent = 0;
+        numDropped = 0;
+        numReject = 0;
+        numHttpError = 0;
+        numHttpOK = 0;
+        numCached = 0;
+        numFiltered = 0;
+        logLatMin = 100;
+        logLatMax = 0;
+        storageFullPct = 0;
+        resetOnLogX();
+    }
+
     virtual void OnLogXDefault(::CsProtocol::Record &)
     {
 
@@ -1076,6 +1099,45 @@ TEST(APITest, LogManager_BadNetwork_Test)
     }
 }
 
+#ifdef HAVE_MAT_WININET_HTTP_CLIENT
+/* This test requires WinInet HTTP client */
+TEST(APITest, LogConfiguration_MsRoot_Check)
+{
+    TestDebugEventListener debugListener;
+    std::list<std::tuple<std::string, bool, unsigned>> testParams =
+    {
+        { "https://v10.events.data.microsoft.com/OneCollector/1.0/",  false, 1}, // MS-Rooted, no MS-Root check:     post succeeds
+        { "https://v10.events.data.microsoft.com/OneCollector/1.0/",  true,  1}, // MS-Rooted, MS-Root check:        post succeeds
+        { "https://self.events.data.microsoft.com/OneCollector/1.0/", false, 1}, // Non-MS rooted, no MS-Root check: post succeeds
+        { "https://self.events.data.microsoft.com/OneCollector/1.0/", true,  0}  // Non-MS rooted, MS-Root check:    post fails
+    };
+
+    // 4 test runs
+    for (const auto &params : testParams)
+    {
+        CleanStorage();
+
+        auto& config = LogManager::GetLogConfiguration();
+        config["stats"]["interval"] = 0;                     // avoid sending stats for this test, just customer events
+        config[CFG_STR_COLLECTOR_URL] = std::get<0>(params);
+        config["http"]["msRootCheck"] = std::get<1>(params); // MS root check depends on what URL we are sending to
+        config[CFG_INT_MAX_TEARDOWN_TIME] = 1;               // up to 1s wait to perform HTTP post on teardown
+        config[CFG_STR_CACHE_FILE_PATH] = GetStoragePath();
+        auto expectedHttpCount = std::get<2>(params);
+
+        auto logger = LogManager::Initialize(TEST_TOKEN, config);
+
+        debugListener.reset();
+        addAllListeners(debugListener);
+        logger->LogEvent("fooBar");
+        LogManager::FlushAndTeardown();
+        removeAllListeners(debugListener);
+
+        EXPECT_EQ(debugListener.numHttpOK, expectedHttpCount);
+    }
+}
+#endif
+
 TEST(APITest, LogManager_GetLoggerSameLoggerMultithreaded)
 {
     auto& config = LogManager::GetLogConfiguration();