confidential-containers / td-shim

Confidential Containers Shim Firmware
Other
97 stars 53 forks source link

Add support for caculating kernel eventlog digist when using OVMF+QEMU #655

Closed OuyangHang33 closed 8 months ago

OuyangHang33 commented 9 months ago

Fix: #633

Tested with local bzImage binary and the result is matched with the programe output mentioned in #633 .

OuyangHang33 commented 9 months ago

@jiazhang0 , would you please review this patch?

jyao1 commented 9 months ago

@jiazhang0 do you have time to review this patch?

portersrc commented 8 months ago

Thanks @OuyangHang33! One final doubt, because I don't have enough context to understand your qemu_patch function:

Should you be predicating any of those byte assignments based on a protocol value? The code you referenced sets a kernel protocol variable here, and this affects whether or not patches get applied, it seems (e.g. compare your buf[0x210] assignment vs. the predicated one).

Hope that makes sense. My intuition for this question is that the digest will be wrong whenever the protocol version affects these cases.

OuyangHang33 commented 8 months ago

Hi @portersrc, based on this 1.2. The Real-Mode Kernel Header, we know that if cmdline_size exsit and is used, then protocol version should be 2.06+. And td-shim needs this cmdline_size in SETUPHEADER: https://github.com/confidential-containers/td-shim/blob/9b4e454acba56c5cdf3c4c4f5df393a309c8daf7/td-shim/src/bin/td-shim/linux/kernel_param.rs#L53 https://github.com/confidential-containers/td-shim/blob/9b4e454acba56c5cdf3c4c4f5df393a309c8daf7/td-shim/src/bin/td-shim/linux/boot.rs#L88 The input kernel protocol version should be 2.06+ (over 0x206). I've added the Note in README and check in code.

OuyangHang33 commented 8 months ago

Thank you @portersrc @Xynnn007 ! Do you have any more questions for this PR?

Xynnn007 commented 8 months ago

Thank you @portersrc @Xynnn007 ! Do you have any more questions for this PR?

Thanks. Let me do a test to check if it is ok now. Once finished I will add an approve to this PR. Sorry for my busy time, I promise it will be ok until the end of the next week

portersrc commented 8 months ago

Thanks for fielding all my questions; yeah looks good to me!

OuyangHang33 commented 8 months ago

Thank you very much @Xynnn007 and @arronwy!