Authenticate recovery share submission (#5832)

.daily_canary

@@ -1,4 +1,4 @@
    -^-     ___           ___
   (- -)   (= =)   |     Y   &  +--?
  (  V  ) /  .  \  |    +---=---'
-/--x-m- /--n-n---xXx--/--yY------>>>----<<<>>]]{{}}--||-
+/--x-m- /--n-n---xXx--/--yY------>>>----<<<>>]]{{}}---||-

CHANGELOG.md

@@ -5,6 +5,12 @@ 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).
 
+## [5.0.0-dev7]
+
+[5.0.0-dev7]: https://github.com/microsoft/CCF/releases/tag/ccf-5.0.0-dev7
+
+- `POST /recovery/members/{memberId}:recover` is now authenticated by COSE Sign1, making it consistent with the other `POST` endpoints in governance, and avoiding a potential denial of service where un-authenticated and un-authorised clients could submit invalid shares repeatedly. The `submit_recovery_share.sh` script has been amended accordingly, and now takes a `--member-id-privk` and `--member-id-cert` (#5821).
+
 ## [5.0.0-dev6]
 
 [5.0.0-dev6]: https://github.com/microsoft/CCF/releases/tag/ccf-5.0.0-dev6

python/utils/submit_recovery_share.sh

@@ -7,7 +7,7 @@ set -e
 function usage()
 {
     echo "Usage:"""
-    echo "  $0 https://<node-address> --member-enc-privk /path/to/member_enc_privk.pem --api-version api_version --cert /path/to/member_cert.pem [CURL_OPTIONS]"
+    echo "  $0 https://<node-address> --member-enc-privk /path/to/member_enc_privk.pem --api-version api_version --member-id-privk /path/to/member_id_privk.pem ----member-id-cert /path/to/member_cert.pem [CURL_OPTIONS]"
     echo "Retrieves the encrypted recovery share for a given member, decrypts the share and submits it for recovery."
     echo ""
     echo "A sufficient number of recovery shares must be submitted by members to initiate the end of recovery procedure."
@@ -35,6 +35,12 @@ while [ "$1" != "" ]; do
         --member-enc-privk)
             member_enc_privk="$2"
             ;;
+        --member-id-privk)
+            member_id_privk="$2"
+            ;;
+        --member-id-cert)
+            member_id_cert="$2"
+            ;;
         --api-version)
             api_version="$2"
             ;;
@@ -45,31 +51,30 @@ while [ "$1" != "" ]; do
     shift
 done
 
-# Loop through all arguments and find cert
-next_is_cert=false
-
-for item in "$@" ; do
-    if [ "$next_is_cert" == true ]; then
-        cert=$item
-        next_is_cert=false
-    fi
-    if [ "$item" == "--cert" ]; then
-        next_is_cert=true
-    fi
-done
-
 if [ -z "${member_enc_privk}" ]; then
     echo "Error: No member encryption private key in arguments (--member-enc-privk)"
     exit 1
 fi
 
-if [ -z "${cert}" ]; then
-    echo "Error: No user certificate in arguments (--cert)"
+if [ -z "${member_id_privk}" ]; then
+    echo "Error: No member identity private key in arguments (--member-id-privk)"
     exit 1
 fi
 
+if [ -z "${member_id_cert}" ]; then
+    echo "Error: No member identity cert in arguments (--member-id-cert)"
+    exit 1
+fi
+
+if [ ! -f "env/bin/activate" ]
+    then
+        python3.8 -m venv env
+fi
+source env/bin/activate
+pip install -q ccf
+
 # Compute member ID, as the SHA-256 fingerprint of the signing certificate
-member_id=$(openssl x509 -in "$cert" -noout -fingerprint -sha256 | cut -d "=" -f 2 | sed 's/://g' | awk '{print tolower($0)}')
+member_id=$(openssl x509 -in "$member_id_cert" -noout -fingerprint -sha256 | cut -d "=" -f 2 | sed 's/://g' | awk '{print tolower($0)}')
 
 if [ "${api_version}" == "classic" ]; then
     get_share_path="gov/encrypted_recovery_share/${member_id}"
@@ -89,5 +94,6 @@ encrypted_share=$(curl -sS --fail -X GET "${node_rpc_address}/${get_share_path}"
 echo "${encrypted_share}" \
     | openssl base64 -d \
     | openssl pkeyutl -inkey "${member_enc_privk}" -decrypt -pkeyopt rsa_padding_mode:oaep -pkeyopt rsa_oaep_md:sha256 \
-    | openssl base64 -A | jq -R '{share: (.)}' \
-    | curl -i -sS --fail -H "Content-Type: application/json" -X POST "${node_rpc_address}/${submit_share_path}" "${@}" -d @-
+    | openssl base64 -A | jq -c -R '{share: (.)}' \
+    | ccf_cose_sign1 --ccf-gov-msg-type recovery_share --ccf-gov-msg-created_at "$(date -uIs)" --signing-key "${member_id_privk}" --signing-cert "${member_id_cert}" --content "-" \
+    | curl -i -sS --fail -H "Content-Type: application/cose" -X POST "${node_rpc_address}/${submit_share_path}" "${@}" --data-binary @-

src/node/gov/handlers/recovery.h

@@ -106,16 +106,26 @@ namespace ccf::gov::endpoints
             return;
           }
 
-          const auto& raw_body = ctx.rpc_ctx->get_request_body();
-          const nlohmann::json params = nlohmann::json::parse(raw_body);
+          const auto& cose_ident =
+            ctx.template get_caller<ccf::MemberCOSESign1AuthnIdentity>();
 
-          auto raw_recovery_share =
-            crypto::raw_from_b64(params["share"].get<std::string>());
+          auto params = nlohmann::json::parse(cose_ident.content);
+          if (cose_ident.member_id != member_id)
+          {
+            detail::set_gov_error(
+              ctx.rpc_ctx,
+              HTTP_STATUS_BAD_REQUEST,
+              ccf::errors::InvalidAuthenticationInfo,
+              fmt::format(
+                "Member ID from path parameter ({}) does not match "
+                "member ID from body signature ({}).",
+                member_id,
+                cose_ident.member_id));
+            return;
+          }
 
-          // Cleanse other copies of secret where possible. Note that this
-          // leaves a JSON-parsed copy, and potentially others in the TLS/HTTP
-          // stack.
-          OPENSSL_cleanse((char*)raw_body.data(), raw_body.size());
+          auto raw_recovery_share =
+            crypto::raw_from_b64(params["share"].template get<std::string>());
 
           size_t submitted_shares_count = 0;
           try
@@ -195,7 +205,7 @@ namespace ccf::gov::endpoints
         "/recovery/members/{memberId}:recover",
         HTTP_POST,
         api_version_adapter(submit_recovery_share),
-        ccf::no_auth_required)
+        detail::active_member_sig_only_policies("recovery_share"))
       .set_openapi_hidden(true)
       .install();
   }

tests/infra/member.py

@@ -363,6 +363,7 @@ def get_and_submit_recovery_share(self, remote_node):
         help_res.check_returncode()
         help_out = help_res.stdout.decode()
         supports_api_version = "--api-version" in help_out
+        support_member_id_cert = "--member-id-cert" in help_out
 
         cmd = [
             self.share_script,
@@ -371,17 +372,33 @@ def get_and_submit_recovery_share(self, remote_node):
             os.path.join(self.common_dir, f"{self.local_id}_enc_privk.pem"),
         ]
 
+        # Versions of the script that support --member-id arguments use COSE Sign1
+        # to authenticate with the service.
+        if support_member_id_cert:
+            cmd += [
+                "--member-id-privk",
+                os.path.join(self.common_dir, f"{self.local_id}_privk.pem"),
+                "--member-id-cert",
+                os.path.join(self.common_dir, f"{self.local_id}_cert.pem"),
+            ]
+
         if supports_api_version:
             cmd += [
                 "--api-version",
                 self.gov_api_impl.API_VERSION,
             ]
 
+        # Versions of the script that do not support --member-id arguments use
+        # client certificates (forward to curl) to authenticate with the service.
+        if not support_member_id_cert:
+            cmd += [
+                "--key",
+                os.path.join(self.common_dir, f"{self.local_id}_privk.pem"),
+                "--cert",
+                os.path.join(self.common_dir, f"{self.local_id}_cert.pem"),
+            ]
+
         cmd += [
-            "--cert",
-            os.path.join(self.common_dir, f"{self.local_id}_cert.pem"),
-            "--key",
-            os.path.join(self.common_dir, f"{self.local_id}_privk.pem"),
             "--cacert",
             os.path.join(self.common_dir, "service_cert.pem"),
         ]