Skip to content

Commit

Permalink
fixup! Fix issues reported by SonarCloud and add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@Danielius1922
Danielius1922 committed Apr 15, 2024
1 parent 3c0f3b0 commit 4ed7413
Show file tree
Hide file tree
Showing 6 changed files with 211 additions and 21 deletions.
4 changes: 4 additions & 0 deletions api/oc_enums_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
extern "C" {
#endif

#define OC_PERM_ALL \
(OC_PERM_CREATE | OC_PERM_RETRIEVE | OC_PERM_UPDATE | OC_PERM_DELETE | \
OC_PERM_NOTIFY)

/** @brief Convert enum value to string view */
oc_string_view_t oc_enum_to_string_view(oc_enum_t val);

Expand Down
15 changes: 8 additions & 7 deletions security/oc_acl_util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#include "api/oc_core_res_internal.h"
#include "api/oc_discovery_internal.h"
#include "api/oc_enums_internal.h"
#include "api/oc_platform_internal.h"
#include "oc_helpers.h"
#include "oc_uuid.h"
Expand Down Expand Up @@ -294,7 +295,7 @@ get_peer_permissions(const oc_resource_t *resource, bool is_DCR, bool is_public,
if (role_cred == NULL) {
break;
}
if (oc_string_len(role_cred->role.role) > 0) {
if (!oc_string_is_empty(&role_cred->role.role)) {
permission |= get_role_permissions(role_cred, resource,
endpoint->device, is_DCR, is_public);
}
Expand All @@ -304,7 +305,7 @@ get_peer_permissions(const oc_resource_t *resource, bool is_DCR, bool is_public,
#ifdef OC_PKI
else {
const oc_sec_cred_t *role_cred = oc_sec_roles_get(peer);
while (role_cred) {
while (role_cred != NULL) {
const oc_sec_cred_t *next = role_cred->next;
uint32_t flags = 0;
if (oc_certs_validate_role_cert(role_cred->ctx, &flags) < 0 ||
Expand All @@ -313,11 +314,11 @@ get_peer_permissions(const oc_resource_t *resource, bool is_DCR, bool is_public,
role_cred = next;
continue;
}
if (oc_string_len(role_cred->role.role) == strlen("oic.role.owner") &&
memcmp(oc_string(role_cred->role.role), "oic.role.owner",
oc_string_len(role_cred->role.role)) == 0) {
OC_DBG("oc_acl: peer's role matches \"oic.role.owner\"");
return true;
oc_string_view_t ownerv = OC_STRING_VIEW(OCF_SEC_ROLE_OWNER);
if (oc_string_view_is_equal(oc_string_view2(&role_cred->role.role),
ownerv)) {
OC_DBG("oc_acl: peer's role matches \"%s\"", OCF_SEC_ROLE_OWNER);
return OC_PERM_ALL;
}
permission |= get_role_permissions(role_cred, resource, endpoint->device,
is_DCR, is_public);
Expand Down
4 changes: 2 additions & 2 deletions security/oc_cred.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ static oc_sec_creds_t g_devices[OC_MAX_NUM_DEVICES];

#ifdef OC_PKI
static oc_string_view_t g_allowed_roles[] = { {
.data = "oic.role.owner",
.length = OC_CHAR_ARRAY_LEN("oic.role.owner"),
.data = OCF_SEC_ROLE_OWNER,
.length = OC_CHAR_ARRAY_LEN(OCF_SEC_ROLE_OWNER),
} };
#endif /* OC_PKI */

Expand Down
2 changes: 2 additions & 0 deletions security/oc_roles_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ enum {
OCF_SEC_ROLES_MAX_NUM = 2,
};

#define OCF_SEC_ROLE_OWNER "oic.role.owner"

/**
* \defgroup server-roles Event timers
*
Expand Down
205 changes: 194 additions & 11 deletions security/unittest/acltest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@
#include "tests/gtest/tls/Peer.h"
#include "util/oc_list.h"

#ifdef OC_PKI
#include "security/oc_certs_internal.h"
#include "security/oc_obt_internal.h"
#include "security/oc_roles_internal.h"
#include "tests/gtest/KeyPair.h"
#include "tests/gtest/Role.h"
#endif /* OC_PKI */

#ifdef OC_HAS_FEATURE_PLGD_TIME
#include "api/plgd/plgd_time_internal.h"
#endif /* OC_HAS_FEATURE_PLGD_TIME */
Expand Down Expand Up @@ -514,6 +522,78 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToRoles)

#endif /* OC_PKI */

TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRBySubject)
{
oc_uuid_t uuid{};
oc_gen_uuid(&uuid);
auto tlsPeer =
oc::tls::MakePeer("coaps://[ff02::41]:1336", MBEDTLS_SSL_IS_CLIENT);
oc_endpoint_t ep = oc::endpoint::FromString(tlsPeer.address);
ASSERT_NE(0, ep.flags & SECURED);
ep.device = kDeviceID;
ep.di = uuid;

oc_tls_peer_t *peer = oc_tls_add_or_get_peer(&ep, tlsPeer.role, nullptr);
ASSERT_NE(nullptr, peer);
ASSERT_EQ(1, oc_tls_num_peers(kDeviceID));

oc_sec_pstat_t *pstat = oc_sec_get_pstat(kDeviceID);
ASSERT_NE(nullptr, pstat);
pstat->s = OC_DOS_RFOTM;

// we use doxm to represent all SVRs
auto *doxm = oc_core_get_resource_by_index(OCF_SEC_DOXM, kDeviceID);
ASSERT_NE(nullptr, doxm);
ASSERT_TRUE(oc_core_is_SVR(doxm, kDeviceID));
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_FALSE(oc_sec_check_acl(OC_DELETE, doxm, &ep));
EXPECT_FALSE(oc_sec_check_acl(OC_FETCH, doxm, &ep));

oc_ace_subject_t subject{};
memcpy(&subject.uuid, &uuid, sizeof(oc_uuid_t));

// allowing retrieve or notify should allow GET access
for (auto perm : std::vector<oc_ace_permissions_t>(
{ OC_PERM_RETRIEVE, OC_PERM_NOTIFY })) {
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));
EXPECT_FALSE(oc_sec_check_acl(OC_FETCH, doxm, &ep));
oc_sec_acl_clear(kDeviceID, nullptr, nullptr);
}

// allowing create or update should allow POST and PUT access
for (auto perm :
std::vector<oc_ace_permissions_t>({ OC_PERM_CREATE, OC_PERM_UPDATE })) {
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));
EXPECT_FALSE(oc_sec_check_acl(OC_FETCH, doxm, &ep));
oc_sec_acl_clear(kDeviceID, nullptr, nullptr);
}

// allowing delete should allow DELETE access
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));
EXPECT_FALSE(oc_sec_check_acl(OC_FETCH, doxm, &ep));
oc_sec_acl_clear(kDeviceID, nullptr, nullptr);
}

TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRByPSK)
{
oc_uuid_t uuid{};
Expand All @@ -538,11 +618,15 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRByPSK)
ASSERT_NE(nullptr, pstat);
pstat->s = OC_DOS_RFOTM;

auto svrs = getSVRs(kDeviceID);
for (auto svr : svrs) {
ASSERT_TRUE(oc_core_is_SVR(svr, kDeviceID));
EXPECT_FALSE(oc_sec_check_acl(OC_GET, svr, &ep));
}
// we use doxm to represent all SVRs
auto *doxm = oc_core_get_resource_by_index(OCF_SEC_DOXM, kDeviceID);
ASSERT_NE(nullptr, doxm);
ASSERT_TRUE(oc_core_is_SVR(doxm, kDeviceID));
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_FALSE(oc_sec_check_acl(OC_DELETE, doxm, &ep));
EXPECT_FALSE(oc_sec_check_acl(OC_FETCH, doxm, &ep));

// add PSK cred
std::vector<unsigned char> pin{ '1', '2', '3' };
Expand All @@ -566,8 +650,6 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRByPSK)
// allowing retrieve or notify should allow GET access
for (auto perm : std::vector<oc_ace_permissions_t>(
{ OC_PERM_RETRIEVE, OC_PERM_NOTIFY })) {
auto *doxm = oc_core_get_resource_by_index(OCF_SEC_DOXM, kDeviceID);
ASSERT_NE(nullptr, doxm);
oc_ace_subject_t subject{};
subject.role = { role, authority };
ASSERT_EQ(true, oc_sec_ace_update_res(OC_SUBJECT_ROLE, &subject, -1, perm,
Expand All @@ -584,8 +666,6 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRByPSK)
// allowing create or update should allow POST and PUT access
for (auto perm :
std::vector<oc_ace_permissions_t>({ OC_PERM_CREATE, OC_PERM_UPDATE })) {
auto *doxm = oc_core_get_resource_by_index(OCF_SEC_DOXM, kDeviceID);
ASSERT_NE(nullptr, doxm);
oc_ace_subject_t subject{};
subject.role = { role, authority };
ASSERT_EQ(true, oc_sec_ace_update_res(OC_SUBJECT_ROLE, &subject, -1, perm,
Expand All @@ -600,8 +680,6 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRByPSK)
}

// allowing delete should allow DELETE access
auto *doxm = oc_core_get_resource_by_index(OCF_SEC_DOXM, kDeviceID);
ASSERT_NE(nullptr, doxm);
oc_ace_subject_t subject{};
subject.role = { role, authority };
ASSERT_EQ(true, oc_sec_ace_update_res(
Expand All @@ -618,4 +696,109 @@ TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRByPSK)
oc_tls_remove_peer(&ep);
}

#if defined(OC_DYNAMIC_ALLOCATION) && defined(OC_PKI)

static std::vector<unsigned char>
getPEM(std::vector<unsigned char> &data)
{
auto it =
std::find(data.begin(), data.end(), static_cast<unsigned char>('\0'));
size_t data_len =
std::distance(data.begin(), it) + 1; // size with NULL terminator
if (data.end() == it || !oc_certs_is_PEM(&data[0], data_len)) {
return {};
}
data.resize(data_len);
return data;
}

TEST_F(TestAcl, oc_sec_check_acl_AccessToSVRByOwnerRoleCred)
{
oc_uuid_t uuid{};
oc_gen_uuid(&uuid);
auto tlsPeer =
oc::tls::MakePeer("coaps://[ff02::41]:1336", MBEDTLS_SSL_IS_CLIENT);
oc_endpoint_t ep = oc::endpoint::FromString(tlsPeer.address);
ASSERT_NE(0, ep.flags & SECURED);
ep.device = kDeviceID;
ep.di = uuid;

oc_tls_peer_t *peer = oc_tls_add_or_get_peer(&ep, tlsPeer.role, nullptr);
ASSERT_NE(nullptr, peer);
peer->uuid = uuid;
ASSERT_EQ(1, oc_tls_num_peers(kDeviceID));

oc_sec_pstat_t *pstat = oc_sec_get_pstat(kDeviceID);
ASSERT_NE(nullptr, pstat);
pstat->s = OC_DOS_RFOTM;

// we use doxm to represent all SVRs
auto *doxm = oc_core_get_resource_by_index(OCF_SEC_DOXM, kDeviceID);
ASSERT_NE(nullptr, doxm);
ASSERT_TRUE(oc_core_is_SVR(doxm, kDeviceID));
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_FALSE(oc_sec_check_acl(OC_DELETE, doxm, &ep));
EXPECT_FALSE(oc_sec_check_acl(OC_FETCH, doxm, &ep));

std::string g_root_subject_name{ "IoTivity-Lite Test" };
std::string g_root_subject{ "C=US, O=OCF, CN=" + g_root_subject_name };
oc::keypair_t g_root_keypair{};

g_root_keypair = oc::GetECPKeyPair(MBEDTLS_ECP_DP_SECP256R1);
oc_obt_generate_root_cert_data_t root_cert_data = {
/*.subject_name = */ g_root_subject.c_str(),
/*.public_key =*/g_root_keypair.public_key.data(),
/*.public_key_size =*/g_root_keypair.public_key_size,
/*.private_key =*/g_root_keypair.private_key.data(),
/*.private_key_size =*/g_root_keypair.private_key_size,
/*.signature_md_alg=*/MBEDTLS_MD_SHA256,
};
int g_root_credid =
oc_obt_generate_self_signed_root_cert(root_cert_data, kDeviceID);
ASSERT_GT(g_root_credid, 0);

std::array<char, 50> uuid_buf{};
ASSERT_TRUE(
oc_certs_encode_CN_with_UUID(&uuid, uuid_buf.data(), uuid_buf.size()));

oc::Roles roles{};
roles.Add(OCF_SEC_ROLE_OWNER, "owner");

oc_obt_generate_role_cert_data_t role_cert_data = {
/*.roles =*/roles.Head(),
/*.subject_name =*/uuid_buf.data(),
/*.public_key =*/g_root_keypair.public_key.data(),
/*.public_key_size =*/g_root_keypair.public_key_size,
/*.issuer_name =*/g_root_subject.c_str(),
/*.issuer_private_key =*/g_root_keypair.private_key.data(),
/*.issuer_private_key_size =*/g_root_keypair.private_key_size,
/*.signature_md_alg=*/MBEDTLS_MD_SHA256,
};
std::vector<unsigned char> role_buf{};
role_buf.resize(4096, '\0');
ASSERT_EQ(0, oc_obt_generate_role_cert_pem(role_cert_data, role_buf.data(),
role_buf.size()));
auto role_pem = getPEM(role_buf);
ASSERT_FALSE(role_pem.empty());

oc_sec_encoded_data_t publicdata = { role_pem.data(), role_pem.size() - 1,
OC_ENCODING_PEM };
int credid = oc_sec_add_new_cred(
kDeviceID, true, peer, -1, OC_CREDTYPE_CERT, OC_CREDUSAGE_ROLE_CERT, "*",
{ 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));

oc_tls_remove_peer(&ep);
}

#endif /* OC_DYNAMIC_ALLOCATION && OC_PKI */

#endif /* OC_SECURITY */
2 changes: 1 addition & 1 deletion security/unittest/rolestest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ TEST_F(TestRolesWithServer, AddRole_FailAssertion)
TEST_F(TestRolesWithServer, AddRole_AssertAllowed)
{
oc::Roles roles{};
roles.Add("oic.role.owner");
roles.Add(OCF_SEC_ROLE_OWNER);

oc_endpoint_t ep = oc::endpoint::FromString("coaps://[::1]:42");
const auto *peer = addPeer(&ep);
Expand Down

0 comments on commit 4ed7413

Please sign in to comment.