From patchwork Fri Oct 4 11:41:14 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aleksandr Smirnov X-Patchwork-Id: 1992706 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=k2.cloud header.i=@k2.cloud header.a=rsa-sha256 header.s=cloudmail header.b=lXIBvgXI; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4XKn096Fjsz1xsn for ; Fri, 4 Oct 2024 21:49:44 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 0D1D940610; Fri, 4 Oct 2024 11:49:42 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id xDgWWWqfGdaF; Fri, 4 Oct 2024 11:49:40 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=2605:bc80:3010:104::8cd3:938; helo=lists.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 1DF794043C Authentication-Results: smtp2.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=k2.cloud header.i=@k2.cloud header.a=rsa-sha256 header.s=cloudmail header.b=lXIBvgXI Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTPS id 1DF794043C; Fri, 4 Oct 2024 11:49:40 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id CEA4EC08A6; Fri, 4 Oct 2024 11:49:39 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 6DB92C08A3 for ; Fri, 4 Oct 2024 11:49:38 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 485A3406CE for ; Fri, 4 Oct 2024 11:49:38 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id UC2ZRxlJ8Qa7 for ; Fri, 4 Oct 2024 11:49:37 +0000 (UTC) X-Greylist: delayed 493 seconds by postgrey-1.37 at util1.osuosl.org; Fri, 04 Oct 2024 11:49:35 UTC DMARC-Filter: OpenDMARC Filter v1.4.2 smtp4.osuosl.org 2E9DD406CC Authentication-Results: smtp4.osuosl.org; dmarc=pass (p=none dis=none) header.from=k2.cloud DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 2E9DD406CC Authentication-Results: smtp4.osuosl.org; dkim=pass (1024-bit key, unprotected) header.d=k2.cloud header.i=@k2.cloud header.a=rsa-sha256 header.s=cloudmail header.b=lXIBvgXI Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=109.73.14.252; helo=mail1.k2.cloud; envelope-from=alekssmirnov@k2.cloud; receiver= Received: from mail1.k2.cloud (mail1.k2.cloud [109.73.14.252]) by smtp4.osuosl.org (Postfix) with ESMTPS id 2E9DD406CC for ; Fri, 4 Oct 2024 11:49:34 +0000 (UTC) From: Aleksandr Smirnov DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=k2.cloud; s=cloudmail; t=1728042077; bh=ffEnYQqznTxqwHwnWpIrnmJyAFL5bqWq8CWPhaCOc9k=; h=From:To:Cc:Subject:Date; b=lXIBvgXIrUq0aV+KZXxpGd7TM+6fAs/3Mc9tha7j8p1A8CwkQUzVfZIZBcc26mlys DwogQBhCNqubMgGXHK4KV65QfExB/bVMo6UFwepLFMkEqXMTODxGVTnCvaGeXILBMU nFuk1feB3hWEh17+9SPSCDOMqAyBsaMz3TqJRe88= To: dev@openvswitch.org Date: Fri, 4 Oct 2024 14:41:14 +0300 Message-Id: <20241004114115.597041-1-alekssmirnov@k2.cloud> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn] northd: Fix issues in RBAC tables recovery. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Aleksandr Gnatyuk Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Aleksandr Smirnov Northd creates hardcoded RBAC role 'ovn-controller' with number of predefined permissions. Then it watches for alternations of the role and permissions and recover them if they were changed. An original code have issues that prevents an user to create any other roles. Also, builtin permissions are not fully protected against modifications. The fix reworks this code to recover builtin permissions properly after all possible modification scenarios. Also, creation of custom roles and permissions becomes available. Signed-off-by: Aleksandr Smirnov Tested-by: Aleksandr Gnatyuk --- northd/ovn-northd.c | 120 ++++++++++++++++++++++++++++++-------------- tests/ovn-northd.at | 80 +++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+), 37 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index d71114f35..8978e674e 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -277,20 +277,16 @@ static struct gen_opts_map supported_dhcpv6_opts[] = { DHCPV6_OPT_FQDN, }; +/* + * Compare predefined permission against RBAC_Permission record. + * Returns true if match, false otherwise. + */ static bool -ovn_rbac_validate_perm(const struct sbrec_rbac_permission *perm) +ovn_rbac_match_perm(const struct sbrec_rbac_permission *perm, + const struct rbac_perm_cfg *pcfg) { - struct rbac_perm_cfg *pcfg; int i, j, n_found; - for (pcfg = rbac_perm_cfg; pcfg->table; pcfg++) { - if (!strcmp(perm->table, pcfg->table)) { - break; - } - } - if (!pcfg->table) { - return false; - } if (perm->n_authorization != pcfg->n_auth || perm->n_update != pcfg->n_update) { return false; @@ -326,54 +322,74 @@ ovn_rbac_validate_perm(const struct sbrec_rbac_permission *perm) return false; } - /* Success, db state matches expected state */ - pcfg->row = perm; return true; } +/* + * Search predefined permission pcfg in the RBAC_Permission. + * If there is no record that match, recover the permission. + */ static void -ovn_rbac_create_perm(struct rbac_perm_cfg *pcfg, - struct ovsdb_idl_txn *ovnsb_txn, - const struct sbrec_rbac_role *rbac_role) +ovn_rbac_validate_perm(struct rbac_perm_cfg *pcfg, + struct ovsdb_idl_txn *ovnsb_txn, + struct ovsdb_idl *ovnsb_idl) { - struct sbrec_rbac_permission *rbac_perm; + const struct sbrec_rbac_permission *perm_row; - rbac_perm = sbrec_rbac_permission_insert(ovnsb_txn); - sbrec_rbac_permission_set_table(rbac_perm, pcfg->table); - sbrec_rbac_permission_set_authorization(rbac_perm, + SBREC_RBAC_PERMISSION_FOR_EACH_SAFE (perm_row, ovnsb_idl) { + if (!strcmp(perm_row->table, pcfg->table) + && ovn_rbac_match_perm(perm_row, pcfg)) { + pcfg->row = perm_row; + + return; + } + } + + pcfg->row = sbrec_rbac_permission_insert(ovnsb_txn); + sbrec_rbac_permission_set_table(pcfg->row, pcfg->table); + sbrec_rbac_permission_set_authorization(pcfg->row, pcfg->auth, pcfg->n_auth); - sbrec_rbac_permission_set_insert_delete(rbac_perm, pcfg->insdel); - sbrec_rbac_permission_set_update(rbac_perm, + sbrec_rbac_permission_set_insert_delete(pcfg->row, pcfg->insdel); + sbrec_rbac_permission_set_update(pcfg->row, pcfg->update, pcfg->n_update); - sbrec_rbac_role_update_permissions_setkey(rbac_role, pcfg->table, - rbac_perm); } +/* + * Make sure that DB Role 'ovn-controller' exists, has no duplicates + * permission list exactly match to predefined permissions. Recreate if + * matching fails. + */ static void check_and_update_rbac(struct ovsdb_idl_txn *ovnsb_txn, struct ovsdb_idl *ovnsb_idl) { const struct sbrec_rbac_role *rbac_role = NULL; - const struct sbrec_rbac_permission *perm_row; const struct sbrec_rbac_role *role_row; struct rbac_perm_cfg *pcfg; + bool rebuild_role_perm = false; + int pcfg_len, i; + /* + * Make sure predefined permissions are presented in the RBAC_Permissions + * table. Otherwise create consistent permissions. + */ for (pcfg = rbac_perm_cfg; pcfg->table; pcfg++) { - pcfg->row = NULL; + ovn_rbac_validate_perm(pcfg, ovnsb_txn, ovnsb_idl); } - SBREC_RBAC_PERMISSION_FOR_EACH_SAFE (perm_row, ovnsb_idl) { - if (!ovn_rbac_validate_perm(perm_row)) { - sbrec_rbac_permission_delete(perm_row); - } - } + /* + * Make sure the role 'ovn-controller' is presented in the RBAC_Role table. + * Otherwise create the role. Remove duplicates if any. + */ SBREC_RBAC_ROLE_FOR_EACH_SAFE (role_row, ovnsb_idl) { - if (strcmp(role_row->name, "ovn-controller")) { - sbrec_rbac_role_delete(role_row); - } else { - rbac_role = role_row; + if (!strcmp(role_row->name, "ovn-controller")) { + if (rbac_role) { + sbrec_rbac_role_delete(role_row); + } else { + rbac_role = role_row; + } } } @@ -382,9 +398,39 @@ check_and_update_rbac(struct ovsdb_idl_txn *ovnsb_txn, sbrec_rbac_role_set_name(rbac_role, "ovn-controller"); } - for (pcfg = rbac_perm_cfg; pcfg->table; pcfg++) { - if (!pcfg->row) { - ovn_rbac_create_perm(pcfg, ovnsb_txn, rbac_role); + /* + * Make sure a permission list attached to the role 'ovn-controller' + * exactly matches to predefined permissions. + * Reassign permission list to the role if any difference has found. + */ + for (pcfg = rbac_perm_cfg, pcfg_len = 0; pcfg->table; pcfg++, pcfg_len++) { + for (i = 0; i < rbac_role->n_permissions; i++) { + if (strcmp(pcfg->table, rbac_role->key_permissions[i]) == 0 + && pcfg->row == rbac_role->value_permissions[i]) { + break; + } + } + + if (i == rbac_role->n_permissions) { + rebuild_role_perm = true; + break; + } + } + + if (pcfg_len != rbac_role->n_permissions) { + rebuild_role_perm = true; + } + + /* + * Rebuild role's permission list. + */ + if (rebuild_role_perm) { + sbrec_rbac_role_set_permissions(rbac_role, NULL, NULL, 0); + + for (pcfg = rbac_perm_cfg; pcfg->table; pcfg++) { + sbrec_rbac_role_update_permissions_setkey(rbac_role, + pcfg->table, + pcfg->row); } } } diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index d459c23c0..3c6b719c1 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -13867,3 +13867,83 @@ check_no_redirect AT_CLEANUP ]) + +AT_SETUP([RBAC -- Recover builtin role and permissions]) +ovn_start + +RBR_BUILTIN_PNAMES=`ovn-sbctl get RBAC_Role ovn-controller permissions | uuidfilt | sed -e 's/<[[^<>]]>*//g' | tr -d '=,{}'` +declare -A RBR_BUILTIN_PERM + +for V in $RBR_BUILTIN_PNAMES +do + _UUID=`ovn-sbctl get RBAC_Role ovn-controller permissions:$V` + RBR_BUILTIN_PERM[[$V]]=`ovn-sbctl get RBAC_Permission $_UUID authorization insert_delete table update | paste -s -d " "` +done + +rbr_match_builtin() { + local RBR_REAL_PNAMES=`ovn-sbctl get RBAC_Role ovn-controller permissions | uuidfilt | sed -e 's/<[[^<>]]>*//g' | tr -d '=,{}'` + + AT_CHECK([test "$RBR_BUILTIN_PNAMES" == "$RBR_REAL_PNAMES"]) + + for V in $RBR_BUILTIN_PNAMES + do + local _UUID=`ovn-sbctl get RBAC_Role ovn-controller permissions:$V` + local RBR_REAL_PERM=`ovn-sbctl get RBAC_Permission $_UUID authorization insert_delete table update | paste -s -d " "` + AT_CHECK([test "${RBR_BUILTIN_PERM[[$V]]}" == "$RBR_REAL_PERM"]) + done +} + +rbr_match_custom() { + local RBR_BUILTIN_PNAMES='Load_Balancer' + local RBR_BUILTIN_ROLE='{Load_Balancer=<0>}' + local RBR_REAL_ROLE=`ovn-sbctl get RBAC_Role custom-role permissions | uuidfilt` + declare -A RBR_BUILTIN_PERM=( + [[Load_Balancer]]='[[]] false Load_Balancer [[]]' + ) + + AT_CHECK([test "$RBR_BUILTIN_ROLE" == "$RBR_REAL_ROLE"]) + + for V in $RBR_BUILTIN_PNAMES + do + local _UUID=`ovn-sbctl get RBAC_Role custom-role permissions:$V` + local _PREAL=`ovn-sbctl get RBAC_Permission $_UUID authorization insert_delete table update | paste -s -d " "` + AT_CHECK([test "${RBR_BUILTIN_PERM[[$V]]}" == "$_PREAL"]) + done +} + +AS_BOX([Recover after role delete]) +check ovn-sbctl destroy RBAC_Role ovn-controller +OVS_WAIT_UNTIL([rbr_match_builtin]) + +AS_BOX([Recover after permissions alternation]) +check ovn-sbctl remove RBAC_Permission Chassis update vtep_logical_switches +OVS_WAIT_UNTIL([rbr_match_builtin]) + +AS_BOX([Recover after permission delete]) +PERM_UID=`ovn-sbctl get RBAC_Role ovn-controller permissions:Chassis` +check check ovn-sbctl destroy RBAC_Permission $PERM_UID +OVS_WAIT_UNTIL([rbr_match_builtin]) + +AS_BOX([Recover after permission link removal from role]) +check ovn-sbctl remove RBAC_Role ovn-controller permissions Chassis +OVS_WAIT_UNTIL([rbr_match_builtin]) + +AS_BOX([Clean unwanted permission added to role]) +AT_CHECK([ovn-sbctl --id=@nr create RBAC_Permission table=Load_Balancer \ +-- set RBAC_Role ovn-controller permissions:Load_Balancer=@nr | uuidfilt], [0], [<0> +]) +OVS_WAIT_UNTIL([rbr_match_builtin]) + +AS_BOX([Clean duplicated role]) +AT_CHECK([ovn-sbctl create RBAC_Role name=ovn-controller | uuidfilt], [0], [<0> +]) +OVS_WAIT_UNTIL([rbr_match_builtin]) + +AS_BOX([Ensure custom role and permission are not automatically deleted]) +PERM_UID=`ovn-sbctl get RBAC_Permission Load_Balancer _uuid` +ovn-sbctl create RBAC_Role name=custom-role permissions:Load_Balancer=$PERM_UID +OVS_WAIT_UNTIL([rbr_match_builtin]) +check rbr_match_custom + +AT_CLEANUP +