Skip to content

Call security context validators during action-set validation #190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
resolvicomai wants to merge 1 commit into from
Closed

Call security context validators during action-set validation #190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions src/backend/SecurityContext.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,8 +531,32 @@ int BSL_SecCtx_ExecutePolicyActionSet(BSL_LibCtx_t *lib, BSL_SecurityResponseSet
bool BSL_SecCtx_ValidatePolicyActionSet(BSL_LibCtx_t *lib, const BSL_BundleRef_t *bundle,
const BSL_SecurityActionSet_t *action_set)
{
(void)lib;
(void)bundle;
(void)action_set;
if (lib == NULL || bundle == NULL || action_set == NULL)

@jeronstone jeronstone May 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer using the BSL_CHKFALSE macro, E.g. BSL_CHKFALSE(NULL != lib), etc.

{
return false;
}

for (size_t action_index = 0; action_index < BSL_SecurityActionSet_CountActions(action_set); action_index++)
{
const BSL_SecurityAction_t *action = BSL_SecurityActionSet_GetActionAtIndex(action_set, action_index);
for (size_t oper_index = 0; oper_index < BSL_SecurityAction_CountSecOpers(action); oper_index++)
{
const BSL_SecOper_t *sec_oper = BSL_SecurityAction_GetSecOperAtIndex(action, oper_index);
const BSL_SecCtxDesc_t *sec_ctx = BSL_SecCtxDict_cget(lib->sc_reg, sec_oper->context_id);
if (sec_ctx == NULL || sec_ctx->validate == NULL)
{
BSL_LOG_WARNING("No security context validator registered for context ID %" PRId64,
sec_oper->context_id);
return false;
}

if (!sec_ctx->validate(lib, bundle, sec_oper))
{
BSL_LOG_WARNING("Security context validator failed for context ID %" PRId64, sec_oper->context_id);
return false;
}
}
}

return true;
}
10 changes: 2 additions & 8 deletions src/security_context/BIB_HMAC_SHA2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,12 @@ bool BSLX_BIB_Validate(BSL_LibCtx_t *lib, const BSL_BundleRef_t *bundle, const B
// Note: Internal API distinction.
// Called before the `_execute` function. This checks ahead of time whether it contains the necessary info in order
// to perform the execution.
(void)lib;
(void)bundle;
(void)sec_oper;
return false;
return lib != NULL && bundle != NULL && sec_oper != NULL;
}

bool BSLX_BCB_Validate(BSL_LibCtx_t *lib, const BSL_BundleRef_t *bundle, const BSL_SecOper_t *sec_oper)

@jeronstone jeronstone May 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR did not introduce this issue, but this function should be moved to BCB_AES_GCM.c

{
(void)lib;
(void)bundle;
(void)sec_oper;
return false;
return lib != NULL && bundle != NULL && sec_oper != NULL;
}

/**
Expand Down
57 changes: 57 additions & 0 deletions test/test_BackendSecurityContext.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,31 @@

static BSL_TestContext_t LocalTestCtx;

static size_t TestSecCtxValidateCallCount = 0;
static uint64_t TestSecCtxValidatedTarget = 0;
static bool TestSecCtxValidateResult = true;

static bool BSL_TestSecCtx_Validate(BSL_LibCtx_t *lib, const BSL_BundleRef_t *bundle, const BSL_SecOper_t *sec_oper)
{
TEST_ASSERT_NOT_NULL(lib);
TEST_ASSERT_NOT_NULL(bundle);
TEST_ASSERT_NOT_NULL(sec_oper);

TestSecCtxValidateCallCount++;
TestSecCtxValidatedTarget = BSL_SecOper_GetTargetBlockNum(sec_oper);
return TestSecCtxValidateResult;
}

static int BSL_TestSecCtx_Execute(BSL_LibCtx_t *lib, BSL_BundleRef_t *bundle, const BSL_SecOper_t *sec_oper,
BSL_SecOutcome_t *sec_outcome)
{
(void)lib;
(void)bundle;
(void)sec_oper;
(void)sec_outcome;
return BSL_SUCCESS;
}

void suiteSetUp(void)
{
TEST_ASSERT_EQUAL_INT(0, BSL_HostDescriptors_Set(MockBPA_Agent_Descriptors(NULL)));
Expand All @@ -70,6 +95,38 @@ void tearDown(void)
TEST_ASSERT_EQUAL(0, BSL_TestContext_Deinit(&LocalTestCtx));
}

void test_SecurityContext_ValidatePolicyActionSet_UsesRegisteredValidator(void)

@jeronstone jeronstone May 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test does properly verify changes! I think the scope of this could be increased for sec ctx execute too since such a test does not exist. As in, verify BSL_TestSecCtx_Execute with the call count & target as well.

@jeronstone jeronstone May 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That case is essentially already exercised by the rest of the tests in this file, but I think an explicit simple test is still useful while we're at it with this one.

{
TestSecCtxValidateCallCount = 0;
TestSecCtxValidatedTarget = 0;
TestSecCtxValidateResult = false;

BSL_SecCtxDesc_t sec_ctx_desc = { 0 };
sec_ctx_desc.validate = BSL_TestSecCtx_Validate;
sec_ctx_desc.execute = BSL_TestSecCtx_Execute;
TEST_ASSERT_EQUAL(BSL_SUCCESS, BSL_API_RegisterSecurityContext(&LocalTestCtx.bsl, 99, sec_ctx_desc));

BSL_SecOper_t sec_oper;
BSL_SecOper_Init(&sec_oper);
BSL_SecOper_Populate(&sec_oper, 99, 1, 2, BSL_SECBLOCKTYPE_BIB, BSL_SECROLE_SOURCE, BSL_POLICYACTION_NOTHING);

BSL_SecurityAction_t action;
BSL_SecurityAction_Init(&action);
TEST_ASSERT_EQUAL(BSL_SUCCESS, BSL_SecurityAction_AppendSecOper(&action, &sec_oper));

BSL_SecurityActionSet_t action_set;
BSL_SecurityActionSet_Init(&action_set);
TEST_ASSERT_EQUAL(BSL_SUCCESS, BSL_SecurityActionSet_AppendAction(&action_set, &action));

TEST_ASSERT_FALSE(
BSL_SecCtx_ValidatePolicyActionSet(&LocalTestCtx.bsl, &LocalTestCtx.mock_bpa_ctr.bundle_ref, &action_set));
TEST_ASSERT_EQUAL_UINT(1, TestSecCtxValidateCallCount);
TEST_ASSERT_EQUAL_UINT64(1, TestSecCtxValidatedTarget);

@jeronstone jeronstone May 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TestSecCtxValidateResult seemingly unused / unverified after being set to false at top of the function.


BSL_SecurityAction_Deinit(&action);
BSL_SecurityActionSet_Deinit(&action_set);
}

/**
* @brief Purpose: Creates a BIB block and adds it to the bundle, confirms it matches the test vector in RFC9173
*
Expand Down
Loading