Skip to content

Commit

Permalink
Update DetectPortParse() to send back the ip protocol value.
Browse files Browse the repository at this point in the history
  • Loading branch information
@poona
poona committed Oct 7, 2013
1 parent 5e23c27 commit d8839c5
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 18 deletions.
5 changes: 3 additions & 2 deletions src/app-layer-parser.c
View file
Expand Up @@ -1667,6 +1667,7 @@ void AppLayerParseProbingParserPorts(const char *al_proto_name, uint16_t al_prot
ConfNode *node;
ConfNode *proto_node = NULL;
ConfNode *port_node = NULL;
uint16_t ip_proto;

r = snprintf(param, sizeof(param), "%s%s%s", "app-layer.protocols.",
al_proto_name, ".detection-ports");
Expand All @@ -1686,8 +1687,8 @@ void AppLayerParseProbingParserPorts(const char *al_proto_name, uint16_t al_prot
/* for each proto */
TAILQ_FOREACH(proto_node, &node->head, next) {
DetectProto dp;
int ip_proto = DetectProtoParse(&dp, proto_node->name);
if (ip_proto < 0) {
r = DetectProtoParse(&dp, proto_node->name, &ip_proto);
if (r < 0) {
SCLogError(SC_ERR_INVALID_YAML_CONF_ENTRY, "Invalid entry for "
"%s.%s", param, proto_node->name);
exit(EXIT_FAILURE);
Expand Down
53 changes: 39 additions & 14 deletions src/detect-engine-proto.c
View file
Expand Up @@ -80,44 +80,66 @@ void DetectProtoFree(DetectProto *dp)
* \param dp Pointer to the DetectProto instance which will be updated with the
* incoming protocol information.
* \param str Pointer to the string containing the protocol name.
* \param ip_proto Pointer to an [out] variable to send back the ip_protocol
* value, for the input sent.
*
* \retval >=0 If proto is detected, -1 otherwise.
*/
int DetectProtoParse(DetectProto *dp, char *str)
int DetectProtoParse(DetectProto *dp, char *str, uint16_t *ip_proto)

This comment has been minimized.

Sign in to view

Copy link
@inliniac

inliniac Oct 8, 2013

Why is ip_proto u16? ip proto is u8 normally

This comment has been minimized.

Sign in to view

Copy link
@poona

poona Oct 8, 2013

Author Owner

Yeah, even I'm not sure why I put u16 there. Will fix this.
{
if (ip_proto != NULL)
*ip_proto = 0;

uint16_t ip_proto_tmp = 0;

if (strcasecmp(str, "tcp") == 0) {
dp->proto[IPPROTO_TCP / 8] |= 1 << (IPPROTO_TCP % 8);
ip_proto_tmp = IPPROTO_TCP;
dp->proto[ip_proto_tmp / 8] |= 1 << (ip_proto_tmp % 8);
SCLogDebug("TCP protocol detected");
} else if (strcasecmp(str, "tcp-pkt") == 0) {
dp->proto[IPPROTO_TCP / 8] |= 1 << (IPPROTO_TCP % 8);
ip_proto_tmp = IPPROTO_TCP;
dp->proto[ip_proto_tmp / 8] |= 1 << (ip_proto_tmp % 8);
SCLogDebug("TCP protocol detected, packets only");
dp->flags |= DETECT_PROTO_ONLY_PKT;
} else if (strcasecmp(str, "tcp-stream") == 0) {
dp->proto[IPPROTO_TCP / 8] |= 1 << (IPPROTO_TCP % 8);
ip_proto_tmp = IPPROTO_TCP;
dp->proto[ip_proto_tmp / 8] |= 1 << (ip_proto_tmp % 8);
SCLogDebug("TCP protocol detected, stream only");
dp->flags |= DETECT_PROTO_ONLY_STREAM;
} else if (strcasecmp(str, "udp") == 0) {
dp->proto[IPPROTO_UDP / 8] |= 1 << (IPPROTO_UDP % 8);
ip_proto_tmp = IPPROTO_UDP;
dp->proto[ip_proto_tmp / 8] |= 1 << (ip_proto_tmp % 8);
SCLogDebug("UDP protocol detected");
} else if (strcasecmp(str, "icmp") == 0) {
/* AWS, how do we handle this? */
ip_proto_tmp = 0;
dp->proto[IPPROTO_ICMP / 8] |= 1 << (IPPROTO_ICMP % 8);
dp->proto[IPPROTO_ICMPV6 / 8] |= 1 << (IPPROTO_ICMPV6 % 8);
SCLogDebug("ICMP protocol detected, sig applies both to ICMPv4 and ICMPv6");
} else if (strcasecmp(str, "sctp") == 0) {
dp->proto[IPPROTO_SCTP / 8] |= 1 << (IPPROTO_SCTP % 8);
ip_proto_tmp = IPPROTO_SCTP;
dp->proto[ip_proto_tmp / 8] |= 1 << (ip_proto_tmp % 8);
SCLogDebug("SCTP protocol detected");
} else if (strcasecmp(str,"ipv4") == 0 ||
strcasecmp(str,"ip4") == 0 ) {
/* AWS, how do we handle this? Some universal value that

This comment has been minimized.

Sign in to view

Copy link
@inliniac

inliniac Oct 8, 2013

This is why we have the proto bitarray. We set it to 0xff and the flag to ANY. Why would we need another solution?

The caller could check this array and/or flag.

This comment has been minimized.

Sign in to view

Copy link
@poona

poona Oct 8, 2013

Author Owner

I will have to check for for IPPROTO_TCP/UDP_SCTP values. Should still work, yeah.
* represents all protocols?*/
ip_proto_tmp = 0;
dp->flags |= (DETECT_PROTO_IPV4 | DETECT_PROTO_ANY);
memset(dp->proto, 0xff, sizeof(dp->proto));
SCLogDebug("IPv4 protocol detected");
} else if (strcasecmp(str,"ipv6") == 0 ||
strcasecmp(str,"ip6") == 0 ) {
/* AWS, how do we handle this? */
ip_proto_tmp = 0;
dp->flags |= (DETECT_PROTO_IPV6 | DETECT_PROTO_ANY);
memset(dp->proto, 0xff, sizeof(dp->proto));
SCLogDebug("IPv6 protocol detected");
} else if (strcasecmp(str,"ip") == 0 ||
strcasecmp(str,"pkthdr") == 0) {
/* AWS, how do we handle this? Some universal value that
* represents all protocols?*/
ip_proto_tmp = 0;
/* Proto "ip" is treated as an "any" */
dp->flags |= DETECT_PROTO_ANY;
memset(dp->proto, 0xff, sizeof(dp->proto));
Expand Down Expand Up @@ -146,6 +168,9 @@ int DetectProtoParse(DetectProto *dp, char *str)
#endif
}

if (ip_proto != NULL)
*ip_proto = ip_proto_tmp;

return 0;
error:
return -1;
Expand Down Expand Up @@ -204,7 +229,7 @@ static int DetectProtoInitTest(DetectEngineCtx **de_ctx, Signature **sig,

*sig = (*de_ctx)->sig_list;

if (DetectProtoParse(dp, str) < 0)
if (DetectProtoParse(dp, str, NULL) < 0)
goto end;

result = 1;
Expand All @@ -222,7 +247,7 @@ static int ProtoTestParse01 (void)
DetectProto dp;
memset(&dp,0,sizeof(DetectProto));

int r = DetectProtoParse(&dp, "6");
int r = DetectProtoParse(&dp, "6", NULL);
if (r < 0) {
return 1;
}
Expand All @@ -239,7 +264,7 @@ static int ProtoTestParse02 (void)
DetectProto dp;
memset(&dp,0,sizeof(DetectProto));

int r = DetectProtoParse(&dp, "tcp");
int r = DetectProtoParse(&dp, "tcp", NULL);
if (r >= 0 && dp.proto[(IPPROTO_TCP/8)] & (1<<(IPPROTO_TCP%8))) {
return 1;
}
Expand All @@ -256,7 +281,7 @@ static int ProtoTestParse03 (void)
DetectProto dp;
memset(&dp,0,sizeof(DetectProto));

int r = DetectProtoParse(&dp, "ip");
int r = DetectProtoParse(&dp, "ip", NULL);
if (r >= 0 && dp.flags & DETECT_PROTO_ANY) {
return 1;
}
Expand All @@ -275,7 +300,7 @@ static int ProtoTestParse04 (void)
memset(&dp,0,sizeof(DetectProto));

/* Check for a bad number */
int r = DetectProtoParse(&dp, "4242");
int r = DetectProtoParse(&dp, "4242", NULL);
if (r < 0) {
return 1;
}
Expand All @@ -294,7 +319,7 @@ static int ProtoTestParse05 (void)
memset(&dp,0,sizeof(DetectProto));

/* Check for a bad string */
int r = DetectProtoParse(&dp, "tcp/udp");
int r = DetectProtoParse(&dp, "tcp/udp", NULL);
if (r < 0) {
return 1;
}
Expand All @@ -312,7 +337,7 @@ static int ProtoTestParse06 (void)
memset(&dp,0,sizeof(DetectProto));

/* Check for a bad string */
int r = DetectProtoParse(&dp, "tcp-pkt");
int r = DetectProtoParse(&dp, "tcp-pkt", NULL);
if (r < 0) {
printf("parsing tcp-pkt failed: ");
return 0;
Expand All @@ -335,7 +360,7 @@ static int ProtoTestParse07 (void)
memset(&dp,0,sizeof(DetectProto));

/* Check for a bad string */
int r = DetectProtoParse(&dp, "tcp-stream");
int r = DetectProtoParse(&dp, "tcp-stream", NULL);
if (r < 0) {
printf("parsing tcp-stream failed: ");
return 0;
Expand Down
5 changes: 4 additions & 1 deletion src/detect-engine-proto.h
View file
Expand Up @@ -24,6 +24,9 @@
#ifndef __DETECT_PROTO_H__
#define __DETECT_PROTO_H__

/* These are DetectProto flags only, and shouldn't be mistaken as
* being chums of our standardized values like IPPROTO_TCP, IPPROTO_UDP,
* etc. */
#define DETECT_PROTO_ANY (1 << 0) /**< Indicate that given protocol
is considered as IP */
#define DETECT_PROTO_ONLY_PKT (1 << 1) /**< Indicate that we only care
Expand All @@ -39,7 +42,7 @@ typedef struct DetectProto_ {
} DetectProto;

/* prototypes */
int DetectProtoParse(DetectProto *dp, char *str);
int DetectProtoParse(DetectProto *dp, char *str, uint16_t *ip_proto);
int DetectProtoContainsProto(DetectProto *, int);

void DetectProtoTests(void);
Expand Down
2 changes: 1 addition & 1 deletion src/detect-parse.c
View file
Expand Up @@ -589,7 +589,7 @@ int SigParseProto(Signature *s, const char *protostr) {
AppLayerProbingParserPort *pp_port;
AppLayerProbingParserElement *pp_pe;

int r = DetectProtoParse(&s->proto, (char *)protostr);
int r = DetectProtoParse(&s->proto, (char *)protostr, NULL);
if (r < 0) {
s->alproto = AppLayerGetProtoByName(protostr);
/* indicate that the signature is app-layer */
Expand Down

5 comments on commit d8839c5

@inliniac
Copy link

Choose a reason for hiding this comment

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

This commit actually addresses the error in Bug 987.

@poona
Copy link
Owner Author

@poona poona commented on d8839c5 Oct 17, 2013

Choose a reason for hiding this comment

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

No, this doesn't address OISF#987.

@inliniac
Copy link

Choose a reason for hiding this comment

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

It did :) I tested each commit separately and it made the error go away.

@poona
Copy link
Owner Author

@poona poona commented on d8839c5 Oct 17, 2013

Choose a reason for hiding this comment

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

This specific commit is not right. I have updated DetectProtoParse() to not send the third argument, and use the bitarray instead, which as you can see has already been pushed into the master through some other PR. This commit in other words holds wrong code.

bug OISF#987 has nothing to do with this commit.

@inliniac
Copy link

@inliniac inliniac commented on d8839c5 Oct 17, 2013 via email

Choose a reason for hiding this comment

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

Please sign in to comment.