-
Notifications
You must be signed in to change notification settings - Fork 120
Make remote_attestation crate no_std compatible #2556
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,15 +5,16 @@ authors = ["Ivan Petrov <[email protected]>"] | |
edition = "2021" | ||
license = "Apache-2.0" | ||
|
||
[features] | ||
default = [] | ||
std = ["anyhow/std", "prost/std"] | ||
|
||
[dependencies] | ||
anyhow = "*" | ||
bincode = "*" | ||
anyhow = { version = "*", default-features = false } | ||
bytes = { version = "*", default-features = false } | ||
log = "*" | ||
prost = "*" | ||
prost = { version = "*", default-features = false, features = ["prost-derive"] } | ||
ring = "*" | ||
serde = { version = "*", features = ["derive"] } | ||
serde-big-array = { version = "*", features = ["const-generics"] } | ||
sha2 = "*" | ||
|
||
[build-dependencies] | ||
prost-build = "*" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,16 +20,17 @@ | |
// protocol. | ||
|
||
use crate::message::EncryptedData; | ||
use alloc::{format, vec, vec::Vec}; | ||
use anyhow::{anyhow, Context}; | ||
use core::convert::TryInto; | ||
use ring::{ | ||
aead::{self, BoundKey}, | ||
agreement, | ||
digest::{digest, SHA256}, | ||
hkdf::{Salt, HKDF_SHA256}, | ||
rand::{SecureRandom, SystemRandom}, | ||
signature::{EcdsaKeyPair, EcdsaSigningAlgorithm, EcdsaVerificationAlgorithm, KeyPair}, | ||
}; | ||
use sha2::{digest::Digest, Sha256}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, but |
||
use std::convert::TryInto; | ||
|
||
/// Length of the encryption nonce. | ||
/// `ring::aead` uses 96-bit (12-byte) nonces. | ||
|
@@ -339,6 +340,7 @@ pub struct Signer { | |
|
||
impl Signer { | ||
pub fn create() -> anyhow::Result<Self> { | ||
// TODO(#2557): Ensure SystemRandom work when building for x86_64 UEFI targets. | ||
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))?; | ||
|
@@ -397,11 +399,8 @@ impl SignatureVerifier { | |
|
||
/// Computes a SHA-256 digest of `input` and returns it in a form of raw bytes. | ||
pub fn get_sha256(input: &[u8]) -> [u8; SHA256_HASH_LENGTH] { | ||
let mut hasher = Sha256::new(); | ||
hasher.update(&input); | ||
hasher | ||
.finalize() | ||
.as_slice() | ||
digest(&SHA256, input) | ||
.as_ref() | ||
.try_into() | ||
.expect("Incorrect SHA-256 hash length") | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we are not restricting versions, is it possible that the future versions of our dependencies will become
std
instead ofno_std
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is possible. I am not sure whether there is a good way to detect that automatically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't this crate fail to compile in that case (at least with the defaul features, i.e. alloc instead of std)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
prost
removesno_std
compatibility and removes thestd
feature so that it always pulls instd
I think it will still compile.std
reexports thecore
andalloc
types, so the types will still be compatible.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drive-by: the best way I've found to check
no_std
compatibility is to have a CI step that cross-compiles for a target with nostd
available.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. At the moment the build requires a target OS because that is required by
ring
. We should be able to build it for a UEFI target once briansmith/ring#1406 has been merged.