CrowdStrike Case: 02501603 - #259
Conversation
carlosmmatos
left a comment
There was a problem hiding this comment.
Thanks for this — the migration is well put together. Building the credential and LogsIngestionClient once in Runtime.__init__(), the clean three-mode dispatch, the least-privilege DCR-scoped role assignment in the Bicep, and the rewritten README are all solid. A few things need to be addressed before this can merge.
Blocking
1. Legacy SharedKey path is broken by the log() return-type change
Submitter.log() was changed from return dumps(json_data) to return json_data so the new SDK's upload() receives a list. But post_data_legacy() still treats its body argument as a serialized JSON string:
content_length = len(body) # body is a list -> 1, not the byte length
...
response = post(uri, data=body, headers=headers, timeout=60) # requests gets a list of dictsI ran the legacy path directly with the dependencies installed. It doesn't just sign a wrong Content-Length into the HMAC (which would 403) — requests.post(data=<list of dicts>) raises before anything is sent:
ValueError: too many values to unpack (expected 2)
So legacy mode crashes on the first detection. flake8 flags the same root cause independently: F401 'json.dumps' imported but unused — nothing serializes anymore. Re-serializing at the top of post_data_legacy() (e.g. body = dumps(body)) fixes both the crash and the unused-import error.
2. flake8 fails the CI gate (flake8 fig exits non-zero)
12 violations, all in the two files below:
fig/backends/azure/__init__.py:F401 'json.dumps' imported but unused(resolved by fixing #1)fig/backends/workspaceone/serverlog/__init__.py: 5×F401unused imports + 6×E265block-comment formatting, from commenting out the handler block (see #3)
Please split out of this PR
3. WorkspaceONE serverlog change is an unrelated regression
fig/backends/workspaceone/serverlog/__init__.py comments out the entire TLSSysLogHandler setup at module level, which silently disables syslog delivery for all WorkspaceONE users. The stated rationale — PROTOCOL_TLSv1_2 was removed in Python 3.12 — doesn't hold up: the constant is still present (deprecated, not removed) on 3.13/3.14, and this project pins python_requires='>=3.6, <3.12' with CI and the container both on 3.11, so 3.12 isn't a target. This is unrelated to the Azure migration and accounts for 11 of the 12 flake8 failures. Recommend pulling it into its own PR; if WS1 genuinely needs TLS-version handling, switching to ssl.SSLContext / ssl.TLSVersion would be the forward-compatible approach rather than disabling the handler.
4. Version bump to 3.5.1
The fig/__init__.py bump is bundled into a feature PR. The repo's convention is a standalone version-bump PR (e.g. #257). Please drop it here and let the release process own it.
Minor
- Pin the new dependencies.
azure-monitor-ingestionandazure-identityare unpinned, unlike their neighbors. Suggestazure-monitor-ingestion==1.0.4andazure-identity>=1.25.3(the versions that resolve and install cleanly on 3.11). - Python floor. Both packages require Python ≥3.8, which conflicts with the
>=3.6floor insetup.py. Since CI and Docker only build 3.11, recommend updating the documented support range to>=3.8. post_data()only catchesHttpResponseError. Credential failures (ClientAuthenticationError) raised byupload()will propagate uncaught. A broader catch or a brief note would be worth adding.
Verified and looking good
- Mode-aware config validation in
fig/config/__init__.py— required fields enforced correctly per auth method, invalidauth_methodrejected. - Bicep:
Monitoring Metrics Publisherrole GUID is correct, role assignment scoped to the DCR only (true least privilege),kind: 'Direct'with the DCR-levellogsIngestionendpoint is the right pattern, and the cross-subscription module pattern is sound. - k8s manifest: ServiceAccount + workload-identity annotations/labels are correct, and the
matchLabelsindentation fix is a nice catch. - bandit: clean. pylint: clean under the project config (only a non-blocking
R0914in the legacy function).
Once the legacy serialization is fixed, the WS1 change and version bump are split out, and the deps are pinned, this should be in good shape. Thanks again for tackling the Data Collector API deprecation.
|
Hey Carlos,
Just to be clear, you’re asking me to remove the version change from the PR, pin the requires.txt, fix the legacy mode, and restore the log handler for workspaceone, correct?
I apologize for the workspace one slip, I’d been deliberately excluding that from previous commits, but was using it in my local testing.
David
From: nialet ***@***.***>
Reply-To: CrowdStrike/falcon-integration-gateway ***@***.***>
Date: Tuesday, June 23, 2026 at 12:30 PM
To: CrowdStrike/falcon-integration-gateway ***@***.***>
Cc: "Warren, David" ***@***.***>, Author ***@***.***>
Subject: Re: [CrowdStrike/falcon-integration-gateway] CrowdStrike Case: 02501603 (PR #259)
You don't often get email from ***@***.*** Learn why this is important<https://aka.ms/LearnAboutSenderIdentification>
@carlosmmatos requested changes on this pull request.
Thanks for this — the migration is well put together. Building the credential and LogsIngestionClient once in Runtime.__init__(), the clean three-mode dispatch, the least-privilege DCR-scoped role assignment in the Bicep, and the rewritten README are all solid. A few things need to be addressed before this can merge.
Blocking
1. Legacy SharedKey path is broken by the log() return-type change
Submitter.log() was changed from return dumps(json_data) to return json_data so the new SDK's upload() receives a list. But post_data_legacy() still treats its body argument as a serialized JSON string:
content_length = len(body) # body is a list -> 1, not the byte length
...
response = post(uri, data=body, headers=headers, timeout=60) # requests gets a list of dicts
I ran the legacy path directly with the dependencies installed. It doesn't just sign a wrong Content-Length into the HMAC (which would 403) — requests.post(data=<list of dicts>) raises before anything is sent:
ValueError: too many values to unpack (expected 2)
So legacy mode crashes on the first detection. flake8 flags the same root cause independently: F401 'json.dumps' imported but unused — nothing serializes anymore. Re-serializing at the top of post_data_legacy() (e.g. body = dumps(body)) fixes both the crash and the unused-import error.
2. flake8 fails the CI gate (flake8 fig exits non-zero)
12 violations, all in the two files below:
* fig/backends/azure/__init__.py: F401 'json.dumps' imported but unused (resolved by fixing #1<#1>)
* fig/backends/workspaceone/serverlog/__init__.py: 5× F401 unused imports + 6× E265 block-comment formatting, from commenting out the handler block (see #3<#3>)
Please split out of this PR
3. WorkspaceONE serverlog change is an unrelated regression
fig/backends/workspaceone/serverlog/__init__.py comments out the entire TLSSysLogHandler setup at module level, which silently disables syslog delivery for all WorkspaceONE users. The stated rationale — PROTOCOL_TLSv1_2 was removed in Python 3.12 — doesn't hold up: the constant is still present (deprecated, not removed) on 3.13/3.14, and this project pins python_requires='>=3.6, <3.12' with CI and the container both on 3.11, so 3.12 isn't a target. This is unrelated to the Azure migration and accounts for 11 of the 12 flake8 failures. Recommend pulling it into its own PR; if WS1 genuinely needs TLS-version handling, switching to ssl.SSLContext / ssl.TLSVersion would be the forward-compatible approach rather than disabling the handler.
4. Version bump to 3.5.1
The fig/__init__.py bump is bundled into a feature PR. The repo's convention is a standalone version-bump PR (e.g. #257<#257>). Please drop it here and let the release process own it.
Minor
* Pin the new dependencies. azure-monitor-ingestion and azure-identity are unpinned, unlike their neighbors. Suggest azure-monitor-ingestion==1.0.4 and azure-identity>=1.25.3 (the versions that resolve and install cleanly on 3.11).
* Python floor. Both packages require Python ≥3.8, which conflicts with the >=3.6 floor in setup.py. Since CI and Docker only build 3.11, recommend updating the documented support range to >=3.8.
* post_data() only catches HttpResponseError. Credential failures (ClientAuthenticationError) raised by upload() will propagate uncaught. A broader catch or a brief note would be worth adding.
Verified and looking good
* Mode-aware config validation in fig/config/__init__.py — required fields enforced correctly per auth method, invalid auth_method rejected.
* Bicep: Monitoring Metrics Publisher role GUID is correct, role assignment scoped to the DCR only (true least privilege), kind: 'Direct' with the DCR-level logsIngestion endpoint is the right pattern, and the cross-subscription module pattern is sound.
* k8s manifest: ServiceAccount + workload-identity annotations/labels are correct, and the matchLabels indentation fix is a nice catch.
* bandit: clean. pylint: clean under the project config (only a non-blocking R0914 in the legacy function).
Once the legacy serialization is fixed, the WS1 change and version bump are split out, and the deps are pinned, this should be in good shape. Thanks again for tackling the Data Collector API deprecation.
—
Reply to this email directly, view it on GitHub<#259?email_source=notifications&email_token=BADD76IMJ2OTAULLZ54FC3T5BK5DHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINJVGU3DIOBUHEZKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-4555648492>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BADD76IPG3LCGD7NGKP7NX35BK5DHAVCNFSNUABFKJSXA33TNF2G64TZHMZTIOBXHEZTANBQHNEXG43VMU5TINRZGQZDANBVHAZKC5QC>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/BADD76KE4AWMZEGUHDDWDQD5BK5DHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINJVGU3DIOBUHEZKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android<https://github.com/notifications/mobile/android/BADD76O2H74YEEVJ354XNEL5BK5DHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINJVGU3DIOBUHEZKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today!
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
Hi David — yep, that's the list:
And honestly no worries on the WS1 slip — easy mistake. Since it was just a local-testing leftover and not a fix you actually need, don't bother splitting it into its own PR; just put the original handler block back. If a real TLS issue with WS1 ever crops up we can deal with it then. Bonus: that revert clears 11 of the 12 flake8 errors on its own, so between it and #1 the lint gate should go green. Ping me for another review when those are in. Thanks! |
|
Yeah, that’s what I did for the WorkspaceONE change.
David
From: nialet ***@***.***>
Reply-To: CrowdStrike/falcon-integration-gateway ***@***.***>
Date: Tuesday, June 23, 2026 at 1:22 PM
To: CrowdStrike/falcon-integration-gateway ***@***.***>
Cc: "Warren, David" ***@***.***>, Author ***@***.***>
Subject: Re: [CrowdStrike/falcon-integration-gateway] CrowdStrike Case: 02501603 (PR #259)
[https://avatars.githubusercontent.com/u/64919342?s=20&v=4]carlosmmatos left a comment (CrowdStrike/falcon-integration-gateway#259)<#259 (comment)>
Hi David — yep, that's the list:
1. Fix legacy mode by re-serializing in post_data_legacy() (body = dumps(body) or similar) so it's back to a JSON string. That also kills the F401 'json.dumps' unused flake8 error.
2. Pin the new deps — azure-monitor-ingestion==1.0.4 and azure-identity>=1.25.3.
3. Drop the 3.5.1 bump from fig/__init__.py — release tooling handles versioning separately.
4. Revert the WorkspaceONE serverlog change.
And honestly no worries on the WS1 slip — easy mistake. Since it was just a local-testing leftover and not a fix you actually need, don't bother splitting it into its own PR; just put the original handler block back. If a real TLS issue with WS1 ever crops up we can deal with it then. Bonus: that revert clears 11 of the 12 flake8 errors on its own, so between it and #1<#1> the lint gate should go green.
Ping me for another review when those are in. Thanks!
—
Reply to this email directly, view it on GitHub<#259?email_source=notifications&email_token=BADD76J3O6TRJFWJBVNNT2T5BLDFJA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINZYGIZDAMRSGIYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4782202220>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BADD76NAI3GRSEZLFGJSMZT5BLDFJAVCNFSNUABFKJSXA33TNF2G64TZHMZTIOBXHEZTANBQHNEXG43VMU5TINRZGQZDANBVHAZKC5QC>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/BADD76OULQ3FWPSM4KFL6TL5BLDFJA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINZYGIZDAMRSGIYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android<https://github.com/notifications/mobile/android/BADD76K2WKGY7GGU4DCEGMT5BLDFJA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINZYGIZDAMRSGIYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today!
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
The PR should be updated.
Cheers,
David
From: nialet ***@***.***>
Reply-To: CrowdStrike/falcon-integration-gateway ***@***.***>
Date: Tuesday, June 23, 2026 at 1:22 PM
To: CrowdStrike/falcon-integration-gateway ***@***.***>
Cc: "Warren, David" ***@***.***>, Author ***@***.***>
Subject: Re: [CrowdStrike/falcon-integration-gateway] CrowdStrike Case: 02501603 (PR #259)
[https://avatars.githubusercontent.com/u/64919342?s=20&v=4]carlosmmatos left a comment (CrowdStrike/falcon-integration-gateway#259)<#259 (comment)>
Hi David — yep, that's the list:
1. Fix legacy mode by re-serializing in post_data_legacy() (body = dumps(body) or similar) so it's back to a JSON string. That also kills the F401 'json.dumps' unused flake8 error.
2. Pin the new deps — azure-monitor-ingestion==1.0.4 and azure-identity>=1.25.3.
3. Drop the 3.5.1 bump from fig/__init__.py — release tooling handles versioning separately.
4. Revert the WorkspaceONE serverlog change.
And honestly no worries on the WS1 slip — easy mistake. Since it was just a local-testing leftover and not a fix you actually need, don't bother splitting it into its own PR; just put the original handler block back. If a real TLS issue with WS1 ever crops up we can deal with it then. Bonus: that revert clears 11 of the 12 flake8 errors on its own, so between it and #1<#1> the lint gate should go green.
Ping me for another review when those are in. Thanks!
—
Reply to this email directly, view it on GitHub<#259?email_source=notifications&email_token=BADD76J3O6TRJFWJBVNNT2T5BLDFJA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINZYGIZDAMRSGIYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4782202220>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BADD76NAI3GRSEZLFGJSMZT5BLDFJAVCNFSNUABFKJSXA33TNF2G64TZHMZTIOBXHEZTANBQHNEXG43VMU5TINRZGQZDANBVHAZKC5QC>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/BADD76OULQ3FWPSM4KFL6TL5BLDFJA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINZYGIZDAMRSGIYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android<https://github.com/notifications/mobile/android/BADD76K2WKGY7GGU4DCEGMT5BLDFJA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINZYGIZDAMRSGIYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today!
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
…gnature helper CI installs via 'pip install -e .[devel]', which reads setup.py's install_requires rather than requirements.txt, so the azure SDK packages were missing during linting and pylint failed with import-errors on the azure imports. Add azure-monitor-ingestion and azure-identity to install_requires to match requirements.txt. Also restore the build_signature() helper (inlined by an earlier commit) so post_data_legacy stays under the pylint too-many-locals threshold, which was the remaining non-zero pylint exit.
Summary
Microsoft has announced the deprecation of the HTTP Data Collector API, which is the mechanism currently used by the Falcon Integration Gateway to transmit detection data to Azure Log Analytics. Per the migration guidance, this API will be retired in the near future. This PR replaces it with the Azure Monitor Logs Ingestion API and transitions authentication from shared key access to RBAC-based identity access.
These changes have been tested in a lab environment and validated end-to-end.
Changes
init.py
build_signature()and the SharedKey-basedpost_data()functionpost_data()using theazure-monitor-ingestionSDK (LogsIngestionClient.upload())post_data_legacy()with the original HMAC implementation for backward compatibilityRuntime.__init__()now constructs the credential andLogsIngestionClientonce at startup based onauth_method, rather than per eventworkload_identity—DefaultAzureCredential, suitable for AKS with Azure Workload Federated Identity; no secrets requiredclient_secret—ClientSecretCredentialwith explicit tenant, client ID, and secretlegacy— original SharedKey path, emits a deprecation warning at startupinit.py
ENV_DEFAULTSentries:AZURE_AUTH_METHOD,AZURE_TENANT_ID,AZURE_CLIENT_ID,AZURE_CLIENT_SECRET,AZURE_DCR_ENDPOINT,AZURE_DCR_IMMUTABLE_IDvalidate_backends()with mode-aware validation; required fields differ per auth methoddefaults.ini
[azure]keys:auth_method = legacy,tenant_id,client_id,client_secret,dcr_endpoint,dcr_immutable_idworkspace_idandprimary_keykeys retained for legacy moderequirements.txt
azure-monitor-ingestionandazure-identityfalcon-integration-gateway.yaml
ServiceAccountresource withazure.workload.identity/client-idandazure.workload.identity/tenant-idannotations to support cross-tenant Workload Federated Identityazure.workload.identity/use: "true"pod label andserviceAccountNamereference to opt the pod into credential injection by the Azure Workload Identity webhookWORKSPACE_IDandPRIMARY_KEYas active secret/env references (retained as commented-out block for legacy mode users)config.inito use the new[azure]keysfalcon-integration-gateway-dcr.bicep (new)
falcon-integration-gateway-table.bicep (new)
FalconIntegrationGatewayLogs_CLcustom table, the Data Collection Rule (kind: Direct), and theMonitoring Metrics Publisherrole assignment scoped to the DCRdcrEndpointanddcrImmutableIdfor direct use in FIG configREADME.md
init.py
3.5.1Bug fixes included
Submitter.log()return value corrected fromdumps(json_data)(string) tojson_data(list) — the SDK'supload()method expects a list, not a pre-serialised stringTLSSysLogHandlerinitialisation commented out at module level to prevent startup failure when WorkspaceOne is not configured;PROTOCOL_TLSv1_2was removed in Python 3.12Testing
workload_identitymode validated on AKS (cross-tenant) — detections confirmed inFalconIntegrationGatewayLogs_CLin Tenant Bclient_secretmode validated with explicit credentialslegacymode validated — deprecation warning logged, detections delivered via original API path