BCB Verifier Fix (#155)

jeronstone · web-flow · commit 621ed426c8c1 · 2026-04-16T18:23:04.000-04:00
* Init, failing

* seqwriter NULL

* Clean up unit tests

* apply format ubuntu

* update docs

* apply format ubuntu

* Adjust function names and add consistency between bib verif/acc and bcb

* apply format ubuntu
diff --git a/mock-bpa-test/test_requirements.py b/mock-bpa-test/test_requirements.py
@@ -822,3 +822,26 @@ def test_BSL_49(self):
             input_data_format=DataFormat.BUNDLEARRAY,
             expected_output_format=DataFormat.BUNDLEARRAY
         ))
+
+    def test_BCB_verifier(self):
+        self._single_test(_TestCase(
+            input_data=[
+                [7, 0, 0, [2, [1, 2]], [2, [2, 1]], [2, [2, 1]], [0, 40], 1000000],
+                [12, 2, 1, 0, bytes.fromhex(
+                    '8101020182028202018482014c5477656c76653132313231328202018203581869c411276fecddc4780df42c8a2af89296fabf34d7fae7008204008181820150efa4b5ac0108e3816c5606479801bc04')],
+                [1, 1, 0, 0, bytes.fromhex(
+                    '3a09c1e63fe23a7f66a59c7303837241e070b02619fc59c5214a22f08cd70795e73e9a')]
+            ],
+            expected_output=[
+                [7, 0, 0, [2, [1, 2]], [2, [2, 1]], [2, [2, 1]], [0, 40], 1000000],
+                [12, 2, 1, 0, bytes.fromhex(
+                    '8101020182028202018482014c5477656c76653132313231328202018203581869c411276fecddc4780df42c8a2af89296fabf34d7fae7008204008181820150efa4b5ac0108e3816c5606479801bc04')],
+                [1, 1, 0, 0, bytes.fromhex(
+                    '3a09c1e63fe23a7f66a59c7303837241e070b02619fc59c5214a22f08cd70795e73e9a')]
+            ],
+            policy_config='0x165',
+            key_set="mock-bpa-test/key_set_1.json",
+            is_working=True,
+            input_data_format=DataFormat.BUNDLEARRAY,
+            expected_output_format=DataFormat.BUNDLEARRAY
+        ))
diff --git a/src/BPSecLib_Private.h b/src/BPSecLib_Private.h
@@ -1113,7 +1113,7 @@ size_t BSL_SecOutcome_CountParams(const BSL_SecOutcome_t *self);
  */
 const BSL_SecParam_t *BSL_SecOutcome_GetParamAt(const BSL_SecOutcome_t *self, size_t index);
 
-/// @brief Returns true if this (the parameters and results) is contained within the given ASK
+/// @brief Returns true if this (the parameters and results) is contained within the given ASB
 /// @todo Can move to backend
 /// @param[in] self
 /// @param[in] outcome
diff --git a/src/CryptoInterface.h b/src/CryptoInterface.h
@@ -284,7 +284,8 @@ int BSL_Cipher_AddData(BSL_Cipher_t *cipher_ctx, BSL_Data_t plaintext, BSL_Data_
  * Add data to encrypt or decrypt to the context sequentially
  * @param cipher_ctx pointer to context to add data to
  * @param[in] reader pointer to sequential reader - input to crypto operation
- * @param[in] writer pointer to sequential writer - output of crypto operation
+ * @param[in] writer pointer to sequential writer (output of crypto operation), or NULL (crypto output will not be
+ * written)
  * @return 0 if successful
  */
 int BSL_Cipher_AddSeq(BSL_Cipher_t *cipher_ctx, BSL_SeqReader_t *reader, BSL_SeqWriter_t *writer);
diff --git a/src/backend/SecurityContext.c b/src/backend/SecurityContext.c
@@ -101,7 +101,8 @@ static int BSL_ExecBIBSource(BSL_SecCtx_Execute_f sec_context_fn, BSL_LibCtx_t *
 
     CHK_PROPERTY(created_block_num > 1);
 
-    const int bib_result = (*sec_context_fn)(lib, bundle, sec_oper, outcome);
+    sec_oper->sec_block_num = created_block_num;
+    const int bib_result    = (*sec_context_fn)(lib, bundle, sec_oper, outcome);
     if (bib_result != 0) // || outcome->is_success == false)
     {
         BSL_LOG_ERR("BIB Source failed!");
@@ -151,8 +152,8 @@ static int BSL_ExecBIBSource(BSL_SecCtx_Execute_f sec_context_fn, BSL_LibCtx_t *
     return res;
 }
 
-static int BSL_ExecBIBAccept(BSL_SecCtx_Execute_f sec_context_fn, BSL_LibCtx_t *lib, BSL_BundleRef_t *bundle,
-                             BSL_SecOper_t *sec_oper, BSL_SecOutcome_t *outcome)
+static int BSL_ExecBIBVerifierAcceptor(BSL_SecCtx_Execute_f sec_context_fn, BSL_LibCtx_t *lib, BSL_BundleRef_t *bundle,
+                                       BSL_SecOper_t *sec_oper, BSL_SecOutcome_t *outcome)
 {
     CHK_ARG_NONNULL(lib);
     CHK_ARG_NONNULL(bundle);
@@ -201,76 +202,70 @@ static int BSL_ExecBIBAccept(BSL_SecCtx_Execute_f sec_context_fn, BSL_LibCtx_t *
     const int sec_context_result = (*sec_context_fn)(lib, bundle, sec_oper, outcome);
     if (sec_context_result != BSL_SUCCESS) // || outcome->is_success == false)
     {
-        BSL_LOG_ERR("BIB Acceptor failed!");
+        BSL_LOG_ERR("BIB Sec Ctx processing for verifier/acceptor failed!");
         BSL_AbsSecBlock_Deinit(&abs_sec_block);
         BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_FAIL_COUNT, 1);
         return BSL_ERR_SECURITY_OPERATION_FAILED;
     }
 
-    bool auth_success = BSL_SecOutcome_IsInAbsSecBlock(outcome, &abs_sec_block);
-    if (!auth_success)
+    if (!BSL_SecOutcome_IsInAbsSecBlock(outcome, &abs_sec_block))
     {
-        BSL_LOG_ERR("BIB Accepting failed");
+        BSL_LOG_ERR("ASB Does not contain expeceted sec params and outcomes");
+        BSL_AbsSecBlock_Deinit(&abs_sec_block);
         BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_FAIL_COUNT, 1);
+        return BSL_ERR_SECURITY_OPERATION_FAILED;
+    }
+
+    // If secop is to verify, processing is complete
+    if (BSL_SecOper_IsRoleVerifier(sec_oper))
+    {
+        BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_VERIFIER_COUNT, 1);
+        BSL_AbsSecBlock_Deinit(&abs_sec_block);
+        return BSL_SUCCESS;
     }
 
     // TODO/FIXME - This logic seems to be correct, but should be refactored and simplified.
     // There are too many branches/conditionals each with their own return statement.
 
-    if (BSL_SecOper_IsRoleAcceptor(sec_oper))
+    // If secop is to accept, BIB must be removed from bundle
+    uint64_t target_block_num = BSL_SecOper_GetTargetBlockNum(sec_oper);
+    int      status           = BSL_AbsSecBlock_StripResults(&abs_sec_block, target_block_num);
+    if (status < 0)
     {
-        uint64_t target_block_num = BSL_SecOper_GetTargetBlockNum(sec_oper);
-        int      status           = BSL_AbsSecBlock_StripResults(&abs_sec_block, target_block_num);
-        if (status < 0)
+        BSL_LOG_ERR("Failure to strip ASB of results");
+        BSL_AbsSecBlock_Deinit(&abs_sec_block);
+        BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_FAIL_COUNT, 1);
+        return BSL_ERR_FAILURE;
+    }
+
+    if (BSL_AbsSecBlock_IsEmpty(&abs_sec_block))
+    {
+        if (BSL_BundleCtx_RemoveBlock(bundle, sec_blk.block_num) != BSL_SUCCESS)
         {
-            BSL_LOG_ERR("Failure to strip ASB of results");
+            BSL_LOG_ERR("Failed to remove block when ASB is empty");
             BSL_AbsSecBlock_Deinit(&abs_sec_block);
             BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_FAIL_COUNT, 1);
-            return BSL_ERR_FAILURE;
+            return BSL_ERR_HOST_CALLBACK_FAILED;
         }
-
-        if (BSL_AbsSecBlock_IsEmpty(&abs_sec_block))
-        {
-            if (BSL_BundleCtx_RemoveBlock(bundle, sec_blk.block_num) != BSL_SUCCESS)
-            {
-                BSL_LOG_ERR("Failed to remove block when ASB is empty");
-                BSL_AbsSecBlock_Deinit(&abs_sec_block);
-                BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_FAIL_COUNT, 1);
-                return BSL_ERR_HOST_CALLBACK_FAILED;
-            }
-        }
-        else
-        {
-            int res = Encode_ASB(lib, bundle, sec_blk.block_num, &abs_sec_block);
-            if (res != BSL_SUCCESS)
-            {
-                BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_FAIL_COUNT, 1);
-                return res;
-            }
-        }
-        BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_ACCEPTOR_COUNT, 1);
-    }
-
-    BSL_AbsSecBlock_Deinit(&abs_sec_block);
-
-    // TODO(bvb) Check postconditions that the block actually was removed
-    if (auth_success)
-    {
-        BSL_LOG_INFO("BIB Accept SUCCESS");
     }
     else
     {
-        BSL_LOG_ERR("BIB Accept FAIL");
-        BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_FAIL_COUNT, 1);
+        int res = Encode_ASB(lib, bundle, sec_blk.block_num, &abs_sec_block);
+        if (res != BSL_SUCCESS)
+        {
+            BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_FAIL_COUNT, 1);
+            return res;
+        }
     }
+    BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_ACCEPTOR_COUNT, 1);
+    BSL_AbsSecBlock_Deinit(&abs_sec_block);
 
-    return auth_success ? BSL_SUCCESS : BSL_ERR_SECURITY_OPERATION_FAILED;
+    return BSL_SUCCESS;
 }
 
-static int BSL_ExecBCBAcceptor(BSL_SecCtx_Execute_f sec_context_fn, BSL_LibCtx_t *lib, BSL_BundleRef_t *bundle,
-                               BSL_SecOper_t *sec_oper, BSL_SecOutcome_t *outcome)
+static int BSL_ExecBCBVerifierAcceptor(BSL_SecCtx_Execute_f sec_context_fn, BSL_LibCtx_t *lib, BSL_BundleRef_t *bundle,
+                                       BSL_SecOper_t *sec_oper, BSL_SecOutcome_t *outcome)
 {
-    (void)lib;
     CHK_ARG_NONNULL(sec_context_fn);
     CHK_ARG_NONNULL(bundle);
     CHK_ARG_NONNULL(sec_oper);
@@ -319,7 +314,7 @@ static int BSL_ExecBCBAcceptor(BSL_SecCtx_Execute_f sec_context_fn, BSL_LibCtx_t
     BSL_SecParam_t results_as_params[result_count];
     for (size_t i = 0; i < result_count; i++)
     {
-        BSL_SecResult_t *result = BSLB_SecResultList_get(abs_sec_block.results, i);
+        const BSL_SecResult_t *result = BSLB_SecResultList_get(abs_sec_block.results, i);
         if (result->target_block_num == sec_oper->target_block_num)
         {
             BSL_Data_t as_data;
@@ -332,62 +327,61 @@ static int BSL_ExecBCBAcceptor(BSL_SecCtx_Execute_f sec_context_fn, BSL_LibCtx_t
     }
 
     const int sec_context_result = (*sec_context_fn)(lib, bundle, sec_oper, outcome);
-    if (sec_context_result != BSL_SUCCESS) // || outcome->is_success == false)
+    if (sec_context_result != BSL_SUCCESS)
     {
-        BSL_LOG_ERR("BCB Acceptor failed!");
+        BSL_LOG_ERR("BCB Sec Ctx processing for verifier/acceptor failed!");
         BSL_AbsSecBlock_Deinit(&abs_sec_block);
         BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_FAIL_COUNT, 1);
         return BSL_ERR_SECURITY_OPERATION_FAILED;
     }
 
-    // TODO/FIXME - This logic seems to be correct, but should be refactored and simplified.
-    // There are too many branches/conditionals each with their own return statement.
+    // If secop is to verify, processing is complete
+    if (BSL_SecOper_IsRoleVerifier(sec_oper))
+    {
+        BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_VERIFIER_COUNT, 1);
+        BSL_AbsSecBlock_Deinit(&abs_sec_block);
+        return BSL_SUCCESS;
+    }
+
+    // If secop is to accept, BCB must be removed from bundle
+    uint64_t target_block_num = BSL_SecOper_GetTargetBlockNum(sec_oper);
+    int      status           = BSL_AbsSecBlock_StripResults(&abs_sec_block, target_block_num);
+    if (status < 0)
+    {
+        BSL_LOG_ERR("Failure to strip ASB of results");
+        BSL_AbsSecBlock_Deinit(&abs_sec_block);
+        BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_FAIL_COUNT, 1);
+        return BSL_ERR_FAILURE;
+    }
 
-    if (BSL_SecOper_IsRoleAcceptor(sec_oper))
+    if (BSL_AbsSecBlock_IsEmpty(&abs_sec_block))
     {
-        uint64_t target_block_num = BSL_SecOper_GetTargetBlockNum(sec_oper);
-        int      status           = BSL_AbsSecBlock_StripResults(&abs_sec_block, target_block_num);
-        if (status < 0)
+        if (BSL_BundleCtx_RemoveBlock(bundle, sec_blk.block_num) != BSL_SUCCESS)
         {
-            BSL_LOG_ERR("Failure to strip ASB of results");
+            BSL_LOG_ERR("Failed to remove block when ASB is empty");
             BSL_AbsSecBlock_Deinit(&abs_sec_block);
             BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_FAIL_COUNT, 1);
-            return BSL_ERR_FAILURE;
+            return BSL_ERR_HOST_CALLBACK_FAILED;
         }
-
-        if (BSL_AbsSecBlock_IsEmpty(&abs_sec_block))
-        {
-            if (BSL_BundleCtx_RemoveBlock(bundle, sec_blk.block_num) != BSL_SUCCESS)
-            {
-                BSL_LOG_ERR("Failed to remove block when ASB is empty");
-                BSL_AbsSecBlock_Deinit(&abs_sec_block);
-                BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_FAIL_COUNT, 1);
-                return BSL_ERR_HOST_CALLBACK_FAILED;
-            }
-        }
-        else
+    }
+    else
+    {
+        int res = Encode_ASB(lib, bundle, sec_blk.block_num, &abs_sec_block);
+        if (res != BSL_SUCCESS)
         {
-            int res = Encode_ASB(lib, bundle, sec_blk.block_num, &abs_sec_block);
-            if (res != BSL_SUCCESS)
-            {
-                BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_FAIL_COUNT, 1);
-                return res;
-            }
+            BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_FAIL_COUNT, 1);
+            return res;
         }
-        BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_ACCEPTOR_COUNT, 1);
     }
-
+    BSL_TlmCounters_IncrementCounter(lib, BSL_TLM_SECOP_ACCEPTOR_COUNT, 1);
     BSL_AbsSecBlock_Deinit(&abs_sec_block);
 
-    // TODO(bvb) Check postconditions that the block actually was removed
     return BSL_SUCCESS;
 }
 
 static int BSL_ExecBCBSource(BSL_SecCtx_Execute_f sec_context_fn, BSL_LibCtx_t *lib, BSL_BundleRef_t *bundle,
                              BSL_SecOper_t *sec_oper, BSL_SecOutcome_t *outcome)
 {
-    (void)lib;
-
     CHK_ARG_NONNULL(sec_context_fn);
     CHK_ARG_NONNULL(bundle);
     CHK_ARG_NONNULL(sec_oper);
@@ -497,7 +491,7 @@ int BSL_SecCtx_ExecutePolicyActionSet(BSL_LibCtx_t *lib, BSL_SecurityResponseSet
             {
                 errcode = BSL_SecOper_IsRoleSource(sec_oper) == true
                               ? BSL_ExecBIBSource(sec_ctx->execute, lib, bundle, sec_oper, outcome)
-                              : BSL_ExecBIBAccept(sec_ctx->execute, lib, bundle, sec_oper, outcome);
+                              : BSL_ExecBIBVerifierAcceptor(sec_ctx->execute, lib, bundle, sec_oper, outcome);
             }
             else
             {
@@ -507,7 +501,7 @@ int BSL_SecCtx_ExecutePolicyActionSet(BSL_LibCtx_t *lib, BSL_SecurityResponseSet
                 }
                 else
                 {
-                    errcode = BSL_ExecBCBAcceptor(sec_ctx->execute, lib, bundle, sec_oper, outcome);
+                    errcode = BSL_ExecBCBVerifierAcceptor(sec_ctx->execute, lib, bundle, sec_oper, outcome);
                 }
             }
 
diff --git a/src/crypto/CryptoInterface.c b/src/crypto/CryptoInterface.c
@@ -548,7 +548,11 @@ int BSL_Cipher_AddSeq(BSL_Cipher_t *cipher_ctx, BSL_SeqReader_t *reader, BSL_Seq
 
         key->stats.stats[BSL_CRYPTO_KEYSTATS_BYTES_PROCESSED] += block_size_int;
         block_size = (size_t)block_size_int;
-        BSL_SeqWriter_Put(writer, write_buf, block_size);
+
+        if (NULL != writer)
+        {
+            BSL_SeqWriter_Put(writer, write_buf, block_size);
+        }
     }
 
     return 0;
diff --git a/src/security_context/BCB_AES_GCM.c b/src/security_context/BCB_AES_GCM.c
@@ -175,18 +175,21 @@ static int BSLX_BCB_Decrypt(BSLX_BCB_t *bcb_context)
     if (retval == BSL_SUCCESS)
     {
         btsd_read = BSL_BundleCtx_ReadBTSD(bcb_context->bundle, bcb_context->target_block.block_num);
-        // output is same size
-        btsd_write = BSL_BundleCtx_WriteBTSD(bcb_context->bundle, bcb_context->target_block.block_num,
-                                             bcb_context->target_block.btsd_len);
         if (!btsd_read)
         {
             BSL_LOG_ERR("Failed to construct reader");
             retval = BSL_ERR_HOST_CALLBACK_FAILED;
         }
-        if (!btsd_write)
+
+        if (bcb_context->overwrite_btsd)
         {
-            BSL_LOG_ERR("Failed to construct writer");
-            retval = BSL_ERR_HOST_CALLBACK_FAILED;
+            btsd_write = BSL_BundleCtx_WriteBTSD(bcb_context->bundle, bcb_context->target_block.block_num,
+                                                 bcb_context->target_block.btsd_len);
+            if (!btsd_write)
+            {
+                BSL_LOG_ERR("Failed to construct writer");
+                retval = BSL_ERR_HOST_CALLBACK_FAILED;
+            }
         }
     }
 
@@ -228,7 +231,10 @@ static int BSLX_BCB_Decrypt(BSLX_BCB_t *bcb_context)
 
     // close write after read
     BSL_SeqReader_Destroy(btsd_read);
-    BSL_SeqWriter_Destroy(btsd_write);
+    if (NULL != btsd_write)
+    {
+        BSL_SeqWriter_Destroy(btsd_write);
+    }
 
     BSL_Data_Deinit(&bcb_context->authtag);
     if (bcb_context->keywrap)
@@ -636,6 +642,9 @@ int BSLX_BCB_Execute(BSL_LibCtx_t *lib _U_, BSL_BundleRef_t *bundle, const BSL_S
         return BSL_ERR_SECURITY_CONTEXT_FAILED;
     }
 
+    // If secop is accpeting BCB, target btsd should be overwritten with resulting plaintext
+    bcb_context.overwrite_btsd = BSL_SecOper_IsRoleAcceptor(sec_oper);
+
     // Select whether to call the encrypt or decrypt function
     int (*crypto_fn)(BSLX_BCB_t *) = BSL_SecOper_IsRoleSource(sec_oper) ? BSLX_BCB_Encrypt : BSLX_BCB_Decrypt;
 
diff --git a/src/security_context/DefaultSecContext_Private.h b/src/security_context/DefaultSecContext_Private.h
@@ -111,6 +111,7 @@ typedef struct BSLX_BCB_s
     bool    skip_aad_sec_block;
     bool    skip_aad_target_block;
     bool    skip_aad_prim_block;
+    bool    overwrite_btsd;
 } BSLX_BCB_t;
 
 int BSLX_BCB_GetParams(const BSL_BundleRef_t *bundle, BSLX_BCB_t *bcb_context, const BSL_SecOper_t *sec_oper);
diff --git a/test/test_PublicInterfaceImpl.c b/test/test_PublicInterfaceImpl.c
