[armenon-rh]

Currently, attestation only works with the mock kbs server.
Against the production grade Trustee KBS, attestation fails due to

1. differences in payload format (JWS JSON vs binary),
2. hashing algorithms (SHA-384 vs SHA512) and
3. key binding structures.

This PR fixes the attestation flow against a trustee KBS (CoCo) server.

1. Use sev crate as a dependency for AttestationReport struct.
2. Create generic formats to incorporate different hash algorithms (sha512, sha384) and payload formats (binary vs JwsJson) combinations to create runtime_data.
3. Make the design extensible such that a new remote verifier can be easily added.

In order to test the changes, follow the steps below

• Build svsm, aproxy.
  FW_FILE=</path/to/OVMF.fd> make FEATURES=svsm:vtpm,svsm:attest,svsm:virtio-drivers,svsm:block igvm; make aproxy; make clippy

• Start the kbs container or the kbs-test program. (See Documentation/docs/developer/KBS-SVSM.md in the current PR)
  podman container start kbs-resource-service
  OR
  kbs-test steps
  cargo run -- --measurement <hash>
# for example cargo run -- --measurement fd4b9d0cd4c5956c579c09b76e9136168eb685514427a33ca640bc1ba3e853fe1267efdc86bd1b720140fdf2f3b58d66

• Start aproxy in trustee-kbs or sample-kbs mode.
  ./bin/aproxy --protocol trustee-kbs --url http://0.0.0.0:8080 --vsock
  OR
  ./bin/aproxy --protocol sample-kbs --url http://0.0.0.0:8080 --vsock

• Start the guest using launch_guest.sh script.
  ./scripts/launch_guest.sh --vsock 3 --qemu </path/to/qemu-system-x86_64> --image <qcow image>

Verified that the attestation token is successfully issued by the trustee KBS instance, see kbs container logs below

2026-06-16T07:48:09.055691Z  INFO actix_web::middleware::logger: 10.8.1.184 "POST /kbs/v0/attest HTTP/1.1" 200 2368 "-" "-" 0.164425
2026-06-16T07:48:09.056259Z  INFO kbs::attestation::backend: Cookie 60e3c85e8d4c471eaadd0ce24c5f6e5d request to get resource
2026-06-16T07:48:09.058087Z  INFO actix_web::middleware::logger: 10.8.1.184 "GET /kbs/v0/resource/coco/mykey/k2 HTTP/1.1" 200 554 "-" "-" 0.001929

Changes in v2:

• Introduced SampleKbs and TrusteeKbs generic protocol backend variants.
• Do not rename Snp fields in libaproxy and svsm
• Rename Kbs to SampleKbs and KbsTrustee to TrusteeKbs
• Remove link in commit message
• Combine include statements.

[luigix25]

I did a quick review.

Don't forget to run make clippy cargo fmt and make test before submitting :)

[luigix25]

@armenon-rh please resolve all the conversation that no longer applies. I run make clippy locally and it's failing:

error[E0152]: found duplicate lang item `panic_impl`
   --> boot/bldr/src/main.rs:403:1
    |
403 | / fn panic(_info: &core::panic::PanicInfo<'_>) -> ! {
404 | |     // SAFETY: raising an undefined instuction exception is always safe.
405 | |     unsafe { core::arch::asm!("ud2") }
406 | |     unreachable!("");
407 | | }
    | |_^
    |
    = note: the lang item is first defined in crate `std` (which `uuid` depends on)
    = note: first definition in `std` loaded from /home/lleonard/.rustup/toolchains/1.88.0-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-8df6be531efb3fd0.rlib
    = note: second definition in the local crate (`bldr`)

not sure why, i didn't investigate, but reverting your changes it passes

[armenon-rh]

@armenon-rh please resolve all the conversation that no longer applies. I run make clippy locally and it's failing:

error[E0152]: found duplicate lang item `panic_impl`
   --> boot/bldr/src/main.rs:403:1
    |
403 | / fn panic(_info: &core::panic::PanicInfo<'_>) -> ! {
404 | |     // SAFETY: raising an undefined instuction exception is always safe.
405 | |     unsafe { core::arch::asm!("ud2") }
406 | |     unreachable!("");
407 | | }
    | |_^
    |
    = note: the lang item is first defined in crate `std` (which `uuid` depends on)
    = note: first definition in `std` loaded from /home/lleonard/.rustup/toolchains/1.88.0-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-8df6be531efb3fd0.rlib
    = note: second definition in the local crate (`bldr`)

not sure why, i didn't investigate, but reverting your changes it passes

Hi Luigi,
I have mentioned in the commit message that make clippy does not work, because sev crate we added in the commit, pulls uuid which is std dependent. So we can run make clippy CARGO_HACK=1
I do not know if this is a good idea.

[luigix25]

I see a lot of changes in Cargo.lock file, I don't think this is expected..

other than that, I'm not an attestation expert, I don't see anything wrong with the code!

[luigix25]

this should be kbs-softhsm branch, right? is there an ongoing work to upstream it?

[armenon-rh]

yes, the POC was done, with changes in my public fork of trustee. I will be sending a PR to the confidential-containers/trustee project.

[armenon-rh]

confidential-containers/trustee@51bd7bb

I am not sure if these changes should be in trustee upstream. This was only created for convenience.

It has the necessary "manual testing" configuration toml files, that help us set up the container and connect to it quickly with a few instructions.

Please guide @tylerfanelli

[stefano-garzarella]

@armenon-rh thanks for this, but I think there is a critical issue about moving the hashing in the proxy.

That said, maybe commit 2 should be split in multiple commits if possible.

[stefano-garzarella]

I'm concerned about the protocol between SVSM and aproxy. @tylerfanelli tried to make it generic, but we're adding a lot of non-generic changes here (fixed parameter order for the Raw type, fixed JSON field names for the JSON format). At this point, the protocol is no longer generic IIUC, so is it still worth using this intermediate protocol, or should we remove it and incorporate the attestation protocol directly into SVSM and implement a simple https proxy in the host?

[armenon-rh]

I'm concerned about the protocol between SVSM and aproxy. @tylerfanelli tried to make it generic, but we're adding a lot of non-generic changes here (fixed parameter order for the Raw type, fixed JSON field names for the JSON format). At this point, the protocol is no longer generic IIUC, so is it still worth using this intermediate protocol, or should we remove it and incorporate the attestation protocol directly into SVSM and implement a simple https proxy in the host?

I totally understand. There are 2 enums that I have introduced and it can become confusing. I can explain the changes.

AFAIK, SVSM can be deployed anywhere. So when its communicating to a backend storage server (to retrieve the secret key) or a verifier to produce evidence, it needs to know what protocol to use. Since we can have multiple such backends, we have tools/aproxy/backend where all of them can be defined in the future. Apart from that it also helps in the communication (HTTP).

Previously we only used to communicate with the kbs-test backend.
SVSM <--> Aproxy <--> kbs-test
It accepted evidence that was created by concatenating raw binary public key + nonce, hashed using sha512.
So SVSM only had the code to do exactly that.

With this series, we can also communicate with the CoCo Trustee KBS backend.
SVSM <--> Aproxy <--> Trustee KBS

You are right about the order of parameters in the raw binary format. I can bring back the negotiation param and thereby order the payload in RawBinary hash according to the order mentioned in the vector.
But while developing, I figured that most storage backends or verifiers available in the market for example Intel, Microsoft,.. predominantly uses the JSON format.
Microsoft
Intel
I can be totally wrong here, but this is the reason why I shifted the binary format inside
KbsBackendType::Legacy => (HashAlgo::Sha512, PayloadFormat::RawBinary) and fixed the order. It is backward compatible and the kbs-test works fine with SVSM.

About the fixed JSON field names,
If a future protocol uses a different JSON schema (e.g., "algorithm" instead of "alg", or different nesting structures), we cannot let SVSM dynamically rename keys based on aproxy input.

1. Doing so would require SVSM to parse a dynamic key-value map and construct arbitrary JSON payloads. If we introduce CBOR, then we would have to construct that payload dynamically.
2. Trying to make JSON schemas dynamically configurable by the untrusted host IMO could be a security risk. Instead, SVSM keeps its formatting statically typed and compiled.

If a future protocol uses a different schema, we simply:

1. Add a new compile-time PayloadFormat variant (e.g., PayloadFormat::Cbor or PayloadFormat::AzureKv).
2. Implement a dedicated, statically verified PayloadFormatter for that layout in SVSM.

I also agree that we can revert the renames in libaproxy and SVSM, going back to report and certs_buf. The naming is only required in aproxy (specifically in TrusteeEvidence).

So to summarize the flow, SVSM can run in any environment. Aproxy will be started on the host with appropriate command line arguments (kbs, kbs-trustee or anything else in the future). This will tell aproxy to fix a communication protocol (Payload format + hash algo). So when aproxy forwards the nonce from storage backend to SVSM, it explicitly also mentions the protocol.

SVSM constructs the runtime_data hash. This is statically fixed. To draw a parallel, just like the kernel uses a compiled-in driver (like VirtIO) to talk to a specific hardware, SVSM uses compile-time static PayloadFormatter for specific attestation standards.
Once the runtime_data hash is created, it will send the attestation request to /attest endpoint:

1. TEE evidence i.e. signed report_data
2. Cleartext runtime_data (contains nonce and tee-pubkey

The trustee kbs in our case, takes the cleartext runtime_data -> serializes into JWS JSON -> hashes it. Then compares this hash with Tee evidence report_data (hardware signed SNP report).

Please correct me if I am wrong.
cc: @tylerfanelli

Note to self:
[ ] Bring back NegotiationParam for raw binary format
[ ] Rename the fields only in aproxy

[stefano-garzarella]

AFAIK, SVSM can be deployed anywhere. So when its communicating to a backend storage server (to retrieve the secret key) or a verifier to produce evidence, it needs to know what protocol to use. Since we can have multiple such backends, we have tools/aproxy/backend where all of them can be defined in the future. Apart from that it also helps in the communication (HTTP).

Not really, the goal was to make SVSM completely attestation protocol-agnostic.

Let me summarize what @berrange, @tylerfanelli , and I discussed some time ago.
Basically, we discussed two options:
1 - Make SVSM completely agnostic regarding the attestation protocol to be used, implement the protocol (e.g. Trustee) entirely in the proxy, and have a generic protocol between SVSM and the proxy. This would ensure that any new attestation protocol to support or change to the them would not require modifications to SVSM, and thus would not affect launch measurements, etc.
2 - Use the proxy as a simple HTTP proxy and fully implement the attestation protocol in SVSM (e.g. Trustee). This drastically simplifies the protocol between the proxy and SVSM, but for every new protocol or change, it was necessary to update SVSM, launch measurements, etc.

We went with Option 1 mainly to avoid making changes to SVSM as much as possible. If we see that Option 1 is difficult to maintain, I'm open to switching to Option 2, but not to having something in between that just complicates things.

Previously we only used to communicate with the kbs-test backend. SVSM <--> Aproxy <--> kbs-test It accepted evidence that was created by concatenating raw binary public key + nonce, hashed using sha512. So SVSM only had the code to do exactly that.

With this series, we can also communicate with the CoCo Trustee KBS backend. SVSM <--> Aproxy <--> Trustee KBS

The goal is clear, I'm worried about the design of the protocol between SVSM and the proxy.

You are right about the order of parameters in the raw binary format. I can bring back the negotiation param and thereby order the payload in RawBinary hash according to the order mentioned in the vector. But while developing, I figured that most storage backends or verifiers available in the market for example Intel, Microsoft,.. predominantly uses the JSON format. Microsoft Intel I can be totally wrong here, but this is the reason why I shifted the binary format inside KbsBackendType::Legacy => (HashAlgo::Sha512, PayloadFormat::RawBinary) and fixed the order. It is backward compatible and the kbs-test works fine with SVSM.

About the fixed JSON field names, If a future protocol uses a different JSON schema (e.g., "algorithm" instead of "alg", or different nesting structures), we cannot let SVSM dynamically rename keys based on aproxy input.

1. Doing so would require SVSM to parse a dynamic key-value map and construct arbitrary JSON payloads. If we introduce CBOR, then we would have to construct that payload dynamically.

2. Trying to make JSON schemas dynamically configurable by the untrusted host IMO could be a security risk. Instead, SVSM keeps its formatting statically typed and compiled.

If a future protocol uses a different schema, we simply:

1. Add a new compile-time PayloadFormat variant (e.g., PayloadFormat::Cbor or PayloadFormat::AzureKv).

2. Implement a dedicated, statically verified PayloadFormatter for that layout in SVSM.

This really brings us back to the discussion of the options I mentioned above. This seems to fall right in the middle of the two, and IMO, we should avoid it at all costs because it takes away the advantages of option 1 and adds the disadvantages of option 2. So IMHO it’s basically the worst-case scenario.

I also agree that we can revert the renames in libaproxy and SVSM, going back to report and certs_buf. The naming is only required in aproxy (specifically in TrusteeEvidence).

So to summarize the flow, SVSM can run in any environment. Aproxy will be started on the host with appropriate command line arguments (kbs, kbs-trustee or anything else in the future). This will tell aproxy to fix a communication protocol (Payload format + hash algo). So when aproxy forwards the nonce from storage backend to SVSM, it explicitly also mentions the protocol.

IMO this should be avoided, if we implement option 1, SVSM - proxy protocol should don't know anything about the attestation protocol used, everything should be handled by the proxy and translated into the generic protocol.

Or we should switch to option 2 and have a simple http proxy and let SVSM handle everything.

I hope I've made it clear that I'm concerned we're no longer doing either Option 1 or Option 2, but something in between that loses the benefits of both.

Thanks,
Stefano

[armenon-rh]

AFAIK, SVSM can be deployed anywhere. So when its communicating to a backend storage server (to retrieve the secret key) or a verifier to produce evidence, it needs to know what protocol to use. Since we can have multiple such backends, we have tools/aproxy/backend where all of them can be defined in the future. Apart from that it also helps in the communication (HTTP).

Not really, the goal was to make SVSM completely attestation protocol-agnostic.

Let me summarize what @berrange, @tylerfanelli , and I discussed some time ago. Basically, we discussed two options: 1 - Make SVSM completely agnostic regarding the attestation protocol to be used, implement the protocol (e.g. Trustee) entirely in the proxy, and have a generic protocol between SVSM and the proxy. This would ensure that any new attestation protocol to support or change to the them would not require modifications to SVSM, and thus would not affect launch measurements, etc. 2 - Use the proxy as a simple HTTP proxy and fully implement the attestation protocol in SVSM (e.g. Trustee). This drastically simplifies the protocol between the proxy and SVSM, but for every new protocol or change, it was necessary to update SVSM, launch measurements, etc.

We went with Option 1 mainly to avoid making changes to SVSM as much as possible. If we see that Option 1 is difficult to maintain, I'm open to switching to Option 2, but not to having something in between that just complicates things.

Previously we only used to communicate with the kbs-test backend. SVSM <--> Aproxy <--> kbs-test It accepted evidence that was created by concatenating raw binary public key + nonce, hashed using sha512. So SVSM only had the code to do exactly that.
With this series, we can also communicate with the CoCo Trustee KBS backend. SVSM <--> Aproxy <--> Trustee KBS

The goal is clear, I'm worried about the design of the protocol between SVSM and the proxy.

You are right about the order of parameters in the raw binary format. I can bring back the negotiation param and thereby order the payload in RawBinary hash according to the order mentioned in the vector. But while developing, I figured that most storage backends or verifiers available in the market for example Intel, Microsoft,.. predominantly uses the JSON format. Microsoft Intel I can be totally wrong here, but this is the reason why I shifted the binary format inside KbsBackendType::Legacy => (HashAlgo::Sha512, PayloadFormat::RawBinary) and fixed the order. It is backward compatible and the kbs-test works fine with SVSM.
About the fixed JSON field names, If a future protocol uses a different JSON schema (e.g., "algorithm" instead of "alg", or different nesting structures), we cannot let SVSM dynamically rename keys based on aproxy input.

1. Doing so would require SVSM to parse a dynamic key-value map and construct arbitrary JSON payloads. If we introduce CBOR, then we would have to construct that payload dynamically.

2. Trying to make JSON schemas dynamically configurable by the untrusted host IMO could be a security risk. Instead, SVSM keeps its formatting statically typed and compiled.

If a future protocol uses a different schema, we simply:

1. Add a new compile-time PayloadFormat variant (e.g., PayloadFormat::Cbor or PayloadFormat::AzureKv).

2. Implement a dedicated, statically verified PayloadFormatter for that layout in SVSM.

This really brings us back to the discussion of the options I mentioned above. This seems to fall right in the middle of the two, and IMO, we should avoid it at all costs because it takes away the advantages of option 1 and adds the disadvantages of option 2. So IMHO it’s basically the worst-case scenario.

I also agree that we can revert the renames in libaproxy and SVSM, going back to report and certs_buf. The naming is only required in aproxy (specifically in TrusteeEvidence).
So to summarize the flow, SVSM can run in any environment. Aproxy will be started on the host with appropriate command line arguments (kbs, kbs-trustee or anything else in the future). This will tell aproxy to fix a communication protocol (Payload format + hash algo). So when aproxy forwards the nonce from storage backend to SVSM, it explicitly also mentions the protocol.

IMO this should be avoided, if we implement option 1, SVSM - proxy protocol should don't know anything about the attestation protocol used, everything should be handled by the proxy and translated into the generic protocol.

Or we should switch to option 2 and have a simple http proxy and let SVSM handle everything.

I hope I've made it clear that I'm concerned we're no longer doing either Option 1 or Option 2, but something in between that loses the benefits of both.

Thanks, Stefano

Thank you so much for explaining the design choices. You have perfectly captured the architectural tradeoffs.

I understand the design choice sits right between option1 and option2
But that is because we have some constraints.

1. We cannot implement option 1 fully because,
   As I discovered from the previous review: The hash must be computed inside SVSM.
   If we delegate this to aproxy (like we had in v2 patches), then an untrusted host
   can hash its own key in runtime_data and sign the report using AMD Secure processor.
   SVSM cannot output a binary hash and expect the aproxy to translate it into
   JWS JSON hash later. Therefore, the exact payload structure must be formatted
   inside SVSM before hashing it.

   Now I agree that we also want to keep SVSM agnostic of the backend storage.
   SVSM does not know about Trustee or specific backend servers. Instead it is only
   aware of standard serialization formats (JwsJson or RawBinary).
   If we come across other backends expecting the same formats, we will be able to
   configure them without having to make any changes in SVSM.
   Maybe we can call it format aware and not backend aware.

2. We cannot implement option 2 fully, because AFAIK we do not have TCP/IP stack in SVSM.
   Adding that in SVSM can increase the attack surface.

The middleground is that the proxy must handle the HTTP layer but SVSM must handle the JwsJson formatting and hashing. This achieves security (from host key substitution) and network safety.

I totally understand the concerns. But I could not figure out a way to achieve both at the same time.

[stefano-garzarella]

I understand the design choice sits right between option1 and option2 But that is because we have some constraints.

1. We cannot implement option 1 fully because,
   As I discovered from the previous review: The hash must be computed inside SVSM.
   If we delegate this to aproxy (like we had in v2 patches), then an untrusted host
   can hash its own key in runtime_data and sign the report using AMD Secure processor.

We already discussed about that, and we agreed we can't do that. Indeed we introduced NegotiationParam exactly for that reason...

   SVSM cannot output a binary hash and expect the aproxy to translate it into
   JWS JSON hash later. Therefore, the exact payload structure must be formatted
   inside SVSM before hashing it.

Right, so the proxy should tell to SVSM how to construct the json, what to put in some field, what name to use, etc. Like we did with NegotiationParam but extending them (e.g. introducing the name to be used in a json) or using some kind of template, or something else. IMO having protocol-specific stuff (order, names, etc.) in SVSM is wrong and must be avoided. If we need to do that, we should move to option 2.

   Now I agree that we also want to keep SVSM agnostic of the backend storage.
   SVSM does not know about Trustee or specific backend servers. Instead it is only
   aware of standard serialization formats (JwsJson or RawBinary).
   If we come across other backends expecting the same formats, we will be able to
   configure them without having to make any changes in SVSM.
   Maybe we can call it format aware and not backend aware.

I don't see the point of doing this TBH. You mean ask other attestation protocols to change to be adapted to our internal protocol? Why? At this point we should go through option 2, I don't understand why we have to complicate our own lives and those of others.

What are the advantages of this approach, compared with option 2?

2. We cannot implement option 2 fully, because AFAIK we do not have TCP/IP stack in SVSM.
   Adding that in SVSM can increase the attack surface.

We did that without any TCP/IP stack in SVSM. It was my first attempt, let me find the code: https://github.com/stefano-garzarella/RAClients

The protocol was very simple:

#[derive(Serialize, Deserialize, Debug, Eq, PartialEq)]
pub enum HttpMethod {
    GET,
    POST,
}

#[derive(Serialize, Deserialize, Debug, Eq, PartialEq)]
pub struct Request {
    pub endpoint: String,
    pub method: HttpMethod,
    pub body: Value,
}

#[derive(Serialize, Deserialize, Debug, Eq, PartialEq)]
pub struct Response {
    pub status: u16,
    pub body: String,
}

The proxy will be used exactly to avoid TCP/IP in SVSM, otherwise we don't need the proxy at all if SVSM can talk TCP/IP.

SVSM will create the entire payload of the message, send it to the proxy with our custom protocol, the proxy will put it in a http packet and send it to the server, opposite on the other direction.

[armenon-rh]

Hi,

I have implemented option2.

In the new design, aproxy is a simple HTTP forwarder. SVSM does the job of constructing secure runtime_data and hashing it. A combination of Payloadformat and Hashalgo (mentioned using --protocol) allows the user to indicate what kind of data format is expected by the backend while bringing up aproxy.

Since we no longer import the external sev crate to match the AttestationReport in aproxy, SVSM defines a few structs in kernel/src/greq/pld_report.rs to help with serialization of binary data into JSON before sending it across to the backend.

I still need to add documentation in the PR, but I will start working on it once the design is approved.

Thanks
Arun.

[tylerfanelli]

Thanks @armenon-rh. I have one main suggestion for this PR. @stefano-garzarella I'd also like your thoughts.

In the negotiation phase of the protocol, we originally designed the NegotiationResponse to contain a set of negotiation parameters that the server expects to be included in the HOST_DATA hash.

struct NegotiationResponse {
    /// Challenge returned from the attestation server to verify freshness of attestation evidence.
    pub challenge: Vec<u8>,

    /// Parameters to be hashed in the specific order defined by the array
    pub params: Vec<NegotiationParam>,
}

However, perhaps this is too generic. AFAIU, each server implementation (Trustee KBS being at the forefront of my mind when considering this) has only one expected format that HOST_DATA hashes should follow. With this in mind, why don't we just throw out the negotiation parameters field altogether and instead specify which server implementation is being used, so SVSM can hash the parameters in the order and format expected by the server. For Trustee KBS, this could be as follows:

struct NegotiationResponse {
    /// Challenge returned from the attestation server to verify freshness of attestation evidence.
    pub challenge: Vec<u8>,

    /// The protocol (ordering and formatting of negotiation parameters) expected by the server.
    pub hash_protocol: HashProtocol,
}

HashProtocol can be an enum specifying the server implementation, like so:

pub enum HashProtocol {
    /// Sample KBS used for testing.
    SampleKbs,

    /// Trustee KBS.
    TrusteeKbs,
}

Then, SVSM can compile the HOST_DATA hash based on the result of the hash_protocol specified in the negotiation response. With this, the hash can be constructed in the correct ordering and formatting (in Trustee's case, JWS JSON).

The downside to this is we're essentially bringing server-specific traits to the SVSM kernel. However, I think this is a relatively small area (i.e. how the negotiation parameters are hashed) requiring this change and thus worth the trouble.

Furthermore, this change is extensible, as when another server proxy comes along, it can specify another HashProtocol enum value and write a module of how to compile the HOST_DATA hash in the way expected by that server.

[stefano-garzarella]

The downside to this is we're essentially bringing server-specific traits to the SVSM kernel. However, I think this is a relatively small area (i.e. how the negotiation parameters are hashed) requiring this change and thus worth the trouble.

Furthermore, this change is extensible, as when another server proxy comes along, it can specify another HashProtocol enum value and write a module of how to compile the HOST_DATA hash in the way expected by that server.

IMO, this means we lose all the benefits of having a generic protocol between SVSM and the proxy, since if we need to support a new attestation protocol or a new version of it, we have to update SVSM and then have a new launch measurement, etc.

At this point, what would be the advantage of having a complicated generic protocol between SVSM and the proxy compared to having a simple HTTP proxy and implementing the entire protocol in SVSM?

The advantages of the latter are a very simple proxy, a simple protocol between the proxy and SVSM, and the entire attestation protocol implemented in a single trusted location (i.e. SVSM), so we can avoid any bad assumption (e.g. making the host to perform the hash, etc.).

[armenon-rh]

Thanks @armenon-rh. I have one main suggestion for this PR. @stefano-garzarella I'd also like your thoughts.

In the negotiation phase of the protocol, we originally designed the NegotiationResponse to contain a set of negotiation parameters that the server expects to be included in the HOST_DATA hash.

struct NegotiationResponse {
    /// Challenge returned from the attestation server to verify freshness of attestation evidence.
    pub challenge: Vec<u8>,

    /// Parameters to be hashed in the specific order defined by the array
    pub params: Vec<NegotiationParam>,
}

However, perhaps this is too generic. AFAIU, each server implementation (Trustee KBS being at the forefront of my mind when considering this) has only one expected format that HOST_DATA hashes should follow. With this in mind, why don't we just throw out the negotiation parameters field altogether and instead specify which server implementation is being used, so SVSM can hash the parameters in the order and format expected by the server. For Trustee KBS, this could be as follows:

struct NegotiationResponse {
    /// Challenge returned from the attestation server to verify freshness of attestation evidence.
    pub challenge: Vec<u8>,

    /// The protocol (ordering and formatting of negotiation parameters) expected by the server.
    pub hash_protocol: HashProtocol,
}

HashProtocol can be an enum specifying the server implementation, like so:

pub enum HashProtocol {
    /// Sample KBS used for testing.
    SampleKbs,

    /// Trustee KBS.
    TrusteeKbs,
}

Then, SVSM can compile the HOST_DATA hash based on the result of the hash_protocol specified in the negotiation response. With this, the hash can be constructed in the correct ordering and formatting (in Trustee's case, JWS JSON).

The downside to this is we're essentially bringing server-specific traits to the SVSM kernel. However, I think this is a relatively small area (i.e. how the negotiation parameters are hashed) requiring this change and thus worth the trouble.

Furthermore, this change is extensible, as when another server proxy comes along, it can specify another HashProtocol enum value and write a module of how to compile the HOST_DATA hash in the way expected by that server.

Thanks @armenon-rh. I have one main suggestion for this PR. @stefano-garzarella I'd also like your thoughts.

In the negotiation phase of the protocol, we originally designed the NegotiationResponse to contain a set of negotiation parameters that the server expects to be included in the HOST_DATA hash.

struct NegotiationResponse {
    /// Challenge returned from the attestation server to verify freshness of attestation evidence.
    pub challenge: Vec<u8>,

    /// Parameters to be hashed in the specific order defined by the array
    pub params: Vec<NegotiationParam>,
}

However, perhaps this is too generic. AFAIU, each server implementation (Trustee KBS being at the forefront of my mind when considering this) has only one expected format that HOST_DATA hashes should follow. With this in mind, why don't we just throw out the negotiation parameters field altogether and instead specify which server implementation is being used, so SVSM can hash the parameters in the order and format expected by the server. For Trustee KBS, this could be as follows:

struct NegotiationResponse {
    /// Challenge returned from the attestation server to verify freshness of attestation evidence.
    pub challenge: Vec<u8>,

    /// The protocol (ordering and formatting of negotiation parameters) expected by the server.
    pub hash_protocol: HashProtocol,
}

HashProtocol can be an enum specifying the server implementation, like so:

pub enum HashProtocol {
    /// Sample KBS used for testing.
    SampleKbs,

    /// Trustee KBS.
    TrusteeKbs,
}

Then, SVSM can compile the HOST_DATA hash based on the result of the hash_protocol specified in the negotiation response. With this, the hash can be constructed in the correct ordering and formatting (in Trustee's case, JWS JSON).

The downside to this is we're essentially bringing server-specific traits to the SVSM kernel. However, I think this is a relatively small area (i.e. how the negotiation parameters are hashed) requiring this change and thus worth the trouble.

Furthermore, this change is extensible, as when another server proxy comes along, it can specify another HashProtocol enum value and write a module of how to compile the HOST_DATA hash in the way expected by that server.

Thank you @tyler for the detailed explanation. This is what I am adding.

/// A parameter that must be hashed into the negotiation hash.
#[derive(Serialize, Deserialize, Debug)]
pub enum NegotiationParam {
    /// Hash the challenge returned from attestation server.
    Challenge,
    /// Hash the EC public key's `Elliptic-Curve-Point-to-Octet-String` encoding.
    EcPublicKeyBytes,
}

pub enum PayloadFormat {
    /// Raw sequential binary representation of public key coordinates and challenge.
    RawBinary,
    /// JWS-compliant JSON formatted representation of runtime_data.
    JwsJson,
}

#[derive(Serialize, Deserialize, Debug)]
pub struct NegotiationResponse {
    /// Challenge returned from the attestation server to verify freshness of attestation evidence.
    #[serde(
        serialize_with = "serialize_base64",
        deserialize_with = "deserialize_base64"
    )]
    pub challenge: Vec<u8>,
    /// The hashing algorithm to use.
    pub hash_algo: HashAlgo,
    /// The payload formatting to use.
    pub payload_format: PayloadFormat,
}

NegotiationParam is still kept so that kbs-test work with the RawBinary formatter

[armenon-rh]

I see a lot of changes in Cargo.lock file, I don't think this is expected..

The sev crate included brings in some dependencies.

[luigix25]

This is a breaking change and we need to update all the doc accordingly as we don't support kbs anymore.
aproxy --protocol kbs would not work.

Is it really worth it to rename it? Open question for you and the maintainers.

[armenon-rh]

I had kept it the same before. But a more descriptive name indeed helps us distinguish between the two.
I do not have a strict preference. Kbs can mean the mock KBS and KbsTrustee would indicate the other.

[armenon-rh]

changes in v3:

• change NegotiationRequest version to 0.2.0 as we have added hash_algo and payload_format in it.
• refactored AttestationDriver, removed double match and setting params to none in JwsJson format.
• updated documentation everywhere to reflect the new arguments to be passed to /bin/aproxy binary.
  instead of kbs, we need to pass sample-kbs / trustee-kbs now.
• Removed #[allow(unused_imports)]. It was added in one commit and removed later.

[luigix25]

there are some typos in the commit messages, please fix them

[luigix25]

can we add default-features = false so we pull in less dependencies?

[armenon-rh]

yes

[luigix25]

bump this to 0.2.0?

[armenon-rh]

yes, will do. Thanks.

[luigix25]

mmh why this anyhow::anyhow?

[armenon-rh]

The use of bail was removed, or I can say, impl TryFrom<&AttestationRequest> for KbsEvidence - was removed . So the import was also removed.
I will explicitly bring anyhow in the scope use anyhow::{anyhow, Context};

[luigix25]

what about using as_ref() instead of clone?

[armenon-rh]

yea, will change it.

[luigix25]

this is pre-existing, but I don't think we need it, do we?

[armenon-rh]

yes, make did not complain after removing it.

[armenon-rh]

changes in v4:

• Add default-features = false for the sev crate to pull less dependencies.
• Bump kbs version to 0.2.0 in the docs. Bring anyhow into scope.
• Change from .clone() to .as_ref(). Remove #[allow(irrefutable_let_patterns)