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_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");
   }
@@ -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_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));

port/android/Makefile

@@ -140,7 +140,7 @@ ifeq ($(PKI),1)
 	CTIMESTAMP=../../api/c-timestamp/timestamp_tm.c
 endif
 
-CTIMESTAMP+=../../api/c-timestamp/timestamp_format.c ../../api/c-timestamp/timestamp_valid.c ../../api/c-timestamp/timestamp_parse.c
+CTIMESTAMP+=../../api/c-timestamp/timestamp_compare.c ../../api/c-timestamp/timestamp_format.c ../../api/c-timestamp/timestamp_valid.c ../../api/c-timestamp/timestamp_parse.c
 
 SRC_PORT_COMMON:=$(wildcard ../../port/common/*.c)
 ifneq ($(MEMORY_TRACE),1)
@@ -255,7 +255,7 @@ ifeq ($(PLGD_DEV_TIME),1)
 endif
 
 ifneq ($(SECURE),0)
-	SRC += $(addprefix ../../security/, oc_acl.c oc_ael.c oc_audit.c oc_certs.c oc_certs_validate.c oc_cred.c oc_csr.c oc_doxm.c oc_entropy.c \
+	SRC += $(addprefix ../../security/, oc_acl.c oc_ael.c oc_audit.c oc_certs.c oc_certs_generate.c oc_certs_validate.c oc_cred.c oc_csr.c oc_doxm.c oc_entropy.c \
 			oc_keypair.c oc_pki.c oc_pstat.c oc_roles.c oc_sdi.c oc_security.c oc_sp.c oc_store.c oc_svr.c oc_tls.c)
 	SRC_COMMON += $(addprefix $(MBEDTLS_DIR)/library/,${DTLS})
 	MBEDTLS_PATCH_FILE := $(MBEDTLS_DIR)/patched.txt

port/arduino/adapter/Makefile

@@ -39,7 +39,7 @@ ifeq ($(DYNAMIC),1)
 endif
 
 ifeq ($(SECURE),1)
-	SEC_SRC += $(addprefix $(ROOT_DIR)/security/, oc_acl.c oc_cred.c oc_certs.c oc_certs_validate.c oc_csr.c oc_doxm.c oc_entropy.c \
+	SEC_SRC += $(addprefix $(ROOT_DIR)/security/, oc_acl.c oc_cred.c oc_certs.c oc_certs_generate.c oc_certs_validate.c oc_csr.c oc_doxm.c oc_entropy.c \
 	              oc_keypair.c oc_pki.c oc_pstat.c oc_roles.c oc_security.c oc_sp.c oc_store.c oc_svr.c oc_tls.c)
 	SRC += $(SEC_SRC)
 	SRC_COMMON += $(addprefix $(MBEDTLS_DIR)/library/,${DTLS})

port/esp32/main/CMakeLists.txt

@@ -157,6 +157,7 @@ if(CONFIG_SECURE)
 		${CMAKE_CURRENT_SOURCE_DIR}/../../../security/oc_ael.c
 		${CMAKE_CURRENT_SOURCE_DIR}/../../../security/oc_audit.c
 		${CMAKE_CURRENT_SOURCE_DIR}/../../../security/oc_certs.c
+		${CMAKE_CURRENT_SOURCE_DIR}/../../../security/oc_certs_generate.c
 		${CMAKE_CURRENT_SOURCE_DIR}/../../../security/oc_certs_validate.c
 		${CMAKE_CURRENT_SOURCE_DIR}/../../../security/oc_cred.c
 		${CMAKE_CURRENT_SOURCE_DIR}/../../../security/oc_csr.c

port/linux/Makefile

@@ -112,7 +112,7 @@ ifeq ($(PKI),1)
 	CTIMESTAMP=../../api/c-timestamp/timestamp_tm.c
 endif
 
-CTIMESTAMP+=../../api/c-timestamp/timestamp_format.c ../../api/c-timestamp/timestamp_valid.c ../../api/c-timestamp/timestamp_parse.c
+CTIMESTAMP+=../../api/c-timestamp/timestamp_compare.c ../../api/c-timestamp/timestamp_format.c ../../api/c-timestamp/timestamp_valid.c ../../api/c-timestamp/timestamp_parse.c
 
 SRC_PORT_COMMON:=$(wildcard ../../port/common/*.c)
 ifneq ($(MEMORY_TRACE),1)
@@ -254,7 +254,7 @@ endif
 
 
 ifneq ($(SECURE),0)
-	SRC += $(addprefix ../../security/,	oc_acl.c oc_ael.c oc_audit.c oc_certs.c oc_certs_validate.c oc_cred.c oc_csr.c oc_doxm.c oc_entropy.c \
+	SRC += $(addprefix ../../security/,	oc_acl.c oc_ael.c oc_audit.c oc_certs.c oc_certs_generate.c oc_certs_validate.c oc_cred.c oc_csr.c oc_doxm.c oc_entropy.c \
 			oc_keypair.c oc_oscore_engine.c oc_oscore_crypto.c oc_oscore_context.c oc_pki.c oc_pstat.c oc_roles.c oc_sdi.c \
 			oc_security.c oc_sp.c oc_store.c oc_svr.c oc_tls.c)
 	SRC_COMMON += $(addprefix $(MBEDTLS_DIR)/library/,${DTLS})

port/windows/vs2015/IoTivity-lite.vcxproj

@@ -427,6 +427,8 @@
     <ClCompile Include="..\..\..\security\oc_ael.c" />
     <ClCompile Include="..\..\..\security\oc_audit.c" />
     <ClCompile Include="..\..\..\security\oc_certs.c" />
+    <ClCompile Include="..\..\..\security\oc_certs_generate.c" />
+    <ClCompile Include="..\..\..\security\oc_certs_validate.c" />
     <ClCompile Include="..\..\..\security\oc_cred.c" />
     <ClCompile Include="..\..\..\security\oc_csr.c" />
     <ClCompile Include="..\..\..\security\oc_doxm.c" />

port/windows/vs2015/IoTivity-lite.vcxproj.filters

@@ -299,6 +299,12 @@
     <ClCompile Include="..\..\..\security\oc_certs.c">
       <Filter>Security</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\..\security\oc_certs_generate.c">
+      <Filter>Security</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\..\security\oc_certs_validate.c">
+      <Filter>Security</Filter>
+    </ClCompile>
     <ClCompile Include="..\..\..\security\oc_keypair.c">
       <Filter>Security</Filter>
     </ClCompile>

security/oc_acl.c

@@ -231,10 +231,9 @@ oc_ace_get_permission(const oc_sec_ace_t *ace, const oc_resource_t *resource,
   oc_ace_wildcard_t wc = 0;
   if (!is_DCR) {
     if (resource->properties & OC_DISCOVERABLE) {
+      wc = OC_ACE_WC_ALL_SECURED;
       if (is_public) {
-        wc = OC_ACE_WC_ALL_PUBLIC | OC_ACE_WC_ALL_SECURED;
-      } else {
-        wc = OC_ACE_WC_ALL_SECURED;
+        wc |= OC_ACE_WC_ALL_PUBLIC;
       }
     } else {
       wc = OC_ACE_WC_ALL;
@@ -556,8 +555,8 @@ oc_sec_check_acl(oc_method_t method, const oc_resource_t *resource,
     }
     if ((pstat->s == OC_DOS_RFPRO || pstat->s == OC_DOS_RFNOP ||
          pstat->s == OC_DOS_SRESET) &&
-        oc_string_len(resource->uri) == 14 &&
-        memcmp(oc_string(resource->uri), "/oic/sec/roles", 14) == 0) {
+        oc_string_is_cstr_equal(&resource->uri, OCF_SEC_ROLES_URI,
+                                OC_CHAR_ARRAY_LEN(OCF_SEC_ROLES_URI))) {
       OC_DBG("oc_acl: peer has implicit access to /oic/sec/roles in RFPRO, "
              "RFNOP, SRESET");
       return true;
@@ -600,7 +599,8 @@ oc_sec_check_acl(oc_method_t method, const oc_resource_t *resource,
     }
 #ifdef OC_PKI
     else {
-      const oc_sec_cred_t *role_cred = peer ? oc_sec_get_roles(peer) : NULL;
+      const oc_sec_cred_t *role_cred =
+        peer != NULL ? oc_sec_roles_get(peer) : NULL;
       while (role_cred) {
         const oc_sec_cred_t *next = role_cred->next;
         uint32_t flags = 0;

security/oc_certs.c

@@ -137,42 +137,6 @@ oc_sec_certs_ecp_group_id_is_allowed(mbedtls_ecp_group_id gid)
          (MBEDTLS_X509_ID_FLAG(gid) & g_allowed_ecp_grpids_mask) != 0;
 }
 
-int
-oc_certs_generate_serial_number(mbedtls_x509write_cert *crt, size_t size)
-{
-  mbedtls_ctr_drbg_context ctr_drbg;
-  mbedtls_ctr_drbg_init(&ctr_drbg);
-
-  mbedtls_entropy_context entropy;
-  mbedtls_entropy_init(&entropy);
-  oc_entropy_add_source(&entropy);
-
-#define PERSONALIZATION_DATA "IoTivity-Lite-Certificate_Serial_Number"
-
-  int ret = mbedtls_ctr_drbg_seed(&ctr_drbg, mbedtls_entropy_func, &entropy,
-                                  (const unsigned char *)PERSONALIZATION_DATA,
-                                  sizeof(PERSONALIZATION_DATA));
-
-#undef PERSONALIZATION_DATA
-
-  if (ret < 0) {
-    OC_ERR("error initializing RNG %d", ret);
-    return -1;
-  }
-
-  mbedtls_ctr_drbg_set_prediction_resistance(&ctr_drbg, MBEDTLS_CTR_DRBG_PR_ON);
-
-  ret = mbedtls_mpi_fill_random(&crt->serial, size, mbedtls_ctr_drbg_random,
-                                &ctr_drbg);
-
-  if (ret < 0) {
-    OC_ERR("error generating random serial number for certificate %d", ret);
-    return -1;
-  }
-
-  return 0;
-}
-
 bool
 oc_certs_is_PEM(const unsigned char *cert, size_t cert_len)
 {
@@ -485,30 +449,6 @@ oc_certs_parse_CN_for_UUID(const unsigned char *cert, size_t cert_size,
   return ok;
 }
 
-bool
-oc_certs_encode_role(const oc_role_t *role, char *buf, size_t buf_len)
-{
-  char *buffer = buf;
-  size_t length = buf_len;
-  int ret = snprintf(buffer, length, "CN=%s", oc_string(role->role));
-  if (ret < 0 || (size_t)ret >= length) {
-    OC_ERR("could not encode role");
-    return false;
-  }
-  if (oc_string_len(role->authority) == 0) {
-    return true;
-  }
-
-  buffer = buf + ret;
-  length -= ret;
-  ret = snprintf(buffer, length, ",OU=%s", oc_string(role->authority));
-  if (ret < 0 || (size_t)ret >= length) {
-    OC_ERR("could not encode authority");
-    return false;
-  }
-  return true;
-}
-
 static bool
 oc_certs_DN_is_CN(const mbedtls_x509_name *dn)
 {
@@ -624,28 +564,6 @@ oc_certs_timestamp_now(void)
   return ts;
 }
 
-bool
-oc_certs_timestamp_format(timestamp_t ts, char *buffer, size_t buffer_size)
-{
-  assert(buffer != NULL);
-
-  struct tm now_tm;
-  memset(&now_tm, 0, sizeof(struct tm));
-  if (timestamp_to_tm_utc(&ts, &now_tm) == NULL) {
-    OC_ERR("cannot convert timestamp to string: invalid timestamp");
-    return false;
-  }
-
-  int ret = snprintf(buffer, buffer_size, "%d%02d%02d%02d%02d%02d",
-                     now_tm.tm_year + 1900, now_tm.tm_mon + 1, now_tm.tm_mday,
-                     now_tm.tm_hour, now_tm.tm_min, now_tm.tm_sec);
-  if (ret < 0 || (size_t)ret >= buffer_size) {
-    OC_ERR("cannot convert timestamp to string: buffer too small");
-    return false;
-  }
-  return true;
-}
-
 uint64_t
 oc_certs_time_to_unix_timestamp(mbedtls_x509_time time)
 {