Allow governance to write to app tables (#7088)

Co-authored-by: Amaury Chamayou <[email protected]>

Author: eddyashton

CHANGELOG.md

@@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [6.0.8]
+
+[6.0.8]: https://github.com/microsoft/CCF/releases/tag/ccf-6.0.8
+
+### Changed
+
+- The constitution's `apply()` function may now write directly to public application (ie - non-governance) tables. Note that this access is _write-only_, so these tables can still not be read from. (#7088)
+
 ## [6.0.7]
 
 [6.0.7]: https://github.com/microsoft/CCF/releases/tag/ccf-6.0.7

doc/audit/read_write_restrictions.rst

@@ -49,12 +49,12 @@ CCF ensures that governance audit is possible offline from a ledger, by consider
 - Governance code must never read from private tables. Doing so might make decisions which could not be reproduced from the ledger by an auditor (ie. without access to ledger secrets).
 - Governance code must never read from application tables. Doing so might produce dependencies on data which was not signed by a member.
 - Governance code running pre-approval must only have read access to tables, and never write.
-- Governance code should not write to application tables, which could be modified further outside of governance.
 - Application code must not modify governance tables, as it could do so without constitution approval.
 
 .. note:: 
 
     An important exemption here is that application code may still `read` from governance tables. This allows authentication, authorization, and metadata to be configured and controlled by governance, but affect the execution of application endpoints.
+    An additional restriction was present until v7.0: "Governance code should not write to application tables, which could be modified further outside of governance". This was determined to be too strict, and prevents governance directly bootstrapping (or correcting) application table state, so was removed.
 
 ..
     A link to this page is included in the CCF source code, and returned in error messages.
@@ -75,9 +75,9 @@ The possible access permissions are elaborated in the table below:
     +==========================+============+============+============+============+============+============+
     | Pre-approval governance  | Read-only  | None       | Read-only  | None       | None       | None       |
     +--------------------------+------------+------------+------------+------------+------------+------------+
-    | Post-approval governance | Read-only  | None       | Writeable  | None       | None       | None       |
+    | Post-approval governance | Read-only  | None       | Read/Write | None       | Write-Only | None       |
     +--------------------------+------------+------------+------------+------------+------------+------------+
-    | Application              | Read-only  | None       | Read-only  | None       | Writeable  | Writeable  |
+    | Application              | Read-only  | None       | Read-only  | None       | Read/Write | Read/Write |
     +--------------------------+------------+------------+------------+------------+------------+------------+
 
 Any violation of these restrictions (eg - calling ``set`` on a `Read-only` table, or ``has`` on a `None` table) results in an exception being thrown.

include/ccf/js/kv_access_permissions.h

@@ -8,8 +8,18 @@ namespace ccf::js
 {
   enum class KVAccessPermissions
   {
-    READ_WRITE,
-    READ_ONLY,
-    ILLEGAL
+    ILLEGAL = 0,
+    READ_ONLY = 1 << 0,
+    WRITE_ONLY = 1 << 1,
+    READ_WRITE = READ_ONLY | WRITE_ONLY
   };
+
+  inline KVAccessPermissions intersect_access_permissions(
+    KVAccessPermissions l, KVAccessPermissions r)
+  {
+    /* This could use std::to_underlying from C++23 */
+    using T = std::underlying_type_t<KVAccessPermissions>;
+    const auto intersection = (T)l & (T)r;
+    return KVAccessPermissions(intersection);
+  }
 }

python/pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "ccf"
-version = "6.0.7"
+version = "6.0.8"
 authors = [
   { name="CCF Team", email="[email protected]" },
 ]

samples/apps/programmability/programmability.cpp

@@ -59,8 +59,8 @@ namespace programmabilityapp
         const auto roles = roles_it->get<std::vector<std::string>>();
         for (const auto& role : roles)
         {
-          auto role_handle =
-            tx.ro<RoleSet>(fmt::format("public:ccf.gov.roles.{}", role));
+          auto role_handle = tx.ro<RoleSet>(
+            fmt::format("public:programmability.roles.{}", role));
           if (role_handle->contains(action))
           {
             return true;

samples/constitutions/roles/set_role_definition.js

@@ -21,12 +21,6 @@ class KVSet {
   clear() {
     this.#map.clear();
   }
-
-  asSetOfStrings() {
-    let set = new Set();
-    this.#map.forEach((_, key) => set.add(ccf.bufToJsonCompatible(key)));
-    return set;
-  }
 }
 
 actions.set(
@@ -41,19 +35,14 @@ actions.set(
     },
     function (args) {
       let roleDefinition = new KVSet(
-        ccf.kv[`public:ccf.gov.roles.${args.role}`],
+        ccf.kv[`public:programmability.roles.${args.role}`],
       );
-      let oldValues = roleDefinition.asSetOfStrings();
+
+      roleDefinition.clear();
+
       let newValues = new Set(args.actions);
-      for (const action of oldValues) {
-        if (!newValues.has(action)) {
-          roleDefinition.delete(ccf.jsonCompatibleToBuf(action));
-        }
-      }
       for (const action of newValues) {
-        if (!oldValues.has(action)) {
-          roleDefinition.add(ccf.jsonCompatibleToBuf(action));
-        }
+        roleDefinition.add(ccf.jsonCompatibleToBuf(action));
       }
     },
   ),

src/js/extensions/ccf/kv.cpp

@@ -107,9 +107,11 @@ namespace ccf::js::extensions
 
         // Name-based policy cannot grant more access (eg - cannot change
         // Read-Only to Read-Write), can only make it more restricted
-        if (proposed_permission > access_permission)
+        const auto combined_permission = ccf::js::intersect_access_permissions(
+          proposed_permission, access_permission);
+        if (combined_permission != access_permission)
         {
-          access_permission = proposed_permission;
+          access_permission = combined_permission;
           explanation = proposed_explanation;
         }
       }

src/js/extensions/ccf/kv_helpers.h

@@ -347,54 +347,57 @@ namespace ccf::js::extensions::kvhelpers
   ARG_COUNT, \
   FUNC_FACTORY_METHOD, \
   SETTER_METHOD, \
-  PERMISSION_ERROR_MIN, \
+  PERMISSION_FLAGS, \
   HANDLE_GETTER) \
   do \
   { \
+    /* This could use std::to_underlying from C++23 */ \
+    const auto permitted = \
+      ccf::js::intersect_access_permissions( \
+        access_permission, PERMISSION_FLAGS) != KVAccessPermissions::ILLEGAL; \
     auto fn_val = ctx.FUNC_FACTORY_METHOD( \
-      access_permission >= PERMISSION_ERROR_MIN ? C_FUNC_NAME##_denied : \
-                                                  C_FUNC_NAME<HANDLE_GETTER>, \
+      !permitted ? C_FUNC_NAME##_denied : C_FUNC_NAME<HANDLE_GETTER>, \
       JS_METHOD_NAME, \
       ARG_COUNT); \
     JS_CHECK_EXC(fn_val); \
-    if (access_permission >= PERMISSION_ERROR_MIN) \
+    if (!permitted) \
     { \
       JS_CHECK_SET( \
         fn_val.set("_error_msg", ctx.new_string(permission_explanation))); \
     } \
     JS_CHECK_SET(view_val.SETTER_METHOD(JS_METHOD_NAME, std::move(fn_val))); \
   } while (0)
 
-#define MAKE_RO_FUNCTION(C_FUNC_NAME, JS_METHOD_NAME, ARG_COUNT) \
+#define MAKE_READ_FUNCTION(C_FUNC_NAME, JS_METHOD_NAME, ARG_COUNT) \
   MAKE_FUNCTION( \
     C_FUNC_NAME, \
     JS_METHOD_NAME, \
     ARG_COUNT, \
     new_c_function, \
     set, \
-    KVAccessPermissions::ILLEGAL, \
+    KVAccessPermissions::READ_ONLY, \
     GetReadOnlyHandle)
 
-#define MAKE_RW_FUNCTION(C_FUNC_NAME, JS_METHOD_NAME, ARG_COUNT) \
+#define MAKE_WRITE_FUNCTION(C_FUNC_NAME, JS_METHOD_NAME, ARG_COUNT) \
   MAKE_FUNCTION( \
     C_FUNC_NAME, \
     JS_METHOD_NAME, \
     ARG_COUNT, \
     new_c_function, \
     set, \
-    KVAccessPermissions::READ_ONLY, \
+    KVAccessPermissions::WRITE_ONLY, \
     GetWriteHandle)
 
-    MAKE_RO_FUNCTION(js_kv_map_has, "has", 1);
-    MAKE_RO_FUNCTION(js_kv_map_get, "get", 1);
+    MAKE_READ_FUNCTION(js_kv_map_has, "has", 1);
+    MAKE_READ_FUNCTION(js_kv_map_get, "get", 1);
 
-    MAKE_RO_FUNCTION(js_kv_map_foreach, "forEach", 1);
-    MAKE_RO_FUNCTION(
+    MAKE_READ_FUNCTION(js_kv_map_foreach, "forEach", 1);
+    MAKE_READ_FUNCTION(
       js_kv_get_version_of_previous_write, "getVersionOfPreviousWrite", 1);
 
-    MAKE_RW_FUNCTION(js_kv_map_set, "set", 2);
-    MAKE_RW_FUNCTION(js_kv_map_delete, "delete", 1);
-    MAKE_RW_FUNCTION(js_kv_map_clear, "clear", 0);
+    MAKE_WRITE_FUNCTION(js_kv_map_set, "set", 2);
+    MAKE_WRITE_FUNCTION(js_kv_map_delete, "delete", 1);
+    MAKE_WRITE_FUNCTION(js_kv_map_clear, "clear", 0);
 
     // This is a _getter_, subtly different from a read-only function
     MAKE_FUNCTION(
@@ -403,7 +406,7 @@ namespace ccf::js::extensions::kvhelpers
       0,
       new_getter_c_function,
       set_getter,
-      KVAccessPermissions::ILLEGAL,
+      KVAccessPermissions::READ_ONLY,
       GetReadOnlyHandle);
 
 #undef MAKE_RW_FUNCTION

src/js/permissions_checks.h

@@ -78,6 +78,10 @@ namespace ccf::js
               {
                 return KVAccessPermissions::READ_ONLY;
               }
+              case (TxAccess::GOV_RW):
+              {
+                return KVAccessPermissions::WRITE_ONLY;
+              }
               default:
               {
                 return KVAccessPermissions::ILLEGAL;
@@ -99,7 +103,8 @@ namespace ccf::js
   {
     char const* table_kind = permission == KVAccessPermissions::READ_ONLY ?
       "read-only" :
-      "inaccessible";
+      (permission == KVAccessPermissions::WRITE_ONLY ? "write-only" :
+                                                       "inaccessible");
 
     char const* exec_context = "unknown";
     switch (access)

src/js/test/js.cpp

@@ -155,10 +155,10 @@ TEST_CASE("Check KV Map access")
     }
 
     {
-      INFO("Public applications tables cannot even be read");
+      INFO("Public applications tables cannot be read, but can be written to");
       REQUIRE(
         check_kv_map_access(TxAccess::GOV_RW, public_app_table_name) ==
-        KVAccessPermissions::ILLEGAL);
+        KVAccessPermissions::WRITE_ONLY);
     }
 
     {