Merge branch 'rumenov/tls' into 'master'

refactor: remove unused code and the custom error for failed TLS public key creation

 

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

rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/public_and_secret_key_store/tests.rs

@@ -999,7 +999,7 @@ mod validate_pks_and_sks {
                     ..required_node_public_keys_and_time().0
                 },
                 expected: ValidatePksAndSksError::TlsCertificateError(PublicKeyInvalid(
-                    "Malformed certificate: TlsPublicKeyCertCreationError { internal_error: \"Error parsing DER: Parsing Error: InvalidDate\" }".to_string(),
+                    "Malformed certificate: TlsPublicKeyCertCreationError(\"Error parsing DER: Parsing Error: InvalidDate\"".to_string(),
                 )),
             },
             ParameterizedTest {

rs/crypto/src/tls/mod.rs

@@ -288,7 +288,7 @@ fn tls_cert_from_registry(
     let raw_cert = tls_cert_from_registry_raw(registry, node_id, registry_version)?;
     TlsPublicKeyCert::try_from(raw_cert).map_err(|e| {
         TlsCertFromRegistryError::CertificateMalformed {
-            internal_error: e.internal_error,
+            internal_error: format!("{e}"),
         }
     })
 }

rs/crypto/tls_interfaces/BUILD.bazel

@@ -28,10 +28,10 @@ rust_library(
     ],
     version = "0.9.0",
     deps = [
-        "//rs/crypto/sha2",
         "//rs/protobuf",
         "//rs/types/types",
         "@crate_index//:serde",
+        "@crate_index//:thiserror",
         "@crate_index//:tokio",
         "@crate_index//:tokio-rustls",
         "@crate_index//:x509-parser",

rs/crypto/tls_interfaces/Cargo.toml

@@ -10,8 +10,8 @@ documentation.workspace = true
 async-trait = "0.1.41"
 ic-types = { path = "../../types/types" }
 ic-protobuf = { path = "../../protobuf" }
-ic-crypto-sha2 = { path = "../sha2" }
 serde = { workspace = true }
+thiserror = "1.0.56"
 tokio = { workspace = true }
 tokio-rustls = "0.24.0"
 x509-parser = "0.15.1"

rs/crypto/tls_interfaces/src/lib.rs

@@ -3,16 +3,13 @@
 #![deny(clippy::unwrap_used)]
 
 use async_trait::async_trait;
-use core::fmt;
-use ic_crypto_sha2::Sha256;
 use ic_protobuf::registry::crypto::v1::X509PublicKeyCert;
 use ic_types::registry::RegistryClientError;
 use ic_types::{NodeId, RegistryVersion};
 use serde::{Deserialize, Deserializer, Serialize};
-use std::cmp::Ordering;
 use std::collections::BTreeSet;
-use std::fmt::{Display, Formatter};
-use std::hash::{Hash, Hasher};
+use std::fmt::{self, Display, Formatter};
+use thiserror::Error;
 use tokio::io::{AsyncRead, AsyncWrite};
 use tokio::net::TcpStream;
 use tokio_rustls::rustls::{ClientConfig, ServerConfig};
@@ -21,43 +18,34 @@ use x509_parser::certificate::X509Certificate;
 #[cfg(test)]
 mod tests;
 
-#[derive(Clone, Debug, Serialize)]
+#[derive(Clone, Debug, Serialize, PartialEq, Eq)]
 /// An X.509 certificate
 pub struct TlsPublicKeyCert {
     // rename, to match previous serializations (which used X509PublicKeyCert)
     #[serde(rename = "certificate_der")]
     der_cached: Vec<u8>,
-    #[serde(skip_serializing)]
-    hash_cached: Vec<u8>,
 }
 
 impl TlsPublicKeyCert {
     /// Creates a certificate from ASN.1 DER encoding
     pub fn new_from_der(cert_der: Vec<u8>) -> Result<Self, TlsPublicKeyCertCreationError> {
         use x509_parser::prelude::FromDer;
-        let (remainder, _cert) =
-            X509Certificate::from_der(&cert_der).map_err(|e| TlsPublicKeyCertCreationError {
-                internal_error: format!("Error parsing DER: {}", e),
-            })?;
+        let (remainder, _cert) = X509Certificate::from_der(&cert_der)
+            .map_err(|e| TlsPublicKeyCertCreationError(format!("Error parsing DER: {}", e)))?;
         if !remainder.is_empty() {
-            return Err(TlsPublicKeyCertCreationError {
-                internal_error: format!(
-                    "DER not fully consumed when parsing. Remainder: {remainder:?}",
-                ),
-            });
+            return Err(TlsPublicKeyCertCreationError(format!(
+                "DER not fully consumed when parsing. Remainder: {remainder:?}"
+            )));
         }
         Ok(Self {
-            hash_cached: Sha256::hash(&cert_der).to_vec(),
             der_cached: cert_der,
         })
     }
 
     /// Creates a certificate from PEM encoding
     pub fn new_from_pem(pem_cert: &str) -> Result<Self, TlsPublicKeyCertCreationError> {
         let cert_der = x509_parser::pem::Pem::read(std::io::Cursor::new(pem_cert))
-            .map_err(|e| TlsPublicKeyCertCreationError {
-                internal_error: format!("Error parsing PEM: {e}"),
-            })?
+            .map_err(|e| TlsPublicKeyCertCreationError(format!("Error parsing PEM: {e}")))?
             .0
             .contents;
         Self::new_from_der(cert_der)
@@ -76,32 +64,9 @@ impl TlsPublicKeyCert {
     }
 }
 
-impl PartialEq for TlsPublicKeyCert {
-    /// Equality is determined by comparison of the SHA256 hash byte arrays.
-    fn eq(&self, rhs: &Self) -> bool {
-        self.hash_cached == rhs.hash_cached
-    }
-}
-
-impl Eq for TlsPublicKeyCert {}
-
-impl Hash for TlsPublicKeyCert {
-    fn hash<H: Hasher>(&self, state: &mut H) {
-        self.hash_cached.hash(state)
-    }
-}
-
-impl PartialOrd for TlsPublicKeyCert {
-    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        Some(self.cmp(other))
-    }
-}
-
-impl Ord for TlsPublicKeyCert {
-    fn cmp(&self, other: &Self) -> Ordering {
-        self.hash_cached.cmp(&other.hash_cached)
-    }
-}
+#[derive(Error, Debug)]
+#[error("{0}")]
+pub struct TlsPublicKeyCertCreationError(String);
 
 impl<'de> Deserialize<'de> for TlsPublicKeyCert {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
@@ -129,20 +94,6 @@ impl TryFrom<X509PublicKeyCert> for TlsPublicKeyCert {
     }
 }
 
-#[derive(Clone, Debug, PartialEq, Eq)]
-/// Errors encountered during creation of a `TlsPublicKeyCert`.
-pub struct TlsPublicKeyCertCreationError {
-    pub internal_error: String,
-}
-
-impl Display for TlsPublicKeyCertCreationError {
-    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        write!(f, "{:?}", self)
-    }
-}
-
-impl std::error::Error for TlsPublicKeyCertCreationError {}
-
 #[derive(Clone, Debug, PartialEq, Eq)]
 /// Errors from a TLS handshake performed as the server. Please refer to the
 /// `TlsHandshake` method for detailed error variant descriptions.

rs/crypto/tls_interfaces/src/tests.rs

@@ -1,7 +1,7 @@
 #![allow(clippy::unwrap_used)]
 
 mod tls_public_key_cert {
-    use crate::{TlsPublicKeyCert, TlsPublicKeyCertCreationError};
+    use crate::TlsPublicKeyCert;
     use assert_matches::assert_matches;
     use ic_crypto_test_utils_reproducible_rng::reproducible_rng;
     use ic_crypto_test_utils_tls::x509_certificates::generate_ed25519_cert;
@@ -57,10 +57,7 @@ mod tls_public_key_cert {
         let empty_der = Vec::new();
 
         let error = TlsPublicKeyCert::new_from_der(empty_der).unwrap_err();
-
-        assert_matches!(error, TlsPublicKeyCertCreationError { internal_error }
-                if internal_error.contains("Error parsing DER")
-        );
+        assert!(format!("{}", error).contains("Error parsing DER"));
     }
 
     #[test]
@@ -69,9 +66,7 @@ mod tls_public_key_cert {
 
         let error = TlsPublicKeyCert::new_from_der(malformed_der).unwrap_err();
 
-        assert_matches!(error, TlsPublicKeyCertCreationError { internal_error }
-            if internal_error.contains("Error parsing DER")
-        );
+        assert!(format!("{}", error).contains("Error parsing DER"));
     }
 
     #[test]
@@ -118,7 +113,7 @@ mod tls_public_key_cert {
 
         let error: Result<TlsPublicKeyCert, json5::Error> = json5::from_str(bad_serialized);
         assert_matches!(error, Err(json5::Error::Message { msg, .. } )
-            if msg.contains("TlsPublicKeyCertCreationError")
+            if msg.contains("Error parsing DER")
         );
     }