2022-09-07 01:47:42

by Matthias Kaehlcke

Subject: [PATCH] LoadPin: Require file with verity root digests to have a header

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 <[email protected]>
Signed-off-by: Matthias Kaehlcke <[email protected]>
---
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(-)

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;
--
2.37.2.789.g6183377224-goog

2022-09-07 23:46:31

by Kees Cook

Subject: Re: [PATCH] LoadPin: Require file with verity root digests to have a header

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

--
Kees Cook