Re: View on pledge promise dynamic string creation

Claudio Jeker wrote on 09.07.2026 19:47:
> On Wed, Jul 08, 2026 at 08:47:34PM +0200, Claudio Jeker wrote:
>> On Wed, Jul 08, 2026 at 07:43:44AM +0100, Thomas Kupper wrote:
>>> Hi,
>>>
>>> I was wondering what are the views on having the pledge(2) promise assembled
>>> (in a 3rd party app). Scanning through base and ports, I could only find
>>> pledge(2) promises as constant string argument. I did assume that it's the
>>> safest way to make sure that the promise can't be fiddled with.
>>>
>>> Since the app (Chrony) was not designed with pledge in mind, if became more
>>> readable to assemble the promise string like:
>>>
>>> (https://gitlab.com/chrony/chrony/-/commit/0ea7607358cc)
>>> --- upstream commit 0ea76073 ---
>>>
>>>    if (snprintf(promises, sizeof (promises), "stdio%s%s%s%s",
>>>                 needs_main_misc ? " rpath wpath cpath inet unix dns" : "",
>>>                 needs_recvfd ? " recvfd" : "",
>>>                 needs_sendfd ? " sendfd" : "",
>>>                 needs_settime ? " settime" : "") >= sizeof (promises))
>>>      assert(0);
>>>
>>>    DEBUG_LOG("Pledging: %s", promises);
>>>
>>>    if (pledge(promises, NULL) < 0)
>>>      LOG_FATAL("pledge() failed");
>>>
>>> ---
>>>
>>> Originally I stuck to the constant strings and Miroslav optimized it after
>>> another change would have resulted in more complex if/else constructs.
>>>
>>
>> We use static strings because they are easier to review.
>> Also the strings all use the same order to again help review of pledge
>> usage. Adding new capabilities to a pledge promise requires a review of
>> all current usage of that promise to understand the impact. Having all
>> pledge calls be the same style makes this a lot easier.
>>
>> It is much harder to argue about your pledge above since it is unclear what
>> will end up in promises without code inspection. In this case it may not
>> matter since chrony is not part of base and ports pledge usage is normally
>> not checked in the above case.
>>
>> Btw. assert(0) as a way to fail after snprintf() failure is special. If
>> compiled with -DNODEBUG the error is just ignore.
> 
> As small follow-up. It is perfectly fine to call pledge multiple times and
> refine the promises over and over again.
> 
> If you look at e.g. usr.bin/nc/netcat.c you can see that the program uses
> a few check to get a initial starting promise but then later on more
> pledge calls are made especially once the the code enters the actuall
> process loop. There is no need to build up the perfect string in a single
> place, in some cases it may be easier to adjust the pledge at a few key
> places.

I understand and Shaun Ren (the original Author of the OpenBSD patch) 
did some #def-ing to add pledge and with that approach you can place it 
where necessary.

But Chrony has only some entry points for defining OS specific 
behavior/functions, all of them in sys_openbsd.[ch]. For example there 
is one for dropping privileges and another for setting system call 
filters. And I did stick to these for upstream OpenBSD support in 
Chrony. And sthen (rightfully) suggested to get OpenBSD support 
upstream-ed instead of a bigger patch for OpenBSD ports.

Similar, I don't now how to implement unveil(2) while adhere to the rules.