Skip to content

CEF parsing issue on first extension key name #331

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

Closed

Conversation

julthomas
Copy link

Hello Rainer,

Could you please consider merging this PR ? All CEF logs I've seen so far do not have a space before the first extension key. Also I could not find a spec stating that their MUST be a space between the last '|' from the header part and the first extension key name, though leading spaces are okay.

Sample log :

CEF:0|FORCEPOINT|Firewall|1.2.3|1234|FW_Related-Connection|0|in=0 out=52 app=TCP/12345 rt=Jan 30 2020 04:47:01 deviceFacility=Packet Filtering act=Allow deviceInboundInterface=0,0 proto=6 dpt=12345 spt=12 dst=1.2.3.4 src=4.3.2.1 dvchost=9.8.7.6 dvc=9.8.7.6 deviceExternalId=FW-ACME node 1 cs1Label=RuleID cs1=2100123.1 cs2Label=NatRuleId cs2=8123.3

This patch break a test, although I reckon this test case should be invalid.

Regards,
Julien

CEF format does not require a space between the last header field
separator and the first key name of the extensions, eg:

CEF:0|a|b|c|d|e|f|field1=value1 field2=value2

The cefGetHdrField() function already eats the ending separator '|'
after an header field. The ++i increment was loosing the first character
of the first extension key name.
This test fails because of commit "cef: fix parsing of the first
extension key name".

I could not find a spec to confirm that a single trailing space
in extension field is a valid CEF format. I tend to think this
should rightfully be invalid.
@julthomas
Copy link
Author

Closing as it is fixed in commit 6e6a50b "FIX CEF PARSER:" by Emile Duquennoy and it has been merged in master.

@julthomas julthomas closed this Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant