how to reproduce

use latest master version 7d7fa7feac7e24d792db9cecf689557af7f59359

Use the following input:

SecRule ARGS "foo" "id:42,phase:2,block,t:none"

crs_read will crate the following file

- actions:
  - act_arg: '42'
    act_name: id
    act_quote: no_quote
    lineno: 1
  - act_arg: '2'
    act_name: phase
    act_quote: no_quote
    lineno: 1
  - act_name: block
    act_quote: no_quote
    lineno: 1
  - act_arg: none
    act_name: t
    act_quote: no_quote
    lineno: 1
  chained: false
  lineno: 1
  operator: ''
  operator_argument: foo
  type: SecRule
  variables:
  - ARGS

crs_write will throw an exception on this

$ ./crs_write.py testy testz
Parsing CRS structure: testy/test.yaml
Exception catched -  (<class 'KeyError'>, KeyError('oplineno'), <traceback object at 0x108e45280>)
Traceback (most recent call last):
  File "./crs_write.py", line 70, in <module>
    mwriter.generate()
  File "/Users/mirko/.local/share/virtualenvs/mirko-jYbmoHav/lib/python3.7/site-packages/msc_pyparser-0.2.3-py3.7.egg/msc_pyparser.py", line 721, in generate
    self.make_secrule(i)
  File "/Users/mirko/.local/share/virtualenvs/mirko-jYbmoHav/lib/python3.7/site-packages/msc_pyparser-0.2.3-py3.7.egg/msc_pyparser.py", line 699, in make_secrule
    if r['lineno'] < r['oplineno']:
KeyError: 'oplineno'

When replacing the rule with

SecRule ARGS "@rx foo" "id:42,phase:2,block,t:none"

it will work

mirkodziadzka-avi commented 4 years ago

cc: @MirkoDziadzka

mirkodziadzka-avi commented 4 years ago

My proposal would be to parse this short form as if @rx would be there. Then the read/write cycle would modify the rule, but - in my personal opinion - the version with implicit operator should be deprecated.

airween commented 4 years ago

Hi @mirkodziadzka-avi,

so many thanks again for your remark. PR #13 fixes this problem, I close the issue.

mirkodziadzka-avi commented 4 years ago

Thanks for the fix.

I transformed my rulset modsec-lang -> YAML -> modsec-lang -> yaml for verification.

The output from 2 and 4 is identical
If I ignore all whitespaces, the output from 1 and 3 is identical, except minor reformatting

As you did predicted:

ARGS,ARGS -> ARGS|ARGS
SecRule ARGS foo -> SecRule ARGS "foo"

and aditional:

SecRule ARGS "foo" "\
 id:42

became

SecRule ARGS "foo" \
 "id:42

At the end, a very impressive performance:

$ cat original/*.conf | wc -l
  431619
$ diff -w -r original regenerated | wc -l
      22

airween commented 4 years ago

Oh, nice to see the performance :).

Anyway, your additional example can be prevented - the MSCWriter class follows the expected CRS formatting rules, and of course, you can overwrite it as you want.

If you interesting, check the beautifier examples (both has a short text description with same name).

digitalwave / msc_pyparser

crs_write raise exception when generating a file when operator is omitted #12

how to reproduce