Re: [PR] BUILD: extend travis-ci matrix
wow. I am surprised how proxy protocl might be used. can we add reg test on that ? вт, 2 июл. 2019 г. в 01:31, Lukas Tribus : > Hello Ilya, hello Willy, > > > On Thu, 27 Jun 2019 at 14:04, Lukas Tribus wrote: > > > > Hello, > > > > On Thu, 27 Jun 2019 at 13:19, Willy Tarreau wrote: > > > > > > Hi guys, > > > > > > On Thu, Jun 27, 2019 at 03:55:54PM +0500, ??? wrote: > > > > you are right, commit messages is not my best. > > > > sorry about that > > > > > > > > anyway, if you think it is wrong, you are welcome to provide a fix > > > > (sometimes, I do wrong things, I'm ok with that) > > > > > > Ilya, don't take it bad, bugs always happen and the problem with bugs > > > is to detect them then to figure how to address them. > > > > Indeed, bugs will always happen, it doesn't matter who the author is, > > and that was not my critique. My issue was merely with the amount of > > changes and the description. > > > > I will provide some more technical context tomorrow. > > SSL_state() would return an int, not an enum, so the comparison would > have to be a bitwise AND (but even OpenSSL got it wrong in their own > server example code). > > Even fixing that, it does not work; neither OpenSSL nor LibreSSL > recognizes the empty handshake. I assume this broke a long time ago in > OpenSSL (before the forks). > > An alternative, even simpler call available in both OpenSSL and > LibreSSL would be SSL_in_before(), but that's really just a: > # define SSL_in_before(a) (SSL_state(a)_ST_BEFORE) > > so it fails just like it's parent function. > > When I say fail, I mean the comparison is never true and we do not > recognize the empty handshake, which I simulated with and without > proxy protocol like this: > > echo -ne "PROXY TCP4 172.30.1.153 172.30.1.153 3052 3443\r\n" | nc > 127.0.0.1 443 > echo -ne "" | nc 127.0.0.1 444 > > > I did not find any way in LibreSSL to make it recognize an empty > handshake, which is why I suggest we remove LibreSSL from the equation > just like we did with BoringSSL. > > I will follow-up with an RFC patch shortly and I'd ask you, Ilya, to > provide feedback. > > > I have yet to verify a possible additional issue with empty handshake > detection in 2.0 and -dev for OpenSSL 1.1.0 that came up during > testing, but are unrelated to this specific change. > > > Regards, > Lukas >
Re: [PR] BUILD: extend travis-ci matrix
Hello Ilya, hello Willy, On Thu, 27 Jun 2019 at 14:04, Lukas Tribus wrote: > > Hello, > > On Thu, 27 Jun 2019 at 13:19, Willy Tarreau wrote: > > > > Hi guys, > > > > On Thu, Jun 27, 2019 at 03:55:54PM +0500, ??? wrote: > > > you are right, commit messages is not my best. > > > sorry about that > > > > > > anyway, if you think it is wrong, you are welcome to provide a fix > > > (sometimes, I do wrong things, I'm ok with that) > > > > Ilya, don't take it bad, bugs always happen and the problem with bugs > > is to detect them then to figure how to address them. > > Indeed, bugs will always happen, it doesn't matter who the author is, > and that was not my critique. My issue was merely with the amount of > changes and the description. > > I will provide some more technical context tomorrow. SSL_state() would return an int, not an enum, so the comparison would have to be a bitwise AND (but even OpenSSL got it wrong in their own server example code). Even fixing that, it does not work; neither OpenSSL nor LibreSSL recognizes the empty handshake. I assume this broke a long time ago in OpenSSL (before the forks). An alternative, even simpler call available in both OpenSSL and LibreSSL would be SSL_in_before(), but that's really just a: # define SSL_in_before(a) (SSL_state(a)_ST_BEFORE) so it fails just like it's parent function. When I say fail, I mean the comparison is never true and we do not recognize the empty handshake, which I simulated with and without proxy protocol like this: echo -ne "PROXY TCP4 172.30.1.153 172.30.1.153 3052 3443\r\n" | nc 127.0.0.1 443 echo -ne "" | nc 127.0.0.1 444 I did not find any way in LibreSSL to make it recognize an empty handshake, which is why I suggest we remove LibreSSL from the equation just like we did with BoringSSL. I will follow-up with an RFC patch shortly and I'd ask you, Ilya, to provide feedback. I have yet to verify a possible additional issue with empty handshake detection in 2.0 and -dev for OpenSSL 1.1.0 that came up during testing, but are unrelated to this specific change. Regards, Lukas
Re: [PR] BUILD: extend travis-ci matrix
Hello, On Thu, 27 Jun 2019 at 13:19, Willy Tarreau wrote: > > Hi guys, > > On Thu, Jun 27, 2019 at 03:55:54PM +0500, ??? wrote: > > you are right, commit messages is not my best. > > sorry about that > > > > anyway, if you think it is wrong, you are welcome to provide a fix > > (sometimes, I do wrong things, I'm ok with that) > > Ilya, don't take it bad, bugs always happen and the problem with bugs > is to detect them then to figure how to address them. Indeed, bugs will always happen, it doesn't matter who the author is, and that was not my critique. My issue was merely with the amount of changes and the description. I will provide some more technical context tomorrow. Thanks, Lukas
Re: [PR] BUILD: extend travis-ci matrix
Hi guys, On Thu, Jun 27, 2019 at 03:55:54PM +0500, ??? wrote: > ??, 27 ???. 2019 ?. ? 15:33, Lukas Tribus : > > > Hello, > > > > On Thu, 27 Jun 2019 at 08:43, ??? wrote: > > >> However this commit also changes error handling with OpenSSL: > > >> > > >> > > https://github.com/haproxy/haproxy/commit/54832b97c65c43ee50b6661a9b4b05885427b5e9#diff-cc9b3f376afbc81cf478eeac17839172L5533 > > >> > > >> > > >> Can you clarify if the OpenSSL change was intentional and if yes, > > elaborate? > > > > > > > > > it was intentional. libressl does not support "packet_size" there. > > > however, openssl is fine with the above way > > > > Then I have to criticize that commit message. > > > > you are right, commit messages is not my best. > sorry about that > > anyway, if you think it is wrong, you are welcome to provide a fix > (sometimes, I do wrong things, I'm ok with that) Ilya, don't take it bad, bugs always happen and the problem with bugs is to detect them then to figure how to address them. In this regard, Lukas is absolutely right. We all know it takes time to learn how to do better commits, and guess what, very often when I work on a bug, end up on a commit, and start to shout "who the hell did that crap?" I discover it's me and I'm totally ashamed of the lack of context in my own message despite probably being the most annoying one about it. This is really something we're extremely picky about as it directly affects our ability to maintain robust stable branches so that users can sleep while their load balancers swallow requests. However please do not just give up suggesting that anyone can take over your patch. It doesn't work this way, everyone is responsible in one way or another for his/her contributions and their impact (and seeing how you care about getting Travis to work fine, I'm pretty sure you feel bad when you see that one of your changes does not work as expected). Please just work with Lukas to try to remember why you thought the change was OK and how Lukas figured this one broke something. Once the context is properly recalled, you may together figure the corner case that's not covered anymore and how to properly address it. Then either of you proposes the patch and this bug doesn't exist anymore. Bugs introduced are not a judgement of anyone's work quality. However the ability to fix one's bug definitely is. That's why we don't care when bugs are accidently introduced, and value the fixes above anything. Over time it pays off. Thanks, Willy
Re: [PR] BUILD: extend travis-ci matrix
чт, 27 июн. 2019 г. в 15:33, Lukas Tribus : > Hello, > > On Thu, 27 Jun 2019 at 08:43, Илья Шипицин wrote: > >> However this commit also changes error handling with OpenSSL: > >> > >> > https://github.com/haproxy/haproxy/commit/54832b97c65c43ee50b6661a9b4b05885427b5e9#diff-cc9b3f376afbc81cf478eeac17839172L5533 > >> > >> > >> Can you clarify if the OpenSSL change was intentional and if yes, > elaborate? > > > > > > it was intentional. libressl does not support "packet_size" there. > > however, openssl is fine with the above way > > Then I have to criticize that commit message. > you are right, commit messages is not my best. sorry about that anyway, if you think it is wrong, you are welcome to provide a fix (sometimes, I do wrong things, I'm ok with that) > > It fixes the build, but it changes a function call, so it's more than > just a build fix. It's also not affecting only the LibreSSL code path, > but the OpenSSL code path also. > A single commit should do one thing only (as opposed to a number of > things) and reflect the changes it does in both title and message. > > "BUILD: enable several LibreSSL hacks, including" when touching > OpenSSL code paths is not enough. This should have been 1) a separate > commit and 2) have a descriptive message like: > > "ssl: change error handling on empty handshake > > Also fixes the build with LibreSSL" > > Because that's what the change does: a different error handling for > empty handshakes, which fixes the build with LibreSSL but the change > affects (affects in that in changes the code) OpenSSL as well. > > The change breaks empty handshake detection in OpenSSL < 1.1.0. We > need to revert this and put the LibreSSL fix in a different #if > branch. > > > > however, openssl is fine with the above way > > That assumption is wrong, but that's not the point. The point is that > we need to do a better job of making one change only per commit and > have title and commit message reflect the actual change and intention, > not the assumed impact. > > > thanks, > lukas >
Re: [PR] BUILD: extend travis-ci matrix
Hello, On Thu, 27 Jun 2019 at 08:43, Илья Шипицин wrote: >> However this commit also changes error handling with OpenSSL: >> >> https://github.com/haproxy/haproxy/commit/54832b97c65c43ee50b6661a9b4b05885427b5e9#diff-cc9b3f376afbc81cf478eeac17839172L5533 >> >> >> Can you clarify if the OpenSSL change was intentional and if yes, elaborate? > > > it was intentional. libressl does not support "packet_size" there. > however, openssl is fine with the above way Then I have to criticize that commit message. It fixes the build, but it changes a function call, so it's more than just a build fix. It's also not affecting only the LibreSSL code path, but the OpenSSL code path also. A single commit should do one thing only (as opposed to a number of things) and reflect the changes it does in both title and message. "BUILD: enable several LibreSSL hacks, including" when touching OpenSSL code paths is not enough. This should have been 1) a separate commit and 2) have a descriptive message like: "ssl: change error handling on empty handshake Also fixes the build with LibreSSL" Because that's what the change does: a different error handling for empty handshakes, which fixes the build with LibreSSL but the change affects (affects in that in changes the code) OpenSSL as well. The change breaks empty handshake detection in OpenSSL < 1.1.0. We need to revert this and put the LibreSSL fix in a different #if branch. > however, openssl is fine with the above way That assumption is wrong, but that's not the point. The point is that we need to do a better job of making one change only per commit and have title and commit message reflect the actual change and intention, not the assumed impact. thanks, lukas
Re: [PR] BUILD: extend travis-ci matrix
ср, 26 июн. 2019 г. в 23:30, Lukas Tribus :
> Hello Ilya, Willy,
>
>
> On Sun, 5 May 2019 at 20:30, Илья Шипицин wrote:
> >
> > something has changed. build is passing now, reg-tests fail (they used
> to work earlier)
> >
> >
> > I'd apply those patches (and we will fix reg-tests later)
>
> One of those patches is 54832b97c6 ("BUILD: enable several LibreSSL
> hacks, including") in the tree.
>
> Now the commit subject and message suggests that this change is only
> about the build and only about LibreSSL.
>
> However this commit also changes error handling with OpenSSL:
>
>
> https://github.com/haproxy/haproxy/commit/54832b97c65c43ee50b6661a9b4b05885427b5e9#diff-cc9b3f376afbc81cf478eeac17839172L5533
>
>
> Can you clarify if the OpenSSL change was intentional and if yes,
> elaborate?
>
it was intentional. libressl does not support "packet_size" there.
however, openssl is fine with the above way
>
>
>
> Thanks,
>
> Lukas
>
Re: [PR] BUILD: extend travis-ci matrix
Hello Ilya, Willy,
On Sun, 5 May 2019 at 20:30, Илья Шипицин wrote:
>
> something has changed. build is passing now, reg-tests fail (they used to
> work earlier)
>
>
> I'd apply those patches (and we will fix reg-tests later)
One of those patches is 54832b97c6 ("BUILD: enable several LibreSSL
hacks, including") in the tree.
Now the commit subject and message suggests that this change is only
about the build and only about LibreSSL.
However this commit also changes error handling with OpenSSL:
https://github.com/haproxy/haproxy/commit/54832b97c65c43ee50b6661a9b4b05885427b5e9#diff-cc9b3f376afbc81cf478eeac17839172L5533
Can you clarify if the OpenSSL change was intentional and if yes, elaborate?
Thanks,
Lukas
Re: [PR] BUILD: extend travis-ci matrix
Hi Lukas, On Mon, May 06, 2019 at 06:51:57PM +0200, Lukas Tribus wrote: > There is room for improvement here. Can you confirm that attaching a > patch file per commit to the email would fix this usability issue? I'd say yes, provided the attachments are prefixed with a sequence number, like git-format-patch does. However I think we should set reasonable limits to 10 patches or less to save all list members from being bombed if someone sends a huge (or even bogus) PR. I think that till a handful of patches (let's say two hands :-)) it's still possible to let various participants give their opinion on different patches. When you see patch 137/375 it's unlikely that anyone will have a look at it so then falling back to the current mode would work better. Similarly I think that if a patch is too large the series should not be forwarded. I'm well aware that "too large" is a bit vague. The idea is that few people if any will spend their time reviewing a 1000-line patch. But this could be applied to the whole series if easier. In the end I think that such thresholds will serve as a hint for reviewers about what to expect when seeing the announce. > I just finally fixed the script for authors with non-ascii names (it > crashed previously), I can add the file attachments to my todo list, > if useful. This could definitely be useful. But don't spend too much valuable time on this, as this is not the source of contribs I'd like to encourage the most considering the amount of fixes they usually require before being merged. By the way I noticed that something changed recently on Github, the curl command receives a redirect and now needs -L to follow it. Thanks! Willy
Re: [PR] BUILD: extend travis-ci matrix
Hello Willy, On Mon, 6 May 2019 at 08:06, Willy Tarreau wrote: > > Hi Ilya, > > > I made another PR > > > > https://github.com/haproxy/haproxy/pull/92 > > Thank you. > > > (I really like automatic PR to mailing list routing) > > Well, it was the only workable workaround we have when people send PRs. > Sadly we can't block them. Apparently only mirror repositories can block > PRs and github doesn't accept to create them anymore. But when there are > multiple patches in the PR it's not usable as-is anymore, it only serves > as a notification that someone sent something. > > For me, PRs tremendously increase the amount of manipulations, which is > why I tend to postpone them until I think this will not divert me too > much from what I am doing. I just timed the operations on this small > batch and it went from ~15 seconds to review and merge your 4 patches > if they were in mails (just pressing Enter in mutt to review them, > possibly "e" to quickly edit if needed, and "A" to apply), then "g" to > ack the merge, to around 3 minutes to : There is room for improvement here. Can you confirm that attaching a patch file per commit to the email would fix this usability issue? I just finally fixed the script for authors with non-ascii names (it crashed previously), I can add the file attachments to my todo list, if useful. Lukas
Re: [PR] BUILD: extend travis-ci matrix
I have just noticed that "scripts/build-ssl.sh" is missing in .travis.yml so, unfortunately, we are running all builds against xenial openssl-1.0.2 (no openssl-1.1.X, no libressl...) I'm not sure when "scripts/build-ssl.sh" disappeared. I'll send new patch soon пн, 6 мая 2019 г. в 11:05, Willy Tarreau : > Hi Ilya, > > > I made another PR > > > > https://github.com/haproxy/haproxy/pull/92 > > Thank you. > > > (I really like automatic PR to mailing list routing) > > Well, it was the only workable workaround we have when people send PRs. > Sadly we can't block them. Apparently only mirror repositories can block > PRs and github doesn't accept to create them anymore. But when there are > multiple patches in the PR it's not usable as-is anymore, it only serves > as a notification that someone sent something. > > For me, PRs tremendously increase the amount of manipulations, which is > why I tend to postpone them until I think this will not divert me too > much from what I am doing. I just timed the operations on this small > batch and it went from ~15 seconds to review and merge your 4 patches > if they were in mails (just pressing Enter in mutt to review them, > possibly "e" to quickly edit if needed, and "A" to apply), then "g" to > ack the merge, to around 3 minutes to : > - copy the link > - open a browser > - paste the link into the browser's URL bar > - locate the links to the commits in the page > - open each of them in a separate tab > - visit each tab in turn, and for each of them, press Ctrl-L and > append ".patch" at the end of the URL, then press enter > - after the patches are loaded, visit all of them again, to look at the > dates and figure in what order to apply them > - then for each of them : > - review the patch > - Ctrl-S to save the patch > - remember the file name (commit ID) and press Enter > - Alt-tab to switch to the xterm in the development directory > - git am ~/.patch > - rm -f ~/.patch > - Alt-tab to switch back to browser again > - Ctrl-W to close the tab > - close browser window > - back to mutt to ack merging and rant about my hate of this workflow > > => This is way longer and more error-prone than the first variant. And >this doesn't even include commenting if I disagree on something, which >additionally requires copy-pasting the relevant parts of the patch >into the response message. There is a reason why most projects >developed on github are so low quality with so horrible commits >and zero maintenance : it encourages laziness and dirtiness. It >costs so much effort to fix minor issues and encourage people to >improve, compared to lazily click on "merge" that you simply think >"bah..." and you merge it as-is, making it impossible to review them >later when trying to make a maintenance version. So you end up with >continuous development made of a pile of junk patches :-/ > > Anyway now I've finally applied your patches. In the future, if you > want to help with a smoother process, please either attach your patches > or use git-send-email. The most efficient workflow is one patch per mail, > like git-send-email does, as it eases reviews (which can also be done by > multiple persons). If you attach them, make sure to use the file names > created by git-format-patch so that there's no doubt when saving files > for manual editing for example. Also it's important to indicate the > intent with a patch or a patch series, i.e. "please merge this", "please > don't merge this, it's just for discussing", "it should be mergeable once > the makefile is fixed", "I think it's OK for merging but I'd prefer an > extra review first", "how about we'd do this", etc. I'm all for being > accommodating with submissions and slightly edit them if needed because > I know that if the work has to be 100% on either side it can total more > time than 90/10 or even 95/5 (but I never edit signed patches beyond > fixing merging issues though). I'm just very careful about the time > spent on my side because I know I'm already a bottleneck, so each extra > minute added here delays everything else. > > Thanks! > Willy >
Re: [PR] BUILD: extend travis-ci matrix
well, I hope travis-ci will be useful (or we will drop it). as for PR, I meant that it should have been sent to list anyway (but it was not sent for some reason). I can send using "git send email" as well пн, 6 мая 2019 г. в 11:05, Willy Tarreau : > Hi Ilya, > > > I made another PR > > > > https://github.com/haproxy/haproxy/pull/92 > > Thank you. > > > (I really like automatic PR to mailing list routing) > > Well, it was the only workable workaround we have when people send PRs. > Sadly we can't block them. Apparently only mirror repositories can block > PRs and github doesn't accept to create them anymore. But when there are > multiple patches in the PR it's not usable as-is anymore, it only serves > as a notification that someone sent something. > > For me, PRs tremendously increase the amount of manipulations, which is > why I tend to postpone them until I think this will not divert me too > much from what I am doing. I just timed the operations on this small > batch and it went from ~15 seconds to review and merge your 4 patches > if they were in mails (just pressing Enter in mutt to review them, > possibly "e" to quickly edit if needed, and "A" to apply), then "g" to > ack the merge, to around 3 minutes to : > - copy the link > - open a browser > - paste the link into the browser's URL bar > - locate the links to the commits in the page > - open each of them in a separate tab > - visit each tab in turn, and for each of them, press Ctrl-L and > append ".patch" at the end of the URL, then press enter > - after the patches are loaded, visit all of them again, to look at the > dates and figure in what order to apply them > - then for each of them : > - review the patch > - Ctrl-S to save the patch > - remember the file name (commit ID) and press Enter > - Alt-tab to switch to the xterm in the development directory > - git am ~/.patch > - rm -f ~/.patch > - Alt-tab to switch back to browser again > - Ctrl-W to close the tab > - close browser window > - back to mutt to ack merging and rant about my hate of this workflow > > => This is way longer and more error-prone than the first variant. And >this doesn't even include commenting if I disagree on something, which >additionally requires copy-pasting the relevant parts of the patch >into the response message. There is a reason why most projects >developed on github are so low quality with so horrible commits >and zero maintenance : it encourages laziness and dirtiness. It >costs so much effort to fix minor issues and encourage people to >improve, compared to lazily click on "merge" that you simply think >"bah..." and you merge it as-is, making it impossible to review them >later when trying to make a maintenance version. So you end up with >continuous development made of a pile of junk patches :-/ > > Anyway now I've finally applied your patches. In the future, if you > want to help with a smoother process, please either attach your patches > or use git-send-email. The most efficient workflow is one patch per mail, > like git-send-email does, as it eases reviews (which can also be done by > multiple persons). If you attach them, make sure to use the file names > created by git-format-patch so that there's no doubt when saving files > for manual editing for example. Also it's important to indicate the > intent with a patch or a patch series, i.e. "please merge this", "please > don't merge this, it's just for discussing", "it should be mergeable once > the makefile is fixed", "I think it's OK for merging but I'd prefer an > extra review first", "how about we'd do this", etc. I'm all for being > accommodating with submissions and slightly edit them if needed because > I know that if the work has to be 100% on either side it can total more > time than 90/10 or even 95/5 (but I never edit signed patches beyond > fixing merging issues though). I'm just very careful about the time > spent on my side because I know I'm already a bottleneck, so each extra > minute added here delays everything else. > > Thanks! > Willy >
Re: [PR] BUILD: extend travis-ci matrix
Hi Ilya, > I made another PR > > https://github.com/haproxy/haproxy/pull/92 Thank you. > (I really like automatic PR to mailing list routing) Well, it was the only workable workaround we have when people send PRs. Sadly we can't block them. Apparently only mirror repositories can block PRs and github doesn't accept to create them anymore. But when there are multiple patches in the PR it's not usable as-is anymore, it only serves as a notification that someone sent something. For me, PRs tremendously increase the amount of manipulations, which is why I tend to postpone them until I think this will not divert me too much from what I am doing. I just timed the operations on this small batch and it went from ~15 seconds to review and merge your 4 patches if they were in mails (just pressing Enter in mutt to review them, possibly "e" to quickly edit if needed, and "A" to apply), then "g" to ack the merge, to around 3 minutes to : - copy the link - open a browser - paste the link into the browser's URL bar - locate the links to the commits in the page - open each of them in a separate tab - visit each tab in turn, and for each of them, press Ctrl-L and append ".patch" at the end of the URL, then press enter - after the patches are loaded, visit all of them again, to look at the dates and figure in what order to apply them - then for each of them : - review the patch - Ctrl-S to save the patch - remember the file name (commit ID) and press Enter - Alt-tab to switch to the xterm in the development directory - git am ~/.patch - rm -f ~/.patch - Alt-tab to switch back to browser again - Ctrl-W to close the tab - close browser window - back to mutt to ack merging and rant about my hate of this workflow => This is way longer and more error-prone than the first variant. And this doesn't even include commenting if I disagree on something, which additionally requires copy-pasting the relevant parts of the patch into the response message. There is a reason why most projects developed on github are so low quality with so horrible commits and zero maintenance : it encourages laziness and dirtiness. It costs so much effort to fix minor issues and encourage people to improve, compared to lazily click on "merge" that you simply think "bah..." and you merge it as-is, making it impossible to review them later when trying to make a maintenance version. So you end up with continuous development made of a pile of junk patches :-/ Anyway now I've finally applied your patches. In the future, if you want to help with a smoother process, please either attach your patches or use git-send-email. The most efficient workflow is one patch per mail, like git-send-email does, as it eases reviews (which can also be done by multiple persons). If you attach them, make sure to use the file names created by git-format-patch so that there's no doubt when saving files for manual editing for example. Also it's important to indicate the intent with a patch or a patch series, i.e. "please merge this", "please don't merge this, it's just for discussing", "it should be mergeable once the makefile is fixed", "I think it's OK for merging but I'd prefer an extra review first", "how about we'd do this", etc. I'm all for being accommodating with submissions and slightly edit them if needed because I know that if the work has to be 100% on either side it can total more time than 90/10 or even 95/5 (but I never edit signed patches beyond fixing merging issues though). I'm just very careful about the time spent on my side because I know I'm already a bottleneck, so each extra minute added here delays everything else. Thanks! Willy
Re: [PR] BUILD: extend travis-ci matrix
I made another PR https://github.com/haproxy/haproxy/pull/92 (I really like automatic PR to mailing list routing) LibreSSL builds were ok when I sent a patch for the first time. seem there are some regression. actually, patch is almost identical (I rebased it to current master) вс, 5 мая 2019 г. в 23:29, Илья Шипицин : > something has changed. build is passing now, reg-tests fail (they used to > work earlier) > > > I'd apply those patches (and we will fix reg-tests later) > > вс, 5 мая 2019 г. в 15:51, Willy Tarreau : > >> On Sun, May 05, 2019 at 02:26:11PM +0500, ??? wrote: >> > can we also apply patch from >> > https://www.mail-archive.com/haproxy@formilux.org/msg33439.html ? >> > it should repair libressl builds >> >> Ah indeed I remember having noticed this one and postponed it because it >> needed to be edited to have a commit message and I didn't have the time >> to study it further. Could you please provide a commit message indicating >> what problem it fixes, how/when this problem manifests itself and how the >> patch fixes it ? >> >> Thanks! >> willy >> >
Re: [PR] BUILD: extend travis-ci matrix
something has changed. build is passing now, reg-tests fail (they used to
work earlier)
I'd apply those patches (and we will fix reg-tests later)
вс, 5 мая 2019 г. в 15:51, Willy Tarreau :
> On Sun, May 05, 2019 at 02:26:11PM +0500, ??? wrote:
> > can we also apply patch from
> > https://www.mail-archive.com/haproxy@formilux.org/msg33439.html ?
> > it should repair libressl builds
>
> Ah indeed I remember having noticed this one and postponed it because it
> needed to be edited to have a commit message and I didn't have the time
> to study it further. Could you please provide a commit message indicating
> what problem it fixes, how/when this problem manifests itself and how the
> patch fixes it ?
>
> Thanks!
> willy
>
From f59ae0892b1ec660ec62e1284048b004d6bc995a Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin
Date: Sun, 5 May 2019 23:27:54 +0500
Subject: [PATCH 3/3] BUILD: enable several LibreSSL hacks, including
SSL_SESSION_get0_id_context is introduced in LibreSSL-2.7.0
async operations are not supported by LibreSSL
early data is not supported by LibreSSL
packet_length is removed from SSL struct in LibreSSL
---
include/proto/openssl-compat.h | 4 ++--
include/proto/ssl_sock.h | 2 +-
src/cli.c | 2 +-
src/ssl_sock.c | 44 +-
4 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/include/proto/openssl-compat.h b/include/proto/openssl-compat.h
index ffee2e40..ed5c1ba3 100644
--- a/include/proto/openssl-compat.h
+++ b/include/proto/openssl-compat.h
@@ -89,9 +89,9 @@ static inline int SSL_SESSION_set1_id_context(SSL_SESSION *s, const unsigned cha
}
#endif
-#if (OPENSSL_VERSION_NUMBER < 0x101fL) || defined(LIBRESSL_VERSION_NUMBER)
+#if (OPENSSL_VERSION_NUMBER < 0x101fL) || (defined(LIBRESSL_VERSION_NUMBER) && (LIBRESSL_VERSION_NUMBER < 0x207fL))
/*
- * Functions introduced in OpenSSL 1.1.0 and not yet present in LibreSSL
+ * Functions introduced in OpenSSL 1.1.0 and in LibreSSL 2.7.0
*/
static inline const unsigned char *SSL_SESSION_get0_id_context(const SSL_SESSION *sess, unsigned int *sid_ctx_length)
diff --git a/include/proto/ssl_sock.h b/include/proto/ssl_sock.h
index ce52fb74..586ebb90 100644
--- a/include/proto/ssl_sock.h
+++ b/include/proto/ssl_sock.h
@@ -85,7 +85,7 @@ SSL_CTX *ssl_sock_get_generated_cert(unsigned int key, struct bind_conf *bind_co
int ssl_sock_set_generated_cert(SSL_CTX *ctx, unsigned int key, struct bind_conf *bind_conf);
unsigned int ssl_sock_generated_cert_key(const void *data, size_t len);
-#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC)
+#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC) && !defined(LIBRESSL_VERSION_NUMBER)
void ssl_async_fd_handler(int fd);
void ssl_async_fd_free(int fd);
#endif
diff --git a/src/cli.c b/src/cli.c
index 88fbae33..e91e33b3 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -1002,7 +1002,7 @@ static int cli_io_handler_show_fd(struct appctx *appctx)
(fdt.iocb == poller_pipe_io_handler) ? "poller_pipe_io_handler" :
(fdt.iocb == mworker_accept_wrapper) ? "mworker_accept_wrapper" :
#ifdef USE_OPENSSL
-#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC)
+#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC) && !defined(LIBRESSL_VERSION_NUMBER)
(fdt.iocb == ssl_async_fd_free) ? "ssl_async_fd_free" :
(fdt.iocb == ssl_async_fd_handler) ? "ssl_async_fd_handler" :
#endif
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index f2d80e8c..e11ddb53 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -57,7 +57,7 @@
#include
#endif
-#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC)
+#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC) && !defined(LIBRESSL_VERSION_NUMBER)
#include
#endif
@@ -575,7 +575,7 @@ fail_get:
}
#endif
-#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC)
+#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC) && !defined(LIBRESSL_VERSION_NUMBER)
/*
* openssl async fd handler
*/
@@ -2297,7 +2297,7 @@ static void ssl_sock_switchctx_set(SSL *ssl, SSL_CTX *ctx)
SSL_set_SSL_CTX(ssl, ctx);
}
-#if (OPENSSL_VERSION_NUMBER >= 0x10101000L) || defined(OPENSSL_IS_BORINGSSL)
+#if ((OPENSSL_VERSION_NUMBER >= 0x10101000L) || defined(OPENSSL_IS_BORINGSSL)) && !defined(LIBRESSL_VERSION_NUMBER)
static int ssl_sock_switchctx_err_cbk(SSL *ssl, int *al, void *priv)
{
@@ -4029,7 +4029,7 @@ ssl_sock_initial_ctx(struct bind_conf *bind_conf)
SSL_CTX_set_options(ctx, options);
-#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC)
+#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC) && !defined(LIBRESSL_VERSION_NUMBER)
if (global_ssl.async)
mode |= SSL_MODE_ASYNC;
#endif
@@ -4041,7 +4041,7 @@ ssl_sock_initial_ctx(struct bind_conf *bind_conf)
#ifdef
Re: [PR] BUILD: extend travis-ci matrix
On Sun, May 05, 2019 at 02:26:11PM +0500, ??? wrote: > can we also apply patch from > https://www.mail-archive.com/haproxy@formilux.org/msg33439.html ? > it should repair libressl builds Ah indeed I remember having noticed this one and postponed it because it needed to be edited to have a commit message and I didn't have the time to study it further. Could you please provide a commit message indicating what problem it fixes, how/when this problem manifests itself and how the patch fixes it ? Thanks! willy
Re: [PR] BUILD: extend travis-ci matrix
can we also apply patch from https://www.mail-archive.com/haproxy@formilux.org/msg33439.html ? it should repair libressl builds вс, 5 мая 2019 г. в 13:25, Willy Tarreau : > On Fri, May 03, 2019 at 10:23:05AM +, PR Bot wrote: > > Description: > >added openssl-1.0.2, 1.1.0, 1.1.1, libressl-2.7.5, 2.8.3, 2.9.1 > >added linux-ppc64le image > > Applied, thanks Ilya, > Willy >
Re: [PR] BUILD: extend travis-ci matrix
On Fri, May 03, 2019 at 10:23:05AM +, PR Bot wrote: > Description: >added openssl-1.0.2, 1.1.0, 1.1.1, libressl-2.7.5, 2.8.3, 2.9.1 >added linux-ppc64le image Applied, thanks Ilya, Willy
[PR] BUILD: extend travis-ci matrix
Dear list! Author: Ilya Shipitsin Number of patches: 1 This is an automated relay of the Github pull request: BUILD: extend travis-ci matrix Patch title(s): BUILD: extend travis-ci matrix Link: https://github.com/haproxy/haproxy/pull/91 Edit locally: wget https://github.com/haproxy/haproxy/pull/91.patch && vi 91.patch Apply locally: curl https://github.com/haproxy/haproxy/pull/91.patch | git am - Description: added openssl-1.0.2, 1.1.0, 1.1.1, libressl-2.7.5, 2.8.3, 2.9.1 added linux-ppc64le image libressl builds are yet broken. they will get repaired after separate patch (already sent to mailing list) Instructions: This github pull request will be closed automatically; patch should be reviewed on the haproxy mailing list (haproxy@formilux.org). Everyone is invited to comment, even the patch's author. Please keep the author and list CCed in replies. Please note that in absence of any response this pull request will be lost.
