On 6/9/21 10:34 AM, Gilles CHEHADE wrote:
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/
Yes, this was very helpful and was also where I started from :D
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.
Ah, this makes it a lot more clearer on what should be done. Now I can
work towards this in a better fashion.
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 might be hard to concurrently with having table_proc present, as it
does exactly this - use table_proc backend if the backend name is not
present in the table names.
So what I could do is to remove table_proc and introduce table_procexec
and simultaneously update the opensmtpd-extras port to use
table_procexec API.
Does that sound like a feasible approach?
I'll send an updated diff soonish.
Cheers,
Aisha
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
