-
Notifications
You must be signed in to change notification settings - Fork 140
protocol: support backwards compatibility for v0.2.0 #982
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
protocol: support backwards compatibility for v0.2.0 #982
Conversation
Allow v0.4.0 attestation reports to be created from v0.2.0 reports and tweak our version check so that we can specify a list of supported protocol versions rather than just one. This does not imply that we have a policy of backwards compatibility. This does not support v0.3.0 (which was never released) and it is very unlikely that anything before v0.2.0 will ever be supported. Some of these changes could also go in kbs-types, but because the KBS protocol itself does not say anything about backwards comaptibility, I consider this a feature of Trustee and have implemented it here. This could be adjusted in the future. Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
aea0915 to
0eb2b2e
Compare
mkulke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have tests with 0.2 fixtures?
I mentioned this in #907 (comment). I am not sure the best way to implement. I think this is something that we should probably do for v1.0.0+. For v0.2.0 I could go either way. |
|
I love this and hope we offer this up for very selfish reasons. |
|
@fitzthum Probably there is more than one thing that can go wrong, not only the kbs_protocol. |
Yes, it is somewhat debatable if this is in scope of the KBS Protocol. Even with this PR there are plenty of ways for things to break. In this case, it would be trivial to allow serde to deserialize this enum from either string. I suggested this on confidential-containers/guest-components#1115 and I would still like to see that implemented, but probably not in this PR. |
|
That said there might be a deeper incompatibility in the Az evidence. If we fixed the deserialization of the enum I'm not sure if attestation would actually work on Az given some of the crate bumps. cc @mkulke |
yes, this particular issue might be fixable by silently converting the strings for 0.2. evidence, but Tobin's right: there have been various sev crate bumps in between releases that might had an effect on how evidence are being serialized. Also, at some point the vTPM's EKpub was added as part of the evidence, so we had accommodate that in the verifier. So far there has been a known-to-work permutation of kata+gc+trustee with each CoCo release. If we now want to bolt on backwards compatibility, that's a bit of a chore, but doable, I think. In any case, to claim backwards compatibility to work (and keep working in the future) we would need unit tests that use v0.2.0 evidence fixtures, no? |
|
Would it make sense to target for backwards compatibility from 0.4.0 onwards ? |
What that means needs to be defined first. A couple of corner cases were already discussed: e.g., if an attester changes the evidence format the verifier fails to understand or if the TEE name gets changed. These are opague to the protocol but users will see errors if Trustee/gc versions do not match. IOW, the compatibility goes beyond just the KBS protocol version. |
Maybe like Tobin suggested, just promote 0.4.0 to 1.0.0 coupled with a formal schema that can be enforced and tested against. I personally wouldn't really advocate for that now, since it looks like we're still iterating on a lot of aspects (like should vTPM be a CPU or a device?), but if it's causing real pain for adopters, then maybe it's worth it. |
For adopters, we can start publishing a version matrix of guest-components and trustee. This will avoid surprises. And as for compatibility we can start defining what it means from protocol standpoint, evidence format, attester names, new attesters etc as @mythi mentioned and then have these in the CI to catch breakages. Before end-of-this year, we will most likely have one more release (0.5.0). So with the foundation in place we can think about that being 1.0.0. |
that's a good idea, I think we can do this for a few releases back. I think whatever was defined in versions.yaml of kata for a given CoCo release was authoritative, since this was used in e2e tests and also picked up by peerpods. |
Many people have run into problems using the older kbs clients with current versions of Trustee. Although the KBS Protocol is not backwards compatible (prior to v1.0.0), in this case it is possible to support this behavior with a relatively small set of changes. This does not imply that we will offer similar support for any other versions. It is unlikely that we will. In fact, this PR might not even be merged, but if not you can at least apply it yourself. If we merge this PR, we might drop support for v0.2.0 in the future.
The changes here are as follows.
Overall I think this is probably clean enough to support, especially given how many people have run into problems here. That said, it may differ a little bit from how we implement backwards compatibility in the future. For instance we may want to consider adding versioned endpoints.
Some of these changes could also go in kbs-types, but because the KBS protocol itself does not say anything about backwards comaptibility, I consider this a feature of Trustee and have implemented it here. This could be adjusted in the future.
It's also worth thinking about whether this creates any security issues (i.e. downgrading the protocol to skip attesting devices). I don't think it does, but you may want to ponder this independently.