Re: [squid-dev] [PATCH] Bug 4662 adding --with-libressl build option
On 02/01/2017 08:20 AM, Marcus Kool wrote: >> Do you think we can compromise and call it USE_OPENSSL_OR_LIBRESSL ? > or call it USE_OPENSSL_API > > and then the code will eventually have none or few occurrences of > USE_OPENSSL and USE_LIBRESSL to deal with OpenSSL and LibreSSL specifics. Yes, and I have suggested this natural solution before (and again a few minutes ago). The correct decision making algorithm is really simple if you start from the right end: 1. Agree to continue to use a single primary SSL guard macro. Ignore its exact spelling of that macro for now. Agree that it _means_ that Squid is configured to use an OpenSSL API (which has many implementations/flavors). 2. Discuss weather --with-libressl is necessary or there is a better way to detect LibreSSL presence. 3a. If --with-libressl is necessary, rename USE_OPENSSL to USE_OPENSSL_API and add code to set that renamed macro. ./configure will set USE_OPENSSL and USE_LIBRESSL, of course. 3b. Otherwise, do not rename USE_OPENSSL. ./configure will continue to set it when building with OpenSSL or LibreSSL libraries, of course. 4. Add code to fix bug 4662. Alex. ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] [PATCH] Bug 4662 adding --with-libressl build option
Executive summary: * Still no agreement on whether or how to rename the primary SSL guard. * Possibly an agreement to continue using a single primary SSL guard?? * Clarification that --with-libressl itself is a relatively minor issue. * A firm veto on adding support for the 3rd SSL API. Whether that veto applies to bug 4662 depends on the fix, but a correct fix will not add support for the 3rd API IMO. Details below. On 02/01/2017 06:51 AM, Amos Jeffries wrote: > Do you think we can compromise and call it USE_OPENSSL_OR_LIBRESSL ? I would not view that name as a compromise: It still enumerates specific libraries instead of naming the API used by the code it protects. You can try to argue that it avoids code duplication but the value of this literal/meaningless avoidance is very small. In other words, this solution would preserve 95% of the bad properties of the original proposal while giving me a superficial 5% satisfaction of removing spaces from (a || b). [rant]Compromises in design and development are rarely a good idea. Two correct designs are rarely on the same curve. Adding them up and dividing by two usually gives a solution that is worse than either of the alternatives. Please do not misinterpret this general statement as an implication that there are two correct designs here; I have not seen them (or I would not be objecting).[/rant] I would like to capitalize on your willingness to use a single macro. This macro should name the associated API. I think it is a bad idea to call the associated API "OpenSSL or LibreSSL" when we know that * there are other libraries that implement the same API (leading to USE_OPENSSL_OR_LIBRESSL_OR_FOO_OR_BAR) * LibreSSL provides more than one API (leading to USE_OPENSSL_OR_LIBRESSL_PART_PROVIDING_OPENSSL_API) If we do not add --with-libressl, then USE_OPENSSL would be a natural choice given the current code. In case we decide to add --with-libressl, I can offer the following alternatives and remain open to other suggestions: * USING_OPENSSL * USING_OPENSSL_API * USE_OPENSSL_API * USE_OPENSSL_FLAVORS * USE_ANY_OPENSSL * HAVE_OPENSSL_API >> Repeating (a || b) condition many times is bad. Using a single (c) >> condition many times is not. We should not be arguing about that basic >> programming principle! > So you wish de define a 3rd macro in addition to the other two I wish we add no new macros until they become necessary. You are adding macros. I am content with what we already have for this high-level context: A single USE_OPENSSL macro. The existing USE_OPENSSL macro is not the problem. The lack of an explicit --with-libressl option is not the problem. The problem is that Squid relies on the OPENSSL_VERSION_NUMBER (which LibreSSL has changed in an incompatible way). This is a minor compatibility problem that has several correct solutions, none of which require changing most Squid code dealing with the OpenSSL API. Here are a few sketches: A. Define SQUID_OPENSSL_VERSION to match available API version (in OpenSSL terms) and use that instead of raw OPENSSL_VERSION_NUMBER. B. Replace raw OPENSSL_VERSION_NUMBER uses with macros that guard the corresponding features (without adding feature tests). C. Replace OPENSSL_VERSION_NUMBER with feature tests. Solution C is the best/cleanest but also requires more work. I am not sure, but I suspect that all solutions can be implemented without adding a new --with-* option for every OpenSSL API implementation (LibreSSL, BoringSSL, etc.). >> I believe we can simply continue to use USE_OPENSSL almost everywhere it >> is used today. No "fixing" is needed in this context except changing >> what you think that macro should stand for. > So a hypothetical patch is submitted to use, lets say a function > parameter is made const, in the LibreSSL but breaks when building with > OpenSSL. Is required to be wrapped in USE_OPENSSL. > > Can you spot the contradiction? There is no contradiction if you interpret USE_OPENSSL correctly. Yes, there could be other macros/guards inside USE_OPENSSL code, including, if really necessary USE_LIBRESSL set by --with-libressl. In the latter case, USE_OPENSSL itself (i.e., the primary guard for the code using OpenSSL API) would have to be renamed to USING_OPENSSL_API or some such. Needless to say, those other macros/guards have to be confined to a few exceptional cases. >> In other words, you seem to abstract at the level of file name spelling. >> I abstract at the level of APIs. Naturally, it is difficult to agree on >> a single scheme that accommodates both approaches! > I am following the autoconf guidelines I rest my case! We should be following autoconf guidelines when implementing a high-level design. We should not follow autoconf guidelines to come up with or understand a high-level design. Again and again, you are obsessed with letters such as those used to spell the USE_OPENSSL macro. We should be obsessed with that macro meaning instead. > The
Re: [squid-dev] [PATCH] Bug 4662 adding --with-libressl build option
Do you think we can compromise and call it USE_OPENSSL_OR_LIBRESSL ? or call it USE_OPENSSL_API and then the code will eventually have none or few occurrences of USE_OPENSSL and USE_LIBRESSL to deal with OpenSSL and LibreSSL specifics. Marcus ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] [PATCH] Bug 4662 adding --with-libressl build option
On 02/01/2017 05:00 AM, Eliezer Croitoru wrote: > I do believe that for the latest hardware with beefy CPU, code > repetition in C++ might not be much of a regression but not everybody > can replace their systems hardware every year. (If my assumption > about code repetition affecting older systems is wrong let me know) In general, code duplication itself has negligible impact on performance (which can go either way, depending on the circumstances). Code duplication leads to bugs and increases the cost of development. Its long-term effect is often exponential. I speculate that at least 20% of Squid bugs are caused by code duplication, and an average Squid development project is at least 20% more expensive because of it. It is a big deal! Fortunately, it is usually easy to avoid when writing new code. Unfortunately, I have to keep begging and begging to avoid it in new code, as if it is some obscure or unimportant problem and not a basic development principle. Alex. ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] [PATCH] Bug 4662 adding --with-libressl build option
I want to add my opinion on the matter as a system administrator compared to some developers about LibreSSL. The systems I am working are mostly stable or attempts to be stable. I mentioned in the past that we should consider sticking on releases with something stable compared to non-stable as an approach. Squid 4.0.X is trying to be Stable "Compatible" and what I consider stable in the Linux systems I work are: - Debian - CentOS - Ubuntu 14.04 (16.04 is exactly on the point between stable enough for use to stable for enterprise). - FreeBSD 10.X - Solaris Debian is far behind in libreSSL support and CentOS is not trying to be compatible in the near future. So it leaves me\us with something that tries to be stable such as Ubuntu 16.04. They do not offer LibreSSL officially(as far as I know) and it's the same for FreeBSD 10.X. and Solaris(as far as I know) I believe that 4.0.X should not try to be compatible with LibreSSL and this is due to the fact that the developers of the SSL related code needs a stable ground. BUG 4662[http://bugs.squid-cache.org/show_bug.cgi?id=4662] is a result of a testing experiment with a not enough production or non-tested enough by many software. I believe that at this stage we should define the exact point in the future that we would try to support LibreSSL instead of supporting it. The developers of the SSL related code should define if the goal is 5.0 or another more advanced version. Also I do believe that for the latest hardware with beefy CPU, code repetition in C++ might not be much of a regression but not everybody can replace their systems hardware every year. (If my assumption about code repetition affecting older systems is wrong let me know) I hope that my words helped and contributed to the subject in hands. Regards, Eliezer Eliezer Croitoru Linux System Administrator Mobile: +972-5-28704261 Email: elie...@ngtech.co.il -Original Message- From: squid-dev [mailto:squid-dev-boun...@lists.squid-cache.org] On Behalf Of Alex Rousskov Sent: Tuesday, January 31, 2017 6:50 PM To: Squid Developers <squid-dev@lists.squid-cache.org> Subject: Re: [squid-dev] [PATCH] Bug 4662 adding --with-libressl build option On 01/31/2017 08:20 AM, Amos Jeffries wrote: > On 31/01/2017 7:04 a.m., Alex Rousskov wrote: >> On 01/29/2017 04:26 AM, Amos Jeffries wrote: >>> This is I think all we need to do code-wise to resolve the Bug 4662 >>> issues with LibreSSL being incompatible with OpenSSL 1.1. >> I do not think these changes should be committed. As you probably >> know from earlier communication, I think we should avoid using both >> USE_OPENSSL and USE_LIBRESSL in the code if LibreSSL is [treated as] >> a replacement for OpenSSL. I have suggested several ways to avoid the >> dangerous and needless repetition of (USE_OPENSSL || USE_LIBRESSL) >> conditions, and we even seemed to agree on one of those solutions. > IMO we only agreed on the HAVE_* macro usage for determining whether > the > 1.1 API was in use. Which is included in this patch. I do not limit "feature tests" to "1.1 API". Any feature is eligible. In fact, it may be a good idea to test individual features rather than bundle many of them into a single API version test (that will become obsolete as parts of that API are going to change). However, those details are probably not very important when resolving the core disagreement here. > I don't think the repetition of (USE_OPENSSL || USE_LIBRESSL) is > either needless or dangerous. The needless part is not a matter of opinion. It is a fact -- you do not need to repeat the (USE_OPENSSL || USE_LIBRESSL) test. You may use a single USE_FOOBAR macro to avoid this code repetition. The exact spelling of FOOBAR is a separate question. Whether code duplication is bad, dangerous, unwanted, etc. is certainly a matter of opinion and subject to context, but I doubt you can convince me that it is not in this case, especially as we have started using USE_OPENSSL macro in complex expressions involving GnuTLS. > No more than USE_OPENSSL was in the same spot. Repeating (a || b) condition many times is bad. Using a single (c) condition many times is not. We should not be arguing about that basic programming principle! > Fixing the sheer number of uses is not in scope. > > Keep it simple. I believe we can simply continue to use USE_OPENSSL almost everywhere it is used today. No "fixing" is needed in this context except changing what you think that macro should stand for. > Only one macro per library. --with-foo sets USE_FOO. For you, the libraries provided by OpenSSL and LibreSSL project are two different "libraries". For me, they are two slightly different flavors of the same "library". IMO, the developers should not be forced to think which flavor is in use now
Re: [squid-dev] [PATCH] Bug 4662 adding --with-libressl build option
On 01/31/2017 08:20 AM, Amos Jeffries wrote: > On 31/01/2017 7:04 a.m., Alex Rousskov wrote: >> On 01/29/2017 04:26 AM, Amos Jeffries wrote: >>> This is I think all we need to do code-wise to resolve the Bug 4662 >>> issues with LibreSSL being incompatible with OpenSSL 1.1. >> I do not think these changes should be committed. As you probably know >> from earlier communication, I think we should avoid using both >> USE_OPENSSL and USE_LIBRESSL in the code if LibreSSL is [treated as] a >> replacement for OpenSSL. I have suggested several ways to avoid the >> dangerous and needless repetition of (USE_OPENSSL || USE_LIBRESSL) >> conditions, and we even seemed to agree on one of those solutions. > IMO we only agreed on the HAVE_* macro usage for determining whether the > 1.1 API was in use. Which is included in this patch. I do not limit "feature tests" to "1.1 API". Any feature is eligible. In fact, it may be a good idea to test individual features rather than bundle many of them into a single API version test (that will become obsolete as parts of that API are going to change). However, those details are probably not very important when resolving the core disagreement here. > I don't think the repetition of (USE_OPENSSL || USE_LIBRESSL) is either > needless or dangerous. The needless part is not a matter of opinion. It is a fact -- you do not need to repeat the (USE_OPENSSL || USE_LIBRESSL) test. You may use a single USE_FOOBAR macro to avoid this code repetition. The exact spelling of FOOBAR is a separate question. Whether code duplication is bad, dangerous, unwanted, etc. is certainly a matter of opinion and subject to context, but I doubt you can convince me that it is not in this case, especially as we have started using USE_OPENSSL macro in complex expressions involving GnuTLS. > No more than USE_OPENSSL was in the same spot. Repeating (a || b) condition many times is bad. Using a single (c) condition many times is not. We should not be arguing about that basic programming principle! > Fixing the sheer number of uses is not in scope. > > Keep it simple. I believe we can simply continue to use USE_OPENSSL almost everywhere it is used today. No "fixing" is needed in this context except changing what you think that macro should stand for. > Only one macro per library. --with-foo sets USE_FOO. For you, the libraries provided by OpenSSL and LibreSSL project are two different "libraries". For me, they are two slightly different flavors of the same "library". IMO, the developers should not be forced to think which flavor is in use now or which flavors are supported today, except in very few low-level cases where that information might be needed. With feature tests, the number of such cases might be zero. In other words, you seem to abstract at the level of file name spelling. I abstract at the level of APIs. Naturally, it is difficult to agree on a single scheme that accommodates both approaches! Needless to say, the two projects may eventually diverge their APIs so that one is no longer a flavor of the other. This day has not come yet, and one could hope that LibreSSL folks would either avoid this divergence or have the decency to stop using "OPENSSL" in their diverged API names. > NP: I am only adding the "|| USE_LIBRESSL" parts in this patch because I > happen to know that the existing code is being used with LibreSSL and > found to be working. Future code will have to prove itself separately > for each library, OR at the submitters discretion (passing audit and QA > also) list the librares believed to be safe with it. I believe the approach you have described is inferior to treating both OpenSSL and LibreSSL as providing the same overall API while avoiding (or treating specially) the rare parts of the API that differ. Today, there is only one such known exception in Squid context -- the OPENSSL_VERSION_NUMBER macro. We seem to agree that we should not be using that part of the API anyway, so there will be no exceptions at all after OPENSSL_VERSION_NUMBER is replaced with feature tests. Your approach appears to be based on the assumption that enabling or disabling individual code pieces dealing with OpenSSL/LibreSSL APIs based on "known to be used and seemingly working" test will preserve the overall integrity of the code. I believe that assumption is wrong and that low-level approach is too dangerous. IMO, the decision whether to support LibreSSL in Squid should be made on a much higher level than an individual piece of code. Levels such as "whole Squid" or "non-bumping Squid" would be more appropriate. Exceptions might be made for small, isolated functionality, but they should be carefully considered exceptions, not the rule. The considerations in the above two paragraphs are orthogonal to avoiding code duplication but avoiding code duplication is easier when LibreSSL support decisions are made at a level higher than individual preprocessor statements. > It is not a blocker,
Re: [squid-dev] [PATCH] Bug 4662 adding --with-libressl build option
On 31/01/2017 7:04 a.m., Alex Rousskov wrote: > On 01/29/2017 04:26 AM, Amos Jeffries wrote: >> This is I think all we need to do code-wise to resolve the Bug 4662 >> issues with LibreSSL being incompatible with OpenSSL 1.1. >> >> The libraries cannot both be linked either way. If both --with-* options >> are provided LibreSSL currently overrides OpenSSL. I picked that >> preference order because AFAICS the LibreSSL has the lower overall >> security footprint while providing the same (or better) needed >> functionality. >> >> >> NP: If there are no objections I would like to fast-track this and apply >> in ~3 days (allowing for today being a sunday) for a slightly late >> 4.0.18 beta. > > I do not think these changes should be committed. As you probably know > from earlier communication, I think we should avoid using both > USE_OPENSSL and USE_LIBRESSL in the code if LibreSSL is [treated as] a > replacement for OpenSSL. I have suggested several ways to avoid the > dangerous and needless repetition of (USE_OPENSSL || USE_LIBRESSL) > conditions, and we even seemed to agree on one of those solutions. > IMO we only agreed on the HAVE_* macro usage for determining whether the 1.1 API was in use. Which is included in this patch. I don't think the repetition of (USE_OPENSSL || USE_LIBRESSL) is either needless or dangerous. No more than USE_OPENSSL was in the same spot. Fixing the sheer number of uses is not in scope, tempting as it was to drop some more completely. Keep it simple. Only one macro per library. --with-foo sets USE_FOO. The rest is normal precompiler operators which anyone knowing C/C++ should be able to easily read and can combine the libraries in all sorts of fancy ways _iff_ necessary. Going by that policy both libraries get named for code they support and the || is used. NP: I am only adding the "|| USE_LIBRESSL" parts in this patch because I happen to know that the existing code is being used with LibreSSL and found to be working. Future code will have to prove itself separately for each library, OR at the submitters discretion (passing audit and QA also) list the librares believed to be safe with it. > > FWIW, I do not think bug 4662 blocks 4.0.18 beta. > It is not a blocker, but I would like to avoid declaring LibreSSL as no longer supported by v4 since the fix is not exactly hard. Amos ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] [PATCH] Bug 4662 adding --with-libressl build option
On 01/29/2017 04:26 AM, Amos Jeffries wrote: > This is I think all we need to do code-wise to resolve the Bug 4662 > issues with LibreSSL being incompatible with OpenSSL 1.1. > > The libraries cannot both be linked either way. If both --with-* options > are provided LibreSSL currently overrides OpenSSL. I picked that > preference order because AFAICS the LibreSSL has the lower overall > security footprint while providing the same (or better) needed > functionality. > > > NP: If there are no objections I would like to fast-track this and apply > in ~3 days (allowing for today being a sunday) for a slightly late > 4.0.18 beta. I do not think these changes should be committed. As you probably know from earlier communication, I think we should avoid using both USE_OPENSSL and USE_LIBRESSL in the code if LibreSSL is [treated as] a replacement for OpenSSL. I have suggested several ways to avoid the dangerous and needless repetition of (USE_OPENSSL || USE_LIBRESSL) conditions, and we even seemed to agree on one of those solutions. FWIW, I do not think bug 4662 blocks 4.0.18 beta. Alex. ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
