Merge branch 'kb/cleanup' into 'master'

feat(NODE-1137): SEV-SNP library cleanup Cleanup sev library. Rename sev to snp to distinguish between sev and icos sev libraries. Remove reference to sevtool. Also remove icos_sev_interface and move the interfaces(pub errors and traits) to newly renamed snp library. See merge request dfinity-lab/public/ic!15526
dfinity · Nov 6, 2023 · 5019bf4 · 5019bf4 · ilbertt · Nov 12, 2023
2 parents f03bfc9 + 5778b05
commit 5019bf4
Show file tree

Hide file tree

Showing 29 changed files with 63 additions and 136 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -130,7 +130,6 @@ members = [
   "rs/ic_os/setupos-inject-configuration",
   "rs/ic_os/partition_tools",
   "rs/ic_os/setupos-disable-checks",
-  "rs/ic_os/sev_interfaces",
   "rs/ic_os/sev",
   "rs/ic_os/snptool",
   "rs/ic_os/vsock/guest",

diff --git a/ic-os/guestos/defs.bzl b/ic-os/guestos/defs.bzl
@@ -40,8 +40,6 @@ def image_deps(mode, malicious = False):
             "//publish/binaries:orchestrator": "/opt/ic/bin/orchestrator:0755",
             ("//publish/malicious:replica" if malicious else "//publish/binaries:replica"): "/opt/ic/bin/replica:0755",  # Install the malicious replica if set
             "//publish/binaries:sandbox_launcher": "/opt/ic/bin/sandbox_launcher:0755",
-            "//publish/binaries:snptool": "/opt/ic/bin/snptool:0755",
-            "@sevtool": "/opt/ic/bin/sevtool:0755",
             "//publish/binaries:state-tool": "/opt/ic/bin/state-tool:0755",
             "//publish/binaries:vsock_guest": "/opt/ic/bin/vsock_guest:0755",
             "//ic-os/utils:infogetty": "/opt/ic/bin/infogetty:0755",

diff --git a/ic-os/guestos/rootfs/etc/udev/rules.d/20-sev-guest.rules b/ic-os/guestos/rootfs/etc/udev/rules.d/20-sev-guest.rules
@@ -0,0 +1 @@
+ACTION=="add", KERNEL=="sev-guest", MODE="0777"
diff --git a/ic-os/guestos/rootfs/opt/ic/bin/setup-sev-certs.sh b/ic-os/guestos/rootfs/opt/ic/bin/setup-sev-certs.sh
@@ -3,33 +3,18 @@
 set -e
 
 if [[ -e "/dev/sev-guest" ]]; then
-    # Set permissions for sev-guest device
-    # TODO: This should move to guest_launch tool
-    chmod 777 /dev/sev-guest
     if [[ -f "/boot/config/vcek.pem" ]]; then
         # For prod we expect that the host will generate the ask and vcek certs and pass in via the config.
+        # We do this to avoid dependence on AMD server.
+        # Also ARK endpoint is rate limited.
+        # If certs are not provided via config, they will be fetched when required (i.e during p2p tls handshake)
         if ! cmp -s "/boot/config/ark.pem" "/opt/ic/share/ark.pem"; then
             echo "/boot/config/ark.pem does not match /opt/ic/share/ark.pem"
         fi
-        # Always use our hard coded ark.pem as the root of trust.
-        cp "/opt/ic/share/ark.pem" "/run/ic-node/config/ark.pem"
         for f in ask.pem vcek.pem; do
             cp "/boot/config/${f}" "/run/ic-node/config/${f}"
         done
-    else
-        # For test/farm we expect that the host will store the PEM files via SNP_GET_EXT_REPORT because the
-        # config may be generated on a machine other than the host.
-        (
-            cd /run/ic-node/config
-            /opt/ic/bin/snptool get-certs
-        )
-        if [[ -f "/run/ic-node/config/ask.cert" && -f "/run/ic-node/config/vcek.cert" ]]; then
-            # Always use our hard coded ask.pem as the root of trust.
-            cp "/opt/ic/share/ark.pem" "/run/ic-node/config/ark.pem"
-            for f in ask vcek; do
-                # We are storing the PEM files instead of the DER files on the host.
-                mv "/run/ic-node/conifg/${f}.cert" "/run/ic-node/config/${f}.pem"
-            done
-        fi
     fi
+    # Always use our hard coded ark.pem as the root of trust.
+    cp "/opt/ic/share/ark.pem" "/run/ic-node/config/ark.pem"
 fi
diff --git a/rs/ic_os/sev/BUILD.bazel b/rs/ic_os/sev/BUILD.bazel
@@ -16,11 +16,11 @@ rust_library(
             "@crate_index//:sev",
         ],
     }) + [
-        "//rs/ic_os/sev_interfaces",
         "//rs/interfaces/registry",
         "//rs/registry/helpers",
         "//rs/registry/subnet_features",
         "//rs/types/base_types",
+        "//rs/types/types",
         "@crate_index//:openssl",
         "@crate_index//:serde",
         "@crate_index//:serde_cbor",

diff --git a/rs/ic_os/sev/Cargo.toml b/rs/ic_os/sev/Cargo.toml
@@ -8,7 +8,7 @@ documentation.workspace = true
 
 [dependencies]
 async-trait = "0.1.41"
-ic-icos-sev-interfaces = { path = "../sev_interfaces" }
+ic-types = { path = "../../types/types" }
 ic-interfaces-registry = { path = "../../interfaces/registry" }
 ic-registry-client-helpers = { path = "../../registry/helpers" }
 ic-registry-subnet-type = { path = "../../registry/subnet_type" }
@@ -21,7 +21,7 @@ thiserror = "1"
 tokio = { workspace = true }
 openssl = "0.10.55"
 [target.'cfg(all(target_os = "linux", target_arch = "x86_64"))'.dependencies]
-sev = { version = "1.2.0", features = ["openssl"] }
+sev = { version = "1.2.1", features = ["openssl"] }
 
 [dev-dependencies]
 assert_matches = "1.5.0"
diff --git a/rs/ic_os/sev/src/lib.rs b/rs/ic_os/sev/src/lib.rs
@@ -8,8 +8,44 @@ mod other;
 use core::fmt;
 #[cfg(not(all(target_os = "linux", target_arch = "x86_64")))]
 pub use other::*;
+
+use async_trait::async_trait;
+use ic_types::registry::RegistryClientError;
+use ic_types::{NodeId, RegistryVersion};
 use std::fmt::{Display, Formatter};
 
+#[derive(Clone, Debug, PartialEq, Eq, thiserror::Error)]
+pub enum ValidateAttestationError {
+    RegistryError(RegistryClientError),
+    RegistryDataMissing {
+        node_id: NodeId,
+        registry_version: RegistryVersion,
+        description: String,
+    },
+    HandshakeError {
+        description: String,
+    },
+}
+
+impl Display for ValidateAttestationError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        write!(f, "{:?}", self)
+    }
+}
+
+// Perform mutual attestation over a stream.
+// The registry_version is that of the latest CUP and is used
+// to determine if SEV-SNP is enabled on the subnet.
+#[async_trait]
+pub trait ValidateAttestedStream<S> {
+    async fn perform_attestation_validation(
+        &self,
+        mut stream: S,
+        peer: NodeId,
+        registry_version: RegistryVersion,
+    ) -> Result<S, ValidateAttestationError>;
+}
+
 #[derive(Clone, Debug, PartialEq, Eq, thiserror::Error)]
 pub enum SnpError {
     SnpNotEnabled { description: String },

diff --git a/rs/ic_os/sev/src/linux_amd64.rs b/rs/ic_os/sev/src/linux_amd64.rs
@@ -11,9 +11,9 @@ References:
 */
 
 use crate::SnpError;
+use crate::{ValidateAttestationError, ValidateAttestedStream};
 use async_trait::async_trait;
 use ic_base_types::{NodeId, RegistryVersion};
-use ic_icos_sev_interfaces::{ValidateAttestationError, ValidateAttestedStream};
 use ic_interfaces_registry::RegistryClient;
 use ic_registry_client_helpers::{crypto::CryptoRegistry, node::NodeRegistry};
 use openssl::ecdsa::EcdsaSig;

diff --git a/rs/ic_os/sev/src/other.rs b/rs/ic_os/sev/src/other.rs
@@ -1,7 +1,6 @@
-use crate::SnpError;
+use crate::{SnpError, ValidateAttestationError, ValidateAttestedStream};
 use async_trait::async_trait;
 use ic_base_types::{NodeId, RegistryVersion};
-use ic_icos_sev_interfaces::{ValidateAttestationError, ValidateAttestedStream};
 use ic_interfaces_registry::RegistryClient;
 use std::sync::Arc;
 use tokio::io::{AsyncRead, AsyncWrite};

diff --git a/rs/ic_os/sev_interfaces/BUILD.bazel b/rs/ic_os/sev_interfaces/BUILD.bazel
diff --git a/rs/ic_os/sev_interfaces/Cargo.toml b/rs/ic_os/sev_interfaces/Cargo.toml
diff --git a/rs/ic_os/sev_interfaces/src/lib.rs b/rs/ic_os/sev_interfaces/src/lib.rs
diff --git a/rs/ic_os/snptool/Cargo.toml b/rs/ic_os/snptool/Cargo.toml
@@ -3,8 +3,8 @@ name = "snptool"
 version = "0.1.0"
 authors = ["DFINITY"]
 edition = "2021"
-description = "Utility for AMD SEV"
-keywords = ["amd", "sev"]
+description = "Utility for AMD SEV-SNP"
+keywords = ["amd", "sev", "snp"]
 exclude = [ ".gitignore", ".github/*" ]
 
 [[bin]]

diff --git a/rs/orchestrator/src/registration.rs b/rs/orchestrator/src/registration.rs
@@ -630,9 +630,9 @@ fn generate_nonce() -> Vec<u8> {
         .to_vec()
 }
 
-/// Get a chip_id from SNP guest firmware by calling the snptool.
-/// If snptool returns the error "unable to open /dev/sev-guest",
-/// it could be that the guest is not SEV-SNP enabled. Return an empty chip_id in that case.
+/// Get a chip_id from SNP guest firmware via SEV library.
+/// If SEV-SNP in not enabled on the guest, return None.
+/// In other cases, return the error and notify the Node Provider.
 fn get_snp_chip_id() -> OrchestratorResult<Option<Vec<u8>>> {
     match get_chip_id() {
         // Chip_id returned successfully

diff --git a/rs/p2p/quic_transport/BUILD.bazel b/rs/p2p/quic_transport/BUILD.bazel
@@ -10,7 +10,7 @@ DEPENDENCIES = [
     "//rs/async_utils",
     "//rs/crypto/tls_interfaces",
     "//rs/crypto/utils/tls",
-    "//rs/ic_os/sev_interfaces",
+    "//rs/ic_os/sev",
     "//rs/interfaces/registry",
     "//rs/monitoring/logger",
     "//rs/monitoring/metrics",
@@ -39,7 +39,6 @@ DEPENDENCIES = [
 ]
 
 DEV_DEPENDENCIES = [
-    "//rs/ic_os/sev",
     "//rs/p2p/test_utils",
     "//rs/test_utilities/logger",
     "//rs/types/types_test_utils",

diff --git a/rs/p2p/quic_transport/Cargo.toml b/rs/p2p/quic_transport/Cargo.toml
@@ -17,9 +17,9 @@ http-serde = "1.1.2"
 ic-async-utils = { path = "../../async_utils" }
 ic-crypto-tls-interfaces = { path = "../../crypto/tls_interfaces" }
 ic-crypto-utils-tls = { path = "../../crypto/utils/tls" }
-ic-icos-sev-interfaces = { path = "../../ic_os/sev_interfaces" }
 ic-interfaces-registry = { path = "../../interfaces/registry" }
 ic-base-types = { path = "../../types/base_types" }
+ic-icos-sev = { path = "../../ic_os/sev" }
 ic-logger = { path = "../../monitoring/logger" }
 ic-metrics = { path = "../../monitoring/metrics" }
 ic-peer-manager = { path = "../../p2p/peer_manager" }
@@ -42,7 +42,6 @@ tower = { workspace = true }
 
 [dev-dependencies]
 criterion = { version = "0.5", features = ["async_tokio"] }
-ic-icos-sev = { path = "../../ic_os/sev" }
 ic-p2p-test-utils = { path = "../test_utils" }
 ic-test-utilities-logger = { path = "../../test_utilities/logger" }
 ic-types-test-utils = { path = "../../types/types_test_utils" }

diff --git a/rs/p2p/quic_transport/src/connection_manager.rs b/rs/p2p/quic_transport/src/connection_manager.rs
@@ -47,7 +47,7 @@ use ic_crypto_tls_interfaces::{
 use ic_crypto_utils_tls::{
     node_id_from_cert_subject_common_name, tls_pubkey_cert_from_rustls_certs,
 };
-use ic_icos_sev_interfaces::ValidateAttestedStream;
+use ic_icos_sev::ValidateAttestedStream;
 use ic_interfaces_registry::RegistryClient;
 use ic_logger::{error, info, ReplicaLogger};
 use ic_metrics::MetricsRegistry;

diff --git a/rs/p2p/quic_transport/src/lib.rs b/rs/p2p/quic_transport/src/lib.rs
@@ -42,7 +42,7 @@ use either::Either;
 use http::{Request, Response};
 use ic_base_types::NodeId;
 use ic_crypto_tls_interfaces::{TlsConfig, TlsStream};
-use ic_icos_sev_interfaces::ValidateAttestedStream;
+use ic_icos_sev::ValidateAttestedStream;
 use ic_interfaces_registry::RegistryClient;
 use ic_logger::{info, ReplicaLogger};
 use ic_metrics::MetricsRegistry;

diff --git a/rs/p2p/quic_transport/tests/common.rs b/rs/p2p/quic_transport/tests/common.rs
@@ -5,7 +5,7 @@ use std::{
 
 use ic_base_types::{NodeId, RegistryVersion};
 use ic_crypto_tls_interfaces::{SomeOrAllNodes, TlsConfig, TlsConfigError};
-use ic_icos_sev_interfaces::{ValidateAttestationError, ValidateAttestedStream};
+use ic_icos_sev::{ValidateAttestationError, ValidateAttestedStream};
 use ic_p2p_test_utils::{temp_crypto_component_with_tls_keys, RegistryConsensusHandle};
 use tokio::io::{AsyncRead, AsyncWrite};
 use tokio_rustls::rustls::{ClientConfig, ServerConfig};

diff --git a/rs/p2p/test_utils/BUILD.bazel b/rs/p2p/test_utils/BUILD.bazel
@@ -9,7 +9,6 @@ DEPENDENCIES = [
     "//rs/crypto/temp_crypto",
     "//rs/crypto/tls_interfaces",
     "//rs/ic_os/sev",
-    "//rs/ic_os/sev_interfaces",
     "//rs/interfaces",
     "//rs/monitoring/logger",
     "//rs/monitoring/metrics",

diff --git a/rs/p2p/test_utils/Cargo.toml b/rs/p2p/test_utils/Cargo.toml
@@ -18,7 +18,6 @@ ic-crypto-tls-interfaces = { path = "../../crypto/tls_interfaces" }
 ic-logger = { path = "../../monitoring/logger" }
 ic-metrics = { path = "../../monitoring/metrics" }
 ic-icos-sev = { path = "../../ic_os/sev" }
-ic-icos-sev-interfaces = { path = "../../ic_os/sev_interfaces" }
 ic-interfaces = { path = "../../interfaces" }
 ic-peer-manager = { path = "../peer_manager" }
 ic-protobuf = { path = "../../protobuf" }

diff --git a/rs/p2p/test_utils/src/turmoil.rs b/rs/p2p/test_utils/src/turmoil.rs
@@ -17,8 +17,7 @@ use either::Either;
 use futures::{future::BoxFuture, FutureExt};
 use ic_artifact_manager::run_artifact_processor;
 use ic_crypto_tls_interfaces::{TlsConfig, TlsStream};
-use ic_icos_sev::Sev;
-use ic_icos_sev_interfaces::ValidateAttestedStream;
+use ic_icos_sev::{Sev, ValidateAttestedStream};
 use ic_interfaces::{
     artifact_manager::{ArtifactProcessorEvent, JoinGuard},
     artifact_pool::UnvalidatedArtifactEvent,