Skip to content

Comments

Smtp server detection 1125 v2.0#11125

Closed
catenacyber wants to merge 2 commits intoOISF:masterfrom
catenacyber:smtp-server-detection-1125-v2.0
Closed

Smtp server detection 1125 v2.0#11125
catenacyber wants to merge 2 commits intoOISF:masterfrom
catenacyber:smtp-server-detection-1125-v2.0

Conversation

@catenacyber
Copy link
Contributor

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

Describe changes:

  • smtp server detection (ie to_client)
  • ftp server detection (ie to_client)

SV_BRANCH=OISF/suricata-verify#1850

Follow up on #8892 after merge of #10982

@catenacyber
Copy link
Contributor Author

Coming from #3806 !!!

}

#[no_mangle]
pub unsafe extern "C" fn rs_validate_domain(input: *const u8, in_len: u32) -> u32 {
Copy link
Member

Choose a reason for hiding this comment

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

I think this supposed to use a different naming style now, right @jasonish?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, changing

// or if client side is unknown despite having received bytes
r = ALPROTO_FTP;
}
for (uint32_t i = 4; i < len; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

maybe better to use memchr for this? Can have optimized implementations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok doing

Copy link
Member

Choose a reason for hiding this comment

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

probably out of scope for this PR, but there is a rust memchr crate that claims some pretty good performance too IIRC

@codecov
Copy link

codecov bot commented May 23, 2024

Codecov Report

Attention: Patch coverage is 82.72727% with 19 lines in your changes are missing coverage. Please review.

Project coverage is 84.15%. Comparing base (0aaec69) to head (f84948e).
Report is 10 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11125      +/-   ##
==========================================
+ Coverage   84.10%   84.15%   +0.05%     
==========================================
  Files         926      926              
  Lines      250712   247478    -3234     
==========================================
- Hits       210858   208271    -2587     
+ Misses      39854    39207     -647     
Flag Coverage Δ
fuzzcorpus 64.21% <82.71%> (+0.01%) ⬆️
livemode 19.57% <14.81%> (-0.01%) ⬇️
pcap 46.52% <80.24%> (+0.02%) ⬆️
suricata-verify 62.88% <80.24%> (+0.05%) ⬆️
unittests 61.96% <48.18%> (-0.25%) ⬇️

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

@ct0br0
Copy link

ct0br0 commented May 23, 2024

qa tokens ran out. ignore the 'pipeline canceled' incoming

@catenacyber
Copy link
Contributor Author

Continued in #11128

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.

3 participants