Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

m365_defender: remove invalid IP addresses #9060

Merged
merged 2 commits into from Feb 12, 2024
Merged

Conversation

efd6
Copy link
Contributor

@efd6 efd6 commented Feb 6, 2024

Proposed commit message

M365 Defender apparently sometimes sets empty fields to "-" rather than not setting them. This can cause errors in future processing and mapping, so attempt to convert and remove if not valid.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Screenshots

@efd6 efd6 self-assigned this Feb 6, 2024
M365 Defender apparently sometimes sets empty fields to "-" rather than
not setting them. This can cause errors in future processing and
mapping, so attempt to convert and remove if not valid.
@elasticmachine
Copy link

elasticmachine commented Feb 6, 2024

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@efd6 efd6 marked this pull request as ready for review February 6, 2024 04:10
@efd6 efd6 requested a review from a team as a code owner February 6, 2024 04:10
@elasticmachine
Copy link

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

@efd6
Copy link
Contributor Author

efd6 commented Feb 6, 2024

I don't believe sonarqube here. The two added tests are guaranteed to fail without the changes, so the coverage cannot be zero.

Pipeline tests without the pipeline changes.

--- Test results for package: m365_defender - START ---
FAILURE DETAILS:
m365_defender/event test-device.log:
--- want
+++ got
@@ -3444,17 +3444,23 @@
         {
             "@timestamp": "2023-07-19T14:09:43.873Z",
             "destination": {
+                "ip": "-",
                 "port": 25
             },
             "ecs": {
                 "version": "8.11.0"
+            },
+            "error": {
+                "message": [
+                    "Processor \"conditional\" with tag \"geoip_source_ip\" in pipeline \"pipeline_device-1707192945406082295\" failed with message \"'-' is not an IP string literal.\""
+                ]
             },
             "event": {
                 "action": "smtpconnectioninspected",
                 "category": [
                     "network"
                 ],
-                "kind": "event",
+                "kind": "pipeline_error",
                 "original": "{\"Tenant\":\"DefaultTenant\",\"category\":\"AdvancedHunting-DeviceNetworkEvents\",\"operationName\":\"Publish\",\"properties\":{\"Timestamp\": \"2023-07-19T14:09:43.8734771Z\",\"DeviceId\": \"22bb10ffe3104214b20fc7de339a2b053e915e5c\",\"DeviceName\": \"janeslaptop1.corporatedomain\",\"ActionType\": \"SmtpConnectionInspected\",\"RemoteIP\": \"-\",\"RemotePort\": 25,\"RemoteUrl\":null,\"LocalIP\": \"-\",\"LocalPort\": 60697,\"Protocol\": \"Tcp\",\"LocalIPType\":null,\"RemoteIPType\":null,\"InitiatingProcessSHA1\":null,\"InitiatingProcessSHA256\":null,\"InitiatingProcessMD5\":null,\"InitiatingProcessFileName\":null,\"InitiatingProcessFileSize\":null,\"InitiatingProcessVersionInfoCompanyName\":null,\"InitiatingProcessVersionInfoProductName\":null,\"InitiatingProcessVersionInfoProductVersion\":null,\"InitiatingProcessVersionInfoInternalFileName\":null,\"InitiatingProcessVersionInfoOriginalFileName\":null,\"InitiatingProcessVersionInfoFileDescription\":null,\"InitiatingProcessId\": 0,\"InitiatingProcessCommandLine\":null,\"InitiatingProcessCreationTime\":null,\"InitiatingProcessFolderPath\":null,\"InitiatingProcessParentFileName\":null,\"InitiatingProcessParentId\": 0,\"InitiatingProcessParentCreationTime\":null,\"InitiatingProcessAccountDomain\":null,\"InitiatingProcessAccountName\":null,\"InitiatingProcessAccountSid\":null,\"InitiatingProcessAccountUpn\":null,\"InitiatingProcessAccountObjectId\":null,\"InitiatingProcessIntegrityLevel\":null,\"InitiatingProcessTokenElevation\": \"None\",\"ReportId\": 18984951960,\"AppGuardContainerId\":null,\"AdditionalFields\": {  \"direction\": \"Out\",  \"fuids\": \"[]\",  \"helo\": \"janeslaptop1.corporatedomain\",  \"last_reply\": \"220 2.0.0 SMTP server ready\",  \"path\": \"[\\\"89.160.20.112\\\",\\\"89.160.20.112\\\"]\",  \"tls\": \"true\",  \"trans_depth\": \"1\",  \"uid\": \"0278e28ff5d8eff6d3\"}},\"tenantId\":\"12345af3-bc0e-4f36-b08e-27759e912345\",\"time\":\"2023-07-19T18:03:34.9948950Z\"}"
             },
             "host": {
@@ -3486,12 +3492,14 @@
                         "token_elevation": "None"
                     },
                     "local": {
+                        "ip": "-",
                         "port": 60697
                     },
                     "network_direction": "Out",
                     "operation_name": "Publish",
                     "protocol": "Tcp",
                     "remote": {
+                        "ip": "-",
                         "port": 25
                     },
                     "report_id": "18984951960",
@@ -3513,13 +3521,8 @@
                 },
                 "pid": 0
             },
-            "related": {
-                "hosts": [
-                    "22bb10ffe3104214b20fc7de339a2b053e915e5c",
-                    "janeslaptop1.corporatedomain"
-                ]
-            },
             "source": {
+                "ip": "-",
                 "port": 60697
             },
             "tags": [
@@ -3532,11 +3535,16 @@
             "ecs": {
                 "version": "8.11.0"
             },
+            "error": {
+                "message": [
+                    "Processor \"conditional\" with tag \"geoip_host_ip\" in pipeline \"pipeline_device-1707192945406082295\" failed with message \"'-' is not an IP string literal.\""
+                ]
+            },
             "event": {
                 "category": [
                     "host"
                 ],
-                "kind": "event",
+                "kind": "pipeline_error",
                 "original": "{\"Tenant\":\"DefaultTenant\",\"category\":\"AdvancedHunting-DeviceInfo\",\"operationName\":\"Publish\",\"properties\":{\"AadDeviceId\":null,\"AdditionalFields\":null,\"AssetValue\":\"testvalue\",\"IsInternetFacing\":true,\"DeviceManualTags\":\"testtags\",\"DeviceDynamicTags\":\"testdynamictags\",\"ExposureLevel\":\"testlevel\",\"SensorHealthState\":\"somestatus\",\"ExclusionReason\":\"somereason\",\"IsExcluded\":false,\"ClientVersion\":\"10.8210.19041.2006\",\"DeviceCategory\":\"Endpoint\",\"DeviceId\":\"999b6fd7c532534ba50b3232fa992c38a2712345\",\"DeviceName\":\"testmachine6\",\"DeviceSubtype\":null,\"DeviceType\":\"Workstation\",\"IsAzureADJoined\":false,\"JoinType\":null,\"LoggedOnUsers\":\"[{\\\"UserName\\\":\\\"administrator1\\\"}, {\\\"UserName\\\":\\\"administrator2\\\"}]\",\"MachineGroup\":\"UnassignedGroup\",\"MergedDeviceIds\":null,\"MergedToDeviceId\":null,\"Model\":null,\"OSArchitecture\":null,\"OSBuild\":null,\"OSDistribution\":null,\"OSPlatform\":null,\"OSVersion\":null,\"OSVersionInfo\":null,\"OnboardingStatus\":\"Onboarded\",\"PublicIP\":\"-\",\"RegistryDeviceTag\":\"evaluation\",\"ReportId\":12942,\"Timestamp\":\"2022-11-08T05:56:25.8832339Z\",\"Vendor\":null},\"tenantId\":\"12345af3-bc0e-4f36-b08e-27759e912345\",\"time\":\"2022-11-08T06:01:15.8987913Z\"}",
                 "type": [
                     "info"
@@ -3544,6 +3552,7 @@
             },
             "host": {
                 "id": "999b6fd7c532534ba50b3232fa992c38a2712345",
+                "ip": "-",
                 "name": "testmachine6",
                 "type": "Workstation"
             },
@@ -3572,6 +3581,9 @@
                     "machine_group": "UnassignedGroup",
                     "onboarding_status": "Onboarded",
                     "operation_name": "Publish",
+                    "public_ip": {
+                        "value": "-"
+                    },
                     "registry": {
                         "device_tag": "evaluation"
                     },
@@ -3590,10 +3602,6 @@
                 "version": "10.8210.19041.2006"
             },
             "related": {
-                "hosts": [
-                    "999b6fd7c532534ba50b3232fa992c38a2712345",
-                    "testmachine6"
-                ],
                 "user": [
                     "administrator1",
                     "administrator2"



╭───────────────┬─────────────┬───────────┬─────────────────────────────────────┬─────────────────────────────────────────────────────────────────────────┬──────────────╮
│ PACKAGE       │ DATA STREAM │ TEST TYPE │ TEST NAME                           │ RESULT                                                                  │ TIME ELAPSED │
├───────────────┼─────────────┼───────────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────┼──────────────┤
│ m365_defender │ alert       │ pipeline  │ test-alert.log                      │ PASS                                                                    │   3.482331ms │
│ m365_defender │ event       │ pipeline  │ test-alert.log                      │ PASS                                                                    │   3.599916ms │
│ m365_defender │ event       │ pipeline  │ test-app-and-identity.log           │ PASS                                                                    │   9.116242ms │
│ m365_defender │ event       │ pipeline  │ test-device.log                     │ FAIL: test case failed: Expected results are different from actual ones │   42.07229ms │
│ m365_defender │ event       │ pipeline  │ test-email.log                      │ PASS                                                                    │   5.932495ms │
│ m365_defender │ incident    │ pipeline  │ test-incident.log                   │ PASS                                                                    │  15.840665ms │
│ m365_defender │ log         │ pipeline  │ test-m365-defender-empty-ndjson.log │ PASS                                                                    │   1.458203ms │
│ m365_defender │ log         │ pipeline  │ test-m365-defender-ndjson.log       │ PASS                                                                    │   4.543968ms │
╰───────────────┴─────────────┴───────────┴─────────────────────────────────────┴─────────────────────────────────────────────────────────────────────────┴──────────────╯
--- Test results for package: m365_defender - END   ---

Mapping result in this case (-g).

FAILURE DETAILS:
m365_defender/event test-device.log:
[0] unexpected pipeline error: [Processor "conditional" with tag "geoip_host_ip" in pipeline "pipeline_device-1707193038453976177" failed with message "'-' is not an IP string literal."]
[1] unexpected pipeline error: [Processor "conditional" with tag "geoip_source_ip" in pipeline "pipeline_device-1707193038453976177" failed with message "'-' is not an IP string literal."]

@chrisberkhout chrisberkhout self-requested a review February 9, 2024 15:18
@chrisberkhout
Copy link
Contributor

I wonder if this is complete. Is the scope for this to only handle some particular IP fields that get geoip enrichment?

Looking for ip-type fields I see

in PR data stream field
alert m365_defender.alert.evidence.ip_address
alert m365_defender.alert.evidence.ip_interfaces
alert m365_defender.alert.evidence.sender_ip
event destination.ip
event host.ip
event m365_defender.event.destination.ip_address
x event m365_defender.event.file.origin_ip
event m365_defender.event.ip_address
event m365_defender.event.ipv4_dhcp
event m365_defender.event.ipv6_dhcp
x event m365_defender.event.local.ip
x event m365_defender.event.public_ip.value
x event m365_defender.event.remote.ip
x event m365_defender.event.request.source_ip
event m365_defender.event.sender.ipv4
event m365_defender.event.sender.ipv6
event related.ip
event source.ip
incident host.ip
incident m365_defender.incident.alert.evidence.ip_address
incident m365_defender.incident.alert.evidence.sender_ip
incident related.ip
incident source.ip
log host.ip
log related.ip

Within the event data stream there are also different pipelines handling different event categories.

For example, the json.properties.RemoteIP / m365_defender.event.remote.ip field is handled in the PR for the device category, but it is handled differently (by adding an error.message) for the alert category.

@efd6
Copy link
Contributor Author

efd6 commented Feb 11, 2024

I wonder if this is complete. Is the scope for this to only handle some particular IP fields that get geoip enrichment?

That was the original scope, but I think it will be worth doing the rest. I will add those changes. Thanks.

@efd6
Copy link
Contributor Author

efd6 commented Feb 11, 2024

in PR data stream field
handled alert m365_defender.alert.evidence.ip_address
handled alert m365_defender.alert.evidence.ip_interfaces
handled alert m365_defender.alert.evidence.sender_ip
handled event destination.ip
handled event host.ip
handled event m365_defender.event.destination.ip_address
x event m365_defender.event.file.origin_ip
handled event m365_defender.event.ip_address
handled event m365_defender.event.ipv4_dhcp
handled event m365_defender.event.ipv6_dhcp
x event m365_defender.event.local.ip
x event m365_defender.event.public_ip.value
x event m365_defender.event.remote.ip
x event m365_defender.event.request.source_ip
handled event m365_defender.event.sender.ipv4
handled event m365_defender.event.sender.ipv6
handled indirectly via above event related.ip
handled indirectly via above event source.ip
never referenced incident host.ip
handled incident m365_defender.incident.alert.evidence.ip_address
handled incident m365_defender.incident.alert.evidence.sender_ip
handled indirectly via above incident related.ip
handled indirectly via above incident source.ip
never referenced log host.ip
added log related.ip

@elasticmachine
Copy link

💚 Build Succeeded

History

cc @efd6

Copy link

Quality Gate failed Quality Gate failed

Failed conditions

0.0% 0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

Copy link
Contributor

@chrisberkhout chrisberkhout left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the extra info. What's in the diff is good and I don't want to hold that up.

A couple of observations about the unchanged handling of IP addresses:

  • Most of the already handled cases append an error message on failure but don't remove the field or clear its contents. The convert processor will leave the original value if it can't successfully convert. So if it's a bad but non-empty value, it'll probably cause a failure later on.
  • I think removing field: _ingest._value probably won't remove it from the thing being iterated over.

@efd6
Copy link
Contributor Author

efd6 commented Feb 12, 2024

I've examined a representative subset (all of alert) of the foreach conversions and altering the input to be an invalid value results in the expected behaviour, so I think this is good to merge.

@efd6 efd6 merged commit ec0aa9f into elastic:main Feb 12, 2024
4 of 5 checks passed
@elasticmachine
Copy link

Package m365_defender - 2.7.1 containing this change is available at https://epr.elastic.co/search?package=m365_defender

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

M365 Defender Geo IP failing when IP is "-"
3 participants