> On 9 Jun 2021, at 15:47, Aisha Tammy <openbsd.t...@aisha.cc> wrote:
>
> On 6/9/21 5:19 AM, Gilles CHEHADE wrote:
>> Hi,
>>
>> I wrote table_procexec (despite the copyright which I copy-pasted and forgot
>> to replace author) so just providing a bit of insight:
> Ah, I did not know that. I will fix the header in the next patch
No problem, here’s a write-up for reference:
https://poolp.org/posts/2020-05-28/may-2020-opensmtpd-6.7.1p1-release-table-procexec-and-many-pocs/
>> table_procexec was written as a proof of concept for a new table protocol
>> inspired by the filter protocol to make it easier to write privsep table
>> backends using any language or library available without requiring
>> dependencies in the daemon.
>>
>> I implemented it as a table backend because it was the easiest way to show a
>> working implementation of the protocol without making changes in the daemon,
>> the table_procexec backend sits in between the daemon and “new” tables to
>> translate old protocol queries into new protocol queries and new protocol
>> results into old protocol results,
>> but the intent was to move the underlying implementation of the table API to
>> the protocol and not retain this table proxy.
> Do you mean that ALL table backends should be replaced by the new model? Yes,
> it should be possible to do that but I don't know if that should be done
> simultaneously as introducing procexec.
Builtin tables within the daemon do not need an update because they are built
in the lookup process and do not rely on IPC.
Table backends from opensmtpd-extras would all need an update, yes, but the
change only affects the protocol and not the interface so the change doesn’t
happen in the backends but in the api library that abstracts communication with
the daemon, backends just need to be relinked to the new library, since the
opensmtpd-extras are expected to match specific opensmtpd releases (no
backwards compatibility) and they are rebuilt whenever a package is created,
this isn’t as big and hurtful as it looks.
I agree that maybe this should not be done simultaneously to introducing
procexec but I still don’t think the way it is introduced here is the proper
way:
Ultimately you want to be able to do:
table foobar mytable:/etc/smtpd/mytable.conf
and not be aware that there’s a table-procexec or a procexec protocol ensuring
communication with the daemon.
The fact that you introduce “procexec” in a way that requires it to appear in
the config will make this very hard to hide/remove/change in the future as
people will start relying on it.
Maybe the simplest way is to temporarily introduce procexec as a builtin table
backend, like you did, but do the necessary work in parse.y and table.c to
detect that a table backend in smtpd.conf is meant to be executed by
table-procexec so the daemon can do it transparently.
This way, if or when the table API relies on the procexec protocol instead of
the old protocol, the table-procexec layer can be removed and people will not
see a difference.
>> Not my call but I don’t think what you’re proposing is the proper way to
>> integrate it.
>>
>> There are a few downsides in plugging it this way but the most annoying one
>> is that instead of starting a table with a configuration parameter, this is
>> tricking it into executing the table_procexec and using the configuration
>> path as the path to the “new” table, which means that you have no way to
>> provide the new table with a configuration for itself.
> I don't know if this is true. I was able to provide parameters to the
> procexec executable via
>
Yes, it works (and is how I did my testing), but this is kind of abusing the
mechanism it is built upon.
In theory you should provide the backend + a configuration file path:
table aliases aliases_procexec:/path/to/config
not:
table aliases "proc-exec:/usr/local/bin/aliases_procexec
root.t...@bsd.ac"
What you did works but is not how it is intended to work and will be at odds
with other backends that expect a config file or a table mapping.
I think your diff is very close to something nice but it lacks some adaptations
in parse.y / table.c to hide the plumbing of table_procexec which should remain
invisible to the user IMO.
This is just my 2cts, maybe others have a different opinion on this of course
Gilles
