From b6464e80d65fd5bfd9e6206ee305f0da9c88c096 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 27 Jun 2024 10:09:45 +0200 Subject: [PATCH 1/2] hostnamed: if polkit authentication fails for Varlink Describe() call, don't reply to client with an error The logic of the Describe() call was supposed to be: if we can acquire the PK priv to get the product UUID then let's return the product UUID, and if we cannot then return the data without it. This didn't work however, since the polkit varlink glue would immediately propagate the error it acquired from polkit its own client. Let's turn this off, optionally, so that hostnamed can handle this nicely. --- src/hostname/hostnamed.c | 4 +++- src/shared/bus-polkit.c | 12 +++++++----- src/shared/bus-polkit.h | 1 + 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index cda1205e268..a8ebf97cd26 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -1621,11 +1621,13 @@ static int vl_method_describe(Varlink *link, sd_json_variant *parameters, Varlin if (r != 0) return r; - r = varlink_verify_polkit_async( + r = varlink_verify_polkit_async_full( link, c->bus, "org.freedesktop.hostname1.get-hardware-serial", /* details= */ NULL, + UID_INVALID, + POLKIT_DONT_REPLY, &c->polkit_registry); if (r == 0) return 0; /* No authorization for now, but the async polkit stuff will call us again when it has it */ diff --git a/src/shared/bus-polkit.c b/src/shared/bus-polkit.c index aefc84a00ca..00c55463c82 100644 --- a/src/shared/bus-polkit.c +++ b/src/shared/bus-polkit.c @@ -786,11 +786,13 @@ int varlink_verify_polkit_async_full( if (r != 0) log_debug("Found matching previous polkit authentication for '%s'.", action); if (r < 0) { - /* Reply with a nice error */ - if (sd_bus_error_has_name(&error, SD_BUS_ERROR_INTERACTIVE_AUTHORIZATION_REQUIRED)) - (void) varlink_error(link, VARLINK_ERROR_INTERACTIVE_AUTHENTICATION_REQUIRED, NULL); - else if (ERRNO_IS_NEG_PRIVILEGE(r)) - (void) varlink_error(link, VARLINK_ERROR_PERMISSION_DENIED, NULL); + if (!FLAGS_SET(flags, POLKIT_DONT_REPLY)) { + /* Reply with a nice error */ + if (sd_bus_error_has_name(&error, SD_BUS_ERROR_INTERACTIVE_AUTHORIZATION_REQUIRED)) + (void) varlink_error(link, VARLINK_ERROR_INTERACTIVE_AUTHENTICATION_REQUIRED, NULL); + else if (ERRNO_IS_NEG_PRIVILEGE(r)) + (void) varlink_error(link, VARLINK_ERROR_PERMISSION_DENIED, NULL); + } return r; } diff --git a/src/shared/bus-polkit.h b/src/shared/bus-polkit.h index 25616a0a450..ba83cedbe18 100644 --- a/src/shared/bus-polkit.h +++ b/src/shared/bus-polkit.h @@ -11,6 +11,7 @@ typedef enum PolkitFLags { POLKIT_ALLOW_INTERACTIVE = 1 << 0, /* Allow interactive auth (typically not required, because can be derived from bus message/link automatically) */ POLKIT_ALWAYS_QUERY = 1 << 1, /* Query polkit even if client is privileged */ POLKIT_DEFAULT_ALLOW = 1 << 2, /* If polkit is not around, assume "allow" rather than the usual "deny" */ + POLKIT_DONT_REPLY = 1 << 3, /* Varlink: don't immediately propagate polkit error to the Varlink client */ } PolkitFlags; int bus_test_polkit(sd_bus_message *call, const char *action, const char **details, uid_t good_user, bool *_challenge, sd_bus_error *e); From 6678b9acc6435be56d0193dd0b40325fa615bd1f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 27 Jun 2024 10:12:04 +0200 Subject: [PATCH 2/2] hostnamed: make sure we can actually properly parse 'allowInteractiveAuthentication' varlink parameter If people want they should be able to turn on this flag, to allow interactive auth. Let's make sure this actually works. i.e. add it to the introspection data and don't refuse the parameter in Describe(). (note the varlink handling already does parameter validation through varlink_dispatch(), hence we can just drop any further validation) --- src/hostname/hostnamed.c | 3 --- src/shared/varlink-io.systemd.Hostname.c | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index a8ebf97cd26..a0bf1bfdc89 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -1636,9 +1636,6 @@ static int vl_method_describe(Varlink *link, sd_json_variant *parameters, Varlin * the product ID which we'll check explicitly. */ privileged = r > 0; - if (sd_json_variant_elements(parameters) > 0) - return varlink_error_invalid_parameter(link, parameters); - _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL; r = build_describe_response(c, privileged, &v); if (r < 0) diff --git a/src/shared/varlink-io.systemd.Hostname.c b/src/shared/varlink-io.systemd.Hostname.c index a6c6aec2a8c..247bca6da31 100644 --- a/src/shared/varlink-io.systemd.Hostname.c +++ b/src/shared/varlink-io.systemd.Hostname.c @@ -4,6 +4,7 @@ static VARLINK_DEFINE_METHOD( Describe, + VARLINK_DEFINE_INPUT(allowInteractiveAuthentication, VARLINK_BOOL, VARLINK_NULLABLE), VARLINK_DEFINE_OUTPUT(Hostname, VARLINK_STRING, 0), VARLINK_DEFINE_OUTPUT(StaticHostname, VARLINK_STRING, VARLINK_NULLABLE), VARLINK_DEFINE_OUTPUT(PrettyHostname, VARLINK_STRING, VARLINK_NULLABLE),