Merge branch 'mathias-CRP-1539-add-socket-for-crypto-vault-metrics' i…

…nto 'master'

feat(crypto): CRP-1539: Add socket for retrieving metrics from crypto csp vault

Add a new Unix domain socket to allow the replica to retrieve metrics from the crypto CSP vault process. This follows the similar approach taken for the Bitcoin adapters, and the canister HTTP adapter. Adding the Unix domain socket is the first step in enabling metrics to be retrieved from the crypto CSP vault process - the rest of the functionality will be added in follow-up MRs.

This MR also adds a system test verifying that a socket has been created, and checks that the permission of the metrics socket, as well as the existing socket, are correct. 

See merge request dfinity-lab/public/ic!14634

ic-os/guestos/rootfs/etc/systemd/system/ic-crypto-csp.socket

@@ -2,7 +2,12 @@
 Description=Socket for IC Crypto Service Provider
 
 [Socket]
+# The order specified here defines the order in which the
+# process receives the sockets. 'socket' will be passed as FD(3)
+# 'metrics' will be passed as FD(4) to the crypto csp service.
+# https://www.freedesktop.org/software/systemd/man/systemd.socket.html
 ListenStream=/run/ic-node/crypto-csp/socket
+ListenStream=/run/ic-node/crypto-csp/metrics
 SocketUser=ic-csp-vault
 SocketGroup=ic-csp-vault-socket
 SocketMode=0660

rs/crypto/src/bin/ic-crypto-csp.rs

@@ -6,7 +6,9 @@ use ic_metrics::MetricsRegistry;
 use std::os::unix::io::FromRawFd;
 use std::path::PathBuf;
 
-const IC_CRYPTO_CSP_SOCKET_NAME: &str = "ic-crypto-csp.socket";
+// This corresponds to the name of the file where the sockets are defined, i.e.,
+// /ic-os/guestos/rootfs/etc/systemd/system/ic-crypto-csp.socket
+const IC_CRYPTO_CSP_SOCKET_FILENAME: &str = "ic-crypto-csp.socket";
 
 #[derive(Parser)]
 #[clap(
@@ -29,7 +31,7 @@ async fn main() {
 
     let sks_dir = ic_config.crypto.crypto_root.as_path();
 
-    ensure_single_named_systemd_socket(IC_CRYPTO_CSP_SOCKET_NAME);
+    ensure_n_named_systemd_sockets(2);
     let systemd_socket_listener = listener_from_first_systemd_socket();
 
     // The `AsyncGuard` must be kept in scope for asynchronously logged messages to appear in the logs.
@@ -72,14 +74,25 @@ fn get_ic_config(replica_config_file: PathBuf) -> Config {
     Config::load_with_tmpdir(ConfigSource::File(replica_config_file), tmpdir)
 }
 
-fn ensure_single_named_systemd_socket(socket_name: &str) {
+fn ensure_n_named_systemd_sockets(num_expected_sockets: usize) {
     const SYSTEMD_SOCKET_NAMES: &str = "LISTEN_FDNAMES"; // see https://www.freedesktop.org/software/systemd/man/sd_listen_fds.html
     let systemd_socket_names =
         std::env::var(SYSTEMD_SOCKET_NAMES).expect("failed to read systemd socket names");
-    if systemd_socket_names != socket_name {
+    let num_sockets = systemd_socket_names
+        .split(':')
+        .map(|socket_name| {
+            if IC_CRYPTO_CSP_SOCKET_FILENAME != socket_name {
+                panic!(
+                    "Expected to receive {} systemd socket(s) named '{}' but instead got '{}'",
+                    num_expected_sockets, IC_CRYPTO_CSP_SOCKET_FILENAME, systemd_socket_names
+                );
+            }
+        })
+        .count();
+    if num_sockets != num_expected_sockets {
         panic!(
-            "Expected to receive a single systemd socket named '{}' but instead got '{}'",
-            socket_name, systemd_socket_names
+            "Expected to receive {} systemd socket named '{}' but instead got {} ('{}')",
+            num_expected_sockets, IC_CRYPTO_CSP_SOCKET_FILENAME, num_sockets, systemd_socket_names
         );
     }
 }

rs/tests/Cargo.toml

@@ -541,6 +541,10 @@ path = "networking/update_workload_large_payload.rs"
 name = "ic-systest-canister-sig-verification-cache-test"
 path = "crypto/canister_sig_verification_cache_test.rs"
 
+[[bin]]
+name = "ic-systest-ic-crypto-csp-socket-test"
+path = "crypto/ic_crypto_csp_socket_test.rs"
+
 [[bin]]
 name = "ic-systest-ic-crypto-csp-umask-test"
 path = "crypto/ic_crypto_csp_umask_test.rs"

rs/tests/crypto/BUILD.bazel

@@ -39,3 +39,11 @@ system_test(
     runtime_deps = GUESTOS_RUNTIME_DEPS,
     deps = DEPENDENCIES + ["//rs/tests"],
 )
+
+system_test(
+    name = "ic_crypto_csp_socket_test",
+    proc_macro_deps = MACRO_DEPENDENCIES,
+    target_compatible_with = ["@platforms//os:linux"],  # requires libssh that does not build on Mac OS
+    runtime_deps = GUESTOS_RUNTIME_DEPS,
+    deps = DEPENDENCIES + ["//rs/tests"],
+)

rs/tests/crypto/ic_crypto_csp_socket_test.rs

@@ -0,0 +1,15 @@
+#[rustfmt::skip]
+
+use anyhow::Result;
+use ic_tests::crypto::ic_crypto_csp_socket_test::ic_crypto_csp_socket_test;
+use ic_tests::crypto::ic_crypto_csp_socket_test::setup_with_single_node;
+use ic_tests::driver::group::SystemTestGroup;
+use ic_tests::systest;
+
+fn main() -> Result<()> {
+    SystemTestGroup::new()
+        .with_setup(setup_with_single_node)
+        .add_test(systest!(ic_crypto_csp_socket_test))
+        .execute_from_args()?;
+    Ok(())
+}

rs/tests/src/crypto/ic_crypto_csp_socket_test.rs

@@ -0,0 +1,137 @@
+/* tag::catalog[]
+Title:: ic-crypto-csp socket test
+
+Goal:: Ensure that the Unix domain sockets for the crypto csp are created and have the correct
+permissions. In particular, ensure that `socket` and `metrics` sockets in the
+`/run/ic-node/crypto-csp/` directory have read and write permissions for the `ic-csp-vault` user
+(the owner) and the `ic-csp-vault-socket` group, and no permissions for others, and has the
+`ic-csp-vault` owner and `ic-csp-vault-socket` group (which contains the `ic-replica` user).
+
+Runbook::
+. Set up a subnet with a single node
+. Wait for the node to start up correctly and be healthy
+. Retrieve the file metadata (permissions, timestamp, inode number) of the sockets
+. Verify that the permissions, owner, and group of the sockets are correct
+
+Success:: Both sockets for the crypto csp exist, and that they have the correct permissions, owner,
+and group.
+
+Coverage::
+. The sockets for the crypto csp are created
+. The permissions, owner, and group, of the sockets are set correctly for the `ic-crypto-csp` process
+
+
+end::catalog[] */
+
+use crate::driver::ic::InternetComputer;
+use crate::driver::test_env::TestEnv;
+use crate::driver::test_env_api::{
+    GetFirstHealthyNodeSnapshot, HasTopologySnapshot, IcNodeContainer, IcNodeSnapshot, SshSession,
+};
+use ic_registry_subnet_type::SubnetType;
+use slog::{info, Logger};
+
+pub fn setup_with_single_node(env: TestEnv) {
+    InternetComputer::new()
+        .add_fast_single_node_subnet(SubnetType::System)
+        .setup_and_start(&env)
+        .expect("failed to setup IC under test");
+
+    env.topology_snapshot()
+        .subnets()
+        .for_each(|subnet| subnet.await_all_nodes_healthy().unwrap());
+}
+
+const SOCKET_DIR: &str = "/run/ic-node/crypto-csp";
+const SOCKET_NAMES: [&str; 2] = ["socket", "metrics"];
+
+pub fn ic_crypto_csp_socket_test(env: TestEnv) {
+    let logger = env.logger();
+    let node = env.get_first_healthy_node_snapshot();
+
+    for socket_name in &SOCKET_NAMES {
+        let socket_metadata = SocketMetadata::retrieve(socket_name, SOCKET_DIR, &node, &logger);
+        info!(
+            logger,
+            "{}/{} socket metadata: {:?}", SOCKET_DIR, socket_name, socket_metadata
+        );
+
+        // The socket shall have permissions '660'.
+        // This corresponds to '-rw-rw----', i.e., read & write for the owner and the group, but
+        // no permissions for others.
+        assert!(socket_metadata.has_permissions(660));
+        assert!(socket_metadata.has_owner("ic-csp-vault"));
+        assert!(socket_metadata.has_group("ic-csp-vault-socket"));
+        assert!(socket_metadata.has_type("socket"));
+    }
+}
+
+#[derive(Debug)]
+struct SocketMetadata {
+    permissions: u16,
+    owner: String,
+    group: String,
+    file_type: String,
+}
+
+impl From<String> for SocketMetadata {
+    fn from(value: String) -> Self {
+        // Example output from "stat -c '%a %U %G %F' /var/lib/ic/crypto/sks_data.pb".
+        // Columns are:
+        //  - file permissions in octal
+        //  - owner
+        //  - group
+        //  - file type
+        // 660 ic-csp-vault ic-csp-vault-socket socket
+        let mut field_iter = value.split_whitespace();
+        let permissions = field_iter.next().expect("no permissions field");
+        let owner = field_iter.next().expect("no owner field");
+        let group = field_iter.next().expect("no group field");
+        let file_type = field_iter.next().expect("no file type field");
+        let no_more_fields = field_iter.next();
+        assert!(
+            no_more_fields.is_none(),
+            "unexpected field: {:?}",
+            no_more_fields
+        );
+
+        SocketMetadata {
+            permissions: permissions.parse().expect("error parsing permissions"),
+            owner: String::from(owner),
+            group: String::from(group),
+            file_type: String::from(file_type),
+        }
+    }
+}
+
+impl SocketMetadata {
+    fn retrieve(socket: &str, path: &str, node: &IcNodeSnapshot, logger: &Logger) -> Self {
+        let stat_cmd = format!("sudo stat -c '%a %U %G %F' {}/{}", path, socket);
+        info!(
+            logger,
+            "retrieving socket metadata using command: {}", stat_cmd
+        );
+        let stat_output = node
+            .block_on_bash_script(stat_cmd.as_str())
+            .expect("unable to get socket metadata using SSH")
+            .trim()
+            .to_string();
+        SocketMetadata::from(stat_output)
+    }
+
+    fn has_permissions(&self, permissions: u16) -> bool {
+        self.permissions == permissions
+    }
+
+    fn has_group(&self, group: &str) -> bool {
+        self.group == group
+    }
+
+    fn has_owner(&self, owner: &str) -> bool {
+        self.owner == owner
+    }
+
+    fn has_type(&self, file_type: &str) -> bool {
+        self.file_type == file_type
+    }
+}

rs/tests/src/crypto/mod.rs

@@ -1,3 +1,4 @@
+pub mod ic_crypto_csp_socket_test;
 pub mod ic_crypto_csp_umask_test;
 pub mod request_signature_test;
 pub mod rpc_csp_vault_reconnection_test;