Messages in this thread | | | Subject | Re: [PATCH Part1 v5 34/38] x86/sev: Add snp_msg_seqno() helper | From | Brijesh Singh <> | Date | Thu, 9 Sep 2021 14:26:49 -0500 |
| |
On 9/9/21 11:21 AM, Peter Gonda wrote: > On Thu, Sep 9, 2021 at 10:17 AM Brijesh Singh <brijesh.singh@amd.com> wrote: >> >> >> >> On 9/9/21 10:43 AM, Peter Gonda wrote: >> ... >> >>>> >>>> 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. >> >>>>> >>>> >>>> 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) > >> >> thanks >
| |