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

Detect generic uint 4112 v7 #7305

Closed

Conversation

catenacyber
Copy link
Contributor

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/4112
https://redmine.openinfosecfoundation.org/issues/2697

Describe changes:

  • Makes use of generic DetectUint structure for dsize and dcerpc, ttl, tcpmss, filesize, streamsize (and template2)
  • Uses pre filter for streamsize on the way

Replaces #7302 with stream size addition

Further work, but this PR can already be merged:

  • more keywords in C use specific versions, but they are more complex than just an integer
  • look for uses of set_uint in loggers to see if we can easily a new keywords
> git grep _LT src/*.h | grep DETEC
src/detect-ipproto.h:#define DETECT_IPPROTO_OP_LT     '<' /**< "less than" operator */
src/detect-iprep.h:#define DETECT_IPREP_OP_LT        0
src/detect-tls-cert-validity.h:#define DETECT_TLS_VALIDITY_LT (1<<1) /* less than */
src/detect-urilen.h:#define DETECT_URILEN_LT   0   /**< "less than" operator */

suricata-verify-pr: 809

Ticket: 4112

Move it away from http2 to generic core crate.
And use it for DCERPC (and SMB)

And remove the C version.
Main change in API is the free function is not free itself, but
a rust wrapper around unbox.
@codecov
Copy link

codecov bot commented Apr 25, 2022

Codecov Report

Merging #7305 (9fa86a4) into master (ddf9c9d) will decrease coverage by 0.13%.
The diff coverage is 73.06%.

@@            Coverage Diff             @@
##           master    #7305      +/-   ##
==========================================
- Coverage   75.82%   75.69%   -0.14%     
==========================================
  Files         656      654       -2     
  Lines      190051   189055     -996     
==========================================
- Hits       144102   143101    -1001     
- Misses      45949    45954       +5     
Flag Coverage Δ
fuzzcorpus 60.03% <60.69%> (-0.38%) ⬇️
suricata-verify 51.68% <34.28%> (+0.14%) ⬆️
unittests 61.01% <54.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@jasonish
Copy link
Member

Minor nit: Our current convention, at least by looking at git log is to have the ticket # as the last line.

@catenacyber
Copy link
Contributor Author

Minor nit: Our current convention, at least by looking at git log is to have the ticket # as the last line.

@victorjulien What is the right format ?
Bug or Ticket ?
With a sharp or not ?
Is it documented somewhere ?

Ticket: 4112
Ticket: 2697

By the way, adds the prefilter feature
@suricata-qa
Copy link

ERROR:
ERROR: Invalid Signature config error in tlpr1_asan_cfg QA test

ERROR: QA failed on tlpr1_asan_cfg.

Pipeline 7171

@victorjulien
Copy link
Member

Needs investigation of the qalab failures

@victorjulien
Copy link
Member

Minor nit: Our current convention, at least by looking at git log is to have the ticket # as the last line.

@victorjulien What is the right format ? Bug or Ticket ? With a sharp or not ? Is it documented somewhere ?

Not documented yet, but here is the regex from the new check-ticket script

"(Task|Feature|Ticket|Bug|Issue)\:?\s+#?(\d+)"

@victorjulien victorjulien marked this pull request as draft April 30, 2022 06:10
@catenacyber
Copy link
Contributor Author

Not documented yet, but here is the regex from the new check-ticket script

"(Task|Feature|Ticket|Bug|Issue)\:?\s+#?(\d+)"

Ok, is there a difference between the different wordings ?
I guess I have been using Ticket every time

@catenacyber
Copy link
Contributor Author

ERROR: ERROR: Invalid Signature config error in tlpr1_asan_cfg QA test

ERROR: QA failed on tlpr1_asan_cfg.

Pipeline 7171

@pevma @ct0br0 it looks like old rules are still used in the QA :

$ sudo $SPREFIX/bin/suricata -c $SYAML -T $SARGS
[33](https://gitlab.oisf.net/dev/suricata-ci-trex/-/jobs/156625#L33)[2273626] 26/4/2022 -- 01:33:20 - (suricata.c:1729) <Info> (ParseCommandLine) -- Running suricata under test mode
[34](https://gitlab.oisf.net/dev/suricata-ci-trex/-/jobs/156625#L34)[2273626] 26/4/2022 -- 01:33:20 - (suricata.c:1142) <Notice> (LogVersion) -- This is Suricata version 7.0.0-dev (9fa86a4e5 2022-04-25) running in SYSTEM mode
[35](https://gitlab.oisf.net/dev/suricata-ci-trex/-/jobs/156625#L35)[2273626] 26/4/2022 -- 01:33:36 - (detect-engine-loader.c:185) <Error> (DetectLoadSigFile) -- [ERRCODE: SC_ERR_INVALID_SIGNATURE(39)] - error parsing signature "alert udp $HOME_NET 1234 -> $EXTERNAL_NET 4000 (msg:"ET MALWARE NAZAR EYService Pong response"; id:1; ttl:<0; content:"101|3b|0000|3b|"; reference:url,blog.malwarelab.pl/posts/nazar_eyservice_comm; classtype:command-and-control; sid:2030055; rev:2; metadata:attack_target Client_Endpoint, created_at 2020_04_29, deployment Perimeter, former_category MALWARE, performance_impact Low, signature_severity Major, updated_at 2020_04_29;)" from file /opt/gitrun/suri/var/lib/suricata/rules/suricata.rules at line 68749
[36](https://gitlab.oisf.net/dev/suricata-ci-trex/-/jobs/156625#L36)[2273626] 26/4/2022 -- 01:33:40 - (detect-engine-loader.c:185) <Error> (DetectLoadSigFile) -- [ERRCODE: SC_ERR_INVALID_SIGNATURE(39)] - error parsing signature "alert udp $HOME_NET 1234 -> $EXTERNAL_NET 4000 (msg:"ET MALWARE NAZAR EYService OSInfo response"; id:1; ttl:<0; content:"100|3b|"; reference:url,blog.malwarelab.pl/posts/nazar_eyservice_comm; classtype:command-and-control; sid:2030056; rev:2; metadata:attack_target Client_Endpoint, created_at 2020_04_29, deployment Perimeter, former_category MALWARE, performance_impact Low, signature_severity Major, updated_at 2020_08_19;)" from file /opt/gitrun/suri/var/lib/suricata/rules/suricata.rules at line 74939
[37](https://gitlab.oisf.net/dev/suricata-ci-trex/-/jobs/156625#L37)[2273626] 26/4/2022 -- 01:34:39 - (suricata.c:2330) <Error> (LoadSignatures) -- [ERRCODE: SC_ERR_NO_RULES_LOADED(43)] - Loading signatures failed.

Could you fix this ?

@pevma
Copy link
Member

pevma commented Apr 30, 2022

Yes.I have open an internal ticket to investigate.
The rules are the correct ones , updated ok , seems it might be related to cleaning up the environment.

@catenacyber
Copy link
Contributor Author

Replaced by #7357

@catenacyber catenacyber closed this May 2, 2022
glongo added a commit to glongo/suricata that referenced this pull request Oct 4, 2024
The encryption key subfield of the media description field is not
logged when it should be.

Ticket OISF#7305
glongo added a commit to glongo/suricata that referenced this pull request Oct 15, 2024
The encryption key subfield of the media description field is not
logged when it should be.

Ticket OISF#7305
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants