From 8f796e40a561bd9200fde3c8885e6255a2dd4250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 28 Aug 2020 16:23:16 +0200 Subject: [PATCH 01/14] shared/{user,group}-record-nss: adjust filtering of "valid" passwords We would reject various passwords that glibc accepts, for example "" or any descrypted password. Accounts with empty password are definitely useful, for example for testing or in scenarios where a password is not needed. Also, using weak encryption methods is probably not a good idea, it's not the job of our nss helpers to decide that: they should just faithfully forward whatever data is there. Also rename the function to make it more obvious that the returned answer is not in any way certain. --- src/shared/group-record-nss.c | 2 +- src/shared/libcrypt-util.c | 19 ++++++++++++------- src/shared/libcrypt-util.h | 2 +- src/shared/user-record-nss.c | 2 +- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/shared/group-record-nss.c b/src/shared/group-record-nss.c index 5c4fae865a..b018a46e18 100644 --- a/src/shared/group-record-nss.c +++ b/src/shared/group-record-nss.c @@ -37,7 +37,7 @@ int nss_group_to_group_record( g->gid = grp->gr_gid; if (sgrp) { - if (hashed_password_valid(sgrp->sg_passwd)) { + if (looks_like_hashed_password(sgrp->sg_passwd)) { g->hashed_password = strv_new(sgrp->sg_passwd); if (!g->hashed_password) return -ENOMEM; diff --git a/src/shared/libcrypt-util.c b/src/shared/libcrypt-util.c index f41685ae45..bf6605508a 100644 --- a/src/shared/libcrypt-util.c +++ b/src/shared/libcrypt-util.c @@ -74,13 +74,18 @@ int make_salt(char **ret) { #endif } -bool hashed_password_valid(const char *s) { - - /* Returns true if the specified string is a 'valid' hashed UNIX password, i.e. if starts with '$' or - * with '!$' (the latter being a valid, yet locked password). */ - - if (isempty(s)) +bool looks_like_hashed_password(const char *s) { + /* Returns false if the specified string is certainly not a hashed UNIX password. crypt(5) lists + * various hashing methods. We only reject (return false) strings which are documented to have + * different meanings. + * + * In particular, we allow locked passwords, i.e. strings starting with "!", including just "!", + * i.e. the locked empty password. See also fc58c0c7bf7e4f525b916e3e5be0de2307fef04e. + */ + if (!s) return false; - return STARTSWITH_SET(s, "$", "!$"); + s += strspn(s, "!"); /* Skip (possibly duplicated) locking prefix */ + + return !STR_IN_SET(s, "x", "*"); } diff --git a/src/shared/libcrypt-util.h b/src/shared/libcrypt-util.h index 93f0e13ffb..8a860ceb0d 100644 --- a/src/shared/libcrypt-util.h +++ b/src/shared/libcrypt-util.h @@ -19,4 +19,4 @@ int make_salt(char **ret); -bool hashed_password_valid(const char *s); +bool looks_like_hashed_password(const char *s); diff --git a/src/shared/user-record-nss.c b/src/shared/user-record-nss.c index b27a12c55d..b4c35b8a53 100644 --- a/src/shared/user-record-nss.c +++ b/src/shared/user-record-nss.c @@ -66,7 +66,7 @@ int nss_passwd_to_user_record( hr->uid = pwd->pw_uid; hr->gid = pwd->pw_gid; - if (spwd && hashed_password_valid(spwd->sp_pwdp)) { + if (spwd && looks_like_hashed_password(spwd->sp_pwdp)) { strv_free_erase(hr->hashed_password); hr->hashed_password = strv_new(spwd->sp_pwdp); if (!hr->hashed_password) From 31be0e9e00f94b376c3b60027af43045305844b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 28 Aug 2020 16:37:09 +0200 Subject: [PATCH 02/14] basic/escape: use consistent location for "*" in function declarations I think it's nicer to move it to the left, since the function is already a pointer by itself, and it just happens to return a pointer, and the two concepts are completely separate. --- src/basic/escape.c | 14 +++++++------- src/basic/escape.h | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/basic/escape.c b/src/basic/escape.c index 116efa4119..7589d597a2 100644 --- a/src/basic/escape.c +++ b/src/basic/escape.c @@ -75,7 +75,7 @@ int cescape_char(char c, char *buf) { return buf - buf_old; } -char *cescape_length(const char *s, size_t n) { +char* cescape_length(const char *s, size_t n) { const char *f; char *r, *t; @@ -96,7 +96,7 @@ char *cescape_length(const char *s, size_t n) { return r; } -char *cescape(const char *s) { +char* cescape(const char *s) { assert(s); return cescape_length(s, strlen(s)); @@ -360,7 +360,7 @@ int cunescape_length_with_prefix(const char *s, size_t length, const char *prefi return t - r; } -char *xescape_full(const char *s, const char *bad, size_t console_width, bool eight_bits) { +char* xescape_full(const char *s, const char *bad, size_t console_width, bool eight_bits) { char *ans, *t, *prev, *prev2; const char *f; @@ -427,14 +427,14 @@ char *xescape_full(const char *s, const char *bad, size_t console_width, bool ei return ans; } -char *escape_non_printable_full(const char *str, size_t console_width, bool eight_bit) { +char* escape_non_printable_full(const char *str, size_t console_width, bool eight_bit) { if (eight_bit) return xescape_full(str, "", console_width, true); else return utf8_escape_non_printable_full(str, console_width); } -char *octescape(const char *s, size_t len) { +char* octescape(const char *s, size_t len) { char *r, *t; const char *f; @@ -462,7 +462,7 @@ char *octescape(const char *s, size_t len) { } -static char *strcpy_backslash_escaped(char *t, const char *s, const char *bad, bool escape_tab_nl) { +static char* strcpy_backslash_escaped(char *t, const char *s, const char *bad, bool escape_tab_nl) { assert(bad); for (; *s; s++) { @@ -481,7 +481,7 @@ static char *strcpy_backslash_escaped(char *t, const char *s, const char *bad, b return t; } -char *shell_escape(const char *s, const char *bad) { +char* shell_escape(const char *s, const char *bad) { char *r, *t; r = new(char, strlen(s)*2+1); diff --git a/src/basic/escape.h b/src/basic/escape.h index 0b00b116ed..fa267813b3 100644 --- a/src/basic/escape.h +++ b/src/basic/escape.h @@ -43,8 +43,8 @@ typedef enum EscapeStyle { * syntax (a string enclosed in $'') instead of plain quotes. */ } EscapeStyle; -char *cescape(const char *s); -char *cescape_length(const char *s, size_t n); +char* cescape(const char *s); +char* cescape_length(const char *s, size_t n); int cescape_char(char c, char *buf); int cunescape_length_with_prefix(const char *s, size_t length, const char *prefix, UnescapeFlags flags, char **ret); @@ -56,12 +56,12 @@ static inline int cunescape(const char *s, UnescapeFlags flags, char **ret) { } int cunescape_one(const char *p, size_t length, char32_t *ret, bool *eight_bit, bool accept_nul); -char *xescape_full(const char *s, const char *bad, size_t console_width, bool eight_bits); -static inline char *xescape(const char *s, const char *bad) { +char* xescape_full(const char *s, const char *bad, size_t console_width, bool eight_bits); +static inline char* xescape(const char *s, const char *bad) { return xescape_full(s, bad, SIZE_MAX, false); } -char *octescape(const char *s, size_t len); -char *escape_non_printable_full(const char *str, size_t console_width, bool eight_bit); +char* octescape(const char *s, size_t len); +char* escape_non_printable_full(const char *str, size_t console_width, bool eight_bit); -char *shell_escape(const char *s, const char *bad); +char* shell_escape(const char *s, const char *bad); char* shell_maybe_quote(const char *s, EscapeStyle style); From 52d3fbc83f4a81e43910d3b5820bd6d4d2284f20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 28 Aug 2020 17:11:46 +0200 Subject: [PATCH 03/14] shared: merge {user,group}-record-show.[ch] It is natural to include both, and in total they declared three functions. Let's merge them for simplicity. --- src/shared/group-record-show.c | 79 ---------------------------------- src/shared/group-record-show.h | 6 --- src/shared/meson.build | 2 - src/shared/user-record-show.c | 73 ++++++++++++++++++++++++++++++- src/shared/user-record-show.h | 2 + src/userdb/userdbctl.c | 1 - 6 files changed, 74 insertions(+), 89 deletions(-) delete mode 100644 src/shared/group-record-show.c delete mode 100644 src/shared/group-record-show.h diff --git a/src/shared/group-record-show.c b/src/shared/group-record-show.c deleted file mode 100644 index 8b59f919fa..0000000000 --- a/src/shared/group-record-show.c +++ /dev/null @@ -1,79 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1+ */ - -#include "format-util.h" -#include "group-record-show.h" -#include "strv.h" -#include "user-util.h" -#include "userdb.h" - -void group_record_show(GroupRecord *gr, bool show_full_user_info) { - int r; - - printf(" Group name: %s\n", - group_record_group_name_and_realm(gr)); - - printf(" Disposition: %s\n", user_disposition_to_string(group_record_disposition(gr))); - - if (gr->last_change_usec != USEC_INFINITY) { - char buf[FORMAT_TIMESTAMP_MAX]; - printf(" Last Change: %s\n", format_timestamp(buf, sizeof(buf), gr->last_change_usec)); - } - - if (gid_is_valid(gr->gid)) - printf(" GID: " GID_FMT "\n", gr->gid); - - if (show_full_user_info) { - _cleanup_(userdb_iterator_freep) UserDBIterator *iterator = NULL; - - r = membershipdb_by_group(gr->group_name, 0, &iterator); - if (r < 0) { - errno = -r; - printf(" Members: (can't acquire: %m)"); - } else { - const char *prefix = " Members:"; - - for (;;) { - _cleanup_free_ char *user = NULL; - - r = membershipdb_iterator_get(iterator, &user, NULL); - if (r == -ESRCH) - break; - if (r < 0) { - errno = -r; - printf("%s (can't iterate: %m\n", prefix); - break; - } - - printf("%s %s\n", prefix, user); - prefix = " "; - } - } - } else { - const char *prefix = " Members:"; - char **i; - - STRV_FOREACH(i, gr->members) { - printf("%s %s\n", prefix, *i); - prefix = " "; - } - } - - if (!strv_isempty(gr->administrators)) { - const char *prefix = " Admins:"; - char **i; - - STRV_FOREACH(i, gr->administrators) { - printf("%s %s\n", prefix, *i); - prefix = " "; - } - } - - if (gr->description && !streq(gr->description, gr->group_name)) - printf(" Description: %s\n", gr->description); - - if (!strv_isempty(gr->hashed_password)) - printf(" Passwords: %zu\n", strv_length(gr->hashed_password)); - - if (gr->service) - printf(" Service: %s\n", gr->service); -} diff --git a/src/shared/group-record-show.h b/src/shared/group-record-show.h deleted file mode 100644 index 12bdbd1724..0000000000 --- a/src/shared/group-record-show.h +++ /dev/null @@ -1,6 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1+ */ -#pragma once - -#include "group-record.h" - -void group_record_show(GroupRecord *gr, bool show_full_user_info); diff --git a/src/shared/meson.build b/src/shared/meson.build index 38762f020e..c149ff4cd8 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -115,8 +115,6 @@ shared_sources = files(''' gpt.h group-record-nss.c group-record-nss.h - group-record-show.c - group-record-show.h group-record.c group-record.h id128-print.c diff --git a/src/shared/user-record-show.c b/src/shared/user-record-show.c index 551df720ba..33787c083f 100644 --- a/src/shared/user-record-show.c +++ b/src/shared/user-record-show.c @@ -2,7 +2,6 @@ #include "format-util.h" #include "fs-util.h" -#include "group-record.h" #include "process-util.h" #include "rlimit-util.h" #include "strv.h" @@ -506,3 +505,75 @@ void user_record_show(UserRecord *hr, bool show_full_group_info) { if (hr->service) printf(" Service: %s\n", hr->service); } + +void group_record_show(GroupRecord *gr, bool show_full_user_info) { + int r; + + printf(" Group name: %s\n", + group_record_group_name_and_realm(gr)); + + printf(" Disposition: %s\n", user_disposition_to_string(group_record_disposition(gr))); + + if (gr->last_change_usec != USEC_INFINITY) { + char buf[FORMAT_TIMESTAMP_MAX]; + printf(" Last Change: %s\n", format_timestamp(buf, sizeof(buf), gr->last_change_usec)); + } + + if (gid_is_valid(gr->gid)) + printf(" GID: " GID_FMT "\n", gr->gid); + + if (show_full_user_info) { + _cleanup_(userdb_iterator_freep) UserDBIterator *iterator = NULL; + + r = membershipdb_by_group(gr->group_name, 0, &iterator); + if (r < 0) { + errno = -r; + printf(" Members: (can't acquire: %m)"); + } else { + const char *prefix = " Members:"; + + for (;;) { + _cleanup_free_ char *user = NULL; + + r = membershipdb_iterator_get(iterator, &user, NULL); + if (r == -ESRCH) + break; + if (r < 0) { + errno = -r; + printf("%s (can't iterate: %m\n", prefix); + break; + } + + printf("%s %s\n", prefix, user); + prefix = " "; + } + } + } else { + const char *prefix = " Members:"; + char **i; + + STRV_FOREACH(i, gr->members) { + printf("%s %s\n", prefix, *i); + prefix = " "; + } + } + + if (!strv_isempty(gr->administrators)) { + const char *prefix = " Admins:"; + char **i; + + STRV_FOREACH(i, gr->administrators) { + printf("%s %s\n", prefix, *i); + prefix = " "; + } + } + + if (gr->description && !streq(gr->description, gr->group_name)) + printf(" Description: %s\n", gr->description); + + if (!strv_isempty(gr->hashed_password)) + printf(" Passwords: %zu\n", strv_length(gr->hashed_password)); + + if (gr->service) + printf(" Service: %s\n", gr->service); +} diff --git a/src/shared/user-record-show.h b/src/shared/user-record-show.h index bd22be2ae0..4dcee180a2 100644 --- a/src/shared/user-record-show.h +++ b/src/shared/user-record-show.h @@ -2,7 +2,9 @@ #pragma once #include "user-record.h" +#include "group-record.h" const char *user_record_state_color(const char *state); void user_record_show(UserRecord *hr, bool show_full_group_info); +void group_record_show(GroupRecord *gr, bool show_full_user_info); diff --git a/src/userdb/userdbctl.c b/src/userdb/userdbctl.c index 12c6943ebd..0e3204f932 100644 --- a/src/userdb/userdbctl.c +++ b/src/userdb/userdbctl.c @@ -8,7 +8,6 @@ #include "fd-util.h" #include "format-table.h" #include "format-util.h" -#include "group-record-show.h" #include "main-func.h" #include "pager.h" #include "parse-util.h" From c4651e3156463758ffde9a791197d62f76caf6ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 28 Aug 2020 17:25:14 +0200 Subject: [PATCH 04/14] userdbctl: add forgotten --output mode in help --- src/userdb/userdbctl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/userdb/userdbctl.c b/src/userdb/userdbctl.c index 0e3204f932..0c135fae14 100644 --- a/src/userdb/userdbctl.c +++ b/src/userdb/userdbctl.c @@ -686,7 +686,8 @@ static int parse_argv(int argc, char *argv[]) { else if (streq(optarg, "help")) { puts("classic\n" "friendly\n" - "json"); + "json\n" + "table"); return 0; } else return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid --output= mode: %s", optarg); From 4fcc9c4962993718d405e95e7a4460a75298927f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 28 Aug 2020 18:11:48 +0200 Subject: [PATCH 05/14] userdb: fix typo --- src/userdb/userdbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/userdb/userdbd.c b/src/userdb/userdbd.c index dbc285e61a..e456104dd7 100644 --- a/src/userdb/userdbd.c +++ b/src/userdb/userdbd.c @@ -11,7 +11,7 @@ /* This service offers two Varlink services, both implementing io.systemd.UserDatabase: * - * → io.systemd.NameServiceSwitch: this is a compatibility interface for glibc NSS: it response to + * → io.systemd.NameServiceSwitch: this is a compatibility interface for glibc NSS: it responds to * name lookups by checking the classic NSS interfaces and responding that. * * → io.systemd.Multiplexer: this multiplexes lookup requests to all Varlink services that have a From f7dc8248d3402a6c7a4c1ccabf58ae83df713417 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 28 Aug 2020 18:21:33 +0200 Subject: [PATCH 06/14] man: add hint how to show password strings with userdbctl I started working on a command-line switch to show passwords also in "pretty" mode. I can submit that code for review if anyone thinks that woul be useful, but after writing the man page I realized that it's a fairly niche case, and the hint in the man page is a sufficient replacement. --- man/userdbctl.xml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/man/userdbctl.xml b/man/userdbctl.xml index 9a69f33edb..c86937760f 100644 --- a/man/userdbctl.xml +++ b/man/userdbctl.xml @@ -59,7 +59,13 @@ user friendly, human readable output is generated; if table a minimal, tabular output is generated; if json a JSON formatted output is generated. Defaults to friendly if a user/group is specified on the command line, - table otherwise. + table otherwise. + + Note that most output formats do not show all available information. In particular, + classic and table show only the most important fields. Various + modes also do not show password hashes. Use json to view all fields, including + any authentication fields. + From 77472d06a4740d820ebccdb04e217d6b7d66dd50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 28 Aug 2020 19:22:20 +0200 Subject: [PATCH 07/14] varlink: do not parse invalid messages twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upon reception of a message which fails in json_parse(), we would proceed to parse it again from a deferred callback and hang. Once we have realized that the message is invalid, let's move the pointer in the buffer even if the message is invalid. We don't want to look at this data again. (before) $ build-rawhide/userdbctl --output=json user test.user n/a: varlink: setting state idle-client /run/systemd/userdb/io.systemd.Multiplexer: Sending message: {"method":"io.systemd.UserDatabase.GetUserRecord","parameters":{"userName":"test.user","service":"io.systemd.Multiplexer"}} /run/systemd/userdb/io.systemd.Multiplexer: varlink: changing state idle-client → awaiting-reply /run/systemd/userdb/io.systemd.Multiplexer: New incoming message: {...} /run/systemd/userdb/io.systemd.Multiplexer: varlink: changing state awaiting-reply → pending-disconnect /run/systemd/userdb/io.systemd.Multiplexer: New incoming message: {...} /run/systemd/userdb/io.systemd.Multiplexer: varlink: changing state pending-disconnect → disconnected ^C (after) $ n/a: varlink: setting state idle-client /run/systemd/userdb/io.systemd.Multiplexer: Sending message: {"method":"io.systemd.UserDatabase.GetUserRecord","parameters":{"userName":"test.user","service":"io.systemd.Multiplexer"}} /run/systemd/userdb/io.systemd.Multiplexer: varlink: changing state idle-client → awaiting-reply /run/systemd/userdb/io.systemd.Multiplexer: New incoming message: {...} /run/systemd/userdb/io.systemd.Multiplexer: Failed to parse JSON: Invalid argument /run/systemd/userdb/io.systemd.Multiplexer: varlink: changing state awaiting-reply → pending-disconnect /run/systemd/userdb/io.systemd.Multiplexer: varlink: changing state pending-disconnect → processing-disconnect Got lookup error: io.systemd.Disconnected /run/systemd/userdb/io.systemd.Multiplexer: varlink: changing state processing-disconnect → disconnected Failed to find user test.user: Input/output error This should fix #16683 and https://bugs.gentoo.org/735072. --- src/shared/varlink.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/shared/varlink.c b/src/shared/varlink.c index e2b4bb623d..99ae9265f9 100644 --- a/src/shared/varlink.c +++ b/src/shared/varlink.c @@ -579,11 +579,17 @@ static int varlink_parse_message(Varlink *v) { sz = e - begin + 1; - varlink_log(v, "New incoming message: %s", begin); + varlink_log(v, "New incoming message: %s", begin); /* FIXME: should we output the whole message here before validation? + * This may produce a non-printable journal entry if the message + * is invalid. We may also expose privileged information. */ r = json_parse(begin, 0, &v->current, NULL, NULL); - if (r < 0) - return r; + if (r < 0) { + /* If we encounter a parse failure flush all data. We cannot possibly recover from this, + * hence drop all buffered data now. */ + v->input_buffer_index = v->input_buffer_size = v->input_buffer_unscanned = 0; + return varlink_log_errno(v, r, "Failed to parse JSON: %m"); + } v->input_buffer_size -= sz; From e12b6e1951dd1dcde6c0c10fda0b9bb148e1847c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 28 Aug 2020 19:47:19 +0200 Subject: [PATCH 08/14] json: split out string formatting to a separate function It's complicated enough to deserve it's own function. No functional change. --- src/shared/json.c | 108 +++++++++++++++++++++++----------------------- 1 file changed, 54 insertions(+), 54 deletions(-) diff --git a/src/shared/json.c b/src/shared/json.c index 04c721e157..11ad6091a4 100644 --- a/src/shared/json.c +++ b/src/shared/json.c @@ -1482,6 +1482,58 @@ static int print_source(FILE *f, JsonVariant *v, JsonFormatFlags flags, bool whi return 0; } +static void json_format_string(FILE *f, const char *q, JsonFormatFlags flags) { + assert(q); + + fputc('"', f); + + if (flags & JSON_FORMAT_COLOR) + fputs(ANSI_GREEN, f); + + for (; *q; q++) + switch (*q) { + case '"': + fputs("\\\"", f); + break; + + case '\\': + fputs("\\\\", f); + break; + + case '\b': + fputs("\\b", f); + break; + + case '\f': + fputs("\\f", f); + break; + + case '\n': + fputs("\\n", f); + break; + + case '\r': + fputs("\\r", f); + break; + + case '\t': + fputs("\\t", f); + break; + + default: + if ((signed char) *q >= 0 && *q < ' ') + fprintf(f, "\\u%04x", *q); + else + fputc(*q, f); + break; + } + + if (flags & JSON_FORMAT_COLOR) + fputs(ANSI_NORMAL, f); + + fputc('"', f); +} + static int json_format(FILE *f, JsonVariant *v, JsonFormatFlags flags, const char *prefix) { int r; @@ -1554,61 +1606,9 @@ static int json_format(FILE *f, JsonVariant *v, JsonFormatFlags flags, const cha fputs(ANSI_NORMAL, f); break; - case JSON_VARIANT_STRING: { - const char *q; - - fputc('"', f); - - if (flags & JSON_FORMAT_COLOR) - fputs(ANSI_GREEN, f); - - for (q = json_variant_string(v); *q; q++) { - - switch (*q) { - - case '"': - fputs("\\\"", f); - break; - - case '\\': - fputs("\\\\", f); - break; - - case '\b': - fputs("\\b", f); - break; - - case '\f': - fputs("\\f", f); - break; - - case '\n': - fputs("\\n", f); - break; - - case '\r': - fputs("\\r", f); - break; - - case '\t': - fputs("\\t", f); - break; - - default: - if ((signed char) *q >= 0 && *q < ' ') - fprintf(f, "\\u%04x", *q); - else - fputc(*q, f); - break; - } - } - - if (flags & JSON_FORMAT_COLOR) - fputs(ANSI_NORMAL, f); - - fputc('"', f); + case JSON_VARIANT_STRING: + json_format_string(f, json_variant_string(v), flags); break; - } case JSON_VARIANT_ARRAY: { size_t i, n; From 80ab31a43577ab95eb3ddfac637bd792989555b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 1 Sep 2020 10:43:21 +0200 Subject: [PATCH 09/14] shared/utf8: add utf8_is_valid_n() Sometimes we need to check strings without the terminating NUL. Add a variant that does that. --- src/basic/utf8.c | 18 +++++++++++------- src/basic/utf8.h | 5 ++++- src/test/test-utf8.c | 20 ++++++++++++++++++++ 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/src/basic/utf8.c b/src/basic/utf8.c index 174075be54..f0233397ef 100644 --- a/src/basic/utf8.c +++ b/src/basic/utf8.c @@ -150,18 +150,22 @@ bool utf8_is_printable_newline(const char* str, size_t length, bool allow_newlin return true; } -char *utf8_is_valid(const char *str) { - const char *p; +char *utf8_is_valid_n(const char *str, size_t len_bytes) { + /* Check if the string is composed of valid utf8 characters. If length len_bytes is given, stop after + * len_bytes. Otherwise, stop at NUL. */ assert(str); - p = str; - while (*p) { + for (const char *p = str; len_bytes != (size_t) -1 ? (size_t) (p - str) < len_bytes : *p != '\0'; ) { int len; - len = utf8_encoded_valid_unichar(p, (size_t) -1); - if (len < 0) - return NULL; + if (_unlikely_(*p == '\0') && len_bytes != (size_t) -1) + return NULL; /* embedded NUL */ + + len = utf8_encoded_valid_unichar(p, + len_bytes != (size_t) -1 ? len_bytes - (p - str) : (size_t) -1); + if (_unlikely_(len < 0)) + return NULL; /* invalid character */ p += len; } diff --git a/src/basic/utf8.h b/src/basic/utf8.h index 52b487955b..f315ea0f1e 100644 --- a/src/basic/utf8.h +++ b/src/basic/utf8.h @@ -14,7 +14,10 @@ bool unichar_is_valid(char32_t c); -char *utf8_is_valid(const char *s) _pure_; +char *utf8_is_valid_n(const char *str, size_t len_bytes) _pure_; +static inline char *utf8_is_valid(const char *s) { + return utf8_is_valid_n(s, (size_t) -1); +} char *ascii_is_valid(const char *s) _pure_; char *ascii_is_valid_n(const char *str, size_t len); diff --git a/src/test/test-utf8.c b/src/test/test-utf8.c index 8937f56237..66003ac13e 100644 --- a/src/test/test-utf8.c +++ b/src/test/test-utf8.c @@ -18,6 +18,25 @@ static void test_utf8_is_printable(void) { assert_se(utf8_is_printable("\t", 1)); } +static void test_utf8_n_is_valid(void) { + log_info("/* %s */", __func__); + + assert_se( utf8_is_valid_n("ascii is valid unicode", 21)); + assert_se( utf8_is_valid_n("ascii is valid unicode", 22)); + assert_se(!utf8_is_valid_n("ascii is valid unicode", 23)); + assert_se( utf8_is_valid_n("\342\204\242", 0)); + assert_se(!utf8_is_valid_n("\342\204\242", 1)); + assert_se(!utf8_is_valid_n("\342\204\242", 2)); + assert_se( utf8_is_valid_n("\342\204\242", 3)); + assert_se(!utf8_is_valid_n("\342\204\242", 4)); + assert_se( utf8_is_valid_n("", 0)); + assert_se( utf8_is_valid_n("", 1)); + assert_se( utf8_is_valid_n("", 2)); + assert_se( utf8_is_valid_n("", 3)); + assert_se( utf8_is_valid_n("", 4)); + assert_se(!utf8_is_valid_n("", 5)); +} + static void test_utf8_is_valid(void) { log_info("/* %s */", __func__); @@ -216,6 +235,7 @@ static void test_utf8_to_utf16(void) { } int main(int argc, char *argv[]) { + test_utf8_n_is_valid(); test_utf8_is_valid(); test_utf8_is_printable(); test_ascii_is_valid(); From ea9afe0064adbcc5ff7c8d44026e996f4c8e3e89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 30 Aug 2020 19:52:10 +0200 Subject: [PATCH 10/14] shared/json: reject non-utf-8 strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JSON strings must be utf-8-clean. We also verify this in json_parse_string() so we would reject a message with invalid utf-8 anyway. It would probably be slightly cheaper to detect non-conformaning strings in serialization, but then we'd have to fail serialization. By doing this early, we give the caller a chance to handle the error nicely. The test is adjusted to contain a valid utf-8 string after decoding of the utf-32 encoding in json ("विवेकख्यातिरविप्लवा हानोपायः।", something about the cessation of ignorance). --- src/shared/json.c | 9 ++++++++- src/test/test-json.c | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/shared/json.c b/src/shared/json.c index 11ad6091a4..e938e59ab6 100644 --- a/src/shared/json.c +++ b/src/shared/json.c @@ -405,6 +405,9 @@ int json_variant_new_stringn(JsonVariant **ret, const char *s, size_t n) { return 0; } + if (!utf8_is_valid_n(s, n)) /* JSON strings must be valid UTF-8 */ + return -EUCLEAN; + r = json_variant_new(&v, JSON_VARIANT_STRING, n + 1); if (r < 0) return r; @@ -636,8 +639,12 @@ int json_variant_new_array_strv(JsonVariant **ret, char **l) { return r; w->is_reference = true; - } else + } else { + if (!utf8_is_valid_n(l[v->n_elements], k)) /* JSON strings must be valid UTF-8 */ + return -EUCLEAN; + memcpy(w->string, l[v->n_elements], k+1); + } } v->normalized = true; diff --git a/src/test/test-json.c b/src/test/test-json.c index 032619a425..3295287a67 100644 --- a/src/test/test-json.c +++ b/src/test/test-json.c @@ -543,7 +543,7 @@ int main(int argc, char *argv[]) { test_variant("{\"k\": \"v\", \"foo\": [1, 2, 3], \"bar\": {\"zap\": null}}", test_1); test_variant("{\"mutant\": [1, null, \"1\", {\"1\": [1, \"1\"]}], \"thisisaverylongproperty\": 1.27}", test_2); - test_variant("{\"foo\" : \"\\uDBFF\\uDFFF\\\"\\uD9FF\\uDFFFFFF\\\"\\uDBFF\\uDFFF\\\"\\uD9FF\\uDFFF\\uDBFF\\uDFFFF\\uDBFF\\uDFFF\\uDBFF\\uDFFF\\uDBFF\\uDFFF\\uDBFF\\uDFFF\\\"\\uD9FF\\uDFFFFF\\\"\\uDBFF\\uDFFF\\\"\\uD9FF\\uDFFF\\uDBFF\\uDFFF\"}", NULL); + test_variant("{\"foo\" : \"\\u0935\\u093f\\u0935\\u0947\\u0915\\u0916\\u094d\\u092f\\u093e\\u0924\\u093f\\u0930\\u0935\\u093f\\u092a\\u094d\\u0932\\u0935\\u093e\\u0020\\u0939\\u093e\\u0928\\u094b\\u092a\\u093e\\u092f\\u0903\\u0964\"}", NULL); test_variant("[ 0, -0, 0.0, -0.0, 0.000, -0.000, 0e0, -0e0, 0e+0, -0e-0, 0e-0, -0e000, 0e+000 ]", test_zeroes); From 4d7f51756a5b665342f32f89ea7a2ea887e8fe01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 1 Sep 2020 09:29:13 +0200 Subject: [PATCH 11/14] test-json: add function headers --- src/test/test-json.c | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/src/test/test-json.c b/src/test/test-json.c index 3295287a67..6da93d6aed 100644 --- a/src/test/test-json.c +++ b/src/test/test-json.c @@ -3,6 +3,7 @@ #include #include "alloc-util.h" +#include "escape.h" #include "fd-util.h" #include "fileio.h" #include "json-internal.h" @@ -17,6 +18,10 @@ static void test_tokenizer(const char *data, ...) { void *state = NULL; va_list ap; + _cleanup_free_ char *cdata; + assert_se(cdata = cescape(data)); + log_info("/* %s data=\"%s\" */", __func__, cdata); + va_start(ap, data); for (;;) { @@ -82,6 +87,10 @@ static void test_variant(const char *data, Test test) { _cleanup_free_ char *s = NULL; int r; + _cleanup_free_ char *cdata; + assert_se(cdata = cescape(data)); + log_info("/* %s data=\"%s\" */", __func__, cdata); + r = json_parse(data, 0, &v, NULL, NULL); assert_se(r == 0); assert_se(v); @@ -140,6 +149,8 @@ static void test_1(JsonVariant *v) { JsonVariant *p, *q; unsigned i; + log_info("/* %s */", __func__); + /* 3 keys + 3 values */ assert_se(json_variant_elements(v) == 6); @@ -173,6 +184,8 @@ static void test_1(JsonVariant *v) { static void test_2(JsonVariant *v) { JsonVariant *p, *q; + log_info("/* %s */", __func__); + /* 2 keys + 2 values */ assert_se(json_variant_elements(v) == 4); @@ -216,13 +229,12 @@ static void test_2(JsonVariant *v) { } static void test_zeroes(JsonVariant *v) { - size_t i; - /* Make sure zero is how we expect it. */ + log_info("/* %s */", __func__); assert_se(json_variant_elements(v) == 13); - for (i = 0; i < json_variant_elements(v); i++) { + for (size_t i = 0; i < json_variant_elements(v); i++) { JsonVariant *w; size_t j; @@ -255,6 +267,8 @@ static void test_zeroes(JsonVariant *v) { } static void test_build(void) { + log_info("/* %s */", __func__); + _cleanup_(json_variant_unrefp) JsonVariant *a = NULL, *b = NULL; _cleanup_free_ char *s = NULL, *t = NULL; @@ -355,6 +369,8 @@ static void test_source(void) { "false, 7.5, {} ]\n" "}\n"; + log_info("/* %s */", __func__); + _cleanup_fclose_ FILE *f = NULL; _cleanup_(json_variant_unrefp) JsonVariant *v = NULL; @@ -376,15 +392,16 @@ static void test_source(void) { } static void test_depth(void) { + log_info("/* %s */", __func__); + _cleanup_(json_variant_unrefp) JsonVariant *v = NULL; - unsigned i; int r; v = JSON_VARIANT_STRING_CONST("start"); /* Let's verify that the maximum depth checks work */ - for (i = 0;; i++) { + for (unsigned i = 0;; i++) { _cleanup_(json_variant_unrefp) JsonVariant *w = NULL; assert_se(i <= UINT16_MAX); @@ -415,6 +432,8 @@ static void test_depth(void) { } static void test_normalize(void) { + log_info("/* %s */", __func__); + _cleanup_(json_variant_unrefp) JsonVariant *v = NULL, *w = NULL; _cleanup_free_ char *t = NULL; @@ -459,12 +478,13 @@ static void test_normalize(void) { } static void test_bisect(void) { + log_info("/* %s */", __func__); + _cleanup_(json_variant_unrefp) JsonVariant *v = NULL; - char c; /* Tests the bisection logic in json_variant_by_key() */ - for (c = 'z'; c >= 'a'; c--) { + for (char c = 'z'; c >= 'a'; c--) { if ((c % 3) == 0) continue; @@ -484,7 +504,7 @@ static void test_bisect(void) { json_variant_dump(v, JSON_FORMAT_COLOR|JSON_FORMAT_PRETTY, NULL, NULL); - for (c = 'a'; c <= 'z'; c++) { + for (char c = 'a'; c <= 'z'; c++) { JsonVariant *k; const char *z; From e60775cb7b634f887cea2c1755ac2c417b804a0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 30 Aug 2020 21:25:12 +0200 Subject: [PATCH 12/14] shared: merge {user,group}-record-nss.{c,h} They both are both short and contain similar parts and various helper will be shared between both parts of the code so it's easier to use a single file. --- src/nss-systemd/nss-systemd.c | 2 +- src/nss-systemd/userdb-glue.c | 2 +- src/shared/group-record-nss.c | 219 ---------------------------------- src/shared/group-record-nss.h | 15 --- src/shared/meson.build | 2 - src/shared/user-record-nss.c | 213 +++++++++++++++++++++++++++++++++ src/shared/user-record-nss.h | 11 +- src/shared/userdb.c | 1 - src/userdb/userwork.c | 1 - 9 files changed, 225 insertions(+), 241 deletions(-) delete mode 100644 src/shared/group-record-nss.c delete mode 100644 src/shared/group-record-nss.h diff --git a/src/nss-systemd/nss-systemd.c b/src/nss-systemd/nss-systemd.c index 5dc5aacdff..0e8c13f7ea 100644 --- a/src/nss-systemd/nss-systemd.c +++ b/src/nss-systemd/nss-systemd.c @@ -6,13 +6,13 @@ #include "env-util.h" #include "errno-util.h" #include "fd-util.h" -#include "group-record-nss.h" #include "macro.h" #include "nss-systemd.h" #include "nss-util.h" #include "pthread-util.h" #include "signal-util.h" #include "strv.h" +#include "user-record-nss.h" #include "user-util.h" #include "userdb-glue.h" #include "userdb.h" diff --git a/src/nss-systemd/userdb-glue.c b/src/nss-systemd/userdb-glue.c index 8e5b3eba6c..2ac299d9a7 100644 --- a/src/nss-systemd/userdb-glue.c +++ b/src/nss-systemd/userdb-glue.c @@ -2,9 +2,9 @@ #include "env-util.h" #include "fd-util.h" -#include "group-record-nss.h" #include "nss-systemd.h" #include "strv.h" +#include "user-record-nss.h" #include "user-record.h" #include "userdb-glue.h" #include "userdb.h" diff --git a/src/shared/group-record-nss.c b/src/shared/group-record-nss.c deleted file mode 100644 index b018a46e18..0000000000 --- a/src/shared/group-record-nss.c +++ /dev/null @@ -1,219 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1+ */ - -#include "errno-util.h" -#include "group-record-nss.h" -#include "libcrypt-util.h" -#include "strv.h" - -int nss_group_to_group_record( - const struct group *grp, - const struct sgrp *sgrp, - GroupRecord **ret) { - - _cleanup_(group_record_unrefp) GroupRecord *g = NULL; - int r; - - assert(grp); - assert(ret); - - if (isempty(grp->gr_name)) - return -EINVAL; - - if (sgrp && !streq_ptr(sgrp->sg_namp, grp->gr_name)) - return -EINVAL; - - g = group_record_new(); - if (!g) - return -ENOMEM; - - g->group_name = strdup(grp->gr_name); - if (!g->group_name) - return -ENOMEM; - - g->members = strv_copy(grp->gr_mem); - if (!g->members) - return -ENOMEM; - - g->gid = grp->gr_gid; - - if (sgrp) { - if (looks_like_hashed_password(sgrp->sg_passwd)) { - g->hashed_password = strv_new(sgrp->sg_passwd); - if (!g->hashed_password) - return -ENOMEM; - } - - r = strv_extend_strv(&g->members, sgrp->sg_mem, 1); - if (r < 0) - return r; - - g->administrators = strv_copy(sgrp->sg_adm); - if (!g->administrators) - return -ENOMEM; - } - - r = json_build(&g->json, JSON_BUILD_OBJECT( - JSON_BUILD_PAIR("groupName", JSON_BUILD_STRING(g->group_name)), - JSON_BUILD_PAIR("gid", JSON_BUILD_UNSIGNED(g->gid)), - JSON_BUILD_PAIR_CONDITION(!strv_isempty(g->members), "members", JSON_BUILD_STRV(g->members)), - JSON_BUILD_PAIR_CONDITION(!strv_isempty(g->hashed_password), "privileged", JSON_BUILD_OBJECT(JSON_BUILD_PAIR("hashedPassword", JSON_BUILD_STRV(g->hashed_password)))), - JSON_BUILD_PAIR_CONDITION(!strv_isempty(g->administrators), "administrators", JSON_BUILD_STRV(g->administrators)))); - if (r < 0) - return r; - - g->mask = USER_RECORD_REGULAR | - (!strv_isempty(g->hashed_password) ? USER_RECORD_PRIVILEGED : 0); - - *ret = TAKE_PTR(g); - return 0; -} - -int nss_sgrp_for_group(const struct group *grp, struct sgrp *ret_sgrp, char **ret_buffer) { - size_t buflen = 4096; - int r; - - assert(grp); - assert(ret_sgrp); - assert(ret_buffer); - - for (;;) { - _cleanup_free_ char *buf = NULL; - struct sgrp sgrp, *result; - - buf = malloc(buflen); - if (!buf) - return -ENOMEM; - - r = getsgnam_r(grp->gr_name, &sgrp, buf, buflen, &result); - if (r == 0) { - if (!result) - return -ESRCH; - - *ret_sgrp = *result; - *ret_buffer = TAKE_PTR(buf); - return 0; - } - if (r < 0) - return -EIO; /* Weird, this should not return negative! */ - if (r != ERANGE) - return -r; - - if (buflen > SIZE_MAX / 2) - return -ERANGE; - - buflen *= 2; - buf = mfree(buf); - } -} - -int nss_group_record_by_name( - const char *name, - bool with_shadow, - GroupRecord **ret) { - - _cleanup_free_ char *buf = NULL, *sbuf = NULL; - struct group grp, *result; - bool incomplete = false; - size_t buflen = 4096; - struct sgrp sgrp, *sresult = NULL; - int r; - - assert(name); - assert(ret); - - for (;;) { - buf = malloc(buflen); - if (!buf) - return -ENOMEM; - - r = getgrnam_r(name, &grp, buf, buflen, &result); - if (r == 0) { - if (!result) - return -ESRCH; - - break; - } - - if (r < 0) - return log_debug_errno(SYNTHETIC_ERRNO(EIO), "getgrnam_r() returned a negative value"); - if (r != ERANGE) - return -r; - if (buflen > SIZE_MAX / 2) - return -ERANGE; - - buflen *= 2; - buf = mfree(buf); - } - - if (with_shadow) { - r = nss_sgrp_for_group(result, &sgrp, &sbuf); - if (r < 0) { - log_debug_errno(r, "Failed to do shadow lookup for group %s, ignoring: %m", result->gr_name); - incomplete = ERRNO_IS_PRIVILEGE(r); - } else - sresult = &sgrp; - } else - incomplete = true; - - r = nss_group_to_group_record(result, sresult, ret); - if (r < 0) - return r; - - (*ret)->incomplete = incomplete; - return 0; -} - -int nss_group_record_by_gid( - gid_t gid, - bool with_shadow, - GroupRecord **ret) { - - _cleanup_free_ char *buf = NULL, *sbuf = NULL; - struct group grp, *result; - bool incomplete = false; - size_t buflen = 4096; - struct sgrp sgrp, *sresult = NULL; - int r; - - assert(ret); - - for (;;) { - buf = malloc(buflen); - if (!buf) - return -ENOMEM; - - r = getgrgid_r(gid, &grp, buf, buflen, &result); - if (r == 0) { - if (!result) - return -ESRCH; - break; - } - - if (r < 0) - return log_debug_errno(SYNTHETIC_ERRNO(EIO), "getgrgid_r() returned a negative value"); - if (r != ERANGE) - return -r; - if (buflen > SIZE_MAX / 2) - return -ERANGE; - - buflen *= 2; - buf = mfree(buf); - } - - if (with_shadow) { - r = nss_sgrp_for_group(result, &sgrp, &sbuf); - if (r < 0) { - log_debug_errno(r, "Failed to do shadow lookup for group %s, ignoring: %m", result->gr_name); - incomplete = ERRNO_IS_PRIVILEGE(r); - } else - sresult = &sgrp; - } else - incomplete = true; - - r = nss_group_to_group_record(result, sresult, ret); - if (r < 0) - return r; - - (*ret)->incomplete = incomplete; - return 0; -} diff --git a/src/shared/group-record-nss.h b/src/shared/group-record-nss.h deleted file mode 100644 index 077c22d89f..0000000000 --- a/src/shared/group-record-nss.h +++ /dev/null @@ -1,15 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1+ */ -#pragma once - -#include -#include - -#include "group-record.h" - -/* Synthesize GroupRecord objects from NSS data */ - -int nss_group_to_group_record(const struct group *grp, const struct sgrp *sgrp, GroupRecord **ret); -int nss_sgrp_for_group(const struct group *grp, struct sgrp *ret_sgrp, char **ret_buffer); - -int nss_group_record_by_name(const char *name, bool with_shadow, GroupRecord **ret); -int nss_group_record_by_gid(gid_t gid, bool with_shadow, GroupRecord **ret); diff --git a/src/shared/meson.build b/src/shared/meson.build index c149ff4cd8..572ac1de64 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -113,8 +113,6 @@ shared_sources = files(''' geneve-util.h gpt.c gpt.h - group-record-nss.c - group-record-nss.h group-record.c group-record.h id128-print.c diff --git a/src/shared/user-record-nss.c b/src/shared/user-record-nss.c index b4c35b8a53..0da77dfc39 100644 --- a/src/shared/user-record-nss.c +++ b/src/shared/user-record-nss.c @@ -290,3 +290,216 @@ int nss_user_record_by_uid( (*ret)->incomplete = incomplete; return 0; } + +int nss_group_to_group_record( + const struct group *grp, + const struct sgrp *sgrp, + GroupRecord **ret) { + + _cleanup_(group_record_unrefp) GroupRecord *g = NULL; + int r; + + assert(grp); + assert(ret); + + if (isempty(grp->gr_name)) + return -EINVAL; + + if (sgrp && !streq_ptr(sgrp->sg_namp, grp->gr_name)) + return -EINVAL; + + g = group_record_new(); + if (!g) + return -ENOMEM; + + g->group_name = strdup(grp->gr_name); + if (!g->group_name) + return -ENOMEM; + + g->members = strv_copy(grp->gr_mem); + if (!g->members) + return -ENOMEM; + + g->gid = grp->gr_gid; + + if (sgrp) { + if (looks_like_hashed_password(sgrp->sg_passwd)) { + g->hashed_password = strv_new(sgrp->sg_passwd); + if (!g->hashed_password) + return -ENOMEM; + } + + r = strv_extend_strv(&g->members, sgrp->sg_mem, 1); + if (r < 0) + return r; + + g->administrators = strv_copy(sgrp->sg_adm); + if (!g->administrators) + return -ENOMEM; + } + + r = json_build(&g->json, JSON_BUILD_OBJECT( + JSON_BUILD_PAIR("groupName", JSON_BUILD_STRING(g->group_name)), + JSON_BUILD_PAIR("gid", JSON_BUILD_UNSIGNED(g->gid)), + JSON_BUILD_PAIR_CONDITION(!strv_isempty(g->members), "members", JSON_BUILD_STRV(g->members)), + JSON_BUILD_PAIR_CONDITION(!strv_isempty(g->hashed_password), "privileged", JSON_BUILD_OBJECT(JSON_BUILD_PAIR("hashedPassword", JSON_BUILD_STRV(g->hashed_password)))), + JSON_BUILD_PAIR_CONDITION(!strv_isempty(g->administrators), "administrators", JSON_BUILD_STRV(g->administrators)))); + if (r < 0) + return r; + + g->mask = USER_RECORD_REGULAR | + (!strv_isempty(g->hashed_password) ? USER_RECORD_PRIVILEGED : 0); + + *ret = TAKE_PTR(g); + return 0; +} + +int nss_sgrp_for_group(const struct group *grp, struct sgrp *ret_sgrp, char **ret_buffer) { + size_t buflen = 4096; + int r; + + assert(grp); + assert(ret_sgrp); + assert(ret_buffer); + + for (;;) { + _cleanup_free_ char *buf = NULL; + struct sgrp sgrp, *result; + + buf = malloc(buflen); + if (!buf) + return -ENOMEM; + + r = getsgnam_r(grp->gr_name, &sgrp, buf, buflen, &result); + if (r == 0) { + if (!result) + return -ESRCH; + + *ret_sgrp = *result; + *ret_buffer = TAKE_PTR(buf); + return 0; + } + if (r < 0) + return -EIO; /* Weird, this should not return negative! */ + if (r != ERANGE) + return -r; + + if (buflen > SIZE_MAX / 2) + return -ERANGE; + + buflen *= 2; + buf = mfree(buf); + } +} + +int nss_group_record_by_name( + const char *name, + bool with_shadow, + GroupRecord **ret) { + + _cleanup_free_ char *buf = NULL, *sbuf = NULL; + struct group grp, *result; + bool incomplete = false; + size_t buflen = 4096; + struct sgrp sgrp, *sresult = NULL; + int r; + + assert(name); + assert(ret); + + for (;;) { + buf = malloc(buflen); + if (!buf) + return -ENOMEM; + + r = getgrnam_r(name, &grp, buf, buflen, &result); + if (r == 0) { + if (!result) + return -ESRCH; + + break; + } + + if (r < 0) + return log_debug_errno(SYNTHETIC_ERRNO(EIO), "getgrnam_r() returned a negative value"); + if (r != ERANGE) + return -r; + if (buflen > SIZE_MAX / 2) + return -ERANGE; + + buflen *= 2; + buf = mfree(buf); + } + + if (with_shadow) { + r = nss_sgrp_for_group(result, &sgrp, &sbuf); + if (r < 0) { + log_debug_errno(r, "Failed to do shadow lookup for group %s, ignoring: %m", result->gr_name); + incomplete = ERRNO_IS_PRIVILEGE(r); + } else + sresult = &sgrp; + } else + incomplete = true; + + r = nss_group_to_group_record(result, sresult, ret); + if (r < 0) + return r; + + (*ret)->incomplete = incomplete; + return 0; +} + +int nss_group_record_by_gid( + gid_t gid, + bool with_shadow, + GroupRecord **ret) { + + _cleanup_free_ char *buf = NULL, *sbuf = NULL; + struct group grp, *result; + bool incomplete = false; + size_t buflen = 4096; + struct sgrp sgrp, *sresult = NULL; + int r; + + assert(ret); + + for (;;) { + buf = malloc(buflen); + if (!buf) + return -ENOMEM; + + r = getgrgid_r(gid, &grp, buf, buflen, &result); + if (r == 0) { + if (!result) + return -ESRCH; + break; + } + + if (r < 0) + return log_debug_errno(SYNTHETIC_ERRNO(EIO), "getgrgid_r() returned a negative value"); + if (r != ERANGE) + return -r; + if (buflen > SIZE_MAX / 2) + return -ERANGE; + + buflen *= 2; + buf = mfree(buf); + } + + if (with_shadow) { + r = nss_sgrp_for_group(result, &sgrp, &sbuf); + if (r < 0) { + log_debug_errno(r, "Failed to do shadow lookup for group %s, ignoring: %m", result->gr_name); + incomplete = ERRNO_IS_PRIVILEGE(r); + } else + sresult = &sgrp; + } else + incomplete = true; + + r = nss_group_to_group_record(result, sresult, ret); + if (r < 0) + return r; + + (*ret)->incomplete = incomplete; + return 0; +} diff --git a/src/shared/user-record-nss.h b/src/shared/user-record-nss.h index 0eb78d5b52..e2a87f664c 100644 --- a/src/shared/user-record-nss.h +++ b/src/shared/user-record-nss.h @@ -1,15 +1,24 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ #pragma once +#include +#include #include #include +#include "group-record.h" #include "user-record.h" -/* Synthesizes a UserRecord object from NSS data */ +/* Synthesize UserRecord and GroupRecord objects from NSS data */ int nss_passwd_to_user_record(const struct passwd *pwd, const struct spwd *spwd, UserRecord **ret); int nss_spwd_for_passwd(const struct passwd *pwd, struct spwd *ret_spwd, char **ret_buffer); int nss_user_record_by_name(const char *name, bool with_shadow, UserRecord **ret); int nss_user_record_by_uid(uid_t uid, bool with_shadow, UserRecord **ret); + +int nss_group_to_group_record(const struct group *grp, const struct sgrp *sgrp, GroupRecord **ret); +int nss_sgrp_for_group(const struct group *grp, struct sgrp *ret_sgrp, char **ret_buffer); + +int nss_group_record_by_name(const char *name, bool with_shadow, GroupRecord **ret); +int nss_group_record_by_gid(gid_t gid, bool with_shadow, GroupRecord **ret); diff --git a/src/shared/userdb.c b/src/shared/userdb.c index 94120862df..57e58a61a9 100644 --- a/src/shared/userdb.c +++ b/src/shared/userdb.c @@ -6,7 +6,6 @@ #include "dlfcn-util.h" #include "errno-util.h" #include "fd-util.h" -#include "group-record-nss.h" #include "missing_syscall.h" #include "parse-util.h" #include "set.h" diff --git a/src/userdb/userwork.c b/src/userdb/userwork.c index d7202099be..a68011b3fc 100644 --- a/src/userdb/userwork.c +++ b/src/userdb/userwork.c @@ -7,7 +7,6 @@ #include "env-util.h" #include "fd-util.h" -#include "group-record-nss.h" #include "group-record.h" #include "io-util.h" #include "main-func.h" From 5c0b7380121e7d2f3556839d48626c903869b378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 30 Aug 2020 20:34:12 +0200 Subject: [PATCH 13/14] user-record-nss: check if strings from pwd/spwd/grp/sgrp are valid utf-8 strv_extend_strv_utf8_only() uses a temporary buffer to make the implementation conscise. Otherwise we'd have to rewrite all of strv_extend_strv() which didn't seem worth the trouble for this one use outside of a hot path. If the data is not serializable, we just pretend it doesn't exists. This fixes #16683 and https://bugs.gentoo.org/735072 in a second way. --- src/shared/user-record-nss.c | 48 +++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/src/shared/user-record-nss.c b/src/shared/user-record-nss.c index 0da77dfc39..d06c8abdbe 100644 --- a/src/shared/user-record-nss.c +++ b/src/shared/user-record-nss.c @@ -6,10 +6,35 @@ #include "strv.h" #include "user-record-nss.h" #include "user-util.h" +#include "utf8.h" #define SET_IF(field, condition, value, fallback) \ field = (condition) ? (value) : (fallback) +static inline const char* utf8_only(const char *s) { + return s && utf8_is_valid(s) ? s : NULL; +} + +static inline int strv_extend_strv_utf8_only(char ***dst, char **src, bool filter_duplicates) { + _cleanup_free_ char **t = NULL; + size_t l, j = 0; + + /* First, do a shallow copy of s, filtering for only valid utf-8 strings */ + l = strv_length(src); + t = new(char*, l + 1); + if (!t) + return -ENOMEM; + + for (size_t i = 0; i < l; i++) + if (utf8_is_valid(src[i])) + t[j++] = src[i]; + if (j == 0) + return 0; + + t[j] = NULL; + return strv_extend_strv(dst, t, filter_duplicates); +} + int nss_passwd_to_user_record( const struct passwd *pwd, const struct spwd *spwd, @@ -55,18 +80,19 @@ int nss_passwd_to_user_record( free_and_replace(hr->real_name, mangled); } - r = free_and_strdup(&hr->home_directory, empty_to_null(pwd->pw_dir)); + r = free_and_strdup(&hr->home_directory, utf8_only(empty_to_null(pwd->pw_dir))); if (r < 0) return r; - r = free_and_strdup(&hr->shell, empty_to_null(pwd->pw_shell)); + r = free_and_strdup(&hr->shell, utf8_only(empty_to_null(pwd->pw_shell))); if (r < 0) return r; hr->uid = pwd->pw_uid; hr->gid = pwd->pw_gid; - if (spwd && looks_like_hashed_password(spwd->sp_pwdp)) { + if (spwd && + looks_like_hashed_password(utf8_only(spwd->sp_pwdp))) { /* Ignore locked, disabled, and mojibake passwords */ strv_free_erase(hr->hashed_password); hr->hashed_password = strv_new(spwd->sp_pwdp); if (!hr->hashed_password) @@ -316,26 +342,26 @@ int nss_group_to_group_record( if (!g->group_name) return -ENOMEM; - g->members = strv_copy(grp->gr_mem); - if (!g->members) - return -ENOMEM; + r = strv_extend_strv_utf8_only(&g->members, grp->gr_mem, false); + if (r < 0) + return r; g->gid = grp->gr_gid; if (sgrp) { - if (looks_like_hashed_password(sgrp->sg_passwd)) { + if (looks_like_hashed_password(utf8_only(sgrp->sg_passwd))) { g->hashed_password = strv_new(sgrp->sg_passwd); if (!g->hashed_password) return -ENOMEM; } - r = strv_extend_strv(&g->members, sgrp->sg_mem, 1); + r = strv_extend_strv_utf8_only(&g->members, sgrp->sg_mem, true); if (r < 0) return r; - g->administrators = strv_copy(sgrp->sg_adm); - if (!g->administrators) - return -ENOMEM; + r = strv_extend_strv_utf8_only(&g->administrators, sgrp->sg_adm, false); + if (r < 0) + return r; } r = json_build(&g->json, JSON_BUILD_OBJECT( From ceaf24d4d3cbc84fc00687a2027fdb67fe61ab5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 31 Aug 2020 13:01:23 +0200 Subject: [PATCH 14/14] TODO: add entry --- TODO | 3 +++ 1 file changed, 3 insertions(+) diff --git a/TODO b/TODO index e25ca89510..6e877a9230 100644 --- a/TODO +++ b/TODO @@ -4,6 +4,9 @@ Bugfixes: manager or system manager can be always set. It would be better to reject them when parsing config. +* userdbctl: "Password OK: yes" is shown even when there are no passwords + or the password is locked. + External: * Fedora: add an rpmlint check that verifies that all unit files in the RPM are listed in %systemd_post macros.