Refactor oc_cred.c and oc_roles.c

- refactor certificate generation and other parts of code with
issues reported by SonarCloud
- add unit tests

.github/workflows/sonar-cloud-analysis.yml

@@ -28,7 +28,7 @@ jobs:
           # cloud (ipv4+tcp) on, collection create on, push on, rfotm on
           - build_args: "-DOC_CLOUD_ENABLED=ON -DOC_COLLECTIONS_IF_CREATE_ENABLED=ON -DOC_PUSH_ENABLED=ON -DOC_DISCOVERY_RESOURCE_OBSERVABLE_ENABLED=ON -DOC_RESOURCE_ACCESS_IN_RFOTM_ENABLED=ON"
           # ipv6 dns on, oscore off, rep realloc on
-          - build_args: "-DOC_DNS_LOOKUP_IPV6_ENABLED=ON -DOC_OSCORE_ENABLED=OFF -DOC_REP_ENCODING_REALLOC=ON"
+          - build_args: "-DOC_DNS_LOOKUP_IPV6_ENABLED=ON -DOC_OSCORE_ENABLED=OFF -DOC_REPRESENTATION_REALLOC_ENCODING_ENABLED=ON"
           # ipv4 on, tcp on, dynamic allocation off, rfotm on
           - build_args: "-DOC_IPV4_ENABLED=ON -DOC_TCP_ENABLED=ON -DOC_DYNAMIC_ALLOCATION_ENABLED=OFF -DOC_RESOURCE_ACCESS_IN_RFOTM_ENABLED=ON"
           # security off, cloud (ipv4+tcp), collection create on, push on, introspection IDD off

api/client/unittest/clientcbtest.cpp

@@ -57,7 +57,7 @@ class TestClientCB : public testing::Test {
   static oc_discovery_flags_t discoveryHandler(const char *, const char *,
                                                oc_string_array_t,
                                                oc_interface_mask_t,
-                                               oc_endpoint_t *,
+                                               const oc_endpoint_t *,
                                                oc_resource_properties_t, void *)
   {
     TestClientCB::discoveryHandlerInvoked = true;
@@ -66,7 +66,7 @@ class TestClientCB : public testing::Test {
 
   static oc_discovery_flags_t discoveryAllHandler(
     const char *, const char *, oc_string_array_t, oc_interface_mask_t,
-    oc_endpoint_t *, oc_resource_properties_t, bool, void *)
+    const oc_endpoint_t *, oc_resource_properties_t, bool, void *)
   {
     TestClientCB::discoveryAllHandlerInvoked = true;
     return OC_STOP_DISCOVERY;

api/oc_base64.c

@@ -128,7 +128,8 @@ oc_base64_decode(uint8_t *str, size_t len)
   /* The Base64 input string is decoded in-place. */
   size_t i = 0;
   int j = 0;
-  unsigned char val_c = 0, val_s = 0;
+  unsigned char val_c = 0;
+  unsigned char val_s = 0;
 
   /* Process every input character */
   for (i = 0; i < len; i++) {

api/oc_client_api.c

@@ -167,7 +167,7 @@ prepare_coap_request(oc_client_cb_t *cb)
   if (cb->method == OC_PUT || cb->method == OC_POST) {
     g_request_buffer = oc_blockwise_alloc_request_buffer(
       oc_string(cb->uri) + 1, oc_string_len(cb->uri) - 1, &cb->endpoint,
-      cb->method, OC_BLOCKWISE_CLIENT, OC_MIN_APP_DATA_SIZE);
+      cb->method, OC_BLOCKWISE_CLIENT, (uint32_t)OC_MIN_APP_DATA_SIZE);
     if (!g_request_buffer) {
       OC_ERR("g_request_buffer is NULL");
       return false;

api/oc_client_role.c

@@ -27,7 +27,7 @@
 oc_role_t *
 oc_get_all_roles(void)
 {
-  return oc_sec_get_role_creds();
+  return oc_sec_role_creds_get();
 }
 
 static void
@@ -74,7 +74,7 @@ oc_assert_role(const char *role, const char *authority,
     return false;
   }
   oc_tls_select_cert_ciphersuite();
-  if (!oc_init_post("/oic/sec/roles", endpoint, NULL, handler, HIGH_QOS,
+  if (!oc_init_post(OCF_SEC_ROLES_URI, endpoint, NULL, handler, HIGH_QOS,
                     user_data)) {
     OC_ERR("cannot init POST");
   }
@@ -94,7 +94,7 @@ void
 oc_assert_all_roles(const oc_endpoint_t *endpoint,
                     oc_response_handler_t handler, void *user_data)
 {
-  oc_tls_peer_t *peer = oc_tls_get_peer(endpoint);
+  const oc_tls_peer_t *peer = oc_tls_get_peer(endpoint);
   if (oc_tls_uses_psk_cred(peer)) {
     return;
   }
@@ -103,7 +103,7 @@ oc_assert_all_roles(const oc_endpoint_t *endpoint,
   if (roles == NULL) {
     return;
   }
-  if (!oc_init_post("/oic/sec/roles", endpoint, NULL, handler, HIGH_QOS,
+  if (!oc_init_post(OCF_SEC_ROLES_URI, endpoint, NULL, handler, HIGH_QOS,
                     user_data)) {
     OC_ERR("cannot init POST");
   }

api/oc_collection.c

@@ -832,7 +832,8 @@ oc_handle_collection_batch_request(oc_method_t method, oc_request_t *request,
   assert(request != NULL);
   int ecode = oc_status_code(OC_STATUS_OK);
   int pcode = oc_status_code(OC_STATUS_BAD_REQUEST);
-  CborEncoder encoder, prev_link;
+  CborEncoder encoder;
+  CborEncoder prev_link;
   oc_request_t rest_request = { 0 };
   oc_response_t response = { 0 };
   oc_response_buffer_t response_buffer;

api/oc_core_res.c

@@ -16,10 +16,11 @@
  *
  ****************************************************************************/
 
-#include "oc_core_res.h"
 #include "messaging/coap/oc_coap.h"
 #include "oc_api.h"
+#include "oc_core_res.h"
 #include "oc_core_res_internal.h"
+#include "oc_csr.h"
 #include "oc_discovery.h"
 #include "oc_endpoint.h"
 #include "oc_introspection_internal.h"
@@ -28,6 +29,7 @@
 #include "oc_ri_internal.h"
 #include "oc_main.h"
 #include "oc_server_api_internal.h"
+#include "oc_swupdate_internal.h"
 #include "port/oc_assert.h"
 #include "util/oc_atomic.h"
 #include "util/oc_compiler.h"
@@ -45,6 +47,7 @@
 #ifdef OC_SECURITY
 #include "security/oc_doxm_internal.h"
 #include "security/oc_pstat.h"
+#include "security/oc_roles_internal.h"
 #include "security/oc_sdi_internal.h"
 #include "security/oc_sp_internal.h"
 #include "security/oc_tls_internal.h"
@@ -205,8 +208,9 @@ oc_core_device_handler(oc_request_t *request, oc_interface_mask_t iface_mask,
   size_t device = request->resource->device;
   oc_rep_start_root_object();
 
-  char di[OC_UUID_LEN], piid[OC_UUID_LEN];
+  char di[OC_UUID_LEN];
   oc_uuid_to_str(&g_oc_device_info[device].di, di, OC_UUID_LEN);
+  char piid[OC_UUID_LEN];
   if (request->origin && request->origin->version != OIC_VER_1_1_0) {
     oc_uuid_to_str(&g_oc_device_info[device].piid, piid, OC_UUID_LEN);
   }
@@ -873,12 +877,12 @@ oc_core_get_resource_type_by_uri(const char *uri)
     return OCF_SEC_SP;
   }
 #ifdef OC_PKI
-  if (core_is_resource_uri(uri, uri_len, "/oic/sec/csr",
-                           OC_CHAR_ARRAY_LEN("/oic/sec/csr"))) {
+  if (core_is_resource_uri(uri, uri_len, OCF_SEC_CSR_URI,
+                           OC_CHAR_ARRAY_LEN(OCF_SEC_CSR_URI))) {
     return OCF_SEC_CSR;
   }
-  if (core_is_resource_uri(uri, uri_len, "/oic/sec/roles",
-                           OC_CHAR_ARRAY_LEN("/oic/sec/roles"))) {
+  if (core_is_resource_uri(uri, uri_len, OCF_SEC_ROLES_URI,
+                           OC_CHAR_ARRAY_LEN(OCF_SEC_ROLES_URI))) {
     return OCF_SEC_ROLES;
   }
 #endif /* OC_PKI */
@@ -888,8 +892,8 @@ oc_core_get_resource_type_by_uri(const char *uri)
   }
 #endif /* OC_SECURITY */
 #ifdef OC_SOFTWARE_UPDATE
-  if (core_is_resource_uri(uri, uri_len, "/oc/swu",
-                           OC_CHAR_ARRAY_LEN("/oc/swu"))) {
+  if (core_is_resource_uri(uri, uri_len, OCF_SW_UPDATE_URI,
+                           OC_CHAR_ARRAY_LEN(OCF_SW_UPDATE_URI))) {
     return OCF_SW_UPDATE;
   }
 #endif /* OC_SOFTWARE_UPDATE */

api/oc_discovery.c

@@ -1052,12 +1052,12 @@ oc_discovery_process_payload(const uint8_t *payload, size_t len,
   OC_MEMB_LOCAL(rep_objects, oc_rep_t, OC_MAX_NUM_REP_OBJECTS);
   oc_rep_set_pool(&rep_objects);
 
-  oc_rep_t *links = 0, *rep, *p;
+  oc_rep_t *p = NULL;
   int s = oc_parse_rep(payload, len, &p);
   if (s != 0) {
     OC_WRN("error parsing discovery response");
   }
-  links = rep = p;
+  oc_rep_t *rep = p;
   /*  While the oic.wk.res schema over the baseline interface provides for an
    *  array of objects, only one object is present and used in practice.
    *
@@ -1070,6 +1070,7 @@ oc_discovery_process_payload(const uint8_t *payload, size_t len,
     rep = rep->value.object;
   }
 
+  oc_rep_t *links = p;
   while (rep != NULL) {
     switch (rep->type) {
     /*  Ignore other oic.wk.res properties over here as they're known

api/oc_main.c

@@ -48,6 +48,7 @@
 #include "security/oc_tls_internal.h"
 #ifdef OC_PKI
 #include "security/oc_keypair_internal.h"
+#include "security/oc_roles_internal.h"
 #endif /* OC_PKI */
 #include "security/oc_sdi_internal.h"
 #endif /* OC_SECURITY */
@@ -445,6 +446,9 @@ oc_main_shutdown(void)
 
   oc_sec_svr_free();
 #ifdef OC_PKI
+#ifdef OC_CLIENT
+  oc_sec_role_creds_free();
+#endif /* OC_CLIENT */
   oc_sec_ecdsa_free_keypairs();
 #endif /* OC_PKI */
 #endif /* OC_SECURITY */

api/oc_resource_factory.c

@@ -145,7 +145,7 @@ get_collection_instance_uri(oc_collection_t *collection, char *uri,
   if (index == 0) {
     return false;
   }
-  unsigned len = oc_string_len(collection->res.uri) + 1;
+  size_t len = oc_string_len(collection->res.uri) + 1;
   if (len > uri_size) {
     // uri too long for output buffer
     return false;
@@ -157,14 +157,14 @@ get_collection_instance_uri(oc_collection_t *collection, char *uri,
   uri[oc_string_len(collection->res.uri)] = '/';
 
   int written = snprintf(NULL, 0, "%d", index);
-  if ((written <= 0) || (len + (unsigned)written + 1 > uri_size)) {
+  if ((written <= 0) || (len + (size_t)written + 1 > uri_size)) {
     // cannot fit the index converted to string into uri
     return false;
   }
 
   written = snprintf(uri + len, uri_size - len, "%d", index);
   // check for truncation by snprintf
-  return (written > 0) && ((unsigned)written <= uri_size - len);
+  return (written > 0) && ((size_t)written <= uri_size - len);
 }
 
 oc_rt_created_t *

api/oc_ri.c

@@ -848,9 +848,9 @@ oc_ri_audit_log(oc_method_t method, const oc_resource_t *resource,
   snprintf(aux[idx++], LINE_WIDTH, "device is in %s", state_str_val[state]);
   snprintf(aux[idx++], LINE_WIDTH, "No roles asserted");
 #ifdef OC_PKI
-  if (peer) {
+  if (peer != NULL) {
     size_t pos = 0;
-    for (oc_sec_cred_t *rc = oc_sec_get_roles(peer); rc && pos < LINE_WIDTH;
+    for (oc_sec_cred_t *rc = oc_sec_roles_get(peer); rc && pos < LINE_WIDTH;
          rc = rc->next) {
       pos += snprintf(aux[idx - 1] + pos, LINE_WIDTH - pos - 1, "%s ",
                       oc_string(rc->role.role));
@@ -1194,7 +1194,7 @@ ri_invoke_coap_entity_set_response(coap_packet_t *response,
 
   if (response_buffer->code ==
       oc_status_code(OC_STATUS_REQUEST_ENTITY_TOO_LARGE)) {
-    coap_set_header_size1(response, OC_BLOCK_SIZE);
+    coap_set_header_size1(response, (uint32_t)OC_BLOCK_SIZE);
   }
 
   /* response_buffer->code at this point contains a valid CoAP status
@@ -1393,7 +1393,7 @@ oc_ri_invoke_coap_entity_handler(const coap_packet_t *request,
       OC_DBG("creating new block-wise response state");
       *ctx.response_state = oc_blockwise_alloc_response_buffer(
         uri_path, uri_path_len, endpoint, method, OC_BLOCKWISE_SERVER,
-        OC_MIN_APP_DATA_SIZE);
+        (uint32_t)OC_MIN_APP_DATA_SIZE);
       if (*ctx.response_state == NULL) {
         OC_ERR("failure to alloc response state");
         bad_request = true;

api/oc_server_api.c

@@ -844,7 +844,8 @@ oc_notify_observers_delayed_ticks(oc_resource_t *resource,
     return;
   }
   oc_remove_delayed_callback(resource, &oc_observe_notification_delayed);
-  oc_set_delayed_callback(resource, &oc_observe_notification_delayed, ticks);
+  oc_ri_add_timed_event_callback_ticks(resource,
+                                       &oc_observe_notification_delayed, ticks);
 }
 
 void

api/oc_uuid.c

@@ -82,7 +82,7 @@ oc_str_to_uuid(const char *str, oc_uuid_t *uuid)
       uuid->id[j++] = c;
       c = 0;
     } else {
-      c = c << 4;
+      c = (uint8_t)(c << 4);
     }
     k++;
   }

api/unittest/reptest.cpp

@@ -235,9 +235,9 @@ TEST_F(TestRepWithPool, OCRepSetGetInt)
   EXPECT_FALSE(oc_rep_get_int(rep.get(), "zero", nullptr));
   EXPECT_FALSE(oc_rep_get_int(rep.get(), "not_a_key", &zero_out));
 
-  CheckJson(rep.get(),
-            "{\"ultimate_answer\":10000000000,\"negative\":-1024,\"zero\":0}",
-            false);
+  std::string json =
+    R"({"ultimate_answer":10000000000,"negative":-1024,"zero":0})";
+  CheckJson(rep.get(), json, false);
   std::string pretty_json = "{\n"
                             "  \"ultimate_answer\" : 10000000000,\n"
                             "  \"negative\" : -1024,\n"
@@ -301,7 +301,7 @@ TEST_F(TestRepWithPool, OCRepSetGetUint)
   EXPECT_FALSE(oc_rep_get_int(rep.get(), "not_a_key", &zero_out));
 
   std::string json =
-    "{\"ultimate_answer\":42,\"larger_than_int\":3000000000,\"zero\":0}";
+    R"({"ultimate_answer":42,"larger_than_int":3000000000,"zero":0})";
   CheckJson(rep.get(), json, false);
   std::string pretty_json = "{\n"
                             "  \"ultimate_answer\" : 42,\n"
@@ -341,7 +341,7 @@ TEST_F(TestRepWithPool, OCRepSetGetBool)
   EXPECT_FALSE(oc_rep_get_bool(rep.get(), "true_flag", nullptr));
   EXPECT_FALSE(oc_rep_get_bool(rep.get(), "not_a_key", &true_flag_out));
 
-  std::string json = "{\"true_flag\":true,\"false_flag\":false}";
+  std::string json = R"({"true_flag":true,"false_flag":false})";
   CheckJson(rep.get(), json, false);
   std::string pretty_json = "{\n"
                             "  \"true_flag\" : true,\n"
@@ -407,9 +407,8 @@ TEST_F(TestRepWithPool, OCRepSetGetTextString)
   EXPECT_FALSE(
     oc_rep_get_string(rep.get(), "not_a_key", &hal9000_out, &str_len));
 
-  std::string json = "{\"empty\":\"\","
-                     "\"hal9000\":\"Dave\","
-                     "\"ru_character_set\":\"Привет, мир\"}";
+  std::string json =
+    R"({"empty":"","hal9000":"Dave","ru_character_set":"Привет, мир"})";
   CheckJson(rep.get(), json, false);
   std::string pretty_json = "{\n"
                             "  \"empty\" : \"\",\n"
@@ -468,8 +467,8 @@ TEST_F(TestRepWithPool, OCRepSetGetByteString)
   EXPECT_FALSE(oc_rep_get_byte_string(rep.get(), "not_a_key",
                                       &test_byte_string_out, &str_len));
 
-  std::string json = "{\"empty_byte_string\":\"\","
-                     "\"test_byte_string\":\"AQIDBAIA\"}";
+  std::string json =
+    R"({"empty_byte_string":"","test_byte_string":"AQIDBAIA"})";
   CheckJson(rep.get(), json, false);
   std::string pretty_json = "{\n"
                             "  \"empty_byte_string\" : \"\",\n"
@@ -484,7 +483,7 @@ TEST_F(TestRepWithPool, OCRepSetGetByteString)
   // Decoding of byte string is an all or nothing action. Since there
   // is not enough room in the too_small output buffer nothing is placed in the
   // buffer and remaining space is left empty.
-  std::string too_small_json = "{\"empty_byte_string\":\"\",";
+  std::string too_small_json = R"({"empty_byte_string":"",)";
   EXPECT_STREQ(too_small_json.c_str(), too_small.data());
 }
 
@@ -932,7 +931,7 @@ TEST_F(TestRepWithPool, OCRepSetGetObject)
   EXPECT_EQ(5, c_out_size);
   EXPECT_STREQ("three", c_out);
 
-  std::string json = "{\"my_object\":{\"a\":1,\"b\":false,\"c\":\"three\"}}";
+  std::string json = R"({"my_object":{"a":1,"b":false,"c":"three"}})";
   CheckJson(rep.get(), json, false);
   std::string pretty_json = "{\n"
                             "  \"my_object\" : {\n"
@@ -1120,9 +1119,9 @@ TEST_F(TestRepWithPool, OCRepSetGetObjectArray)
   EXPECT_STREQ("AI computer", job_out);
 
   std::string json =
-    "{\"space_2001\":[{\"name\":\"Dave Bowman\","
-    "\"job\":\"astronaut\"},{\"name\":\"Frank Poole\",\"job\":\"astronaut\"}"
-    ",{\"name\":\"Hal 9000\",\"job\":\"AI computer\"}]}";
+    R"({"space_2001":[{"name":"Dave Bowman","job":"astronaut"},)"
+    R"({"name":"Frank Poole","job":"astronaut"},)"
+    R"({"name":"Hal 9000","job":"AI computer"}]})";
   CheckJson(rep.get(), json, false);
   std::string pretty_json = "{\n"
                             "  \"space_2001\" : [\n"
@@ -1209,8 +1208,8 @@ TEST_F(TestRepWithPool, OCRepAddGetByteStringArray)
             0);
 
   std::string json =
-    "{\"barray\":[\"AQECAwQFBg==\","
-    "\"AQECAwUIEyE0VYk=\",\"QkJCQkJCQkJCQkJCQkJCQkJCQkI=\",\"AAD/AAA=\"]}";
+    R"({"barray":["AQECAwQFBg==",)"
+    R"("AQECAwUIEyE0VYk=","QkJCQkJCQkJCQkJCQkJCQkJCQkI=","AAD/AAA="]})";
   CheckJson(rep.get(), json, false);
   std::string pretty_json = "{\n"
                             "  \"barray\" : [\n"
@@ -1314,11 +1313,11 @@ TEST_F(TestRepWithPool, OCRepSetGetStringArray)
   EXPECT_STREQ(STR3.c_str(), oc_string_array_get_item(quotes_out, 3));
 
   // clang-format off
-  std::string json = "{\"quotes\":"
-                     "[\"" + STR0 + "\","
-                     "\"" + STR1 + "\","
-                     "\"" + STR2 + "\","
-                     "\"" + STR3 + "\"]}";
+  std::string json = R"({"quotes":)"
+                     R"([")" + STR0 + R"(",)"
+                     R"(")" + STR1 + R"(",)"
+                     R"(")" + STR2 + R"(",)"
+                     R"(")" + STR3 + R"("]})";
   // clang-format on
   CheckJson(rep.get(), json, false);
   // clang-format off
@@ -1501,8 +1500,8 @@ TEST_F(TestRepWithPool, OCRepRootArrayObject)
   EXPECT_TRUE(oc_rep_get_int(rep_out, "count", &count_out));
   EXPECT_EQ(100, count_out);
 
-  std::string json = "[{\"href\":\"/light/1\",\"rep\":{\"state\":true}},"
-                     "{\"href\":\"/count/1\",\"rep\":{\"count\":100}}]";
+  std::string json = R"([{"href":"/light/1","rep":{"state":true}},)"
+                     R"({"href":"/count/1","rep":{"count":100}}])";
   CheckJson(rep.get(), json, false);
   std::string pretty_json = "[\n"
                             "  {\n"