Merge pull request #34778 from poettering/userdb-error-tweak

userdb: handle userbd replies indicating invalid user/group names like record not found

src/libsystemd/libsystemd.sym

@@ -975,6 +975,7 @@ global:
         sd_varlink_error_errno;
         sd_varlink_error_invalid_parameter;
         sd_varlink_error_invalid_parameter_name;
+        sd_varlink_error_is_invalid_parameter;
         sd_varlink_error_to_errno;
         sd_varlink_errorb;
         sd_varlink_flush;

src/libsystemd/sd-varlink/sd-varlink.c

@@ -4112,3 +4112,23 @@ _public_ int sd_varlink_error_to_errno(const char *error, sd_json_variant *param
 
         return -EBADR; /* Catch-all */
 }
+
+_public_ int sd_varlink_error_is_invalid_parameter(const char *error, sd_json_variant *parameter, const char *name) {
+
+        /* Returns true if the specified error result is an invalid parameter error for the parameter 'name' */
+
+        if (!streq_ptr(error, SD_VARLINK_ERROR_INVALID_PARAMETER))
+                return false;
+
+        if (!name)
+                return true;
+
+        if (!sd_json_variant_is_object(parameter))
+                return false;
+
+        sd_json_variant *e = sd_json_variant_by_key(parameter, "parameter");
+        if (!e || !sd_json_variant_is_string(e))
+                return false;
+
+        return streq(sd_json_variant_string(e), name);
+}

src/shared/userdb.c

@@ -171,9 +171,15 @@ static int userdb_on_query_reply(
         if (error_id) {
                 log_debug("Got lookup error: %s", error_id);
 
+                /* Convert various forms of record not found into -ESRCH, since NSS typically doesn't care,
+                 * about the details. Note that if a userName specification is refused as invalid parameter,
+                 * we also turn this into -ESRCH following the logic that there cannot be a user record for a
+                 * completely invalid user name. */
                 if (STR_IN_SET(error_id,
                                "io.systemd.UserDatabase.NoRecordFound",
-                               "io.systemd.UserDatabase.ConflictingRecordFound"))
+                               "io.systemd.UserDatabase.ConflictingRecordFound") ||
+                    sd_varlink_error_is_invalid_parameter(error_id, parameters, "userName") ||
+                    sd_varlink_error_is_invalid_parameter(error_id, parameters, "groupName"))
                         r = -ESRCH;
                 else if (streq(error_id, "io.systemd.UserDatabase.ServiceNotAvailable"))
                         r = -EHOSTDOWN;

src/systemd/sd-varlink.h

@@ -266,6 +266,8 @@ int sd_varlink_invocation(sd_varlink_invocation_flags_t flags);
 
 int sd_varlink_error_to_errno(const char *error, sd_json_variant *parameters);
 
+int sd_varlink_error_is_invalid_parameter(const char *error, sd_json_variant *parameter, const char *name);
+
 /* Define helpers so that __attribute__((cleanup(sd_varlink_unrefp))) and similar may be used. */
 _SD_DEFINE_POINTER_CLEANUP_FUNC(sd_varlink, sd_varlink_unref);
 _SD_DEFINE_POINTER_CLEANUP_FUNC(sd_varlink, sd_varlink_close_unref);

src/test/test-varlink.c

@@ -326,7 +326,7 @@ static int block_fd_handler(sd_event_source *s, int fd, uint32_t revents, void *
         return 0;
 }
 
-int main(int argc, char *argv[]) {
+TEST(chat) {
         _cleanup_(sd_event_source_unrefp) sd_event_source *block_event = NULL;
         _cleanup_(sd_varlink_server_unrefp) sd_varlink_server *s = NULL;
         _cleanup_(sd_varlink_flush_close_unrefp) sd_varlink *c = NULL;
@@ -337,8 +337,6 @@ int main(int argc, char *argv[]) {
         pthread_t t;
         const char *sp;
 
-        test_setup_logging(LOG_DEBUG);
-
         assert_se(mkdtemp_malloc("/tmp/varlink-test-XXXXXX", &tmpdir) >= 0);
         sp = strjoina(tmpdir, "/socket");
 
@@ -377,6 +375,59 @@ int main(int argc, char *argv[]) {
         assert_se(sd_event_loop(e) >= 0);
 
         assert_se(pthread_join(t, NULL) == 0);
+}
 
+static int method_invalid(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) {
+        int r;
+
+        sd_json_dispatch_field table[] = {
+                { "iexist", SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, 0, SD_JSON_MANDATORY },
+                {}
+        };
+
+        const char *p = NULL;
+
+        r = sd_varlink_dispatch(link, parameters, table, &p);
+        if (r != 0)
+                return r;
+
+        assert_not_reached();
+}
+
+static int reply_invalid(sd_varlink *link, sd_json_variant *parameters, const char *error_id, sd_varlink_reply_flags_t flags, void *userdata) {
+        assert(sd_varlink_error_is_invalid_parameter(error_id, parameters, "idontexist"));
+        assert(sd_event_exit(sd_varlink_get_event(link), EXIT_SUCCESS) >= 0);
         return 0;
 }
+
+TEST(invalid_parameter) {
+        _cleanup_(sd_event_unrefp) sd_event *e = NULL;
+        assert_se(sd_event_default(&e) >= 0);
+
+        _cleanup_(sd_varlink_server_unrefp) sd_varlink_server *s = NULL;
+        assert_se(sd_varlink_server_new(&s, 0) >= 0);
+
+        assert_se(sd_varlink_server_attach_event(s, e, 0) >= 0);
+
+        assert_se(sd_varlink_server_bind_method(s, "foo.mytest.Invalid", method_invalid) >= 0);
+
+        int connfd[2];
+        assert_se(socketpair(AF_UNIX, SOCK_STREAM|SOCK_NONBLOCK|SOCK_CLOEXEC, 0, connfd) >= 0);
+        assert_se(sd_varlink_server_add_connection(s, connfd[0], /* ret= */ NULL) >= 0);
+
+        _cleanup_(sd_varlink_unrefp) sd_varlink *c = NULL;
+        assert_se(sd_varlink_connect_fd(&c, connfd[1]) >= 0);
+
+        assert_se(sd_varlink_attach_event(c, e, 0) >= 0);
+
+        assert_se(sd_varlink_bind_reply(c, reply_invalid) >= 0);
+
+        assert_se(sd_varlink_invokebo(c, "foo.mytest.Invalid",
+                                      SD_JSON_BUILD_PAIR_STRING("iexist", "foo"),
+                                      SD_JSON_BUILD_PAIR_STRING("idontexist", "bar")) >= 0);
+
+
+        assert_se(sd_event_loop(e) >= 0);
+}
+
+DEFINE_TEST_MAIN(LOG_DEBUG);