diff mbox series

LoadPin: Require file with verity root digests to have a header

Message ID 20220906181725.1.I3f51d1bb0014e5a5951be4ad3c5ad7c7ca1dfc32@changeid (mailing list archive)
State Handled Elsewhere
Headers show
Series LoadPin: Require file with verity root digests to have a header | expand

Commit Message

Matthias Kaehlcke Sept. 7, 2022, 1:18 a.m. UTC
LoadPin expects the file with trusted verity root digests to be
an ASCII file with one digest (hex value) per line. A pinned
root could contain files that meet these format requirements,
even though the hex values don't represent trusted root
digests.

Add a new requirement to the file format which consists in
the first line containing a fixed string. This prevents
attackers from feeding files with an otherwise valid format
to LoadPin.

Suggested-by: Sarthak Kukreti <sarthakkukreti@chromium.org>
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
It could be argued that this change breaks existing users
of the LoadPin verity feature. The risk of this actually
happening seems very low given that the feature only
landed in v6.0, which hasn't been released yet.

 security/loadpin/Kconfig   |  7 ++++++-
 security/loadpin/loadpin.c | 16 +++++++++++++++-
 2 files changed, 21 insertions(+), 2 deletions(-)

Comments

Kees Cook Sept. 7, 2022, 11:21 p.m. UTC | #1
On Tue, 6 Sep 2022 18:18:12 -0700, Matthias Kaehlcke wrote:
> LoadPin expects the file with trusted verity root digests to be
> an ASCII file with one digest (hex value) per line. A pinned
> root could contain files that meet these format requirements,
> even though the hex values don't represent trusted root
> digests.
> 
> Add a new requirement to the file format which consists in
> the first line containing a fixed string. This prevents
> attackers from feeding files with an otherwise valid format
> to LoadPin.
> 
> [...]

Applied to for-next/hardening, thanks!

[1/1] LoadPin: Require file with verity root digests to have a header
      https://git.kernel.org/kees/c/d64f3001976e
diff mbox series

Patch

diff --git a/security/loadpin/Kconfig b/security/loadpin/Kconfig
index 994c1d9376e6..6724eaba3d36 100644
--- a/security/loadpin/Kconfig
+++ b/security/loadpin/Kconfig
@@ -33,4 +33,9 @@  config SECURITY_LOADPIN_VERITY
 	  on the LoadPin securityfs entry 'dm-verity'. The ioctl
 	  expects a file descriptor of a file with verity digests as
 	  parameter. The file must be located on the pinned root and
-	  contain one digest per line.
+	  start with the line:
+
+	  # LOADPIN_TRUSTED_VERITY_ROOT_DIGESTS
+
+	  This is followed by the verity digests, with one digest per
+	  line.
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index 44521582dcba..de41621f4998 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -21,6 +21,8 @@ 
 #include <linux/dm-verity-loadpin.h>
 #include <uapi/linux/loadpin.h>
 
+#define VERITY_DIGEST_FILE_HEADER "# LOADPIN_TRUSTED_VERITY_ROOT_DIGESTS"
+
 static void report_load(const char *origin, struct file *file, char *operation)
 {
 	char *cmdline, *pathname;
@@ -292,9 +294,21 @@  static int read_trusted_verity_root_digests(unsigned int fd)
 
 	p = strim(data);
 	while ((d = strsep(&p, "\n")) != NULL) {
-		int len = strlen(d);
+		int len;
 		struct dm_verity_loadpin_trusted_root_digest *trd;
 
+		if (d == data) {
+			/* first line, validate header */
+			if (strcmp(d, VERITY_DIGEST_FILE_HEADER)) {
+				rc = -EPROTO;
+				goto err;
+			}
+
+			continue;
+		}
+
+		len = strlen(d);
+
 		if (len % 2) {
 			rc = -EPROTO;
 			goto err;