New flag to scrub IP addresses

docs/cpp-start-android.md

@@ -14,6 +14,8 @@ The Gradle wrapper in ```android_build``` builds two modules, ```app``` and ```m
 
 On Android, there are two database implementations to choose from. By default (the main branch on Github), the SDK will use the Android-supported androidx.Room database package. This reduces APK size because we don't need to compile and link in a copy of SQLite in native code (SQLite is hundreds of kB per ABI of APK file size). Room does have a slight CPU performance disadvantage since database transactions cross the JNI boundary when native code uses it. If you wish to change from Room to the native SQLite implementation, you should change the two module ```build.gradle``` files (app and maesdk). In those files, you will see an argument to CMake to select Room: ```"-DUSE_ROOM=1"```. Change this to ```"-DUSE_ROOM=0``` to select the native SQLite.
 
+When using the Room implementation, the ```maesdk``` AAR brings ```androidx.room``` as a transitive dependency, pinned in ```lib/android_build/maesdk/build.gradle``` (currently ```2.8.4```). The SDK's native (JNI) code is compiled and tested against this version and the Room-generated schema. Because Gradle resolves a single ```androidx.room``` version for the entire app, if your app (or one of its dependencies) selects a different version, the SDK's native code runs against it. **Do not force ```androidx.room``` below the version the SDK is built against**, and prefer aligning your app on the bundled version (or a compatible newer one). A significantly different Room version can change the shape of query results that cross the JNI boundary and has historically caused native crashes in record retrieval (issue #1227); the SDK now guards against null results defensively, but version alignment avoids subtle behavior differences.
+
 The Room database implementation adds one additional initialization requirement, since it needs a pointer to the JVM and an object reference to the application context. See below (4.5) for the required call to either ```connectContext``` (in Java) or ```ConnectJVM``` (in C++) to set this up.
 
 If you are building on Windows, this helper script [build-android.cmd](../build-android.cmd) is provided to illustrate how to deploy the necessary SDK and NDK dependencies. Once you installed the necessary dependencies, you may use Android Studio IDE for local builds. See [ide.cmd](../lib/android_build/ide.cmd) that shows how to build the project from IDE. The `app` project (`maesdktest`) allows to build and run all SDK tests on either emulator or real Android device. While the tests are running, you can monitor the test results in logcat output.

lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogConfigurationKey.java

@@ -29,6 +29,9 @@ public enum LogConfigurationKey {
   /** Enable network detector. */
   CFG_BOOL_ENABLE_NET_DETECT("enableNetworkDetector", Boolean.class),
 
+  /** Scrub (obfuscate) the client IP address at the collector. Applied unless explicitly set to false (on by default; not present in the default configuration). */
+  CFG_BOOL_ENABLE_IP_SCRUBBING("enableIpScrubbing", Boolean.class),
+
   CFG_BOOL_TPM_CLOCK_SKEW_ENABLED("clockSkewEnabled", Boolean.class),
 
   /** Parameter that allows to check if the SDK is running on UTC mode */

lib/decorators/EventPropertiesDecorator.hpp

@@ -6,6 +6,8 @@
 #define EVENTPROPERTIESDECORATOR_HPP
 
 #include "IDecorator.hpp"
+#include "ILogManager.hpp"
+#include "RecordFlagConstants.hpp"
 #include "EventProperties.hpp"
 #include "CorrelationVector.hpp"
 #include "utils/Utils.hpp"
@@ -16,15 +18,6 @@
 
 namespace MAT_NS_BEGIN {
 
-// Bit remapping has to happen on bits passed via API surface.
-// Ref CS2.1+ : https://osgwiki.com/wiki/CommonSchema/flags
-// #define MICROSOFT_EVENTTAG_MARK_PII 0x08000000
-#define RECORD_FLAGS_EVENTTAG_MARK_PII 0x00080000
-// #define MICROSOFT_EVENTTAG_HASH_PII 0x04000000
-#define RECORD_FLAGS_EVENTTAG_HASH_PII 0x00100000
-// #define MICROSOFT_EVENTTAG_DROP_PII 0x02000000
-#define RECORD_FLAGS_EVENTTAG_DROP_PII 0x00200000
-
     class EventPropertiesDecorator : public IDecorator
     {
     protected:
@@ -125,6 +118,14 @@ namespace MAT_NS_BEGIN {
             int64_t tags = eventProperties.GetPolicyBitFlags();
             int64_t flags = 0;
 
+            // Scrub/obfuscate the client IP address at the collector by default.
+            // Hosts that require the client IP (e.g. for geo-location enrichment)
+            // can opt out by setting CFG_BOOL_ENABLE_IP_SCRUBBING = false.
+            ILogConfiguration& config = m_owner.GetLogConfiguration();
+            if (!config.HasConfig(CFG_BOOL_ENABLE_IP_SCRUBBING) || config[CFG_BOOL_ENABLE_IP_SCRUBBING])
+            {
+                flags |= RECORD_FLAGS_EVENTTAG_SCRUB_IP;
+            }
             // We must remap from one bitfield set to another, no way to bit-shift :(
             // At the moment 1DS SDK in direct upload mode supports DROP and MARK tags only:
             flags |= (tags & MICROSOFT_EVENTTAG_MARK_PII) ? RECORD_FLAGS_EVENTTAG_MARK_PII : 0;

lib/decorators/RecordFlagConstants.hpp

@@ -0,0 +1,30 @@
+//
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+//
+#ifndef RECORDFLAGCONSTANTS_HPP
+#define RECORDFLAGCONSTANTS_HPP
+
+#include "ctmacros.hpp"
+
+#include <cstdint>
+
+namespace MAT_NS_BEGIN {
+
+    // On-wire CS protocol record.flags bits. These are distinct from the
+    // API-surface MICROSOFT_EVENTTAG_* policy flags and are remapped onto
+    // record.flags by EventPropertiesDecorator. Kept in a dedicated header so
+    // other components (e.g. the stats pipeline) can reference a single bit
+    // definition without depending on the decorator's inline implementation.
+    // Ref CS2.1+: https://osgwiki.com/wiki/CommonSchema/flags
+    // (API-surface MICROSOFT_EVENTTAG_MARK_PII 0x08000000)
+    static constexpr std::int64_t RECORD_FLAGS_EVENTTAG_MARK_PII = 0x00080000;
+    // (API-surface MICROSOFT_EVENTTAG_HASH_PII 0x04000000)
+    static constexpr std::int64_t RECORD_FLAGS_EVENTTAG_HASH_PII = 0x00100000;
+    // (API-surface MICROSOFT_EVENTTAG_DROP_PII 0x02000000)
+    static constexpr std::int64_t RECORD_FLAGS_EVENTTAG_DROP_PII = 0x00200000;
+    static constexpr std::int64_t RECORD_FLAGS_EVENTTAG_SCRUB_IP = 0x00400000;
+
+} MAT_NS_END
+
+#endif // RECORDFLAGCONSTANTS_HPP

lib/include/public/ILogConfiguration.hpp

@@ -104,6 +104,16 @@ namespace MAT_NS_BEGIN
     /// </summary>
     static constexpr const char* const CFG_BOOL_ENABLE_NET_DETECT = "enableNetworkDetector";
 
+    /// <summary>
+    /// Request collector-side scrubbing (obfuscation) of the client IP address.
+    /// Applied unless explicitly set to false (on by default; the key is not
+    /// present in the default configuration). Opt out when the client IP is
+    /// needed, e.g. for geo-location enrichment. Honored by the OneCollector
+    /// direct-upload path; in UTC mode client privacy is governed by the OS UTC
+    /// pipeline instead.
+    /// </summary>
+    static constexpr const char* const CFG_BOOL_ENABLE_IP_SCRUBBING = "enableIpScrubbing";
+
     /// <summary>
     /// Parameter that allows to check if the SDK is running on UTC mode
     /// </summary>

lib/offline/OfflineStorage_Room.cpp

@@ -240,6 +240,14 @@ namespace MAT_NS_BEGIN
             MATSDK_THROW(std::logic_error("whereFilter not implemented"));
         }
 
+        if (!env)
+        {
+            return;
+        }
+        if (!m_room)
+        {
+            return;
+        }
         auto room_class = env->GetObjectClass(m_room);
         auto deleteByToken = env->GetMethodID(room_class,
                                               "deleteByToken",
@@ -274,6 +282,10 @@ namespace MAT_NS_BEGIN
             {
                 return;
             }
+            if (!m_room)
+            {
+                return;
+            }
             auto room_class = env->GetObjectClass(m_room);
             auto method = env->GetMethodID(room_class, "deleteById", "([J)J");
             ThrowLogic(env, "Unable to get deleteById method");
@@ -377,6 +389,10 @@ namespace MAT_NS_BEGIN
             {
                 return false;
             }
+            if (!m_room)
+            {
+                return false;
+            }
             auto room_class = env->GetObjectClass(m_room);
             auto reserve = env->GetMethodID(room_class, "getAndReserve",
                                             "(IJJJ)[Lcom/microsoft/applications/events/StorageRecord;");
@@ -424,11 +440,26 @@ namespace MAT_NS_BEGIN
                 int persist_lb = static_cast<int>(EventPersistence_Normal);
                 int persist_ub = static_cast<int>(EventPersistence_DoNotStoreOnDisk);
 
+                // Set if a null array element is hit below, so the early-release
+                // path skips releaseUnconsumed (which would index into the null).
+                bool sawNullElement = false;
                 for (index = 0; index < limit; ++index)
                 {
                     env.pushLocalFrame(32);
                     auto record = env->GetObjectArrayElement(selected, index);
                     ThrowLogic(env, "getAndReserve element");
+                    if (!record)
+                    {
+                        // Null array element (observed with some androidx.room
+                        // versions): pop this frame and stop rather than
+                        // dereferencing null in GetObjectClass. We cannot safely
+                        // release the tail here (it contains this null and Java
+                        // releaseUnconsumed indexes from 0), so leave the
+                        // remaining reservations to expire and be retried.
+                        sawNullElement = true;
+                        env.popLocalFrame();
+                        break;
+                    }
                     if (!record_class)
                     {
                         // Promote to a global ref so it survives popLocalFrame on
@@ -518,11 +549,14 @@ namespace MAT_NS_BEGIN
                 if (index < limit)
                 {
                     // we did not consume all these events
-                    auto release = env->GetMethodID(room_class, "releaseUnconsumed",
-                                                    "([Lcom/microsoft/applications/events/StorageRecord;I)V");
-                    ThrowLogic(env, "releaseUnconsumed");
-                    env->CallVoidMethod(m_room, release, selected, static_cast<int>(index));
-                    ThrowRuntime(env, "call ru");
+                    if (!sawNullElement)
+                    {
+                        auto release = env->GetMethodID(room_class, "releaseUnconsumed",
+                                                        "([Lcom/microsoft/applications/events/StorageRecord;I)V");
+                        ThrowLogic(env, "releaseUnconsumed");
+                        env->CallVoidMethod(m_room, release, selected, static_cast<int>(index));
+                        ThrowRuntime(env, "call ru");
+                    }
                     break;  // break out of the request > collected loop--end early by request
                 }
             }
@@ -633,6 +667,14 @@ namespace MAT_NS_BEGIN
         try
         {
             ConnectedEnv env(s_vm);
+            if (!env)
+            {
+                return;
+            }
+            if (!m_room)
+            {
+                return;
+            }
             auto room_class = env->GetObjectClass(m_room);
             ThrowLogic(env, "GetObjectClass for m_room");
             auto release = env->GetMethodID(room_class,
@@ -700,6 +742,12 @@ namespace MAT_NS_BEGIN
                     env.pushLocalFrame(8);
                     auto byTenant = env->GetObjectArrayElement(results, index);
                     ThrowRuntime(env, "Exception fetching element from results");
+                    if (!byTenant)
+                    {
+                        // Skip a null array element rather than dereference null.
+                        env.popLocalFrame();
+                        continue;
+                    }
                     if (!bt_class)
                     {
                         // Promote to a global ref so it survives popLocalFrame.
@@ -794,6 +842,10 @@ namespace MAT_NS_BEGIN
 
             static constexpr char newRecordSignature[] =
                 "(JIIJIJ[B)Lcom/microsoft/applications/events/StorageRecord;";
+            if (!m_room)
+            {
+                return 0;
+            }
             auto room_class = env->GetObjectClass(m_room);
 
             size_t count = std::min<size_t>(records.size(), INT32_MAX);
@@ -924,6 +976,14 @@ namespace MAT_NS_BEGIN
         try
         {
             ConnectedEnv env(s_vm);
+            if (!env)
+            {
+                return false;
+            }
+            if (!m_room)
+            {
+                return false;
+            }
             auto room_class = env->GetObjectClass(m_room);
             auto delete_method = env->GetMethodID(room_class, "deleteSetting",
                                                   "(Ljava/lang/String;)V");
@@ -964,6 +1024,14 @@ namespace MAT_NS_BEGIN
         try
         {
             ConnectedEnv env(s_vm);
+            if (!env)
+            {
+                return false;
+            }
+            if (!m_room)
+            {
+                return false;
+            }
             auto room_class = env->GetObjectClass(m_room);
             jmethodID store_setting = env->GetMethodID(
                 room_class,
@@ -1081,6 +1149,10 @@ namespace MAT_NS_BEGIN
 
     size_t OfflineStorage_Room::GetSizeInternal(ConnectedEnv& env) const
     {
+        if (!m_room)
+        {
+            return 0;
+        }
         auto room_class = env->GetObjectClass(m_room);
         auto method = env->GetMethodID(room_class, "totalSize", "()J");
         if (!method)
@@ -1107,6 +1179,10 @@ namespace MAT_NS_BEGIN
             {
                 return 0;
             }
+            if (!m_room)
+            {
+                return 0;
+            }
             auto room_class = env->GetObjectClass(m_room);
             auto count_id = env->GetMethodID(room_class, "getRecordCount", "(I)J");
             ThrowLogic(env, "getRecordCount");
@@ -1162,6 +1238,10 @@ namespace MAT_NS_BEGIN
         {
             return false;
         }
+        if (!m_room)
+        {
+            return false;
+        }
         auto room_class = env->GetObjectClass(m_room);
         auto trim_id = env->GetMethodID(room_class, "trim", "(J)J");
         ThrowLogic(env, "trim");
@@ -1192,6 +1272,14 @@ namespace MAT_NS_BEGIN
         {
             ConnectedEnv env(s_vm);
 
+            if (!env)
+            {
+                return records;
+            }
+            if (!m_room)
+            {
+                return records;
+            }
             auto room_class = env->GetObjectClass(m_room);
             auto method = env->GetMethodID(room_class, "getRecords",
                                            "(ZIJ)[Lcom/microsoft/applications/events/StorageRecord;");

lib/stats/Statistics.cpp

@@ -9,6 +9,7 @@
 #include "ILogManager.hpp"
 #include "mat/config.h"
 #include "utils/Utils.hpp"
+#include "decorators/RecordFlagConstants.hpp"
 #include <oacr.h>
 
 namespace MAT_NS_BEGIN {
@@ -83,6 +84,13 @@ namespace MAT_NS_BEGIN {
             result &= m_baseDecorator.decorate(record);
             // Allow stats to capture Part A common properties, but not the custom
             result &= m_semanticContextDecorator.decorate(record, true);
+            // Stats events bypass EventPropertiesDecorator, so apply the same
+            // collector-side client-IP scrub here (on by default; opt out via
+            // CFG_BOOL_ENABLE_IP_SCRUBBING = false).
+            if (!m_config.HasConfig(CFG_BOOL_ENABLE_IP_SCRUBBING) || m_config[CFG_BOOL_ENABLE_IP_SCRUBBING])
+            {
+                record.flags |= RECORD_FLAGS_EVENTTAG_SCRUB_IP;
+            }
             if (result)
             {
                 IncomingEventContext evt(PAL::generateUuidString(), tenantToken, EventLatency_Normal, EventPersistence_Normal, &record);

tests/unittests/EventPropertiesDecoratorTests.cpp

@@ -28,6 +28,19 @@ class TestEventPropertiesDecorator : public EventPropertiesDecorator
     }
 };
 
+// NullLogManager hands out a single shared static ILogConfiguration, which would
+// leak configuration across tests. This subclass owns a per-instance configuration
+// so the IP-scrubbing opt-out can be exercised in isolation.
+class ConfigurableLogManager : public NullLogManager
+{
+public:
+    ILogConfiguration localConfig;
+    ILogConfiguration& GetLogConfiguration() override
+    {
+        return localConfig;
+    }
+};
+
 static std::unique_ptr<Record> PopulateRecordForDropPii()
 {
     auto record = std::unique_ptr<Record>(new Record{});
@@ -545,3 +558,41 @@ TEST(EventPropertiesDecoratorTests, DropPiiPartA_StripsValues)
     EXPECT_THAT(record->extSdk[0].installId, Eq(""));
     EXPECT_THAT(record->cV, Eq(""));
 }
+
+TEST(EventPropertiesDecoratorTests, Decorate_ScrubIp_EnabledByDefault)
+{
+    ConfigurableLogManager logManager;  // CFG_BOOL_ENABLE_IP_SCRUBBING not set
+    EventPropertiesDecorator decorator(logManager);
+    Record record;
+    EventProperties props {"TestEvent"};
+    EventLatency latency = EventLatency::EventLatency_Normal;
+
+    EXPECT_TRUE(decorator.decorate(record, latency, props));
+    EXPECT_TRUE(record.flags & RECORD_FLAGS_EVENTTAG_SCRUB_IP);
+}
+
+TEST(EventPropertiesDecoratorTests, Decorate_ScrubIp_OptOutViaConfig)
+{
+    ConfigurableLogManager logManager;
+    logManager.localConfig[CFG_BOOL_ENABLE_IP_SCRUBBING] = false;
+    EventPropertiesDecorator decorator(logManager);
+    Record record;
+    EventProperties props {"TestEvent"};
+    EventLatency latency = EventLatency::EventLatency_Normal;
+
+    EXPECT_TRUE(decorator.decorate(record, latency, props));
+    EXPECT_FALSE(record.flags & RECORD_FLAGS_EVENTTAG_SCRUB_IP);
+}
+
+TEST(EventPropertiesDecoratorTests, Decorate_ScrubIp_ExplicitlyEnabled)
+{
+    ConfigurableLogManager logManager;
+    logManager.localConfig[CFG_BOOL_ENABLE_IP_SCRUBBING] = true;
+    EventPropertiesDecorator decorator(logManager);
+    Record record;
+    EventProperties props {"TestEvent"};
+    EventLatency latency = EventLatency::EventLatency_Normal;
+
+    EXPECT_TRUE(decorator.decorate(record, latency, props));
+    EXPECT_TRUE(record.flags & RECORD_FLAGS_EVENTTAG_SCRUB_IP);
+}