On Fri, 13 Aug 2021 at 15:05, Andrew Scull <ascull@xxxxxxxxxx> wrote: > > On Mon, Aug 09, 2021 at 07:01:57PM +0000, Dov Murik wrote: > > The new sev_secret module exposes the confidential computing (coco) > > secret area via securityfs interface. > > > > When the module is loaded (and securityfs is mounted, typically under > > /sys/kernel/security), a "coco/sev_secret" directory is created in > > securityfs. In it, a file is created for each secret entry. The name > > of each such file is the GUID of the secret entry, and its content is > > the secret data. > > > > This allows applications running in a confidential computing setting to > > read secrets provided by the guest owner via a secure secret injection > > mechanism (such as AMD SEV's LAUNCH_SECRET command). > > > > Removing (unlinking) files in the "coco/sev_secret" directory will zero > > out the secret in memory, and remove the filesystem entry. If the > > module is removed and loaded again, that secret will not appear in the > > filesystem. > > We've also been looking into a similar secret mechanism recently in the > context of Android and protected KVM [1]. Our secrets would come from a > different source, likely described as a reserved-memory node in the DT, > but would need to be exposed to userspace in the same way as the SEV > secrets. Originally I tried using a character device, but this approach > with securityfs feels neater to me. > Agreed. I particularly like how deleting the file wipes the secret from memory. > We're also looking to pass secrets from the bootloader to Linux, outside > of any virtualization or confidential compute context (at least a far as > I have understood the meaning of the term). Again, this feels like it > would be exposed to userspace in the same way. > Indeed. > It would be good to be able to share the parts that would be common. I > expect that would mean the operations for a secret file and for a > directory of secrets at a minimum. But it might also influence the paths > in securityfs; I see, looking back, that the "coco" directory was added > since the RFC but would a generalized "secret" subsystem make sense? Or > would it be preferable for each case to define their own path? > I think we should avoid 'secret', to be honest. Even if protected KVM is not riding the SEV/TDX wave, I think confidential computing is still an accurate description of its semantics. > [1] -- https://lwn.net/Articles/836693/ > > > +static int sev_secret_unlink(struct inode *dir, struct dentry *dentry) > > +{ > > + struct sev_secret *s = sev_secret_get(); > > + struct inode *inode = d_inode(dentry); > > + struct secret_entry *e = (struct secret_entry *)inode->i_private; > > + int i; > > + > > + if (e) { > > + /* Zero out the secret data */ > > + memzero_explicit(e->data, secret_entry_data_len(e)); > > Would there be a benefit in flushing these zeros? > Do you mean cache clean+invalidate? Better to be precise here. > > + e->guid = NULL_GUID; > > + } > > + > > + inode->i_private = NULL; > > + > > + for (i = 0; i < SEV_SECRET_NUM_FILES; i++) > > + if (s->fs_files[i] == dentry) > > + s->fs_files[i] = NULL; > > + > > + /* > > + * securityfs_remove tries to lock the directory's inode, but we reach > > + * the unlink callback when it's already locked > > + */ > > + inode_unlock(dir); > > + securityfs_remove(dentry); > > + inode_lock(dir); > > + > > + return 0; > > +}