On Thu, Aug 27, 2015 at 10:57:23AM -0000, David Woodhouse wrote: > > Luis R. Rodriguez <mcgrof@xxxxxxxx> wrote: > > > >> "PKCS#7: Add an optional authenticated attribute to hold firmware name" > >> https://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/commit/?h=fwsign-pkcs7&id=1448377a369993f864915743cfb34772e730213good > >> > >> 1.3.6.1.4.1.2312.16 Linux kernel > >> 1.3.6.1.4.1.2312.16.2 - PKCS#7/CMS SignerInfo attribute types > >> 1.3.6.1.4.1.2312.16.2.1 - firmwareName > >> > >> I take it you are referring to this? > > > > Yes. > > > >> If we follow this model we'd then need something like: > >> > >> 1.3.6.1.4.1.2312.16.2.2 - seLinuxPolicyName > >> > >> That should mean each OID that has different file names would need to be > >> explicit about and have a similar entry on the registry. I find that > >> pretty redundant and would like to avoid that if possible. > > > > firmwareName is easy for people to understand - it's the name the kernel > > asks for and the filename of the blob. seLinuxPolicyName is, I think, a > > lot more tricky since a lot of people don't use SELinux, and most that do > > don't understand it (most people that use it aren't even really aware of > > it). > > > > If you can use the firmwareName as the SELinux/LSM key, I would suggest > > doing so - even if you dress it up as a path > > (/lib/firmware/<firmwareName>). > > In conversation with Mimi last week she was very keen on the model where > we load modules & firmware in such a fashion that the kernel has access to > the original inode -- by passing in a fd, Sure, so let's be specific to ensure what Mimi needs is there. I though there was work needed on modules but that seems covered and work then seems only needed for kexec and SELinux policy files (and a review of other possible file consumers in the kernel) for what you describe. I also went ahead and studied areas where we can share code now as I was looking at this code now, and also would like to recap on the idea of possibly just sharing the same LSM hook for all "read this special file from the fs in the kernel" cases. Details below. Fortnately the LSM hooks uses struct file and with this you can get the inode with this: struct inode *inode = file_inode(file); For modules we have this LSM hook: int (*kernel_module_from_file)(struct file *file); This can be used for finit_module(). Its used as follows, the fd comes from finit_module() syscall. SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags) { ... err = copy_module_from_fd(fd, &info); if (err) return err; ... } static int copy_module_from_fd(int fd, struct load_info *info) { struct fd f = fdget(fd); ... err = security_kernel_module_from_file(f.file); if (err) goto out; } For firmware we have this LSM hook: int (*kernel_fw_from_file)(struct file *file, char *buf, size_t size); > or in the firmware case by doing the fs lookup directly. Right so now that firmware usermode helper is behind us (systemd ripped it) we do the fs lookup directly ourselves. One of my side goals with the extensible firmware API was also to allow for us to take a leap and let drivers skip completely the usermode helper so we can then phase that code to the only required remainign user: the dell-rbu driver. Anyway, once we have the path built up we use it as follows. static int fw_read_file(const char *path, void **_buf, size_t *_size) { struct file *file; ... file = filp_open(path, O_RDONLY, 0); if (IS_ERR(file)) return PTR_ERR(file); ... rc = security_kernel_fw_from_file(file, buf, size); if (rc) goto err_buf; ... } I was under the impression that work was needed to add an LSM hook which would grant the LSM access to the file specific data for modules but that's already there with finit_module()! So Mimi needs is already there for modules as well now. We have no LSM hook for kexec, even though the kernel does have access to the fd, so if you wanted the struct file for an LSM it should be possible as the syscall for kexec is: SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd, unsigned long, cmdline_len, const char __user *, cmdline_ptr, unsigned long, flags) { ... } I noted earlier however that kexec is currently an x86 thing only though, and Howells clarified that this is because we want kernel image signing as an option (its a Kconfig option), and only PE supports a built-in signing method. Its unclear to me who extends ELF and if its worthwhile to consider adding support there for a signing method. Howells noted that there are rumours other archs would support kexec, its unclear what they would use. Even though kexec remains x86 specific an LSM for it should easily be possible to add, but more on this below... > So surely you have all the SELinux labelling you need? SELinux uses: security_load_policy(data, len), refer to selinuxfs sel_load_ops. Since its write operation on its file_operation is sel_write_load() and that is as follows: static ssize_t sel_write_load(struct file *file, const char __user *buf, size_t count, loff_t *ppos) { ... } We should be able to add yet-another LSM hook here to let the kernel / LSM have access to the inode, is that LSM hook desirable ? But folks, before you answer note that there's a growing trend here! Its point 1 Kees had made earlier. I was hesitant to go into details as I think fw signing needs to be baked first but.. since we're reviewing all these details now it seems logical to go down the rabbit hole further. Everywhere where we fetch a file from within the kernel either directly (say firmware load, 802.11 regulatory request) or from userspace request (SELinux policy load node) we end up having to sprinkle a new LSM hook. In fact for modules and kexec there were syscalls added too. There might be a possiblity for sharing some of these requests / code so some review is in order for it. Here's my review if we wanted to try sharing things, in consideration and review of: * SELinux policy files * modules * firmware / system data (consider replacing CRDA) * kexec ---- * SELinux policy files: sel_write_load() is very specific, its part of the selinuxfs and it just uses copy_from_user() to dump the data from the file onto a vmalloc'd piece of memory. We don't exactly read arbitrary files from the fs then. If we *really* wanted to generalize things further we probably could but I'm not going to lead any discussion about design over selinuxfs, I'll let the folks behind it think about that themselves. * modules * firmware / system data modules + firmware: there seems to be some code sharing we could possibly do for both fw_read_file() and copy_module_from_fd(), note we are going to use different keys for vetting each of these. It may be possible to share the LSM hook here. All parties would just need to agree. * kexec kexec works by reading files and setting up pointers for the different segments it needs for bootup, it does this for both the kernel and initrd if present. It however uses its own copy_file_from_fd() routine and no surprise here, there's code that can be shared as well. We'd be using a separate signature for kexec, so that'd be vetted on its own already. It may be possible to share the same LSM hook here, again all parties would just need to agree. ---- So conclusion: After fw signing gets baked (or I'll do that as I work with the system data helpers) there is possible work here to consolidate firmware's fw_read_file(), module's fw_read_file(), and kexec's copy_file_from_fd() into a core kernel tiny helper that gets it done right for all. If we really wanted to we could also just use the same LSM hook for all, this hook would surely have the struct file as Mimi wants as well. Unless I misunderstood things, at the Linux security summit it seemed folks thought this was reasonable and desirable. One of the gains then would be that the kernel can grow for different use cases and files can be fetched as needed but we wouldn't have to add yet-another-LSM hook for each new purpose, we'd just be sharing the same fetch / LSM hook. Please discuss and let me know if this still stands, I'll work towards any agreed upon direction with the fw signing code. And again, there may other parts of the kernel that do similar work, just as we found out about SELinux policy files. Those need to be identified and studied separatley. I guess we can use grammar to hunt these down. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
