Merge pull request #88 from BranchMetrics/CORE-1740/fix-cert-verifica…

…tion [CORE-1740] Improve TLS configuration and handling
BranchMetrics · Mar 2, 2021 · 3c7e396 · 3c7e396
2 parents f851a5e + 9e33fb4
commit 3c7e396
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 28 deletions.
diff --git a/BranchSDK/src/BranchIO/Branch.cpp b/BranchSDK/src/BranchIO/Branch.cpp
@@ -171,7 +171,7 @@ Branch *Branch::create(const String& branchKey, AppInfo* pInfo) {
 #endif
 
     // Set these on the current app
-    if (hasGlobalTrackingDisabled && !storage.has("advertiser.trackingDisbled")) {
+    if (hasGlobalTrackingDisabled && !storage.has("advertiser.trackingDisabled")) {
         storage.setBoolean("advertiser.trackingDisabled", isGlobalTrackingDisabled);
     }
     if (!globalDeviceFingerprintId.empty() && !storage.has("session.device_fingerprint_id")) {

diff --git a/BranchSDK/src/BranchIO/Util/APIClientSession.cpp b/BranchSDK/src/BranchIO/Util/APIClientSession.cpp
@@ -9,6 +9,7 @@
 #include <Poco/Net/HTTPRequest.h>
 #include <Poco/Net/HTTPResponse.h>
 #include <Poco/Net/NetException.h>
+#include <Poco/Net/SSLManager.h>
 #include <Poco/NullStream.h>
 #include <Poco/StreamCopier.h>
 #include <Poco/URI.h>
@@ -25,19 +26,6 @@ using namespace Poco::Net;
 
 namespace BranchIO {
 
-Poco::Net::Context::Ptr APIClientSession::getContext() {
-    return new Context
-#if defined(__linux__) || defined(__unix__) || defined(__APPLE__)
-        // Unix / Mac
-        (Context::CLIENT_USE, "", "", "",
-         Context::VERIFY_NONE, 9, false,
-         "ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH");
-#else
-        // Windows
-        (Context::CLIENT_USE, "");
-#endif
-}
-
 APIClientSession&
 APIClientSession::instance() {
     // constructed the first time through
@@ -48,7 +36,7 @@ APIClientSession::instance() {
 APIClientSession::APIClientSession(const std::string& urlBase) :
     HTTPSClientSession(URI(urlBase).getHost(),
     URI(urlBase).getPort(),
-    getContext()),
+    Poco::Net::SSLManager::instance().defaultClientContext()),
     _urlBase(urlBase),
     _shuttingDown(false) {
 }
@@ -107,7 +95,10 @@ APIClientSession::post(
     }
     catch (const Poco::Exception& e) {
         if (isShuttingDown()) return false;
-        callback.onStatus(0, 0, string(e.what()) + ": " + e.message());
+        string description(e.what());
+        description += ": " + e.message();
+        BRANCH_LOG_W("Request failed. " << description);
+        callback.onStatus(0, 0, description);
     }
     return false;
 }

diff --git a/BranchSDK/src/BranchIO/Util/APIClientSession.h b/BranchSDK/src/BranchIO/Util/APIClientSession.h
@@ -20,12 +20,6 @@ class APIClientSession
     : public Poco::Net::HTTPSClientSession,
       public virtual IClientSession {
  public:
-    /**
-     * @return SSL session context
-     * @todo(jdee): Document
-     */
-    static Poco::Net::Context::Ptr getContext();
-
     /**
      * @return this instance
      * @todo(jdee): Document

diff --git a/BranchSDK/src/BranchIO/Util/RequestManager.cpp b/BranchSDK/src/BranchIO/Util/RequestManager.cpp
@@ -2,6 +2,8 @@
 
 #include "RequestManager.h"
 
+#include <Poco/Net/PrivateKeyPassphraseHandler.h>
+#include <Poco/Net/SSLManager.h>
 #include <Poco/TaskNotification.h>
 #include <cassert>
 
@@ -12,6 +14,7 @@
 #include "BranchIO/Util/Storage.h"
 
 using namespace Poco;
+using Poco::Net::Context;
 
 namespace BranchIO {
 
@@ -22,6 +25,14 @@ RequestManager::RequestManager(IPackagingInfo& packagingInfo, IClientSession *cl
     _clientSession(clientSession),
     _shuttingDown(false),
     _currentRequest(nullptr) {
+    /*
+     * Note that the following call will always generate an exception warning message in Visual Studio like:
+     * Exception thrown at 0x00007FF87DB8D759 in TestBed-Local.exe: Microsoft C++ exception: Poco::Net::NoCertificateException at memory location 0x000000FD251FC1F0.
+     * This is deliberately thrown by Poco internally when initializing a secure socket:
+     * https://github.com/pocoproject/poco/blob/poco-1.10.1/NetSSL_Win/src/SecureSocketImpl.cpp#L692.
+     * This is not a fatal error. API clients do not supply a cert for authentication.
+     */
+    Poco::Net::SSLManager::instance().initializeClient(nullptr, nullptr, new Context(Context::TLS_CLIENT_USE, "" /* no client cert required */));
 }
 
 RequestManager::~RequestManager() {
@@ -153,7 +164,7 @@ RequestManager::RequestTask::runTask() {
     if (_manager.getPackagingInfo().getAdvertiserInfo().isTrackingDisabled()) {
         payload.set(Defines::JSONKEY_TRACKING_DISABLED, true);
         // remove all identifiable fields
-        // Based on https://github.com/BranchMetrics/ios-branch-deep-linking-attribution/blob/master/Branch-SDK/BNCServerInterface.m#L396-L411
+        // Based on https://github.com/BranchMetrics/ios-branch-deep-linking-attribution/blob/master/Branch-SDK/BNCServerInterface.m around line 400.
         payload.remove(Defines::JSONKEY_APP_DEVELOPER_IDENTITY);   // developer_identity
         payload.remove(Defines::JSONKEY_APP_IDENTITY);             // identity
         payload.remove(Defines::JSONKEY_DEVICE_LOCAL_IP_ADDRESS);  // local_ip
@@ -165,15 +176,20 @@ RequestManager::RequestTask::runTask() {
 
     // Send request synchronously
     // _clientSession may be passed in for testing. If not, we
-    // create a real one here
+    // create/reuse a real one here
     JSONObject result;
     if (_manager.getClientSession()) {
         result = _request.send(_event.getAPIEndpoint(), payload, *_callback, _manager.getClientSession());
     } else {
-        APIClientSession clientSession(BRANCH_IO_URL_BASE);
-        _manager.setClientSession(&clientSession);
-        result = _request.send(_event.getAPIEndpoint(), payload, *_callback, &clientSession);
-        _manager.setClientSession(nullptr);
+        try {
+            APIClientSession clientSession(BRANCH_IO_URL_BASE);
+            _manager.setClientSession(&clientSession);
+            result = _request.send(_event.getAPIEndpoint(), payload, *_callback, &clientSession);
+            _manager.setClientSession(nullptr);
+        }
+        catch (Poco::Exception& e) {
+            BRANCH_LOG_E("Connection failed. " << e.what() << ": " << e.message());
+        }
     }
 
     _event.handleResult(result);