Use vendored version of ring in remote attestation (#2661)

jul-sh · web-flow · commit 76f13134df3d · 2022-04-08T14:45:46.000+01:00
* Clarify cargo license for all ring versions

* Patch the ring build script to enable builds from source code

Turns out that ring includes pre-generated objects in cargo releases. It uses logic in its build scripts to determine whether the build is from a version published on cargo, or from source code. This logic is somewhat naively implemented by checking for the absence of a `.git` directory. This check falesly leads ring to believe it is building from a version published on crates.io, and fails when it tries to include the pregenerated files not present in the source code. To build from source code we must patch this flag.

* Use the vendored version of ring

Requires updating our code to accomodate API changes since the vendored version is newer

* bust CI cache

* Allow deprecated code inside of vendored ring. It includes code that is, and our CI fails on warnings
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -64,7 +64,7 @@ jobs:
         uses: actions/cache@v2
         env:
           # Increment this value to invalidate previous cache entries.
-          CACHE_VERSION: 2
+          CACHE_VERSION: 3
         with:
           path: |
             ./cargo-cache/bin
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/deny.toml b/deny.toml
@@ -43,7 +43,6 @@ copyleft = "deny"
 
 [[licenses.clarify]]
 name = "ring"
-version = "*"
 expression = "MIT AND ISC AND OpenSSL"
 license-files = [{ path = "LICENSE", hash = 3171872035 }]
 
diff --git a/oak_functions/loader/fuzz/Cargo.lock b/oak_functions/loader/fuzz/Cargo.lock
diff --git a/remote_attestation/rust/Cargo.toml b/remote_attestation/rust/Cargo.toml
@@ -14,7 +14,7 @@ anyhow = { version = "*", default-features = false }
 bytes = { version = "*", default-features = false }
 log = "*"
 prost = { version = "*", default-features = false, features = ["prost-derive"] }
-ring = "*"
+ring = { path = "../../third_party/ring" }
 
 [build-dependencies]
 prost-build = "*"
diff --git a/remote_attestation/rust/src/crypto.rs b/remote_attestation/rust/src/crypto.rs
@@ -234,11 +234,10 @@ impl KeyNegotiator {
     ) -> anyhow::Result<(EncryptionKey, DecryptionKey)> {
         let type_ = self.type_.clone();
         let self_public_key = self.public_key().context("Couldn't get self public key")?;
-        let (encryption_key, decryption_key) = agreement::agree_ephemeral(
+        agreement::agree_ephemeral(
             self.private_key,
             &agreement::UnparsedPublicKey::new(KEY_AGREEMENT_ALGORITHM, peer_public_key),
-            anyhow!("Couldn't derive session keys"),
-            |key_material| {
+            |key_material| -> anyhow::Result<(EncryptionKey, DecryptionKey)> {
                 let key_material = key_material
                     .try_into()
                     .map_err(anyhow::Error::msg)
@@ -251,44 +250,54 @@ impl KeyNegotiator {
                 match type_ {
                     // On the server side `self_public_key` is the server key.
                     KeyNegotiatorType::Server => {
-                        let encryption_key = Self::key_derivation_function(
-                            key_material,
-                            SERVER_KEY_PURPOSE,
-                            &self_public_key,
-                            &peer_public_key,
+                        let encryption_key = EncryptionKey(
+                            Self::key_derivation_function(
+                                key_material,
+                                SERVER_KEY_PURPOSE,
+                                &self_public_key,
+                                &peer_public_key,
+                            )
+                            .context("Couldn't derive decryption key")?,
                         );
-                        let decryption_key = Self::key_derivation_function(
-                            key_material,
-                            CLIENT_KEY_PURPOSE,
-                            &self_public_key,
-                            &peer_public_key,
+                        let decryption_key = DecryptionKey(
+                            Self::key_derivation_function(
+                                key_material,
+                                CLIENT_KEY_PURPOSE,
+                                &self_public_key,
+                                &peer_public_key,
+                            )
+                            .context("Couldn't derive encryption key")?,
                         );
                         Ok((encryption_key, decryption_key))
                     }
                     // On the client side `peer_public_key` is the server key.
                     KeyNegotiatorType::Client => {
-                        let encryption_key = Self::key_derivation_function(
-                            key_material,
-                            CLIENT_KEY_PURPOSE,
-                            &peer_public_key,
-                            &self_public_key,
+                        let encryption_key = EncryptionKey(
+                            Self::key_derivation_function(
+                                key_material,
+                                CLIENT_KEY_PURPOSE,
+                                &peer_public_key,
+                                &self_public_key,
+                            )
+                            .context("Couldn't derive decryption key")?,
                         );
-                        let decryption_key = Self::key_derivation_function(
-                            key_material,
-                            SERVER_KEY_PURPOSE,
-                            &peer_public_key,
-                            &self_public_key,
+                        let decryption_key = DecryptionKey(
+                            Self::key_derivation_function(
+                                key_material,
+                                SERVER_KEY_PURPOSE,
+                                &peer_public_key,
+                                &self_public_key,
+                            )
+                            .context("Couldn't derive encryption key")?,
                         );
                         Ok((encryption_key, decryption_key))
                     }
                 }
             },
         )
-        .context("Couldn't agree on session keys")?;
-        Ok((
-            EncryptionKey(encryption_key.context("Couldn't derive encryption key")?),
-            DecryptionKey(decryption_key.context("Couldn't derive decryption key")?),
-        ))
+        .map_err(anyhow::Error::msg)
+        .context("Couldn't derive session keys")?
+        .context("Couldn't agree on session keys")
     }
 
     /// Derives a session key from `key_material` using HKDF.
@@ -351,8 +360,9 @@ impl Signer {
         let rng = ring::rand::SystemRandom::new();
         let key_pair_pkcs8 = EcdsaKeyPair::generate_pkcs8(SIGNING_ALGORITHM, &rng)
             .map_err(|error| anyhow!("Couldn't generate PKCS#8 key pair: {:?}", error))?;
-        let key_pair = EcdsaKeyPair::from_pkcs8(SIGNING_ALGORITHM, key_pair_pkcs8.as_ref())
-            .map_err(|error| anyhow!("Couldn't parse generated key pair: {:?}", error))?;
+        let key_pair =
+            EcdsaKeyPair::from_pkcs8(SIGNING_ALGORITHM, key_pair_pkcs8.as_ref(), &rng)
+                .map_err(|error| anyhow!("Couldn't parse generated key pair: {:?}", error))?;
 
         Ok(Self { key_pair })
     }
diff --git a/third_party/ring/build.rs b/third_party/ring/build.rs
@@ -330,7 +330,17 @@ fn ring_build_rs_main() {
     // If `.git` doesn't exist then assume that this is a packaged build where
     // we want to optimize for minimizing the build tools required: No Perl,
     // no nasm, etc.
-    let use_pregenerated = !is_git;
+    //
+    // Oak Note:
+    // ring includes pre-generated objects in cargo releases. It uses logic in
+    // its build scripts to determine whether the build is from a version
+    // published on cargo, or from source code. This logic is somewhat naively
+    // implemented by checking for the absence of a `.git` directory. This
+    // check falesly leads ring to believe it is building from a version
+    // published on crates.io, and fails when it tries to include the
+    // pregenerated files not present in the source code. To build from source
+    // code we must patch this flag.
+    let use_pregenerated = false;
 
     // During local development, force warnings in non-Rust code to be treated
     // as errors. Since warnings are highly compiler-dependent and compilers
diff --git a/third_party/ring/src/lib.rs b/third_party/ring/src/lib.rs
@@ -48,7 +48,10 @@
     missing_debug_implementations,
     non_camel_case_types,
     non_snake_case,
-    unsafe_code
+    unsafe_code,
+    // Oak Note: Do not warn when building deprecated code. Ring includes
+    // code that is, and our CI fails on warnings
+    deprecated
 )]
 // `#[derive(...)]` uses `trivial_numeric_casts` and `unused_qualifications`
 // internally.
