From fa96afb4c4ad1d5e82839dddb686398601784d53 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 26 Aug 2023 14:03:14 +0200 Subject: [PATCH] sd-id128: do not allow null 'app_id' param If it is null, we get the 'base' param unchanged: $ build/systemd-id128 show 00000000000000000000000000000001 \ --app-specific=00000000000000000000000000000000 00000000000000000000000000000001 This is not good, because it breaks our promise that the base (usually either machine-id or boot-id) cannot be derived from the result. Some application using the library could use a null app id, inadvertently exposing the machine or boot id. (This could happen because of forgotten initialization, or maybe because the app id is configurable, and the user configures it wrongly.) Note: the other way the secret is not exposed: $ build/systemd-id128 show 00000000000000000000000000000000 \ --app-specific=00000000000000000000000000000002 4f63080959264900b0d88d999dae2d3a Normally systemd would not allow a null machine-id or boot-id, but we can let the user do the calculation that if they want to. --- man/sd_id128_get_machine.xml | 5 ++++- src/id128/id128.c | 4 +++- src/libsystemd/sd-id128/sd-id128.c | 1 + src/test/test-id128.c | 4 ++++ 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/man/sd_id128_get_machine.xml b/man/sd_id128_get_machine.xml index fc13ae50f4..31e4b02d9b 100644 --- a/man/sd_id128_get_machine.xml +++ b/man/sd_id128_get_machine.xml @@ -198,7 +198,10 @@ -ENXIO Returned by sd_id128_get_invocation() if no invocation ID is - set. + set. Also returned by sd_id128_get_app_specific(), + sd_id128_get_machine_app_specific(), and + sd_id128_get_boot_app_specific() when the app_id + parameter is all zeros. diff --git a/src/id128/id128.c b/src/id128/id128.c index 29cd2e2dc0..18e0880805 100644 --- a/src/id128/id128.c +++ b/src/id128/id128.c @@ -235,7 +235,9 @@ static int parse_argv(int argc, char *argv[]) { break; case 'a': - r = sd_id128_from_string(optarg, &arg_app); + r = id128_from_string_nonzero(optarg, &arg_app); + if (r == -ENXIO) + return log_error_errno(r, "Application ID cannot be all zeros."); if (r < 0) return log_error_errno(r, "Failed to parse \"%s\" as application-ID: %m", optarg); break; diff --git a/src/libsystemd/sd-id128/sd-id128.c b/src/libsystemd/sd-id128/sd-id128.c index adc73e4fdc..9fda79ae26 100644 --- a/src/libsystemd/sd-id128/sd-id128.c +++ b/src/libsystemd/sd-id128/sd-id128.c @@ -346,6 +346,7 @@ _public_ int sd_id128_get_app_specific(sd_id128_t base, sd_id128_t app_id, sd_id } buf; assert_return(ret, -EINVAL); + assert_return(!sd_id128_is_null(app_id), -ENXIO); hmac_sha256(&base, sizeof(base), &app_id, sizeof(app_id), buf.hmac); diff --git a/src/test/test-id128.c b/src/test/test-id128.c index 10c061b917..ae7df27d8f 100644 --- a/src/test/test-id128.c +++ b/src/test/test-id128.c @@ -192,6 +192,10 @@ TEST(id128) { assert_se(sd_id128_get_machine_app_specific(SD_ID128_MAKE(51,df,0b,4b,c3,b0,4c,97,80,e2,99,b9,8c,a3,73,b8), &id2) >= 0); assert_se(!sd_id128_equal(id, id2)); } + + /* Check return values */ + assert_se(sd_id128_get_app_specific(SD_ID128_ALLF, SD_ID128_NULL, &id) == -ENXIO); + assert_se(sd_id128_get_app_specific(SD_ID128_NULL, SD_ID128_ALLF, &id) == 0); } TEST(sd_id128_get_invocation) { -- 2.25.1