-
Notifications
You must be signed in to change notification settings - Fork 33
feat: support SEV-SNP Report Version 5 #115
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.
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,9 +20,9 @@ pub struct KeyArgs { | |||||||||||
| #[arg(short, long, value_name = "vmpl", default_value = "1")] | ||||||||||||
| pub vmpl: Option<u32>, | ||||||||||||
|
|
||||||||||||
| /// Specify which Guest Field Select bits to enable. It is a 6 digit binary string. For each bit, 0 denotes off and 1 denotes on. | ||||||||||||
| /// The least significant (rightmost) bit is Guest Policy followed by Image ID, Family ID, Measurement, SVN, TCB Version which is the most significant (leftmost) bit. | ||||||||||||
| #[arg(short, long = "guest_field_select", value_name = "######")] | ||||||||||||
| /// Specify which Guest Field Select bits to enable. It is a 6 or 7 digit binary string. For each bit, 0 denotes off and 1 denotes on. | ||||||||||||
| /// The least significant (rightmost) bit is Guest Policy followed by Image ID, Family ID, Measurement, SVN, TCB Version, and Launch Mitigation Vector which is the most significant (leftmost) bit. | ||||||||||||
| #[arg(short, long = "guest_field_select", value_name = "#######")] | ||||||||||||
| pub gfs: Option<String>, | ||||||||||||
|
|
||||||||||||
| /// Specify the guest SVN to mix into the key. Must not exceed the guest SVN provided at launch in the ID block. | ||||||||||||
|
|
@@ -32,6 +32,10 @@ pub struct KeyArgs { | |||||||||||
| /// Specify the TCB version to mix into the derived key. Must not exceed CommittedTcb. | ||||||||||||
| #[arg(short, long = "tcb_version")] | ||||||||||||
| pub tcbv: Option<u64>, | ||||||||||||
|
|
||||||||||||
| /// Specify the launch mitigation vector to mix into the derived key. | ||||||||||||
| #[arg(short, long = "launch_mit_vector")] | ||||||||||||
| pub lmv: Option<u64>, | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| pub fn get_derived_key(args: KeyArgs) -> Result<()> { | ||||||||||||
|
|
@@ -54,12 +58,18 @@ pub fn get_derived_key(args: KeyArgs) -> Result<()> { | |||||||||||
|
|
||||||||||||
| let gfs = match args.gfs { | ||||||||||||
| Some(gfs) => { | ||||||||||||
| let value: u64 = u64::from_str_radix(gfs.as_str(), 2).unwrap(); | ||||||||||||
| if value <= 63 { | ||||||||||||
| value | ||||||||||||
| } else { | ||||||||||||
| return Err(anyhow::anyhow!("Invalid Guest Field Select option.")); | ||||||||||||
| if gfs.len() != 6 && gfs.len() != 7 { | ||||||||||||
| return Err(anyhow::anyhow!( | ||||||||||||
| "Invalid Guest Field Select option. Must be 6 or 7 digits." | ||||||||||||
| )); | ||||||||||||
| } | ||||||||||||
| if gfs.chars().any(|c| c != '0' && c != '1') { | ||||||||||||
| return Err(anyhow::anyhow!( | ||||||||||||
| "Invalid Guest Field Select option. Must be a binary string." | ||||||||||||
|
Member
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. Do we perhaps want to make this input hex? Seems like that would be more reasonable.
Contributor
Author
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. I agree — specifying this as a fixed-length hex string (16 characters for 64 bits) could reduce confusion compared to the current 6–7 bit binary format.
Member
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. I support the idea
Contributor
Author
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. If we switch the Alternatively, we could provide a parser like: fn parse_u64_multi_format(s: &str) -> Result<u64, ParseIntError> {
if let Some(hex) = s.strip_prefix("0x") {
u64::from_str_radix(hex, 16)
} else if let Some(bin) = s.strip_prefix("0b") {
u64::from_str_radix(bin, 2)
} else {
u64::from_str_radix(s, 10)
}
}and then use it as follows: #[arg(short, long = "guest_field_select", value_parser = parse_u64_multi_format)]
pub gfs: Option<u64>,This would allow us to support multiple formats. (Though requiring a
Member
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. I've been thinking about this, and I like the idea of supporting multiple formats. There isn’t really a single obvious way to provide this information. The reason GuestFieldSelect was made a binary string is that each field is represented by a binary number—1 meaning enabled and 0 meaning disabled. Meanwhile, the TCB is also a u64 field, but it has a completely different meaning since you’re providing values of the TCB, where each value can be expressed as a decimal. Then, the MIT VECTOR is a vector of values, so hex makes the most sense there. If you could extend the parser so we can see what that would look like, we could then implement the same approach in other functionality like this:
Contributor
Author
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. Thanks for the feedback! I'm thinking of implementing this as a separate module (e.g., For this PR, since its primary purpose is to add support for the new ABI version, I would suggest keeping (inheriting) the current implementation (
Member
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. Yes, maybe we will cut a minor release when this PR is merged and then make bigger changes when 7.0.0 and the new parser implementation is merged. |
||||||||||||
| )); | ||||||||||||
| } | ||||||||||||
| let value: u64 = u64::from_str_radix(gfs.as_str(), 2).unwrap(); | ||||||||||||
| value | ||||||||||||
| } | ||||||||||||
| None => 0, | ||||||||||||
| }; | ||||||||||||
|
|
@@ -68,10 +78,20 @@ pub fn get_derived_key(args: KeyArgs) -> Result<()> { | |||||||||||
|
|
||||||||||||
| let tcbv: u64 = args.tcbv.unwrap_or(0); | ||||||||||||
|
|
||||||||||||
| let request = DerivedKey::new(root_key_select, GuestFieldSelect(gfs), vmpl, gsvn, tcbv); | ||||||||||||
| let request = DerivedKey::new( | ||||||||||||
| root_key_select, | ||||||||||||
| GuestFieldSelect(gfs), | ||||||||||||
| vmpl, | ||||||||||||
| gsvn, | ||||||||||||
| tcbv, | ||||||||||||
| args.lmv, | ||||||||||||
| ); | ||||||||||||
|
|
||||||||||||
| let msg_ver = if args.lmv.is_some() { 2 } else { 1 }; | ||||||||||||
|
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. suggestion: The logic for msg_ver may need clarification for future maintainers. Please document the dependency between msg_ver and lmv, or ensure updates to lmv semantics are consistently reflected in this logic.
Suggested change
|
||||||||||||
|
|
||||||||||||
| let mut sev_fw = Firmware::open().context("failed to open SEV firmware device.")?; | ||||||||||||
| let derived_key: [u8; 32] = sev_fw | ||||||||||||
| .get_derived_key(None, request) | ||||||||||||
| .get_derived_key(Some(msg_ver), request) | ||||||||||||
| .context("Failed to request derived key")?; | ||||||||||||
|
|
||||||||||||
| // Create derived key path | ||||||||||||
|
|
||||||||||||
Uh oh!
There was an error while loading. Please reload this page.