Hey Greg,
On 30.09.23 08:20, Greg Kroah-Hartman wrote:
> On Fri, Sep 29, 2023 at 09:26:16PM +0200, Alexander Graf wrote:
>> Hi Arnd!
>>
>> On 29.09.23 19:28, Arnd Bergmann wrote:
>>> On Fri, Sep 29, 2023, at 09:33, Alexander Graf wrote:
>>>> When running Linux inside a Nitro Enclave, the hypervisor provides a
>>>> special virtio device called "NSM". This device has 2 main functions:
>>>>
>>>> 1) Provide attestation reports
>>>> 2) Modify PCR state
>>>> 3) Provide entropy
>>>>
>>>> This patch adds the core NSM driver that exposes a /dev/nsm device node
>>>> which user space can use to request attestation documents and influence
>>>> PCR states. A follow up patch will add a hwrng driver to feed its entropy
>>>> into the kernel.
>>>>
>>>> Originally-by: Petre Eftime <[email protected]>
>>>> Signed-off-by: Alexander Graf <[email protected]>
>>> Hi Alex,
>>>
>>> I've taken a first look at this driver and have some minor comments.
>>
>> Thanks a bunch!
>>
>>
>>> The main point here is that I think we need to look at possible
>>> alternatives for the user space interface, and (if possible) change
>>> to a set of higher-level ioctl commands from the simple passthrough.
>>
>> I'm slightly torn on that bit. I think in hindsight the NSM device probably
>> should have been a reserved vsock CID and the hwrng one should have just
>> been virtio-rng.
>>
>> The problem is that Nitro Enclaves were launched in 2020 and since an
>> ecosystem developed in multiple languages to support building code inside:
>>
>> https://github.com/aws/aws-nitro-enclaves-nsm-api/blob/main/src/driver/mod.rs#L66
>> https://github.com/donkersgoed/aws-nsm-interface/blob/main/aws_nsm_interface/__init__.py#L264-L274
>> https://github.com/hf/nsm/blob/main/nsm.go#L99-L129
>>
>>
>> All of these use the (downstream) ioctl that this patch also implements. We
>> could change it, but instead of making it easier for user space to adapt the
>> device node, it would probably hurt more.
>>
>> I agree that this is not a great place to be in. This driver absolutely
>> should have been upstreamed 3 years ago. But I can't turn back time (yet)
>> :).
> As you know, this is no excuse to put an api in the kernel that isn't
> correct or good for the long-term. Just because people do foolish
> things outside of the kernel tree never means we have to accept them in
> our tree. Instead we can ask them to fix them properly as part of us
> taking the code.
>
> So please, work on doing this right.
Sorry if my message above came over as a push to put an "incorrect api"
into the kernel.
In situations like this where you can either give user space full access
to the device's command space through a generic API or you can create
command awareness in the kernel and make it the kernel's task to learn
about each command, IMHO it's never a clear cut on which one is better.
Especially in virtual environments where the set of commands can change
quickly over time.
So what I was trying to say above is that *if* we consider both paths
equally viable, I'd err on the one that enables the existing ecosystem.
However if there are good reasons to not do command pass-through, I'm
all for abstracting it away :)
Looking at prior art, the most similar implementations to this are TPMs
and virtio-vsock. With virtio-vsock, kernel space has no idea what it
talks to on the other hand and makes it 100% user space's problem. With
TPMs, you typically use /dev/tpm0 to gain raw command access to the
target device. So while we could engineer something smarter here, I'm
not convinced yet it's a net win.
Alex
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879