> On 15. Oct 2020, at 16:32, Demi M. Obenour <[email protected]> wrote: > > On 10/15/20 12:48 AM, Demi M. Obenour wrote: >> On 10/14/20 7:31 PM, [email protected] wrote: >>> October 14, 2020 11:31 PM, "Demi M. Obenour" <[email protected]> wrote: >>>> That makes sense. I will make a separate PR that just has some >>>> automake fixes. >>>> >>> >>> perfect >> >> I filed https://github.com/OpenSMTPD/OpenSMTPD/pull/1093. That’s the >> biggest diff in the whole series. >> >>>>> 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. >>>> >>> >>> To be honest, I haven't looked at the diff yet as it was a bit too big >>> and slowly catching up with the hundreds of mails I'm late reading. >> >> Sorry about that, and thanks for the quick response. The individual >> commits are much easier to review separately, but that isn’t >> necessarily obvious from the GitHub UI. >> >> https://github.com/OpenSMTPD/OpenSMTPD/commit/115d6177adb008e45dd20bf716f576266237b96a >> is a trivial fix to error handling in mail.lmtp. >> >> https://github.com/OpenSMTPD/OpenSMTPD/commit/9cc9a02c2d10dd76c615ad8d228ca80c8cd68429 >> is probably the most controversial diff, and I would greatly >> appreciate if you could review it before I send it off to tech@. >> It avoids spawning a shell for built-in MDAs, instead using execle(). >> This is a security, performance, and correctness improvement, and >> fixes GitHub issue #921. The trade-off is that it adds a small >> amount of special-case code for each built-in delivery method, and >> skips MDA wrappers for them. In my use-case, this is a good thing, >> but I understand if others disagree. >> >> https://github.com/OpenSMTPD/OpenSMTPD/commit/dea15ef7dad1e60a3e3c2fb3bb7d30dc8413576e >> rejects empty maildir and LMTP paths when parsing the configuration >> file, to avoid more confusing errors during delivery. >> >> https://github.com/OpenSMTPD/OpenSMTPD/commit/f99c69644788b7d0d6a02a33884e1285347c018d >> makes the MDA command available (via the `MDA` environment variable) >> to MDA wrappers. Using the %{mda} expansion quickly turned into a >> quoting nightmare that I didn’t want to deal with. >> >> https://github.com/OpenSMTPD/OpenSMTPD/commit/b10863e4a9e8d3899db182c51122e8a2aebafb3c >> redoes getpwnam lookups in the parent process, to protect against >> receiving spoofed values from an unprivileged (and therefore less >> trusted) child process. >> >> Finally, >> https://github.com/OpenSMTPD/OpenSMTPD/commit/eebff03ea7226272fa45191c953772abcefff9aa >> fixes some type confusion and shell injection bugs I found. The type >> confusion bugs involve imsg processing, and could only be exploited >> if an OpenSMTPD process has already been compromised. Therefore, >> I don’t consider them to be security holes. The shell injection >> bugs aren’t exploitable by an attacker, but they do cause incorrect >> behavior in certain cases. >> >> Thank you so much for your time and effort. I have been very >> pleasantly surprised by your quick and thorough responses. >> >> Sincerely, >> >> Demi > > The above URLs have the wrong repository name, sorry. The correct > repository is https://github.com/DemiMarie/OpenSMTPD.
As mentioned by Gilles earlier, please send them as unified diffs to [email protected] <mailto:[email protected]> if you want to have them reviewed.
