Add endpoint to get previous service identity (microsoft#3880)

eddyashton · Jun 9, 2022 · 6619669 · 6619669
1 parent 0612980
commit 6619669
Show file tree

Hide file tree

Showing 17 changed files with 235 additions and 37 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## Unreleased
 
+### Added
+
+- Added a `GET /node/service/previous_identity` endpoint, which can be used during a recovery to look up the identity of the service before the catastrophic failure (#3880).
+
 ### Removed
 
 - Removed deprecated `set_execute_outside_consensus()` API (#3886, #3673).

diff --git a/doc/audit/builtin_maps.rst b/doc/audit/builtin_maps.rst
@@ -174,6 +174,15 @@ Service configuration.
    :project: CCF
    :members:
 
+``service.previous_service_identity``
+~~~~~~~~~~~~~~~~
+
+PEM identity of previous service, which this service recovered from.
+
+**Key** Sentinel value 0, represented as a little-endian 64-bit unsigned integer.
+
+**Value** Previous service identity, represented as a PEM-encoded JSON string.
+
 ``proposals``
 ~~~~~~~~~~~~~
 

diff --git a/doc/schemas/node_openapi.json b/doc/schemas/node_openapi.json
@@ -324,6 +324,17 @@
         ],
         "type": "object"
       },
+      "GetServicePreviousIdentity__Out": {
+        "properties": {
+          "previous_service_identity": {
+            "$ref": "#/components/schemas/Pem"
+          }
+        },
+        "required": [
+          "previous_service_identity"
+        ],
+        "type": "object"
+      },
       "GetState__Out": {
         "properties": {
           "last_recovered_seqno": {
@@ -743,7 +754,7 @@
   "info": {
     "description": "This API provides public, uncredentialed access to service and node state.",
     "title": "CCF Public Node API",
-    "version": "2.17.1"
+    "version": "2.18.0"
   },
   "openapi": "3.0.0",
   "paths": {
@@ -1134,6 +1145,22 @@
         }
       }
     },
+    "/node/service/previous_identity": {
+      "get": {
+        "responses": {
+          "200": {
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/GetServicePreviousIdentity__Out"
+                }
+              }
+            },
+            "description": "Default response description"
+          }
+        }
+      }
+    },
     "/node/state": {
       "get": {
         "responses": {

diff --git a/src/enclave/enclave.h b/src/enclave/enclave.h
@@ -70,7 +70,6 @@ namespace ccf
 
   public:
     Enclave(
-      const EnclaveConfig& ec,
       std::unique_ptr<ringbuffer::Circuit> circuit_,
       std::unique_ptr<ringbuffer::WriterFactory> basic_writer_factory_,
       std::unique_ptr<oversized::WriterFactory> writer_factory_,

diff --git a/src/enclave/main.cpp b/src/enclave/main.cpp
@@ -221,7 +221,6 @@ extern "C"
     try
     {
       enclave = new ccf::Enclave(
-        ec,
         std::move(circuit),
         std::move(basic_writer_factory),
         std::move(writer_factory),

diff --git a/src/node/historical_queries_adapter.cpp b/src/node/historical_queries_adapter.cpp
@@ -315,7 +315,8 @@ namespace ccf::historical
     auto& ctx,
     ccf::historical::StatePtr& state,
     AbstractStateCache& state_cache,
-    std::shared_ptr<NetworkIdentitySubsystem> network_identity_subsystem)
+    std::shared_ptr<NetworkIdentitySubsystemInterface>
+      network_identity_subsystem)
   {
     try
     {
@@ -401,8 +402,8 @@ namespace ccf::historical
     const TxIDExtractor& extractor)
   {
     auto& state_cache = node_context.get_historical_state();
-    std::shared_ptr<NetworkIdentitySubsystem> network_identity_subsystem =
-      node_context.get_subsystem<NetworkIdentitySubsystem>();
+    auto network_identity_subsystem =
+      node_context.get_subsystem<NetworkIdentitySubsystemInterface>();
 
     return [f, &state_cache, network_identity_subsystem, available, extractor](
              endpoints::EndpointContext& args) {

diff --git a/src/node/node_state.h b/src/node/node_state.h
@@ -1414,17 +1414,30 @@ namespace ccf
         return;
       }
 
-      if (
-        service_info->status == ServiceStatus::RECOVERING &&
-        (!config.recover.previous_service_identity ||
-         !identities.previous.has_value()))
+      if (service_info->status == ServiceStatus::RECOVERING)
       {
-        throw std::logic_error(
-          "Recovery with service certificates requires both, a previous "
-          "service identity certificate during node startup and a "
-          "transition_service_to_open proposal that contains previous and "
-          "next "
-          "service certificates");
+        const auto prev_ident = tx.ro<ccf::PreviousServiceIdentity>(
+                                    ccf::Tables::PREVIOUS_SERVICE_IDENTITY)
+                                  ->get();
+        if (!prev_ident.has_value() || !identities.previous.has_value())
+        {
+          throw std::logic_error(
+            "Recovery with service certificates requires both, a previous "
+            "service identity written to the KV during recovery genesis and a "
+            "transition_service_to_open proposal that contains previous and "
+            "next service certificates");
+        }
+
+        const crypto::Pem from_proposal(
+          identities.previous->data(), identities.previous->size());
+        if (prev_ident.value() != from_proposal)
+        {
+          throw std::logic_error(fmt::format(
+            "Previous service identity does not match.\nActual:\n{}\nIn "
+            "proposal:\n{}",
+            prev_ident->str(),
+            from_proposal.str()));
+        }
       }
 
       if (identities.next != service_info->cert)

diff --git a/src/node/rpc/network_identity_interface.h b/src/node/rpc/network_identity_interface.h
@@ -4,6 +4,7 @@
 
 #include "ccf/node_subsystem_interface.h"
 
+#include <optional>
 #include <string>
 #include <vector>
 

diff --git a/src/node/rpc/node_frontend.h b/src/node/rpc/node_frontend.h
@@ -21,6 +21,7 @@
 #include "node/session_metrics.h"
 #include "node_interface.h"
 #include "service/genesis_gen.h"
+#include "service/tables/previous_service_identity.h"
 
 namespace ccf
 {
@@ -113,6 +114,18 @@ namespace ccf
   DECLARE_JSON_REQUIRED_FIELDS(
     SelfSignedNodeCertificateInfo, self_signed_certificate);
 
+  struct GetServicePreviousIdentity
+  {
+    struct Out
+    {
+      crypto::Pem previous_service_identity;
+    };
+  };
+
+  DECLARE_JSON_TYPE(GetServicePreviousIdentity::Out);
+  DECLARE_JSON_REQUIRED_FIELDS(
+    GetServicePreviousIdentity::Out, previous_service_identity);
+
   class NodeEndpoints : public CommonEndpointRegistry
   {
   private:
@@ -357,7 +370,7 @@ namespace ccf
       openapi_info.description =
         "This API provides public, uncredentialed access to service and node "
         "state.";
-      openapi_info.document_version = "2.17.1";
+      openapi_info.document_version = "2.18.0";
     }
 
     void init_handlers() override
@@ -819,6 +832,32 @@ namespace ccf
         .set_auto_schema<void, GetNetworkInfo::Out>()
         .install();
 
+      auto service_previous_identity = [this](auto& args, nlohmann::json&&) {
+        auto psi_handle = args.tx.template ro<ccf::PreviousServiceIdentity>(
+          ccf::Tables::PREVIOUS_SERVICE_IDENTITY);
+        const auto psi = psi_handle->get();
+        if (psi.has_value())
+        {
+          GetServicePreviousIdentity::Out out;
+          out.previous_service_identity = psi.value();
+          return make_success(out);
+        }
+        else
+        {
+          return make_error(
+            HTTP_STATUS_NOT_FOUND,
+            ccf::errors::ResourceNotFound,
+            "This service is not a recovery of a previous service.");
+        }
+      };
+      make_read_only_endpoint(
+        "/service/previous_identity",
+        HTTP_GET,
+        json_read_only_adapter(service_previous_identity),
+        no_auth_required)
+        .set_auto_schema<void, GetServicePreviousIdentity::Out>()
+        .install();
+
       auto get_nodes = [this](auto& args, nlohmann::json&&) {
         const auto parsed_query =
           http::parse_query(args.rpc_ctx->get_request_query());
@@ -1305,16 +1344,6 @@ namespace ccf
         // Retire all nodes, in case there are any (i.e. post recovery)
         g.retire_active_nodes();
 
-        NodeInfo node_info = {
-          in.node_info_network,
-          {in.quote_info},
-          in.public_encryption_key,
-          NodeStatus::TRUSTED,
-          std::nullopt,
-          ds::to_hex(in.code_digest.data),
-          in.certificate_signing_request,
-          in.public_key};
-
         // Genesis transaction (i.e. not after recovery)
         if (in.genesis_info.has_value())
         {
@@ -1358,6 +1387,15 @@ namespace ccf
           ctx.tx.rw(network.node_endorsed_certificates);
         endorsed_certificates->put(in.node_id, in.node_endorsed_certificate);
 
+        NodeInfo node_info = {
+          in.node_info_network,
+          {in.quote_info},
+          in.public_encryption_key,
+          NodeStatus::TRUSTED,
+          std::nullopt,
+          ds::to_hex(in.code_digest.data),
+          in.certificate_signing_request,
+          in.public_key};
         g.add_node(in.node_id, node_info);
 
 #ifdef GET_QUOTE

diff --git a/src/service/genesis_gen.h b/src/service/genesis_gen.h
@@ -9,6 +9,7 @@
 #include "ccf/tx.h"
 #include "network_tables.h"
 #include "node/ledger_secrets.h"
+#include "service/tables/previous_service_identity.h"
 
 #include <algorithm>
 #include <fstream>
@@ -290,6 +291,15 @@ namespace ccf
       const crypto::Pem& service_cert, bool recovering = false)
     {
       auto service = tx.rw(tables.service);
+
+      if (service->has())
+      {
+        const auto prev_service_info = service->get();
+        auto previous_service_identity = tx.wo<ccf::PreviousServiceIdentity>(
+          ccf::Tables::PREVIOUS_SERVICE_IDENTITY);
+        previous_service_identity->put(prev_service_info->cert);
+      }
+
       service->put(
         {service_cert,
          recovering ? ServiceStatus::RECOVERING : ServiceStatus::OPENING,

diff --git a/src/service/network_tables.h b/src/service/network_tables.h
@@ -76,7 +76,7 @@ namespace ccf
     UserInfo user_info;
 
     //
-    // Node table
+    // Node tables
     //
     Nodes nodes;
     NodeEndorsedCertificates node_endorsed_certificates;

diff --git a/src/service/tables/previous_service_identity.h b/src/service/tables/previous_service_identity.h
@@ -0,0 +1,20 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the Apache 2.0 License.
+#pragma once
+
+#include "ccf/crypto/pem.h"
+#include "ccf/kv/value.h"
+
+#include <string>
+#include <vector>
+
+namespace ccf
+{
+  using PreviousServiceIdentity = ServiceValue<crypto::Pem>;
+
+  namespace Tables
+  {
+    static constexpr auto PREVIOUS_SERVICE_IDENTITY =
+      "public:ccf.gov.service.previous_service_identity";
+  }
+}
diff --git a/tests/infra/network.py b/tests/infra/network.py
@@ -21,7 +21,6 @@
 import http
 import pprint
 import functools
-import shutil
 from datetime import datetime, timedelta
 from infra.consortium import slurp_file
 
@@ -1333,9 +1332,20 @@ def verify_service_certificate_validity_period(self, expected_validity_days):
         )
 
     def save_service_identity(self, args):
-        current_identity = os.path.join(self.common_dir, "service_cert.pem")
+        n = self.find_random_node()
+        with n.client() as c:
+            r = c.get("/node/network")
+            assert r.status_code == 200, r
+            current_ident = r.body.json()["service_certificate"]
+        prev_cert_count = 0
         previous_identity = os.path.join(self.common_dir, "previous_service_cert.pem")
-        shutil.copy(current_identity, previous_identity)
+        while os.path.exists(previous_identity):
+            prev_cert_count += 1
+            previous_identity = os.path.join(
+                self.common_dir, f"previous_service_cert_{prev_cert_count}.pem"
+            )
+        with open(previous_identity, "w", encoding="utf-8") as f:
+            f.write(current_ident)
         args.previous_service_identity_file = previous_identity
         return args
 

diff --git a/tests/lts_compatibility.py b/tests/lts_compatibility.py
@@ -531,12 +531,12 @@ def run_ledger_compatibility_since_first(args, local_branch, use_snapshot):
                     network.get_committed_snapshots(primary) if use_snapshot else None
                 )
 
+                network.save_service_identity(args)
                 network.stop_all_nodes(
                     skip_verification=True,
                     accept_ledger_diff=is_ledger_chunk_breaking,
                 )
                 ledger_dir, committed_ledger_dirs = primary.get_ledger()
-                network.save_service_identity(args)
 
                 # Check that ledger and snapshots can be parsed
                 ccf.ledger.Ledger(committed_ledger_dirs).get_latest_public_state()