From 80590e369a0e42995332deb0d7ea0fdb7b955423 Mon Sep 17 00:00:00 2001 From: Jimmy Dee Date: Wed, 24 Feb 2021 16:53:47 -0800 Subject: [PATCH 1/6] Reuse singleton APIClientSession. Still not working. --- .../src/BranchIO/Util/APIClientSession.cpp | 16 +------ .../src/BranchIO/Util/APIClientSession.h | 6 --- .../src/BranchIO/Util/RequestManager.cpp | 45 +++++++++++++++++-- 3 files changed, 44 insertions(+), 23 deletions(-) diff --git a/BranchSDK/src/BranchIO/Util/APIClientSession.cpp b/BranchSDK/src/BranchIO/Util/APIClientSession.cpp index a3f9d76b..bf63b992 100644 --- a/BranchSDK/src/BranchIO/Util/APIClientSession.cpp +++ b/BranchSDK/src/BranchIO/Util/APIClientSession.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -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) { } diff --git a/BranchSDK/src/BranchIO/Util/APIClientSession.h b/BranchSDK/src/BranchIO/Util/APIClientSession.h index d7c674a5..09ecd4cd 100644 --- 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 index 0976759c..4f7d6daf 100644 --- a/BranchSDK/src/BranchIO/Util/RequestManager.cpp +++ b/BranchSDK/src/BranchIO/Util/RequestManager.cpp @@ -2,6 +2,8 @@ #include "RequestManager.h" +#include +#include #include #include @@ -12,6 +14,7 @@ #include "BranchIO/Util/Storage.h" using namespace Poco; +using Poco::Net::Context; namespace BranchIO { @@ -22,6 +25,42 @@ RequestManager::RequestManager(IPackagingInfo& packagingInfo, IClientSession *cl _clientSession(clientSession), _shuttingDown(false), _currentRequest(nullptr) { + // https://pocoproject.org/docs/Poco.Net.Context.html + /* + * From : + * Context(Usage usage, + const std::string& certificateNameOrPath, + VerificationMode verMode = VERIFY_RELAXED, + int options = OPT_DEFAULTS, + const std::string& certificateStoreName = CERT_STORE_MY); + /// Creates a Context. + /// + /// * usage specifies whether the context is used by a client or server, + /// as well as which protocol to use. + /// * certificateNameOrPath specifies either the subject name of the certificate to use, + /// or the path of a PKCS #12 file containing the certificate and corresponding private key. + /// If a subject name is specified, the certificate must be located in the certificate + /// store specified by certificateStoreName. If a path is given, the OPT_LOAD_CERT_FROM_FILE + /// option must be set. + /// * verificationMode specifies whether and how peer certificates are validated. + /// * options is a combination of Option flags. + /// * certificateStoreName specifies the name of the Windows certificate store + /// to use for loading the certificate. Predefined constants + /// CERT_STORE_MY, CERT_STORE_ROOT, etc. can be used. + /// + /// Note: you can use OpenSSL to convert a certificate and private key in PEM format + /// into PKCS #12 format required to import into the Context: + /// + /// openssl pkcs12 -export -inkey cert.key -in cert.crt -out cert.pfx + + */ + Poco::Net::SSLManager::instance().initializeClient(0, 0, new Context( + Context::TLS_CLIENT_USE, + "" /* no client cert required */, + Context::VERIFY_NONE, /* no client cert required */ + Context::OPT_DEFAULTS, /* OPT_PERFORM_REVOCATION_CHECK | OPT_TRUST_ROOTS_WIN_CERT_STORE | OPT_USE_STRONG_CRYPTO */ + Context::CERT_STORE_CA + )); } RequestManager::~RequestManager() { @@ -153,7 +192,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,12 +204,12 @@ 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); + APIClientSession& clientSession(APIClientSession::instance()); _manager.setClientSession(&clientSession); result = _request.send(_event.getAPIEndpoint(), payload, *_callback, &clientSession); _manager.setClientSession(nullptr); From b7f44280abb2fc97e22e9edef2624328e61bfc8c Mon Sep 17 00:00:00 2001 From: Jimmy Dee Date: Fri, 26 Feb 2021 12:55:36 -0800 Subject: [PATCH 2/6] Revert to APIClientSession per request --- BranchSDK/src/BranchIO/Util/RequestManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BranchSDK/src/BranchIO/Util/RequestManager.cpp b/BranchSDK/src/BranchIO/Util/RequestManager.cpp index 4f7d6daf..66c14495 100644 --- a/BranchSDK/src/BranchIO/Util/RequestManager.cpp +++ b/BranchSDK/src/BranchIO/Util/RequestManager.cpp @@ -209,7 +209,7 @@ RequestManager::RequestTask::runTask() { if (_manager.getClientSession()) { result = _request.send(_event.getAPIEndpoint(), payload, *_callback, _manager.getClientSession()); } else { - APIClientSession& clientSession(APIClientSession::instance()); + APIClientSession clientSession(BRANCH_IO_URL_BASE); _manager.setClientSession(&clientSession); result = _request.send(_event.getAPIEndpoint(), payload, *_callback, &clientSession); _manager.setClientSession(nullptr); From c7f9c7f6db0c4dc55fdb1b9f5c66198b7b6c5f87 Mon Sep 17 00:00:00 2001 From: Jimmy Dee Date: Fri, 26 Feb 2021 13:37:49 -0800 Subject: [PATCH 3/6] Use Root store instead of CA --- BranchSDK/src/BranchIO/Util/RequestManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BranchSDK/src/BranchIO/Util/RequestManager.cpp b/BranchSDK/src/BranchIO/Util/RequestManager.cpp index 66c14495..06868240 100644 --- a/BranchSDK/src/BranchIO/Util/RequestManager.cpp +++ b/BranchSDK/src/BranchIO/Util/RequestManager.cpp @@ -59,7 +59,7 @@ RequestManager::RequestManager(IPackagingInfo& packagingInfo, IClientSession *cl "" /* no client cert required */, Context::VERIFY_NONE, /* no client cert required */ Context::OPT_DEFAULTS, /* OPT_PERFORM_REVOCATION_CHECK | OPT_TRUST_ROOTS_WIN_CERT_STORE | OPT_USE_STRONG_CRYPTO */ - Context::CERT_STORE_CA + Context::CERT_STORE_ROOT /* Cert:\LocalMachine\Root */ )); } From 7a834bb088427ea6618053bf750e4d0c67af1ef9 Mon Sep 17 00:00:00 2001 From: Jimmy Dee Date: Tue, 2 Mar 2021 12:07:32 -0800 Subject: [PATCH 4/6] Simplify SSL initialization. Clarifying comments. Additional logging of failures. --- .../src/BranchIO/Util/APIClientSession.cpp | 5 +++- .../src/BranchIO/Util/RequestManager.cpp | 28 +++++++++++-------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/BranchSDK/src/BranchIO/Util/APIClientSession.cpp b/BranchSDK/src/BranchIO/Util/APIClientSession.cpp index bf63b992..ea07c531 100644 --- a/BranchSDK/src/BranchIO/Util/APIClientSession.cpp +++ b/BranchSDK/src/BranchIO/Util/APIClientSession.cpp @@ -95,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/RequestManager.cpp b/BranchSDK/src/BranchIO/Util/RequestManager.cpp index 06868240..7753cfcd 100644 --- a/BranchSDK/src/BranchIO/Util/RequestManager.cpp +++ b/BranchSDK/src/BranchIO/Util/RequestManager.cpp @@ -54,13 +54,14 @@ RequestManager::RequestManager(IPackagingInfo& packagingInfo, IClientSession *cl /// openssl pkcs12 -export -inkey cert.key -in cert.crt -out cert.pfx */ - Poco::Net::SSLManager::instance().initializeClient(0, 0, new Context( - Context::TLS_CLIENT_USE, - "" /* no client cert required */, - Context::VERIFY_NONE, /* no client cert required */ - Context::OPT_DEFAULTS, /* OPT_PERFORM_REVOCATION_CHECK | OPT_TRUST_ROOTS_WIN_CERT_STORE | OPT_USE_STRONG_CRYPTO */ - Context::CERT_STORE_ROOT /* Cert:\LocalMachine\Root */ - )); + /* + * 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(0, 0, new Context(Context::TLS_CLIENT_USE, "" /* no client cert required */)); } RequestManager::~RequestManager() { @@ -209,10 +210,15 @@ RequestManager::RequestTask::runTask() { 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); From c14fd704394bedb564c826b55650e986e2880bb6 Mon Sep 17 00:00:00 2001 From: Jimmy Dee Date: Tue, 2 Mar 2021 12:29:37 -0800 Subject: [PATCH 5/6] Further clean-up --- .../src/BranchIO/Util/RequestManager.cpp | 31 +------------------ 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/BranchSDK/src/BranchIO/Util/RequestManager.cpp b/BranchSDK/src/BranchIO/Util/RequestManager.cpp index 7753cfcd..7bde3db7 100644 --- a/BranchSDK/src/BranchIO/Util/RequestManager.cpp +++ b/BranchSDK/src/BranchIO/Util/RequestManager.cpp @@ -25,35 +25,6 @@ RequestManager::RequestManager(IPackagingInfo& packagingInfo, IClientSession *cl _clientSession(clientSession), _shuttingDown(false), _currentRequest(nullptr) { - // https://pocoproject.org/docs/Poco.Net.Context.html - /* - * From : - * Context(Usage usage, - const std::string& certificateNameOrPath, - VerificationMode verMode = VERIFY_RELAXED, - int options = OPT_DEFAULTS, - const std::string& certificateStoreName = CERT_STORE_MY); - /// Creates a Context. - /// - /// * usage specifies whether the context is used by a client or server, - /// as well as which protocol to use. - /// * certificateNameOrPath specifies either the subject name of the certificate to use, - /// or the path of a PKCS #12 file containing the certificate and corresponding private key. - /// If a subject name is specified, the certificate must be located in the certificate - /// store specified by certificateStoreName. If a path is given, the OPT_LOAD_CERT_FROM_FILE - /// option must be set. - /// * verificationMode specifies whether and how peer certificates are validated. - /// * options is a combination of Option flags. - /// * certificateStoreName specifies the name of the Windows certificate store - /// to use for loading the certificate. Predefined constants - /// CERT_STORE_MY, CERT_STORE_ROOT, etc. can be used. - /// - /// Note: you can use OpenSSL to convert a certificate and private key in PEM format - /// into PKCS #12 format required to import into the Context: - /// - /// openssl pkcs12 -export -inkey cert.key -in cert.crt -out cert.pfx - - */ /* * 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. @@ -61,7 +32,7 @@ RequestManager::RequestManager(IPackagingInfo& packagingInfo, IClientSession *cl * 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(0, 0, new Context(Context::TLS_CLIENT_USE, "" /* no client cert required */)); + Poco::Net::SSLManager::instance().initializeClient(nullptr, nullptr, new Context(Context::TLS_CLIENT_USE, "" /* no client cert required */)); } RequestManager::~RequestManager() { From 9e33fb4dc9a61a0dfd864fc5c47822d08d377e7b Mon Sep 17 00:00:00 2001 From: Jimmy Dee Date: Tue, 2 Mar 2021 12:33:34 -0800 Subject: [PATCH 6/6] Corrected typo --- BranchSDK/src/BranchIO/Branch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BranchSDK/src/BranchIO/Branch.cpp b/BranchSDK/src/BranchIO/Branch.cpp index 2830b08f..693d5658 100644 --- 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")) {