Fixing edge cases in the mock-bpa EID and EID Pattern decoders (#74)

* Fixing edge cases in the mock-bpa EID and EID Pattern decoders

* Avoid extreme logging for large test runs

Author: BrianSipos

.github/workflows/fuzzing.yaml

@@ -87,15 +87,16 @@ jobs:
         run: >
           ./build.sh prep
           -DBUILD_DOCS_API=OFF -DBUILD_DOCS_MAN=OFF
-          -DBUILD_UNITTEST=OFF -DBUILD_FUZZING=ON -DBUILD_COVERAGE=ON
+          -DBUILD_MOCK_BPA=OFF -DBUILD_UNITTEST=OFF
+          -DBUILD_FUZZING=ON -DBUILD_COVERAGE=ON
       - name: Build
         run: ./build.sh
       - name: Install
         run: ./build.sh install
       - name: Test
         run: ./build.sh check
       - name: Report corpus
-        run: for FN in $(find test/*cbor*corpus -type f); do echo $FN; cborseq2diag.rb <$FN; done
+        run: for FN in $(find test/*cbor*corpus -type f); do echo $FN; cborseq2diag.rb <$FN || true; done
       - name: Collect coverage
         run: ./build.sh coverage
       - name: Archive coverage

CMakeGraphVizOptions.cmake

@@ -1,2 +1,3 @@
 set(GRAPHVIZ_GENERATE_PER_TARGET TRUE)
-set(GRAPHVIZ_IGNORE_TARGETS test_* unity gcov)
+set(GRAPHVIZ_CUSTOM_TARGETS FALSE)
+set(GRAPHVIZ_IGNORE_TARGETS "^test_.+" "^fuzz_.+" "^unity$" "^gcov$" "^m$" "\\.so$")

CMakeLists.txt

@@ -25,6 +25,7 @@ option(BUILD_LIB "Build the library itself" ON)
 option(BUILD_SHARED_LIBS "Build using shared libraries" ON)
 option(BUILD_DOCS_API "Enable API documentation building" OFF)
 option(BUILD_DOCS_MAN "Enable manpage building" OFF)
+option(BUILD_MOCK_BPA "Enable building the Mock BPA executable" ON)
 option(BUILD_UNITTEST "Enable building unit tests" ON)
 option(TEST_MEMCHECK "Enable test runtime memory checking" ON)
 option(BUILD_COVERAGE "Enable runtime coverage logging and reporting" OFF)

cmake/Findlibfuzzer.cmake

@@ -9,6 +9,11 @@ if(NOT CMAKE_C_COMPILER_ID MATCHES "Clang"
   message(FATAL "Can only fuzz with clang compiler")
 endif()
 
+# Options for all compilation units
+add_compile_options(
+  -fsanitize=fuzzer-no-link
+)
+
 function(add_fuzz_test)
   set(options OPTIONAL )
   set(oneValueArgs TARGET MAIN_NAME RUNS_COUNT)

src/BPSecLib_Private.h

@@ -186,6 +186,32 @@ void BSL_openlog(void);
  */
 void BSL_closelog(void);
 
+/** Interpret a text name as a severity level.
+ *
+ * @param[out] severity The associated severity level.
+ * @param[in] name The text name, which is case insensitive.
+ * @return Zero if successful.
+ */
+int BSL_LogGetSeverity(int *severity, const char *name);
+
+/** Set the least severity enabled for logging.
+ * Other events will be dropped by the logging facility.
+ * This function is multi-thread safe.
+ *
+ * @param severity The severity from a subset of the POSIX syslog values.
+ * @sa BSL_log_is_enabled_for()
+ */
+void BSL_LogSetLeastSeverity(int severity);
+
+/** Determine if a particular severity is being logged.
+ * This function is multi-thread safe.
+ *
+ * @param severity The severity from a subset of the POSIX syslog values.
+ * @return True if the severity level will be logged.
+ * @sa BSL_log_set_least_severity()
+ */
+bool BSL_LogIsEnabledFor(int severity);
+
 /** Log an event.
  *
  * @param severity The severity from a subset of the POSIX syslog values.

src/CMakeLists.txt

@@ -148,8 +148,8 @@ target_link_libraries(bsl_dynamic PUBLIC bsl_front)
 target_link_libraries(bsl_dynamic PUBLIC MLIB::mlib)
 target_link_libraries(bsl_dynamic PUBLIC QCBOR::qcbor)
 
-
 add_subdirectory(mock_bpa)
+
 find_program(CLANG_FORMAT "clang-format")
 if(CLANG_FORMAT)
     # Define which files to format

src/backend/HostInterface.c

@@ -159,11 +159,11 @@ int BSL_Host_GetSecSrcEID(BSL_HostEID_t *eid)
     return HostDescriptorTable.get_host_eid_fn(HostDescriptorTable.user_data, eid);
 }
 
-int BSL_HostEID_EncodeToCBOR(const BSL_HostEID_t *eid, void *user_data)
+int BSL_HostEID_EncodeToCBOR(const BSL_HostEID_t *eid, void *encoder)
 {
     CHK_ARG_NONNULL(eid);
-    CHK_ARG_NONNULL(user_data);
-    return HostDescriptorTable.eid_to_cbor(user_data, eid);
+    CHK_ARG_NONNULL(encoder);
+    return HostDescriptorTable.eid_to_cbor(encoder, eid);
 }
 
 int BSL_HostEID_DecodeFromCBOR(BSL_HostEID_t *eid, void *decoder)
@@ -181,8 +181,7 @@ int BSL_HostEID_DecodeFromText(BSL_HostEID_t *eid, const char *text)
     CHK_ARG_NONNULL(eid);
     CHK_ARG_NONNULL(text);
 
-    // Basic sanity check, may need to remove.
-    CHK_PRECONDITION(strlen(text) < 100);
+    CHK_PRECONDITION(eid->handle != NULL);
     CHK_PRECONDITION(HostDescriptorTable.eid_from_text != NULL);
 
     return HostDescriptorTable.eid_from_text(eid, text, HostDescriptorTable.user_data);
@@ -205,7 +204,6 @@ int BSL_HostEIDPattern_DecodeFromText(BSL_HostEIDPattern_t *pat, const char *tex
 {
     CHK_ARG_NONNULL(pat);
     CHK_ARG_NONNULL(text);
-    CHK_ARG_EXPR(strlen(text) < 100);
     CHK_PRECONDITION(HostDescriptorTable.eidpat_from_text != NULL);
     return HostDescriptorTable.eidpat_from_text(pat, text, HostDescriptorTable.user_data);
 }

src/backend/LoggingStderr.c

@@ -35,6 +35,7 @@
 
 #include <m-buffer.h>
 #include <m-string.h>
+#include <m-atomic.h>
 
 /// Number of events to buffer to I/O thread
 #define BSL_LOG_QUEUE_SIZE 100
@@ -82,14 +83,48 @@ static void BSL_LogEvent_event_deinit(BSL_LogEvent_event_t *obj)
     string_clear(obj->context);
 }
 
+static void BSL_LogEvent_event_init_set(BSL_LogEvent_event_t *obj, const BSL_LogEvent_event_t *src)
+{
+    obj->thread    = src->thread;
+    obj->timestamp = src->timestamp;
+    obj->severity  = src->severity;
+    string_init_set(obj->context, src->context);
+    string_init_set(obj->message, src->message);
+}
+
+static void BSL_LogEvent_event_init_move(BSL_LogEvent_event_t *obj, BSL_LogEvent_event_t *src)
+{
+    obj->thread    = src->thread;
+    obj->timestamp = src->timestamp;
+    obj->severity  = src->severity;
+    string_init_move(obj->context, src->context);
+    string_init_move(obj->message, src->message);
+}
+
+static void BSL_LogEvent_event_set(BSL_LogEvent_event_t *obj, const BSL_LogEvent_event_t *src)
+{
+    obj->thread    = src->thread;
+    obj->timestamp = src->timestamp;
+    obj->severity  = src->severity;
+    string_set(obj->context, src->context);
+    string_set(obj->message, src->message);
+}
+
 /// OPLIST for BSL_LogEvent_event_t
-#define M_OPL_BSL_LogEvent_event_t() (INIT(API_2(BSL_LogEvent_event_init)), CLEAR(API_2(BSL_LogEvent_event_deinit)))
+#define M_OPL_BSL_LogEvent_event_t()                                                     \
+    (INIT(API_2(BSL_LogEvent_event_init)), INIT_SET(API_6(BSL_LogEvent_event_init_set)), \
+     INIT_MOVE(API_6(BSL_LogEvent_event_init_move)), SET(API_6(BSL_LogEvent_event_set)), \
+     CLEAR(API_2(BSL_LogEvent_event_deinit)))
 
 // NOLINTBEGIN
 /// @cond Doxygen_Suppress
-M_BUFFER_DEF(BSL_LogEvent_queue, BSL_LogEvent_event_t, BSL_LOG_QUEUE_SIZE, M_BUFFER_THREAD_SAFE | M_BUFFER_BLOCKING)
+M_BUFFER_DEF(BSL_LogEvent_queue, BSL_LogEvent_event_t, BSL_LOG_QUEUE_SIZE,
+             M_BUFFER_THREAD_SAFE | M_BUFFER_BLOCKING | M_BUFFER_PUSH_INIT_POP_MOVE)
 /// @endcond
 
+/// Shared least severity
+static atomic_int least_severity = LOG_DEBUG;
+
 /// Shared safe queue
 static BSL_LogEvent_queue_t event_queue;
 /// Sink thread ID
@@ -156,17 +191,19 @@ static void write_log(const BSL_LogEvent_event_t *event)
 
 static void *work_sink(void *arg _U_)
 {
-    while (true)
+    bool running = true;
+    while (running)
     {
         BSL_LogEvent_event_t event;
         BSL_LogEvent_queue_pop(&event, event_queue);
         if (string_empty_p(event.message))
         {
-            BSL_LogEvent_event_deinit(&event);
-            break;
+            running = false;
+        }
+        else
+        {
+            write_log(&event);
         }
-
-        write_log(&event);
         BSL_LogEvent_event_deinit(&event);
     }
     return NULL;
@@ -198,6 +235,7 @@ void BSL_closelog(void)
     BSL_LogEvent_event_t event;
     BSL_LogEvent_event_init(&event);
     BSL_LogEvent_queue_push(event_queue, event);
+    BSL_LogEvent_event_deinit(&event);
 
     int res = pthread_join(thr_sink, NULL);
     if (res)
@@ -214,12 +252,59 @@ void BSL_closelog(void)
     {
         atomic_store(&thr_valid, false);
     }
+
+    // no consumer after join above
+    BSL_LogEvent_queue_clear(event_queue);
+}
+
+int BSL_LogGetSeverity(int *severity, const char *name)
+{
+    CHKERR1(severity)
+    CHKERR1(name)
+
+    for (size_t ix = 0; ix < sizeof(sev_names) / sizeof(const char *); ++ix)
+    {
+        if (!sev_names[ix])
+        {
+            continue;
+        }
+        if (strcasecmp(sev_names[ix], name) == 0)
+        {
+            *severity = (int)ix;
+            return 0;
+        }
+    }
+    return 2;
+}
+
+void BSL_LogSetLeastSeverity(int severity)
+{
+    if ((severity < 0) || (severity > LOG_DEBUG))
+    {
+        return;
+    }
+
+    atomic_store(&least_severity, severity);
+}
+
+bool BSL_LogIsEnabledFor(int severity)
+{
+    if ((severity < 0) || (severity > LOG_DEBUG))
+    {
+        return false;
+    }
+
+    const int limit = atomic_load(&least_severity);
+    // lower severity has higher define value
+    const bool enabled = (limit >= severity);
+
+    return enabled;
 }
 
 // NOLINTBEGIN
 void BSL_LogEvent(int severity, const char *filename, int lineno, const char *funcname, const char *format, ...)
 {
-    if ((severity < 0) || (severity > LOG_DEBUG))
+    if (!BSL_LogIsEnabledFor(severity))
     {
         return;
     }
@@ -251,28 +336,25 @@ void BSL_LogEvent(int severity, const char *filename, int lineno, const char *fu
         va_end(val);
     }
 
-    if (string_empty_p(event.message))
-    {
-        // ignore empty messages
-        BSL_LogEvent_event_deinit(&event);
-        return;
-    }
-
-    if (atomic_load(&thr_valid))
-    {
-        BSL_LogEvent_queue_push(event_queue, event);
-    }
-    else
+    // ignore empty messages
+    if (!string_empty_p(event.message))
     {
-        BSL_LogEvent_event_t manual;
-        BSL_LogEvent_event_init(&manual);
-        manual.severity = LOG_CRIT;
-        string_set_str(manual.message, "BSL_LogEvent() called before BSL_openlog()");
-        write_log(&manual);
-        BSL_LogEvent_event_deinit(&manual);
+        if (atomic_load(&thr_valid))
+        {
+            BSL_LogEvent_queue_push(event_queue, event);
+        }
+        else
+        {
+            BSL_LogEvent_event_t manual;
+            BSL_LogEvent_event_init(&manual);
+            manual.severity = LOG_CRIT;
+            string_set_str(manual.message, "BSL_LogEvent() called before BSL_openlog()");
+            write_log(&manual);
+            BSL_LogEvent_event_deinit(&manual);
 
-        write_log(&event);
-        BSL_LogEvent_event_deinit(&event);
+            write_log(&event);
+        }
     }
+    BSL_LogEvent_event_deinit(&event);
 }
 // NOLINTEND

src/mock_bpa/CMakeLists.txt

@@ -61,15 +61,9 @@ target_link_libraries(bsl_mock_bpa PUBLIC MLIB::mlib)
 target_link_libraries(bsl_mock_bpa PUBLIC QCBOR::qcbor)
 target_link_libraries(bsl_mock_bpa PUBLIC Jansson::Jansson)
 target_link_libraries(bsl_mock_bpa PUBLIC bsl_front bsl_dynamic bsl_default_sc bsl_sample_pp)
-
-add_executable(bsl-mock-bpa)
-target_sources(bsl-mock-bpa PRIVATE mock_bpa.c)
-target_link_libraries(bsl-mock-bpa PUBLIC bsl_mock_bpa bsl_crypto)
-target_link_libraries(bsl-mock-bpa PUBLIC Threads::Threads)
-
 # Installation config
 install(
-  TARGETS bsl_mock_bpa bsl-mock-bpa
+  TARGETS bsl_mock_bpa
   PUBLIC_HEADER DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/bsl"
     RUNTIME
         COMPONENT test
@@ -79,3 +73,18 @@ install(
     ARCHIVE
         COMPONENT devel
 )
+
+if(BUILD_MOCK_BPA)
+  add_executable(bsl-mock-bpa)
+  target_sources(bsl-mock-bpa PRIVATE mock_bpa.c)
+  target_link_libraries(bsl-mock-bpa PUBLIC bsl_mock_bpa bsl_crypto)
+  target_link_libraries(bsl-mock-bpa PUBLIC Threads::Threads)
+
+  # Installation config
+  install(
+    TARGETS bsl-mock-bpa
+    PUBLIC_HEADER DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/bsl"
+      RUNTIME
+          COMPONENT test
+  )
+endif(BUILD_MOCK_BPA)