chore: allow validating SNP measurement hashes in non-linux OS #362
Conversation
Signed-off-by: Matias Fontanini <[email protected]>
mfontanini
requested review from
DGonzalezVillal,
larrydewey and
tylerfanelli
as code owners
|
Hello @mfontanini thank you for the PR. I don't love the idea of moving the PageType to launch. I could give better suggestions if I understood your use case a little better. What I'm imagining is that you are trying to generate hashes in a non-linux environment but, you can't because the measurement library is not compiling because of the linux flag. Is that correct? |
|
Yes, that's correct. I want to validate an attestation, including generating measurement hashes, in mac/windows (and linux ofc) but the way |
|
I see. I'm thinking you've hit in a bigger issue, where maybe we are using this gating functions in the wrong way. For this particular PR, you can remove the Linux gate on the measurement library and that should unblock your use case. That way you don't need to move PageType. I'll add to the to-do list to make sure other functionality isn't being blocked this way. |
|
thoughts @tylerfanelli @larrydewey |
|
That doesn't work unfortunately, I tried it. I don't remember what was the build error (and I no longer have access to a mac) but there's lots in |
DGonzalezVillal
left a comment
DGonzalezVillal
left a comment
There was a problem hiding this comment.
Wait I understand what this is doing better now. This is a good solution. Thoughts @tylerfanelli
| //! This ensures (at compile time) that the right steps are called in the | ||
| //! right order. | ||
| use crate::launch::PageType; |
There was a problem hiding this comment.
FYI if this solution works this should probably be a pub use so this doesn't break backwards compatibility.
There was a problem hiding this comment.
I don't think that would be the case because PageType is now defined in mod right? we are not exporting it from launch::snp, but from launch directly, if my reasoning is correct.
There was a problem hiding this comment.
This is a breaking change as it is. Any code that was doing use sev::launch::snp::PageType will fail to compile unless this re-export is added ^. They can always fix it to use sev::launch::PageType but still, an avoidable breaking change.
There was a problem hiding this comment.
I see, i would prefer avoiding breaking changes
There was a problem hiding this comment.
Please make that change
|
But now that I think about it. Would it make more sense to move PageType to measurement and have launch import it. Because PageType will get compiled regardless now |
4 participants
This allows all symbols that are needed for SNP measurrement hash generation (e.g. everything needed to invoke
snp_calc_launch_digest) to be used in non linux OSes. The main thing preventing this seemed to belaunch::snp::PageTypesince all oflaunch::snpwas gated behindtarget_os = "linux". I moved this type up tolaunch, which is maybe not ideal. I'm open to alternatives such as creating 2 modules withinlaunch::snpso one contains thisPageType(and maybe something else?), or anything else that makes sense.