Re: [PATCH RFC 3/4] firmware: Add a signature check
On Mon, 2012-11-05 at 18:20 +0100, Takashi Iwai wrote: > Add a feature to check the firmware signature, specified via Kconfig > CONFIG_FIRMWARE_SIG. The signature check is performed only for the > direct fw loading without udev. Also no check for built-in firmware > blobs is implemented yet. > > Signed-off-by: Takashi Iwai > --- > drivers/base/Kconfig | 6 + > drivers/base/firmware_class.c | 56 +++--- > include/linux/firmware.h | 7 + > kernel/module_signing.c | 63 > +++ > 4 files changed, 128 insertions(+), 4 deletions(-) > > diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig > index b34b5cd..3696fd7 100644 > --- a/drivers/base/Kconfig > +++ b/drivers/base/Kconfig > @@ -145,6 +145,12 @@ config EXTRA_FIRMWARE_DIR > this option you can point it elsewhere, such as /lib/firmware/ or > some other directory containing the firmware files. > > +config FIRMWARE_SIG > + bool "Firmware signature verification" > + depends on FW_LOADER && MODULE_SIG > + help > + Enable firmware signature check. > + > config DEBUG_DRIVER > bool "Driver Core verbose debug messages" > depends on DEBUG_KERNEL > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c > index 8945f4e..575bc4c 100644 > --- a/drivers/base/firmware_class.c > +++ b/drivers/base/firmware_class.c > @@ -36,6 +36,11 @@ MODULE_AUTHOR("Manuel Estrada Sainz"); > MODULE_DESCRIPTION("Multi purpose firmware loading support"); > MODULE_LICENSE("GPL"); > > +#ifdef CONFIG_FIRMWARE_SIG > +static bool sig_enforce; > +module_param(sig_enforce, bool, 0644); > +#endif > + > /* Builtin firmware support */ > > #ifdef CONFIG_FW_LOADER > @@ -287,7 +292,7 @@ static noinline long fw_file_size(struct file *file) > return st.size; > } > > -static bool fw_read_file_contents(struct file *file, struct firmware_buf > *fw_buf) > +static bool fw_read_file_contents(struct file *file, void **bufp, size_t > *sizep) > { > long size; > char *buf; > @@ -302,11 +307,39 @@ static bool fw_read_file_contents(struct file *file, > struct firmware_buf *fw_buf > vfree(buf); > return false; > } > - fw_buf->data = buf; > - fw_buf->size = size; > + *bufp = buf; > + *sizep = size; > return true; > } > > +#ifdef CONFIG_FIRMWARE_SIG > +static bool verify_signature(struct firmware_buf *buf, const char *path) > +{ > + const unsigned long markerlen = sizeof(FIRMWARE_SIG_STRING) - 1; > + struct file *file; > + void *sig_data; > + size_t sig_size; > + bool success; > + > + file = filp_open(path, O_RDONLY, 0); > + if (IS_ERR(file)) > + return false; > + > + success = fw_read_file_contents(file, _data, _size); > + fput(file); > + if (success) { > + if (sig_size > markerlen && > + !memcmp(sig_data, FIRMWARE_SIG_STRING, markerlen)) > + success = !fw_verify_sig(buf->data, buf->size, > + sig_data + markerlen, > + sig_size - markerlen); > + pr_debug("verified signature %s: %d\n", path, success); > + vfree(sig_data); > + } > + return success; > +} > +#endif /* CONFIG_FIRMWARE_SIG */ > + > static bool fw_get_filesystem_firmware(struct firmware_buf *buf) > { > int i; > @@ -320,8 +353,23 @@ static bool fw_get_filesystem_firmware(struct > firmware_buf *buf) > file = filp_open(path, O_RDONLY, 0); > if (IS_ERR(file)) > continue; > - success = fw_read_file_contents(file, buf); > + success = fw_read_file_contents(file, >data, >size); > fput(file); > +#ifdef CONFIG_FIRMWARE_SIG > + if (success) { > + snprintf(path, PATH_MAX, "%s/%s.sig", fw_path[i], > + buf->fw_id); > + if (!verify_signature(buf, path)) { > + pr_err("Invalid signature file %s\n", path); > + if (sig_enforce) { > + vfree(buf->data); > + buf->data = NULL; > + buf->size = 0; > + success = false; > + } > + } > + } > +#endif /* CONFIG_FIRMWARE_SIG */ The existing kernel modules are read by userspace into a buffer, which is passed to the kernel. As there is no way of verifying what was read by userspace is the same as what was passed to the kernel, the signature verification is done, in the kernel, on the buffer contents. The new kernel module syscall passes a file descriptor. With commit "41110a4 ima: support new kernel module syscall" the kernel module
[PATCH RFC 3/4] firmware: Add a signature check
Add a feature to check the firmware signature, specified via Kconfig CONFIG_FIRMWARE_SIG. The signature check is performed only for the direct fw loading without udev. Also no check for built-in firmware blobs is implemented yet. Signed-off-by: Takashi Iwai --- drivers/base/Kconfig | 6 + drivers/base/firmware_class.c | 56 +++--- include/linux/firmware.h | 7 + kernel/module_signing.c | 63 +++ 4 files changed, 128 insertions(+), 4 deletions(-) diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index b34b5cd..3696fd7 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -145,6 +145,12 @@ config EXTRA_FIRMWARE_DIR this option you can point it elsewhere, such as /lib/firmware/ or some other directory containing the firmware files. +config FIRMWARE_SIG + bool "Firmware signature verification" + depends on FW_LOADER && MODULE_SIG + help + Enable firmware signature check. + config DEBUG_DRIVER bool "Driver Core verbose debug messages" depends on DEBUG_KERNEL diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 8945f4e..575bc4c 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -36,6 +36,11 @@ MODULE_AUTHOR("Manuel Estrada Sainz"); MODULE_DESCRIPTION("Multi purpose firmware loading support"); MODULE_LICENSE("GPL"); +#ifdef CONFIG_FIRMWARE_SIG +static bool sig_enforce; +module_param(sig_enforce, bool, 0644); +#endif + /* Builtin firmware support */ #ifdef CONFIG_FW_LOADER @@ -287,7 +292,7 @@ static noinline long fw_file_size(struct file *file) return st.size; } -static bool fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf) +static bool fw_read_file_contents(struct file *file, void **bufp, size_t *sizep) { long size; char *buf; @@ -302,11 +307,39 @@ static bool fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf vfree(buf); return false; } - fw_buf->data = buf; - fw_buf->size = size; + *bufp = buf; + *sizep = size; return true; } +#ifdef CONFIG_FIRMWARE_SIG +static bool verify_signature(struct firmware_buf *buf, const char *path) +{ + const unsigned long markerlen = sizeof(FIRMWARE_SIG_STRING) - 1; + struct file *file; + void *sig_data; + size_t sig_size; + bool success; + + file = filp_open(path, O_RDONLY, 0); + if (IS_ERR(file)) + return false; + + success = fw_read_file_contents(file, _data, _size); + fput(file); + if (success) { + if (sig_size > markerlen && + !memcmp(sig_data, FIRMWARE_SIG_STRING, markerlen)) + success = !fw_verify_sig(buf->data, buf->size, +sig_data + markerlen, +sig_size - markerlen); + pr_debug("verified signature %s: %d\n", path, success); + vfree(sig_data); + } + return success; +} +#endif /* CONFIG_FIRMWARE_SIG */ + static bool fw_get_filesystem_firmware(struct firmware_buf *buf) { int i; @@ -320,8 +353,23 @@ static bool fw_get_filesystem_firmware(struct firmware_buf *buf) file = filp_open(path, O_RDONLY, 0); if (IS_ERR(file)) continue; - success = fw_read_file_contents(file, buf); + success = fw_read_file_contents(file, >data, >size); fput(file); +#ifdef CONFIG_FIRMWARE_SIG + if (success) { + snprintf(path, PATH_MAX, "%s/%s.sig", fw_path[i], +buf->fw_id); + if (!verify_signature(buf, path)) { + pr_err("Invalid signature file %s\n", path); + if (sig_enforce) { + vfree(buf->data); + buf->data = NULL; + buf->size = 0; + success = false; + } + } + } +#endif /* CONFIG_FIRMWARE_SIG */ if (success) break; } diff --git a/include/linux/firmware.h b/include/linux/firmware.h index e4279fe..2e9e457 100644 --- a/include/linux/firmware.h +++ b/include/linux/firmware.h @@ -79,4 +79,11 @@ static inline int uncache_firmware(const char *name) } #endif +#ifdef CONFIG_FIRMWARE_SIG +#define FIRMWARE_SIG_STRING "~Linux firmware signature~\n" +/* defined in kernel/module_signing.c */ +int fw_verify_sig(const void *fw_data, size_t fw_size, + const void *sig_data, size_t sig_size); +#endif + #endif diff --git
[PATCH RFC 3/4] firmware: Add a signature check
Add a feature to check the firmware signature, specified via Kconfig CONFIG_FIRMWARE_SIG. The signature check is performed only for the direct fw loading without udev. Also no check for built-in firmware blobs is implemented yet. Signed-off-by: Takashi Iwai ti...@suse.de --- drivers/base/Kconfig | 6 + drivers/base/firmware_class.c | 56 +++--- include/linux/firmware.h | 7 + kernel/module_signing.c | 63 +++ 4 files changed, 128 insertions(+), 4 deletions(-) diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index b34b5cd..3696fd7 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -145,6 +145,12 @@ config EXTRA_FIRMWARE_DIR this option you can point it elsewhere, such as /lib/firmware/ or some other directory containing the firmware files. +config FIRMWARE_SIG + bool Firmware signature verification + depends on FW_LOADER MODULE_SIG + help + Enable firmware signature check. + config DEBUG_DRIVER bool Driver Core verbose debug messages depends on DEBUG_KERNEL diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 8945f4e..575bc4c 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -36,6 +36,11 @@ MODULE_AUTHOR(Manuel Estrada Sainz); MODULE_DESCRIPTION(Multi purpose firmware loading support); MODULE_LICENSE(GPL); +#ifdef CONFIG_FIRMWARE_SIG +static bool sig_enforce; +module_param(sig_enforce, bool, 0644); +#endif + /* Builtin firmware support */ #ifdef CONFIG_FW_LOADER @@ -287,7 +292,7 @@ static noinline long fw_file_size(struct file *file) return st.size; } -static bool fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf) +static bool fw_read_file_contents(struct file *file, void **bufp, size_t *sizep) { long size; char *buf; @@ -302,11 +307,39 @@ static bool fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf vfree(buf); return false; } - fw_buf-data = buf; - fw_buf-size = size; + *bufp = buf; + *sizep = size; return true; } +#ifdef CONFIG_FIRMWARE_SIG +static bool verify_signature(struct firmware_buf *buf, const char *path) +{ + const unsigned long markerlen = sizeof(FIRMWARE_SIG_STRING) - 1; + struct file *file; + void *sig_data; + size_t sig_size; + bool success; + + file = filp_open(path, O_RDONLY, 0); + if (IS_ERR(file)) + return false; + + success = fw_read_file_contents(file, sig_data, sig_size); + fput(file); + if (success) { + if (sig_size markerlen + !memcmp(sig_data, FIRMWARE_SIG_STRING, markerlen)) + success = !fw_verify_sig(buf-data, buf-size, +sig_data + markerlen, +sig_size - markerlen); + pr_debug(verified signature %s: %d\n, path, success); + vfree(sig_data); + } + return success; +} +#endif /* CONFIG_FIRMWARE_SIG */ + static bool fw_get_filesystem_firmware(struct firmware_buf *buf) { int i; @@ -320,8 +353,23 @@ static bool fw_get_filesystem_firmware(struct firmware_buf *buf) file = filp_open(path, O_RDONLY, 0); if (IS_ERR(file)) continue; - success = fw_read_file_contents(file, buf); + success = fw_read_file_contents(file, buf-data, buf-size); fput(file); +#ifdef CONFIG_FIRMWARE_SIG + if (success) { + snprintf(path, PATH_MAX, %s/%s.sig, fw_path[i], +buf-fw_id); + if (!verify_signature(buf, path)) { + pr_err(Invalid signature file %s\n, path); + if (sig_enforce) { + vfree(buf-data); + buf-data = NULL; + buf-size = 0; + success = false; + } + } + } +#endif /* CONFIG_FIRMWARE_SIG */ if (success) break; } diff --git a/include/linux/firmware.h b/include/linux/firmware.h index e4279fe..2e9e457 100644 --- a/include/linux/firmware.h +++ b/include/linux/firmware.h @@ -79,4 +79,11 @@ static inline int uncache_firmware(const char *name) } #endif +#ifdef CONFIG_FIRMWARE_SIG +#define FIRMWARE_SIG_STRING ~Linux firmware signature~\n +/* defined in kernel/module_signing.c */ +int fw_verify_sig(const void *fw_data, size_t fw_size, + const void *sig_data, size_t sig_size); +#endif + #endif diff --git
Re: [PATCH RFC 3/4] firmware: Add a signature check
On Mon, 2012-11-05 at 18:20 +0100, Takashi Iwai wrote: Add a feature to check the firmware signature, specified via Kconfig CONFIG_FIRMWARE_SIG. The signature check is performed only for the direct fw loading without udev. Also no check for built-in firmware blobs is implemented yet. Signed-off-by: Takashi Iwai ti...@suse.de --- drivers/base/Kconfig | 6 + drivers/base/firmware_class.c | 56 +++--- include/linux/firmware.h | 7 + kernel/module_signing.c | 63 +++ 4 files changed, 128 insertions(+), 4 deletions(-) diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index b34b5cd..3696fd7 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -145,6 +145,12 @@ config EXTRA_FIRMWARE_DIR this option you can point it elsewhere, such as /lib/firmware/ or some other directory containing the firmware files. +config FIRMWARE_SIG + bool Firmware signature verification + depends on FW_LOADER MODULE_SIG + help + Enable firmware signature check. + config DEBUG_DRIVER bool Driver Core verbose debug messages depends on DEBUG_KERNEL diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 8945f4e..575bc4c 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -36,6 +36,11 @@ MODULE_AUTHOR(Manuel Estrada Sainz); MODULE_DESCRIPTION(Multi purpose firmware loading support); MODULE_LICENSE(GPL); +#ifdef CONFIG_FIRMWARE_SIG +static bool sig_enforce; +module_param(sig_enforce, bool, 0644); +#endif + /* Builtin firmware support */ #ifdef CONFIG_FW_LOADER @@ -287,7 +292,7 @@ static noinline long fw_file_size(struct file *file) return st.size; } -static bool fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf) +static bool fw_read_file_contents(struct file *file, void **bufp, size_t *sizep) { long size; char *buf; @@ -302,11 +307,39 @@ static bool fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf vfree(buf); return false; } - fw_buf-data = buf; - fw_buf-size = size; + *bufp = buf; + *sizep = size; return true; } +#ifdef CONFIG_FIRMWARE_SIG +static bool verify_signature(struct firmware_buf *buf, const char *path) +{ + const unsigned long markerlen = sizeof(FIRMWARE_SIG_STRING) - 1; + struct file *file; + void *sig_data; + size_t sig_size; + bool success; + + file = filp_open(path, O_RDONLY, 0); + if (IS_ERR(file)) + return false; + + success = fw_read_file_contents(file, sig_data, sig_size); + fput(file); + if (success) { + if (sig_size markerlen + !memcmp(sig_data, FIRMWARE_SIG_STRING, markerlen)) + success = !fw_verify_sig(buf-data, buf-size, + sig_data + markerlen, + sig_size - markerlen); + pr_debug(verified signature %s: %d\n, path, success); + vfree(sig_data); + } + return success; +} +#endif /* CONFIG_FIRMWARE_SIG */ + static bool fw_get_filesystem_firmware(struct firmware_buf *buf) { int i; @@ -320,8 +353,23 @@ static bool fw_get_filesystem_firmware(struct firmware_buf *buf) file = filp_open(path, O_RDONLY, 0); if (IS_ERR(file)) continue; - success = fw_read_file_contents(file, buf); + success = fw_read_file_contents(file, buf-data, buf-size); fput(file); +#ifdef CONFIG_FIRMWARE_SIG + if (success) { + snprintf(path, PATH_MAX, %s/%s.sig, fw_path[i], + buf-fw_id); + if (!verify_signature(buf, path)) { + pr_err(Invalid signature file %s\n, path); + if (sig_enforce) { + vfree(buf-data); + buf-data = NULL; + buf-size = 0; + success = false; + } + } + } +#endif /* CONFIG_FIRMWARE_SIG */ The existing kernel modules are read by userspace into a buffer, which is passed to the kernel. As there is no way of verifying what was read by userspace is the same as what was passed to the kernel, the signature verification is done, in the kernel, on the buffer contents. The new kernel module syscall passes a file descriptor. With commit 41110a4 ima: support new kernel module syscall the kernel module signature is measured/appraised like any other file in the IMA policy. Although the default policy should already be