-
Notifications
You must be signed in to change notification settings - Fork 97
attest: add attestation testing with kbs-test integration #1002
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,6 +69,45 @@ QEMU=/path/to/qemu make test-in-svsm TEST_ARGS='--nocc -- --no-netdev' | |
| A list of parameters for `launch_guest.sh` is listed in the | ||
| [INSTALL.md](../installation/INSTALL.md) document. | ||
|
|
||
| ## Attestation tests | ||
|
|
||
| Attestation can be tested in the same infrastructure by running the | ||
| attest-enabled test image together with `kbs-test` and `aproxy`. | ||
|
|
||
| ### Requirements | ||
|
|
||
| In addition to the requirements for in-SVSM tests, attestation tests require: | ||
|
|
||
| - The `kbs-test` server from [coconut-svsm/kbs-test](https://github.com/coconut-svsm/kbs-test) | ||
| - The `aproxy` binary (built automatically as part of the SVSM build) | ||
|
|
||
| ### Running | ||
|
|
||
| Clone and build `kbs-test`: | ||
|
|
||
| ```shell | ||
| git clone https://github.com/coconut-svsm/kbs-test.git ../kbs-test | ||
| ``` | ||
|
|
||
| Run attestation tests: | ||
|
|
||
| ```shell | ||
| KBS_TEST_DIR=../kbs-test QEMU=/path/to/qemu \ | ||
| make FEATURES_TEST=vtpm,virtio-drivers,block,attest \ | ||
| TEST_IN_SVSM_SCRIPT=./scripts/test-in-svsm-attest.sh \ | ||
| TEST_IN_SVSM_DEPS=aproxy \ | ||
| test-in-svsm | ||
|
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. Please don't reuse |
||
| ``` | ||
|
|
||
| You can replace `KBS_TEST_DIR` with `KBS_TEST_BIN=/path/to/kbs-test` if you | ||
| already have a `kbs-test` binary built. | ||
|
|
||
| The test will: | ||
| 1. Start a local `kbs-test` server | ||
| 2. Start an `aproxy` instance | ||
| 3. Run the SVSM tests with attestation enabled | ||
| 4. Verify "attestation successful" appears in the output | ||
|
|
||
| ## Miri | ||
|
|
||
| Miri is an Undefined Behavior detection tool for Rust. It can run binaries and | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,8 @@ XBUILD_ARGS_TEST += --feature ${FEATURES_TEST} | |
| endif | ||
|
|
||
| TEST_ARGS ?= | ||
| TEST_IN_SVSM_SCRIPT ?= ./scripts/test-in-svsm.sh | ||
| TEST_IN_SVSM_DEPS ?= | ||
|
|
||
| CARGO ?= cargo | ||
| CLIPPY_OPTIONS ?= --all-features | ||
|
|
@@ -125,8 +127,9 @@ miri: | |
|
|
||
| test-igvm: $(IGVM_TEST_FILES) | ||
|
|
||
| test-in-svsm: bin/coconut-test-qemu.igvm $(IGVMMEASUREBIN) | ||
| ./scripts/test-in-svsm.sh $(TEST_ARGS) | ||
| test-in-svsm: $(IGVMMEASUREBIN) $(TEST_IN_SVSM_DEPS) | ||
| cargo xbuild $(XBUILD_ARGS_TEST) ./configs/test/qemu-test-target.json | ||
|
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. Why removing the dep with |
||
| TEST_IGVM=$(CURDIR)/bin/coconut-test-qemu.igvm $(TEST_IN_SVSM_SCRIPT) $(TEST_ARGS) | ||
|
|
||
| test-in-hyperv: bin/coconut-test-hyperv.igvm | ||
|
|
||
|
|
@@ -195,4 +198,4 @@ clean: | |
|
|
||
| distclean: clean | ||
|
|
||
| .PHONY: test miri clean clippy bin/stage2.bin bin/svsm-kernel.elf bin/test-kernel.elf stage1_elf_trampoline distclean $(APROXYBIN) $(IGVM_FILES) $(IGVM_TEST_FILES) | ||
| .PHONY: test miri clean clippy bin/stage2.bin bin/svsm-kernel.elf bin/test-kernel.elf stage1_elf_full stage1_elf_trampoline stage1_elf_test distclean $(APROXYBIN) $(IGVM_FILES) $(IGVM_TEST_FILES) test-in-svsm | ||
|
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. Why adding |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -355,3 +355,110 @@ fn hash( | |
|
|
||
| try_to_vec(&sha.finalize()).or(Err(AttestationError::VecAlloc)) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
|
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. CC @tylerfanelli on these tests.
Contributor
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. Tests LGTM. |
||
| use super::*; | ||
| use alloc::vec; | ||
| use cocoon_tpm_tpm2_interface::{Tpm2bEccParameter, TpmBuffer}; | ||
|
|
||
| fn make_ecc_point(x: &[u8], y: &[u8]) -> TpmsEccPoint<'static> { | ||
| TpmsEccPoint { | ||
| x: Tpm2bEccParameter { | ||
| buffer: TpmBuffer::Owned(x.to_vec()), | ||
| }, | ||
| y: Tpm2bEccParameter { | ||
| buffer: TpmBuffer::Owned(y.to_vec()), | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| mod negotiation_hash { | ||
| use super::*; | ||
|
|
||
| /// hash() feeds NegotiationParams into SHA-512 in the order they | ||
| /// appear in `response.params`. The ordering matters because | ||
| /// the server dictates which fields contribute to the attestation | ||
| /// hash and in what order. These two tests verify that | ||
| /// [Challenge, EcPublicKeyBytes] and [EcPublicKeyBytes, Challenge] | ||
| /// produce different digests, therefore confirming the function | ||
| /// respects the param ordering from the negotiation response. | ||
| #[test] | ||
| fn challenge_then_ec_key() { | ||
| let challenge = vec![0xdd; 48]; | ||
| let x = vec![0x10; 66]; | ||
| let y = vec![0x20; 66]; | ||
| let response = NegotiationResponse { | ||
| challenge: challenge.clone(), | ||
| params: vec![ | ||
| NegotiationParam::Challenge, | ||
| NegotiationParam::EcPublicKeyBytes, | ||
| ], | ||
| }; | ||
| let pub_key = make_ecc_point(&x, &y); | ||
|
|
||
| let result = hash(&response, &pub_key).unwrap(); | ||
|
|
||
| let mut sha = Sha512::new(); | ||
| sha.update(&challenge); | ||
| sha.update(&x); | ||
| sha.update(&y); | ||
| let expected = sha.finalize(); | ||
| assert_eq!(result, expected.as_slice()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn ec_key_then_challenge() { | ||
| let challenge = vec![0xee; 24]; | ||
| let x = vec![0x30; 10]; | ||
| let y = vec![0x40; 10]; | ||
| let response = NegotiationResponse { | ||
| challenge: challenge.clone(), | ||
| params: vec![ | ||
| NegotiationParam::EcPublicKeyBytes, | ||
| NegotiationParam::Challenge, | ||
| ], | ||
| }; | ||
| let pub_key = make_ecc_point(&x, &y); | ||
|
|
||
| let result = hash(&response, &pub_key).unwrap(); | ||
|
|
||
| let mut sha = Sha512::new(); | ||
| sha.update(&x); | ||
| sha.update(&y); | ||
| sha.update(&challenge); | ||
| let expected = sha.finalize(); | ||
| assert_eq!(result, expected.as_slice()); | ||
| } | ||
|
|
||
| /// Changing the param order must change the hash. This is a | ||
| /// security property: if the server negotiates a different param | ||
| /// list, the resulting attestation evidence must differ. | ||
| #[test] | ||
| fn different_order_produces_different_hash() { | ||
| let challenge = vec![0x42; 32]; | ||
| let x = vec![0x01; 10]; | ||
| let y = vec![0x02; 10]; | ||
| let pub_key = make_ecc_point(&x, &y); | ||
|
|
||
| let response_chal_first = NegotiationResponse { | ||
| challenge: challenge.clone(), | ||
| params: vec![ | ||
| NegotiationParam::Challenge, | ||
| NegotiationParam::EcPublicKeyBytes, | ||
| ], | ||
| }; | ||
| let response_key_first = NegotiationResponse { | ||
| challenge: challenge.clone(), | ||
| params: vec![ | ||
| NegotiationParam::EcPublicKeyBytes, | ||
| NegotiationParam::Challenge, | ||
| ], | ||
| }; | ||
|
|
||
| let hash1 = hash(&response_chal_first, &pub_key).unwrap(); | ||
| let hash2 = hash(&response_key_first, &pub_key).unwrap(); | ||
| assert_ne!(hash1, hash2); | ||
| } | ||
| } | ||
| } | ||
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.
Sorry, I'm still confused, why you need to run kbs-test and aproxy for the tests you added.
Can you tell me which of the tests we're adding require communication with KBS?
Maybe I missed something.
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.
None of the unit tests in commits 2 and 3 require kbs-test or aproxy.
commit 1 (test-in-svsm-attest.sh) needs kbs-test and aproxy as it is an end-to-end integration test
that boots the SVSM test image on real SNP hardware with the attest feature enabled, connects it to a local kbs-test server via aproxy, and checks that pre-boot attestation completes successfully ("attestation successful" in output).
This is the part that addresses the kbs-test integration mentioned in the original issue.
The TESTING.md is documenting the integration script.
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.
So commit 1 and commits 2/3 are completely unrelated, right?
Why putting them in a single PR ?