tests: add rule to check for tcp_mss - v1

[0xEniola]

Test for rule type for tcp-mss keyword

Related to
Issue: #6355

Redmine ticket: https://redmine.openinfosecfoundation.org/issues/6355

Suricata PR: OISF/suricata#9673

[jufajardini]

Right path, but two adjustments needed ;)

[jufajardini]

I don't think this should be commented out.

[jufajardini]

This check will fail with the rule you've provided ;)

[0xEniola]

Actually, I have tried with other working tests, I don't necessarily have to test for name, and the check still passes.

But for this tcp-mss.
Even testing for name doesn't work.

That's where the issue is, I don't get the reason why it does not even recognise tcp.mss as the name, that's why I did that.

Because normally, it will pass even without checking for name matches.

So I feel there's an issue with my code.

[jufajardini]

I got it. It's not with the code.
The check should be for lists, not list ;)
Also, as I pointed out, the size check won't work with the rule you've added for this test.

[jufajardini]

When such failures occur, what I do is open and compare my changes with a similar test, and carefully look for possible differences. That's what I did for this case. ;)

[0xEniola]

I got it. It's not with the code.
The check should be for lists, not list ;)
Also, as I pointed out, the size check won't work with the rule you've added for this test.

Wiiiiiiii!

Thank you for catching that 😂.
It's really funny how the issues always be right in my face but still subtle.

But, why isn't the size check working though? I've looked through it as well; it says Rule matches on packets rules_analysis.txt but still fails.

[0xEniola]

OK! I was tinkering around, and figured I'd have to include a test for all the options before the test could pass, which means a test for the mode, arg1 and arg2.

Which means, for each option, the code to log them must be present.

So I did jb_set_uint for each of them.

And found out mode: 1 = <, mode: 3 = >, and mode: 5 is used for min-max.

I'm not sure if arg2 is used for tcp.mss, but if it is, it is indeed better that you add a test to showcase that.
Great catch about the mode bit. As I am not familiar with this type of keyword, I wondered how the >/</ and similar was being represented, but didn't investigate further. This should definitely be present, thanks for pointing out! It also means there's more work to be done on the Suricata side of this task, as you will have to first log these fields, if you want to test the output here. I'll update my review there! :)

Yes! I have added the tests locally here.

But then, about adding the function that will relate the mode bits to their corresponding characters for setting up a proper configuration file, I would like to suggest;

• lt for Less than (< value).
• gt for Greater than (> value).
• rg or range for Range (min_value - max_value).

[jufajardini]

Thanks for asking this.
Any reason not to add the full sentence? It feels to me that this makes it slightly harder to understand, in a situation where I'm not sure there's a need for concision - since this mode is only used for testing, so output size isn't so critical, I think 🤔

[jufajardini]

We probably have also >= and <=, besides exact value too, right?

[0xEniola]

We probably have also >= and <=, besides exact value too, right?

Yes, there are these and even !, but the format provided for tcp.mss in the userguide for header keyword rules, only those three were given.

[0xEniola]

Thanks for asking this.
Any reason not to add the full sentence? It feels to me that this makes it slightly harder to understand, in a situation where I'm not sure there's a need for concision - since this mode is only used for testing, so output size isn't so critical, I think 🤔

Oh yes! Actually, understanding is paramount.

I'll change it to full sentences then.

• less than
• greater than
• range

[0xEniola]

When suricata-verify is done running, it says “FAILED”, but shows that “Rule matches on packet” in the rules_analysis.txt file.

[jufajardini]

When suricata-verify is done running, it says “FAILED”, but shows that “Rule matches on packet” in the rules_analysis.txt file.

Can you share the complete output for that, please? I'll double check your code and see if I can figure out what's going on.

[0xEniola]

When suricata-verify is done running, it says “FAILED”, but shows that “Rule matches on packet” in the rules_analysis.txt file.

Can you share the complete output for that, please? I'll double check your code and see if I can figure out what's going on.

Which of the output? rules. json or rules_analysis.txt?

[jufajardini]

When suricata-verify is done running, it says “FAILED”, but shows that “Rule matches on packet” in the rules_analysis.txt file.

Can you share the complete output for that, please? I'll double check your code and see if I can figure out what's going on.

Which of the output? rules. json or rules_analysis.txt?

The one you mentioned ;)

But I think at least we've figured why the test was failing.