CR comments and fixes

This commit is contained in:
Yoni.Linder 2017-05-19 14:16:21 +03:00 committed by alexei-argus
parent 6029510ee7
commit 0aa5ebde5d

View file

@ -24,11 +24,7 @@
#include "ndpi_protocols.h"
#ifdef NDPI_PROTOCOL_SOMEIP
// CR: these MQTT references are no longer relevant, rigth?
/**
* The type of control messages in mqtt version 3.1.1
* see http://docs.oasis-open.org/mqtt/mqtt/v3.1.1
*/
// CR: these MQTT references are no longer relevant, rigth? ANS: true.
enum SOMEIP_MESSAGE_TYPES {
REQUEST = 0x00,
REQUEST_NO_RETURN = 0x01,
@ -53,7 +49,8 @@ enum SOMEIP_RETURN_CODES {
E_WRONG_PROTOCOL_VERSION = 0x07,
E_WRONG_INTERFACE_VERSION = 0x08,
E_MALFORMED_MESSAGE = 0x09,
E_WRONG_MESSAGE_TYPE = 0x0a
E_WRONG_MESSAGE_TYPE = 0x0a,
E_RETURN_CODE_LEGAL_THRESHOLD = 0x40 //return codes from 0x40 (inclusive) and upwards are illegal.
};
enum SPECIAL_MESSAGE_IDS {
@ -62,6 +59,21 @@ enum SPECIAL_MESSAGE_IDS {
MSG_SD = 0xffff8100
};
enum PROTOCOL_VERSION{
LEGAL_PROTOCOL_VERSION = 0x01
};
enum MAGIC_COOKIE_CONSTANTS{
MC_REQUEST_ID = 0xDEADBEEF,
MC_LENGTH = 0x08,
MC_INTERFACE_VERSION = 0x01
};
enum DEFAULT_PROTOCOL_PORTS{
PORT_DEFAULT_CLIENT = 30491,
PORT_DEFAULT_SERVER = 30501,
PORT_DEFAULT_SD = 30490
};
/**
* Entry point when protocol is identified.
@ -83,12 +95,12 @@ void ndpi_search_someip (struct ndpi_detection_module_struct *ndpi_struct,
//####Maybe check carrier protocols?####
NDPI_LOG(NDPI_PROTOCOL_SOMEIP, ndpi_struct, NDPI_LOG_DEBUG, "SOME/IP search called...\n");
// CR: can packet be const?
struct ndpi_packet_struct *packet = &flow->packet;
// CR: can packet be const? ANS: Probably yeah, needs testing but I changed it.
struct const ndpi_packet_struct *packet = &flow->packet;
if (packet->detected_protocol_stack[0] != NDPI_PROTOCOL_UNKNOWN) {
return;
}
// CR: let's reach a decision in this issue.
// CR: let's reach a decision in this issue. ANS: I think it's unnecessary and would get dropped on length checks or whatever, so we can remove this.
/*NDPI_LOG(NDPI_PROTOCOL_SOMEIP, ndpi_struct, NDPI_LOG_DEBUG, "SOME/IP detection...\n");
if (flow->packet_counter > 10) {
@ -100,8 +112,8 @@ void ndpi_search_someip (struct ndpi_detection_module_struct *ndpi_struct,
*/
//we extract the Message ID and Request ID and check for special cases later
u_int32_t message_id = (u_int32_t) ((packet->payload[0]<<24)+(packet->payload[1]<<16)+(packet->payload[2]<<8)+packet->payload[3]);
u_int32_t request_id = (u_int32_t) ((packet->payload[8]<<24)+(packet->payload[9]<<16)+(packet->payload[10]<<8)+packet->payload[11]);
u_int32_t message_id = ntohl(*((u_int32_t *)packet->payload[0]));
u_int32_t request_id = ntohl(*((u_int32_t *)packet->payload[8]));
NDPI_LOG(NDPI_PROTOCOL_SOMEIP, ndpi_struct, NDPI_LOG_DEBUG, "====>>>> SOME/IP Message ID: %08x [len: %u]\n",
message_id, packet->payload_packet_len);
@ -119,8 +131,8 @@ void ndpi_search_someip (struct ndpi_detection_module_struct *ndpi_struct,
*/
// we extract the remaining length
// CR: cast the payload to unsigned int, then use ntohl
u_int32_t someip_len = (u_int32_t) ((packet->payload[4]<<24) + (packet->payload[5]<<16) + (packet->payload[6]<<8) +packet->payload[7]);
// CR: cast the payload to unsigned int, then use ntohl ANS: done
u_int32_t someip_len = ntohl(*((u_int32_t *)packet->payload[4]));
if (packet->payload_packet_len != (someip_len + 8)) {
NDPI_LOG(NDPI_PROTOCOL_SOMEIP, ndpi_struct, NDPI_LOG_DEBUG, "Excluding SOME/IP .. Length field invalid!\n");
NDPI_ADD_PROTOCOL_TO_BITMASK(flow->excluded_protocol_bitmask, NDPI_PROTOCOL_SOMEIP);
@ -129,8 +141,8 @@ void ndpi_search_someip (struct ndpi_detection_module_struct *ndpi_struct,
u_int8_t protocol_version = (u_int8_t) (packet->payload[12]);
NDPI_LOG(NDPI_PROTOCOL_SOMEIP, ndpi_struct, NDPI_LOG_DEBUG,"====>>>> SOME/IP protocol version: [%d]\n",protocol_version);
// CR: don't use magic numbers, convert this to a constant instead
if (protocol_version != 0x01){
// CR: don't use magic numbers, convert this to a constant instead ANS: done
if (protocol_version != LEGAL_PROTOCOL_VERSION){
NDPI_LOG(NDPI_PROTOCOL_SOMEIP, ndpi_struct, NDPI_LOG_DEBUG, "Excluding SOME/IP .. invalid protocol version!\n");
NDPI_ADD_PROTOCOL_TO_BITMASK(flow->excluded_protocol_bitmask, NDPI_PROTOCOL_SOMEIP);
return;
@ -141,9 +153,10 @@ void ndpi_search_someip (struct ndpi_detection_module_struct *ndpi_struct,
u_int8_t message_type = (u_int8_t) (packet->payload[14]);
NDPI_LOG(NDPI_PROTOCOL_SOMEIP, ndpi_struct, NDPI_LOG_DEBUG,"====>>>> SOME/IP message type: [%d]\n",message_type);
// CR: don't use magic numbers, convert these to constants instead
if ((message_type != 0x00) && (message_type != 0x01) && (message_type != 0x02) && (message_type != 0x40) && (message_type != 0x41) &&
(message_type != 0x42) && (message_type != 0x80) && (message_type != 0x81) && (message_type != 0xc0) && (message_type != 0xc1)) {
// CR: don't use magic numbers, convert these to constants instead ANS: done
if ((message_type != REQUEST) && (message_type != REQUEST_NO_RETURN) && (message_type != NOTIFICATION) && (message_type != REQUEST_ACK) &&
(message_type != REQUEST_NO_RETURN_ACK) && (message_type != NOTIFICATION_ACK) && (message_type != RESPONSE) &&
(message_type != ERROR) && (message_type != RESPONSE_ACK) && (message_type != ERROR_ACK)) {
NDPI_LOG(NDPI_PROTOCOL_SOMEIP, ndpi_struct, NDPI_LOG_DEBUG, "Excluding SOME/IP .. invalid message type!\n");
NDPI_ADD_PROTOCOL_TO_BITMASK(flow->excluded_protocol_bitmask, NDPI_PROTOCOL_SOMEIP);
return;
@ -151,17 +164,17 @@ void ndpi_search_someip (struct ndpi_detection_module_struct *ndpi_struct,
u_int8_t return_code = (u_int8_t) (packet->payload[15]);
NDPI_LOG(NDPI_PROTOCOL_SOMEIP, ndpi_struct, NDPI_LOG_DEBUG,"====>>>> SOME/IP return code: [%d]\n", return_code);
// CR: don't use magic numbers, convert this to a constant instead
if ((return_code > 0x3f)) {
// CR: don't use magic numbers, convert this to a constant instead ANS: done
if ((return_code >= E_RETURN_CODE_LEGAL_THRESHOLD)) {
NDPI_LOG(NDPI_PROTOCOL_SOMEIP, ndpi_struct, NDPI_LOG_DEBUG, "Excluding SOME/IP .. invalid return code!\n");
NDPI_ADD_PROTOCOL_TO_BITMASK(flow->excluded_protocol_bitmask, NDPI_PROTOCOL_SOMEIP);
return;
}
if (message_id == MSG_MAGIC_COOKIE){
// CR: don't use magic numbers, convert these to constants instead
if ((someip_len == 0x08) && (request_id == 0xDEADBEEF) && (interface_version == 0x01) &&
(message_type == 0x01) && (return_code == 0x00)){
// CR: don't use magic numbers, convert these to constants instead ANS:done
if ((someip_len == MC_LENGTH) && (request_id == MC_REQUEST_ID) && (interface_version == MC_INTERFACE_VERSION) &&
(message_type == REQUEST_NO_RETURN\) && (return_code == E_OK)){
NDPI_LOG(NDPI_PROTOCOL_SOMEIP, ndpi_struct, NDPI_LOG_DEBUG, "SOME/IP found Magic Cookie\n",message_type);
ndpi_int_someip_add_connection(ndpi_struct, flow);
return;
@ -174,9 +187,9 @@ void ndpi_search_someip (struct ndpi_detection_module_struct *ndpi_struct,
}
if (message_id == MSG_MAGIC_COOKIE_ACK){
// CR: don't use magic numbers, convert these to constants instead
if ((someip_len == 0x08) && (request_id == 0xDEADBEEF) && (interface_version == 0x01) &&
(message_type == 0x02) && (return_code == 0x00)){
// CR: don't use magic numbers, convert these to constants instead ANS: done
if ((someip_len == MC_LENGTH) && (request_id == MC_REQUEST_ID) && (interface_version == MC_INTERFACE_VERSION\) &&
(message_type == REQUEST_NO_RETURN) && (return_code == E_OK)){
NDPI_LOG(NDPI_PROTOCOL_SOMEIP, ndpi_struct, NDPI_LOG_DEBUG, "SOME/IP found Magic Cookie ACK\n",message_type);
ndpi_int_someip_add_connection(ndpi_struct, flow);
return;
@ -189,24 +202,26 @@ void ndpi_search_someip (struct ndpi_detection_module_struct *ndpi_struct,
}
if (message_id == MSG_SD){
// CR: let's talk about this (i.e. what should be here right now? what documentation should we leave behind?)
// CR: let's talk about this (i.e. what should be here right now? what documentation should we leave behind?) ANS: a TON of stuff. SD is basically another protocol built ontop SOMEIP. at the very least I expect it to be as long as everything else we've done already.
//####Service Discovery message. Fill in later!####
}
// CR: while this is for demo purposes, the port numbers are as specified in the SOME/IP document, so we should change the
// comment to reflect this.
// Also, don't use magic numbers, use constants.
// comment to reflect this. ANS: done
// Also, don't use magic numbers, use constants. ANS: done
//Filtering by port as per request. This is PURELY for demo purposes and the rest of the check must be filled in later on!
//Filtering by port.
//This check is NOT a 100% thing - these ports are mentioned in the documentation but the documentation also states they haven't been approved by IANA yet, and that the user is free to use different ports.
//This is is PURELY for demo purposes and the rest of the check must be filled in later on!
if (packet->l4_protocol == IPPROTO_UDP){
if ((packet->udp->dest == ntohs(30491)) || (packet->udp->dest == ntohs(30501)) || (packet->udp->dest == ntohs(30490))) {
if ((packet->udp->dest == ntohs(PORT_DEFAULT_CLIENT)) || (packet->udp->dest == ntohs(PORT_DEFAULT_SERVER)) || (packet->udp->dest == ntohs(PORT_DEFAULT_SD))) {
NDPI_LOG(NDPI_PROTOCOL_SOMEIP, ndpi_struct, NDPI_LOG_DEBUG, "SOME/IP found\n",message_type);
ndpi_int_someip_add_connection(ndpi_struct, flow);
return;
}
}
if (packet->l4_protocol == IPPROTO_TCP){
if ((packet->tcp->dest == ntohs(30491)) || (packet->tcp->dest == ntohs(30501))) {
if ((packet->tcp->dest == ntohs(PORT_DEFAULT_CLIENT)) || (packet->tcp->dest == ntohs(PORT_DEFAULT_SERVER))) {
NDPI_LOG(NDPI_PROTOCOL_SOMEIP, ndpi_struct, NDPI_LOG_DEBUG, "SOME/IP found\n",message_type);
ndpi_int_someip_add_connection(ndpi_struct, flow);
return;