Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221019150333.1047423-1-pgonda@google.com> <528937ab-8046-d5d1-26ff-50ef35f5635f@amd.com>
In-Reply-To: <528937ab-8046-d5d1-26ff-50ef35f5635f@amd.com>
From:   Peter Gonda <pgonda@google.com>
Date:   Wed, 19 Oct 2022 11:40:18 -0600
Message-ID: <CAMkAt6ritG1zmOreh9WYLYAGww0EJQy+m-Y0nfxD5+gpTkpJ1w@mail.gmail.com>
Subject: Re: [PATCH] virt: Prevent AES-GCM IV reuse in SNP guest driver
To:     Tom Lendacky <thomas.lendacky@amd.com>
Cc:     Borislav Petkov <bp@suse.de>, Michael Roth <michael.roth@amd.com>,
        Haowen Bai <baihaowen@meizu.com>,
        Yang Yingliang <yangyingliang@huawei.com>,
        Marc Orr <marcorr@google.com>,
        David Rientjes <rientjes@google.com>,
        Ashish Kalra <Ashish.Kalra@amd.com>,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        Dionna Glaze <dionnaglaze@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Oct 19, 2022 at 11:03 AM Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> On 10/19/22 10:03, Peter Gonda wrote:
> > The ASP and an SNP guest use a series of AES-GCM keys called VMPCKs to
> > communicate securely with each other. The IV to this scheme is a
> > sequence number that both the ASP and the guest track. Currently this
> > sequence number in a guest request must exactly match the sequence
> > number tracked by the ASP. This means that if the guest sees an error
> > from the host during a request it can only retry that exact request or
> > disable the VMPCK to prevent an IV reuse. AES-GCM cannot tolerate IV
> > reuse see:
> > https://csrc.nist.gov/csrc/media/projects/block-cipher-techniques/documents/bcm/comments/800-38-series-drafts/gcm/joux_comments.pdf
>
> I wonder if we can at least still support the extended report length query
> by having the kernel allocate the required pages when the error is
> SNP_GUEST_REQ_INVALID_LEN and retry the exact request again. If there are
> no errors on the second request, the sequence numbers can be safely
> updated, but the kernel returns the original error (which will provide the
> caller with the number of pages required).

I think we can but I thought fixing the security bug could come first,
then the usability fix after. Dionna was planning on working on that
fix.

In that flow how does userspace get the data? Its called the ioctl
with not enough output buffer space. What if the userspace calls the
ioctl with no buffers space allocated, so its trying to query the
length. We just send the host the request without any encrypted data.

>
> For the rate-limiting patch series [1], the rate-limiting will have to be
> performed within the kernel, while the mutex is held, and then retry the
> exact request again. Otherwise, that error will require disabling the
> VMPCK. Either that, or the hypervisor must provide the rate limiting.
>
> Thoughts?
>
> [1] https://lore.kernel.org/lkml/20221013160040.2858732-1-dionnaglaze@google.com/

Yes I think if the host rate limits the guest. The guest kernel should
retry the exact message. Which mutex are you referring too?