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.

Sign up for GitHub

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

Jump to bottom

Applayer plugin 5053 v3.9 #12066

Closed
wants to merge 14 commits into from
Closed

Applayer plugin 5053 v3.9 #12066

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
5ddb2ba
protodetect: make expectation_proto part of AppLayerProtoDetectCtx
catenacyber Sep 2, 2024
c6bd4ef
protodetect: use dynamic number of app-layer protos
catenacyber Sep 2, 2024
1a39105
protodetect: use dynamic number of app-layer protos
catenacyber Sep 2, 2024
3338c82
frames: use dynamic number of app-layer protos
catenacyber Sep 2, 2024
4b08f81
util: parenthesis for macro
catenacyber Sep 2, 2024
aac2715
app-layer/stats: use dynamic number of app-layer protos
catenacyber Sep 2, 2024
2545897
fuzz: use dynamic number of app-layer protos
catenacyber Sep 2, 2024
42db243
app-layer/parser: use dynamic number of app-layer protos
catenacyber Sep 2, 2024
eb2c7f8
profiling: use dynamic number of app-layer protos
catenacyber Oct 29, 2024
45679d1
fuzz: better init for protocol detection
catenacyber Oct 29, 2024
df05782
fuzz: simplify target for protocol detection
catenacyber Oct 29, 2024
ff98a23
sip: remove UPDATE method for detection
catenacyber Oct 29, 2024
867f486
app-layer: make number of alprotos dynamic
catenacyber Oct 17, 2024
b34acb1
plugins: app-layer plugins
catenacyber Nov 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion rust/src/core.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,13 @@ pub fn init_ffi(context: &'static SuricataContext)
{
catenacyber marked this conversation as resolved.
Show resolved Hide resolved
unsafe {
SC = Some(context);
ALPROTO_FAILED = StringToAppProto("failed\0".as_ptr());
}
}

#[no_mangle]
pub extern "C" fn SCUpdateAlprotoFailed(alproto: AppProto) {
unsafe {
ALPROTO_FAILED = alproto;
}
}

Expand Down
2 changes: 1 addition & 1 deletion scripts/setup-app-layer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def patch_app_layer_protos_h(protoname):
open(filename, "w").write(output.getvalue())

def patch_app_layer_protos_c(protoname):
filename = "src/app-layer-protos.c"
filename = "src/app-layer.c"
print("Patching %s." % (filename))
output = io.StringIO()

Expand Down
89 changes: 43 additions & 46 deletions src/app-layer-protos.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,56 +24,19 @@

#include "suricata-common.h"
#include "app-layer-protos.h"
#include "rust.h"

AppProto ALPROTO_FAILED = ALPROTO_MAX_STATIC;
Copy link
Member

Choose a reason for hiding this comment

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

I don't like the coding style violations here, macro style name for vars, function style name for a var, etc.

Copy link
Member

Choose a reason for hiding this comment

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

can also imagine that the now global var ALPROTO_MAX would be replaced by a function AppProtoMax() or something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why a function instead of a variable ?

AppProto ALPROTO_MAX = ALPROTO_MAX_STATIC + 1;
#define ARRAY_CAP_STEP 16
AppProto AppProtoStringsCap = ALPROTO_MAX_STATIC + 1;

typedef struct AppProtoStringTuple {
AppProto alproto;
const char *str;
} AppProtoStringTuple;

const AppProtoStringTuple AppProtoStrings[ALPROTO_MAX] = {
{ ALPROTO_UNKNOWN, "unknown" },
{ ALPROTO_HTTP1, "http1" },
{ ALPROTO_FTP, "ftp" },
{ ALPROTO_SMTP, "smtp" },
{ ALPROTO_TLS, "tls" },
{ ALPROTO_SSH, "ssh" },
{ ALPROTO_IMAP, "imap" },
{ ALPROTO_JABBER, "jabber" },
{ ALPROTO_SMB, "smb" },
{ ALPROTO_DCERPC, "dcerpc" },
{ ALPROTO_IRC, "irc" },
{ ALPROTO_DNS, "dns" },
{ ALPROTO_MODBUS, "modbus" },
{ ALPROTO_ENIP, "enip" },
{ ALPROTO_DNP3, "dnp3" },
{ ALPROTO_NFS, "nfs" },
{ ALPROTO_NTP, "ntp" },
{ ALPROTO_FTPDATA, "ftp-data" },
{ ALPROTO_TFTP, "tftp" },
{ ALPROTO_IKE, "ike" },
{ ALPROTO_KRB5, "krb5" },
{ ALPROTO_QUIC, "quic" },
{ ALPROTO_DHCP, "dhcp" },
{ ALPROTO_SNMP, "snmp" },
{ ALPROTO_SIP, "sip" },
{ ALPROTO_RFB, "rfb" },
{ ALPROTO_MQTT, "mqtt" },
{ ALPROTO_PGSQL, "pgsql" },
{ ALPROTO_TELNET, "telnet" },
{ ALPROTO_WEBSOCKET, "websocket" },
{ ALPROTO_LDAP, "ldap" },
{ ALPROTO_DOH2, "doh2" },
{ ALPROTO_TEMPLATE, "template" },
{ ALPROTO_RDP, "rdp" },
{ ALPROTO_HTTP2, "http2" },
{ ALPROTO_BITTORRENT_DHT, "bittorrent-dht" },
{ ALPROTO_POP3, "pop3" },
{ ALPROTO_HTTP, "http" },
{ ALPROTO_FAILED, "failed" },
#ifdef UNITTESTS
{ ALPROTO_TEST, "test" },
#endif
};
AppProtoStringTuple *AppProtoStrings = NULL;

const char *AppProtoToString(AppProto alproto)
{
Expand All @@ -87,7 +50,7 @@ const char *AppProtoToString(AppProto alproto)
proto_name = "http_any";
break;
default:
if (alproto < ARRAY_SIZE(AppProtoStrings)) {
if (alproto < ALPROTO_MAX) {
BUG_ON(AppProtoStrings[alproto].alproto != alproto);
proto_name = AppProtoStrings[alproto].str;
}
Expand All @@ -101,10 +64,44 @@ AppProto StringToAppProto(const char *proto_name)
return ALPROTO_UNKNOWN;

// We could use a Multi Pattern Matcher
for (size_t i = 0; i < ARRAY_SIZE(AppProtoStrings); i++) {
for (size_t i = 0; i < ALPROTO_MAX; i++) {
if (strcmp(proto_name, AppProtoStrings[i].str) == 0)
return AppProtoStrings[i].alproto;
}

return ALPROTO_UNKNOWN;
}

void RegisterAppProtoString(AppProto alproto, const char *proto_name)
Copy link
Member

Choose a reason for hiding this comment

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

can ALPROTO_FAILED just be a constant instead of having it be a dynamic value for no real reason I can see?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ALPROTO_FAILED is used in comparison like < ALPROTO_FAILED ...
Will think about it again

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like we can move ALPROTO_FAILED to 1 and change the comparisons... in next PR version

catenacyber marked this conversation as resolved.
Show resolved Hide resolved
{
if (alproto < ALPROTO_FAILED) {
if (AppProtoStrings == NULL) {
AppProtoStrings = SCCalloc(AppProtoStringsCap, sizeof(AppProtoStringTuple));
if (AppProtoStrings == NULL) {
FatalError("Unable to allocate AppProtoStrings");
}
AppProtoStrings[ALPROTO_FAILED].str = "failed";
AppProtoStrings[ALPROTO_FAILED].alproto = ALPROTO_FAILED;
SCUpdateAlprotoFailed(ALPROTO_FAILED);
}
AppProtoStrings[alproto].str = proto_name;
AppProtoStrings[alproto].alproto = alproto;
} else if (alproto == ALPROTO_FAILED && alproto + 1 == ALPROTO_MAX) {
if (ALPROTO_MAX == AppProtoStringsCap) {
void *tmp = SCRealloc(AppProtoStrings,
sizeof(AppProtoStringTuple) * (AppProtoStringsCap + ARRAY_CAP_STEP));
if (tmp == NULL) {
FatalError("Unable to reallocate AppProtoStrings");
}
AppProtoStringsCap += ARRAY_CAP_STEP;
AppProtoStrings = tmp;
}
AppProtoStrings[alproto].str = proto_name;
AppProtoStrings[alproto].alproto = alproto;
ALPROTO_FAILED++;
AppProtoStrings[ALPROTO_FAILED].str = "failed";
AppProtoStrings[ALPROTO_FAILED].alproto = ALPROTO_FAILED;
SCUpdateAlprotoFailed(ALPROTO_FAILED);
ALPROTO_MAX++;
}
}
9 changes: 5 additions & 4 deletions src/app-layer-protos.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,18 @@ enum AppProtoEnum {
// HTTP for any version (ALPROTO_HTTP1 (version 1) or ALPROTO_HTTP2)
ALPROTO_HTTP,

/* used by the probing parser when alproto detection fails
* permanently for that particular stream */
ALPROTO_FAILED,
catenacyber marked this conversation as resolved.
Show resolved Hide resolved
#ifdef UNITTESTS
ALPROTO_TEST,
#endif /* UNITESTS */
/* keep last */
ALPROTO_MAX,
ALPROTO_MAX_STATIC,
catenacyber marked this conversation as resolved.
Show resolved Hide resolved
};
// NOTE: if ALPROTO's get >= 256, update SignatureNonPrefilterStore

/* not using the enum as that is a unsigned int, so 4 bytes */
typedef uint16_t AppProto;
extern AppProto ALPROTO_FAILED;
extern AppProto ALPROTO_MAX;

static inline bool AppProtoIsValid(AppProto a)
{
Expand Down Expand Up @@ -173,4 +172,6 @@ const char *AppProtoToString(AppProto alproto);
*/
AppProto StringToAppProto(const char *proto_name);

void RegisterAppProtoString(AppProto alproto, const char *proto_name);

#endif /* SURICATA_APP_LAYER_PROTOS_H */
108 changes: 75 additions & 33 deletions src/app-layer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -905,42 +905,39 @@ int AppLayerHandleUdp(ThreadVars *tv, AppLayerThreadCtx *tctx, Packet *p, Flow *
tctx->alpd_tctx, f, p->payload, p->payload_len, IPPROTO_UDP, flags, &reverse_flow);
PACKET_PROFILING_APP_PD_END(tctx);

switch (*alproto) {
case ALPROTO_UNKNOWN:
if (*alproto_otherdir != ALPROTO_UNKNOWN) {
// Use recognized side
f->alproto = *alproto_otherdir;
// do not keep ALPROTO_UNKNOWN for this side so as not to loop
*alproto = *alproto_otherdir;
if (*alproto_otherdir == ALPROTO_FAILED) {
SCLogDebug("ALPROTO_UNKNOWN flow %p", f);
}
} else {
// First side of protocol is unknown
*alproto = ALPROTO_FAILED;
if (*alproto == ALPROTO_UNKNOWN) {
if (*alproto_otherdir != ALPROTO_UNKNOWN) {
// Use recognized side
f->alproto = *alproto_otherdir;
// do not keep ALPROTO_UNKNOWN for this side so as not to loop
*alproto = *alproto_otherdir;
if (*alproto_otherdir == ALPROTO_FAILED) {
SCLogDebug("ALPROTO_UNKNOWN flow %p", f);
}
break;
case ALPROTO_FAILED:
if (*alproto_otherdir != ALPROTO_UNKNOWN) {
// Use recognized side
f->alproto = *alproto_otherdir;
if (*alproto_otherdir == ALPROTO_FAILED) {
SCLogDebug("ALPROTO_UNKNOWN flow %p", f);
}
} else {
// First side of protocol is unknown
*alproto = ALPROTO_FAILED;
}
} else if (*alproto == ALPROTO_FAILED) {
if (*alproto_otherdir != ALPROTO_UNKNOWN) {
// Use recognized side
f->alproto = *alproto_otherdir;
if (*alproto_otherdir == ALPROTO_FAILED) {
SCLogDebug("ALPROTO_UNKNOWN flow %p", f);
}
// else wait for second side of protocol
break;
default:
if (*alproto_otherdir != ALPROTO_UNKNOWN && *alproto_otherdir != ALPROTO_FAILED) {
if (*alproto_otherdir != *alproto) {
AppLayerDecoderEventsSetEventRaw(
&p->app_layer_events, APPLAYER_MISMATCH_PROTOCOL_BOTH_DIRECTIONS);
// data already sent to parser, we cannot change the protocol to use the one
// of the server
}
} else {
f->alproto = *alproto;
}
// else wait for second side of protocol
} else {
if (*alproto_otherdir != ALPROTO_UNKNOWN && *alproto_otherdir != ALPROTO_FAILED) {
if (*alproto_otherdir != *alproto) {
AppLayerDecoderEventsSetEventRaw(
&p->app_layer_events, APPLAYER_MISMATCH_PROTOCOL_BOTH_DIRECTIONS);
// data already sent to parser, we cannot change the protocol to use the one
// of the server
}
} else {
f->alproto = *alproto;
}
}
if (*alproto_otherdir == ALPROTO_UNKNOWN) {
if (f->alproto == ALPROTO_UNKNOWN) {
Expand Down Expand Up @@ -1027,11 +1024,56 @@ void AppLayerListSupportedProtocols(void)
}

/***** Setup/General Registration *****/
static void AppLayerNamesSetup(void)
{
RegisterAppProtoString(ALPROTO_UNKNOWN, "unknown");
Copy link
Member

Choose a reason for hiding this comment

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

should this move into the actual parser 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.

Maybe as a next step that makes a protocol like SNMP behave like a plugin ? (no static definition of ALPROTO_SNMP)

RegisterAppProtoString(ALPROTO_HTTP1, "http1");
RegisterAppProtoString(ALPROTO_FTP, "ftp");
RegisterAppProtoString(ALPROTO_SMTP, "smtp");
RegisterAppProtoString(ALPROTO_TLS, "tls");
RegisterAppProtoString(ALPROTO_SSH, "ssh");
RegisterAppProtoString(ALPROTO_IMAP, "imap");
RegisterAppProtoString(ALPROTO_JABBER, "jabber");
RegisterAppProtoString(ALPROTO_SMB, "smb");
RegisterAppProtoString(ALPROTO_DCERPC, "dcerpc");
RegisterAppProtoString(ALPROTO_IRC, "irc");
RegisterAppProtoString(ALPROTO_DNS, "dns");
RegisterAppProtoString(ALPROTO_MODBUS, "modbus");
RegisterAppProtoString(ALPROTO_ENIP, "enip");
RegisterAppProtoString(ALPROTO_DNP3, "dnp3");
RegisterAppProtoString(ALPROTO_NFS, "nfs");
RegisterAppProtoString(ALPROTO_NTP, "ntp");
RegisterAppProtoString(ALPROTO_FTPDATA, "ftp-data");
RegisterAppProtoString(ALPROTO_TFTP, "tftp");
RegisterAppProtoString(ALPROTO_IKE, "ike");
RegisterAppProtoString(ALPROTO_KRB5, "krb5");
RegisterAppProtoString(ALPROTO_QUIC, "quic");
RegisterAppProtoString(ALPROTO_DHCP, "dhcp");
RegisterAppProtoString(ALPROTO_SNMP, "snmp");
RegisterAppProtoString(ALPROTO_SIP, "sip");
RegisterAppProtoString(ALPROTO_RFB, "rfb");
RegisterAppProtoString(ALPROTO_MQTT, "mqtt");
RegisterAppProtoString(ALPROTO_PGSQL, "pgsql");
RegisterAppProtoString(ALPROTO_TELNET, "telnet");
RegisterAppProtoString(ALPROTO_WEBSOCKET, "websocket");
RegisterAppProtoString(ALPROTO_LDAP, "ldap");
RegisterAppProtoString(ALPROTO_DOH2, "doh2");
RegisterAppProtoString(ALPROTO_TEMPLATE, "template");
RegisterAppProtoString(ALPROTO_RDP, "rdp");
RegisterAppProtoString(ALPROTO_HTTP2, "http2");
RegisterAppProtoString(ALPROTO_BITTORRENT_DHT, "bittorrent-dht");
RegisterAppProtoString(ALPROTO_POP3, "pop3");
RegisterAppProtoString(ALPROTO_HTTP, "http");
#ifdef UNITTESTS
RegisterAppProtoString(ALPROTO_TEST, "test");
#endif
}

int AppLayerSetup(void)
{
SCEnter();

AppLayerNamesSetup();
AppLayerProtoDetectSetup();
AppLayerParserSetup();

Expand Down
3 changes: 2 additions & 1 deletion src/detect-engine-build.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,8 @@ static json_t *RulesGroupPrintSghStats(const DetectEngineCtx *de_ctx, const SigG
} mpm_stats[max_buffer_type_id];
memset(mpm_stats, 0x00, sizeof(mpm_stats));

uint32_t alstats[ALPROTO_MAX] = {0};
uint32_t alstats[ALPROTO_MAX];
memset(alstats, 0, ALPROTO_MAX * sizeof(uint32_t));
uint32_t mpm_sizes[max_buffer_type_id][256];
memset(mpm_sizes, 0, sizeof(mpm_sizes));
uint32_t alproto_mpm_bufs[ALPROTO_MAX][max_buffer_type_id];
Expand Down
3 changes: 2 additions & 1 deletion src/runmodes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,8 @@ void RunModeInitializeOutputs(void)
char tls_store_present = 0;

// ALPROTO_MAX is set to its final value
LoggerId logger_bits[ALPROTO_MAX] = { 0 };
LoggerId logger_bits[ALPROTO_MAX];
memset(logger_bits, 0, ALPROTO_MAX * sizeof(LoggerId));
TAILQ_FOREACH(output, &outputs->head, next) {

output_config = ConfNodeLookupChild(output, output->val);
Expand Down