From 4745cdf78f7de335b74b687ab1e431496e69963e Mon Sep 17 00:00:00 2001 From: Daniel Adam Date: Thu, 14 Apr 2022 13:52:05 +0200 Subject: [PATCH] Update oc_sec_apply_acl callback (#233) * security: add custom data tag to acl structure * Update oc_sec_apply_acl callback Provide additional data to oc_sec_on_apply_acl_cb_t callback to be able to distinguish between a created ace, a replaced ace and a duplicate ace. --- include/oc_acl.h | 60 ++++++- include/oc_cred.h | 10 +- security/oc_acl.c | 330 ++++++++++++++++++++++++------------ security/oc_cred.c | 23 ++- security/oc_cred_internal.h | 2 +- 5 files changed, 291 insertions(+), 134 deletions(-) diff --git a/include/oc_acl.h b/include/oc_acl.h index 2f48c58f37..8aa21cf078 100644 --- a/include/oc_acl.h +++ b/include/oc_acl.h @@ -125,6 +125,7 @@ typedef struct oc_sec_ace_t oc_ace_subject_t subject; ///< subject int aceid; ///< ACE identifier oc_ace_permissions_t permission; ///< permissions + oc_string_t tag; ///< custom user tag } oc_sec_ace_t; /** @@ -136,23 +137,68 @@ typedef struct oc_sec_ace_t OC_API oc_sec_acl_t *oc_sec_get_acl(size_t device); +/** + * @brief Remove access control entry from given device + * + * @param ace Access control entry to remove + * @param device Index of the device + */ +OC_API +void oc_sec_remove_ace(oc_sec_ace_t *ace, size_t device); + +/** + * @brief Get access control entry with given aceid from given device + * + * @param device Index of the device + * @return Access control list + */ +OC_API +oc_sec_ace_t *oc_sec_get_ace_by_aceid(int aceid, size_t device); + +/** + * @brief Remove access control entry with aceid from given device + * + * @param aceid Access control entry id + * @param device Index of the device + * @return true Access control entry with given id was found and removed + * @return false Otherwise + */ +OC_API +bool oc_sec_remove_ace_by_aceid(int aceid, size_t device); + /** * @brief Add initial access control list for core resources of a device * * @param device Index of the device + * @return true On success + * @return false On failure */ OC_API -void oc_sec_acl_add_bootstrap_acl(size_t device); +bool oc_sec_acl_add_bootstrap_acl(size_t device); + +typedef struct oc_sec_on_apply_acl_data_t +{ + oc_uuid_t rowneruuid; ///< Uuid of the resource owner + oc_sec_ace_t *ace; ///< New or updated access control entry + const oc_sec_ace_t + *replaced_ace; ///< In case of replacement of an existing ACE this is + ///< the original ACE that was replaced; the + ///< ACE will be deallocated after the call of + ///< oc_sec_on_apply_acl_cb_t from oc_sec_apply_acl + bool created; ///< True if a new ACE was created; False if the ACE + ///< replaced an already existing ACE or it was a + ///< duplicate and the operation was skipped + bool created_resource; ///< At least one new resource was created in the ACE +} oc_sec_on_apply_acl_data_t; /** * @brief Callback invoked with a created / updated access control entry * - * @param rowneruuid Uuid of the resource owner - * @param ace New or updated access control entry + * @param data Data with new/updated ACL data * @param user_data User data passed from the caller */ -typedef void (*oc_sec_on_apply_acl_cb_t)(oc_uuid_t rowneruuid, - oc_sec_ace_t *ace, void *user_data); +typedef void (*oc_sec_on_apply_acl_cb_t)(oc_sec_on_apply_acl_data_t data, + void *user_data); /** * @brief Parse payload and add/update access control list @@ -162,8 +208,8 @@ typedef void (*oc_sec_on_apply_acl_cb_t)(oc_uuid_t rowneruuid, * @param on_apply_ace_cb Callback invoked when a new access control entry is * added or updated * @param on_apply_ace_data User data passed to the on_apply_ace_cb function - * @return int -1 On failure - * @return int 0 Payload was successfully parsed + * @return -1 On failure + * @return 0 Payload was successfully parsed */ OC_API int oc_sec_apply_acl(oc_rep_t *rep, size_t device, diff --git a/include/oc_cred.h b/include/oc_cred.h index 79a61cb721..119ca7d9a8 100644 --- a/include/oc_cred.h +++ b/include/oc_cred.h @@ -121,7 +121,7 @@ typedef struct oc_sec_creds_t } oc_sec_creds_t; /** - * @brief read credential usaga + * @brief read credential usage * * @param credusage credential usage as type * @return const char* credential usage as string @@ -167,7 +167,7 @@ const char *oc_cred_credtype_string(oc_sec_credtype_t credtype); typedef struct oc_sec_on_apply_cred_data_t { - oc_sec_cred_t *cred; ///< New or updated credential + oc_sec_cred_t *cred; ///< new or updated credential const oc_sec_cred_t *replaced; ///< in case of modification of an existing credential this is ///< the original credential that has been replaced; the @@ -179,10 +179,10 @@ typedef struct oc_sec_on_apply_cred_data_t } oc_sec_on_apply_cred_data_t; /** - * @brief Callback invoked with a created / updated credential + * @brief callback invoked with a created / updated credential * - * @param data Data with new/updated credential data - * @param user_data User data passed from the caller + * @param data data with new/updated credential data + * @param user_data user data passed from the caller */ typedef void (*oc_sec_on_apply_cred_cb_t)(oc_sec_on_apply_cred_data_t data, void *user_data); diff --git a/security/oc_acl.c b/security/oc_acl.c index 7974745876..33cdabf5e2 100644 --- a/security/oc_acl.c +++ b/security/oc_acl.c @@ -661,6 +661,11 @@ oc_sec_encode_acl(size_t device, oc_interface_mask_t iface_mask, oc_rep_close_array(aclist2, resources); oc_rep_set_uint(aclist2, permission, sub->permission); oc_rep_set_int(aclist2, aceid, sub->aceid); + if (to_storage) { + if (oc_string_len(sub->tag) > 0) { + oc_rep_set_text_string(aclist2, tag, oc_string(sub->tag)); + } + } oc_rep_object_array_end_item(aclist2); sub = sub->next; } @@ -672,39 +677,15 @@ oc_sec_encode_acl(size_t device, oc_interface_mask_t iface_mask, return true; } -static oc_ace_res_t * -oc_sec_ace_get_res(oc_ace_subject_type_t type, oc_ace_subject_t *subject, - const char *href, oc_ace_wildcard_t wildcard, int aceid, - uint16_t permission, size_t device, bool create) +static oc_sec_ace_t * +oc_sec_add_new_ace(oc_ace_subject_type_t type, oc_ace_subject_t *subject, + int aceid, uint16_t permission, const char *tag, + size_t device) { - oc_sec_ace_t *ace = - oc_sec_acl_find_subject(NULL, type, subject, aceid, permission, device); - oc_ace_res_t *res = NULL; - - if (ace) { - goto got_ace; - } - - if (create) { - goto new_ace; - } - - goto done; - -got_ace: - res = oc_sec_ace_find_resource(NULL, ace, href, wildcard); - if (!res && create) { - goto new_res; - } - - goto done; - -new_ace: - ace = oc_memb_alloc(&ace_l); - + oc_sec_ace_t *ace = oc_memb_alloc(&ace_l); if (!ace) { OC_WRN("insufficient memory to add new ACE"); - goto done; + return NULL; } OC_LIST_STRUCT_INIT(ace, resources); @@ -744,55 +725,116 @@ oc_sec_ace_get_res(oc_ace_subject_type_t type, oc_ace_subject_t *subject, } ace->permission = permission; + if (tag) { + oc_new_string(&ace->tag, tag, strlen(tag)); + } oc_list_add(aclist[device].subjects, ace); + return ace; +} -new_res: - res = oc_memb_alloc(&res_l); - if (res) { - res->wildcard = 0; - if (wildcard != OC_ACE_NO_WC) { - res->wildcard = wildcard; - } +static oc_ace_res_t * +oc_sec_add_new_ace_res(const char *href, oc_ace_wildcard_t wildcard, + uint16_t permission) +{ + oc_ace_res_t *res = oc_memb_alloc(&res_l); + if (!res) { + OC_WRN("insufficient memory to add new resource to ACE"); + return NULL; + } + res->wildcard = 0; + if (wildcard != OC_ACE_NO_WC) { + res->wildcard = wildcard; + } #ifdef OC_DEBUG - switch (res->wildcard) { - case OC_ACE_WC_ALL_SECURED: - OC_DBG("Adding wildcard resource + with permission %d", permission); - break; - case OC_ACE_WC_ALL_PUBLIC: - OC_DBG("Adding wildcard resource - with permission %d", permission); - break; - case OC_ACE_WC_ALL: - OC_DBG("Adding wildcard resource * with permission %d", permission); - break; - default: - break; - } + switch (res->wildcard) { + case OC_ACE_WC_ALL_SECURED: + OC_DBG("Adding wildcard resource + with permission %d", permission); + break; + case OC_ACE_WC_ALL_PUBLIC: + OC_DBG("Adding wildcard resource - with permission %d", permission); + break; + case OC_ACE_WC_ALL: + OC_DBG("Adding wildcard resource * with permission %d", permission); + break; + default: + break; + } +#else /* !OC_DEBUG */ + (void)permission; #endif /* OC_DEBUG */ - if (href) { - oc_new_string(&res->href, href, strlen(href)); - OC_DBG("Adding resource %s with permission %d", href, permission); - } - - oc_list_add(ace->resources, res); - } else { - OC_WRN("insufficient memory to add new resource to ACE"); + if (href) { + oc_new_string(&res->href, href, strlen(href)); + OC_DBG("Adding resource %s with permission %d", href, permission); } - -done: return res; } +typedef struct oc_ace_res_data_t +{ + oc_ace_res_t *res; + bool created; +} oc_ace_res_data_t; + +static oc_ace_res_data_t +oc_sec_ace_get_res(oc_sec_ace_t *ace, const char *href, + oc_ace_wildcard_t wildcard, uint16_t permission, bool create) +{ + oc_assert(ace != NULL); + oc_ace_res_t *res = oc_sec_ace_find_resource(NULL, ace, href, wildcard); + if (res) { + oc_ace_res_data_t data = { res, false }; + return data; + } + if (create) { + res = oc_sec_add_new_ace_res(href, wildcard, permission); + } + if (!res) { + oc_ace_res_data_t data = { NULL, false }; + return data; + } + oc_list_add(ace->resources, res); + oc_ace_res_data_t data = { res, true }; + return data; +} + +typedef struct oc_sec_ace_update_data_t +{ + oc_sec_ace_t *ace; + bool created; + bool created_resource; +} oc_sec_ace_update_data_t; + static bool oc_sec_ace_update_res(oc_ace_subject_type_t type, oc_ace_subject_t *subject, - int aceid, uint16_t permission, const char *href, - oc_ace_wildcard_t wildcard, size_t device) + int aceid, uint16_t permission, const char *tag, + const char *href, oc_ace_wildcard_t wildcard, + size_t device, oc_sec_ace_update_data_t *data) { - if (oc_sec_ace_get_res(type, subject, href, wildcard, aceid, permission, - device, true)) - return true; - return false; + oc_sec_ace_t *ace = + oc_sec_acl_find_subject(NULL, type, subject, aceid, permission, device); + bool created = false; + if (!ace) { + ace = oc_sec_add_new_ace(type, subject, aceid, permission, tag, device); + if (!ace) { + return false; + } + created = true; + } + oc_ace_res_data_t res_data = + oc_sec_ace_get_res(ace, href, wildcard, permission, true); + if (res_data.res == NULL) { + oc_sec_remove_ace(ace, device); + return false; + } + + if (data) { + data->ace = ace; + data->created = created; + data->created_resource = res_data.created; + } + return true; } static void @@ -819,25 +861,62 @@ oc_ace_free_resources(size_t device, oc_sec_ace_t **ace, const char *href) } } -static bool -oc_acl_remove_ace(int aceid, size_t device) +static void +oc_acl_free_ace(oc_sec_ace_t *ace, size_t device) { - bool removed = false; - oc_sec_ace_t *ace = oc_list_head(aclist[device].subjects), *next = 0; + oc_ace_free_resources(device, &ace, NULL); + if (ace->subject_type == OC_SUBJECT_ROLE) { + oc_free_string(&ace->subject.role.role); + oc_free_string(&ace->subject.role.authority); + } + oc_free_string(&ace->tag); + oc_memb_free(&ace_l, ace); +} + +oc_sec_ace_t * +oc_sec_get_ace_by_aceid(int aceid, size_t device) +{ + oc_sec_ace_t *ace = oc_list_head(aclist[device].subjects); while (ace != NULL) { - next = ace->next; if (ace->aceid == aceid) { - oc_list_remove(aclist[device].subjects, ace); - oc_ace_free_resources(device, &ace, NULL); - if (ace->subject_type == OC_SUBJECT_ROLE) { - oc_free_string(&ace->subject.role.role); - oc_free_string(&ace->subject.role.authority); - } - oc_memb_free(&ace_l, ace); - removed = true; - break; + return ace; } - ace = next; + ace = ace->next; + } + return NULL; +} + +static oc_sec_ace_t * +oc_acl_remove_ace_from_device(oc_sec_ace_t *ace, size_t device) +{ + return oc_list_remove2(aclist[device].subjects, ace); +} + +static oc_sec_ace_t * +oc_acl_remove_ace_from_device_by_aceid(int aceid, size_t device) +{ + oc_sec_ace_t *ace = oc_sec_get_ace_by_aceid(aceid, device); + if (ace) { + return oc_acl_remove_ace_from_device(ace, device); + } + return false; +} + +void +oc_sec_remove_ace(oc_sec_ace_t *ace, size_t device) +{ + oc_acl_remove_ace_from_device(ace, device); + oc_acl_free_ace(ace, device); +} + +bool +oc_sec_remove_ace_by_aceid(int aceid, size_t device) +{ + bool removed = false; + oc_sec_ace_t *ace = oc_acl_remove_ace_from_device_by_aceid(aceid, device); + if (ace != NULL) { + oc_acl_free_ace(ace, device); + removed = true; } return removed; } @@ -848,12 +927,7 @@ oc_sec_clear_acl(size_t device) oc_sec_acl_t *acl_d = &aclist[device]; oc_sec_ace_t *ace = (oc_sec_ace_t *)oc_list_pop(acl_d->subjects); while (ace != NULL) { - oc_ace_free_resources(device, &ace, NULL); - if (ace->subject_type == OC_SUBJECT_ROLE) { - oc_free_string(&ace->subject.role.role); - oc_free_string(&ace->subject.role.authority); - } - oc_memb_free(&ace_l, ace); + oc_acl_free_ace(ace, device); ace = (oc_sec_ace_t *)oc_list_pop(acl_d->subjects); } } @@ -890,9 +964,8 @@ oc_sec_acl_add_created_resource_ace(const char *href, oc_endpoint_t *client, perm |= OC_PERM_CREATE; } - oc_sec_ace_update_res(OC_SUBJECT_UUID, &subject, -1, perm, href, 0, device); - - return true; + return oc_sec_ace_update_res(OC_SUBJECT_UUID, &subject, -1, perm, NULL, href, + 0, device, NULL); } #endif /* OC_COLLECTIONS && OC_SERVER && OC_COLLECTIONS_IF_CREATE */ @@ -952,6 +1025,7 @@ oc_sec_decode_acl(oc_rep_t *rep, bool from_storage, size_t device, oc_ace_subject_type_t subject_type = 0; uint16_t permission = 0; int aceid = -1; + char *tag = NULL; oc_rep_t *resources = 0; memset(&subject, 0, sizeof(oc_ace_subject_t)); oc_rep_t *ace = aclist2->value.object; @@ -967,6 +1041,12 @@ oc_sec_decode_acl(oc_rep_t *rep, bool from_storage, size_t device, aceid = (int)ace->value.integer; } break; + + case OC_REP_STRING: + if (len == 3 && memcmp(oc_string(ace->name), "tag", 3) == 0) { + tag = oc_string(ace->value.string); + } + break; case OC_REP_OBJECT_ARRAY: if (len == 9 && memcmp(oc_string(ace->name), "resources", 9) == 0) resources = ace->value.object_array; @@ -1011,8 +1091,12 @@ oc_sec_decode_acl(oc_rep_t *rep, bool from_storage, size_t device, ace = ace->next; } + oc_sec_ace_t *upd_ace = NULL; + oc_sec_ace_t *replaced_ace = NULL; + bool created = false; + bool created_resource = false; if (aceid != -1 && !unique_aceid(aceid, device)) { - oc_acl_remove_ace(aceid, device); + replaced_ace = oc_acl_remove_ace_from_device_by_aceid(aceid, device); } while (resources != NULL) { @@ -1066,8 +1150,16 @@ oc_sec_decode_acl(oc_rep_t *rep, bool from_storage, size_t device, resource = resource->next; } - oc_sec_ace_update_res(subject_type, &subject, aceid, permission, href, - wc, device); + oc_sec_ace_update_data_t ace_upd = { NULL, false, false }; + if (oc_sec_ace_update_res(subject_type, &subject, aceid, permission, + tag, href, wc, device, &ace_upd)) { + upd_ace = ace_upd.ace; + created |= ace_upd.created; + created_resource |= ace_upd.created_resource; + } else { + OC_WRN("failed to create resource(href:%s wildcard:%d)", + href != NULL ? href : "", wc); + } /* The following code block attaches "coap" endpoints to resources linked to an anon-clear ACE. This logic is being @@ -1098,14 +1190,18 @@ oc_sec_decode_acl(oc_rep_t *rep, bool from_storage, size_t device, } if (on_apply_ace_cb != NULL) { - oc_sec_ace_t *ace = oc_sec_acl_find_subject( - NULL, subject_type, &subject, aceid, permission, device); - - if (ace != NULL) { - on_apply_ace_cb(aclist[device].rowneruuid, ace, on_apply_ace_data); + if (upd_ace != NULL) { + oc_sec_on_apply_acl_data_t acl_data = { aclist[device].rowneruuid, + upd_ace, replaced_ace, + created, created_resource }; + on_apply_ace_cb(acl_data, on_apply_ace_data); } } + if (replaced_ace) { + oc_acl_free_ace(replaced_ace, device); + } + if (subject_type == OC_SUBJECT_ROLE) { oc_free_string(&subject.role.role); oc_free_string(&subject.role.authority); @@ -1122,23 +1218,41 @@ oc_sec_decode_acl(oc_rep_t *rep, bool from_storage, size_t device, return true; } -void +bool oc_sec_acl_add_bootstrap_acl(size_t device) { oc_ace_subject_t _anon_clear; memset(&_anon_clear, 0, sizeof(oc_ace_subject_t)); _anon_clear.conn = OC_CONN_ANON_CLEAR; - oc_sec_ace_update_res(OC_SUBJECT_CONN, &_anon_clear, -1, OC_PERM_RETRIEVE, - "/oic/res", OC_ACE_NO_WC, device); - oc_sec_ace_update_res(OC_SUBJECT_CONN, &_anon_clear, -1, OC_PERM_RETRIEVE, - "/oic/d", OC_ACE_NO_WC, device); - oc_sec_ace_update_res(OC_SUBJECT_CONN, &_anon_clear, -1, OC_PERM_RETRIEVE, - "/oic/p", OC_ACE_NO_WC, device); + bool ret = true; + if (!oc_sec_ace_update_res(OC_SUBJECT_CONN, &_anon_clear, -1, + OC_PERM_RETRIEVE, NULL, "/oic/res", OC_ACE_NO_WC, + device, NULL)) { + OC_ERR("oc_acl: Failed to bootstrap %s resource", "/oic/res"); + ret = false; + } + if (!oc_sec_ace_update_res(OC_SUBJECT_CONN, &_anon_clear, -1, + OC_PERM_RETRIEVE, NULL, "/oic/d", OC_ACE_NO_WC, + device, NULL)) { + OC_ERR("oc_acl: Failed to bootstrap %s resource", "/oic/d"); + ret = false; + } + if (!oc_sec_ace_update_res(OC_SUBJECT_CONN, &_anon_clear, -1, + OC_PERM_RETRIEVE, NULL, "/oic/p", OC_ACE_NO_WC, + device, NULL)) { + OC_ERR("oc_acl: Failed to bootstrap %s resource", "/oic/p"); + ret = false; + } #ifdef OC_WKCORE - oc_sec_ace_update_res(OC_SUBJECT_CONN, &_anon_clear, -1, OC_PERM_RETRIEVE, - "/.well-known/core", OC_ACE_NO_WC, device); + if (!oc_sec_ace_update_res(OC_SUBJECT_CONN, &_anon_clear, -1, + OC_PERM_RETRIEVE, NULL, "/.well-known/core", + OC_ACE_NO_WC, device, NULL)) { + OC_ERR("oc_acl: Failed to bootstrap %s resource", "/.well-known/core"); + ret = false; + } #endif + return ret; } int @@ -1187,7 +1301,7 @@ delete_acl(oc_request_t *request, oc_interface_mask_t iface_mask, void *data) if (ret != -1) { aceid = (int)strtoul(query_param, NULL, 10); if (aceid != 0) { - if (oc_acl_remove_ace(aceid, request->resource->device)) { + if (oc_sec_remove_ace_by_aceid(aceid, request->resource->device)) { success = true; } } diff --git a/security/oc_cred.c b/security/oc_cred.c index cb80b3afeb..1d919e8ac8 100644 --- a/security/oc_cred.c +++ b/security/oc_cred.c @@ -96,8 +96,9 @@ oc_sec_get_cred_by_credid(int credid, size_t device) { oc_sec_cred_t *cred = oc_list_head(devices[device].creds); while (cred != NULL) { - if (cred->credid == credid) + if (cred->credid == credid) { return cred; + } cred = cred->next; } return NULL; @@ -213,8 +214,7 @@ get_new_credid(bool roles_resource, oc_tls_peer_t *client, size_t device) static oc_sec_cred_t * oc_sec_remove_cred_from_device(oc_sec_cred_t *cred, size_t device) { - oc_list_remove(devices[device].creds, cred); - return cred; + return oc_list_remove2(devices[device].creds, cred); } static oc_sec_cred_t * @@ -277,13 +277,10 @@ oc_sec_remove_cred(oc_sec_cred_t *cred, size_t device) bool oc_sec_remove_cred_by_credid(int credid, size_t device) { - oc_sec_cred_t *cred = oc_list_head(devices[device].creds); - while (cred != NULL) { - if (cred->credid == credid) { - oc_sec_remove_cred(cred, device); - return true; - } - cred = cred->next; + oc_sec_cred_t *cred = oc_sec_get_cred_by_credid(credid, device); + if (cred != NULL) { + oc_sec_remove_cred(cred, device); + return true; } return false; } @@ -1125,7 +1122,7 @@ dump_cred(void *data) bool oc_sec_decode_cred(oc_rep_t *rep, oc_sec_cred_t **owner, bool from_storage, bool roles_resource, oc_tls_peer_t *client, size_t device, - oc_sec_on_apply_cred_cb_t oc_apply_cred_cb, + oc_sec_on_apply_cred_cb_t on_apply_cred_cb, void *on_apply_cred_data) { oc_sec_pstat_t *ps = oc_sec_get_pstat(device); @@ -1425,13 +1422,13 @@ oc_sec_decode_cred(oc_rep_t *rep, oc_sec_cred_t **owner, bool from_storage, *owner = cr; (*owner)->owner_cred = true; } - if (oc_apply_cred_cb) { + if (on_apply_cred_cb) { oc_sec_on_apply_cred_data_t cred_data = { cr, add_cred_data.replaced_cred, add_cred_data.created, }; - oc_apply_cred_cb(cred_data, on_apply_cred_data); + on_apply_cred_cb(cred_data, on_apply_cred_data); } } if (add_cred_data.replaced_cred) { diff --git a/security/oc_cred_internal.h b/security/oc_cred_internal.h index 460edbe60c..3e61859e95 100644 --- a/security/oc_cred_internal.h +++ b/security/oc_cred_internal.h @@ -54,7 +54,7 @@ void oc_sec_encode_cred(bool persist, size_t device, bool oc_sec_decode_cred(oc_rep_t *rep, oc_sec_cred_t **owner, bool from_storage, bool roles_resource, struct oc_tls_peer_t *client, size_t device, - oc_sec_on_apply_cred_cb_t oc_apply_cred_cb, + oc_sec_on_apply_cred_cb_t on_apply_cred_cb, void *on_apply_cred_data); /** * @brief Allocate and initialize a new credential and append it to global list