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

Support more upper layer services (many, many, many more) #446

Closed
wants to merge 4 commits into from

Conversation

GyulyVGC
Copy link
Owner

@GyulyVGC GyulyVGC commented Feb 1, 2024

Warning

This PR was replaced by #450


As requested in #374 and by multiple other users of the application (e.g., #289, #338), this PR adds support for several new upper layer services in addition to the 24 already supported application protocols.

The full list of the new supported services, taken from Nmap, is basically the IANA assigned ports list plus some trojans and worms.
It contains 6438 different services/protocols/trojans/worms.

While I could use a file-lookup approach to resolve the name of a service given a port number and the transport protocol, I decided to create a short bash script that automatically curls the full list of services and generates a Rust file containing a single (very long) match expression wrapped in a new method.
This strategy may seem a little unconventional but should guarantee un-matchable performances (pun intended) in the port-to-service mapping procedure.

Since apparently there isn't a way to autofix clippy::match_same_arms, the bash script also takes care of merging duplicate branches of the match.

Fixes #374

@GyulyVGC GyulyVGC added the enhancement New feature, request, or improvement label Feb 1, 2024
@GyulyVGC GyulyVGC added this to the v1.3.0 milestone Feb 1, 2024
@GyulyVGC GyulyVGC marked this pull request as draft February 1, 2024 22:25
@GyulyVGC GyulyVGC self-assigned this Feb 1, 2024
printf '//! This file is generated automatically via ./services.sh\n\n'
printf 'use crate::networking::types::protocol::Protocol;\n\n'
printf '#[allow(clippy::too_many_lines, clippy::unnested_or_patterns)]\n'
printf 'pub fn get_service(port_info: (u16, Protocol)) -> String {\n'
Copy link

Choose a reason for hiding this comment

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

Suggested change
printf 'pub fn get_service(port_info: (u16, Protocol)) -> String {\n'
printf 'pub fn get_service(port_info: (u16, Protocol)) -> Option<&'static str> {\n'

so the caller can decide how they want to represent unknown services and whether they need owned String. If you care about performance you should avoid String allocations.

Copy link

@bradengroom bradengroom left a comment

Choose a reason for hiding this comment

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

lgtm

@yawaramin
Copy link

If you do this, you'll have to cut a new release every time you want to change even a single character of any of the service definitions. It will quickly become unsustainable.

@GyulyVGC
Copy link
Owner Author

GyulyVGC commented Feb 2, 2024

If you do this, you'll have to cut a new release every time you want to change even a single character of any of the service definitions. It will quickly become unsustainable.

I see your point.

Anyway, I don't judge it a priority to update this service list.
The most common services assignments are stable, well-known, and long lasting conventions maintained directly by IANA.

@GyulyVGC
Copy link
Owner Author

GyulyVGC commented Feb 6, 2024

After having long discussed this PR with the Rust community of Reddit, I came to the conclusion that a better strategy is to generate a compile-time, static, perfect hash map for the network services.
The new approach still guarantees high performances and avoids the complexities of this PR.

New PR: #450.

@GyulyVGC GyulyVGC closed this Feb 6, 2024
@GyulyVGC GyulyVGC deleted the services branch February 6, 2024 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, request, or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Request] More Port/Application Info
4 participants