[GitHub] [metron] nickwallen opened a new pull request #1570: METRON-2326 Unable to Call ENRICHMENT_GET from Threat Triage Rule Reason Field

[GitBox <gi...@apache.org>]

nickwallen opened a new pull request #1570: METRON-2326 Unable to Call ENRICHMENT_GET from Threat Triage Rule Reason Field
URL: https://github.com/apache/metron/pull/1570
 
 
   A Threat Triage Rule's "reason" field can contain executable Stellar to provide an operator context as to why a rule fired during Threat Triage.  This "reason" field is unable to call any function that requires a `StellarContext` during initialization.  For example, I cannot call the function `ENRICHMENT_GET`.
   
   ## Root Cause
   
   There is some additional Stellar validation performed on a threat triage rule's reason and rule field in `ThreatTriageConfig.setRiskLevelRules`.  In this validation, no `StellarContext` is initialized which causes the `ENRICHMENT_GET` function to fail as described.
   
   ## The Fix
   
   I have found that the additional validation in `ThreatTriageConfig.setRiskLevelRules` is not necessary, nor useful, for validating a rule's reason or rule field.  
   
   This validation is incorrectly throwing exceptions in cases like this where the user has provided a valid Stellar expression. This is because the Stellar execution environment is not configured and initialized in the same manner as the one used to actually run the Threat Triage rules at run-time.
   
   Even if we corrected this, I have found that there are no error conditions that this validation would catch that would not be caught by validation performed elsewhere.
   
   1. Any error condition discoverable at 'compile-time' is checked before the triage rule is loaded into Zk.  If the user attempts to load invalid rules using the REPL or `zk_load_configs`, all 'compile-time' errors that `ThreatTriageConfig.setRiskLevelRules` might catch, are already caught. 
   
       For example, if the "reason" expression calls a non-existent function or uses invalid syntax, an exception will be thrown and the rule will not load. 
   
   2. Any other 'run-time' errors can only be checked when a message's fields are available to the expression.  These sorts of errors would not be caught by the validation in `ThreatTriageConfig.setRiskLevelRules` and instead will be caught by the `ThreatTriageProcessor`.
   
       For example, if the "reason" expression refers to a field that does not exist or attempts to divide by a field that has a value of 0.
   
   
   ## Changes
   
   1. Remove the validation of Stellar expressions from `ThreatTriageConfig.setRiskLevelRules`.
   
   
   ## Acceptance Testing
   
   1. Spin-up the development environment.
   
   1. Turn off all sensor-stubs except snort.
       ```
       service sensor-stubs stop
       service sensor-stubs start snort
       ```
   
   1. Create a file called `user.csv`.
       ```
       [root@node1 ~]# cat user.csv
       jdoe,192.168.138.2
       jane,192.168.66.1
       ciana,192.168.138.158
       danixa,95.163.121.204
       jim,192.168.66.121
       ```
     
   1. Create a file called `user-extractor.json`.
       ```
       {
         "config": {
           "columns": {
             "user": 0,
             "ip": 1
           },
           "indicator_column": "ip",
           "separator": ",",
           "type": "user"
         },
         "extractor": "CSV"
       }
       ```
   
   1. Import the data that we will use for enrichment.
       ```
       source /etc/default/metron
       $METRON_HOME/bin/flatfile_loader.sh -i ./user.csv -t enrichment -c t -e ./user-extractor.json
       ```
   
   1. Validate that the enrichment loaded successfully.
       ```
       [root@node1 0.7.2]# source /etc/default/metron
       [root@node1 0.7.2]# $METRON_HOME/bin/stellar -z $ZOOKEEPER
       
       [Stellar]>>> ip_dst_addr := "192.168.138.2"
       192.168.138.2
       
       [Stellar]>>> ENRICHMENT_GET('user', ip_dst_addr, 'enrichment', 't')
       {ip=192.168.138.2, user=jdoe}
       ```
   
   1. Create a threat triage rule that attempts an ENRICHMENT_GET.
       ```
       [Stellar]>>> conf := SHELL_EDIT()
       {
         "enrichment": {
           "fieldMap": {
             "stellar": {
               "config": {
                 "is_alert": "true"
               }
             }
           },
           "fieldToTypeMap": {},
           "config": {}
         },
         "threatIntel": {
           "fieldMap": {},
           "fieldToTypeMap": {},
           "config": {},
           "triageConfig": {
             "riskLevelRules": [
               {
                 "name": "Rule1",
                 "comment": "This rule should work.",
                 "rule": "true",
                 "reason": "FORMAT('Call to ENRICHMENT_GET=%s', ENRICHMENT_GET('user', ip_dst_addr, 'enrichment', 't'))",
                 "score": "100"
               }
             ],
             "aggregator": "MAX",
             "aggregationConfig": {}
           }
         },
         "configuration": {}
       }
       
       [Stellar]>>> CONFIG_PUT("ENRICHMENT", conf, "snort")
       ```
   
   1. Ensure that the snort telemetry contains a reason field with the enriched value.
       ```
       [Stellar]>>> KAFKA_TAIL( "indexing", 10)
       [{...
         "threat.triage.rules.0.reason":"Call to ENRICHMENT_GET={ip=192.168.138.158, user=ciana}"
         }]
       ```
   
   ## Pull Request Checklist
   
   ### For all changes:
   - [ ] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
   - [ ] Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
   - [ ] Has your PR been rebased against the latest commit within the target branch (typically master)?
   
   
   ### For code changes:
   - [ ] Have you included steps to reproduce the behavior or problem that is being changed or addressed?
   - [ ] Have you included steps or a guide to how the change may be verified and tested manually?
   - [ ] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:
   - [ ] Have you written or updated unit tests and or integration tests to verify your changes?
   - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
   - [ ] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services