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
OpenPGP_0xB288B55FFF9C22C1.asc
Description: application/pgp-keys
OpenPGP_signature
Description: OpenPGP digital signature
