Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request provides the correct fix for signal 6 and 10 crashes we discussed in pull req #203. The main issue was that LibreSSL versions up to v2.7.4 behave like OPENSSL_VERSION_NUMBER < 0x1000200fL, beware not just OPENSSL_VERSION_NUMBER < 0x10100000L.
SSLproxy with a similar patch is running for days without any crash on OpenBSD. Looking at the code enabled/disabled by these modified directives for libressl I think that they are quite related with the crashes we discussed in pull req #203.
What convinces me even further that this patch fixes the crashes I was having in the last couple of weeks is that I had most (not all) of those directives in the sources up until August 3rd and never had any crash for a year until that date. I had them since OpenBSD 5.9, but decided to remove them because 6.3 did not produce any errors or warnings without them. Obviously, I was wrong.
I should note that these crashes were not about the previous LibreSSL version 2.7.3, or having LibreSSL and OpenSSL installed on the same system, or an issue/corruption/async in/between the systems. I have rebuilt the systems, have only LibreSSL, and initially was able to reproduce the crashes with LibreSSL 2.7.4 too.
I have also tested adding the following lines in ssl.h, instead of adding new macros to many directives:
I'd prefer this solution, because it is quite simple and achieves the same result. But with this solution, I also have to hush OpenSSL version mismatch warnings for LibreSSL, which may not be desirable, unless I can fix those warnings for LibreSSL instead of silencing them.
Beside my practical tests, the version choice of 0x10002000L makes theoretical sense too, because according to libressl-portable README "LibreSSL is API compatible with OpenSSL 1.0.1, but does not yet include all new APIs from OpenSSL 1.0.2 and later. LibreSSL also includes APIs not yet present in OpenSSL. The current common API subset is OpenSSL 1.0.1".
Since there is no difference between sslsplit and sslproxy code affected by this patch, imo the same issue must exist in sslsplit (hence this pull request). Note again that without this patch, sslproxy builds fine and also runs fine for a while, but it eventually crashes on OpenBSD (LibreSSL). So I think that if sslsplit is left running long enough, it too will crash on OpenBSD.
My only concern with this patch is that this is OpenBSD and/or LibreSSL specific. SSLproxy on UTMFW cannot run without it, so I'll always have it in my project. But since this patch clutters the sources even further (different OpenSSL versions do that already), perhaps it should be part of the OpenBSD sslsplit port instead. Btw, the patches in the OpenBSD port for sslsplit 5.2 add libressl macros to a couple of directives, but they are not enough to fix the crashes I'm having (which I have tested with sslproxy). On the other hand, perhaps some of the libressl macros added in this patch are redundant, not needed to fix those crashes. But as it is hard to test it, I don't know which ones they are, if any.
As for the weird stack traces with core dumps, I think it was because I was using gdb with sslproxy binaries compiled using clang. Since release(8) on OpenBSD does not build lldb by default (but I can see that its code is available in the sources), i.e. lldb is not available on OpenBSD 6.3 release, I couldn't try and see if lldb gives meaningful traces. I should try to build it myself in the future (so yes, I still don't know what triggers the crashes).
So, @droe I trust your judgment to decide whether to merge this pull request or not.