Include ID block when launching guests - #247
Open
markg-github wants to merge 4 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Adds SEV-SNP ID block generation and launch integration so guests can be launched with a signed ID block containing measurement, policy, and metadata.
Changes:
- Adds a host-side
generate-id-blockmodule and service to patch/signid-block.b64. - Updates guest launch to pass
policy,id-block, andid-authto QEMU. - Adds a browser-based ID block template builder and related documentation.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
tools/id-block-README.md |
Documents the ID block builder and default policy/template. |
tools/id-block-builder.html |
Adds browser UI for generating unsigned ID block templates. |
modules/report/host/display-guest-logs/.../display-guest-logs.sh |
Adds timeout messaging for guest test log display. |
modules/launch/host/mkosi.conf |
Includes the new generate-id-block module. |
modules/launch/host/launch-guest/.../launch-guest.service |
Orders guest launch after ID block generation. |
modules/launch/host/launch-guest/.../launch-guest.sh |
Builds QEMU command dynamically and adds ID block arguments. |
modules/launch/host/launch-guest/.../id-block.b64 |
Adds default unsigned ID block template. |
modules/launch/host/launch-done/.../launch-done.service |
Adds ID block generation to launch completion dependencies. |
modules/launch/host/generate-id-block/README.md |
Documents runtime ID block patching/signing. |
modules/launch/host/generate-id-block/.../generate-id-block.service |
Adds oneshot service for ID block generation. |
modules/launch/host/generate-id-block/.../generate_id_block.py |
Implements measurement patching, ECDSA signing, and ID auth generation. |
modules/launch/host/generate-id-block/mkosi.conf |
Adds cryptography dependency for host image. |
modules/build/guest/mkosi.conf |
Adds Python package to guest build configuration. |
.github/workflows/build-and-release.yml |
Adds release job dependency to build job. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+306
to
+310
| function dl(content, name, type) { | ||
| const url = URL.createObjectURL(new Blob([content], {type})); | ||
| Object.assign(document.createElement('a'), {href:url, download:name}).click(); | ||
| URL.revokeObjectURL(url); | ||
| } |
Comment on lines
+50
to
+54
| # Extract policy from id-block (bytes 88-95, LE u64) so LAUNCH_START and | ||
| # LAUNCH_FINISH see the same value; without this QEMU uses its own default. | ||
| POLICY=$(base64 -d "${ID_BLOCK_FILE}" | python3 -c \ | ||
| "import sys; d=sys.stdin.buffer.read(); print(hex(int.from_bytes(d[88:96],'little')))") | ||
| SEV_SNP_OBJECT="${SEV_SNP_OBJECT},policy=${POLICY},id-block=${ID_BLOCK_B64},id-auth=${ID_AUTH_B64}" |
|
|
||
| The most permissive valid policy on the test platform is bits 16+17 set, all | ||
| else clear. The tool defaults to this value (`00000000000b` in the 48-bit hex | ||
| input, yielding full policy `0x000000000000b0000`). |
Comment on lines
+1
to
+3
| [Unit] | ||
| Description=Patch guest measurement into id-block.b64 | ||
| DefaultDependencies=no |
Comment on lines
+65
to
+66
| needs: create-release | ||
| if: ${{ always() && (github.event_name == 'pull_request' || needs.create-release.result == 'success') }} |
Comment on lines
+14
to
+22
| 1. Reads the guest measurement from `guest_measurement.txt` (48-byte SHA-384, | ||
| written as `0x<96 hex chars>`) | ||
| 2. Decodes the ID block template from `id-block.b64` (must be exactly 96 bytes) | ||
| 3. Patches bytes 0–47 (the `ld` field) with the actual guest measurement | ||
| 4. Generates an ephemeral P-384 key pair | ||
| 5. Signs the patched ID block with ECDSA-P384-SHA384 | ||
| 6. Writes the signed ID block back to `id-block.b64` | ||
| 7. Writes a 4096-byte `ID_AUTH_INFO` structure to `id-auth.b64` containing the | ||
| signature and the ephemeral public key (no author key) |
Contributor
There was a problem hiding this comment.
This is a good approach. I don't mind keeping this approach but snpguest actually provides functionality to generate the id-block and the id-auth. Also allowing to pass the measurement, familiy id, svn and policy you want to load into the id-block.
Comment on lines
+3
to
+4
| Browser-based tool for building unsigned ID block templates for SEV-SNP guest | ||
| launch. Opens locally — no server needed. |
Contributor
There was a problem hiding this comment.
That's a really cool tool! Again I'm sorry I didn't let you know that snpguest could actually calculate it, and that will probably work best for testing, but I wonder if there's somewhere we can place this work.
Comment on lines
+52
to
+60
| POLICY=$(base64 -d "${ID_BLOCK_FILE}" | python3 -c " | ||
| import sys | ||
| d = sys.stdin.buffer.read() | ||
| n = len(d) | ||
| if n != 96: | ||
| print(f'ERROR: id-block decoded to {n} bytes (expected 96)', file=sys.stderr) | ||
| sys.exit(1) | ||
| print(hex(int.from_bytes(d[88:96], 'little'))) | ||
| ") |
Comment on lines
+69
to
+73
| # Read metadata from environment, falling back to defaults | ||
| family_id = os.environ.get("ID_BLOCK_FAMILY_ID", DEFAULT_FAMILY_ID) | ||
| image_id = os.environ.get("ID_BLOCK_IMAGE_ID", DEFAULT_IMAGE_ID) | ||
| guest_svn = os.environ.get("ID_BLOCK_GUEST_SVN", DEFAULT_GUEST_SVN) | ||
| policy = os.environ.get("ID_BLOCK_POLICY", DEFAULT_POLICY) |
Comment on lines
+98
to
+99
| result = subprocess.run(cmd, capture_output=True, text=True) | ||
| # Temp files are removed when the TemporaryDirectory context exits |
Comment on lines
+13
to
+14
| Environment=ID_BLOCK_FAMILY_ID=sev-certify-fam0 | ||
| Environment=ID_BLOCK_IMAGE_ID=sev-certify-img0 |
Comment on lines
+32
to
+36
| # Metadata defaults (match the committed template values documented in DESIGN.md) | ||
| DEFAULT_FAMILY_ID = "sev-certify-fam0" | ||
| DEFAULT_IMAGE_ID = "sev-certify-img0" | ||
| DEFAULT_GUEST_SVN = "48" | ||
| DEFAULT_POLICY = "0xb0000" |
| Override any of these with a systemd drop-in (`systemctl edit generate-id-block`) | ||
| without modifying the script or service file. | ||
|
|
||
| For the rationale behind the default values see `DESIGN.md` in this workspace. |
| launch-done.service | ||
| ``` | ||
|
|
||
| If `calculate-measurement.service` is skipped (no AMDSEV OVMF), `guest_measurement.txt` is absent. `generate-id-block.service` detects this, exits 0, and `launch-guest.sh` launches without an ID block. |
|
|
||
| - **Additive principle**: ID block support must not cause failures that would not have occurred without it. This is why absent `guest_measurement.txt` causes exit 0 (not 1). | ||
| - **Ephemeral keys**: Deliberate — for benchmark/test use, not production attestation. The signature satisfies firmware's structural requirement, not external verifiability. | ||
| - See `../DESIGN.md` (workspace-level) for full rationale on policy bit choices and why the DEBUG bit (19) is set. |
Comment on lines
+46
to
+47
| # If the measurement file doesn't exist, calculate-measurement.service | ||
| # was skipped (e.g. AMDSEV OVMF not present). Nothing to do. |
Comment on lines
+41
to
+49
| guest_measurement_sha256sum=$(echo "${calculated_measurement_hex}" | sha256sum | cut -d ' ' -f 1 | xxd -r -p | base64) | ||
| dbg "Measurement (hex): ${calculated_measurement_hex}" | ||
| dbg "Measurement (sha256): ${guest_measurement_sha256sum}" | ||
|
|
||
| # Convert Measurement to the appropriate sha format to pass in as host data | ||
| calculated_measurement_hex=$(awk -F "0x" '{print $2}' "${MEASUREMENT_FILE}" ) | ||
| guest_measurement_sha256sum=$(echo "${calculated_measurement_hex}" | sha256sum | cut -d ' ' -f 1 | xxd -r -p | base64 ) | ||
| # Build sev-snp-guest object; append ID block args if files are present | ||
| SEV_SNP_OBJECT="sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,kernel-hashes=on,host-data=${guest_measurement_sha256sum}" | ||
| if [ -f "${ID_BLOCK_FILE}" ] && [ -f "${ID_AUTH_FILE}" ]; then | ||
| ID_BLOCK_B64=$(cat "${ID_BLOCK_FILE}") | ||
| ID_AUTH_B64=$(cat "${ID_AUTH_FILE}") |
Comment on lines
+7
to
+13
| # Print prompt to tty1 | ||
| echo "Tests complete. System will reboot in $TIMEOUT seconds." > /dev/$TTY | ||
| echo "Press any key to cancel reboot and drop into root shell..." > /dev/$TTY | ||
| # echo "Tests complete. System will reboot in $TIMEOUT seconds." > /dev/$TTY | ||
| # echo "Press any key to cancel reboot and drop into root shell..." > /dev/$TTY | ||
|
|
||
| # Wait for keypress on tty1 | ||
| if read -t "$TIMEOUT" -n 1 < /dev/$TTY; then | ||
| echo "Key pressed — reboot cancelled." > /dev/$TTY | ||
| # if read -t "$TIMEOUT" -n 1 < /dev/$TTY; then | ||
| # echo "Key pressed — reboot cancelled." > /dev/$TTY |
Comment on lines
+47
to
+49
| if [ -f "${ID_BLOCK_FILE}" ] && [ -f "${ID_AUTH_FILE}" ]; then | ||
| ID_BLOCK_B64=$(cat "${ID_BLOCK_FILE}") | ||
| ID_AUTH_B64=$(cat "${ID_AUTH_FILE}") |
Comment on lines
+10
to
+12
| # ID block metadata — override any of these to change what is embedded in the | ||
| # signed block without modifying the script. Defaults match the benchmark | ||
| # platform configuration documented in DESIGN.md. |
Comment on lines
+30
to
+33
| | `ID_BLOCK_FAMILY_ID` | `0000000000000000000000000000fad0` | 16-byte family ID (32 hex chars) | | ||
| | `ID_BLOCK_IMAGE_ID` | `0000000000000000000000000000aed0` | 16-byte image ID (32 hex chars) | | ||
| | `ID_BLOCK_GUEST_SVN` | `48` | Guest security version number | | ||
| | `ID_BLOCK_POLICY` | `0xb0000` | Guest policy flags (bits 16+17+19) | |
| Override any of these with a systemd drop-in (`systemctl edit generate-id-block`) | ||
| without modifying the script or service file. | ||
|
|
||
| For the rationale behind the default values see `DESIGN.md` in this workspace. |
Comment on lines
+8
to
+13
| Metadata (family_id, image_id, guest_svn, policy) is read from environment | ||
| variables with defaults chosen for the benchmark test platform: | ||
| ID_BLOCK_FAMILY_ID — 32 hex chars (default: 0000000000000000000000000000fad0) | ||
| ID_BLOCK_IMAGE_ID — 32 hex chars (default: 0000000000000000000000000000aed0) | ||
| ID_BLOCK_GUEST_SVN — decimal integer (default: 48) | ||
| ID_BLOCK_POLICY — decimal or 0x-prefixed hex (default: 0xb0000) |
Comment on lines
+53
to
+56
| | `ID_BLOCK_FAMILY_ID` | `0000000000000000000000000000fad0` | 32 hex chars | | ||
| | `ID_BLOCK_IMAGE_ID` | `0000000000000000000000000000aed0` | 32 hex chars | | ||
| | `ID_BLOCK_GUEST_SVN` | `48` | Decimal | | ||
| | `ID_BLOCK_POLICY` | `0xb0000` | Bits 16 (SMT), 17 (MBO), 19 (DEBUG) | |
|
|
||
| - **Additive principle**: ID block support must not cause failures that would not have occurred without it. This is why absent `guest_measurement.txt` causes exit 0 (not 1). | ||
| - **Ephemeral keys**: Deliberate — for benchmark/test use, not production attestation. The signature satisfies firmware's structural requirement, not external verifiability. | ||
| - See `../DESIGN.md` (workspace-level) for full rationale on policy bit choices and why the DEBUG bit (19) is set. |
Comment on lines
+10
to
+11
| ID_BLOCK_FAMILY_ID — 32 hex chars (default: 0000000000000000000000000000fad0) | ||
| ID_BLOCK_IMAGE_ID — 32 hex chars (default: 0000000000000000000000000000aed0) |
Comment on lines
+30
to
+31
| | `ID_BLOCK_FAMILY_ID` | `0000000000000000000000000000fad0` | 16-byte family ID (32 hex chars) | | ||
| | `ID_BLOCK_IMAGE_ID` | `0000000000000000000000000000aed0` | 16-byte image ID (32 hex chars) | |
| Override any of these with a systemd drop-in (`systemctl edit generate-id-block`) | ||
| without modifying the script or service file. | ||
|
|
||
| For the rationale behind the default values see `DESIGN.md` in this workspace. |
Comment on lines
+10
to
+12
| # ID block metadata — override any of these to change what is embedded in the | ||
| # signed block without modifying the script. Defaults match the benchmark | ||
| # platform configuration documented in DESIGN.md. |
Comment on lines
+62
to
+66
| - See `../DESIGN.md` (workspace-level) for full rationale on policy bit choices and why the DEBUG bit (19) is set. | ||
|
|
||
| ## Cross-Repo Context | ||
|
|
||
| This branch (`pr/id-block`) is part of a workspace at `~/code/git/features/id-block/` comparing this implementation against `virtee/snpguest` (in the sibling `virtee/` directory). The workspace `CLAUDE.md` and `DESIGN.md` explain the comparison goals and key differences. |
| @@ -1,5 +1,6 @@ | |||
| [Include] | |||
| Include=./guest-measurement | |||
| Include=./generate-id-block | |||
Comment on lines
+284
to
+286
| if profile.id_block and profile.id_auth: | ||
| parts.append(f"id-block={profile.id_block}") | ||
| parts.append(f"id-auth={profile.id_auth}") |
| id_block_b64 = id_block_file.read_text().strip() | ||
| id_auth_b64 = id_auth_file.read_text().strip() | ||
|
|
||
| ctx.profile = replace(ctx.profile, id_block=id_block_b64, id_auth=id_auth_b64) |
| ) | ||
|
|
||
| from .models import StepContext, StepHandlerResult | ||
| from .vm_profile import VMProfile, VMProfileError |
Comment on lines
+30
to
+33
| | `ID_BLOCK_FAMILY_ID` | `0000000000000000000000000000fad0` | 16-byte family ID (32 hex chars) | | ||
| | `ID_BLOCK_IMAGE_ID` | `0000000000000000000000000000aed0` | 16-byte image ID (32 hex chars) | | ||
| | `ID_BLOCK_GUEST_SVN` | `48` | Guest security version number | | ||
| | `ID_BLOCK_POLICY` | `0xb0000` | Guest policy flags (bits 16+17+19) | |
Comment on lines
+8
to
+13
| Metadata (family_id, image_id, guest_svn, policy) is read from environment | ||
| variables with defaults chosen for the benchmark test platform: | ||
| ID_BLOCK_FAMILY_ID — 32 hex chars (default: 0000000000000000000000000000fad0) | ||
| ID_BLOCK_IMAGE_ID — 32 hex chars (default: 0000000000000000000000000000aed0) | ||
| ID_BLOCK_GUEST_SVN — decimal integer (default: 48) | ||
| ID_BLOCK_POLICY — decimal or 0x-prefixed hex (default: 0xb0000) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (9)
sev_verify/cvm_props.py:35
- sev_verify.cvm_props is now imported by existing tests (e.g. attestation_test), but it unconditionally imports cryptography at module import time. This will break running any tests on systems without cryptography even if ID block generation isn’t used. Make cryptography an optional import at module load and emit a clear error when generate_id_block is invoked without it (and ensure it’s declared as a dependency where the project is packaged).
# may need to change this library
from cryptography.hazmat.primitives.asymmetric import ec
from cryptography.hazmat.primitives.serialization import (
Encoding,
NoEncryption,
PrivateFormat,
)
sev_verify/cert_tests/c3_0/c3_0_0_2/id_block_test.py:323
- These vm_stop steps will always error after an expected vm_launch rejection because the harness never sets
launchon VMLaunchError. That produces noisyvm_stop: no running guesterrors and will be counted as failed steps in the final summary. Remove the vm_stop step (there is no VM to stop if launch was rejected).
Step.for_vm_stop(
name="Stop VM (after bad measurement)",
type="info",
timeout=60,
),
sev_verify/cert_tests/c3_0/c3_0_0_2/id_block_test.py:342
- These vm_stop steps will always error after an expected vm_launch rejection because the harness never sets
launchon VMLaunchError. That produces noisyvm_stop: no running guesterrors and will be counted as failed steps in the final summary. Remove the vm_stop step (there is no VM to stop if launch was rejected).
Step.for_vm_stop(
name="Stop VM (after SMT policy)",
type="info",
timeout=60,
),
sev_verify/cert_tests/c3_0/c3_0_0_2/id_block_test.py:361
- These vm_stop steps will always error after an expected vm_launch rejection because the harness never sets
launchon VMLaunchError. That produces noisyvm_stop: no running guesterrors and will be counted as failed steps in the final summary. Remove the vm_stop step (there is no VM to stop if launch was rejected).
Step.for_vm_stop(
name="Stop VM (after ABI version)",
type="info",
timeout=60,
),
sev_verify/runner.py:185
- When vm_launch raises VMLaunchError but the step is considered a pass (because expected_result matches), the StepResult still records the message in stderr and leaves stdout empty. This makes a passing-but-expected failure look like it emitted an error, and also drops the text that was actually used for stdout_contains matching.
step=step,
result="pass" if passed else "error",
exit_code=1,
stderr=str(exc),
duration_ms=duration_ms,
sev_verify/vm_profile.py:296
- If only one of id_block / id_auth is set, the code silently omits both QEMU properties, which can hide misconfiguration. Since these values are a paired input to QEMU, fail fast when only one is provided (and quote the values consistently with host-data).
if profile.id_block and profile.id_auth:
parts.append(f"id-block={profile.id_block}")
parts.append(f"id-auth={profile.id_auth}")
sev_verify/cvm_props.py:178
- generate_id_block assumes cryptography is importable (ec is available). If cryptography is missing (or made optional), return a StepHandlerResult with a clear actionable message instead of throwing an AttributeError when ec is None.
id_key = ec.generate_private_key(ec.SECP384R1())
auth_key = ec.generate_private_key(ec.SECP384R1())
sev_verify/cvm_props.py:38
- VMProfile is imported but never used in this module (only VMProfileError is referenced). Dropping unused imports avoids confusion and keeps lint/static-checkers clean.
from .vm_profile import VMProfile, VMProfileError
sev_verify/cert_tests/c3_0/c3_0_0_2/id_block_test.py:217
- If ID_BLOCK_POLICY is already configured with SMT=0 (bit 16 cleared), clearing it again won’t create an incompatible policy and the subsequent “expect rejection” launch may succeed. Detect this and fail early with an explanatory message so the test doesn’t produce a false negative/positive depending on environment configuration.
policy = os.environ.get("ID_BLOCK_POLICY", DEFAULT_POLICY)
policy_int = int(policy, 0)
# Clear SMT bit (16) — guest demands no SMT, but host has SMT active
incompatible_policy = hex(policy_int & ~(1 << 16))
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (6)
sev_verify/vm_profile.py:306
- If only one of id_block/id_auth is set, it is silently ignored, which can lead to confusing "no ID block used" behavior. It’s safer to validate that they are provided together (and quote the values for consistency with host-data).
parts.append("author-key-enabled=true")
if profile.id_block and profile.id_auth:
parts.append(f"id-block={profile.id_block}")
parts.append(f"id-auth={profile.id_auth}")
sev_verify/cert_tests/c3_0/c3_0_0_2/id_block_test.py:323
- After an expected-rejection vm_launch, vm_launch may raise and return no launch handle (launch=None). In that case, a following vm_stop step always produces an "error" ("no running guest"), adding spurious failures to the test output. Cleanup is already handled by execute_test()'s finally block when a launch exists.
Step.for_vm_stop(
name="Stop VM (after bad measurement)",
type="info",
timeout=60,
),
sev_verify/cert_tests/c3_0/c3_0_0_2/id_block_test.py:342
- Same issue as above: after an expected-rejection vm_launch, launch may be None, so vm_stop reports an avoidable "no running guest" error.
Step.for_vm_stop(
name="Stop VM (after SMT policy)",
type="info",
timeout=60,
),
sev_verify/cert_tests/c3_0/c3_0_0_2/id_block_test.py:361
- Same issue as above: if the expected-rejection vm_launch raises, this vm_stop step cannot succeed because there is no running guest to stop.
Step.for_vm_stop(
name="Stop VM (after ABI version)",
type="info",
timeout=60,
),
sev_verify/runner.py:190
- When a vm_launch raises VMLaunchError and the step’s expected_result does not match, this should be reported as a normal step failure ("fail"), not an internal execution error ("error"). Using "error" here makes launch-outcome mismatches inconsistent with other step kinds and inflates severity in certification summaries.
StepResult(
step=step,
result="pass" if passed else "error",
exit_code=1,
stderr=str(exc),
duration_ms=duration_ms,
),
sev_verify/cvm_props.py:39
- Unused import: VMProfile is imported but never referenced in this module. This can break linting in environments that enforce unused-import checks.
from .models import StepContext, StepHandlerResult
from .vm_profile import VMProfile, VMProfileError
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (7)
sev_verify/cert_tests/c3_0/c3_0_0_2/id_block_test.py:323
- After a negative vm_launch that is expected to be rejected, run_vm_launch_step may return launch=None (e.g. when QEMU exits immediately and raises VMLaunchError). In that case this vm_stop will always produce "vm_stop: no running guest" errors, which is noisy and can skew the step failure count. Consider dropping this vm_stop when the preceding launch is expected to hard-exit.
Step.for_vm_stop(
name="Stop VM (after bad measurement)",
type="info",
timeout=60,
),
sev_verify/cert_tests/c3_0/c3_0_0_2/id_block_test.py:342
- Same issue as above: if the SMT-incompatible launch is rejected and QEMU exits immediately, launch=None and this vm_stop becomes a guaranteed "no running guest" error. Consider removing this vm_stop for expected hard-exit launches.
Step.for_vm_stop(
name="Stop VM (after SMT policy)",
type="info",
timeout=60,
),
sev_verify/cert_tests/c3_0/c3_0_0_2/id_block_test.py:361
- Same issue as above: if the ABI-major policy launch is rejected and QEMU exits immediately, launch=None and this vm_stop produces a "no running guest" error. Consider removing this vm_stop for expected hard-exit launches.
Step.for_vm_stop(
name="Stop VM (after ABI version)",
type="info",
timeout=60,
),
sev_verify/cert_tests/c3_0/c3_0_0_2/id_block_test.py:69
- family_id/image_id are padded with ljust(16) but not truncated/validated, so values longer than 16 bytes will never match the 16-byte report fields (and non-ASCII values will raise UnicodeEncodeError). Validate ASCII and max length before padding to avoid misleading failures.
expected_family = family_id.encode("ascii").ljust(16, b"\x00")
expected_image = image_id.encode("ascii").ljust(16, b"\x00")
sev_verify/cvm_props.py:29
- The comment "may need to change this library" is ambiguous and doesn’t explain what constraint is expected to change. Replace it with a concrete note about why cryptography is used here (ephemeral P-384 key generation for snpguest).
# may need to change this library
sev_verify/cvm_props.py:38
- VMProfile is imported but never used in this module, which adds unnecessary coupling and can confuse readers. Import only VMProfileError here.
from .vm_profile import VMProfile, VMProfileError
sev_verify/vm_profile.py:306
- _build_sev_snp_guest_object silently drops ID block configuration if only one of id_block/id_auth is set, which can lead to launching without an ID block even though the profile appears configured. It’s safer to fail fast when exactly one is provided (and quoting the values is consistent with host-data).
if profile.id_block and profile.id_auth:
parts.append(f"id-block={profile.id_block}")
parts.append(f"id-auth={profile.id_auth}")
markg-github
force-pushed
the
pr/id-block
branch
from
August 19, 2026 17:22
fb7ec93 to
22d3413
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (7)
sev_verify/cvm_props.py:35
- This introduces a new runtime dependency on the third-party
cryptographypackage, but the project’spyproject.tomlcurrently has no declared dependencies. As-is, installs from source/wheels will fail at import time unless cryptography is present implicitly; please add it as an explicit project dependency (or refactor to avoid it).
from cryptography.hazmat.primitives.asymmetric import ec
from cryptography.hazmat.primitives.serialization import (
Encoding,
NoEncryption,
PrivateFormat,
)
sev_verify/cvm_props.py:138
calculate_measurement()assumessnpguestcreated the measurement file on success; if it doesn’t (or the file is removed between run and read),read_text()will raise and crash the runner instead of returning a structured StepHandlerResult.
measurement_file = ctx.artifact_dir / _MEASUREMENT_FILE
result = subprocess.run(
[
"snpguest", "generate", "measurement",
"--vcpu-type", "EPYC-v4",
sev_verify/cvm_props.py:29
- The comment "may need to change this library" is a vague TODO and doesn’t explain the requirement/constraints for the chosen crypto library. It’s better to either remove it or replace it with a concrete rationale (e.g., "used to generate ephemeral P-384 keys for snpguest id-block generation").
# may need to change this library
sev_verify/cvm_props.py:221
generate_id_block()reads id-block/auth output files without guarding against them being absent (e.g., snpguest exits 0 but doesn’t write files, or files are removed). This will raise and abort the whole test run instead of returning a clean failure.
id_block_b64 = id_block_file.read_text().strip()
id_auth_b64 = id_auth_file.read_text().strip()
ctx.profile = replace(ctx.profile, id_block=id_block_b64, id_auth=id_auth_b64, policy=policy)
sev_verify/cvm_props.py:176
family_id/image_idare 16-byte fields in the SNP report/ID block, but the env var values aren’t validated here. If a user sets a longer or non-ASCII value, failures will be confusing and may occur downstream. Consider validating/normalizing up-front with a clear error.
family_id = os.environ.get("ID_BLOCK_FAMILY_ID", DEFAULT_FAMILY_ID)
image_id = os.environ.get("ID_BLOCK_IMAGE_ID", DEFAULT_IMAGE_ID)
guest_svn = os.environ.get("ID_BLOCK_GUEST_SVN", DEFAULT_GUEST_SVN)
policy = os.environ.get("ID_BLOCK_POLICY", DEFAULT_POLICY)
sev_verify/vm_profile.py:306
_build_sev_snp_guest_object()silently omits the ID block if only one ofid_block/id_authis set. That makes misconfiguration hard to spot (e.g., config provides only one value) and can lead to launching without an ID block unintentionally.
if profile.id_block and profile.id_auth:
parts.append(f"id-block={profile.id_block}")
parts.append(f"id-auth={profile.id_auth}")
sev_verify/cert_tests/c3_0/c3_0_0_2/id_block_test.py:69
verify_id_block_fields()buildsexpected_family/expected_imagewith.ljust(16, ...)but does not enforce a max length. If an env var value exceeds 16 bytes,expected_*becomes >16 bytes and the comparison against the fixed-size report field will always fail with a misleading diff.
family_id = os.environ.get("ID_BLOCK_FAMILY_ID", DEFAULT_FAMILY_ID)
image_id = os.environ.get("ID_BLOCK_IMAGE_ID", DEFAULT_IMAGE_ID)
guest_svn = int(os.environ.get("ID_BLOCK_GUEST_SVN", DEFAULT_GUEST_SVN))
policy_int = int(os.environ.get("ID_BLOCK_POLICY", DEFAULT_POLICY), 0)
expected_family = family_id.encode("ascii").ljust(16, b"\x00")
expected_image = image_id.encode("ascii").ljust(16, b"\x00")
markg-github
marked this pull request as ready for review
August 19, 2026 17:49
amd-aliem
requested changes
Aug 24, 2026
amd-aliem
left a comment
amd-aliem
left a comment
Contributor
There was a problem hiding this comment.
A few comments - also could you please show a working e2e output (link an issue in your fork where you ran this new code).
| ), | ||
|
|
||
| # ── Negative: incompatible policy (SMT=0 on SMT-active host) ── | ||
| Step.for_callable( |
Contributor
There was a problem hiding this comment.
If I'm reading it right it looks like set_incompatible_policy errors if the host has SMT disabled. I would prefer not to add 'SMT must be enabled' as a prereq to this test suite.
Contributor
Author
There was a problem hiding this comment.
Yeah, I'll look again at the other *allowed bits in the guest policy. It would be nice if, for a given, less permissive guest policy, some QEMU command lines worked (launched) and some didn't vs. being at the mercy of a platform setting like SMT. If none of the bits work this way, do you think it would be better to skip this sub-test when SMT is disabled or not have the sub-test at all? I agree that a change to the current code is required either way.
| import subprocess | ||
| from pathlib import Path | ||
|
|
||
| from sev_verify.cvm_props import MeasurementError, read_measurement |
Contributor
There was a problem hiding this comment.
import calculate_measurement too? you're using the local one still, below, in step 1 of this test. I think remove the calculate_measurement in this file.
| Step.for_vm_launch( | ||
| name="Launch with bad measurement (expect rejection)", | ||
| type="required", | ||
| expected_result="exit_code:1", |
Contributor
There was a problem hiding this comment.
it might be more accurate to look for the reason for failure instead of accepting any VM launch error (could be something other than ID block). But I don't know how difficult that would be to implement, so I'll leave it up to you. If we didn't change VM profile settings from the previous tests it's unlikely that something else would case a failure.
Contributor
Author
There was a problem hiding this comment.
ACK
My small PR (#289) that merged last week facilitates the improvement alluded to in the comment.
| snpguest generate id-block, and updates ctx.profile with the resulting | ||
| id_block and id_auth values so that vm_launch passes them to QEMU. | ||
|
|
||
| ID block metadata is read from environment variables with the same defaults |
Contributor
There was a problem hiding this comment.
What is this generate-id-block service? Is this comment still relevant?
Contributor
Author
There was a problem hiding this comment.
ACK - stale comment
| def from_bytes(cls, raw: bytes) -> TcbVersion: | ||
| # byte 0 BOOT_LOADER, byte 1 TEE, bytes 2-5 reserved, | ||
| # byte 6 SNP, byte 7 MICROCODE. | ||
| return cls(bootloader=raw[0], tee=raw[1], snp=raw[6], microcode=raw[7]) |
| bootloader: int | ||
| tee: int | ||
| snp: int | ||
| microcode: int |
Contributor
Author
There was a problem hiding this comment.
I did decide, as Claude would say, to fail loudly on CPUs or report versions that I didn't test. What do you think?
| @@ -0,0 +1,181 @@ | |||
| """Parse the SEV-SNP ATTESTATION_REPORT binary structure. | |||
Contributor
There was a problem hiding this comment.
I don't like the idea of re-implementing report parsing and re-defining all of the report fields. I think if we need better interfaces to the report, we should update snpguest. We can add a --json maybe. We're missing out on exercising our own sev crate and user-facing tool, and we're increasing our own work whenever a new report version comes out (have to update it here as well).
We can perhaps continue with this approach in the short term if there's no way around it, but I really do not want this to be a long-term approach.
Contributor
Author
There was a problem hiding this comment.
I'll open an issue (snpguest) or two (and sev) in virtee and not change this code yet.
Contributor
Author
There was a problem hiding this comment.
@amd-aliem , you opened #304 and I added a comment related to key/field names to it. Should I still open an issue or two in virtee?
| description = "Verify ID block acceptance, report field binding, and launch rejection" | ||
| module = "cert_tests.c3_0.c3_0_0_2.id_block_test" | ||
| scope = "mixed" | ||
| level = "3.0.0-2" |
Contributor
There was a problem hiding this comment.
3.0.0-1, I think we're still operating under the 'group tests under one dash release' methodology
Contributor
Author
|
Thank you very much for the comments, @amd-aliem. Please forgive any short replies like ACK, they just mean understood and update on the way. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
sev_verify/vm_profile.py:306
- If only one of id_block/id_auth is set, the launch string silently omits both fields, which can hide a misconfiguration (e.g. a test expecting an ID block will actually launch without one). Consider failing fast when exactly one is provided (and when either is empty), so the error is explicit.
if profile.id_block and profile.id_auth:
parts.append(f"id-block={profile.id_block}")
parts.append(f"id-auth={profile.id_auth}")
Comment on lines
+29
to
+30
| # may need to change this library | ||
| from cryptography.hazmat.primitives.asymmetric import ec |
Comment on lines
+72
to
+77
| guest_svn = int(os.environ.get("ID_BLOCK_GUEST_SVN", DEFAULT_GUEST_SVN)) | ||
| policy_int = int(os.environ.get("ID_BLOCK_POLICY", DEFAULT_POLICY), 0) | ||
|
|
||
| expected_family = family_id.encode("ascii").ljust(16, b"\x00") | ||
| expected_image = image_id.encode("ascii").ljust(16, b"\x00") | ||
|
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.
Suppressed comments (3)
sev_verify/vm_profile.py:306
- If only one of id_block/id_auth is provided, this currently silently omits both from the QEMU sev-snp-guest object, which can hide misconfiguration (e.g., config sets id_block but forgets id_auth). Consider failing fast when exactly one is set so users get a clear error instead of launching without the intended ID block.
if profile.id_block and profile.id_auth:
parts.append(f"id-block={profile.id_block}")
parts.append(f"id-auth={profile.id_auth}")
sev_verify/cert_tests/c3_0/c3_0_0_2/id_block_test.py:76
- family_id/image_id are taken from environment variables and then ASCII-encoded without validation. If either contains non-ASCII characters, this will raise UnicodeEncodeError and crash the step instead of returning a structured StepHandlerResult. Also, the report fields are fixed 16 bytes; if the env var is longer than 16, ljust() won’t truncate and comparisons become confusing. Validate ASCII and max length before encoding/padding.
expected_family = family_id.encode("ascii").ljust(16, b"\x00")
expected_image = image_id.encode("ascii").ljust(16, b"\x00")
sev_verify/cvm_props.py:30
- This inline comment is vague (“may need to change this library”) and doesn’t explain an actionable follow-up. It’s better to either remove it or replace it with a specific TODO (what/why/when).
# may need to change this library
from cryptography.hazmat.primitives.asymmetric import ec
Comment on lines
+22
to
+29
| # calculate_measurement is imported, not redefined: it is shared with the ID | ||
| # block test via cvm_props, and the handler for the step below resolves by name | ||
| # on this module, so the import is what makes it available. | ||
| from sev_verify.cvm_props import ( | ||
| MeasurementError, | ||
| calculate_measurement, | ||
| read_measurement, | ||
| ) |
| ) | ||
|
|
||
| from .models import StepContext, StepHandlerResult | ||
| from .vm_profile import VMProfile, VMProfileError |
Contributor
Author
Here's one: markg-github#209 |
Contributor
Author
Done |
There was a problem hiding this comment.
🟡 Changes recommended
A few confirmed issues (input validation for ID block fields, robustness around measurement file reads, and small doc/error-message inaccuracies) should be addressed to avoid silent misconfiguration and harness crashes.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (3) — in code that hasn't changed since the last review.
sev_verify/cvm_props.py:227
- calculate_measurement assumes snpguest created guest_measurement.txt and unconditionally reads it; if snpguest returns 0 but fails to write the file (or the filesystem is read-only), this will raise and crash the harness instead of returning a structured StepHandlerResult.
sev_verify/vm_profile.py:306 - If only one of id_block/id_auth is set, the ID block is silently omitted from the QEMU sev-snp-guest object, which can lead to unexpected launches without an ID block. Consider validating that both fields are set together (and quoting the values, consistent with host-data) so misconfiguration fails fast and the command line remains robust.
sev_verify/attestation_report.py:307 - The ReportUnsupportedCpu message reads “has not been validated against.” (missing an object). Tweaking this wording makes the error clearer and more professional for users hitting new CPU models.
- Files reviewed: 18/19 changed files
- Comments generated: 2
- Review effort level: Lite
Comment on lines
+37
to
+39
| from .models import StepContext, StepHandlerResult | ||
| # from .vm_profile import VMProfile, VMProfileError | ||
| from .vm_profile import VMProfileError |
Comment on lines
+67
to
+70
| Read once and shared between the step that builds an ID block and the step | ||
| that checks the resulting report, so the two cannot disagree about what was | ||
| asked for. Deriving expectations separately from the environment would let | ||
| the check pass against values the ID block was never built with. |
Contributor
Author
|
A few more changes. Unfortunately, one has a relatively high line count: the patch wasn't consistent with how it managed some of the values added to the ID block. Passing cert with all the changes: markg-github#214 |
Adds shared callables (calculate_measurement, generate_id_block) in sev_verify/id_block.py for use by any test that requires an ID block. Extends VMProfile with id_block/id_auth fields so vm_launch passes them to QEMU when present. Fixes the step loop in cli.py so callable steps can update ctx.profile before vm_launch sees it. fix: pass policy from ID block to QEMU sev-snp-guest object docs: clarify why profile is re-read from ctx each step iteration feat: add actual ID block test with report verification and negative launches Check expected_result on VMLaunchError in run_vm_launch_step so that vm_launch steps can declare expected_result="exit_code:1" for launches that should be rejected by firmware. Add id_block_test at cert level 3.0.0-2: - Positive: launch with valid ID block, verify guest_svn, policy, family_id, image_id in the attestation report via snpguest display - Negative: bad measurement (digest mismatch) - Negative: SMT=0 policy on SMT-active host (platform incompatibility) - Negative: ABI_MAJOR=255 (impossible firmware version) Add vm_stop (type=info) after each negative launch step so that launch is reset to None regardless of whether VMLaunchError fired or the launch unexpectedly succeeded. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> fix: validate guest measurement at each point of use guest_measurement.txt was read without any length or format check. A present-but-malformed file reached snpguest as an opaque argument, surfacing as a confusing subprocess error, and verify_report_fields read it with no guard at all — an absent file raised FileNotFoundError. Add read_measurement() with MeasurementMissing / MeasurementMalformed, validating the 96-character hex digest (48-byte MEASUREMENT field) at each consumer rather than at write time: the producer runs in an earlier step, so a check there says nothing about what a later step reads. Absence stays distinguishable from corruption. generate_id_block still exits 0 and skips when the file is missing (additive principle), but now fails when it is present and malformed. feat: verify ID block fields by parsing report.bin directly verify_id_block_fields shelled out to `snpguest display report` and recovered four fields with regexes over its human-readable output. That couples a hardware assertion to a CLI's formatting: the labels come from the sev crate's Display impl, not from snpguest itself, so a crate bump can silently turn "field mismatch" into "field not found" and fail the test for a reason that has nothing to do with the platform. Parse the binary structure instead. ATTESTATION_REPORT has a fixed layout, and unlike the CLI text it is self-describing — VERSION is the first four bytes, so every report states its own layout and can be checked on the spot rather than inferred from a tool probe. Layout is version-dependent, tracking firmware and so CPU generation, but versions have only ever appended fields: everything below 0x188 is common to v2 and v3, and the CPUID triple at 0x188 is v3+. All four fields this test needs sit in the stable head.
Addresses the FMC/Turin review comments on AMDEPYC#247. The gap was worse than "unsupported": TCB_VERSION is laid out differently on Turin, and the two layouts are indistinguishable from the eight bytes alone. byte Milan/Genoa Turin/Venice 0 BOOT_LOADER FMC 1 TEE BOOT_LOADER 2 -- TEE 3 -- SNP 6 SNP -- 7 MICROCODE MICROCODE Decoding a Turin report with the legacy layout yielded BOOT_LOADER=FMC, TEE=BOOT_LOADER, SNP=0 — plausible values, no error. The sev crate handles this by dispatching on processor Generation (from_legacy_bytes/from_turin_bytes); we hardcoded one layout. Resolve a generation from the host's CPUID family/model, which is authoritative for a report produced by a guest on this machine and, unlike the report's own CPUID copy, present regardless of report version. parse() takes it as an argument so it stays a pure function of its input, unit-testable without hardware. When the report is v3+ it carries CPUID too, and the two are cross-checked: a mismatch means the report did not come from this machine. SUPPORTED_GENERATIONS records what has been validated on hardware, not what we believe we could decode — currently Genoa alone. An unrecognised processor raises rather than assuming a transcribed layout is right: a certification harness reporting a pass on silicon it has never run on is the failure this gate exists to prevent. Layouts for generations not yet exercised are recorded alongside it, so adding one is an entry plus a validation note. Keyed on family/model pairs rather than family/model/stepping triples: AMD scopes SEV firmware images by family and model only, and snpguest's get_processor_model partitions the same way. The module docstring now also records that this is a short-term stand-in. snpguest already parses reports correctly and generation-aware via the sev crate but exposes them only as human-readable text; once it gains machine-readable output this module should be replaced by consuming that rather than extended to cover further generations. test: assert why a negative launch was rejected, not just that it failed Addresses the review comment asking whether the negative launches could check the reason for failure rather than accepting any launch error. expected_result="exit_code:1" is satisfied by anything that fails, including a boot timeout — so these steps could not distinguish a real firmware rejection from a hung guest, and would have passed even if the ID block were ignored entirely. The harness fix merged as AMDEPYC#289 makes the firmware's own text reachable from both launch-failure paths (the raise path passes str(exc) through _check_expected_values; the ok=False path now carries QEMU's stderr tail), so it can be asserted directly. Observed on an EPYC 9654 at BIOS 1.10.6: bad measurement SNP_LAUNCH_FINISH ret=-5 fw_error=11 'Bad measurement' SMT policy SNP_LAUNCH_START ret=-22 fw_error=0 '' ABI version SNP_LAUNCH_START ret=-5 fw_error=7 'Policy is not allowed' The SMT case is refused by KVM before the firmware sees it, so it has no firmware string to match; it asserts SNP_LAUNCH_START instead, which still rules out a boot timeout. Worth noting that this test therefore exercises KVM's policy validation rather than the firmware's. refactor: drop the duplicated calculate_measurement feat: improve SMT sub-test fix: satisfy crypto module dependency This fixes sev-certify automated runs. For sev_verify only runs, it may or may not. See the updated sev_verify/readme.md. fix: decode reports whose firmware left the CPUID fields empty The parser refused any report whose CPUID bytes did not resolve to a validated generation, reporting it as an unsupported processor. That is wrong when the processor is fine and the firmware simply did not populate the field: SEV firmware 1.55 build 38 leaves all three bytes zero in version-3 reports, where build 39 fills them in. On such a platform every report was rejected, and the error blamed the CPU. The host's CPUID is authoritative — it describes the silicon this code runs on, which is what the TCB_VERSION layout depends on, and it is present regardless of report version. The report's copy is a cross-check, useful only when it resolves. So an unresolvable one is now recorded in a new cpuid_note field and decoding proceeds with the host's generation. The exception is kept for the case that does indicate a problem: both CPUIDs resolve to validated generations and disagree, meaning the report came from another machine. That branch only becomes reachable once a second generation is added to SUPPORTED_GENERATIONS; with one validated generation every disagreeing CPUID is unresolvable instead. That ordering is deliberate — a report is called foreign only when both generations are ones we have actually validated against. With no host generation supplied and an unresolvable CPUID, TCB_VERSION is now left undecoded rather than raising, matching what a v2 report already does. docs: say how to add a report version, not just a processor SUPPORTED_GENERATIONS explains how to validate and add a processor generation; KNOWN_VERSIONS only stated which versions were readable and refused the rest, leaving the reader to work out what adding one involves. Record the procedure, and that versions 4 and 5 already exist and are refused. Note the cheap case explicitly: the sev crate's ReportVariant mapping groups versions sharing a layout, so a version grouped with one already listed reads identically, which makes v4 straightforward and v5 the one needing scrutiny. Also note that additions are not always new offsets — v5's GuestPolicy and PlatformInfo changes are new bits in existing fields. State that version and generation are independent axes, since the two gates look redundant until it is clear that one decides which fields exist and the other how TCB_VERSION is ordered. feat: validate Turin and report version 5 on hardware Adds the second processor generation and the second report version, both confirmed on an EPYC 9575F rather than transcribed. The Turin TCB_VERSION layout had been taken from the sev crate and never exercised. A version 5 report from that machine carried REPORTED_TCB bytes 0103020600000062, which decode under the Turin layout to bootloader=3 tee=2 snp=6 microcode=98 fmc=1 — matching snphost show tcb exactly. The same bytes under the legacy layout give bootloader=1 tee=3 snp=0 microcode=98 and no FMC: plausible values, silently wrong. That is the failure this gate exists to prevent, now demonstrated rather than asserted. That report also showed version 5 moves none of the fields read here: TCB at 0x180 and CPUID at 0x188 both decoded correctly, so v5's additions are the new GuestPolicy and PlatformInfo bits the crate documents rather than relocations. Version 4 remains refused, never having been seen. Two consequences worth noting. The generation mismatch branch in parse() was unreachable while only one generation was validated; with two, a report from the wrong machine is now genuinely detected. And Turin firmware 1.58 populates the report CPUID fields that Genoa's BIOS-supplied 1.55 build 38 leaves zeroed, so both sides of that behaviour are now covered by tests.
The Layout section listed five of the ten modules. attestation_report.py and cvm_props.py are introduced by this branch and were never added; environment.py, os_info.py and output.py predate it. "How it works" described a manifest entry as "name, scope, module path", but TestDefinition also carries level and host_changes. host_changes was documented only in the flags table, which is not where someone writing a test looks — and the judgement it requires is not obvious, so the distinction is stated: launching a guest does not count, changing platform configuration does. feat: decode newer report versions rather than refusing them An unknown report version was refused outright, the same treatment given an unknown processor. The two mistakes are not comparable. Misidentifying the processor generation means misreading bytes that are present: TCB_VERSION decodes to plausible but wrong values with nothing in the data to reveal it. That is worth refusing over. A newer report version is the opposite — fields have only ever been appended, so everything read here stays where it was and the cost is missing what is new rather than misreading what is old. The version is also self-describing, sitting in the first four bytes and always present, which is exactly what the generation is not. The gate had fired exactly once in practice, on Turin's version 5 reports, and it was wrong to fire: it would have refused a report that decodes correctly, which is how the Turin support in this branch was nearly missed. The sev crate is already permissive here, mapping any unrecognised version onto its newest variant. So a version at or above the validated range is decoded with the newest validated offsets and the assumption recorded in a new version_note field. Versions below the range are still refused, since there fields may genuinely not exist rather than merely going unread. KNOWN_VERSIONS now means validated, not accepted.
The ID_BLOCK_* environment variables were read in six places: once to build
the ID block, once to check the resulting report, and four more times across
the negative cases that rebuild it. Each site parsed them independently, and
the checker derived its expectations from the environment rather than from
what the generator had actually used — so the two could drift and the test
would still report a clean pass or an unexplained mismatch.
Read them once, in cvm_props.read_id_block_metadata(), and share the result.
The conversions are now guarded. int() on a non-numeric value or
encode("ascii") on a non-ASCII one previously raised out of the handler as a
bare ValueError or UnicodeEncodeError; they now fail the step with a message
naming the variable at fault.
FAMILY_ID and IMAGE_ID are length-checked against the 16-byte field. ljust()
pads but never truncates, so an over-long value produced an expectation longer
than the report field, which could never match and reported itself as a byte
diff that did not say why.
Policy is carried as an int from the point it is read, rather than being
re-parsed from a string at each negative case. VMProfile already accepts
str | int and formats it, so nothing downstream changes.
This file contains hidden or 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
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.
This PR is motivated by the need for an SEV derived keys test in sev-certify/verify. SEV ID blocks include fields that affect what are valid inputs to SEV key derivation. If an ID block isn't used when an SEV guest is launched, valid key derivation inputs are limited.
An ID block includes expected measurement, guest policy, Family ID, Image ID, etc. The expected measurement must match the HW/FW-calculated measurement. Note that sev-certify has always calculated an expected measurement even though it's only now, with this PR, that an ID block is included when launching guests. Ignoring the ID block, sev-certify used the expected measurement as "host data". This is still the case.
ID blocks are signed, but they can be self-signed, which is what this PR does.
Here's an example of a passing cert generated with this PR: markg-github#209