From 9748d71ae16b1e46a6055c0b0b1f5f48d04a7e07 Mon Sep 17 00:00:00 2001 From: Daniel Adam Date: Tue, 16 Apr 2024 14:15:01 +0200 Subject: [PATCH] fixup! Fix issues reported by SonarCloud and add tests --- security/unittest/acltest.cpp | 247 ++++++++++++++++++++++++++-------- 1 file changed, 194 insertions(+), 53 deletions(-) diff --git a/security/unittest/acltest.cpp b/security/unittest/acltest.cpp index 3bc8569b4..84eddc4ba 100644 --- a/security/unittest/acltest.cpp +++ b/security/unittest/acltest.cpp @@ -19,6 +19,7 @@ #ifdef OC_SECURITY #include "api/oc_core_res_internal.h" +#include "api/oc_discovery_internal.h" #include "api/oc_ri_internal.h" #include "api/oc_runtime_internal.h" #include "oc_acl.h" @@ -46,6 +47,10 @@ #include "tests/gtest/Role.h" #endif /* OC_PKI */ +#ifdef OC_SOFTWARE_UPDATE +#include "api/oc_swupdate_internal.h" +#endif /* OC_SOFTWARE_UPDATE */ + #ifdef OC_HAS_FEATURE_PLGD_TIME #include "api/plgd/plgd_time_internal.h" #endif /* OC_HAS_FEATURE_PLGD_TIME */ @@ -55,6 +60,7 @@ #endif /* OC_HAS_FEATURE_PUSH */ #include +#include #include #include #include @@ -76,6 +82,7 @@ class TestAcl : public testing::Test { #if defined(OC_DYNAMIC_ALLOCATION) && defined(OC_PKI) g_root_keypair = oc::GetECPKeyPair(MBEDTLS_ECP_DP_SECP256R1); #endif /* OC_DYNAMIC_ALLOCATION && OC_PKI */ + oc_set_con_res_announced(true); } static void TearDownTestCase() { oc_random_destroy(); } @@ -94,6 +101,9 @@ class TestAcl : public testing::Test { plgd_time_create_resource(); #endif /* OC_HAS_FEATURE_PLGD_TIME */ oc_sec_svr_create(); +#ifdef OC_SOFTWARE_UPDATE + oc_swupdate_create(); +#endif /* OC_SOFTWARE_UPDATE */ oc_mbedtls_init(); @@ -122,16 +132,32 @@ class TestAcl : public testing::Test { oc_network_event_handler_mutex_destroy(); } - static std::vector getSVRs(size_t device) + template + static std::vector getResources(size_t device, Filter filter) { - std::vector svrs; - for (int i = OCF_SEC_DOXM; i < OCF_D; ++i) { + std::vector resources; + for (int i = OCF_P; i <= OCF_D; ++i) { oc_resource_t *resource = oc_core_get_resource_by_index(i, device); - if (oc_core_is_SVR(resource, device)) { - svrs.push_back(resource); + EXPECT_NE(nullptr, resource); + if (filter(device, resource)) { + resources.push_back(resource); } } - return svrs; + return resources; + } + + static std::vector getSVRs(size_t device) + { + return getResources(device, [](size_t device, oc_resource_t *resource) { + return oc_core_is_SVR(resource, device); + }); + } + + static std::vector getNonSVRs(size_t device) + { + return getResources(device, [](size_t device, oc_resource_t *resource) { + return !oc_core_is_SVR(resource, device); + }); } #if defined(OC_DYNAMIC_ALLOCATION) && defined(OC_PKI) @@ -171,8 +197,7 @@ TEST_F(TestAcl, oc_sec_acl_add_bootstrap_acl) TEST_F(TestAcl, oc_sec_acl_clear) { - oc_ace_subject_t anon_clear; - memset(&anon_clear, 0, sizeof(oc_ace_subject_t)); + oc_ace_subject_t anon_clear{}; anon_clear.conn = OC_CONN_ANON_CLEAR; EXPECT_EQ(true, oc_sec_ace_update_res(OC_SUBJECT_CONN, &anon_clear, -1, OC_PERM_RETRIEVE, nullptr, "/test/a", @@ -180,8 +205,7 @@ TEST_F(TestAcl, oc_sec_acl_clear) oc_uuid_t uuid{}; oc_gen_uuid(&uuid); - oc_ace_subject_t subject; - memset(&subject, 0, sizeof(oc_ace_subject_t)); + oc_ace_subject_t subject{}; memcpy(&subject.uuid, &uuid, sizeof(oc_uuid_t)); EXPECT_EQ(true, oc_sec_ace_update_res(OC_SUBJECT_UUID, &subject, -1, OC_PERM_UPDATE, nullptr, "/test/b", @@ -555,16 +579,27 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToRoles) #endif /* OC_PKI */ static void -assertUnauthorizedAccessToSVRResource(oc_resource_t *resource, size_t device, - oc_endpoint_t *ep) +assertUnauthorizedAccessToResource(const oc_resource_t *resource, size_t device, + const oc_endpoint_t *ep, bool isSVR) { - ASSERT_TRUE(oc_core_is_SVR(resource, device)); + ASSERT_EQ(isSVR, oc_core_is_SVR(resource, device)); ASSERT_FALSE(oc_sec_check_acl(OC_GET, resource, ep)); ASSERT_FALSE(oc_sec_check_acl(OC_POST, resource, ep)); ASSERT_FALSE(oc_sec_check_acl(OC_PUT, resource, ep)); ASSERT_FALSE(oc_sec_check_acl(OC_DELETE, resource, ep)); } +static void +checkAccessToResource(const oc_resource_t *resource, const oc_endpoint_t *ep, + bool allowGet = true, bool allowPost = true, + bool allowPut = true, bool allowDelete = true) +{ + EXPECT_EQ(allowGet, oc_sec_check_acl(OC_GET, resource, ep)); + EXPECT_EQ(allowPost, oc_sec_check_acl(OC_POST, resource, ep)); + EXPECT_EQ(allowPut, oc_sec_check_acl(OC_PUT, resource, ep)); + EXPECT_EQ(allowDelete, oc_sec_check_acl(OC_DELETE, resource, ep)); +} + TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRBySubject) { oc_uuid_t uuid{}; @@ -587,7 +622,7 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRBySubject) // we use doxm to represent all SVRs auto *doxm = oc_core_get_resource_by_index(OCF_SEC_DOXM, kDeviceID); ASSERT_NE(nullptr, doxm); - assertUnauthorizedAccessToSVRResource(doxm, kDeviceID, &ep); + assertUnauthorizedAccessToResource(doxm, kDeviceID, &ep, true); oc_ace_subject_t subject{}; memcpy(&subject.uuid, &uuid, sizeof(oc_uuid_t)); @@ -598,10 +633,7 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRBySubject) ASSERT_EQ(true, oc_sec_ace_update_res(OC_SUBJECT_UUID, &subject, -1, perm, nullptr, oc_string(doxm->uri), OC_ACE_NO_WC, kDeviceID, nullptr)); - EXPECT_TRUE(oc_sec_check_acl(OC_GET, doxm, &ep)); - EXPECT_FALSE(oc_sec_check_acl(OC_POST, doxm, &ep)); - EXPECT_FALSE(oc_sec_check_acl(OC_PUT, doxm, &ep)); - EXPECT_FALSE(oc_sec_check_acl(OC_DELETE, doxm, &ep)); + checkAccessToResource(doxm, &ep, true, false, false, false); EXPECT_FALSE(oc_sec_check_acl(OC_FETCH, doxm, &ep)); oc_sec_acl_clear(kDeviceID, nullptr, nullptr); } @@ -612,10 +644,7 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRBySubject) ASSERT_EQ(true, oc_sec_ace_update_res(OC_SUBJECT_UUID, &subject, -1, perm, nullptr, oc_string(doxm->uri), OC_ACE_NO_WC, kDeviceID, nullptr)); - EXPECT_FALSE(oc_sec_check_acl(OC_GET, doxm, &ep)); - EXPECT_TRUE(oc_sec_check_acl(OC_POST, doxm, &ep)); - EXPECT_TRUE(oc_sec_check_acl(OC_PUT, doxm, &ep)); - EXPECT_FALSE(oc_sec_check_acl(OC_DELETE, doxm, &ep)); + checkAccessToResource(doxm, &ep, false, true, true, false); EXPECT_FALSE(oc_sec_check_acl(OC_FETCH, doxm, &ep)); oc_sec_acl_clear(kDeviceID, nullptr, nullptr); } @@ -624,10 +653,7 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRBySubject) ASSERT_EQ(true, oc_sec_ace_update_res( OC_SUBJECT_UUID, &subject, -1, OC_PERM_DELETE, nullptr, oc_string(doxm->uri), OC_ACE_NO_WC, kDeviceID, nullptr)); - EXPECT_FALSE(oc_sec_check_acl(OC_GET, doxm, &ep)); - EXPECT_FALSE(oc_sec_check_acl(OC_POST, doxm, &ep)); - EXPECT_FALSE(oc_sec_check_acl(OC_PUT, doxm, &ep)); - EXPECT_TRUE(oc_sec_check_acl(OC_DELETE, doxm, &ep)); + checkAccessToResource(doxm, &ep, false, false, false, true); EXPECT_FALSE(oc_sec_check_acl(OC_FETCH, doxm, &ep)); oc_sec_acl_clear(kDeviceID, nullptr, nullptr); @@ -661,7 +687,7 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRByPSK) // we use doxm to represent all SVRs auto *doxm = oc_core_get_resource_by_index(OCF_SEC_DOXM, kDeviceID); ASSERT_NE(nullptr, doxm); - assertUnauthorizedAccessToSVRResource(doxm, kDeviceID, &ep); + assertUnauthorizedAccessToResource(doxm, kDeviceID, &ep, true); // add PSK cred std::vector pin{ '1', '2', '3' }; @@ -690,10 +716,7 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRByPSK) ASSERT_EQ(true, oc_sec_ace_update_res(OC_SUBJECT_ROLE, &subject, -1, perm, nullptr, oc_string(doxm->uri), OC_ACE_NO_WC, kDeviceID, nullptr)); - EXPECT_TRUE(oc_sec_check_acl(OC_GET, doxm, &ep)); - EXPECT_FALSE(oc_sec_check_acl(OC_POST, doxm, &ep)); - EXPECT_FALSE(oc_sec_check_acl(OC_PUT, doxm, &ep)); - EXPECT_FALSE(oc_sec_check_acl(OC_DELETE, doxm, &ep)); + checkAccessToResource(doxm, &ep, true, false, false, false); EXPECT_FALSE(oc_sec_check_acl(OC_FETCH, doxm, &ep)); oc_sec_acl_clear(kDeviceID, nullptr, nullptr); } @@ -706,10 +729,7 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRByPSK) ASSERT_EQ(true, oc_sec_ace_update_res(OC_SUBJECT_ROLE, &subject, -1, perm, nullptr, oc_string(doxm->uri), OC_ACE_NO_WC, kDeviceID, nullptr)); - EXPECT_FALSE(oc_sec_check_acl(OC_GET, doxm, &ep)); - EXPECT_TRUE(oc_sec_check_acl(OC_POST, doxm, &ep)); - EXPECT_TRUE(oc_sec_check_acl(OC_PUT, doxm, &ep)); - EXPECT_FALSE(oc_sec_check_acl(OC_DELETE, doxm, &ep)); + checkAccessToResource(doxm, &ep, false, true, true, false); EXPECT_FALSE(oc_sec_check_acl(OC_FETCH, doxm, &ep)); oc_sec_acl_clear(kDeviceID, nullptr, nullptr); } @@ -720,10 +740,7 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRByPSK) ASSERT_EQ(true, oc_sec_ace_update_res( OC_SUBJECT_ROLE, &subject, -1, OC_PERM_DELETE, nullptr, oc_string(doxm->uri), OC_ACE_NO_WC, kDeviceID, nullptr)); - EXPECT_FALSE(oc_sec_check_acl(OC_GET, doxm, &ep)); - EXPECT_FALSE(oc_sec_check_acl(OC_POST, doxm, &ep)); - EXPECT_FALSE(oc_sec_check_acl(OC_PUT, doxm, &ep)); - EXPECT_TRUE(oc_sec_check_acl(OC_DELETE, doxm, &ep)); + checkAccessToResource(doxm, &ep, false, false, false, true); EXPECT_FALSE(oc_sec_check_acl(OC_FETCH, doxm, &ep)); oc_sec_acl_clear(kDeviceID, nullptr, nullptr); @@ -756,7 +773,7 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRByOwnerRoleCred) // we use doxm to represent all SVRs auto *doxm = oc_core_get_resource_by_index(OCF_SEC_DOXM, kDeviceID); ASSERT_NE(nullptr, doxm); - assertUnauthorizedAccessToSVRResource(doxm, kDeviceID, &ep); + assertUnauthorizedAccessToResource(doxm, kDeviceID, &ep, true); std::array uuid_buf{}; ASSERT_TRUE( @@ -776,11 +793,7 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRByOwnerRoleCred) { nullptr, 0, OC_ENCODING_UNSUPPORTED }, publicdata, OC_STRING_VIEW_NULL, OC_STRING_VIEW_NULL, OC_STRING_VIEW_NULL, nullptr); ASSERT_NE(-1, credid); - - EXPECT_TRUE(oc_sec_check_acl(OC_GET, doxm, &ep)); - EXPECT_TRUE(oc_sec_check_acl(OC_POST, doxm, &ep)); - EXPECT_TRUE(oc_sec_check_acl(OC_PUT, doxm, &ep)); - EXPECT_TRUE(oc_sec_check_acl(OC_DELETE, doxm, &ep)); + checkAccessToResource(doxm, &ep); oc_tls_remove_peer(&ep); } @@ -808,7 +821,7 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRByNonOwnerRoleCred) // we use doxm to represent all SVRs auto *doxm = oc_core_get_resource_by_index(OCF_SEC_DOXM, kDeviceID); ASSERT_NE(nullptr, doxm); - assertUnauthorizedAccessToSVRResource(doxm, kDeviceID, &ep); + assertUnauthorizedAccessToResource(doxm, kDeviceID, &ep, true); std::array uuid_buf{}; ASSERT_TRUE( @@ -837,11 +850,7 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRByNonOwnerRoleCred) ASSERT_TRUE(oc_sec_ace_update_res(OC_SUBJECT_ROLE, &write, -1, OC_PERM_UPDATE, nullptr, oc_string(doxm->uri), OC_ACE_NO_WC, kDeviceID, nullptr)); - - EXPECT_FALSE(oc_sec_check_acl(OC_GET, doxm, &ep)); - EXPECT_TRUE(oc_sec_check_acl(OC_POST, doxm, &ep)); - EXPECT_TRUE(oc_sec_check_acl(OC_PUT, doxm, &ep)); - EXPECT_FALSE(oc_sec_check_acl(OC_DELETE, doxm, &ep)); + checkAccessToResource(doxm, &ep, false, true, true, false); oc_sec_acl_clear(kDeviceID, nullptr, nullptr); oc_tls_remove_peer(&ep); @@ -851,12 +860,144 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRByNonOwnerRoleCred) TEST_F(TestAcl, oc_sec_check_acl_AccessToNonSVRByCryptConn) { - // TODO + oc_endpoint_t ep = oc::endpoint::FromString("coaps://[ff02::41]:1336"); + ASSERT_NE(0, ep.flags & SECURED); + ep.device = kDeviceID; + + oc_sec_pstat_t *pstat = oc_sec_get_pstat(kDeviceID); + ASSERT_NE(nullptr, pstat); + pstat->s = OC_DOS_RFNOP; + + auto resources = + getResources(kDeviceID, [](size_t device, oc_resource_t *resource) { + oc_string_view_t uriv = oc_string_view2(&resource->uri); +#ifdef OC_HAS_FEATURE_PLGD_TIME + if (plgd_is_time_resource_uri(uriv)) { + return false; + } +#endif /* OC_HAS_FEATURE_PLGD_TIME */ +#ifdef OC_WKCORE + if (oc_is_wkcore_resource_uri(uriv)) { + return false; + } +#endif /* OC_WKCORE */ + return !oc_core_is_SVR(resource, device); + }); + for (auto res : resources) { + assertUnauthorizedAccessToResource(res, kDeviceID, &ep, false); + } + + auto setAnonConnPermission = [resources](oc_ace_permissions_t permission) { + for (auto res : resources) { + oc_ace_subject_t anon_crypt{}; + anon_crypt.conn = OC_CONN_AUTH_CRYPT; + if (!oc_sec_ace_update_res(OC_SUBJECT_CONN, &anon_crypt, -1, permission, + nullptr, oc_string(res->uri), OC_ACE_NO_WC, + kDeviceID, nullptr)) { + return false; + } + } + return true; + }; + + // allow delete access to all resources + ASSERT_TRUE(setAnonConnPermission(OC_PERM_DELETE)); + for (auto res : resources) { + checkAccessToResource(res, &ep, false, false, false, true); + } + + // allow update access to all resources + ASSERT_TRUE(setAnonConnPermission(OC_PERM_UPDATE)); + for (auto res : resources) { + checkAccessToResource(res, &ep, false, true, true, true); + } + + // allow retrieve access to all resources + ASSERT_TRUE(setAnonConnPermission(OC_PERM_RETRIEVE)); + for (auto res : resources) { + checkAccessToResource(res, &ep); + } + + // but using anon connection shouldn't work + oc_endpoint_t epAnon = oc::endpoint::FromString("coap://[ff02::41]:1336"); + ASSERT_EQ(0, epAnon.flags & SECURED); + epAnon.device = kDeviceID; + for (auto res : resources) { + checkAccessToResource(res, &epAnon, false, false, false, false); + } + + oc_sec_acl_clear(kDeviceID, nullptr, nullptr); } TEST_F(TestAcl, oc_sec_check_acl_AccessToNonSVRByAnonConn) { - // TODO + oc_endpoint_t ep = oc::endpoint::FromString("coap://[ff02::41]:1336"); + ASSERT_EQ(0, ep.flags & SECURED); + ep.device = kDeviceID; + + oc_sec_pstat_t *pstat = oc_sec_get_pstat(kDeviceID); + ASSERT_NE(nullptr, pstat); + pstat->s = OC_DOS_RFNOP; + + auto resources = + getResources(kDeviceID, [](size_t device, oc_resource_t *resource) { + oc_string_view_t uriv = oc_string_view2(&resource->uri); +#ifdef OC_HAS_FEATURE_PLGD_TIME + if (plgd_is_time_resource_uri(uriv)) { + return false; + } +#endif /* OC_HAS_FEATURE_PLGD_TIME */ +#ifdef OC_WKCORE + if (oc_is_wkcore_resource_uri(uriv)) { + return false; + } +#endif /* OC_WKCORE */ + return !oc_core_is_SVR(resource, device); + }); + for (auto res : resources) { + assertUnauthorizedAccessToResource(res, kDeviceID, &ep, false); + } + + auto setAnonConnPermission = [resources](oc_ace_permissions_t permission) { + for (auto res : resources) { + oc_ace_subject_t anon_clear{}; + anon_clear.conn = OC_CONN_ANON_CLEAR; + if (!oc_sec_ace_update_res(OC_SUBJECT_CONN, &anon_clear, -1, permission, + nullptr, oc_string(res->uri), OC_ACE_NO_WC, + kDeviceID, nullptr)) { + return false; + } + } + return true; + }; + + // allow retrieve access to all resources + ASSERT_TRUE(setAnonConnPermission(OC_PERM_RETRIEVE)); + for (auto res : resources) { + checkAccessToResource(res, &ep, true, false, false, false); + } + + // allow update access to all resources + ASSERT_TRUE(setAnonConnPermission(OC_PERM_UPDATE)); + for (auto res : resources) { + checkAccessToResource(res, &ep, true, true, true, false); + } + + // allow delete access to all resources + ASSERT_TRUE(setAnonConnPermission(OC_PERM_DELETE)); + for (auto res : resources) { + checkAccessToResource(res, &ep, true, true, true, true); + } + + // using secure connection should also work + oc_endpoint_t epCrypt = oc::endpoint::FromString("coaps://[ff02::41]:1336"); + ASSERT_NE(0, epCrypt.flags & SECURED); + epCrypt.device = kDeviceID; + for (auto res : resources) { + checkAccessToResource(res, &epCrypt); + } + + oc_sec_acl_clear(kDeviceID, nullptr, nullptr); } #endif /* OC_SECURITY */