From 847d64d959067ea767b497bd1e01ac3f10baefee Mon Sep 17 00:00:00 2001 From: Sergio Correia Date: Mon, 20 Apr 2026 12:09:57 +0100 Subject: [PATCH 1/3] fix: correct off-by-one in token_to_jwe() snprintf() returns the number of characters written excluding the null terminator. The pkt->used-- was intended to "remove the null terminator" from the count, but since snprintf already excludes it, this incorrectly truncated the last byte of actual JWE data. This caused LUKS2 auto-unlock via udisks2 to always fail with the TPM2 pin: the Rust clevis-pin-tpm2 strictly validates base64url encoding and rejects the truncated authentication tag. The Tang pin appeared unaffected because the jose C library is lenient about truncated base64url input. Also fix the boundary check to use >= instead of >, which would have accepted a JWE that was silently truncated by snprintf when the output exactly filled the buffer. Assisted-by: Claude Opus 4.6 Signed-off-by: Sergio Correia --- src/luks/udisks2/clevis-luks-udisks2.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/luks/udisks2/clevis-luks-udisks2.c b/src/luks/udisks2/clevis-luks-udisks2.c index 8b48705b..2a537add 100644 --- a/src/luks/udisks2/clevis-luks-udisks2.c +++ b/src/luks/udisks2/clevis-luks-udisks2.c @@ -481,10 +481,9 @@ token_to_jwe(const char *json, pkt_t *pkt) pkt->used = snprintf(pkt->data, sizeof(pkt->data), "%s.%s.%s.%s.%s", prt, key, iv, ct, tag); - if (pkt->used < 0 || (size_t) pkt->used > sizeof(pkt->data)) + if (pkt->used < 0 || (size_t) pkt->used >= sizeof(pkt->data)) return false; - pkt->used--; /* Remove null terminator. */ return true; } From aa059909ac7c1b7ac496c656d22ad881698812da Mon Sep 17 00:00:00 2001 From: Sergio Correia Date: Mon, 20 Apr 2026 12:14:45 +0100 Subject: [PATCH 2/3] test: add unit test for token_to_jwe() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract token_to_jwe() and pkt_t into a separate compilation unit (token-to-jwe.c/h) so they can be tested independently without pulling in udisks2, luksmeta, or audit dependencies. The test verifies the critical invariant that pkt->used equals strlen(pkt->data) after conversion — the exact condition violated by the off-by-one bug fixed in the previous commit. Assisted-by: Claude Opus 4.6 Signed-off-by: Sergio Correia --- src/luks/udisks2/clevis-luks-udisks2.c | 42 +------- src/luks/udisks2/meson.build | 11 +- src/luks/udisks2/test-token-to-jwe.c | 133 +++++++++++++++++++++++++ src/luks/udisks2/token-to-jwe.c | 59 +++++++++++ src/luks/udisks2/token-to-jwe.h | 35 +++++++ 5 files changed, 239 insertions(+), 41 deletions(-) create mode 100644 src/luks/udisks2/test-token-to-jwe.c create mode 100644 src/luks/udisks2/token-to-jwe.c create mode 100644 src/luks/udisks2/token-to-jwe.h diff --git a/src/luks/udisks2/clevis-luks-udisks2.c b/src/luks/udisks2/clevis-luks-udisks2.c index 2a537add..d0b0199e 100644 --- a/src/luks/udisks2/clevis-luks-udisks2.c +++ b/src/luks/udisks2/clevis-luks-udisks2.c @@ -17,18 +17,18 @@ * along with this program. If not, see . */ +#include "token-to-jwe.h" + #include #include #include #include -#include #include #include #include -#include #include #include @@ -37,7 +37,6 @@ #include #include -#define MAX_UDP 65507 #define UERR ((uid_t) -1) #define GERR ((gid_t) -1) @@ -50,11 +49,6 @@ u[0x0], u[0x1], u[0x2], u[0x3], u[0x4], u[0x5], u[0x6], u[0x7], \ u[0x8], u[0x9], u[0xa], u[0xb], u[0xc], u[0xd], u[0xe], u[0xf] -typedef struct { - ssize_t used; - char data[MAX_UDP]; -} pkt_t; - enum { PIPE_RD = 0, PIPE_WR = 1 @@ -455,38 +449,6 @@ grp2gid(const char *grp) return tmp ? tmp->gr_gid : GERR; } -static bool -token_to_jwe(const char *json, pkt_t *pkt) -{ - json_auto_t *tokn = NULL; - const json_t *jwe = NULL; - const char *prt = NULL; - const char *key = NULL; - const char *tag = NULL; - const char *iv = NULL; - const char *ct = NULL; - - tokn = json_loads(json, 0, NULL); - if (!tokn) - return false; - - jwe = json_object_get(tokn, "jwe"); - if (!jwe) - return false; - - if (json_unpack((json_t *) jwe, "{s:s,s:s,s:s,s:s,s:s}", - "protected", &prt, "encrypted_key", &key, "iv", &iv, - "ciphertext", &ct, "tag", &tag) < 0) - return false; - - pkt->used = snprintf(pkt->data, sizeof(pkt->data), - "%s.%s.%s.%s.%s", prt, key, iv, ct, tag); - if (pkt->used < 0 || (size_t) pkt->used >= sizeof(pkt->data)) - return false; - - return true; -} - int main(int argc, char *const argv[]) { diff --git a/src/luks/udisks2/meson.build b/src/luks/udisks2/meson.build index e747d4ba..866055b7 100644 --- a/src/luks/udisks2/meson.build +++ b/src/luks/udisks2/meson.build @@ -12,7 +12,8 @@ if udisks2.found() and audit.found() and gio.found() configuration: data, ) - executable('clevis-luks-udisks2', 'clevis-luks-udisks2.c', + executable('clevis-luks-udisks2', + ['clevis-luks-udisks2.c', 'token-to-jwe.c'], dependencies: [udisks2, luksmeta, audit, jansson], install_dir: libexecdir, install: true, @@ -20,3 +21,11 @@ if udisks2.found() and audit.found() and gio.found() else warning('Will not build udisks2 support due to missing dependencies!') endif + +if jansson.found() + test_token_to_jwe = executable('test-token-to-jwe', + ['test-token-to-jwe.c', 'token-to-jwe.c'], + dependencies: [jansson], + ) + test('token-to-jwe', test_token_to_jwe) +endif diff --git a/src/luks/udisks2/test-token-to-jwe.c b/src/luks/udisks2/test-token-to-jwe.c new file mode 100644 index 00000000..9b7b6944 --- /dev/null +++ b/src/luks/udisks2/test-token-to-jwe.c @@ -0,0 +1,133 @@ +/* vim: set tabstop=8 shiftwidth=4 softtabstop=4 expandtab smarttab colorcolumn=80: */ +/* + * Copyright (c) 2026 Red Hat, Inc. + * Author: Sergio Correia + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "token-to-jwe.h" + +#include +#include +#include +#include + +static const char valid_token[] = + "{" + " \"type\": \"clevis\"," + " \"keyslots\": [\"1\"]," + " \"jwe\": {" + " \"protected\": \"eyJhbGciOiJkaXIiLCJlbmMiOiJBMjU2R0NNIn0\"," + " \"encrypted_key\": \"\"," + " \"iv\": \"oB2uB6_a2LCQnhNk\"," + " \"ciphertext\": \"Gss774jh5EcnMA5NacAxuX8\"," + " \"tag\": \"6L9KBrn6-R1---wTikJTrA\"" + " }" + "}"; + +static void +test_basic_conversion(void) +{ + pkt_t pkt = {}; + const char *expected = + "eyJhbGciOiJkaXIiLCJlbmMiOiJBMjU2R0NNIn0" + "." + "." + "oB2uB6_a2LCQnhNk" + "." + "Gss774jh5EcnMA5NacAxuX8" + "." + "6L9KBrn6-R1---wTikJTrA"; + + assert(token_to_jwe(valid_token, &pkt)); + assert(strcmp(pkt.data, expected) == 0); + fprintf(stderr, "test_basic_conversion: PASS\n"); +} + +static void +test_used_equals_strlen(void) +{ + pkt_t pkt = {}; + + assert(token_to_jwe(valid_token, &pkt)); + assert(pkt.used == (ssize_t) strlen(pkt.data)); + fprintf(stderr, "test_used_equals_strlen: PASS\n"); +} + +static void +test_invalid_json(void) +{ + pkt_t pkt = {}; + + assert(!token_to_jwe(NULL, &pkt)); + assert(!token_to_jwe("not json", &pkt)); + assert(!token_to_jwe("{}", &pkt)); + assert(!token_to_jwe("{\"jwe\":{}}", &pkt)); + assert(!token_to_jwe("{\"jwe\":{\"protected\":\"a\"}}", &pkt)); + fprintf(stderr, "test_invalid_json: PASS\n"); +} + +static void +test_empty_components(void) +{ + const char *json = + "{\"jwe\":{" + "\"protected\":\"\"," + "\"encrypted_key\":\"\"," + "\"iv\":\"\"," + "\"ciphertext\":\"\"," + "\"tag\":\"\"" + "}}"; + pkt_t pkt = {}; + + assert(token_to_jwe(json, &pkt)); + assert(strcmp(pkt.data, "....") == 0); + assert(pkt.used == 4); + assert(pkt.used == (ssize_t) strlen(pkt.data)); + fprintf(stderr, "test_empty_components: PASS\n"); +} + +static void +test_single_char_components(void) +{ + const char *json = + "{\"jwe\":{" + "\"protected\":\"a\"," + "\"encrypted_key\":\"b\"," + "\"iv\":\"c\"," + "\"ciphertext\":\"d\"," + "\"tag\":\"e\"" + "}}"; + pkt_t pkt = {}; + + assert(token_to_jwe(json, &pkt)); + assert(strcmp(pkt.data, "a.b.c.d.e") == 0); + assert(pkt.used == 9); + assert(pkt.used == (ssize_t) strlen(pkt.data)); + fprintf(stderr, "test_single_char_components: PASS\n"); +} + +int +main(void) +{ + test_basic_conversion(); + test_used_equals_strlen(); + test_invalid_json(); + test_empty_components(); + test_single_char_components(); + + fprintf(stderr, "All tests passed.\n"); + return EXIT_SUCCESS; +} diff --git a/src/luks/udisks2/token-to-jwe.c b/src/luks/udisks2/token-to-jwe.c new file mode 100644 index 00000000..ab70e980 --- /dev/null +++ b/src/luks/udisks2/token-to-jwe.c @@ -0,0 +1,59 @@ +/* vim: set tabstop=8 shiftwidth=4 softtabstop=4 expandtab smarttab colorcolumn=80: */ +/* + * Copyright (c) 2015 Red Hat, Inc. + * Author: Nathaniel McCallum + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "token-to-jwe.h" + +#include +#include +#include + +bool +token_to_jwe(const char *json, pkt_t *pkt) +{ + json_auto_t *tokn = NULL; + const json_t *jwe = NULL; + const char *prt = NULL; + const char *key = NULL; + const char *tag = NULL; + const char *iv = NULL; + const char *ct = NULL; + + if (!json) + return false; + + tokn = json_loads(json, 0, NULL); + if (!tokn) + return false; + + jwe = json_object_get(tokn, "jwe"); + if (!jwe) + return false; + + if (json_unpack((json_t *) jwe, "{s:s,s:s,s:s,s:s,s:s}", + "protected", &prt, "encrypted_key", &key, "iv", &iv, + "ciphertext", &ct, "tag", &tag) < 0) + return false; + + pkt->used = snprintf(pkt->data, sizeof(pkt->data), + "%s.%s.%s.%s.%s", prt, key, iv, ct, tag); + if (pkt->used < 0 || (size_t) pkt->used >= sizeof(pkt->data)) + return false; + + return true; +} diff --git a/src/luks/udisks2/token-to-jwe.h b/src/luks/udisks2/token-to-jwe.h new file mode 100644 index 00000000..3082ff7e --- /dev/null +++ b/src/luks/udisks2/token-to-jwe.h @@ -0,0 +1,35 @@ +/* vim: set tabstop=8 shiftwidth=4 softtabstop=4 expandtab smarttab colorcolumn=80: */ +/* + * Copyright (c) 2015 Red Hat, Inc. + * Author: Nathaniel McCallum + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef TOKEN_TO_JWE_H +#define TOKEN_TO_JWE_H + +#include +#include + +#define MAX_UDP 65507 + +typedef struct { + ssize_t used; + char data[MAX_UDP]; +} pkt_t; + +bool token_to_jwe(const char *json, pkt_t *pkt); + +#endif /* TOKEN_TO_JWE_H */ From 06a992f548d93c598e7e67f8f84be5127485cac3 Mon Sep 17 00:00:00 2001 From: Sergio Correia Date: Mon, 20 Apr 2026 12:23:11 +0100 Subject: [PATCH 3/3] fix: correct snprintf truncation check in log_attempt() The check used `r == sizeof(msg)` which only catches exact-fit truncation. When snprintf would have written more than sizeof(msg) characters, it returns a value greater than sizeof(msg), and the old check would miss it. Use `>= sizeof(msg)` to catch all truncation cases, matching the fix applied to token_to_jwe(). Assisted-by: Claude Opus 4.6 Signed-off-by: Sergio Correia --- src/luks/udisks2/clevis-luks-udisks2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/luks/udisks2/clevis-luks-udisks2.c b/src/luks/udisks2/clevis-luks-udisks2.c index d0b0199e..cd969ea0 100644 --- a/src/luks/udisks2/clevis-luks-udisks2.c +++ b/src/luks/udisks2/clevis-luks-udisks2.c @@ -420,7 +420,7 @@ log_attempt(int log, struct crypt_device *cd, bool success) "op=recovered-key-for uuid=%s %s", uuid, dev); free(dev); - if (r < 0 || r == sizeof(msg)) + if (r < 0 || (size_t) r >= sizeof(msg)) return false; return audit_log_user_message(log, AUDIT_USER_DEVICE, msg,