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

Update azuredeploy.json for RecordedFuture-ThreatIntelligenceImport to conform with request body required #9947

Conversation

jusso-dev
Copy link
Contributor

@jusso-dev jusso-dev commented Feb 13, 2024

The current API spec expects an array of "indicators" and string of "sourcesystem" params, currently this is passing the value of STIX indicators as a named array "indicators" but needs to be "value" as per the spec - https://learn.microsoft.com/en-us/azure/sentinel/upload-indicators-api#sample-request-body

Required items, please complete

Change(s):

  • Updated RecordedFuture-ThreatIntelligenceImport azuredeploy.json to meet API spec

Reason for Change(s):

Version Updated:

  • Required only for Detections/Analytic Rule templates
  • See guidance below

Testing Completed:

  • Deploy azuredeploy.json and validate change

Checked that the validations are passing and have addressed any issues that are present:

  • I have successfully deployed with this change and it resolved the 400 bad request errors

The current API spec expects an array of indicators and sourcesystem params, currently this is passing the value of STIX indicators as a named array "indicators" but needs to be "value" as per the spec - https://learn.microsoft.com/en-us/azure/sentinel/upload-indicators-api#sample-request-body
@jusso-dev jusso-dev requested review from a team as code owners February 13, 2024 07:22
@v-atulyadav v-atulyadav added Playbook Playbook specialty review needed Solution Solution specialty review needed labels Feb 13, 2024
@v-prasadboke
Copy link
Contributor

Thanks for raising the PR, We will investigate this PR and update you about the same before 15 February, 2024.

@v-prasadboke
Copy link
Contributor

Hello @jusso-dev, Please accept license agreement

@jusso-dev
Copy link
Contributor Author

@microsoft-github-policy-service agree

@v-atulyadav v-atulyadav merged commit a608248 into Azure:master Feb 19, 2024
29 checks passed
@RecordedFutureOskbo
Copy link
Contributor

@jusso-dev @v-prasadboke @v-atulyadav
This change makes the RecordedFuture-ThreatIntelligenceImport logic app fail. The Solutions/Recorded Future/Playbooks/IndicatorImport/RecordedFuture-ThreatIntelligenceImport/azuredeploy.json goes into a logic app block that seems to only accept lists of indicators:

JSON body - { "sourcesystem":"AnySource", "indicators": [] }

image

This is the error message we get when using value. I guess there are logic in the logic app that converts indicators -> value behind the scenes. I reached out to one of the logic apps devs whom I had contacts with before about this confusion.

{
  "error": {
    "code": "UploadIndicatorsValidationErrors",
    "message": "Indicators array is required and cannot be empty. ",
    "target": null,
    "additionalInfo": null
  },
  "debugInfo": "clientRequestId: 73a8a8fc-cb6f-418f-9def-46955751190e"
}

@jusso-dev
Copy link
Contributor Author

jusso-dev commented Feb 22, 2024

@RecordedFutureOskbo is it possible the connector versions are different targeting a different API version under the covers? I have this deployed in my clients tenant and I'm getting the opposite of that error when I try and use "indicators" as the named array.

Secondly, the current api spec suggest "value" is the current accepted named array parameter, which makes me further believe there is an API version mismatch happening somewhere.

@RecordedFutureOskbo
Copy link
Contributor

@RecordedFutureOskbo is it possible the connector versions are different targeting a different API version under the covers? I have this deployed in my clients tenant and I'm getting the opposite of that error when I try and use "indicators" as the named array.

Secondly, the current api spec suggest "value" is the current accepted named array parameter, which makes me further believe there is an API version mismatch happening somewhere.

We have seen differences using different datacenters? I'm testing this in west-europe and east-us.

@RecordedFutureOskbo
Copy link
Contributor

RecordedFutureOskbo commented Feb 22, 2024

@jusso-dev are you using the deprecated version?
image

Also, the documentation on the logic app does not specify the data format at all
https://learn.microsoft.com/en-us/connectors/azuresentinel/#threat-intelligence---upload-indicators-of-compromise-(v2)-(preview) (╯°□°)╯︵ ┻━┻

@jusso-dev
Copy link
Contributor Author

jusso-dev commented Feb 22, 2024

@RecordedFutureOskbo no I was using the latest version available at - Solutions/Recorded Future/Playbooks/IndicatorImport/RecordedFuture-ThreatIntelligenceImport/azuredeploy.json

Here's the current working, deployed Logic App:

image

The API doco I was referring to specifies that the "value" named array parameter is the required named array parameter to be passed - https://learn.microsoft.com/en-us/azure/sentinel/upload-indicators-api#sample-request-body

Edit - and when I try using "indicators" as the named array, and the latest plugin by manually selecting the latest "Action" task in the Logic App I get the following error:

image

@jusso-dev
Copy link
Contributor Author

@RecordedFutureOskbo is it possible the connector versions are different targeting a different API version under the covers? I have this deployed in my clients tenant and I'm getting the opposite of that error when I try and use "indicators" as the named array.
Secondly, the current api spec suggest "value" is the current accepted named array parameter, which makes me further believe there is an API version mismatch happening somewhere.

We have seen differences using different datacenters? I'm testing this in west-europe and east-us.

We are using Australia East and it's working with "value" as a named array parameter.

@RecordedFutureOskbo
Copy link
Contributor

This is all very strange, I reached out to Microsoft and this is the response I got back.

When you use the V2 action, it requires a "sourcesystem" and an "indicators" array, whereas when you use the older "Deprecated" action, it requires "sourcesystem" and a "value" array in the request body. Please let me know if this change is still causing you errors.

I also created a logic app in Australia East and it requested indicators

image

Are you using standard or consumption logic apps? I run consumption when testing.

@jusso-dev
Copy link
Contributor Author

That is strange, I'm definitely using the current connector action and it's a consumption Logic App in Australia East, I get the above error if I change it to "indicators" so I'm at a loss what's causing this. The target LAW is in Aus Central I don't know if that is related to this issue?

@RecordedFutureOskbo
Copy link
Contributor

Message from Microsoft Sentinel team:

I found the bug, it was an api endpoint mismatch only in the Australia regions. The fix is checked in, and it should be deployed on Monday! You should be able to use the logic app actions as expected in all other regions.

Still working on updating the api spec.

@jusso-dev
Copy link
Contributor Author

jusso-dev commented Feb 24, 2024

@RecordedFutureOskbo excellent! Thank you for following up on this and finding a fix 😊

I'll revert this PR in another PR

EDIT - PR to revert this raised - #10037

@RecordedFutureOskbo
Copy link
Contributor

Great, thanks @jusso-dev I'm afk until Tuesday.

@jusso-dev
Copy link
Contributor Author

Hi @v-prasadboke and @v-atulyadav, any ETA on getting the API spec updated? This is causing a lot of confusion.

@RecordedFutureOskbo
Copy link
Contributor

I got the following message about the spec @jusso-dev. Some confusion can depend on we are not using the API directly. We are using the logic-app provided in power platform -> https://learn.microsoft.com/en-us/connectors/azuresentinel/#threat-intelligence---upload-indicators-of-compromise-(v2)-(preview) where the spec is lacking format.

So the API spec is probably correct if you use the API directly via CURL or other request libs.

// Oskar

@jusso-dev
Copy link
Contributor Author

Thanks Oskar that makes sense, I think some clarification even if it's a sentence or two would greatly improve the documentation.

@austinmccollum
Copy link
Contributor

updated the upload indicators api doc to reflect the different endpoint and array name depending on the logic app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Playbook Playbook specialty review needed Solution Solution specialty review needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants