fix error after upgrading ring and webpki.

Signed-off-by: Yang, Longlong <[email protected]>
intel · Dec 1, 2023 · d516783 · d516783
1 parent a844642
commit d516783
Show file tree

Hide file tree

Showing 11 changed files with 101 additions and 66 deletions.
diff --git a/external/patches/ring/0001-Support-x86_64-unknown-none-target.patch b/external/patches/ring/0001-Support-x86_64-unknown-none-target.patch
@@ -74,15 +74,3 @@ index f7b94108b..3bdc8cd29 100644
      let _ = c
          .arg("-o")
          .arg(out_file.to_str().expect("Invalid path"))
-diff --git a/src/rand.rs b/src/rand.rs
-index 809791c79..b861b719a 100644
---- a/src/rand.rs
-+++ b/src/rand.rs
-@@ -126,6 +126,7 @@ impl crate::sealed::Sealed for SystemRandom {}
- // implementation.
- #[cfg(any(
-     all(feature = "less-safe-getrandom-custom-or-rdrand", target_os = "none"),
-+    target_os = "none",
-     target_os = "aix",
-     target_os = "android",
-     target_os = "dragonfly",
diff --git a/fuzz-target/fuzzlib/Cargo.toml b/fuzz-target/fuzzlib/Cargo.toml
@@ -11,7 +11,7 @@ afl = { version = "=0.12.12", optional = true }
 spdmlib = { path = "../../spdmlib", default-features = false, features=["spdm-ring"] }
 simple_logger = "4.2.0"
 log = "0.4.13"
-ring = { version = "0.16.20" }
+ring = { version = "0.17.6" }
 flexi_logger = "0.27.2"
 spdmlib-test = { path = "../../test/spdmlib-test" }
 futures = { version = "0.3", default-features = false }

diff --git a/sh_script/pre-build.sh b/sh_script/pre-build.sh
@@ -3,14 +3,14 @@
 format-patch() {
     # apply the patch set for ring
     pushd external/ring
-    git reset --hard 9cc0d45f4d8521f467bb3a621e74b1535e118188
+    git reset --hard 464d367252354418a2c17feb806876d4d89a8508
     git clean -xdf
     git apply ../patches/ring/0001-Support-x86_64-unknown-none-target.patch
     popd
 
     # apply the patch set for webpki
     pushd external/webpki
-    git reset --hard 0b7cbf2d327d7665d9d06072bf46b2e7ca05f065
+    git reset --hard f84a538a5cd281ba1ffc0d54bbe5824cf5969703
     git clean -xdf
     git apply ../patches/webpki/0001-Add-support-for-verifying-certificate-chain-with-EKU.patch
     popd

diff --git a/spdmlib/Cargo.toml b/spdmlib/Cargo.toml
@@ -16,10 +16,9 @@ log = "0.4.13"
 bytes = { version="1", default-features=false }
 conquer-once = { version = "0.3.2", default-features = false }
 lazy_static = { version = "1.0", features = ["spin_no_std"], optional = true }
-
-ring = { version = "0.16.20",  optional = true }
-webpki = { version = "0.22.0", default-features = false, features = ["alloc"], optional = true}
-untrusted = { version = "0.7.1", optional = true }
+ring = { version = "0.17.6", default-features = false, features = ["alloc", "less-safe-getrandom-custom-or-rdrand"], optional = true }
+webpki = { version = "0.22.4", default-features = false, features = ["alloc"], optional = true}
+untrusted = { version = "0.9.0", optional = true }
 zeroize = { version = "1.5.0", features = ["zeroize_derive"]}
 
 futures = { version = "0.3", default-features = false }

diff --git a/spdmlib/src/crypto/spdm_ring/cert_operation_impl.rs b/spdmlib/src/crypto/spdm_ring/cert_operation_impl.rs
@@ -58,22 +58,88 @@ fn verify_cert_chain(cert_chain: &[u8]) -> SpdmResult {
         &webpki::ECDSA_P384_SHA384,
     ];
 
-    let certs_der = untrusted::Input::from(cert_chain);
-    let reader = &mut untrusted::Reader::new(certs_der);
-
     let mut certs = Vec::new();
+    let mut certs_walker = 0;
+    let cert_chain_len = cert_chain.len();
     loop {
-        let start = reader.mark();
-        match der::expect_tag_and_get_value(reader, der::Tag::Sequence) {
-            Ok(_) => {
-                let end = reader.mark();
-                let cert = reader
-                    .get_input_between_marks(start, end)
-                    .map_err(|_| SPDM_STATUS_INVALID_CERT)?;
-                certs.push(cert.as_slice_less_safe())
+        let start = if certs_walker < cert_chain_len {
+            certs_walker
+        } else {
+            break;
+        };
+
+        let tag = cert_chain[certs_walker];
+        if usize::from(der::Tag::Sequence) != tag as usize {
+            break;
+        }
+
+        certs_walker += 1;
+        if certs_walker >= cert_chain_len {
+            break;
+        }
+
+        // If the high order bit of the first byte is set to zero then the length
+        // is encoded in the seven remaining bits of that byte. Otherwise, those
+        // seven bits represent the number of bytes used to encode the length.
+        let length_byte0 = cert_chain[certs_walker];
+
+        let length = match length_byte0 {
+            n if (n & 0x80) == 0 => n as usize,
+            0x81 => {
+                certs_walker += 1;
+                if certs_walker >= cert_chain_len {
+                    break;
+                }
+
+                let second_byte = cert_chain[certs_walker];
+                if second_byte < 128 {
+                    break; // Not the canonical encoding.
+                }
+
+                certs_walker += 1;
+                if certs_walker >= cert_chain_len {
+                    break;
+                }
+
+                second_byte as usize
+            }
+            0x82 => {
+                certs_walker += 1;
+                if certs_walker >= cert_chain_len {
+                    break;
+                }
+
+                let second_byte = cert_chain[certs_walker] as usize;
+
+                certs_walker += 1;
+                if certs_walker >= cert_chain_len {
+                    break;
+                }
+
+                let third_byte = cert_chain[certs_walker] as usize;
+
+                certs_walker += 1;
+                if certs_walker >= cert_chain_len {
+                    break;
+                }
+
+                let combined = (second_byte << 8) | third_byte;
+                if combined < 256 {
+                    break; // Not the canonical encoding.
+                }
+                combined
+            }
+            _ => {
+                break; // We don't support longer lengths.
             }
-            Err(_) => break,
+        };
+
+        certs_walker += length;
+        if certs_walker > cert_chain_len {
+            break;
         }
+
+        certs.push(&cert_chain[start..certs_walker]);
     }
     let certs_len = certs.len();
 
@@ -117,14 +183,7 @@ fn verify_cert_chain(cert_chain: &[u8]) -> SpdmResult {
 
     // we cannot call verify_is_valid_tls_server_cert because it will check verify_cert::EKU_SERVER_AUTH.
     if cert
-        .verify_cert_chain_with_eku(
-            EKU_SPDM_RESPONDER_AUTH,
-            ALL_SIGALGS,
-            &anchors,
-            inters,
-            time,
-            0,
-        )
+        .verify_cert_chain_with_eku(EKU_SPDM_RESPONDER_AUTH, ALL_SIGALGS, &anchors, inters, time)
         .is_ok()
     {
         info!("Cert verification Pass\n");

diff --git a/spdmlib/src/crypto/spdm_ring/dhe_impl.rs b/spdmlib/src/crypto/spdm_ring/dhe_impl.rs
@@ -38,15 +38,9 @@ impl SpdmDheKeyExchange for SpdmDheKeyExchangeP256 {
         let peer_public_key =
             ring::agreement::UnparsedPublicKey::new(&ring::agreement::ECDH_P256, pubkey.as_ref());
         let mut final_key = BytesMutStrubbed::new();
-        match ring::agreement::agree_ephemeral(
-            self.0,
-            &peer_public_key,
-            ring::error::Unspecified,
-            |key_material| {
-                final_key.extend_from_slice(key_material);
-                Ok(())
-            },
-        ) {
+        match ring::agreement::agree_ephemeral(self.0, &peer_public_key, |key_material| {
+            final_key.extend_from_slice(key_material);
+        }) {
             Ok(()) => Some(SpdmDheFinalKeyStruct::from(final_key)),
             Err(_) => None,
         }
@@ -82,15 +76,9 @@ impl SpdmDheKeyExchange for SpdmDheKeyExchangeP384 {
         let peer_public_key =
             ring::agreement::UnparsedPublicKey::new(&ring::agreement::ECDH_P384, pubkey.as_ref());
         let mut final_key = BytesMutStrubbed::new();
-        match ring::agreement::agree_ephemeral(
-            self.0,
-            &peer_public_key,
-            ring::error::Unspecified,
-            |key_material| {
-                final_key.extend_from_slice(key_material);
-                Ok(())
-            },
-        ) {
+        match ring::agreement::agree_ephemeral(self.0, &peer_public_key, |key_material| {
+            final_key.extend_from_slice(key_material);
+        }) {
             Ok(()) => Some(SpdmDheFinalKeyStruct::from(final_key)),
             Err(_) => None,
         }

diff --git a/spdmlib/src/crypto/spdm_ring/hkdf_impl.rs b/spdmlib/src/crypto/spdm_ring/hkdf_impl.rs
@@ -50,7 +50,7 @@ fn hkdf_expand(
         _ => return None,
     }?;
 
-    if prk.data_size as usize != algo.hmac_algorithm().digest_algorithm().output_len {
+    if prk.data_size as usize != algo.hmac_algorithm().digest_algorithm().output_len() {
         return None;
     }
 

diff --git a/test/spdm-emu/Cargo.toml b/test/spdm-emu/Cargo.toml
@@ -8,9 +8,9 @@ edition = "2018"
 
 [dependencies]
 log = "0.4.13"
-ring = { version = "0.16.20" }
-webpki = { version = "0.22.0", default-features = false, features = ["alloc"]}
-untrusted = { version = "0.7.1" }
+ring = { version = "0.17.6" }
+webpki = { version = "0.22.4", default-features = false, features = ["alloc"]}
+untrusted = { version = "0.9.0" }
 codec = { path = "../../codec" }
 spdmlib = { path = "../../spdmlib", default-features = false }
 mctp_transport = { path = "../../mctp_transport" }

diff --git a/test/spdm-emu/src/crypto_callback.rs b/test/spdm-emu/src/crypto_callback.rs
@@ -106,9 +106,9 @@ fn sign_ecdsa_asym_algo(
     };
     let der_file = std::fs::read(key_file_path).expect("unable to read key der!");
     let key_bytes = der_file.as_slice();
-
+    let rng = ring::rand::SystemRandom::new();
     let key_pair: ring::signature::EcdsaKeyPair =
-        ring::signature::EcdsaKeyPair::from_pkcs8(algorithm, key_bytes).ok()?;
+        ring::signature::EcdsaKeyPair::from_pkcs8(algorithm, key_bytes, &rng).ok()?;
 
     let rng = ring::rand::SystemRandom::new();
 
@@ -153,7 +153,7 @@ fn sign_rsa_asym_algo(
     let key_pair: ring::signature::RsaKeyPair =
         ring::signature::RsaKeyPair::from_der(key_bytes).ok()?;
 
-    if key_len != key_pair.public_modulus_len() {
+    if key_len != key_pair.public().modulus_len() {
         panic!();
     }
 

diff --git a/test/spdmlib-test/Cargo.toml b/test/spdmlib-test/Cargo.toml
@@ -9,7 +9,7 @@ edition = "2021"
 spdmlib = { path = "../../spdmlib", default-features = false, features=["spdm-ring"] }
 codec = { path = "../../codec", features = ["alloc"] }
 log = "0.4.13"
-ring = { version = "0.16.20" }
+ring = { version = "0.17.6" }
 bytes = { version="1", default-features=false }
 futures = { version = "0.3", default-features = false }
 async-trait = "0.1.71"

diff --git a/test/spdmlib-test/src/common/secret_callback.rs b/test/spdmlib-test/src/common/secret_callback.rs
@@ -304,8 +304,9 @@ fn sign_ecdsa_asym_algo(
     let der_file = std::fs::read(key_file_path).expect("unable to read key der!");
     let key_bytes = der_file.as_slice();
 
+    let rng = ring::rand::SystemRandom::new();
     let key_pair: ring::signature::EcdsaKeyPair =
-        ring::signature::EcdsaKeyPair::from_pkcs8(algorithm, key_bytes).unwrap();
+        ring::signature::EcdsaKeyPair::from_pkcs8(algorithm, key_bytes, &rng).unwrap();
 
     let rng = ring::rand::SystemRandom::new();