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

Smb dcerpc logging #7584

Closed
wants to merge 2 commits into from
Closed

Smb dcerpc logging #7584

wants to merge 2 commits into from

Conversation

regit
Copy link
Contributor

@regit regit commented Jun 29, 2022

Make sure these boxes are signed before submitting your Pull Request -- thank you.

Link to redmine ticket: https://redmine.openinfosecfoundation.org/issues/5413

Describe changes:

  • log dcerpc interface
  • extract context ID from dcerpc bind request
  • update JSON schema

As context id is used to know to which variant of the endpoint the
request is done, it is interesting to parse it.

Feature OISF#5413.
When doing a DCERPC request, we can use the context id to log the
interface that is used. Doing that we can see in one single event
what is the DCERPC interface and opnum that are used. This allows
to have all the information needed to resolve the request to a
function call.

Feature OISF#5413.
@regit regit requested review from victorjulien, jasonish and a team as code owners June 29, 2022 09:25
if i.context_id == x.context_id {
jsb.open_object("interface")?;
let ifstr = uuid::Uuid::from_slice(&i.uuid);
let ifstr = ifstr.map(|ifstr| ifstr.to_hyphenated().to_string()).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

is this unwrap safe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to check but I did reuse the code that is used to log DCERPC list of interfaces (in bind operation).

Copy link
Member

Choose a reason for hiding this comment

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

Ok I see this is a common pattern indeed.

@codecov
Copy link

codecov bot commented Jun 29, 2022

Codecov Report

Merging #7584 (89ae60d) into master (a898409) will decrease coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #7584      +/-   ##
==========================================
- Coverage   75.80%   75.74%   -0.07%     
==========================================
  Files         658      658              
  Lines      186526   186523       -3     
==========================================
- Hits       141399   141283     -116     
- Misses      45127    45240     +113     
Flag Coverage Δ
fuzzcorpus 59.84% <ø> (-0.12%) ⬇️
suricata-verify 52.36% <ø> (-0.06%) ⬇️
unittests 60.72% <ø> (-0.01%) ⬇️

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

@suricata-qa
Copy link

ERROR:

ERROR: QA failed on ips_afp_drop_chk.

Pipeline 8024

This was referenced Aug 25, 2022
@victorjulien
Copy link
Member

Merged in #7774, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants