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

Rework of #10768 - PR: Merge macos-sshd ruleset into sshd ruleset #11388

Merged
merged 10 commits into from Jan 17, 2022

Conversation

fabamatic
Copy link
Contributor

@fabamatic fabamatic commented Dec 16, 2021

Related issue
#10768

Description

This PR aims to resolve issues detected under #10768 PR.
The issues resolved are:

  • Fixed rules descriptions
  • Fixed indentation issues.
  • Fixed rule and test header
  • Fixed .ini filename
  • Fixed test placed in wrong file

Checks and changes

Syntax

  • 1.a Rule tags order must be compliant.
  • 1.b Decoder tags order must be compliant.
  • 1.c XML blocks must as compact as possible.
  • 1.d Only one empty line between rule/decoder and the next rule/decoder.
  • 1.e Decoder extracted fields names must use '_' whether a space is needed.
  • 1.f Decoder name must use '-' whether a space is needed.

Grammar

  • 2.a Grammar quality.
  • 2.b Similar phrases must keep tenses and expressions.
  • 2.c Grammar basic rules like capitalization, punctuation marks, sentence construction, and others.

Semantic

  • 3.a New decoder, rule, or test are written in the correct file and grouped correctly inside the file.
  • 3.b Group similar rules under same ID's group.
  • 3.c:
    • 3.c1 Find and reuse group name before creating a new one.
    • 3.c2 Include a new group definition in a PR comment.
    • 3.c3 Any group name must use '_' whether it does need a space char.
    • 3.c4 The groups inside a tag must be sorted in alphabetical order.
  • 3.d Rule level should be compliant with the documentation.

Unit testing

  • 4.a A metadata block at the beginning of the .ini file describing software, version, and logs source.
  • 4.b Each new rule must have at least one test entry in the correct .ini file.
  • 4.c Runtest.py must pass and must include the results in raw format here.
All rules and decoders test.

- [ File = ./tests/sophos_fw.ini ] ---------
..........

- [ File = ./tests/dropbear.ini ] ---------
...

- [ File = ./tests/squid_rules.ini ] ---------
..

- [ File = ./tests/sudo.ini ] ---------
........

- [ File = ./tests/gitlab.ini ] ---------
.........................

- [ File = ./tests/glpi.ini ] ---------
...

- [ File = ./tests/unbound.ini ] ---------


- [ File = ./tests/mailscanner.ini ] ---------
.

- [ File = ./tests/samba.ini ] ---------
....

- [ File = ./tests/cisco_ios.ini ] ---------
.................

- [ File = ./tests/cimserver.ini ] ---------
..

- [ File = ./tests/audit_recon.ini ] ---------
..

- [ File = ./tests/fortigate.ini ] ---------
........................................

- [ File = ./tests/cisco_asa.ini ] ---------
........................................................................................

- [ File = ./tests/systemd.ini ] ---------
..

- [ File = ./tests/openldap.ini ] ---------
.........

- [ File = ./tests/oscap.ini ] ---------
................................

- [ File = ./tests/github.ini ] ---------
....................................................................................................................................................................................................................................................................................................................................

- [ File = ./tests/pfsense.ini ] ---------
..

- [ File = ./tests/apache.ini ] ---------
..................

- [ File = ./tests/api.ini ] ---------
............................

- [ File = ./tests/SonicWall.ini ] ---------
........

- [ File = ./tests/rsh.ini ] ---------
..

- [ File = ./tests/auditd.ini ] ---------
...

- [ File = ./tests/junos.ini ] ---------
...

- [ File = ./tests/owlh.ini ] ---------
....

- [ File = ./tests/pam.ini ] ---------
.....

- [ File = ./tests/sophos.ini ] ---------
........

- [ File = ./tests/modsecurity.ini ] ---------
......

- [ File = ./tests/paloalto.ini ] ---------
................

- [ File = ./tests/cylance.ini ] ---------
.......

- [ File = ./tests/pix.ini ] ---------
......................

- [ File = ./tests/sophos-utm-firewall.ini ] ---------
......

- [ File = ./tests/exim.ini ] ---------
.......

- [ File = ./tests/freepbx.ini ] ---------
......

- [ File = ./tests/trendmicro-cloud-one.ini ] ---------
.................

- [ File = ./tests/kernel_usb.ini ] ---------
......

- [ File = ./tests/netscreen.ini ] ---------
....

- [ File = ./tests/panda_paps.ini ] ---------
........

- [ File = ./tests/nextcloud.ini ] ---------
.......

- [ File = ./tests/aws_s3_access.ini ] ---------
.......

- [ File = ./tests/doas.ini ] ---------
....

- [ File = ./tests/icinga.ini ] ---------
....

- [ File = ./tests/apparmor.ini ] ---------
.....

- [ File = ./tests/sshd.ini ] ---------
.............................................

- [ File = ./tests/opensmtpd.ini ] ---------
.......

- [ File = ./tests/proftpd.ini ] ---------
.......

- [ File = ./tests/named.ini ] ---------
.....

- [ File = ./tests/syslog.ini ] ---------
......

- [ File = ./tests/checkpoint_smart1.ini ] ---------
..................

- [ File = ./tests/fireeye.ini ] ---------
...

- [ File = ./tests/office365.ini ] ---------
................................................................................................................................

- [ File = ./tests/vsftpd.ini ] ---------
....

- [ File = ./tests/openvpn_ldap.ini ] ---------
..

- [ File = ./tests/cpanel.ini ] ---------
.......

- [ File = ./tests/audit_scp.ini ] ---------
.

- [ File = ./tests/cloudlfare-waf.ini ] ---------
.............

- [ File = ./tests/web_appsec.ini ] ---------
...............................

- [ File = ./tests/fortiauth.ini ] ---------
....

- [ File = ./tests/cisco_ftd.ini ] ---------
..........................................

- [ File = ./tests/ossec.ini ] ---------
.....

- [ File = ./tests/arbor.ini ] ---------
..

- [ File = ./tests/huawei_usg.ini ] ---------
...

- [ File = ./tests/exchange.ini ] ---------
..

- [ File = ./tests/postfix.ini ] ---------
..

- [ File = ./tests/su.ini ] ---------
.....

- [ File = ./tests/vuln_detector.ini ] ---------
..

- [ File = ./tests/f5_big_ip.ini ] ---------
...........................................

- [ File = ./tests/mcafee_epo.ini ] ---------
.

- [ File = ./tests/audit_lateral.ini ] ---------
......

- [ File = ./tests/sysmon.ini ] ---------
...

- [ File = ./tests/web_rules.ini ] ---------
..........

- [ File = ./tests/iptables.ini ] ---------
........

- [ File = ./tests/eset.ini ] ---------
........

- [ File = ./tests/gcp.ini ] ---------
...........

- [ File = ./tests/firewalld.ini ] ---------
..

- [ File = ./tests/dovecot.ini ] ---------
...............

- [ File = ./tests/nginx.ini ] ---------
............

|Component |  Tested  |  Total   | Coverage |
| -------- | -------- | -------- | -------- |
|  Rules   |   984    |   4175   |  23.57%  |
| Decoders |    77    |   172    |  44.77%  |

|          File           |  Passed  |  Failed  |  Status  |
|        --------         | -------- | -------- | -------- |
|./tests/sysmon.ini       |    3     |    0     |   ✅    |
|./tests/proftpd.ini      |    7     |    0     |   ✅    |
|./tests/glpi.ini         |    3     |    0     |   ✅    |
|./tests/dovecot.ini      |    15    |    0     |   ✅    |
|./tests/exim.ini         |    7     |    0     |   ✅    |
|./tests/huawei_usg.ini   |    3     |    0     |   ✅    |
|./tests/squid_rules.ini  |    2     |    0     |   ✅    |
|./tests/pix.ini          |    22    |    0     |   ✅    |
|./tests/samba.ini        |    4     |    0     |   ✅    |
|./tests/mcafee_epo.ini   |    1     |    0     |   ✅    |
|./tests/checkpoint_smart1.ini|    18    |    0     |   ✅    |
|./tests/pfsense.ini      |    2     |    0     |   ✅    |
|./tests/iptables.ini     |    8     |    0     |   ✅    |
|./tests/cisco_asa.ini    |    88    |    0     |   ✅    |
|./tests/dropbear.ini     |    3     |    0     |   ✅    |
|./tests/systemd.ini      |    2     |    0     |   ✅    |
|./tests/SonicWall.ini    |    8     |    0     |   ✅    |
|./tests/panda_paps.ini   |    8     |    0     |   ✅    |
|./tests/mailscanner.ini  |    1     |    0     |   ✅    |
|./tests/owlh.ini         |    4     |    0     |   ✅    |
|./tests/fortiauth.ini    |    4     |    0     |   ✅    |
|./tests/cisco_ios.ini    |    17    |    0     |   ✅    |
|./tests/vuln_detector.ini|    2     |    0     |   ✅    |
|./tests/cloudlfare-waf.ini|    13    |    0     |   ✅    |
|./tests/gcp.ini          |    11    |    0     |   ✅    |
|./tests/openldap.ini     |    9     |    0     |   ✅    |
|./tests/unbound.ini      |    0     |    0     |   ✅    |
|./tests/oscap.ini        |    32    |    0     |   ✅    |
|./tests/f5_big_ip.ini    |    43    |    0     |   ✅    |
|./tests/sophos.ini       |    8     |    0     |   ✅    |
|./tests/opensmtpd.ini    |    7     |    0     |   ✅    |
|./tests/exchange.ini     |    2     |    0     |   ✅    |
|./tests/icinga.ini       |    4     |    0     |   ✅    |
|./tests/netscreen.ini    |    4     |    0     |   ✅    |
|./tests/doas.ini         |    4     |    0     |   ✅    |
|./tests/rsh.ini          |    2     |    0     |   ✅    |
|./tests/arbor.ini        |    2     |    0     |   ✅    |
|./tests/office365.ini    |   128    |    0     |   ✅    |
|./tests/paloalto.ini     |    16    |    0     |   ✅    |
|./tests/cylance.ini      |    7     |    0     |   ✅    |
|./tests/fortigate.ini    |    40    |    0     |   ✅    |
|./tests/ossec.ini        |    5     |    0     |   ✅    |
|./tests/web_rules.ini    |    10    |    0     |   ✅    |
|./tests/sudo.ini         |    8     |    0     |   ✅    |
|./tests/audit_lateral.ini|    6     |    0     |   ✅    |
|./tests/cisco_ftd.ini    |    42    |    0     |   ✅    |
|./tests/github.ini       |   324    |    0     |   ✅    |
|./tests/postfix.ini      |    2     |    0     |   ✅    |
|./tests/syslog.ini       |    6     |    0     |   ✅    |
|./tests/kernel_usb.ini   |    6     |    0     |   ✅    |
|./tests/api.ini          |    28    |    0     |   ✅    |
|./tests/sophos-utm-firewall.ini|    6     |    0     |   ✅    |
|./tests/apparmor.ini     |    5     |    0     |   ✅    |
|./tests/audit_scp.ini    |    1     |    0     |   ✅    |
|./tests/nginx.ini        |    12    |    0     |   ✅    |
|./tests/named.ini        |    5     |    0     |   ✅    |
|./tests/sophos_fw.ini    |    10    |    0     |   ✅    |
|./tests/openvpn_ldap.ini |    2     |    0     |   ✅    |
|./tests/cimserver.ini    |    2     |    0     |   ✅    |
|./tests/vsftpd.ini       |    4     |    0     |   ✅    |
|./tests/freepbx.ini      |    6     |    0     |   ✅    |
|./tests/sshd.ini         |    45    |    0     |   ✅    |
|./tests/junos.ini        |    3     |    0     |   ✅    |
|./tests/modsecurity.ini  |    6     |    0     |   ✅    |
|./tests/nextcloud.ini    |    7     |    0     |   ✅    |
|./tests/fireeye.ini      |    3     |    0     |   ✅    |
|./tests/cpanel.ini       |    7     |    0     |   ✅    |
|./tests/trendmicro-cloud-one.ini|    17    |    0     |   ✅    |
|./tests/pam.ini          |    5     |    0     |   ✅    |
|./tests/su.ini           |    5     |    0     |   ✅    |
|./tests/apache.ini       |    18    |    0     |   ✅    |
|./tests/gitlab.ini       |    25    |    0     |   ✅    |
|./tests/aws_s3_access.ini|    7     |    0     |   ✅    |
|./tests/firewalld.ini    |    2     |    0     |   ✅    |
|./tests/eset.ini         |    8     |    0     |   ✅    |
|./tests/auditd.ini       |    3     |    0     |   ✅    |
|./tests/web_appsec.ini   |    31    |    0     |   ✅    |
|./tests/audit_recon.ini  |    2     |    0     |   ✅    |


  • 4.d New CDB lists must include the proper test entry in the correct .ini file.

E2E testing

  • 5.a Logs for new or modified decoder/rules sent to a manager running with this PR ruleset appear in Kibana.

  • 5.b There are not affected or broken visualizations on Kibana.

  • 5.c New or modified items can be seen correctly using APP ruleset navigation.

Elasticsearch Template

  • 6.a Known fields with output format managed and usually used for searching are included in template array index.query.default_field.
  • 6.b The new field with the correct date format is stored as a "date" type field in the template.
  • 6.c The new extracted IP fields are in the pipeline as "geo" field and "geo_point" type in the template.
  • 6.d Known fields with output format managed and usually used for searching are included in the template.

Stoppers

  • 7.a No previous rule ID changes without triple check.
  • 7.b No previous decoder name changes without triple check.
  • 7.c No previous file name changes without triple check.
  • 7.d No previous test changes its 'rule' field value without triple check.

Others

  • 8.a Each file has the correct copyright block.
  • 8.b The copyright block doesn't have "Author" only "Created by Wazuh". To include an "Author" request triple check.
  • 8.c The copyright block doesn't use "-".
  • 8.d The rule files don't have any sample log.
  • 8.e The decoder file has sample logs next to the decoder that matches that log.
  • 8.f The decoder and rule files have information about software, version, and any helpful information.
  • 8.g The PR has a single commit with CHANGELOG changes in the correct format.
  • 8.h The new rule ID is not in use.
  • 8.i The new rule ID must be in the defined IDs range.
  • 8.j New rules ID range must be verified with a triple check and noted in the rules ID document.
  • 8.k The new extracted IP fields are in the pipeline as "geo" field and "geo_point" type in the template.

Copy link
Contributor

@jcruzlp jcruzlp left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@72nomada 72nomada left a comment

Choose a reason for hiding this comment

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

LGTM

ruleset/rules/0095-sshd_rules.xml Outdated Show resolved Hide resolved
ruleset/rules/0095-sshd_rules.xml Outdated Show resolved Hide resolved
ruleset/rules/0095-sshd_rules.xml Outdated Show resolved Hide resolved
Copy link
Member

@nmkoremblum nmkoremblum left a comment

Choose a reason for hiding this comment

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

Description labels should be as compact as possible

fabamatic and others added 2 commits January 12, 2022 17:15
Co-authored-by: Nicolás Mariano Koremblum <nmkoremblum@gmail.com>
ruleset/rules/0095-sshd_rules.xml Outdated Show resolved Hide resolved
Copy link
Member

@nmkoremblum nmkoremblum left a comment

Choose a reason for hiding this comment

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

Copyright is missing in ruleset/testing/tests/test_features.ini.

Copy link
Member

@nmkoremblum nmkoremblum left a comment

Choose a reason for hiding this comment

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

Alerts' level should be reviewed, there are many level 0 alerts that seem to be wrong.

@JcabreraC JcabreraC self-requested a review January 13, 2022 15:32
Copy link
Member

@JcabreraC JcabreraC left a comment

Choose a reason for hiding this comment

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

Missing test for rules 5763 and 5764

ruleset/rules/0095-sshd_rules.xml Outdated Show resolved Hide resolved
ruleset/rules/0095-sshd_rules.xml Outdated Show resolved Hide resolved
Copy link
Member

@JcabreraC JcabreraC left a comment

Choose a reason for hiding this comment

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

Missing copyright in ruleset/testing/tests/test_features.ini file.

@fabamatic
Copy link
Contributor Author

fabamatic commented Jan 13, 2022

Missing test for rules 5763 and 5764

Deleted these rules as they are exactly the same as 5718 and 5719.
Added test for 5719

  • [ File = ./tests/sshd.ini ] ---------
    ................................................
Component Tested Total Coverage
Rules 31 4181 0.74%
Decoders 1 172 0.58%
File Passed Failed Status
./tests/sshd.ini 48 0

Copy link
Member

@nmkoremblum nmkoremblum left a comment

Choose a reason for hiding this comment

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

There are commented decoders, please remove them.

Copy link
Member

@nmkoremblum nmkoremblum left a comment

Choose a reason for hiding this comment

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

Former rule 5765's ID was changed to 5763 but its test was not modified.

@fabamatic
Copy link
Contributor Author

Former rule 5765's ID was changed to 5763 but its test was not modified.

Done

ruleset/rules/0095-sshd_rules.xml Outdated Show resolved Hide resolved
ruleset/rules/0095-sshd_rules.xml Outdated Show resolved Hide resolved
Copy link
Member

@JcabreraC JcabreraC left a comment

Choose a reason for hiding this comment

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

LGTM !

Copy link
Member

@juliancnn juliancnn left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@nmkoremblum nmkoremblum left a comment

Choose a reason for hiding this comment

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

LGTM :)

@davidjiglesias davidjiglesias marked this pull request as ready for review January 14, 2022 14:23
@juliancnn juliancnn self-requested a review January 14, 2022 16:21
@davidjiglesias davidjiglesias merged commit 582e1f8 into 4.3 Jan 17, 2022
@davidjiglesias davidjiglesias deleted the 10768-ssh-rules-rework branch January 17, 2022 13:22
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.

None yet

7 participants