From 8157cc0e3e33c97b406cc088cf001ca524154f64 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 1 Mar 2024 14:47:52 +0100 Subject: [PATCH 1/6] bus-util: add generic parser for extracting id128 values from bus messages --- src/core/dbus-manager.c | 11 +---------- src/machine/machinectl.c | 9 ++------- src/run/run.c | 15 ++------------- src/shared/bus-map-properties.c | 11 +---------- src/shared/bus-util.c | 29 +++++++++++++++++++++++++++++ src/shared/bus-util.h | 2 ++ 6 files changed, 37 insertions(+), 40 deletions(-) diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 31b2f1daef4..00fd801cb3f 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -570,23 +570,14 @@ static int method_get_unit_by_invocation_id(sd_bus_message *message, void *userd _cleanup_free_ char *path = NULL; Manager *m = ASSERT_PTR(userdata); sd_id128_t id; - const void *a; Unit *u; - size_t sz; int r; assert(message); /* Anyone can call this method */ - r = sd_bus_message_read_array(message, 'y', &a, &sz); - if (r < 0) - return r; - if (sz == 0) - id = SD_ID128_NULL; - else if (sz == 16) - memcpy(&id, a, sz); - else + if (bus_message_read_id128(message, &id) < 0) return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid invocation ID"); if (sd_id128_is_null(id)) { diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index a7d04f2f4de..b93c73f3cd5 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -767,22 +767,17 @@ static int print_image_hostname(sd_bus *bus, const char *name) { static int print_image_machine_id(sd_bus *bus, const char *name) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - sd_id128_t id = SD_ID128_NULL; - const void *p; - size_t size; + sd_id128_t id; int r; r = bus_call_method(bus, bus_machine_mgr, "GetImageMachineID", NULL, &reply, "s", name); if (r < 0) return r; - r = sd_bus_message_read_array(reply, 'y', &p, &size); + r = bus_message_read_id128(reply, &id); if (r < 0) return r; - if (size == sizeof(sd_id128_t)) - memcpy(&id, p, size); - if (!sd_id128_is_null(id)) printf(" Machine ID: " SD_ID128_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(id)); diff --git a/src/run/run.c b/src/run/run.c index 5181c18c201..c0c65b2ea1f 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -1552,8 +1552,6 @@ static int acquire_invocation_id(sd_bus *bus, const char *unit, sd_id128_t *ret) _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_free_ char *object = NULL; - const void *p; - size_t l; int r; assert(bus); @@ -1576,20 +1574,11 @@ static int acquire_invocation_id(sd_bus *bus, const char *unit, sd_id128_t *ret) if (r < 0) return log_error_errno(r, "Failed to request invocation ID for unit: %s", bus_error_message(&error, r)); - r = sd_bus_message_read_array(reply, 'y', &p, &l); + r = bus_message_read_id128(reply, ret); if (r < 0) return bus_log_parse_error(r); - if (l == 0) { - *ret = SD_ID128_NULL; - return 0; /* no uuid set */ - } - - if (l != sizeof(sd_id128_t)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid UUID size, %zu != %zu.", l, sizeof(sd_id128_t)); - - memcpy(ret, p, l); - return !sd_id128_is_null(*ret); + return 0; } static void set_window_title(PTYForward *f) { diff --git a/src/shared/bus-map-properties.c b/src/shared/bus-map-properties.c index 809759db803..a4833a5c1ee 100644 --- a/src/shared/bus-map-properties.c +++ b/src/shared/bus-map-properties.c @@ -8,21 +8,12 @@ int bus_map_id128(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) { sd_id128_t *p = userdata; - const void *v; - size_t n; int r; - r = sd_bus_message_read_array(m, SD_BUS_TYPE_BYTE, &v, &n); + r = bus_message_read_id128(m, p); if (r < 0) return bus_log_parse_error_debug(r); - if (n == 0) - *p = SD_ID128_NULL; - else if (n == 16) - memcpy((*p).bytes, v, n); - else - return -EINVAL; - return 0; } diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index eea6c2321af..88e1249b8ed 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -754,3 +754,32 @@ int bus_query_sender_pidref( return bus_creds_get_pidref(creds, ret); } + +int bus_message_read_id128(sd_bus_message *m, sd_id128_t *ret) { + const void *a; + size_t sz; + int r; + + assert(m); + + r = sd_bus_message_read_array(m, 'y', &a, &sz); + if (r < 0) + return r; + + switch (sz) { + case 0: + if (ret) + *ret = SD_ID128_NULL; + break; + + case sizeof(sd_id128_t): + if (ret) + memcpy(ret, a, sz); + break; + + default: + return -EINVAL; + } + + return 0; +} diff --git a/src/shared/bus-util.h b/src/shared/bus-util.h index 1004c794069..d55665f502a 100644 --- a/src/shared/bus-util.h +++ b/src/shared/bus-util.h @@ -77,3 +77,5 @@ int bus_property_get_string_set(sd_bus *bus, const char *path, const char *inter int bus_creds_get_pidref(sd_bus_creds *c, PidRef *ret); int bus_query_sender_pidref(sd_bus_message *m, PidRef *ret); + +int bus_message_read_id128(sd_bus_message *m, sd_id128_t *ret); From f3525b78af4bf6972e01a0a8250ebb85452686d6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 1 Mar 2024 14:44:27 +0100 Subject: [PATCH 2/6] hostnamed: our base indentation is 8 spaces, not 9 spaces No idea what was going on here... --- src/hostname/hostnamed.c | 66 ++++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index 4ab429798aa..ee2df8f2ff1 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -286,71 +286,71 @@ static int get_hardware_serial(char **ret) { } static int get_firmware_version(char **ret) { - return get_hardware_firmware_data("bios_version", ret); + return get_hardware_firmware_data("bios_version", ret); } static int get_firmware_vendor(char **ret) { - return get_hardware_firmware_data("bios_vendor", ret); + return get_hardware_firmware_data("bios_vendor", ret); } static int get_firmware_date(usec_t *ret) { - _cleanup_free_ char *bios_date = NULL, *month = NULL, *day = NULL, *year = NULL; - int r; + _cleanup_free_ char *bios_date = NULL, *month = NULL, *day = NULL, *year = NULL; + int r; - assert(ret); + assert(ret); - r = get_hardware_firmware_data("bios_date", &bios_date); - if (r < 0) + r = get_hardware_firmware_data("bios_date", &bios_date); + if (r < 0) return r; - if (r == 0) { + if (r == 0) { *ret = USEC_INFINITY; return 0; - } + } - const char *p = bios_date; - r = extract_many_words(&p, "/", EXTRACT_DONT_COALESCE_SEPARATORS, &month, &day, &year, NULL); - if (r < 0) + const char *p = bios_date; + r = extract_many_words(&p, "/", EXTRACT_DONT_COALESCE_SEPARATORS, &month, &day, &year, NULL); + if (r < 0) return r; - if (r != 3) /* less than three args read? */ + if (r != 3) /* less than three args read? */ return -EINVAL; - if (!isempty(p)) /* more left in the string? */ + if (!isempty(p)) /* more left in the string? */ return -EINVAL; - unsigned m, d, y; - r = safe_atou_full(month, 10 | SAFE_ATO_REFUSE_PLUS_MINUS | SAFE_ATO_REFUSE_LEADING_WHITESPACE, &m); - if (r < 0) + unsigned m, d, y; + r = safe_atou_full(month, 10 | SAFE_ATO_REFUSE_PLUS_MINUS | SAFE_ATO_REFUSE_LEADING_WHITESPACE, &m); + if (r < 0) return r; - if (m < 1 || m > 12) + if (m < 1 || m > 12) return -EINVAL; - m -= 1; + m -= 1; - r = safe_atou_full(day, 10 | SAFE_ATO_REFUSE_PLUS_MINUS | SAFE_ATO_REFUSE_LEADING_WHITESPACE, &d); - if (r < 0) + r = safe_atou_full(day, 10 | SAFE_ATO_REFUSE_PLUS_MINUS | SAFE_ATO_REFUSE_LEADING_WHITESPACE, &d); + if (r < 0) return r; - if (d < 1 || d > 31) + if (d < 1 || d > 31) return -EINVAL; - r = safe_atou_full(year, 10 | SAFE_ATO_REFUSE_PLUS_MINUS | SAFE_ATO_REFUSE_LEADING_WHITESPACE, &y); - if (r < 0) + r = safe_atou_full(year, 10 | SAFE_ATO_REFUSE_PLUS_MINUS | SAFE_ATO_REFUSE_LEADING_WHITESPACE, &y); + if (r < 0) return r; - if (y < 1970 || y > (unsigned) INT_MAX) + if (y < 1970 || y > (unsigned) INT_MAX) return -EINVAL; - y -= 1900; + y -= 1900; - struct tm tm = { + struct tm tm = { .tm_mday = d, .tm_mon = m, .tm_year = y, - }; - time_t v = timegm(&tm); - if (v == (time_t) -1) + }; + time_t v = timegm(&tm); + if (v == (time_t) -1) return -errno; - if (tm.tm_mday != (int) d || tm.tm_mon != (int) m || tm.tm_year != (int) y) + if (tm.tm_mday != (int) d || tm.tm_mon != (int) m || tm.tm_year != (int) y) return -EINVAL; /* date was not normalized? (e.g. "30th of feb") */ - *ret = (usec_t) v * USEC_PER_SEC; + *ret = (usec_t) v * USEC_PER_SEC; - return 0; + return 0; } static const char* valid_chassis(const char *chassis) { From cb0734d53f42cd6dfc4800cfaed5f070433b5da7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 1 Mar 2024 14:45:37 +0100 Subject: [PATCH 3/6] hostnamed: in get_hardware_firmware_data() don't dup a string if we shan't return it --- src/hostname/hostnamed.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index ee2df8f2ff1..25e7b193cd1 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -249,7 +249,6 @@ static int get_hardware_model(char **ret) { static int get_hardware_firmware_data(const char *sysattr, char **ret) { _cleanup_(sd_device_unrefp) sd_device *device = NULL; - _cleanup_free_ char *b = NULL; const char *s = NULL; int r; @@ -263,16 +262,24 @@ static int get_hardware_firmware_data(const char *sysattr, char **ret) { return log_debug_errno(r, "Failed to open /sys/class/dmi/id device, ignoring: %m"); (void) sd_device_get_sysattr_value(device, sysattr, &s); - if (!isempty(s)) { - b = strdup(s); - if (!b) - return -ENOMEM; + + bool empty = isempty(s); + + if (ret) { + if (empty) + *ret = NULL; + else { + _cleanup_free_ char *b = NULL; + + b = strdup(s); + if (!b) + return -ENOMEM; + + *ret = TAKE_PTR(b); + } } - if (ret) - *ret = TAKE_PTR(b); - - return !isempty(s); + return !empty; } static int get_hardware_serial(char **ret) { From 64724e0579366f89914b0b8f8791e127b7fbca93 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 1 Mar 2024 14:46:00 +0100 Subject: [PATCH 4/6] hostnamed: do some validation of the hw serial before we return it Let's make sure the serial contains not control chars, and is UTF-8 clean. In particular the latter matters as D-Bus shouldn't kick us from the bus. --- src/hostname/hostnamed.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index 25e7b193cd1..87662e20912 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -39,6 +39,7 @@ #include "string-table.h" #include "strv.h" #include "user-util.h" +#include "utf8.h" #include "varlink-io.systemd.Hostname.h" #include "virt.h" @@ -283,13 +284,32 @@ static int get_hardware_firmware_data(const char *sysattr, char **ret) { } static int get_hardware_serial(char **ret) { - int r; + _cleanup_free_ char *b = NULL; + int r = 0; - r = get_hardware_firmware_data("product_serial", ret); - if (r <= 0) - return get_hardware_firmware_data("board_serial", ret); + FOREACH_STRING(attr, "product_serial", "board_serial") { + r = get_hardware_firmware_data(attr, &b); + if (r != 0 && !ERRNO_IS_NEG_DEVICE_ABSENT(r)) + break; + } + if (r < 0) + return r; + if (r == 0) + return -ENOENT; - return r; + /* Do some superficial validation: do not allow CCs and make sure D-Bus won't kick us off the bus + * because we send invalid UTF-8 data */ + + if (string_has_cc(b, /* ok= */ NULL)) + return -ENOENT; + + if (!utf8_is_valid(b)) + return -ENOENT; + + if (ret) + *ret = TAKE_PTR(b); + + return 0; } static int get_firmware_version(char **ret) { From 171ddae1a122e9c97b4ef12ccb2d29e1ba7a318a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 1 Mar 2024 14:46:27 +0100 Subject: [PATCH 5/6] hostnamed: add explicit BUS_ERROR_NO_HARDWARE_SERIAL error For the very similar case of the product UUID we have its own error BUS_ERROR_NO_PRODUCT_UUID if we have no UUID. Let's mirror this for the hardware serial, and expose the same, to keep things nicely symmteric. --- src/hostname/hostnamed.c | 3 ++- src/libsystemd/sd-bus/bus-common-errors.c | 1 + src/libsystemd/sd-bus/bus-common-errors.h | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index 87662e20912..e5679ab9315 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -1365,7 +1365,8 @@ static int method_get_hardware_serial(sd_bus_message *m, void *userdata, sd_bus_ r = get_hardware_serial(&serial); if (r < 0) - return r; + return sd_bus_error_set(error, BUS_ERROR_NO_HARDWARE_SERIAL, + "Failed to read hardware serial from firmware."); r = sd_bus_message_new_method_return(m, &reply); if (r < 0) diff --git a/src/libsystemd/sd-bus/bus-common-errors.c b/src/libsystemd/sd-bus/bus-common-errors.c index e44795b1d30..de12ec5e9eb 100644 --- a/src/libsystemd/sd-bus/bus-common-errors.c +++ b/src/libsystemd/sd-bus/bus-common-errors.c @@ -105,6 +105,7 @@ BUS_ERROR_MAP_ELF_REGISTER const sd_bus_error_map bus_common_errors[] = { SD_BUS_ERROR_MAP(BUS_ERROR_TRANSFER_IN_PROGRESS, EBUSY), SD_BUS_ERROR_MAP(BUS_ERROR_NO_PRODUCT_UUID, EOPNOTSUPP), + SD_BUS_ERROR_MAP(BUS_ERROR_NO_HARDWARE_SERIAL, EOPNOTSUPP), SD_BUS_ERROR_MAP(BUS_ERROR_FILE_IS_PROTECTED, EACCES), SD_BUS_ERROR_MAP(BUS_ERROR_READ_ONLY_FILESYSTEM, EROFS), diff --git a/src/libsystemd/sd-bus/bus-common-errors.h b/src/libsystemd/sd-bus/bus-common-errors.h index 36f53dbde14..94dc85d3012 100644 --- a/src/libsystemd/sd-bus/bus-common-errors.h +++ b/src/libsystemd/sd-bus/bus-common-errors.h @@ -106,6 +106,7 @@ #define BUS_ERROR_TRANSFER_IN_PROGRESS "org.freedesktop.import1.TransferInProgress" #define BUS_ERROR_NO_PRODUCT_UUID "org.freedesktop.hostname1.NoProductUUID" +#define BUS_ERROR_NO_HARDWARE_SERIAL "org.freedesktop.hostname1.NoHardwareSerial" #define BUS_ERROR_FILE_IS_PROTECTED "org.freedesktop.hostname1.FileIsProtected" #define BUS_ERROR_READ_ONLY_FILESYSTEM "org.freedesktop.hostname1.ReadOnlyFilesystem" From 75c7df05fbc8b6b4ee4ed25165b0622175c56e77 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 1 Mar 2024 14:43:20 +0100 Subject: [PATCH 6/6] hostnamectl: display product uuid + hardware serial in regular status output hostnamed provides this, hence hostnamectl should show it --- src/hostname/hostnamectl.c | 60 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/src/hostname/hostnamectl.c b/src/hostname/hostnamectl.c index 4abcf40c263..a70c9572910 100644 --- a/src/hostname/hostnamectl.c +++ b/src/hostname/hostnamectl.c @@ -59,6 +59,8 @@ typedef struct StatusInfo { usec_t firmware_date; sd_id128_t machine_id; sd_id128_t boot_id; + const char *hardware_serial; + sd_id128_t product_uuid; uint32_t vsock_cid; } StatusInfo; @@ -193,6 +195,14 @@ static int print_status_info(StatusInfo *i) { return table_log_add_error(r); } + if (!sd_id128_is_null(i->product_uuid)) { + r = table_add_many(table, + TABLE_FIELD, "Product UUID", + TABLE_UUID, i->product_uuid); + if (r < 0) + return table_log_add_error(r); + } + if (i->vsock_cid != VMADDR_CID_ANY) { r = table_add_many(table, TABLE_FIELD, "AF_VSOCK CID", @@ -274,6 +284,14 @@ static int print_status_info(StatusInfo *i) { return table_log_add_error(r); } + if (!isempty(i->hardware_serial)) { + r = table_add_many(table, + TABLE_FIELD, "Hardware Serial", + TABLE_STRING, i->hardware_serial); + if (r < 0) + return table_log_add_error(r); + } + if (!isempty(i->firmware_version)) { r = table_add_many(table, TABLE_FIELD, "Firmware Version", @@ -400,6 +418,48 @@ static int show_all_names(sd_bus *bus) { if (r < 0) return log_error_errno(r, "Failed to query system properties: %s", bus_error_message(&error, r)); + _cleanup_(sd_bus_message_unrefp) sd_bus_message *product_uuid_reply = NULL; + r = bus_call_method(bus, + bus_hostname, + "GetProductUUID", + &error, + &product_uuid_reply, + "b", + false); + if (r < 0) { + log_full_errno(sd_bus_error_has_names( + &error, + BUS_ERROR_NO_PRODUCT_UUID, + SD_BUS_ERROR_INTERACTIVE_AUTHORIZATION_REQUIRED, + SD_BUS_ERROR_UNKNOWN_METHOD) ? LOG_DEBUG : LOG_WARNING, + r, "Failed to query product UUID, ignoring: %s", bus_error_message(&error, r)); + sd_bus_error_free(&error); + } else { + r = bus_message_read_id128(product_uuid_reply, &info.product_uuid); + if (r < 0) + return bus_log_parse_error(r); + } + + _cleanup_(sd_bus_message_unrefp) sd_bus_message *hardware_serial_reply = NULL; + r = bus_call_method(bus, + bus_hostname, + "GetHardwareSerial", + &error, + &hardware_serial_reply, + NULL); + if (r < 0) + log_full_errno(sd_bus_error_has_names( + &error, + BUS_ERROR_NO_HARDWARE_SERIAL, + SD_BUS_ERROR_INTERACTIVE_AUTHORIZATION_REQUIRED, + SD_BUS_ERROR_UNKNOWN_METHOD) ? LOG_DEBUG : LOG_WARNING, + r, "Failed to query hardware serial, ignoring: %s", bus_error_message(&error, r)); + else { + r = sd_bus_message_read_basic(hardware_serial_reply, 's', &info.hardware_serial); + if (r < 0) + return bus_log_parse_error(r); + } + /* For older version of hostnamed. */ if (!arg_host) { if (sd_id128_is_null(info.machine_id))