Skip to content
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.

Sign up for GitHub

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

Temporarily support missing raw cmd line in the evidence #4958

Merged
merged 3 commits into from
Mar 25, 2024
Merged

Temporarily support missing raw cmd line in the evidence #4958

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
55b9fe7
Temporarily support missing raw cmd line in the evidence. Verificatio…
k-naliuka Mar 25, 2024
ee1c259
Address review comments
k-naliuka Mar 25, 2024
d4651d1
Fix tests
k-naliuka Mar 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 24 additions & 13 deletions oak_attestation_verification/src/verifier.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,14 +517,27 @@ fn verify_kernel_layer(
)
.context("kernel failed verification")?;

verify_regex(
values.kernel_raw_cmd_line.as_str(),
reference_values
.kernel_cmd_line_regex
.as_ref()
.context("no kernel command line regex reference values")?,
)
.context("kernel command line failed verification")?;
if let Some(kernel_raw_cmd_line) = values.kernel_raw_cmd_line.as_ref() {
verify_regex(
kernel_raw_cmd_line.as_str(),
reference_values
.kernel_cmd_line_regex
.as_ref()
.context("no kernel command line regex reference values")?,
)
.context("kernel command line failed verification")?;
} else {
// Support missing kernel_cmd_line_regex but only if the corresponding reference
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW this still feels backwards to me (i.e. we should start from the expected reference value). I know we agreed to clean it up later on anyway :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack :)

// value is set to skip. This is a temporary workaround until all clients are
// migrated.
anyhow::ensure!(
matches!(
reference_values.kernel_cmd_line_regex.as_ref().unwrap().r#type.as_ref(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also just noticed that this unwrap() call will crash/panic if no kernel_cmd_line_regex field is specified in the ReferenceValues, as opposed to generating a (recoverable) error., which seems undesirable given that that field is now optional.

Some(regex_reference_value::Type::Skip(_))
),
"No kernel_raw_cmd_line provided"
)
}

verify_measurement_digest(
values.init_ram_fs.as_ref().context("no initial RAM disk evidence value")?,
Expand Down Expand Up @@ -938,11 +951,9 @@ fn extract_kernel_values(claims: &ClaimsSet) -> anyhow::Result<KernelLayerData>
Some(value_to_raw_digest(extract_value(values, SETUP_DATA_MEASUREMENT_ID)?)?);
let kernel_cmd_line =
Some(value_to_raw_digest(extract_value(values, KERNEL_COMMANDLINE_MEASUREMENT_ID)?)?);
let kernel_raw_cmd_line = String::from(
extract_value(values, KERNEL_COMMANDLINE_ID)?
.as_text()
.expect("kernel cmd line CWT value is not a text"),
);
let kernel_raw_cmd_line = extract_value(values, KERNEL_COMMANDLINE_ID)
.ok()
.map(|v| String::from(v.as_text().expect("kernel_raw_cmd_line found but is not a string")));
let init_ram_fs = Some(value_to_raw_digest(extract_value(values, INITRD_MEASUREMENT_ID)?)?);
let memory_map = Some(value_to_raw_digest(extract_value(values, MEMORY_MAP_MEASUREMENT_ID)?)?);
let acpi = Some(value_to_raw_digest(extract_value(values, ACPI_MEASUREMENT_ID)?)?);
Expand Down
Binary file added oak_attestation_verification/testdata/rk_obsolete_evidence.binarypb
View file
Open in desktop
Binary file not shown.
20 changes: 20 additions & 0 deletions oak_attestation_verification/testdata/rk_obsolete_evidence.textproto
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may make sense to establish a new pattern, where instead of having a single 'obsolete' evidence copy and a single 'current' evidence copy you accumulate more and more evidence protos over time and simply name them by date (e.g. 'rk_evidence_20240312' etc.) and then always add more tests to verifier_tests.rs as new fields are added or old ones are deprecated, always keeping old tests with old Evidence and old ReferenceValues around to ensure that they continue to pass. That probably would ensure we have a better chance of catching backward-incompatible changes using this unit test.

Not necessarily something you need to address in this CL, but you could consider naming this as 'rk_evidence_20240312' or something similar already.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, done. Though I hope that with existing users migrated and our patterns becoming more stable we'll end up with just one version here again :)

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# proto-file: proto/attestation/evidence.proto
# proto-message: oak.attestaton.v1.Evidence
#
# Valid real-world evidence for a Restricted Kernel chain, used for testing.
# Generated on 2024-03-12. `oc_evidence.binarypb` is the same instance in
# serialized binary format. ECA: Embedded Certification Authority
#
# The stage0 binary is measured in the attestation report.
root_layer {
platform: AMD_SEV_SNP
remote_attestation_report: "\002\000\000\000\000\000\000\000\000\000\003\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\001\000\000\000\003\000\000\000\000\000\024\321\001\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000w2\252D\223\000Db\2172\355\260\235\024\250\335\376\036\200\002\346\036x\347a\230\255\005}\005x#\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\366\337 T\243\207\363\370)\221A\226\010mY\222dk|\275\203Bp\304\333 \\\323hy\227~\340`\026\301\346\\\236\304S\3434\2415>\223:\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\3111\312\374\2427ynn\373\n\264W\226\014=\323P\304\216\230\007\256\306g%\316^my>\343\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\003\000\000\000\000\000\024\321\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\013\272\370\027|i\366~39}\203*\3533\270-d\256\351S\246#\001\351\017r\000r\232\313\351\222G\'wbZ*B\'\2553\322\336\303\366\024\215aAa\033\264\237d\315m\317\227Z>\267\300\003\000\000\000\000\000\024\321\0207\001\000\0207\001\000\003\000\000\000\000\000\024\321\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\310\274\324\320<\030\t+\017\2040\361\344\020\376\376\374\323\\\347\231\241\270\226\354U}\255\0062E\333\255\202eC\370\035{\02321\321<\314\266\020Z\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\232\355A\263Lm\2007m\237\0265Wu\322\277\371\305\343\204\302\007R\001eLq\305\344Ue\336\013FT\317<\263\373!\203\313\"}\311\301\252\235\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000"
eca_public_key: "\247\001\002\002T\256jOA}\026Vj\025\024\341\306\323\036K\3671\201\032\336\003&\004\201\002 \001!X \361v\376\347?\377\233\211\240\326Za\0369\335\202\r\005d\267\016\331\365\221\\\007w\0261$\264^\"X \314\345!\266e[\365\377R\034\036S\245 ^\353\322{\377]\212\367X\367\034;V\203y>\344\317"
}
layers {
eca_certificate: "\204C\241\001&\241\004RAsymmetricECDSA256Y\001\340\245\001x(ae6a4f417d16566a1514e1c6d31e4bf731811ade\002x(25c27a19fda31d66242035a30892ead3746e6920:\000GDWXf\247\001\002\002T%\302z\031\375\243\035f$ 5\243\010\222\352\323tni \003&\004\201\002 \001!X N\330n\007\260Wi\347Q\337\300.\336\264\234\216hL\213z\235\342E\0358\000\312\0378k)\'\"X \344\302\006i\255\257:|L\370\2555\326\307d_\261VSO6\315}z\336\312hc\\bT\210:\000GDXB \000:\000GDZ\246:\000GD`\241:\000GDkX \314\216\243\312j\305\340\247s\342[\037\017}\365j\356\340wB\033R\206\375\345BOc\005\007\373N:\000GDa\241:\000GDkX +\230Xm\231\005\246\005\302\225\327|a\350\317\322\002z\345\270\240N\357\251\001\2046\366\255\021B\227:\000GDb\241:\000GDkX L\320 \202\r\246c\006?A\205\312\024\247\350\003\315|\234\241H<d\3506\333\204\006\004\266\372\301:\000GDc\241:\000GDkX \000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000:\000GDd\241:\000GDkX \001u>\274BEU\250\r5\255\252!\365\310\305\360\tH.\213\\\305\332\360I\274\033\277\376\274{:\000GDe\241:\000GDkX d\365U2r\207\242\024\024vh\036NM\330\r_u\253\234\'om\270\357\374U#m\272\231SX@d\016\\\353\314\230\267A\231\017\232\267\241A\037\322e\333\325\3222\210\322\244\360\322\212L\220\212\030\0073\265\211\032\356=\354\215\226\201e\247\334\230\334C}\014l\317H\336]b2\277YJ\004\217\036\357"
}
application_keys {
encryption_public_key_certificate: "\204C\241\001&\241\004RAsymmetricECDSA256X\351\245\001x(25c27a19fda31d66242035a30892ead3746e6920\002x(ba848bf3163adcebaa6a86e85593e64112bec8f7:\000GDWXD\246\001\001\002T\272\204\213\363\026:\334\353\252j\206\350U\223\346A\022\276\310\367\0038\036\004\201\005 \004!X \361u\235\355Wh\224\375\250\342\213~H\241>\240s\346Ns\250\351\247\346\256Z=\300U\323)\016:\000GDXB \000:\000GD[\242:\000GDf\241:\000GDkX \225\377$\246\2763\321\225\302\237O\342\256\342r\025\'\371X\244$T\247\3042$^\305\354\214T\235:\000GDi\241:\000GDk@X@d\365\367\267\333b\327yE\352\003\276\263>\r\031b\346\344`\242^\317\204B\313\256\371\303\263\020\034R\327`\356\007\311\200\324\375<\316\366\036\265 E\274\010$\'\256M-\277\016\362\013=Kf\023\342"
signing_public_key_certificate: "\204C\241\001&\241\004RAsymmetricECDSA256Y\001\013\245\001x(25c27a19fda31d66242035a30892ead3746e6920\002x(38d213454f2a0f4668c7816c35ac857c7f85a0e6:\000GDWXf\247\001\002\002T8\322\023EO*\017Fh\307\201l5\254\205|\177\205\240\346\003&\004\201\002 \001!X \215\315\337\177~\331}\254.\307`@#\020%G9=\361t\375v\202<\266\236\351\210QW\357\311\"X 8z\255\376\263Vd\320#>\027\177\237\335\377\315<\237\345\227\273\242\000\002&I\312G\2174n\004:\000GDXB \000:\000GD[\242:\000GDf\241:\000GDkX \225\377$\246\2763\321\225\302\237O\342\256\342r\025\'\371X\244$T\247\3042$^\305\354\214T\235:\000GDi\241:\000GDk@X@\215\323\254\332\321Z\177A\344\315Rr(\212\253E\0050-\367=\231\036\237\357\007*b\254\340\317O)\213\370]P\344\3261\356\311\333\007\264\201B\360!\276:\275>\027\007\254\027$\316\314\263\032\247$"
}
49 changes: 49 additions & 0 deletions oak_attestation_verification/tests/verifier_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const ENDORSER_PUBLIC_KEY_PATH: &str = "testdata/oak-development.pem";
const REKOR_PUBLIC_KEY_PATH: &str = "testdata/rekor_public_key.pem";
const CONTAINERS_EVIDENCE_PATH: &str = "testdata/oc_evidence.binarypb";
const RK_EVIDENCE_PATH: &str = "testdata/rk_evidence.binarypb";
const RK_OBSOLETE_EVIDENCE_PATH: &str = "testdata/rk_obsolete_evidence.binarypb";
const FAKE_EVIDENCE_PATH: &str = "testdata/fake_evidence.binarypb";

// Pretend the tests run at this time: 1 Nov 2023, 9:00 UTC
Expand All @@ -65,6 +66,13 @@ fn create_rk_evidence() -> Evidence {
Evidence::decode(serialized.as_slice()).expect("could not decode evidence")
}

// Creates a valid AMD SEV-SNP evidence instance for a restricted kernel
// application but with obsolete DICE data that is still used by some clients.
fn create_rk_obsolete_evidence() -> Evidence {
let serialized = fs::read(RK_OBSOLETE_EVIDENCE_PATH).expect("could not read evidence");
Evidence::decode(serialized.as_slice()).expect("could not decode evidence")
}

// Creates a valid fake evidence instance.
fn create_fake_evidence() -> Evidence {
let serialized = fs::read(FAKE_EVIDENCE_PATH).expect("could not read fake evidence");
Expand Down Expand Up @@ -536,3 +544,44 @@ fn verify_succeeds_with_matching_command_line_reference_value_set() {
assert!(r.is_ok());
assert!(p.status() == Status::Success);
}

#[test]
fn verify_fails_with_command_line_reference_value_set_and_obsolete_evidence() {
let evidence = create_rk_obsolete_evidence();
let endorsements = create_rk_endorsements();
let reference_values = create_rk_reference_values();

let r = verify(NOW_UTC_MILLIS, &evidence, &endorsements, &reference_values);
let p = to_attestation_results(&r);

eprintln!("======================================");
eprintln!("code={} reason={}", p.status as i32, p.reason);
eprintln!("======================================");
assert!(r.is_err());
assert!(p.status() == Status::GenericFailure);
}

#[test]
fn verify_succeeds_with_skip_command_line_reference_value_set_and_obsolete_evidence() {
let evidence = create_rk_obsolete_evidence();
let endorsements = create_rk_endorsements();
let mut reference_values = create_rk_reference_values();
match reference_values.r#type.as_mut() {
Some(reference_values::Type::OakRestrictedKernel(rfs)) => {
rfs.kernel_layer.as_mut().unwrap().kernel_cmd_line_regex = Some(RegexReferenceValue {
r#type: Some(regex_reference_value::Type::Skip(SkipVerification {})),
});
}
Some(_) => {}
None => {}
};

let r = verify(NOW_UTC_MILLIS, &evidence, &endorsements, &reference_values);
let p = to_attestation_results(&r);

eprintln!("======================================");
eprintln!("code={} reason={}", p.status as i32, p.reason);
eprintln!("======================================");
assert!(r.is_ok());
assert!(p.status() == Status::Success);
}
2 changes: 1 addition & 1 deletion proto/attestation/verification.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ message KernelLayerData {
RawDigest kernel_cmd_line = 2 [deprecated = true];

// Command-line that was passed to the kernel during startup.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a comment here stating what the significance is between an absent field and a present-but-empty field.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 in fact I'm not sure I am reading the logic correctly, could you clarify the intention?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment, more context for this change in b/325979696

string kernel_raw_cmd_line = 7;
optional string kernel_raw_cmd_line = 7;

// Measured digests of the initial RAM disk.
RawDigest init_ram_fs = 4;
Expand Down
Loading