upgrade ring to 0.17.5

- Upgrade `ring` git submodule. - Update patch and patch script `preparation.h`. - Update the use of `PublicKey`and `EcdsaKeyPair` to follow the latest API. - Update `Cargo.toml`s and `Cargo.lock`. Signed-off-by: Jiaqi Gao <[email protected]>
confidential-containers · Oct 31, 2023 · a462a60 · a462a60
1 parent 3f18d9f
commit a462a60
Show file tree

Hide file tree

Showing 11 changed files with 126 additions and 105 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/cc-measurement/Cargo.toml b/cc-measurement/Cargo.toml
@@ -7,7 +7,7 @@ edition = "2021"
 
 [dependencies]
 sha2 = { version = "0.10.6", default-features = false, features = ["force-soft"], optional = true }
-ring = { version = "0.16.20", default-features = false, features = ["alloc"], optional = true }
+ring = { version = "0.17.5", default-features = false, features = ["alloc"], optional = true }
 zerocopy = "0.6.0"
 
 [features]

diff --git a/library/patches/ring.diff b/library/patches/ring.diff
@@ -1,90 +1,109 @@
 diff --git a/Cargo.toml b/Cargo.toml
-index c9daac82e..f9e578e39 100644
+index 3378dc84b..f65fd70bd 100644
 --- a/Cargo.toml
 +++ b/Cargo.toml
-@@ -302,6 +302,7 @@ name = "ring"
+@@ -171,7 +171,7 @@ all-features = true
+ name = "ring"
 
  [dependencies]
- untrusted = { version = "0.7.1" }
-+getrandom = { version = "0.2.8", features = ["rdrand"] }
+-getrandom = { version = "0.2.10" }
++getrandom = { version = "0.2.10", features = ["rdrand"] }
+ untrusted = { version = "0.9" }
 
- [target.'cfg(any(target_arch = "x86",target_arch = "x86_64", all(any(target_arch = "aarch64", target_arch = "arm"), any(target_os = "android", target_os = "fuchsia", target_os = "linux"))))'.dependencies]
- spin = { version = "0.5.2", default-features = false }
-@@ -327,7 +328,7 @@ libc = { version = "0.2.80", default-features = false }
-
- # Keep this in sync with `[dependencies]` in pregenerate_asm/Cargo.toml.
- [build-dependencies]
--cc = { version = "1.0.62", default-features = false }
-+cc = { version = "1.0.63", default-features = false }
-
- [features]
- # These features are documented in the top-level module's documentation.
+ [target.'cfg(any(target_arch = "x86",target_arch = "x86_64", all(any(target_arch = "aarch64", target_arch = "arm"), any(target_os = "android", target_os = "fuchsia", target_os = "linux", target_os = "windows"))))'.dependencies]
 diff --git a/build.rs b/build.rs
-index a5a8e1995..c67e4bfb0 100644
+index f7b94108b..3bdc8cd29 100644
 --- a/build.rs
 +++ b/build.rs
-@@ -580,7 +580,7 @@ fn cc(
-     //
+@@ -121,7 +121,9 @@ fn cpp_flags(compiler: &cc::Tool) -> &'static [&'static str] {
+             "-Wenum-compare",
+             "-Wfloat-equal",
+             "-Wformat=2",
+-            "-Winline",
++            // Clear the `-Winline` because warnings will be treated as errors
++            // when `ring` is used as git submodules.
++            // "-Winline",
+             "-Winvalid-pch",
+             "-Wmissing-field-initializers",
+             "-Wmissing-include-dirs",
+@@ -260,6 +262,8 @@ const LINUX_ABI: &[&str] = &[
+     "linux",
+     "redox",
+     "solaris",
++    // For `x86_64-unknown-none` target
++    "none",
+ ];
+
+ /// Operating systems that have the same ABI as macOS on every architecture
+@@ -604,16 +608,29 @@ fn configure_cc(c: &mut cc::Build, target: &Target, include_dir: &Path) {
      // poly1305_vec.c requires <emmintrin.h> which requires <stdlib.h>.
-     if (target.arch == "wasm32" && target.os == "unknown")
--        || (target.os == "linux" && is_musl && target.arch != "x86_64")
-+        || (target.os == "linux" && is_musl && target.arch != "x86_64" || target.os == "none")
+     if (target.arch == "wasm32")
+         || (target.os == "linux" && target.is_musl && target.arch != "x86_64")
++        || (target.os == "none")
      {
          if let Ok(compiler) = c.try_get_compiler() {
              // TODO: Expand this to non-clang compilers in 0.17.0 if practical.
-@@ -589,6 +589,9 @@ fn cc(
-                 let _ = c.define("GFp_NOSTDLIBINC", "1");
+             if compiler.is_like_clang() {
+                 let _ = c.flag("-nostdlibinc");
++                // Required on windows for cross compilation to `x86_64-unknown-none`
++                let _ = c.flag("-ffreestanding");
+                 let _ = c.define("RING_CORE_NOSTDLIBINC", "1");
              }
          }
-+        if target.os == "none" {
-+            let _ = c.flag("-ffreestanding");
+     }
+
++    // `clang` does not define `__ELF__` for `x86_64-unknown-none` target.
++    // Manually define it.
++    if target.os == "none" {
++        if let Ok(compiler) = c.try_get_compiler() {
++            if compiler.is_like_clang() {
++                let _ = c.define("__ELF__", None);
++            }
 +        }
++    }
++
+     if target.force_warnings_into_errors {
+         c.warnings_into_errors(true);
      }
+@@ -645,7 +662,7 @@ fn nasm(file: &Path, arch: &str, include_dir: &Path, out_file: &Path) -> Command
+         std::path::MAIN_SEPARATOR,
+     )));
 
-     if warnings_are_errors {
-@@ -626,7 +629,7 @@ fn nasm(file: &Path, arch: &str, out_file: &Path) -> Command {
-         "x86" => ("win32"),
-         _ => panic!("unsupported arch: {}", arch),
-     };
--    let mut c = Command::new("./target/tools/nasm");
+-    let mut c = Command::new("./target/tools/windows/nasm/nasm");
 +    let mut c = Command::new("nasm");
      let _ = c
          .arg("-o")
          .arg(out_file.to_str().expect("Invalid path"))
 diff --git a/src/rand.rs b/src/rand.rs
-index 9d1864fa1..6ac5cc727 100644
+index 78f4bdc3c..2d1028b84 100644
 --- a/src/rand.rs
 +++ b/src/rand.rs
-@@ -195,6 +195,9 @@ use self::darwin::fill as fill_impl;
- #[cfg(any(target_os = "fuchsia"))]
- use self::fuchsia::fill as fill_impl;
+@@ -148,6 +148,7 @@ impl crate::sealed::Sealed for SystemRandom {}
+             all(target_os = "unknown", feature = "wasm32_unknown_unknown_js")
+         )
+     ),
++    all(target_arch = "x86_64", target_os = "none"),
+ ))]
+ impl sealed::SecureRandom for SystemRandom {
+     #[inline(always)]
+diff --git a/third_party/fiat/curve25519_64_adx.h b/third_party/fiat/curve25519_64_adx.h
+index 9dcbb69fa..0906fda81 100644
+--- a/third_party/fiat/curve25519_64_adx.h
++++ b/third_party/fiat/curve25519_64_adx.h
+@@ -1,7 +1,6 @@
+ #include <stdbool.h>
+ #include <stdint.h>
+ #include <immintrin.h>
+-#include <string.h>
 
-+#[cfg(any(target_os = "none"))]
-+use self::no_std::fill as fill_impl;
-+
- #[cfg(any(target_os = "android", target_os = "linux"))]
- mod sysrand_chunk {
-     use crate::{c, error};
-@@ -431,3 +434,22 @@ mod fuchsia {
-         fn zx_cprng_draw(buffer: *mut u8, length: usize);
-     }
- }
-+
-+#[cfg(any(target_os = "none"))]
-+mod no_std {
-+    use crate::error;
-+
-+    pub fn fill(dest: &mut [u8]) -> Result<(), error::Unspecified> {
-+        fill_impl(dest)
-+    }
-+
-+    #[cfg(not(any(target_arch = "x86_64")))]
-+    fn fill_impl(dest: &mut [u8]) -> Result<(), error::Unspecified> {
-+        Err(error::Unspecified)
-+    }
-+
-+    #[cfg(any(target_arch = "x86_64"))]
-+    fn fill_impl(dest: &mut [u8]) -> Result<(), error::Unspecified> {
-+        getrandom::getrandom(dest).map_err(|_| error::Unspecified)
-+    }
-+}
+ typedef uint64_t fe4[4];
+ typedef uint8_t fiat_uint1;
+@@ -469,7 +468,7 @@ __attribute__((target("adx,bmi2")))
+ void x25519_scalar_mult_adx(uint8_t out[32], const uint8_t scalar[32],
+                             const uint8_t point[32]) {
+   uint8_t e[32];
+-  memcpy(e, scalar, 32);
++  OPENSSL_memcpy(e, scalar, 32);
+   e[0] &= 248;
+   e[31] &= 127;
+   e[31] |= 64;
diff --git a/library/ring b/library/ring
diff --git a/sh_script/preparation.sh b/sh_script/preparation.sh
@@ -3,7 +3,7 @@
 preparation() {
     # apply the patch set for ring
     pushd library/ring
-    git reset --hard 9cc0d45f4d8521f467bb3a621e74b1535e118188
+    git reset --hard c3fda8b4dd57d658923c397c6cfaa33591f6f256
     git clean -f -d
     patch -p 1 -i ../patches/ring.diff
     popd

diff --git a/td-shim-tools/Cargo.toml b/td-shim-tools/Cargo.toml
@@ -52,7 +52,7 @@ der = { version = "0.4.5", features = ["oid"], optional = true }
 env_logger = { version = "0.9.0", optional = true }
 log = { version = "0.4.5", optional = true }
 td-loader = { path = "../td-loader", optional = true }
-ring = { version = "0.16.20", optional = true }
+ring = { version = "0.17.5", optional = true }
 serde_json = { version = "1.0", optional = true }
 serde = { version = "1.0", features = ["derive"], optional = true }
 hex = { version = "0.4", features = ["serde"], optional = true }

diff --git a/td-shim-tools/src/bin/td-shim-sign-payload/main.rs b/td-shim-tools/src/bin/td-shim-sign-payload/main.rs
@@ -12,7 +12,10 @@ use std::{env, io, path::Path};
 use clap::ArgAction;
 use env_logger::Env;
 use log::{error, trace, LevelFilter};
-use ring::signature::{EcdsaKeyPair, RsaKeyPair, ECDSA_P384_SHA384_FIXED_SIGNING};
+use ring::{
+    rand,
+    signature::{EcdsaKeyPair, RsaKeyPair, ECDSA_P384_SHA384_FIXED_SIGNING},
+};
 use td_layout::build_time::TD_SHIM_PAYLOAD_SIZE;
 use td_shim_tools::signer::{PayloadSigner, SigningAlgorithm};
 use td_shim_tools::{InputData, OutputFile};
@@ -107,12 +110,16 @@ fn main() -> io::Result<()> {
             SigningAlgorithm::Rsapss3072Sha384(rsa_key_pair)
         }
         "ECDSA_NIST_P384_SHA384" => {
-            let ecdsa_key_pair =
-                EcdsaKeyPair::from_pkcs8(&ECDSA_P384_SHA384_FIXED_SIGNING, private.as_bytes())
-                    .map_err(|e| {
-                        error!("Can not load DSA private key from {}: {}", private_file, e);
-                        io::Error::new(io::ErrorKind::Other, "Can not load DSA private key")
-                    })?;
+            let rng = rand::SystemRandom::new();
+            let ecdsa_key_pair = EcdsaKeyPair::from_pkcs8(
+                &ECDSA_P384_SHA384_FIXED_SIGNING,
+                private.as_bytes(),
+                &rng,
+            )
+            .map_err(|e| {
+                error!("Can not load DSA private key from {}: {}", private_file, e);
+                io::Error::new(io::ErrorKind::Other, "Can not load DSA private key")
+            })?;
             SigningAlgorithm::EcdsaNistP384Sha384(ecdsa_key_pair)
         }
         _ => {

diff --git a/td-shim-tools/src/signer.rs b/td-shim-tools/src/signer.rs
@@ -6,6 +6,7 @@ use std::io;
 use std::mem::size_of;
 use std::vec::Vec;
 
+use der::Decodable;
 use log::error;
 use ring::rand;
 use ring::signature::{EcdsaKeyPair, KeyPair, RsaKeyPair, RSA_PSS_SHA384};
@@ -14,6 +15,8 @@ use td_shim::secure_boot::{
     PAYLOAD_SIGN_RSA_PSS_3072_SHA384, SIGNED_PAYLOAD_FILE_HEADER_GUID,
 };
 
+use crate::public_key::RsaPublicKeyInfo;
+
 /// Type of public key.
 pub enum SigningAlgorithm {
     Rsapss3072Sha384(RsaKeyPair),
@@ -52,25 +55,23 @@ impl<'a> PayloadSigner<'a> {
 
         match &self.algorithm {
             SigningAlgorithm::Rsapss3072Sha384(rsa_keypair) => {
-                let modulus = rsa_keypair
-                    .public_key()
-                    .modulus()
-                    .big_endian_without_leading_zero();
-                if rsa_keypair.public_modulus_len() != 384 {
+                let public = rsa_keypair.public().as_ref();
+                let public_der = RsaPublicKeyInfo::from_der(public).map_err(|_| {
+                    io::Error::new(io::ErrorKind::InvalidInput, "invalid RSA public key")
+                })?;
+                let modulus = public_der.modulus.as_bytes();
+                if rsa_keypair.public().modulus_len() != 384 {
                     error!(
                         "Invalid RSA public modulus length: {}",
-                        rsa_keypair.public_modulus_len()
+                        rsa_keypair.public().modulus_len()
                     );
                     return Err(io::Error::new(
                         io::ErrorKind::InvalidInput,
                         "invalid RSA public modulus length",
                     ));
                 }
 
-                let exponent = rsa_keypair
-                    .public_key()
-                    .exponent()
-                    .big_endian_without_leading_zero();
+                let exponent = public_der.exponents.as_bytes();
                 if exponent.len() > PAYLOAD_SIGN_RSA_EXPONENT_SIZE {
                     error!(
                         "Invalid RSA exponent length: {}, max {}",
@@ -87,7 +88,7 @@ impl<'a> PayloadSigner<'a> {
                 exp_bytes[PAYLOAD_SIGN_RSA_EXPONENT_SIZE - exponent.len()..]
                     .copy_from_slice(exponent);
 
-                let mut signature: Vec<u8> = vec![0; rsa_keypair.public_modulus_len()];
+                let mut signature: Vec<u8> = vec![0; rsa_keypair.public().modulus_len()];
                 rsa_keypair
                     .sign(&RSA_PSS_SHA384, &rng, &self.signed_image, &mut signature)
                     .map_err(|e| {

diff --git a/td-shim/Cargo.toml b/td-shim/Cargo.toml
@@ -30,7 +30,7 @@ zerocopy = "0.6.0"
 td-loader = { path = "../td-loader", optional = true }
 linked_list_allocator = { version = "0.10", optional = true }
 log = { version = "0.4.13", features = ["release_max_level_off"], optional = true }
-ring = { version = "0.16.20", default-features = false, features = ["alloc"], optional = true }
+ring = { version = "0.17.5", default-features = false, features = ["alloc"], optional = true }
 spin = { version = "0.9.2", optional = true }
 td-exception =  { path = "../td-exception", features = ["tdx"], optional = true }
 td-logger =  { path = "../td-logger", optional = true }

diff --git a/td-shim/src/bin/td-shim/main.rs b/td-shim/src/bin/td-shim/main.rs
@@ -428,3 +428,9 @@ fn secure_boot_verify_payload<'a>(
     return PayloadVerifier::get_payload_image(payload)
         .expect("Unable to get payload image from signed binary");
 }
+
+#[cfg(feature = "secure-boot")]
+#[no_mangle]
+extern "C" fn __assert_fail() {
+    panic!("__assert_fail");
+}
diff --git a/tests/test-td-payload/Cargo.toml b/tests/test-td-payload/Cargo.toml
@@ -22,7 +22,7 @@ scroll = { version = "0.10.0", default-features = false, features = ["derive"]}
 serde = { version = "1.0", default-features = false, features = ["derive"]}
 serde_json = { version = "1.0", default-features = false, features = ["alloc"] }
 x86 = { version = "0.47.0" }
-ring = { version = "0.16.20", default-features = false, features = ["alloc"] }
+ring = { version = "0.17.5", default-features = false, features = ["alloc"] }
 td-shim = { path = "../../td-shim" }
 td-payload = { path = "../../td-payload", features = ["tdx","cet-shstk","stack-guard"] }
 zerocopy = "0.6.0"