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

ftp: Begin conversion process to rust. #12376

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jlucovsky
Copy link
Contributor

@jlucovsky jlucovsky commented Jan 11, 2025

ftp: Partial conversion of the FTP protocol parser to Rust.

This PR represents partial completion of issue 4082.

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

Describe changes:

  • Moved several constructs to Rust: command table, config file handling, data structures.

Provide values to any of the below to override the defaults.

  • To use an LibHTP, Suricata-Verify or Suricata-Update pull request,
    link to the pull request in the respective _BRANCH variable.
  • Leave unused overrides blank or remove.

SV_REPO=
SV_BRANCH=
SU_REPO=
SU_BRANCH=
LIBHTP_REPO=
LIBHTP_BRANCH=

Issue: 4082

As part of the effort to convert the FTP/FTPDATA parser to Rust, move
the enums from C to Rust.
This commit moves the allocation logic from C to Rust for the FTP
parser.

Issue: 4082
Issue: 4082

Move the FTP enumeration values to a new module -- constant.rs -- for
refactoring.
Issue: 4082

Move the FtpCommand structure to Rust.
Issue: 4082

Move config value handling to Rust. Use get_memval for the memcap
and line-len values (since line-len is often expressed with a kb suffix)
Copy link

codecov bot commented Jan 11, 2025

Codecov Report

Attention: Patch coverage is 91.05691% with 11 lines in your changes missing coverage. Please review.

Project coverage is 82.48%. Comparing base (ad7ff1c) to head (00759a9).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12376      +/-   ##
==========================================
- Coverage   82.49%   82.48%   -0.01%     
==========================================
  Files         912      913       +1     
  Lines      258220   258273      +53     
==========================================
+ Hits       213006   213039      +33     
- Misses      45214    45234      +20     
Flag Coverage Δ
fuzzcorpus 60.42% <85.00%> (+0.01%) ⬆️
livemode 19.41% <26.66%> (+<0.01%) ⬆️
pcap 44.37% <83.33%> (+0.02%) ⬆️
suricata-verify 63.24% <88.61%> (-0.01%) ⬇️
unittests 58.08% <40.83%> (+<0.01%) ⬆️

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

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24168

rust/src/ftp/ftp.rs Outdated Show resolved Hide resolved
src/app-layer-ftp.c Outdated Show resolved Hide resolved
@victorjulien
Copy link
Member

Looks pretty clean, and a nice start.

Issue: 4082

Convert the FTP command table to rust. The command table is loaded into
MPM to efficiently return the observed command. Formerly in C, the
conversion to Rust incorporates the FTP command info into the FTP TX
instead of incorporating it as a reference.
Issue: 4082

Rename the existing rs_* parsing functions (pasv, port, epsv, eprt) to
follow the new API nomenclature.
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24171

@jlucovsky jlucovsky marked this pull request as ready for review January 13, 2025 13:08
@jlucovsky jlucovsky requested a review from jasonish as a code owner January 13, 2025 13:08
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