Use vendored version of ring in remote attestation #2661
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jul-sh
force-pushed
the
uefi-ring
branch
7 times, most recently
from
ba87283
to
1e530b7
Compare
jul-sh
changed the title
jul-sh
added
the
blocked
jul-sh
force-pushed
the
uefi-ring
branch
4 times, most recently
from
ac9b353
to
44f0791
Compare
jul-sh
commented
Apr 6, 2022
| @@ -43,7 +43,6 @@ copyleft = "deny" | |||
|
|
|||
| [[licenses.clarify]] | |||
| name = "ring" | |||
| version = "*" | |||
There was a problem hiding this comment.
The reason this needed removing is somewhat counterintuitive:
In cargo, version = "*" does not in fact mean "any version", but instead "any version that is published on crates.io". We need to remove this (optional) key to truly check any version (in our case our vendored version).
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.
jul-sh
removed
the
blocked
Requires updating our code to accomodate API changes since the vendored version is newer
…is, and our CI fails on warnings
jul-sh
commented
Apr 8, 2022
Comment on lines
+237
to
+240
| 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)> { |
There was a problem hiding this comment.
The updated ring version no longer takes an error as a parameter, but instead returns an additional result
jul-sh
requested review from
mariaschett,
conradgrobler and
andrisaar
and removed request for
a team
jul-sh
added
the
approval/any
|
Reproducibility Index: Reproducibility Index diff: diff --git a/reproducibility_index b/reproducibility_index
index 3b985fb..728a5d2 100644
--- a/reproducibility_index
+++ b/reproducibility_index
@@ -1,2 +1,2 @@
-a857b407160f424bfc3a9d92b63861f3dc3fe7e49ee160f8febc18434619d0a9 ./target/x86_64-unknown-linux-musl/release/oak_functions_loader_base
-f2a3fd1fcfd96aae2d64c97116f7613a4670589cf11b9416595d18d5b0c59924 ./target/x86_64-unknown-linux-musl/release/oak_functions_loader_unsafe
+8a7d039cbdea7691dee83744f996b941eb3f174a9eb9376c5b8e06c49fb38f21 ./target/x86_64-unknown-linux-musl/release/oak_functions_loader_base
+9142c165d3c22874be1293d4f1c7f15341d18cc363f79f77931b95668d5c98db ./target/x86_64-unknown-linux-musl/release/oak_functions_loader_unsafe
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uses the vendored version of ring (added in #2681) in our remote attestation crate. This necessitated the following changes: