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

feat: add quantitative testing #355

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Conversation

fzipi
Copy link
Member

@fzipi fzipi commented Sep 14, 2024

what

  • 💡 add quantitative testing to the testing tool !
  • run using a local engine instead of parsing logs!
  • let the tool interface with the corpus to test, instead of pointing to a file
  • allow caching corpus files to ease CI/CD testing caching big files
  • plenty of variables and highly configurable from the cmd line
  • [EXPERIMENTAL] interface for adding more corpus
  • right now the tests are run against CRS mostly, but could be extended to additional rulesets

why

  • reuse the tool we use for testing and add more features
  • speed

future

  • get a threshold for knowing what is considered "bad" or "worse" than before
  • probably using go funcs to process corpus in parallel to lower times. right now is just line by line, which probably is really underperformant

@fzipi fzipi added the enhancement New feature or request label Sep 14, 2024
@fzipi fzipi force-pushed the feat/add-quantitative-testing branch from 3fc916a to 1dcebc6 Compare September 14, 2024 18:30
@fzipi fzipi requested a review from theseion September 14, 2024 19:15
@fzipi fzipi force-pushed the feat/add-quantitative-testing branch 3 times, most recently from 9ad2906 to 6de43c0 Compare September 14, 2024 23:37
@fzipi fzipi marked this pull request as ready for review September 14, 2024 23:41
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi fzipi force-pushed the feat/add-quantitative-testing branch from 6de43c0 to 32bd0e9 Compare September 19, 2024 12:38
Copy link
Collaborator

@theseion theseion left a comment

Choose a reason for hiding this comment

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

Really cool!

cmd/quantitative.go Outdated Show resolved Hide resolved
cmd/quantitative.go Outdated Show resolved Hide resolved
cmd/quantitative.go Outdated Show resolved Hide resolved
cmd/quantitative.go Outdated Show resolved Hide resolved
cmd/quantitative.go Outdated Show resolved Hide resolved
internal/quantitative/runner.go Outdated Show resolved Hide resolved
internal/quantitative/runner.go Outdated Show resolved Hide resolved
internal/quantitative/runner.go Outdated Show resolved Hide resolved
internal/quantitative/runner.go Outdated Show resolved Hide resolved
internal/quantitative/stats.go Show resolved Hide resolved
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
@huberts90
Copy link
Collaborator

Could you please link some basic theoretical explanation to this issue to outline the motivation standing behind this PR.
Thanks in advance!

@fzipi
Copy link
Member Author

fzipi commented Sep 21, 2024

Well, I don't know if there is any "theoretical" explanation here, other than plain numbers.

We take a bunch of standard (meaning it doesn't contain attacks) text grabbed from the internet, and we run it against CRS. We get the percentage of the text that matches certain rules.

If you modify a rule and the numbers go up, your change will get more false positives. That's the gist of quantitative testing around rules.

Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi fzipi force-pushed the feat/add-quantitative-testing branch from df968f9 to b921abe Compare September 21, 2024 23:34
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi fzipi force-pushed the feat/add-quantitative-testing branch from b921abe to 56d047d Compare September 22, 2024 00:21
@fzipi fzipi requested a review from theseion September 22, 2024 00:22
@fzipi
Copy link
Member Author

fzipi commented Sep 22, 2024

BTW, this is experimental until we have a good notion on what output we want from the tool.

@theseion
Copy link
Collaborator

@fzipi there are still two unresolved comments from the previous review.

Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi
Copy link
Member Author

fzipi commented Sep 22, 2024

@fzipi there are still two unresolved comments from the previous review.

I always fall in the hidden comments 🤦

Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi fzipi force-pushed the feat/add-quantitative-testing branch from d742042 to 81f0e99 Compare September 22, 2024 19:31
@fzipi fzipi dismissed theseion’s stale review September 22, 2024 19:31

All comments addressed.

Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi
Copy link
Member Author

fzipi commented Sep 23, 2024

  • Added some factory methods
  • Cleaned up outputs
  • Added line number when printing payload in debug mode

fzipi and others added 2 commits September 22, 2024 22:13
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
internal/quantitative/local_engine.go Outdated Show resolved Hide resolved
internal/quantitative/local_engine.go Outdated Show resolved Hide resolved
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
@fzipi fzipi requested a review from theseion September 25, 2024 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants