docs: overhaul README.md and snpguest.1.adoc by hyperfinitism · Pull Request #139 · virtee/snpguest

hyperfinitism · 2025-12-25T18:38:59Z

The existing documentation (README.md and docs/snpguest.1.adoc) was out of sync with the actual snpguest CLI: README.md omitted numerous subcommands, and descriptions of flags and summary lines were inaccurate (e.g., #132, #135, #138).

This pull request overhauls README.md and docs/snpguest.1.adoc to (partly) resolve these issues.

README.md
- Reorganise the document structure
- Adds full descriptions and examples for all subcommands, especially:
  - snpguest ok
  - snpguest generate measurement
  - snpguest generate ovmf-hash
  - snpguest generate id-block
  - snpguest generate key-digest
- Sync synopsis lines and global options with current code.
docs/snpguest.1.adoc
- Mirror all of the above changes in the AsciiDoc
- Reformat sections and option lists for consistency

DGonzalezVillal

A few nit picks and discussion points, but this is an AMAZING pr. I really really appreciate it.

DGonzalezVillal · 2026-01-06T21:23:34Z

+    Without [-p, --platform] or [-r, --random], the user can pass 64 bytes of data in any file format into $REQUEST_FILE to request an attestation report. The request file is interpreted as raw binary, and the first 64 bytes of the binary are sent to the AMD-SP as the request data. The request data will be bound to the REPORT_DATA field in the attestation report.
+    With the [-r, --random] flag, this command generates a random data for the request, which will be written into $REQUEST_FILE.
+    The [-v, --vmpl] option specifies the Virtual Machine Privilege Level (VMPL) to request an attestation report. The default value is 1. If you specify a higher privilege level (i.e. smaller VMPL value) than the actual one, a firmware error will occur.
+    With the [-p, --platform] flag, this command retrieves an attestation report from vTPM instead of the ASP, and writes the Report Data field in the retrieved report into $REQUEST_FILE. This attestation route is available (and mandatory) on Microsoft Azure Confidential VMs with SEV-SNP isolation. This flag requires the `hyperv` feature. Currently, only the pre-generated attestation report can be retrieved with this command.


I think this attestation route is currently ONLY available on Azure right? I don't think using platform will work on Linux.

I would clarify that this is a pre-generated report stored on the vTPM and that the Report Data field is pre filled. What goes on REQUEST_FILE is what was already present.

I don't think it's wrong just a little clearer on how this flag works.

Basically yes. It is only available on CVMs that implement this design using Microsoft's open source OpenVMM/HCL. AFAIK, no other commercial environment uses this. So I think this flag should be called something else, such as openhcl or azurecvm.

Related Issue: #125
Related PR: #141

DGonzalezVillal · 2026-01-07T21:23:59Z

+- Verify that the REPORTED_TCB and CHIP_ID fields in the report matches the given certificate. The `-s, --signature` option skips this step.
+- Verify the report signature using the given certificate. The `-t, --tcb` option skips this step.


Again subtle difference in functionality. Using -s checks only for signature and -t checks only for t. If they are used jointly, then they both will be checked, not both be skipped.

In fact, if both -t (or -s) and report content flags (e.g. -m) are specified, snpguest verify attestation will verify not only TCB (or signature) but specified report contents:

snpguest/src/verify.rs

Lines 452 to 466 in 5540f31

if args.tcb || args.signature {

if args.tcb {

verify_attestation_tcb(vek.clone(), att_report, proc_model, quiet)?;

}

if args.signature {

verify_attestation_signature(vek, att_report, quiet)?;

}

} else {

verify_attestation_tcb(vek.clone(), att_report, proc_model, quiet)?;

verify_attestation_signature(vek, att_report, quiet)?;

}

// Verify optional fields if provided

if args.measurement.is_some() || args.host_data.is_some() || args.report_data.is_some() {

verify_report_fields(&args, &att_report, quiet)?;

The flags -t and -s conflicts with each other:

snpguest/src/verify.rs

Lines 199 to 205 in 5540f31

/// Run the TCB Verification Exclusively.

#[arg(short, long, conflicts_with = "signature")]

pub tcb: bool,

/// Run the Signature Verification Exclusively.

#[arg(short, long, conflicts_with = "tcb")]

pub signature: bool,

Rather than changing the documentation, it might be better to change the code.

Related Issue: #132

Oh I see, I missed the conflicts_with argument, that is my bad

This commit aligns the documentation with the actual CLI implementation and adds several previously missing subcommands. Co-authored-by: DGonzalezVillal <97119524+DGonzalezVillal@users.noreply.github.com> Signed-off-by: Takuma IMAMURA <209989118+hyperfinitism@users.noreply.github.com>

hyperfinitism · 2026-01-08T16:07:34Z

@DGonzalezVillal Thank you for your detailed review! All comments except for a few have been addressed and the commits have been squashed.

hyperfinitism · 2026-01-08T16:30:57Z

We now maintain similar content in both README.md and docs/snpguest.1.adoc. That duplication may become a maintenance burden. As a follow-up (out of scope for this PR), we could consider consolidating into a single source or auto-generating one from the other (or from comments within the code).

DGonzalezVillal · 2026-01-08T17:48:07Z

We now maintain similar content in both README.md and docs/snpguest.1.adoc. That duplication may become a maintenance burden. As a follow-up (out of scope for this PR), we could consider consolidating into a single source or auto-generating one from the other (or from comments within the code).

This is a good point and something I was thinking about while reviewing your PR.

I think technically speaking the adoc is supposed to be generally used for more detailed, structured, and feature-rich documentation. The purpose of a README file is to provide a concise, high-level introduction. I've always thought that the best way to organize the documentation would be to put the long description of each command in the adoc and then the readme would point to the adoc. In the readme I would keep the instructions on how to build the project and the different attestation workflows users could follow, but with not as much detail.

Let me know your thoughts on that.

@tylerfanelli @hyperfinitism

hyperfinitism · 2026-01-10T17:27:36Z

Thank you for your feedback. I largely agree. Specifically, I think the following approach makes sense:

Generate README.md from src/main.rs using cargo rdme (following the upstream sev crate’s style).
Use cargo doc as the source of truth for complete documentation derived from the code.

The typical usage pattern (e.g. report → fetch ca/vcek or certificates → verify certs/attestation) can remain in README.md, derived from comments in src/main.rs. However, more detailed discussions (security model, threat assumptions, trust boundaries, etc.; see also #125) would likely be better handled as separate documentation.

This keeps the README concise, reduces the risk of docs drifting from the implementation, and lowers maintenance costs.

As a follow-up, we could also enforce doc hygiene in CI (e.g. rustdoc builds and/or treating doc warnings as errors) so contributors are nudged to keep documentation up to date.

DGonzalezVillal · 2026-01-21T22:55:22Z

Hello @hyperfinitism

I agree with this approach. Do you think you can take care of those changes in this PR?

DGonzalezVillal · 2026-03-03T20:16:34Z

Hey @hyperfinitism were you able to take a look at those changes?

hyperfinitism · 2026-03-04T01:51:14Z

Since opening this PR, several changes have been merged to the main branch.

Because of this, the documentation needs to be updated based on the current state of the codebase. Under the new documentation approach (cargo-rdme + rustdoc), all components of the code should also have proper docstrings, many of which are currently missing.

While working on this, I have been rereading the entire codebase to fix existing documentation issues at the same time. So the process is taking some time.

Also, #141 introduced relatively large changes. If that PR gets merged, part of the documentation restructuring work in this PR may become unnecessary, so it might be better to wait for that before continuing.

Add module-level documentation (//!) and doc comments (///) to all source files to support `cargo doc` and prepare for `cargo rdme` README generation. Changes per file: - main.rs: Add top-level crate documentation with usage overview and subcommand table; add #![deny(missing_docs)] to enforce documentation on public items; fix SnpGuestCmd variant descriptions to match actual behavior - report.rs: Document ReportArgs fields accurately; fix --random help (was misleading about default file path); fix --platform help to describe vTPM retrieval on Azure CVMs - certs.rs: Document CertPaths, CertFormat, CertificatesArgs, and all public functions - fetch.rs: Document FetchCmd, Endorsement, ProcType enums; document all submodule Args structs and functions; improve fetch ca/vcek/crl argument descriptions - verify.rs: Fix critical help text errors from issue virtee#132: --tcb now correctly described as "verify TCB only (skip signature)" instead of "run TCB verification exclusively"; --signature now correctly described as "verify signature only (skip TCB)" instead of "run signature verification exclusively"; document decode_hex_or_decimal behavior mismatch (issue virtee#135); document all verification functions - display.rs: Document DisplayCmd and subcommand functions - key.rs: Document KeyArgs with accurate Guest Field Select bit descriptions; document message version auto-selection behavior - ok.rs: Document SEV status MSR bitfield and capability test framework - preattestation.rs: Document all generate subcommands (measurement, ovmf-hash, id-block, key-digest) with accurate argument descriptions - clparser/mod.rs: Document auto-radix parser and FromStrRadix trait - hyperv/mod.rs: Document Hyper-V detection logic and vTPM report retrieval Source of truth: actual code behavior on main branch and the corrected README.md from PR virtee#139 (docs-overhaul branch). https://claude.ai/code/session_01YFz1JMARmZYNFwTQ6juM7Z

auto-assign Bot requested review from DGonzalezVillal and tylerfanelli December 25, 2025 18:39

auto-assign Bot assigned larrydewey and tylerfanelli Dec 25, 2025

hyperfinitism force-pushed the docs-overhaul branch from bf593dc to d9277a2 Compare January 1, 2026 15:57

DGonzalezVillal requested changes Jan 7, 2026

View reviewed changes

hyperfinitism force-pushed the docs-overhaul branch 2 times, most recently from 27e7af8 to 2a56555 Compare January 8, 2026 15:15

hyperfinitism force-pushed the docs-overhaul branch from 459807b to 4c283cc Compare January 8, 2026 16:04

This was referenced Apr 21, 2026

docs: add Rustdoc comments and CI support for documentation generation #151

Draft

docs: define a single-source-of-truth strategy for CLI documentation #152

Open

		- Verify that the REPORTED_TCB and CHIP_ID fields in the report matches the given certificate. The `-s, --signature` option skips this step.
		- Verify the report signature using the given certificate. The `-t, --tcb` option skips this step.

	if args.tcb \|\| args.signature {
	if args.tcb {
	verify_attestation_tcb(vek.clone(), att_report, proc_model, quiet)?;
	}
	if args.signature {
	verify_attestation_signature(vek, att_report, quiet)?;
	}
	} else {
	verify_attestation_tcb(vek.clone(), att_report, proc_model, quiet)?;
	verify_attestation_signature(vek, att_report, quiet)?;
	}

	// Verify optional fields if provided
	if args.measurement.is_some() \|\| args.host_data.is_some() \|\| args.report_data.is_some() {
	verify_report_fields(&args, &att_report, quiet)?;

	/// Run the TCB Verification Exclusively.
	#[arg(short, long, conflicts_with = "signature")]
	pub tcb: bool,

	/// Run the Signature Verification Exclusively.
	#[arg(short, long, conflicts_with = "tcb")]
	pub signature: bool,

Conversation

hyperfinitism commented Dec 25, 2025

Uh oh!

DGonzalezVillal left a comment

Choose a reason for hiding this comment

Uh oh!

DGonzalezVillal Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

hyperfinitism Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DGonzalezVillal Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

hyperfinitism Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DGonzalezVillal Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hyperfinitism commented Jan 8, 2026

Uh oh!

hyperfinitism commented Jan 8, 2026

Uh oh!

DGonzalezVillal commented Jan 8, 2026

Uh oh!

hyperfinitism commented Jan 10, 2026

Uh oh!

DGonzalezVillal commented Jan 21, 2026

Uh oh!

DGonzalezVillal commented Mar 3, 2026

Uh oh!

hyperfinitism commented Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hyperfinitism Jan 8, 2026 •

edited

Loading