cli: new flag for Azure JSON output of constellation verify

[elchead]

Context

Motivated by #2357, this PR provides a JSON output format as communication interface between the CLI and the attestationconfigapi upload tool through a new flag constellation verify --output json.

Proposed change(s)

• new internal pkg verify to provide the types for JSON serialization.
• new --output flag to support 'json' for Azure
• BREAKING: moved --raw to --output raw

Related issue

• AB#3462

Additional info

To only obtain the JSON output needed for the upload tool, some lines need to be omitted:
./constellation verify --force --json | tail -n +3 | head -n -1

Checklist

• Update docs
• Add labels (e.g., for changelog category)
• Is PR title adequate for changelog?
• Link to Milestone

[netlify]

✅ Deploy Preview for constellation-docs canceled.

Name	Link
🔨 Latest commit	8ea7593
🔍 Latest deploy log	https://app.netlify.com/sites/constellation-docs/deploys/6520214f66e2f5000853095b

[3u13r]

This is not the implementation I suggested.

implement a custom mashaller and unmarshaller which recursively parses the attestationDoc to a struct containing the parsed InstanceInfo

I said that we should implement a custom mashaller and unmarsaller. As we already do in e.g. https://github.com/edgelesssys/constellation/blob/cli/verify/json-output/internal/attestation/idkeydigest/idkeydigest.go#L180

Did you encounter any issue with this approach, or why did you prefer your implementation?

[daniel-weisse]

To only obtain the JSON output needed for the upload tool, some lines need to be omitted

This seems like bad UX. If a command offers me the option to receive JSON output, I would expected all output to be JSON, not just some of it.
What about omitting the hints about retrieval of information from the id/state file, and potentially adding the verification status to the JSON output?
E.g.

$ constellation verify
{
  "attestationDocument": {
    // ...
  },
  "verification": "ok",
}

[thomasten]

If a command offers me the option to receive JSON output, I would expected all output to be JSON, not just some of it.

A common way tools solve this is printing the JSON output to stdout and everything else to stderr.

[elchead]

This is not the implementation I suggested.

implement a custom mashaller and unmarshaller which recursively parses the attestationDoc to a struct containing the parsed InstanceInfo

I said that we should implement a custom mashaller and unmarsaller. As we already do in e.g. https://github.com/edgelesssys/constellation/blob/cli/verify/json-output/internal/attestation/idkeydigest/idkeydigest.go#L180

Did you encounter any issue with this approach, or why did you prefer your implementation?

I think if we wan

To only obtain the JSON output needed for the upload tool, some lines need to be omitted

This seems like bad UX. If a command offers me the option to receive JSON output, I would expected all output to be JSON, not just some of it. What about omitting the hints about retrieval of information from the id/state file, and potentially adding the verification status to the JSON output? E.g.

$ constellation verify
{
  "attestationDocument": {
    // ...
  },
  "verification": "ok",
}

I print to stderr now. Do we need the verification ok in the json output too? Since we don't foresee consuming this json in any other way besides the upload tool at the moment, I would not include it. An error of verify should indicate that the verification was not ok right?

[elchead]

This is not the implementation I suggested.

implement a custom mashaller and unmarshaller which recursively parses the attestationDoc to a struct containing the parsed InstanceInfo

I said that we should implement a custom mashaller and unmarsaller. As we already do in e.g. https://github.com/edgelesssys/constellation/blob/cli/verify/json-output/internal/attestation/idkeydigest/idkeydigest.go#L180

Did you encounter any issue with this approach, or why did you prefer your implementation?

I think it's better to use a proper type, because to unmarshal and read the struct we would need this anyways (as required by the upload tool)

[derpsteb]

Recap of our conversation on the phone:

• Ideally we could use a type from go-sev-guest that recursively parses all fields in an SNP report. However, looking at how scattered the bits of parsing logic currently are, it seems like considerably more effort to change the type upstream. It is also questionable if the maintainers would even want such a change.
• I personally don't think it is urgent to upstream this. It would be easy to adapt this PR to an upstream implementation, if one would be released/implemented tomorrow.
• Using newSNPReport in the existing formatter would make sense to reduce code duplication.
• A point in the future when we should try upstreaming this imo: when we plan on using the Report type in more places than attestationconfigapi and verify.
• I opened an issue to see how the upstream maintainers feel about such a contribution. The answer can help inform our decision here: Deeply nested SNP Report type google/go-sev-guest#91

Apart from that: LGTM 👍

[3u13r]

LGMT, I'd suggest to get a second approval for this though.

[3u13r]

euler@work:~/projects/constellation/build/aws$ ./constellation verify --output json
Using endpoint from "constellation-id.json". Specify --node-endpoint to override this.
Using ID from "constellation-id.json". Specify --cluster-id to override this.
Error: printing attestation document: parsing SNP report: parsing report to proto: array size is 0x0, an SEV-SNP attestation report size is 0x4a0

This does not work for AWS (nitro attestation).

euler@work:~/projects/constellation/build/gcp$ ./constellation verify --output json
Using endpoint from "constellation-id.json". Specify --node-endpoint to override this.
Using ID from "constellation-id.json". Specify --cluster-id to override this.
Error: printing attestation document: parsing SNP report: parsing report to proto: array size is 0x0, an SEV-SNP attestation report size is 0x4a0

Also doesn't work on GCP. Either we gracefully handle this as an error with a message that this is currently not supported or we implement this.

Doesn't work on AWS and GCP

[elchead]

good catch, thanks! Since we also only support this for Azure on the default printer, I return an unsupported message:

./constellation verify -o json
Using endpoint from "constellation-id.json". Specify --node-endpoint to override this.
Using ID from "constellation-id.json". Specify --cluster-id to override this.
Error: printing attestation document: JSON output is currently only supported for Azure

[derpsteb]

LGTM 👍

[github-actions]

Coverage report

Package	Old	New	Trend
cli/internal/cmd	56.20%	54.70%	↘️
internal/verify	0.00%	[no test files]	🚨

done

[3u13r]

LGTM