From e1412e7b274686732167dce7faa2cf43c24801d6 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Tue, 28 Jun 2022 21:34:24 +0200 Subject: [PATCH 1/3] smtp: adds server side detection Ticket: #1125 --- rust/src/util.rs | 96 ++++++++++++++++++++++++++++++++++++++++++++ src/app-layer-smtp.c | 57 ++++++++++++++++++++++++++ 2 files changed, 153 insertions(+) diff --git a/rust/src/util.rs b/rust/src/util.rs index a0933689164e..d1d8de0dd01e 100644 --- a/rust/src/util.rs +++ b/rust/src/util.rs @@ -18,7 +18,103 @@ use std::ffi::CStr; use std::os::raw::c_char; +use nom7::bytes::complete::take_while1; +use nom7::character::complete::char; +use nom7::character::{is_alphabetic, is_alphanumeric}; +use nom7::combinator::verify; +use nom7::multi::many1_count; +use nom7::IResult; + #[no_mangle] pub unsafe extern "C" fn rs_check_utf8(val: *const c_char) -> bool { CStr::from_ptr(val).to_str().is_ok() } + +fn is_alphanumeric_or_hyphen(chr: u8) -> bool { + return is_alphanumeric(chr) || chr == b'-'; +} + +fn parse_domain_label(i: &[u8]) -> IResult<&[u8], ()> { + let (i, _) = verify(take_while1(is_alphanumeric_or_hyphen), |x: &[u8]| { + is_alphabetic(x[0]) && x[x.len() - 1] != b'-' + })(i)?; + return Ok((i, ())); +} + +fn parse_subdomain(input: &[u8]) -> IResult<&[u8], ()> { + let (input, _) = parse_domain_label(input)?; + let (input, _) = char('.')(input)?; + return Ok((input, ())); +} + +fn parse_domain(input: &[u8]) -> IResult<&[u8], ()> { + let (input, _) = many1_count(parse_subdomain)(input)?; + let (input, _) = parse_domain_label(input)?; + return Ok((input, ())); +} + +#[no_mangle] +pub unsafe extern "C" fn rs_validate_domain(input: *const u8, in_len: u32) -> u32 { + let islice = build_slice!(input, in_len as usize); + match parse_domain(islice) { + Ok((rem, _)) => { + return (islice.len() - rem.len()) as u32; + } + _ => { + return 0; + } + } +} + +#[cfg(test)] +mod tests { + + use super::*; + + #[test] + fn test_parse_domain() { + let buf0: &[u8] = "a-1.oisf.net more".as_bytes(); + let r0 = parse_domain(buf0); + match r0 { + Ok((rem, _)) => { + // And we should have 5 bytes left. + assert_eq!(rem.len(), 5); + } + _ => { + panic!("Result should have been ok."); + } + } + let buf1: &[u8] = "justatext".as_bytes(); + let r1 = parse_domain(buf1); + match r1 { + Ok((_, _)) => { + panic!("Result should not have been ok."); + } + _ => {} + } + let buf1: &[u8] = "1.com".as_bytes(); + let r1 = parse_domain(buf1); + match r1 { + Ok((_, _)) => { + panic!("Result should not have been ok."); + } + _ => {} + } + let buf1: &[u8] = "a-.com".as_bytes(); + let r1 = parse_domain(buf1); + match r1 { + Ok((_, _)) => { + panic!("Result should not have been ok."); + } + _ => {} + } + let buf1: &[u8] = "a(x)y.com".as_bytes(); + let r1 = parse_domain(buf1); + match r1 { + Ok((_, _)) => { + panic!("Result should not have been ok."); + } + _ => {} + } + } +} diff --git a/src/app-layer-smtp.c b/src/app-layer-smtp.c index 95e84b64c110..728e50a57b71 100644 --- a/src/app-layer-smtp.c +++ b/src/app-layer-smtp.c @@ -1612,6 +1612,50 @@ static int SMTPStateGetEventInfoById(int event_id, const char **event_name, return 0; } +static AppProto SMTPServerProbingParser( + Flow *f, uint8_t direction, const uint8_t *input, uint32_t len, uint8_t *rdir) +{ + // another check for minimum length + if (len < 5) { + return ALPROTO_UNKNOWN; + } + // begins by 220 + if (input[0] != '2' || input[1] != '2' || input[2] != '0') { + return ALPROTO_FAILED; + } + // followed by space or hypen + if (input[3] != ' ' && input[3] != '-') { + return ALPROTO_FAILED; + } + // If client side is SMTP, do not validate domain + // so that server banner can be parsed first. + if (f->alproto_ts == ALPROTO_SMTP) { + for (uint32_t i = 4; i < len; i++) { + if (input[i] == '\n') { + return ALPROTO_SMTP; + } + } + return ALPROTO_UNKNOWN; + } + AppProto r = ALPROTO_UNKNOWN; + if (f->todstbytecnt > 4 && f->alproto_ts == ALPROTO_UNKNOWN) { + // Only validates SMTP if client side is unknown + // despite having received bytes. + r = ALPROTO_SMTP; + } + uint32_t offset = rs_validate_domain(input + 4, len - 4); + if (offset == 0) { + return ALPROTO_FAILED; + } + for (uint32_t i = offset + 4; i < len; i++) { + if (input[i] == '\n') { + return r; + } + } + // This should not go forever because of engine limiting probing parsers. + return ALPROTO_UNKNOWN; +} + static int SMTPRegisterPatternsForProtocolDetection(void) { if (AppLayerProtoDetectPMRegisterPatternCI(IPPROTO_TCP, ALPROTO_SMTP, @@ -1629,6 +1673,19 @@ static int SMTPRegisterPatternsForProtocolDetection(void) { return -1; } + if (!AppLayerProtoDetectPPParseConfPorts( + "tcp", IPPROTO_TCP, "smtp", ALPROTO_SMTP, 0, 5, NULL, SMTPServerProbingParser)) { + AppLayerProtoDetectPPRegister(IPPROTO_TCP, "25", ALPROTO_SMTP, 0, 5, STREAM_TOCLIENT, NULL, + SMTPServerProbingParser); + } + if (AppLayerProtoDetectPMRegisterPatternCSwPP(IPPROTO_TCP, ALPROTO_SMTP, "220 ", 4, 0, + STREAM_TOCLIENT, SMTPServerProbingParser, 5, 5) < 0) { + return -1; + } + if (AppLayerProtoDetectPMRegisterPatternCSwPP(IPPROTO_TCP, ALPROTO_SMTP, "220-", 4, 0, + STREAM_TOCLIENT, SMTPServerProbingParser, 5, 5) < 0) { + return -1; + } return 0; } From caf9282ff60da032caa140501a5ed749dc90e99e Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Fri, 1 Jul 2022 13:30:46 +0200 Subject: [PATCH 2/3] ftp: adds server side detection --- src/app-layer-ftp.c | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/src/app-layer-ftp.c b/src/app-layer-ftp.c index 1c0fea1e3386..d3c7aefdbc0c 100644 --- a/src/app-layer-ftp.c +++ b/src/app-layer-ftp.c @@ -1025,6 +1025,34 @@ static int FTPGetAlstateProgress(void *vtx, uint8_t direction) return FTP_STATE_FINISHED; } +static AppProto FTPServerProbingParser( + Flow *f, uint8_t direction, const uint8_t *input, uint32_t len, uint8_t *rdir) +{ + // another check for minimum length + if (len < 5) { + return ALPROTO_UNKNOWN; + } + // begins by 220 + if (input[0] != '2' || input[1] != '2' || input[2] != '0') { + return ALPROTO_FAILED; + } + // followed by space or hypen + if (input[3] != ' ' && input[3] != '-') { + return ALPROTO_FAILED; + } + AppProto r = ALPROTO_UNKNOWN; + if (f->alproto_ts == ALPROTO_FTP || (f->todstbytecnt > 4 && f->alproto_ts == ALPROTO_UNKNOWN)) { + // only validates FTP if client side was FTP + // or if client side is unknown despite having received bytes + r = ALPROTO_FTP; + } + for (uint32_t i = 4; i < len; i++) { + if (input[i] == '\n') { + return r; + } + } + return ALPROTO_UNKNOWN; +} static int FTPRegisterPatternsForProtocolDetection(void) { @@ -1053,7 +1081,13 @@ static int FTPRegisterPatternsForProtocolDetection(void) { return -1; } - + // Only check FTP on known ports as the banner has nothing special beyond + // the response code shared with SMTP. + if (!AppLayerProtoDetectPPParseConfPorts( + "tcp", IPPROTO_TCP, "ftp", ALPROTO_FTP, 0, 5, NULL, FTPServerProbingParser)) { + AppLayerProtoDetectPPRegister(IPPROTO_TCP, "21", ALPROTO_FTP, 0, 5, STREAM_TOCLIENT, NULL, + FTPServerProbingParser); + } return 0; } From 7f3f6536c226ed176e86dbfb322ebc3bc2d4e557 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Tue, 1 Nov 2022 15:01:52 +0100 Subject: [PATCH 3/3] protodetect: use stricter max probing parser length For probing parsers used with a pattern, restrict the max depth to these probing parsers and not all probing parsers. Finishing protocol detection earlier allows to parse data earlier in the case we recognize only one side. --- src/app-layer-detect-proto.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/app-layer-detect-proto.c b/src/app-layer-detect-proto.c index 863fb920afd6..e7793ec50d25 100644 --- a/src/app-layer-detect-proto.c +++ b/src/app-layer-detect-proto.c @@ -287,6 +287,10 @@ static inline int PMGetProtoInspect(AppLayerProtoDetectThreadCtx *tctx, uint8_t pm_results_bf[(ALPROTO_MAX / 8) + 1]; memset(pm_results_bf, 0, sizeof(pm_results_bf)); + // Do not take pm_ctx->pp_max_len of all probing parsers, + // but only the the probing parsers which matched a pattern. + uint32_t pp_max_len = pm_ctx->mpm_ctx.maxdepth; + /* loop through unique pattern id's. Can't use search_cnt here, * as that contains all matches, tctx->pmq.pattern_id_array_cnt * contains only *unique* matches. */ @@ -296,6 +300,9 @@ static inline int PMGetProtoInspect(AppLayerProtoDetectThreadCtx *tctx, AppProto proto = AppLayerProtoDetectPMMatchSignature( s, tctx, f, flags, buf, buflen, searchlen, rflow); + if (s->pp_max_depth > pp_max_len) { + pp_max_len = s->pp_max_depth; + } /* store each unique proto once */ if (AppProtoIsValid(proto) && !(pm_results_bf[proto / 8] & (1 << (proto % 8))) ) @@ -306,7 +313,7 @@ static inline int PMGetProtoInspect(AppLayerProtoDetectThreadCtx *tctx, s = s->next; } } - if (pm_matches == 0 && buflen >= pm_ctx->pp_max_len) { + if (pm_matches == 0 && buflen >= pp_max_len) { pm_matches = -2; } PmqReset(&tctx->pmq);