Re: [PATCH Part1 v5 34/38] x86/sev: Add snp_msg_seqno() helper

On 9/9/21 11:21 AM, Peter Gonda wrote:

On Thu, Sep 9, 2021 at 10:17 AM Brijesh Singh [off-list ref] wrote:

quoted

On 9/9/21 10:43 AM, Peter Gonda wrote:
...

quoted

quoted

Does this address your concern?

So the 'snp_msg_seqno()' call in 'enc_payload' will not increment the
counter, its only incremented on 'snp_gen_msg_seqno()'? If thats
correct, that addresses my first concern.

Yes, that is goal.

quoted

quoted

quoted

So far, the only user for the snp_msg_seqno() is the attestation driver.
And the driver is designed to serialize the vmgexit request and thus we
should not run into concurrence issue.

That seems a little dangerous as any module new code or out-of-tree
module could use this function thus revealing this race condition
right? Could we at least have a comment on these functions
(snp_msg_seqno and snp_gen_msg_seqno) noting this?

Yes, if the driver is not performing the serialization then we will get
into race condition.

One way to avoid this requirement is to do all the crypto inside the
snp_issue_guest_request() and eliminate the need to export the
snp_msg_seqno().

I will add the comment about it in the function.

Actually I forgot that the sequence number is the only component of
the AES-GCM IV. Seen in 'enc_payload'. Given the AES-GCM spec requires
uniqueness of the IV. I think we should try a little harder than a
comment to guarantee we never expose 2 requests encrypted with the
same sequence number / IV. It's more than just a DOS against the
guest's PSP request ability but also could be a guest security issue,
thoughts?

Ah good point, we should avoid a request with same IV. May be move the 
sequence number increment and save in sevguest drv. Then driver can do 
the sequence get, vmgexit and increment with a protected lock.

thanks

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnvlpubs.nist.gov%2Fnistpubs%2FLegacy%2FSP%2Fnistspecialpublication800-38d.pdf&data=04%7C01%7Cbrijesh.singh%40amd.com%7C46a05f4713834307706608d973ade616%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637668013461202204%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=KCsi5rTQX6L%2BqY07VdBtF8IH0TLNyHn6wTyidgWvXf4%3D&reserved=0
(Section 8 page 18)

quoted

thanks