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

Libhtp rs v28 #12648

Closed
wants to merge 3 commits into from
Closed

Libhtp rs v28 #12648

wants to merge 3 commits into from

Conversation

catenacyber
Copy link
Contributor

@catenacyber catenacyber commented Feb 21, 2025

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

Describe changes:

  • Use libhtp-rs.

Needed rebase from #12508 with needed rebase

Draft to see CI/QA feedback with latest libhtp-rs changes, and not have too many conflicts

Draft/TODOs:

  • fix SV tests for SURI_TLPW1_files_sha256 in my branch libhtp-rs-tlpw1-v1
  • remove libhtp.rs unused stuff (see latest commit in this PR)
  • review TODO comments in libhtp.rs

@victorjulien should I remove the unused functions in the C API proposed by libhtp.rs ?
git grep "pub unsafe extern" rust/htp/src/c_api/ | awk '{print $6}' | cut -d( -f1 | while read i; do echo -n $i; git grep $i | wc -l; done | awk '$2 == 1' shows quite a few functions

@catenacyber catenacyber mentioned this pull request Feb 21, 2025
@catenacyber catenacyber marked this pull request as draft February 21, 2025 10:58
@catenacyber
Copy link
Contributor Author

Red CI because of #12647

@suricata-qa
Copy link

Information:

ERROR: QA failed on SURI_TLPW2_autofp_suri_time.

ERROR: QA failed on SURI_TLPW1_files_sha256.

field baseline test %
SURI_TLPW2_single_stats_chk
.uptime 750 783 104.4%
SURI_TLPW2_autofp_stats_chk
.uptime 143 148 103.5%
SURI_TLPW1_stats_chk
.app_layer.error.http.parser 47 61 129.79%
SURI_TLPR1_stats_chk
.detect.alerts_suppressed 1489442 1555140 104.41%
.app_layer.error.http.parser 574 733 127.7%

Pipeline 24859

@catenacyber
Copy link
Contributor Author

I think git diff master -- src/ is now reviewable

So left TODOs are:

  • fix SV tests for SURI_TLPW1_files_sha256 in my branch libhtp-rs-tlpw1-v1 (I will do that)
  • remove libhtp.rs unused stuff (see latest commit in this PR) @victorjulien what about the unused functions in the C API proposed by libhtp.rs ? Should we remove multipart.rs ?..
  • review TODO comments in libhtp.rs
    • Thoughts about the chunked_len being capped to i32::MAX (as done by libhtp.c)

cccs-rtmorti and others added 3 commits February 23, 2025 21:17
Ticket: OISF#2696

There are a lot of changes here, which are described below.

In general these changes are renaming constants to conform to the
libhtp-rs versions (which are generated by cbindgen); making all htp
types opaque and changing struct->member references to
htp_struct_member() function calls; and a handful of changes to offload
functionality onto libhtp-rs from suricata, such as URI normalization
and transaction cleanup.

Functions introduced to handle opaque htp_tx_t:
- tx->parsed_uri => htp_tx_parsed_uri(tx)
- tx->parsed_uri->path => htp_uri_path(htp_tx_parsed_uri(tx)
- tx->parsed_uri->hostname => htp_uri_hostname(htp_tx_parsed_uri(tx))
- htp_tx_get_user_data() => htp_tx_user_data(tx)
- htp_tx_is_http_2_upgrade(tx) convenience function introduced to detect response status 101
  and “Upgrade: h2c" header.

Functions introduced to handle opaque htp_tx_data_t:
- d->len => htp_tx_data_len()
- d->data => htp_tx_data_data()
- htp_tx_data_tx(data) function to get the htp_tx_t from the htp_tx_data_t
- htp_tx_data_is_empty(data) convenience function introduced to test if the data is empty.

Other changes:

Build libhtp-rs as a crate inside rust. Update autoconf to no longer
use libhtp as an external dependency. Remove HAVE_HTP feature defines
since they are no longer needed.

Make function arguments and return values const where possible

htp_tx_destroy(tx) will now free an incomplete transaction

htp_time_t replaced with standard struct timeval

Callbacks from libhtp now provide the htp_connp_t and the htp_tx_data_t
as separate arguments. This means the connection parser is no longer
fetched from the transaction inside callbacks.

SCHTPGenerateNormalizedUri() functionality moved inside libhtp-rs, which
now provides normalized URI values.
The normalized URI is available with accessor function: htp_tx_normalized_uri()
Configuration settings added to control the behaviour of the URI normalization:
- htp_config_set_normalized_uri_include_all()
- htp_config_set_plusspace_decode()
- htp_config_set_convert_lowercase()
- htp_config_set_double_decode_normalized_query()
- htp_config_set_double_decode_normalized_path()
- htp_config_set_backslash_convert_slashes()
- htp_config_set_bestfit_replacement_byte()
- htp_config_set_convert_lowercase()
- htp_config_set_nul_encoded_terminates()
- htp_config_set_nul_raw_terminates()
- htp_config_set_path_separators_compress()
- htp_config_set_path_separators_decode()
- htp_config_set_u_encoding_decode()
- htp_config_set_url_encoding_invalid_handling()
- htp_config_set_utf8_convert_bestfit()
- htp_config_set_normalized_uri_include_all()
- htp_config_set_plusspace_decode()
Constants related to configuring uri normalization:
- HTP_URL_DECODE_PRESERVE_PERCENT => HTP_URL_ENCODING_HANDLING_PRESERVE_PERCENT
- HTP_URL_DECODE_REMOVE_PERCENT => HTP_URL_ENCODING_HANDLING_REMOVE_PERCENT
- HTP_URL_DECODE_PROCESS_INVALID => HTP_URL_ENCODING_HANDLING_PROCESS_INVALID

htp_config_set_field_limits(soft_limit, hard_limit) changed to
htp_config_set_field_limit(limit) because libhtp didn't implement soft
limits.

libhtp logging API updated to provide HTP_LOG_CODE constants along with
the message. This eliminates the need to perform string matching on
message text to map log messages to HTTP_DECODER_EVENT values, and the
HTP_LOG_CODE values can be used directly. In support of this,
HTP_DECODER_EVENT values are mapped to their corresponding HTP_LOG_CODE
values.

New log events to describe additional anomalies:
HTP_LOG_CODE_REQUEST_TOO_MANY_LZMA_LAYERS
HTP_LOG_CODE_RESPONSE_TOO_MANY_LZMA_LAYERS
HTP_LOG_CODE_PROTOCOL_CONTAINS_EXTRA_DATA
HTP_LOG_CODE_CONTENT_LENGTH_EXTRA_DATA_START
HTP_LOG_CODE_CONTENT_LENGTH_EXTRA_DATA_END
HTP_LOG_CODE_SWITCHING_PROTO_WITH_CONTENT_LENGTH
HTP_LOG_CODE_DEFORMED_EOL
HTP_LOG_CODE_PARSER_STATE_ERROR
HTP_LOG_CODE_MISSING_OUTBOUND_TRANSACTION_DATA
HTP_LOG_CODE_MISSING_INBOUND_TRANSACTION_DATA
HTP_LOG_CODE_ZERO_LENGTH_DATA_CHUNKS
HTP_LOG_CODE_REQUEST_LINE_UNKNOWN_METHOD
HTP_LOG_CODE_REQUEST_LINE_UNKNOWN_METHOD_NO_PROTOCOL
HTP_LOG_CODE_REQUEST_LINE_UNKNOWN_METHOD_INVALID_PROTOCOL
HTP_LOG_CODE_REQUEST_LINE_NO_PROTOCOL
HTP_LOG_CODE_RESPONSE_LINE_INVALID_PROTOCOL
HTP_LOG_CODE_RESPONSE_LINE_INVALID_RESPONSE_STATUS
HTP_LOG_CODE_RESPONSE_BODY_INTERNAL_ERROR
HTP_LOG_CODE_REQUEST_BODY_DATA_CALLBACK_ERROR
HTP_LOG_CODE_RESPONSE_INVALID_EMPTY_NAME
HTP_LOG_CODE_REQUEST_INVALID_EMPTY_NAME
HTP_LOG_CODE_RESPONSE_INVALID_LWS_AFTER_NAME
HTP_LOG_CODE_RESPONSE_HEADER_NAME_NOT_TOKEN
HTP_LOG_CODE_REQUEST_INVALID_LWS_AFTER_NAME
HTP_LOG_CODE_LZMA_DECOMPRESSION_DISABLED
HTP_LOG_CODE_CONNECTION_ALREADY_OPEN
HTP_LOG_CODE_COMPRESSION_BOMB_DOUBLE_LZMA
HTP_LOG_CODE_INVALID_CONTENT_ENCODING
HTP_LOG_CODE_INVALID_GAP
HTP_LOG_CODE_ERROR

The new htp_log API supports consuming log messages more easily than
walking a list and tracking the current offset. Internally, libhtp-rs
now provides log messages as a queue of htp_log_t, which means the
application can simply call htp_conn_next_log() to fetch the next log
message until the queue is empty. Once the application is done with a
log message, they can call htp_log_free() to dispose of it.

Functions supporting htp_log_t:
htp_conn_next_log(conn) - Get the next log message
htp_log_message(log) - To get the text of the message
htp_log_code(log) - To get the HTP_LOG_CODE value
htp_log_free(log) - To free the htp_log_t
@suricata-qa
Copy link

Information:

ERROR: QA failed on SURI_TLPW2_autofp_suri_time.

ERROR: QA failed on SURI_TLPW1_files_sha256.

field baseline test %
SURI_TLPW2_single_stats_chk
.uptime 750 780 104.0%
SURI_TLPW2_autofp_stats_chk
.uptime 143 148 103.5%
SURI_TLPW1_stats_chk
.app_layer.error.http.parser 47 61 129.79%
SURI_TLPR1_stats_chk
.detect.alerts_suppressed 1489442 1555139 104.41%
.app_layer.error.http.parser 574 733 127.7%

Pipeline 24873

@victorjulien
Copy link
Member

As we're getting closer to feature completeness & correctness, the consistent performance regression we see becomes a concern. Has anyone looked into this yet?

@catenacyber catenacyber mentioned this pull request Feb 27, 2025
@catenacyber
Copy link
Contributor Author

Next in #12684

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.

4 participants