tenable / terrascan

Detect compliance and security violations across Infrastructure as Code to mitigate risk before provisioning cloud native infrastructure.
https://runterrascan.io
Apache License 2.0
4.69k stars 496 forks source link

Sarif format tweaks #1063

Open shaopeng-gh opened 2 years ago

shaopeng-gh commented 2 years ago

Description

Hello! this is Shaopeng from Microsoft Sarif team. Our team creates and maintains the Sarif format standard. https://github.com/oasis-tcs/sarif-spec https://github.com/microsoft/sarif-sdk https://github.com/microsoft/sarif-js-sdk https://github.com/microsoft/sarif-website

Glad to see ‎Terrascan supports output of Sarif format. We are using -o sarif option, the output Sarif works well. We do see some minor issues with the result Sarif file, can we suggest a few tweaks so that the output Sarif file can be optimized and better meet the specification?

What I Did

with -o sarif option
cesar-rodriguez commented 2 years ago

Hi @shaopeng-gh,

Thanks for reaching out! Your input to improve this feature would be much appreciated.

shaopeng-gh commented 2 years ago

Thanks @cesar-rodriguez , below are my inputs:

  1. Let's use the latest schema: https://json.schemastore.org/sarif-2.1.0-rtm.5.json I see this is from the package used "owenrumney/go-sarif". I have submitted a PR there. Do we know if the owner of the repo is still active?

  2. Provide "helpUri" if possible helpUri can be added in Rule node, so that user can see related help. For example we are using it to scan ARM templates, I see there are related help in this page like https://docs.accurics.com/projects/accurics-terrascan/en/latest/policies/azure/#azurerm_network_watcher_flow_log. I don't see it is in the code object module. Not sure if you can add it to the Sarif output, it is a nice to have.

  3. The message in the result is the same as in the rules. If that is the case, we can use the "messageStrings" from the rules and just make a linking between results and rules, to simplify. I see the package used does not support this messageStrings so looks like we can not have it. https://github.com/owenrumney/go-sarif/blob/main/sarif/rule.go

fyi the correct usage is like,

in rule node: "messageStrings": { "Default": { "text": "Ensure 'Enforce SSL connection' is set to 'ENABLED' for MySQL Database Server" } } in result node: "message": { "id": "Default" }

  1. The locations should conform with https://tools.ietf.org/html/rfc3986, we currently get "file://C:\\reporoot\\input\\azuredeploy.json" I think this is just a bug in the code for running in Windows, in Linux it is correct. The correct uri path for Windows should be like "file:///C:/reporoot/input/azuredeploy.json" I have submitted a PR for review: https://github.com/accurics/terrascan/pull/1070

  2. The paths should be relative and use originalUriBaseIds if possible, to simplify My understanding is the go-sarif package also does not really have the support for this to be correctly used. It only have the URIBaseId but not the originalUriBaseIds

fyi the correct usage is like,

in result node: "artifactLocation": { "uri": "input/azuredeploy.json", "uriBaseId": "REPO_ROOT" }, the originalUriBaseIds node: "originalUriBaseIds": { "REPO_ROOT": { "uri": "file:///c:/reporoot/" } }

  1. side question I see you have a forGithub version, can you let me know the context, is it that GitHub throw some errors? Bringing this up because may be this is related to the uri invalid issue above, by moving the path to the uriBaseId and only leave the file name in the uri will pass the uri field validation if there is any.

  2. fyi in Sarif .NET SDK we have a Sarif file format validator and the only location url issue above will make the Sarif file as invalid so this one should be fixed. Unfortunately we don't have a SDK for GoLang.

shaopeng-gh commented 2 years ago

update for No.1 about schema version, "owenrumney/go-sarif" has approved my PR and created a new version. We just need to update the package version then. I have updated my PR. #1070