From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 18937 invoked from network); 12 Oct 2021 11:02:40 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 12 Oct 2021 11:02:40 -0000 Received: (qmail 18226 invoked by uid 550); 12 Oct 2021 11:02:36 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 11856 invoked from network); 12 Oct 2021 00:38:45 -0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=eteau4a26zmxk2ijau6sgb3me4huyrhc; d=openwall.com.au; t=1633999101; h=Date:From:To:Subject:Message-ID:References:MIME-Version:Content-Type:In-Reply-To; bh=lPz+89hjbc9yOvtlhHoW+VqUE1/qkbB9mC425kD+GxA=; b=h7QKff7+yUY4x88a1rIicCcIpP9NA81Qxqlkvm1meSwdxv4/iUh4mL9TeNb7EBx5 cz+7GxGvgR7mZa8NtKE84Dwz3OYxUwBsCjOpneUSvdNUkywSBdIsaybKIBkHV91COv/ 3JYcs5rtW1ovyk5af8YgIyqifk+ue3hHrzo2noR0= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=ug7nbtf4gccmlpwj322ax3p6ow6yfsug; d=amazonses.com; t=1633999101; h=Date:From:To:Subject:Message-ID:References:MIME-Version:Content-Type:In-Reply-To:Feedback-ID; bh=lPz+89hjbc9yOvtlhHoW+VqUE1/qkbB9mC425kD+GxA=; b=Uo/jV+CelFIcUVR8bAvxCei1e5qR6Pmsm6Qw6VorDgu+KBoGzd2sH59aoD6Kxjvq X0jVflKNuBPbOzaSi0TOHbnSmweSRk/6KRFvUY0ruOLLgNZC0qtClNCAn+eO+GvmmMy RxdLxRccaZozcDNnj4RYg0qV1ApjvncFQIWc/u9I= ARC-Seal:i=1; a=rsa-sha256; d=openwall.com.au; s=20180402; t=1633999101; cv=none; b=ozBfyZwHIjotE+VG92bCEKWkOaA/uXNc6Hi/jAbM+AfbIqujuzdakT/hNGSA6Z8Au4RyhXtYnn/Jz0BmpmVUvOmoLsXfMsjh+JZ0CDhQ+MSzm9g/u0qxtBvbq1VgAz/I+CaBdEjNMZmS4NYjoRfDo9Wp7WlX+/PvKmO2wFYTqbzxuQDRsPJ1yCWnusJrEsfF+Thh/mLo1ole2s81+Iw8GJkJx0a5vmaXrJyO//6nf1aRrL8E/srI1pQt47h+d8lS+6vjOhabYrrOJ6YBEM/ta1qyCl5CZoC0B2SFC9Gh+b7FtYjxA6J90oi/ROaG7cPFFRffTzgW0K+fgzooUTKllw== ARC-Message-Signature:i=1; a=rsa-sha256; d=openwall.com.au; s=20180402; t=1633999101; c=relaxed/simple; bh=lPz+89hjbc9yOvtlhHoW+VqUE1/qkbB9mC425kD+GxA=; h=From:To:Subject; b=LUBJ5th75tMxWGYVQ5gE/42rM7w4dUmHfaq41thYMjfBk0qHAu0ZforJd/4RYAVpX2RkD9RxzBfvtDXunMUw7HkTawjEG751mlQXlJJAZhpPUq4vB4ta3f/I7LSCCdoOPlIPQpjq5jtHr6ToLUN6K7gPtBCntLWm3q7sFr4zXQY5GC/9EfJhcdd3fgLpddn32wl1Ktrs9wByjBHf9McQJaQ4jSJKqUAkUFfJYVrhL9HgJsipo5mviN6ve25/DyyR/y7ahy+mnJ9oun+d2/Bu5+93PRG0TFBmdl+gFmPSMm6jXK6/1UE2H2KmcZxSHsezqqWS5f6Pagq/5QRb8sYGiA== ARC-Authentication-Results:i=1; mail.openwall.com.au; none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=openwall.com.au; s=20180402; t=1633999101; bh=lPz+89hjbc9yOvtlhHoW+VqUE1/qkbB9mC425kD+GxA=; h=From:To:Subject:From; b=MWjJpdNwNQBouzYU0QifJTvWWDUHVLgUNmt1/r+XOobXsORmzmAb6Xa3GxGLKaFKM U0pbEoin9Pv2OVG35Awbx/s7tCkXZZWFAKgvff2eX4H7yfs7oyjrDWQMQYgXzMdTN+ f5weaDXkV7buOBaOolleMIxieZv6g3GxsR09Euufrf3XalXABlkP6qa0c1iAR2T6QI KUeYsG9nOxbvEmz4OL1G4lu1MY2lyfgnxaCI9tngYlOQu5NDsAValnsfdENm2sYMeQ Vveuf9RGtbzng6xDy9EZetphEWHkZw1T6ZYIZyL1BieJz1Y7OCIAg/x8t/ssza6uyH C8m5H4KHKrCwA== Date: Tue, 12 Oct 2021 00:38:21 +0000 From: "(GalaxyMaster)" To: musl@lists.openwall.com Message-ID: <0100017c71ef9e36-29a22d3f-22a5-495a-9d9e-0eac76c3ff4e-000000@email.amazonses.com> References: <0100017c6f8d9905-d09f62b3-323a-4133-bcad-e27204cfd62c-000000@email.amazonses.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Feedback-ID: 1.us-east-1.Br0WpcLm0XzNPYp+t39aA5qSwb/HYCx3zC5wkQY3G2s=:AmazonSES X-SES-Outgoing: 2021.10.12-54.240.8.29 Subject: Re: [musl] get/set*ent functions and real world applications Enrico, On Mon, Oct 11, 2021 at 02:18:47PM -0300, ??rico Nogueira wrote: > On Mon Oct 11, 2021 at 10:32 AM -03, (GalaxyMaster) wrote: > > > > The whole parssing of password and group entires is quite "dumb", in > > terms that > > it always expects perfectly valid input and produces always correct > > output, but > > I would argue that these functions should be a bit smarter and should > > not make > > assumptions about the validity of the input, i.e treat it as untrusted > > user > > input. > > There's a reason it's recommended that one only make changes to these > files using tools like the ones from the shadow suite. Things in /etc > can, theoretically, only be written to by root or at least trusted > users, so treating as entirely untrusted seems a bit over the top... Well, I prefer to treat anything external to my application as untrusted input, this saves time on troubleshooting weird issues later. In your statement above you put implicit trust into these abstract "trusted users", why guess what these could or could not do if we can handle the input in a safe and deterministic way? There was an argument on this list that libc should not treat/hide developer's mistakes and that's the reason you may see segfaults on a musl-based system more often (it is less forgiving to the poorly written code). Hoowever, I would argue that this particular case requires special treatment from libc since there is no way avoiding these functions if you are working with passwd/group based files. > It seems like both libraries are inconsistent in their own ways. glibc > skips malformed entries when some fields are missing, but fixes a > missing supplemental group entry. musl skips a missing supplemental > group entry, but "fixes" malformed entries with fields missing. > > Maybe striving for consistency by either always skipping or always > fixing entries seems like a more reasonable choice to me, maybe? I am not defending Glibc, but I find their approach to this matter consistent and expected from the common sense point of view. I would rather see musl aligned with it than try to convince everyone of yet another way of doing this. > > With put*() functions the situation a bit better, but still could be > > improved to achieve better compatibility. putpwent() will output > > '(null)' for any NULL pointer passed for the string arguments (due to > > direct call to fprintf() and the UB for that situation), while Glibc > > would output an empty string instead. > > It would seem the function returns EINVAL instead of outputting anything > at all, from my look at the code. I think that's reasonable behavior for > musl to implement, given how badly specified the function is. No, it is not behaving like that, it behaves exactly as I described: === galaxy@musl:~/musl-tests $ cat test-putpwent.c #include #include #include #include #include int main() { struct passwd *pw; FILE *fp; errno = 0; fp = fopen("test-putpwent.output", "w"); if (!fp || errno != 0) return errno; pw = malloc(sizeof(struct passwd)); pw->pw_name = pw->pw_passwd = pw->pw_gecos = pw->pw_dir, pw->pw_shell = NULL; putpwent(pw, fp); fclose(fp); return 0; } galaxy@musl:~/musl-tests $ ls -ld test-putpwent.output ls: cannot access 'test-putpwent.output': No such file or directory galaxy@musl:~/musl-tests $ ./test-putpwent galaxy@musl:~/musl-tests $ cat test-putpwent.output (null):(null):0:0:(null):(null):(null) galaxy@musl:~/musl-tests $ === On a Glibc system, the putpwent() call with pw->pw_name being NULL will fail to produce a record. If the pw->pw_name field is not NULL, then it will produce a record with the name followed by empty string fields, like "user::0:0:::", which is more expected in my opinion. > > Moreover, putpwent() is inconsistent with putgrent() -- the latter > > locks the file before writing and unlocks afterwards, while the former > > is just going ahead with fprintf(). I know these funnctions are thread > > unsafe, but this lack of locking makes putpwent() plainly dangerous on > > a multiuser system. > > I'm pretty sure these functions are always dangerous on multiuser > systems; flockfile(3) is FILE level locking, it just protects from other > threads touching a given FILE object. If you want to protect yourself > from multiple programs handling the file simultaneously, you need file > locking, such as done with fcntl(2). A fair point, I was not familiar with that function and somehow thought it was at the file level. I think that it is not libc's job to do file lockingi here, so I think we are fine in this regard. Thank you for explaining why there is an inconsistency, between putpwent() and putgrent() -- it makes sense. -- (GM)