Re: [PATCH] BUG/MAJOR: thread: lua: Wrong SSL context initialization.
On Thu, Aug 30, 2018 at 07:27:47PM +0200, PiBa-NL wrote: > Op 30-8-2018 om 10:07 schreef Willy Tarreau: > > On Thu, Aug 30, 2018 at 10:03:32AM +0200, Thierry Fournier wrote: > > > Hi Pieter, > > > > > > Your patch makes sense ! > > > Good catch. > > > Willy, could you apply ? > > OK now applied, thank you guys! > > > > Willy > > Willy, thanks. > > Thierry, for the record, its not my patch, Frederic made the patch. Only > thing i provided was a 'reg-test' and some gdb traces.. Not dismissing Fred's work, but for a developer, the most important is to get a reproducer. Once you have a reliable reproducer it can sometimes be much faster to understand the bug and fix it. So your reg test and your gdb traces were extremely valuable, and as long as you will be able to provide this, you will make some people very happy you see :-) Willy
Re: [PATCH] BUG/MAJOR: thread: lua: Wrong SSL context initialization.
Op 30-8-2018 om 10:07 schreef Willy Tarreau: On Thu, Aug 30, 2018 at 10:03:32AM +0200, Thierry Fournier wrote: Hi Pieter, Your patch makes sense ! Good catch. Willy, could you apply ? OK now applied, thank you guys! Willy Willy, thanks. Thierry, for the record, its not my patch, Frederic made the patch. Only thing i provided was a 'reg-test' and some gdb traces.. Anyhow its fixed now, that's the important part ;) Regards, PiBa-NL (Pieter)
Re: [PATCH] BUG/MAJOR: thread: lua: Wrong SSL context initialization.
On Thu, Aug 30, 2018 at 10:03:32AM +0200, Thierry Fournier wrote: > Hi Pieter, > > Your patch makes sense ! > Good catch. > Willy, could you apply ? OK now applied, thank you guys! Willy
Re: [PATCH] BUG/MAJOR: thread: lua: Wrong SSL context initialization.
Hi Pieter, Your patch makes sense ! Good catch. Willy, could you apply ? Thierry > On 29 Aug 2018, at 21:29, PiBa-NL wrote: > > Op 29-8-2018 om 14:29 schreef Olivier Houchard: >> On Wed, Aug 29, 2018 at 02:11:45PM +0200, Frederic Lecaille wrote: >>> This patch is in relation with one bug reproduced by the reg testing file >>> sent by Pieter in this thread: >>> https://www.mail-archive.com/haproxy@formilux.org/msg31079.html >>> >>> Must be checked by Thierry. >>> Must be backported to 1.8. >>> >>> Note that Pieter reg testing files reg-tests/lua/b2.* come with this >>> patch. >>> >>> >>> Fred. >>> From d6d38a354a89b55f91bb9962c5832a089d960b60 Mon Sep 17 00:00:00 2001 >>> From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= >>> Date: Wed, 29 Aug 2018 13:46:24 +0200 >>> Subject: [PATCH] BUG/MAJOR: thread: lua: Wrong SSL context initialization. >>> >>> When calling ->prepare_srv() callback for SSL server which >>> depends on global "nbthread" value, this latter was not already parsed, >>> so equal to 1 default value. This lead to bad memory accesses. >>> >>> Thank you to Pieter (PiBa-NL) for having reported this issue and >>> for having provided a very helpful reg testing file to reproduce >>> this issue (reg-test/lua/b2.*). >>> >> That sounds good, nice catch ! >> >> And yes thanks Pieter, as usual :) >> >> Olivier > > As you've probably already verified, the issue is indeed fixed with this > patch applied on top of master. > > Thanks Frederic & Olivier. > > @Thierry, can you give the 'all okay' ? (or not okay, if it needs a different > fix..) > > Regards, > PiBa-NL (Pieter) >
Re: [PATCH] BUG/MAJOR: thread: lua: Wrong SSL context initialization.
Op 29-8-2018 om 14:29 schreef Olivier Houchard: On Wed, Aug 29, 2018 at 02:11:45PM +0200, Frederic Lecaille wrote: This patch is in relation with one bug reproduced by the reg testing file sent by Pieter in this thread: https://www.mail-archive.com/haproxy@formilux.org/msg31079.html Must be checked by Thierry. Must be backported to 1.8. Note that Pieter reg testing files reg-tests/lua/b2.* come with this patch. Fred. From d6d38a354a89b55f91bb9962c5832a089d960b60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= Date: Wed, 29 Aug 2018 13:46:24 +0200 Subject: [PATCH] BUG/MAJOR: thread: lua: Wrong SSL context initialization. When calling ->prepare_srv() callback for SSL server which depends on global "nbthread" value, this latter was not already parsed, so equal to 1 default value. This lead to bad memory accesses. Thank you to Pieter (PiBa-NL) for having reported this issue and for having provided a very helpful reg testing file to reproduce this issue (reg-test/lua/b2.*). That sounds good, nice catch ! And yes thanks Pieter, as usual :) Olivier As you've probably already verified, the issue is indeed fixed with this patch applied on top of master. Thanks Frederic & Olivier. @Thierry, can you give the 'all okay' ? (or not okay, if it needs a different fix..) Regards, PiBa-NL (Pieter)
Re: [PATCH] BUG/MAJOR: thread: lua: Wrong SSL context initialization.
On Wed, Aug 29, 2018 at 02:11:45PM +0200, Frederic Lecaille wrote: > This patch is in relation with one bug reproduced by the reg testing file > sent by Pieter in this thread: > https://www.mail-archive.com/haproxy@formilux.org/msg31079.html > > Must be checked by Thierry. > Must be backported to 1.8. > > Note that Pieter reg testing files reg-tests/lua/b2.* come with this > patch. > > > Fred. > From d6d38a354a89b55f91bb9962c5832a089d960b60 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= > Date: Wed, 29 Aug 2018 13:46:24 +0200 > Subject: [PATCH] BUG/MAJOR: thread: lua: Wrong SSL context initialization. > > When calling ->prepare_srv() callback for SSL server which > depends on global "nbthread" value, this latter was not already parsed, > so equal to 1 default value. This lead to bad memory accesses. > > Thank you to Pieter (PiBa-NL) for having reported this issue and > for having provided a very helpful reg testing file to reproduce > this issue (reg-test/lua/b2.*). > That sounds good, nice catch ! And yes thanks Pieter, as usual :) Olivier
[PATCH] BUG/MAJOR: thread: lua: Wrong SSL context initialization.
This patch is in relation with one bug reproduced by the reg testing
file sent by Pieter in this thread:
https://www.mail-archive.com/haproxy@formilux.org/msg31079.html
Must be checked by Thierry.
Must be backported to 1.8.
Note that Pieter reg testing files reg-tests/lua/b2.* come with this
patch.
Fred.
>From d6d38a354a89b55f91bb9962c5832a089d960b60 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?=
Date: Wed, 29 Aug 2018 13:46:24 +0200
Subject: [PATCH] BUG/MAJOR: thread: lua: Wrong SSL context initialization.
When calling ->prepare_srv() callback for SSL server which
depends on global "nbthread" value, this latter was not already parsed,
so equal to 1 default value. This lead to bad memory accesses.
Thank you to Pieter (PiBa-NL) for having reported this issue and
for having provided a very helpful reg testing file to reproduce
this issue (reg-test/lua/b2.*).
Must be backported to 1.8.
---
reg-tests/lua/b2.lua | 180 +++
reg-tests/lua/b2.vtc | 33 +++
reg-tests/lua/b2_print_r.lua | 96 +
reg-tests/lua/common.pem | 1 +
src/hlua.c | 19 +++--
5 files changed, 321 insertions(+), 8 deletions(-)
create mode 100644 reg-tests/lua/b2.lua
create mode 100644 reg-tests/lua/b2.vtc
create mode 100644 reg-tests/lua/b2_print_r.lua
create mode 12 reg-tests/lua/common.pem
diff --git a/reg-tests/lua/b2.lua b/reg-tests/lua/b2.lua
new file mode 100644
index ..41e5eeeb
--- /dev/null
+++ b/reg-tests/lua/b2.lua
@@ -0,0 +1,180 @@
+Luacurl = {}
+Luacurl.__index = Luacurl
+setmetatable(Luacurl, {
+ __call = function (cls, ...)
+ return cls.new(...)
+ end,
+})
+function Luacurl.new(server, port, ssl)
+ local self = setmetatable({}, Luacurl)
+ self.sockconnected = false
+ self.server = server
+ self.port = port
+ self.ssl = ssl
+ self.cookies = {}
+ return self
+end
+
+function Luacurl:get(method,url,headers,data)
+ core.Info("MAKING SOCKET")
+ if self.sockconnected == false then
+ self.sock = core.tcp()
+ if self.ssl then
+ local r = self.sock:connect_ssl(self.server,self.port)
+ else
+ local r = self.sock:connect(self.server,self.port)
+ end
+ self.sockconnected = true
+ end
+ core.Info("SOCKET MADE")
+ local request = method.." "..url.." HTTP/1.1"
+ if data ~= nil then
+ request = request .. "\r\nContent-Length: "..string.len(data)
+ end
+ if headers ~= null then
+ for h,v in pairs(headers) do
+ request = request .. "\r\n"..h..": "..v
+ end
+ end
+ cookstring = ""
+ for cook,cookval in pairs(self.cookies) do
+ cookstring = cookstring .. cook.."="..cookval.."; "
+ end
+ if string.len(cookstring) > 0 then
+ request = request .. "\r\nCookie: "..cookstring
+ end
+
+ request = request .. "\r\n\r\n"
+ if data and string.len(data) > 0 then
+ request = request .. data
+ end
+--print(request)
+ core.Info("SENDING REQUEST")
+ self.sock:send(request)
+
+-- core.Info("PROCESSING RESPONSE")
+ return processhttpresponse(self.sock)
+end
+
+function processhttpresponse(socket)
+ local res = {}
+core.Info("1")
+ res.status = socket:receive("*l")
+core.Info("2")
+
+ if res.status == nil then
+ core.Info(" processhttpresponse RECEIVING status: NIL")
+ return res
+ end
+ core.Info(" processhttpresponse RECEIVING status:"..res.status)
+ res.headers = {}
+ res.headerslist = {}
+ repeat
+core.Info("3")
+ local header = socket:receive("*l")
+ if header == nil then
+ return "error"
+ end
+ local valuestart = header:find(":")
+ if valuestart ~= nil then
+ local head = header:sub(1,valuestart-1)
+ local value = header:sub(valuestart+2)
+ table.insert(res.headerslist, {head,value})
+ res.headers[head] = value
+ end
+ until header == ""
+ local bodydone = false
+ if res.headers["Connection"] ~= nil and res.headers["Connection"] == "close" then
+-- core.Info("luacurl processresponse with connection:close")
+ res.body = ""
+ repeat
+core.Info("4")
+ local d = socket:receive("*a")
+ if d ~= nil then
+res.body = res.body .. d
+ end
+ until d == nil or d == 0
+ bodydone = true
+ end
+ if bodydone == false and res.headers["Content-Length"] ~= nil then
+ res.contentlength = tonumber(res.headers["Content-Length"])
+ if res.contentlength == nil then
+ core.Warning("res.contentlength ~NIL = "..res.headers["Content-Length"])
+ end
+-- core.Info("luacur, contentlength="..res.contentlength)
+ res.body = ""
+ repeat
+ local d = socket:receive(res.contentlength)
+ if d == nil then
+--core.Info("luacurl,
