On 10/14/20 3:18 PM, [email protected] wrote: > October 12, 2020 10:06 PM, "Demi M. Obenour" <[email protected]> wrote: > >> I created https://github.com/OpenSMTPD/OpenSMTPD/pull/1087, which >> fixes some bugs and avoids spawning shells when it isn’t necessary >> to do so. Should I split it up into multiple smaller PRs? Also, >> it has only been tested on Linux. >> > > Hello, > > First of all, you should definitely split diffs into multiple ones so > they can be evaluated separately otherwise they are too big to review > and can only be rejected as a whole. I personally can't spend time to > review big diffs at the moment while I could probably review a lot of > smaller ones. > > You should also send the diffs to OpenBSD tech@ list because Github's > commits and pull requests are only done for portability purposes. The > official repository for OpenSMTPD is OpenBSD. The only things that'll > be merged from Github pull requests are ones that make changes out of > smtpd/ subdirectory, or inside smtpd/ subdirectory when it fixes some > build issues on other systems. Any other change should go through the > OpenBSD developers via diffs sent to tech@.
That makes sense. I will make a separate PR that just has some automake fixes. > I do look at the bug tracker still but will only handle portable bits > as I no longer commit to OpenBSD. Do the basic ideas look good? Fixing type confusion bugs in imsg handling is definitely a good thing, but I am less sure about the rest of the changes. My goal is to limit the damage that can be done by a compromised unprivileged process, but it adds some complexity. Specifically, my changes create a distinction between delivery methods specified in smtpd.conf and those specified in aliases or forward files. In the first case, my changes cause OpenSMTPD to avoid spawning a shell, instead directly executing the command with execle(). This is faster, but means that MDA wrappers no longer run. In the second case, however, MDA wrappers will continue to run. I plan on then using a custom MDA wrapper to ensure ensure that only allowlisted MDAs can run. To make this safe, I also make PROC_PARENT re-lookup user information using getpwnam(). Postfix takes a different approach ― its local(8) service is called once for each user that it needs to deliver to. It reads ~/.forward files and then executes commands accordingly. I prefer this approach, but it is a much larger refactor. > Gilles Demi
OpenPGP_0xB288B55FFF9C22C1.asc
Description: application/pgp-keys
OpenPGP_signature
Description: OpenPGP digital signature
