Friday, December 03, 2021

Re: [new] devel/libfixposix

Woops, I identified a correct issue in unistd.c, but the fix was wrong.
Updated tarball attached and upstream notified :)

(update: upstream has accepted one of the two patches)

Also enabling the built-in tests this time, I haven't noticed them the
first time because without --enable-tests `make test' was successful and
listed 0 tests available. (The mkstemp test fails because why check
what malloc returns?)

Omar Polo <op@openbsd.org> writes:

> Ingo Schwarze <schwarze@usta.de> writes:
>
>> Hi Omar,
>
> Hello Ingo,
>
>> Omar Polo wrote on Fri, Dec 03, 2021 at 09:07:02AM +0100:
>>
>>> % pkg_info libfixposix
>>> Information for inst:libfixposix-0.4.3
>>>
>>> Comment:
>>> thin wrapper over POSIX syscalls
>>>
>>> Description:
>>> The purpose of libfixposix is to offer replacements for parts of POSIX
>>> whose behaviour is inconsistent across *NIX flavours.
>>
>> Without looking at the code:
>> This sounds totally scary to me.
>>
>> Wouldn't it be better to provide a dummy instead that just passes
>> the calls through?
>>
>> I mean, decisions about OpenBSD deviating from POSIX are usually
>> made for a reason, aren't they? For example, the first things
>> coming to my mind are rand(3) and gets(3) and printf(%n), and i feel
>> certain there are several more.
>>
>> Software depending on POSIX behaviour that we intentionally disabled
>> should not just patch it back, right? I think it is better to have
>> such software fail to build (or even, admittedly less conveniently,
>> crash at run time).
>>
>> Then, the actual problems can be investigated and fixed properly.
>>
>> Wouldn't this port essentially implement security mitigation
>> countermeasures, as the famous saying by tedu@ goes?
>>
>> This is not an objection, just a question.
>
> I agree with your sentiment, and in fact I was a bit scared too the
> first time I heard of the library, but upon a further look it turns out
> not to be the case fortunately.
>
> What this library does is wrapping most of the functionality of libc, in
> particular macros and constants, behind functions calls. This is needed
> for some higher level language where is impossible or particularly
> cumbersome to access those. One example may be src/lib/select.c:
>
> 55 DSO_PUBLIC void
> 56 lfp_fd_set(int fd, fd_set *set)
> 57 {
> 58 FD_SET(fd, set);
> 59 }
> 60
> 61 DSO_PUBLIC void
> 62 lfp_fd_zero(fd_set *set)
> 63 {
> 64 FD_ZERO(set);
> 65 }
>
> One may argue that it's the fault of the language that doesn't provide
> such wrapper -or equivalent- by itself, and I would agree, but
> unfortunately that's the case. Functions like those allows, for
> example, a Common Lisp program to use select(2).
>
> Most of the content of the library is just wrapping functions from libc
> for (apparently) no reason at all. Take src/lib/resource.c for example:
>
> 27 DSO_PUBLIC int
> 28 lfp_getrlimit(int resource, struct rlimit *rlim)
> 29 {
> 30 return getrlimit(resource, rlim);
> 31 }
>
> So there's nothing like a rand(3) implementation or a different
> printf(3) that allows %n.
>
> I did, however, sit down and read the whole thing from top to bottom
> (the first time I did only a generic scan.) It's not what I would call
> a pretty library, but I think it's innocuous at worst. I zapped a
> "mkostemp NIH" which is, I think, the most grave offender. (the second
> place goes to the "almost" posix_spawn rewrite in src/lib/spawn.c, but
> it's slightly different from posix_spawn and would avoid to touch that
> if possible.)
>
> I'm also starting to think that DESCR needs some tweaking because even
> if that's how upstream describes it, it's not really correct. True,
> there is some platform-dependent code (in particular for macos, but also
> a dummy sendfile(2) for example) but it's still misleading, and your
> mail convinced me of that. pkg/DESCR now reads
>
>> libfixposix provides some of the functionalities from libc behind
>> function calls thus allowing programs written in higher level
>> languages to access things that are usually hidden behind macros or
>> defines.
>
> and the comment is
>
>> thin wrapper over libc functions and macros
>
> I hope it would clear some doubts, but I'm open to improvements (as you
> may have already noted, I'm not a skilled writer ;-)
>
>>> Maintainer: The OpenBSD ports mailing-list <ports@openbsd.org>
>>
>> For such a scary beast, shouldn't there at least be a maintainer
>> who does basic auditing of the library, making sure that it does
>> not cause a security disaster, and making sure that future versions
>> do not introduce vulnerabilities either?
>>
>> To me, this feels like sending a Mastiff to promenade alone in the
>> park instead of leading it on a leash?
>
> I agree on this too, and if nobody objects I'll take MAINTAINER on this
> port.
>
> I'm attaching an updated tarball which fixes a couple of things I
> discovered upon closer look. I'll also try to upstream the patches.
>
>> Yours,
>> Ingo
>
> Thanks!
>
> Omar Polo
>

No comments:

Post a Comment