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_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_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"
@@ -873,12 +876,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 +891,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_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"

api/unittest/resourcetest.cpp

@@ -40,7 +40,6 @@ class TestResourceWithDevice : public testing::Test {
 #ifdef OC_HAS_FEATURE_RESOURCE_ACCESS_IN_RFOTM
     oc_resource_t *con = oc_core_get_resource_by_index(OCF_CON, /*device*/ 0);
     ASSERT_NE(nullptr, con);
-    // oc_resource_make_public(con);
     oc_resource_set_access_in_RFOTM(con, true, OC_PERM_RETRIEVE);
 #endif /* OC_HAS_FEATURE_RESOURCE_ACCESS_IN_RFOTM */
   }
@@ -135,7 +134,7 @@ TEST_F(TestResourceWithDevice, BaselineInterfaceProperties)
   auto get_handler = [](oc_client_response_t *data) {
     ASSERT_EQ(OC_STATUS_OK, data->code);
     oc::TestDevice::Terminate();
-    bool *invoked = static_cast<bool *>(data->user_data);
+    auto *invoked = static_cast<bool *>(data->user_data);
     *invoked = true;
 
     oc_rep_t *rep = data->payload;

include/oc_helpers.h

@@ -177,9 +177,9 @@ void oc_concat_strings(oc_string_t *concat, const char *str1, const char *str2);
 #define oc_double_array(ocdoublearray) (oc_cast(ocdoublearray, double))
 
 #ifdef OC_DYNAMIC_ALLOCATION
-#define STRING_ARRAY_ITEM_MAX_LEN 128
+#define STRING_ARRAY_ITEM_MAX_LEN (128)
 #else /* OC_DYNAMIC_ALLOCATION */
-#define STRING_ARRAY_ITEM_MAX_LEN 32
+#define STRING_ARRAY_ITEM_MAX_LEN (32)
 #endif /* !OC_DYNAMIC_ALLOCATION */
 
 bool _oc_copy_string_to_array(oc_string_array_t *ocstringarray,