Refactor oc_sec_cred_t

- fix issues reported by SonarCloud - add oc_cred_serialize function to serialize selected certificates to PEM string
iotivity · Jan 11, 2024 · 8f16eca · 8f16eca
1 parent 8ff30d1
commit 8f16eca
Show file tree

Hide file tree

Showing 43 changed files with 2,053 additions and 741 deletions.
diff --git a/api/oc_client_role.c b/api/oc_client_role.c
@@ -22,7 +22,9 @@
 
 #include "oc_role.h"
 #include "port/oc_log_internal.h"
+#include "security/oc_cred_util_internal.h"
 #include "security/oc_roles_internal.h"
+#include "util/oc_secure_string_internal.h"
 
 oc_role_t *
 oc_get_all_roles(void)
@@ -65,10 +67,23 @@ oc_assert_role(const char *role, const char *authority,
   if (oc_tls_uses_psk_cred(oc_tls_get_peer(endpoint))) {
     return false;
   }
-  const oc_sec_cred_t *cr =
-    oc_sec_find_role_cred(/*start*/ NULL, role, authority,
-                          /*tag*/ NULL); // ignore tag, we want to serialize
-                                         // only the [role,authority] pairs
+
+  oc_string_view_t role_view =
+    oc_string_view(role, oc_strnlen_s(role, OC_MAX_STRING_LENGTH));
+  if (role_view.data == NULL || role_view.length == OC_MAX_STRING_LENGTH) {
+    OC_ERR("invalid role");
+    return false;
+  }
+  oc_string_view_t authority_view =
+    oc_string_view(authority, oc_strnlen_s(authority, OC_MAX_STRING_LENGTH));
+  if (authority_view.length == OC_MAX_STRING_LENGTH) {
+    OC_ERR("invalid authority");
+    return false;
+  }
+  const oc_sec_cred_t *cr = oc_sec_find_role_cred(
+    /*start*/ NULL, role_view, authority_view,
+    /*tag*/ OC_STRING_VIEW_NULL); // ignore tag, we want to serialize
+                                  // only the [role,authority] pairs
   if (cr == NULL) {
     OC_ERR("no role was found");
     return false;
@@ -112,9 +127,10 @@ oc_assert_all_roles(const oc_endpoint_t *endpoint,
 
   while (roles) {
     const oc_sec_cred_t *cr = oc_sec_find_role_cred(
-      /*start*/ NULL, oc_string(roles->role), oc_string(roles->authority),
-      /*tag*/ NULL); // ignore tag, we want to serialize only the
-                     // [role,authority] pairs
+      /*start*/ NULL, oc_string_view2(&roles->role),
+      oc_string_view2(&roles->authority),
+      /*tag*/ OC_STRING_VIEW_NULL); // ignore tag, we want to serialize only the
+                                    // [role,authority] pairs
     if (cr != NULL) {
       serialize_role_credential(&roles_array, cr);
     }

diff --git a/api/oc_helpers.c b/api/oc_helpers.c
@@ -143,11 +143,10 @@ oc_set_string(oc_string_t *dst, const char *str, size_t str_len)
 oc_string_view_t
 oc_string_view(const char *data, size_t length)
 {
-  oc_string_view_t view = {
+  return (oc_string_view_t){
     .data = data,
     .length = length,
   };
-  return view;
 }
 
 oc_string_view_t

diff --git a/api/oc_rep_encode.c b/api/oc_rep_encode.c
@@ -436,7 +436,7 @@ oc_rep_encoder_write_raw(oc_rep_encoder_t *encoder, const uint8_t *data,
     memcpy(&encoder->ctx, &prevEncoder, sizeof(prevEncoder));
 #else  /* OC_DYNAMIC_ALLOCATION */
     OC_WRN("Insufficient memory: Increase OC_MAX_APP_DATA_SIZE to "
-           "accomodate a larger payload(+%zu)",
+           "accomodate a larger payload(+%lu)",
            len - remaining);
     return CborErrorOutOfMemory;
 #endif /* !OC_DYNAMIC_ALLOCATION */

diff --git a/include/oc_cred.h b/include/oc_cred.h
@@ -132,8 +132,8 @@ typedef struct oc_sec_creds_t
  * @return true if security credential matches the filter
  * @return false otherwise
  */
-typedef bool (*oc_sec_cred_filter_t)(const oc_sec_cred_t *cred,
-                                     void *user_data);
+typedef bool (*oc_sec_cred_filter_t)(const oc_sec_cred_t *cred, void *user_data)
+  OC_NONNULL(1);
 
 #ifdef OC_PKI
 

diff --git a/include/oc_role.h b/include/oc_role.h
@@ -26,6 +26,7 @@
 #include "oc_client_state.h"
 #include "oc_helpers.h"
 #include "oc_endpoint.h"
+#include "util/oc_compiler.h"
 
 #include <stdbool.h>
 
@@ -55,17 +56,18 @@ oc_role_t *oc_get_all_roles(void);
 /**
  * @brief assert the specific role
  *
- * @param role the role
+ * @param role the role (cannot be NULL)
  * @param authority the authority
- * @param endpoint endpoint identifying the connection
- * @param handler the response handler
+ * @param endpoint endpoint identifying the connection (cannot be NULL)
+ * @param handler the response handler (cannot be NULL)
  * @param user_data the user data to be conveyed to the response handler
  * @return true request was initialized and sent
  * @return false otherwise
  */
 bool oc_assert_role(const char *role, const char *authority,
                     const oc_endpoint_t *endpoint,
-                    oc_response_handler_t handler, void *user_data);
+                    oc_response_handler_t handler, void *user_data)
+  OC_NONNULL(1, 3, 4);
 
 /**
  * @brief set automatic role assertion (e.g. for all endpoints with a

diff --git a/port/android/Makefile b/port/android/Makefile
@@ -269,8 +269,9 @@ 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_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 += $(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_cred_util.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
 ifeq ($(DYNAMIC),1)

diff --git a/port/android/ipadapter.c b/port/android/ipadapter.c
@@ -17,7 +17,9 @@
  *
  ****************************************************************************/
 
+#ifndef _GNU_SOURCE
 #define _GNU_SOURCE
+#endif
 
 #include <android/api-level.h>
 #if !defined(__ANDROID_API__) || __ANDROID_API__ == 10000

diff --git a/port/android/tcpadapter.c b/port/android/tcpadapter.c
@@ -16,7 +16,9 @@
  *
  ****************************************************************************/
 
+#ifndef _GNU_SOURCE
 #define _GNU_SOURCE
+#endif
 
 #include "api/oc_message_internal.h"
 #include "api/oc_session_events_internal.h"

diff --git a/port/arduino/adapter/Makefile b/port/arduino/adapter/Makefile
@@ -39,8 +39,9 @@ ifeq ($(DYNAMIC),1)
 endif
 
 ifeq ($(SECURE),1)
-	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)
+	SEC_SRC += $(addprefix $(ROOT_DIR)/security/, oc_acl.c oc_cred.c oc_cred_util.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})
 	MBEDTLS_PATCH_FILE := $(MBEDTLS_DIR)/patched.txt

diff --git a/port/common/posix/oc_loop_event.c b/port/common/posix/oc_loop_event.c
@@ -17,7 +17,9 @@
  ****************************************************************************/
 
 // make pipe2() available
+#ifndef _GNU_SOURCE
 #define _GNU_SOURCE
+#endif
 
 #include "util/oc_features.h"
 

diff --git a/port/common/posix/oc_poll.c b/port/common/posix/oc_poll.c
@@ -17,7 +17,9 @@
  ****************************************************************************/
 
 // make ppoll() available
+#ifndef _GNU_SOURCE
 #define _GNU_SOURCE
+#endif
 
 #include "port/common/posix/oc_poll_internal.h"
 #include "oc_clock_util.h"

diff --git a/port/linux/Makefile b/port/linux/Makefile
@@ -286,9 +286,9 @@ endif
 endif
 
 ifneq ($(SECURE),0)
-	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 += $(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_cred_util.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})
 	MBEDTLS_PATCH_FILE := $(MBEDTLS_DIR)/patched.txt
 ifeq ($(DYNAMIC),1)

diff --git a/port/linux/ip.c b/port/linux/ip.c
@@ -16,7 +16,10 @@
  *
  ******************************************************************/
 
+#ifndef _GNU_SOURCE
 #define _GNU_SOURCE
+#endif
+
 #include "ip.h"
 #include "port/oc_log_internal.h"
 #include "util/oc_macros_internal.h"

diff --git a/port/linux/tcpsession.c b/port/linux/tcpsession.c
@@ -16,7 +16,9 @@
  *
  ****************************************************************************/
 
+#ifndef _GNU_SOURCE
 #define _GNU_SOURCE
+#endif
 
 #include "util/oc_features.h"
 

diff --git a/port/openthread/Makefile b/port/openthread/Makefile
@@ -57,7 +57,7 @@ ifeq ($(MEMORY_TRACE), 1)
 endif
 
 ifeq ($(SECURE),1)
-	SRC_COMMON += oc_acl.c oc_cred.c oc_doxm.c oc_pstat.c oc_dtls.c oc_svr.c oc_store.c oc_sdi.c
+	SRC_COMMON += oc_acl.c oc_cred.c oc_cred_util.c oc_doxm.c oc_pstat.c oc_dtls.c oc_svr.c oc_store.c oc_sdi.c
 	SRC_COMMON += memory_buffer_alloc.c
 	CFLAGS += -DOC_SECURITY
 endif

diff --git a/port/windows/vs2015/IoTivity-lite.vcxproj b/port/windows/vs2015/IoTivity-lite.vcxproj
@@ -461,6 +461,7 @@
     <ClCompile Include="..\..\..\security\oc_certs_generate.c" />
     <ClCompile Include="..\..\..\security\oc_certs_validate.c" />
     <ClCompile Include="..\..\..\security\oc_cred.c" />
+    <ClCompile Include="..\..\..\security\oc_cred_util.c" />
     <ClCompile Include="..\..\..\security\oc_csr.c" />
     <ClCompile Include="..\..\..\security\oc_doxm.c" />
     <ClCompile Include="..\..\..\security\oc_keypair.c" />

diff --git a/port/windows/vs2015/IoTivity-lite.vcxproj.filters b/port/windows/vs2015/IoTivity-lite.vcxproj.filters
@@ -362,6 +362,9 @@
     <ClCompile Include="..\..\..\security\oc_cred.c">
       <Filter>Security</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\..\security\oc_cred_util.c">
+      <Filter>Security</Filter>
+    </ClCompile>
     <ClCompile Include="..\..\..\security\oc_doxm.c">
       <Filter>Security</Filter>
     </ClCompile>

diff --git a/security/oc_certs.c b/security/oc_certs.c
@@ -163,6 +163,7 @@ oc_certs_parse_serial_number(const unsigned char *cert, size_t cert_size,
   int ret = mbedtls_x509_crt_parse(&crt, cert, cert_size);
   if (ret != 0) {
     OC_ERR("could not parse the provided cert %d", ret);
+    mbedtls_x509_crt_free(&crt);
     return ret;
   }
 
@@ -197,6 +198,7 @@ oc_certs_parse_private_key(size_t device, const unsigned char *cert,
   int ret = mbedtls_x509_crt_parse(&crt, cert, cert_size);
   if (ret != 0) {
     OC_ERR("could not parse the provided cert %d", ret);
+    mbedtls_x509_crt_free(&crt);
     return ret;
   }
 
@@ -255,6 +257,7 @@ oc_certs_parse_public_key(const unsigned char *cert, size_t cert_size,
   int ret = mbedtls_x509_crt_parse(&crt, cert, cert_size);
   if (ret != 0) {
     OC_ERR("could not parse the provided cert %d", ret);
+    mbedtls_x509_crt_free(&crt);
     return -1;
   }
 
@@ -273,6 +276,7 @@ oc_certs_parse_public_key_to_oc_string(const unsigned char *cert,
   int ret = mbedtls_x509_crt_parse(&crt, cert, cert_size);
   if (ret != 0) {
     OC_ERR("could not parse the provided cert %d", ret);
+    mbedtls_x509_crt_free(&crt);
     return -1;
   }
 
@@ -372,6 +376,7 @@ oc_certs_parse_CN_for_UUID(const unsigned char *cert, size_t cert_size,
   int ret = mbedtls_x509_crt_parse(&crt, cert, cert_size);
   if (ret != 0) {
     OC_ERR("could not parse the provided cert %d", ret);
+    mbedtls_x509_crt_free(&crt);
     return false;
   }
 
@@ -544,6 +549,7 @@ oc_certs_parse_first_role(const unsigned char *cert, size_t cert_size,
   int ret = mbedtls_x509_crt_parse(&crt, cert, cert_size);
   if (ret != 0) {
     OC_ERR("could not parse the provided cert %d", ret);
+    mbedtls_x509_crt_free(&crt);
     return false;
   }
 

diff --git a/security/oc_certs_generate.c b/security/oc_certs_generate.c
@@ -19,7 +19,8 @@
 
 #include "oc_config.h"
 
-#if defined(OC_SECURITY) && defined(OC_PKI) && defined(OC_DYNAMIC_ALLOCATION)
+#if defined(OC_SECURITY) && defined(OC_PKI) &&                                 \
+  (defined(OC_DYNAMIC_ALLOCATION) || defined(OC_TEST))
 
 #include "port/oc_log_internal.h"
 #include "security/oc_certs_generate_internal.h"
@@ -33,6 +34,7 @@
 #include <mbedtls/oid.h>
 #include <mbedtls/platform.h>
 #include <mbedtls/x509_crt.h>
+#include <stdlib.h>
 
 static bool
 certs_generate_serial_number(mbedtls_mpi *buffer, size_t size)
@@ -632,4 +634,4 @@ oc_certs_generate(const oc_certs_generate_t *data, unsigned char *buffer,
   return ret;
 }
 
-#endif /* OC_SECURITY && OC_PKI && OC_DYNAMIC_ALLOCATION */
+#endif /* OC_SECURITY && OC_PKI && (OC_DYNAMIC_ALLOCATION || OC_TEST) */
diff --git a/security/oc_certs_generate_internal.h b/security/oc_certs_generate_internal.h
@@ -20,7 +20,8 @@
 #ifndef OC_CERTS_GENERATE_INTERNAL_H
 #define OC_CERTS_GENERATE_INTERNAL_H
 
-#if defined(OC_SECURITY) && defined(OC_PKI) && defined(OC_DYNAMIC_ALLOCATION)
+#if defined(OC_SECURITY) && defined(OC_PKI) &&                                 \
+  (defined(OC_DYNAMIC_ALLOCATION) || defined(OC_TEST))
 
 #include "api/c-timestamp/timestamp.h"
 #include "oc_role.h"
@@ -138,6 +139,6 @@ int oc_certs_generate(const oc_certs_generate_t *data, unsigned char *buffer,
 }
 #endif
 
-#endif /* OC_SECURITY & OC_PKI && OC_DYNAMIC_ALLOCATION */
+#endif /* OC_SECURITY & OC_PKI && (OC_DYNAMIC_ALLOCATION || OC_TEST) */
 
 #endif /* OC_CERTS_GENERATE_INTERNAL_H */