Merge pull request ARMmbed#1924 from ARMmbed/IOTTHD-1608

Update CoAP message prevalidation

Author: Arto Kinnunen

source/6LoWPAN/Thread/thread_leader_service.c

@@ -393,11 +393,6 @@ static int thread_leader_service_active_set_cb(int8_t service_id, uint8_t source
 
     response_ptr = payload;
 
-    if (!thread_management_server_source_address_check(this->interface_id, source_address)) {
-        response_code = COAP_MSG_CODE_RESPONSE_BAD_REQUEST;
-        goto send_error_response;
-    }
-
     if (3 <= thread_meshcop_tlv_find(request_ptr->payload_ptr, request_ptr->payload_len, MESHCOP_TLV_CHANNEL, &ptr) &&
             (linkConfiguration->rfChannel != common_read_16_bit(&ptr[1]) || linkConfiguration->channel_page != *ptr)) {
         tr_debug("Channel changed");
@@ -559,11 +554,6 @@ static int thread_leader_service_pending_set_cb(int8_t service_id, uint8_t sourc
 
     tr_info("thread management Pending set");
 
-    if (!thread_management_server_source_address_check(this->interface_id, source_address)) {
-        response_code = COAP_MSG_CODE_RESPONSE_BAD_REQUEST;
-        goto send_error_response;
-    }
-
     if (2 <= thread_meshcop_tlv_data_get_uint16(request_ptr->payload_ptr, request_ptr->payload_len, MESHCOP_TLV_COMMISSIONER_SESSION_ID, &session_id)) {
         // Session id present must be valid
         if (cur->thread_info->registered_commissioner.session_id != session_id) {
@@ -660,7 +650,6 @@ static int thread_leader_service_pending_set_cb(int8_t service_id, uint8_t sourc
 static int thread_leader_service_commissioner_set_cb(int8_t service_id, uint8_t source_address[16], uint16_t source_port, sn_coap_hdr_s *request_ptr)
 {
     thread_leader_service_t *this = thread_leader_service_find_by_service(service_id);
-    sn_coap_msg_code_e response_code = COAP_MSG_CODE_RESPONSE_CHANGED;
     uint16_t session_id;
     uint16_t br_locator;
     uint8_t payload[5]; // 4 + 1
@@ -682,10 +671,6 @@ static int thread_leader_service_commissioner_set_cb(int8_t service_id, uint8_t
 
     tr_info("thread management commissioner set");
 
-    if (!thread_management_server_source_address_check(this->interface_id, source_address)) {
-        response_code = COAP_MSG_CODE_RESPONSE_BAD_REQUEST;
-        goto send_error_response;
-    }
     //Check if the CoAp payload is greater than maximum commissioner data size and reject
     if (request_ptr->payload_len > THREAD_MAX_COMMISSIONER_DATA_SIZE) {
         tr_error("Payload length greater than maximum commissioner data size");
@@ -725,9 +710,7 @@ static int thread_leader_service_commissioner_set_cb(int8_t service_id, uint8_t
 send_response:
     // build response
     ptr = thread_meshcop_tlv_data_write_uint8(ptr, MESHCOP_TLV_STATE, ret == 0 ? 1 : 0xff);
-
-send_error_response:
-    coap_service_response_send(this->coap_service_id, COAP_REQUEST_OPTIONS_NONE, request_ptr, response_code, COAP_CT_OCTET_STREAM, payload, ptr - payload);
+    coap_service_response_send(this->coap_service_id, COAP_REQUEST_OPTIONS_NONE, request_ptr, COAP_MSG_CODE_RESPONSE_CHANGED, COAP_CT_OCTET_STREAM, payload, ptr - payload);
     return 0;
 }
 
@@ -1050,7 +1033,6 @@ static int thread_leader_service_release_cb(int8_t service_id, uint8_t source_ad
 static int thread_leader_service_petition_cb(int8_t service_id, uint8_t source_address[16], uint16_t source_port, sn_coap_hdr_s *request_ptr)
 {
     thread_leader_service_t *this = thread_leader_service_find_by_service(service_id);
-    sn_coap_msg_code_e response_code = COAP_MSG_CODE_RESPONSE_CHANGED;
     uint8_t payload[79];// max length for commissioner id is 64 + 4 byte header + 4 + 1 + 4 + 2
     uint8_t *ptr;
     uint16_t session_id = 0;
@@ -1067,11 +1049,6 @@ static int thread_leader_service_petition_cb(int8_t service_id, uint8_t source_a
 
     tr_debug("Thread management commissioner petition");
 
-    if (!thread_management_server_source_address_check(this->interface_id, source_address)) {
-        response_code = COAP_MSG_CODE_RESPONSE_BAD_REQUEST;
-        goto send_error_response;
-    }
-
     // save values from message
     tlv_length = thread_meshcop_tlv_find(request_ptr->payload_ptr, request_ptr->payload_len, MESHCOP_TLV_COMMISSIONER_ID, &tlv_data_ptr);
 
@@ -1098,8 +1075,7 @@ static int thread_leader_service_petition_cb(int8_t service_id, uint8_t source_a
 
     tr_debug("Petition req recv id %s, RESP session id: %d ret %d", commissioner_id_ptr ? commissioner_id_ptr : "(none)", session_id, ret);
 
-send_error_response:
-    coap_service_response_send(this->coap_service_id, COAP_REQUEST_OPTIONS_NONE, request_ptr, response_code, COAP_CT_OCTET_STREAM, payload, ptr - payload);
+    coap_service_response_send(this->coap_service_id, COAP_REQUEST_OPTIONS_NONE, request_ptr, COAP_MSG_CODE_RESPONSE_CHANGED, COAP_CT_OCTET_STREAM, payload, ptr - payload);
     return 0;
 }
 
@@ -1110,7 +1086,6 @@ static int thread_leader_service_petition_cb(int8_t service_id, uint8_t source_a
 static int thread_leader_service_petition_ka_cb(int8_t service_id, uint8_t source_address[16], uint16_t source_port, sn_coap_hdr_s *request_ptr)
 {
     thread_leader_service_t *this = thread_leader_service_find_by_service(service_id);
-    sn_coap_msg_code_e response_code = COAP_MSG_CODE_RESPONSE_CHANGED;
     uint8_t payload[5]; //status 4 + 1
     uint8_t *ptr;
     uint16_t session_id = 0;
@@ -1125,12 +1100,6 @@ static int thread_leader_service_petition_ka_cb(int8_t service_id, uint8_t sourc
 
     tr_debug("Thread management commissioner keep alive");
 
-    if (!thread_management_server_source_address_check(this->interface_id, source_address)) {
-        response_code = COAP_MSG_CODE_RESPONSE_BAD_REQUEST;
-        ptr = payload;
-        goto send_error_response;
-    }
-
     if (2 <= thread_meshcop_tlv_find(request_ptr->payload_ptr, request_ptr->payload_len, MESHCOP_TLV_COMMISSIONER_SESSION_ID, &ptr)) {
         session_id = common_read_16_bit(ptr);
     }
@@ -1149,8 +1118,7 @@ static int thread_leader_service_petition_ka_cb(int8_t service_id, uint8_t sourc
     ptr = payload;
     ptr = thread_meshcop_tlv_data_write_uint8(ptr, MESHCOP_TLV_STATE, state == true ? 1 : 0xff);
 
-send_error_response:
-    coap_service_response_send(this->coap_service_id, COAP_REQUEST_OPTIONS_NONE, request_ptr, response_code, COAP_CT_OCTET_STREAM, payload, ptr - payload);
+    coap_service_response_send(this->coap_service_id, COAP_REQUEST_OPTIONS_NONE, request_ptr, COAP_MSG_CODE_RESPONSE_CHANGED, COAP_CT_OCTET_STREAM, payload, ptr - payload);
     return 0;
 }
 

source/6LoWPAN/Thread/thread_management_server.c

@@ -72,6 +72,13 @@
 
 #ifdef HAVE_THREAD
 
+#define TRACE_DEEP
+#ifdef TRACE_DEEP
+#define tr_deep   tr_debug
+#else
+#define tr_deep(...)
+#endif
+
 typedef struct scan_query {
     int8_t coap_service_id;
     uint8_t channel_mask[6]; //<!** first byte is channel page
@@ -395,19 +402,14 @@ static int thread_management_server_pending_get_respond(int8_t interface_id, int
 static int thread_management_server_get_command_cb(int8_t service_id, uint8_t source_address[16], uint16_t source_port, sn_coap_hdr_s *request_ptr)
 {
     (void) source_port;
+    (void) source_address;
 
     thread_management_server_t *this = thread_management_find_by_service(service_id);
 
     if (!this) {
         return -1;
     }
 
-    if (!thread_management_server_source_address_check(this->interface_id, source_address)) {
-        // request is coming from illegal address, return error immediately
-        coap_service_response_send(service_id, COAP_REQUEST_OPTIONS_NONE, request_ptr, COAP_MSG_CODE_RESPONSE_BAD_REQUEST, COAP_CT_OCTET_STREAM, NULL, 0);
-        return 0;
-    }
-
     return thread_management_server_tmf_get_request_handler(this->interface_id, service_id, request_ptr);
 
 }
@@ -418,7 +420,6 @@ static int thread_management_server_commissioner_get_cb(int8_t service_id, uint8
     (void) source_port;
     protocol_interface_info_entry_t *cur;
     thread_management_server_t *this = thread_management_find_by_service(service_id);
-    sn_coap_msg_code_e return_code = COAP_MSG_CODE_RESPONSE_CHANGED;
     uint8_t response_msg[2 + 2 + 2 + 2 + 2 + 16 + 2 + 2];
     uint8_t *request_tlv_ptr = NULL;
     uint16_t request_tlv_len;
@@ -434,11 +435,6 @@ static int thread_management_server_commissioner_get_cb(int8_t service_id, uint8
     }
     payload_ptr = ptr = response_msg;
 
-    if (!thread_management_server_source_address_check(this->interface_id, source_address)) {
-        return_code = COAP_MSG_CODE_RESPONSE_BAD_REQUEST;
-        goto send_response;
-    }
-
     if (!cur->thread_info->registered_commissioner.commissioner_valid) {
         //Error in message is responded with Thread status or if we have access rights problem
         tr_warn("No registered commissioner");
@@ -466,7 +462,7 @@ static int thread_management_server_commissioner_get_cb(int8_t service_id, uint8
         goto send_response;
     }
 send_response:
-    coap_service_response_send(this->coap_service_id, COAP_REQUEST_OPTIONS_NONE, request_ptr, return_code, COAP_CT_OCTET_STREAM, payload_ptr, ptr - payload_ptr);
+    coap_service_response_send(this->coap_service_id, COAP_REQUEST_OPTIONS_NONE, request_ptr, COAP_MSG_CODE_RESPONSE_CHANGED, COAP_CT_OCTET_STREAM, payload_ptr, ptr - payload_ptr);
     return 0;
 }
 
@@ -1120,33 +1116,57 @@ static int thread_management_server_announce_begin_cb(int8_t service_id, uint8_t
     return 0;
 }
 
-static int coap_msg_prevalidate_cb(int8_t interface_id, uint8_t source_address[static 16], uint16_t source_port, uint8_t local_address[static 16], uint16_t local_port, char *coap_uri)
+static int coap_msg_prevalidate_cb(int8_t local_interface_id, uint8_t local_address[static 16], uint16_t local_port, int8_t recv_interface_id, uint8_t source_address[static 16], uint16_t source_port, char *coap_uri)
 {
-    protocol_interface_info_entry_t *cur;
-    uint_fast8_t local_addr_scope;
+    protocol_interface_info_entry_t *cur_local, *cur_source;
+    uint_fast8_t addr_scope;
 
     (void) source_address;
     (void) source_port;
     (void) coap_uri;
 
-    cur = protocol_stack_interface_info_get_by_id(interface_id);
+    cur_local = protocol_stack_interface_info_get_by_id(local_interface_id);
+
+    tr_debug("coap_msg_prevalidate_cb %s to %s from %s to port %d", coap_uri, trace_ipv6(local_address), trace_ipv6(source_address), local_port);
 
-    if (!cur) {
-        tr_error("No interface");
+    if (!cur_local) {
+        tr_error("No interface for %d", local_interface_id);
         return -1;
     }
 
     if (local_port != THREAD_MANAGEMENT_PORT) {
         // Message not sent to THREAD_MANAGEMENT_PORT, let it come through
+        tr_deep("Message %s port %d is not mgmt port", coap_uri, local_port);
         return 0;
     }
 
-    /* check our address scope */
-    local_addr_scope = addr_ipv6_scope(local_address, cur);
-    if (local_addr_scope > IPV6_SCOPE_REALM_LOCAL) {
+    // check message source address
+    if (!thread_management_server_source_address_check(local_interface_id, source_address)) {
+        tr_deep("Drop CoAP msg %s from %s", coap_uri, trace_ipv6(source_address));
+        return 3;
+    }
+
+    /* check our local address scope */
+    addr_scope = addr_ipv6_scope(local_address, cur_local);
+    if (addr_scope > IPV6_SCOPE_REALM_LOCAL) {
+        tr_deep("Drop CoAP msg %s to %s due %d", coap_uri, trace_ipv6(local_address), addr_scope);
         return 1;
     }
 
+    if (local_interface_id != recv_interface_id) {
+        // message received from different interface
+        cur_source = protocol_stack_interface_info_get_by_id(recv_interface_id);
+        if (!cur_source) {
+            tr_deep("No cur for if %d", recv_interface_id);
+            return -1;
+        }
+        addr_scope = addr_ipv6_scope(source_address, cur_source);
+        if (addr_scope < IPV6_SCOPE_REALM_LOCAL) {
+            tr_deep("Drop CoAP msg %s from %s to %s due %d", coap_uri, trace_ipv6(source_address), trace_ipv6(local_address), addr_scope);
+            return 2;
+        }
+    }
+
     return 0;
 }
 
@@ -1194,7 +1214,7 @@ int thread_management_server_init(int8_t interface_id)
         ns_dyn_mem_free(this);
         return -3;
     }
-    coap_service_msg_prevalidate_callback_set(this->coap_service_id, coap_msg_prevalidate_cb);
+    coap_service_msg_prevalidate_callback_set(THREAD_MANAGEMENT_PORT, coap_msg_prevalidate_cb);
 #ifdef HAVE_THREAD_ROUTER
     if (thread_leader_service_init(interface_id, this->coap_service_id) != 0) {
         tr_error("Thread leader service init failed");
@@ -1587,10 +1607,15 @@ int thread_management_server_commisoner_data_get(int8_t interface_id, thread_man
 bool thread_management_server_source_address_check(int8_t interface_id, uint8_t source_address[16])
 {
     link_configuration_s *linkConfiguration;
-    linkConfiguration = thread_joiner_application_get_config(interface_id);
 
+    if (memcmp(ADDR_LINK_LOCAL_PREFIX, source_address, 8) == 0) {
+        // Source address is from Link local address
+        return true;
+    }
+
+    linkConfiguration = thread_joiner_application_get_config(interface_id);
     if (!linkConfiguration) {
-        tr_error("No link configuration.");
+        tr_error("No link cfg for if %d", interface_id);
         return false;
     }
 
@@ -1599,15 +1624,12 @@ bool thread_management_server_source_address_check(int8_t interface_id, uint8_t
         // Source address is RLOC or ALOC
     } else if (memcmp(source_address, linkConfiguration->mesh_local_ula_prefix, 8) == 0) {
         // Source address is ML64 TODO this should check that destination address is ALOC or RLOC CoaP Service does not support
-    } else if (memcmp(ADDR_LINK_LOCAL_PREFIX, source_address, 8)) {
-        // Source address is from Link local address
     } else {
-        tr_error("Message out of thread network; ML prefix: %s, src addr: %s",
+        tr_deep("Message out of thread network; ML prefix: %s, src addr: %s",
                  trace_ipv6_prefix(linkConfiguration->mesh_local_ula_prefix, 64),
                  trace_ipv6(source_address));
         return false;
     }
-    // TODO: Add other (security) related checks here
 
     return true;
 }

test/nanostack/unittest/stub/coap_service_api_stub.c

@@ -100,7 +100,7 @@ void coap_service_request_delete_by_service_id(int8_t service_id)
 {
 }
 
-int8_t coap_service_msg_prevalidate_callback_set(int8_t service_id, coap_service_msg_prevalidate_cb *msg_prevalidate_cb)
+int8_t coap_service_msg_prevalidate_callback_set(uint16_t listen_port, coap_service_msg_prevalidate_cb *msg_prevalidate_cb)
 {
     return 0;
 }