Welcome! Log In Create A New Profile

Advanced

ModSecurity: First integration patches

Posted by Thierry Fournier 
Thierry Fournier
ModSecurity: First integration patches
April 11, 2017 11:00AM
Hi list

I join one usage of HAProxy / SPOE, it is WAF offloading.

These patches are a first version, it have some limitations describe
in the README file in the directory contrib/modsecurity.

- Christopher, please check the patch "BUG/MINOR", it is about spoe
functions.

- The exemple of ModSecurity compilation can be improved. It is based
on my local distro.

The feedback are welcome.

Thierry
Olivier Doucet
Re: ModSecurity: First integration patches
April 11, 2017 11:30AM
Hi Thierry,



2017-04-11 10:49 GMT+02:00 Thierry Fournier <[email protected]>:

> Hi list
>
> I join one usage of HAProxy / SPOE, it is WAF offloading.
>
> These patches are a first version, it have some limitations describe
> in the README file in the directory contrib/modsecurity.
>
> - Christopher, please check the patch "BUG/MINOR", it is about spoe
> functions.
>
> - The exemple of ModSecurity compilation can be improved. It is based
> on my local distro.
>
> The feedback are welcome.
>

Nice work for a first version ! I definitely see what great production
usage could be done with it.

Your README file referenced several issues in ModSecurity itself, this is
not something I would expect from this highly used software ...

What happened if for some reason modSecurity does not answer in the timeout
defined ? What happened if modsecurity throws an error ?


Olivier
Thierry Fournier
Re: ModSecurity: First integration patches
April 11, 2017 11:50AM
> On 11 Apr 2017, at 11:24, Olivier Doucet <[email protected]> wrote:
>
> Hi Thierry,
>
>
>
> 2017-04-11 10:49 GMT+02:00 Thierry Fournier <[email protected]>:
> Hi list
>
> I join one usage of HAProxy / SPOE, it is WAF offloading.
>
> These patches are a first version, it have some limitations describe
> in the README file in the directory contrib/modsecurity.
>
> - Christopher, please check the patch "BUG/MINOR", it is about spoe
> functions.
>
> - The exemple of ModSecurity compilation can be improved. It is based
> on my local distro.
>
> The feedback are welcome.
>
> Nice work for a first version ! I definitely see what great production usage could be done with it.


thanks.


> Your README file referenced several issues in ModSecurity itself, this is not something I would expect from this highly used software …


If you’re taking about the section ModSecurity bug, you must keep in mind that
ModSecurity is written for Apache. The integration in HAProxy or NGINX is not
“natural” for these soft. (Hi hope that the V3 will embed a more compatible API).

The first bug is maybe due to my build system. In first time, I compile my own
version of APR, and maybe it is caused by a bad compile parameter. In second
time, I used APR from system, and I have the same bug.

I have found some posts on Internet about this bug, but ant way to fix it.
I suppose that this bug is not available with Apache. The configuration way to
avoir this bug is using this conf: “SecAuditLogType Concurrent"

The second bug is averred only with HAProxy and NGINX. ModSecurity with
Apache doesn’t have this bug. I will be avoided without using wildcards. So, you can
lists each included configuration file.


> What happened if for some reason modSecurity does not answer in the timeout defined ?


Good question. I suppose that the answer is around the SPOE timeouts.


> What happened if modsecurity throws an error ?


The variable “txn.modsec.code” is not set. So this variable is set to 0 if all is good,
not set if an error occurs, or it contains the recommended HTTP return code.

Thierry


> Olivier
>
Aleksandar Lazic
Re: ModSecurity: First integration patches
April 12, 2017 10:10AM
Am 11-04-2017 10:49, schrieb Thierry Fournier:
> Hi list
>
> I join one usage of HAProxy / SPOE, it is WAF offloading.
>
> These patches are a first version, it have some limitations describe
> in the README file in the directory contrib/modsecurity.
>
> - Christopher, please check the patch "BUG/MINOR", it is about spoe
> functions.
>
> - The exemple of ModSecurity compilation can be improved. It is based
> on my local distro.
>
> The feedback are welcome.

I agree with Olivier that's really great stuff ;-)

To which branch (I assume master) can I apply this patch?

I will then create a docker image for testing ;-)

> Thierry

Aleks
Thierry Fournier
Re: ModSecurity: First integration patches
April 12, 2017 10:20AM
> On 12 Apr 2017, at 09:57, Aleksandar Lazic <[email protected]> wrote:
>
>
>
> Am 11-04-2017 10:49, schrieb Thierry Fournier:
>> Hi list
>> I join one usage of HAProxy / SPOE, it is WAF offloading.
>> These patches are a first version, it have some limitations describe
>> in the README file in the directory contrib/modsecurity.
>> - Christopher, please check the patch "BUG/MINOR", it is about spoe
>> functions.
>> - The exemple of ModSecurity compilation can be improved. It is based
>> on my local distro.
>> The feedback are welcome.
>
> I agree with Olivier that's really great stuff ;-)


thanks.


> To which branch (I assume master) can I apply this patch?


You’re right. The core haproxy patch is very light, I guess that the patch will be applied on any branch from 1.7.0 (needs SPOE)


> I will then create a docker image for testing ;-)


Great !


>> Thierry
>
> Aleks
Christopher Faulet
Re: ModSecurity: First integration patches
April 12, 2017 11:00AM
Le 11/04/2017 à 10:49, Thierry Fournier a écrit :
> Hi list
>
> I join one usage of HAProxy / SPOE, it is WAF offloading.
>
> These patches are a first version, it have some limitations describe
> in the README file in the directory contrib/modsecurity.
>
> - Christopher, please check the patch "BUG/MINOR", it is about spoe
> functions.
>
> - The exemple of ModSecurity compilation can be improved. It is based
> on my local distro.
>
> The feedback are welcome.
>

Hi Thierry,

Really nice ! I'll take a look at it soon. Glad to see the first service
that uses the SPOE ! Good job.

--
Christopher Faulet
Aleksandar Lazic
Re: ModSecurity: First integration patches
April 12, 2017 09:30PM
Hi.

Am 12-04-2017 10:08, schrieb Thierry Fournier:
>> On 12 Apr 2017, at 09:57, Aleksandar Lazic <[email protected]> wrote:
>>
>>
>>
>> Am 11-04-2017 10:49, schrieb Thierry Fournier:
>>> Hi list
>>> I join one usage of HAProxy / SPOE, it is WAF offloading.
>>> These patches are a first version, it have some limitations describe
>>> in the README file in the directory contrib/modsecurity.
>>> - Christopher, please check the patch "BUG/MINOR", it is about spoe
>>> functions.
>>> - The exemple of ModSecurity compilation can be improved. It is based
>>> on my local distro.
>>> The feedback are welcome.
>>
>> I agree with Olivier that's really great stuff ;-)
>
>
> thanks.
>
>
>> To which branch (I assume master) can I apply this patch?
>
>
> You’re right. The core haproxy patch is very light, I guess that the
> patch will be applied on any branch from 1.7.0 (needs SPOE)
>
>
>> I will then create a docker image for testing ;-)
>
>
> Great !

Do you have the patches as files where I can download it?
It's easier for docker to call a 'curl -vLO ...' then to go across a
mail body ;-)

>>> Thierry
>>
>> Aleks
Anonymous User
Re: ModSecurity: First integration patches
April 12, 2017 09:40PM
On Wed, 12 Apr 2017 21:21:58 +0200
Aleksandar Lazic <[email protected]> wrote:

> Hi.
>
> Am 12-04-2017 10:08, schrieb Thierry Fournier:
> >> On 12 Apr 2017, at 09:57, Aleksandar Lazic <[email protected]> wrote:
> >>
> >>
> >>
> >> Am 11-04-2017 10:49, schrieb Thierry Fournier:
> >>> Hi list
> >>> I join one usage of HAProxy / SPOE, it is WAF offloading.
> >>> These patches are a first version, it have some limitations describe
> >>> in the README file in the directory contrib/modsecurity.
> >>> - Christopher, please check the patch "BUG/MINOR", it is about spoe
> >>> functions.
> >>> - The exemple of ModSecurity compilation can be improved. It is based
> >>> on my local distro.
> >>> The feedback are welcome.
> >>
> >> I agree with Olivier that's really great stuff ;-)
> >
> >
> > thanks.
> >
> >
> >> To which branch (I assume master) can I apply this patch?
> >
> >
> > You’re right. The core haproxy patch is very light, I guess that the
> > patch will be applied on any branch from 1.7.0 (needs SPOE)
> >
> >
> >> I will then create a docker image for testing ;-)
> >
> >
> > Great !
>
> Do you have the patches as files where I can download it?
> It's easier for docker to call a 'curl -vLO ...' then to go across a
> mail body ;-)


Not sure to understand. I given the patches as file. Note that I'm
testing new email client. So I put the patches here:

http://www.arpalert.org/0001-BUG-MINOR-change-header-declared-function-to-static-.patch
http://www.arpalert.org/0002-MINOR-Add-binary-encoding-request-sample-fetch.patch
http://www.arpalert.org/0003-MINOR-Add-ModSecurity-wrapper-as-contrib.patch

Thierry

> >>> Thierry
> >>
> >> Aleks
>
Aleksandar Lazic
Re: ModSecurity: First integration patches
April 12, 2017 11:40PM
Am 12-04-2017 21:28, schrieb thierry.fournier@arpalert.org:
> On Wed, 12 Apr 2017 21:21:58 +0200
> Aleksandar Lazic <[email protected]> wrote:
>
>> Hi.
>>
>> Am 12-04-2017 10:08, schrieb Thierry Fournier:
>> >> On 12 Apr 2017, at 09:57, Aleksandar Lazic <[email protected]> wrote:
>> >>
>> >>
>> >>
>> >> Am 11-04-2017 10:49, schrieb Thierry Fournier:
>> >>> Hi list
>> >>> I join one usage of HAProxy / SPOE, it is WAF offloading.
>> >>> These patches are a first version, it have some limitations describe
>> >>> in the README file in the directory contrib/modsecurity.
>> >>> - Christopher, please check the patch "BUG/MINOR", it is about spoe
>> >>> functions.
>> >>> - The exemple of ModSecurity compilation can be improved. It is based
>> >>> on my local distro.
>> >>> The feedback are welcome.
>> >>
>> >> I agree with Olivier that's really great stuff ;-)
>> >
>> >
>> > thanks.
>> >
>> >
>> >> To which branch (I assume master) can I apply this patch?
>> >
>> >
>> > You’re right. The core haproxy patch is very light, I guess that the
>> > patch will be applied on any branch from 1.7.0 (needs SPOE)
>> >
>> >
>> >> I will then create a docker image for testing ;-)
>> >
>> >
>> > Great !
>>
>> Do you have the patches as files where I can download it?
>> It's easier for docker to call a 'curl -vLO ...' then to go across a
>> mail body ;-)
>
>
> Not sure to understand. I given the patches as file. Note that I'm
> testing new email client. So I put the patches here:
>
> http://www.arpalert.org/0001-BUG-MINOR-change-header-declared-function-to-static-.patch
> http://www.arpalert.org/0002-MINOR-Add-binary-encoding-request-sample-fetch.patch
> http://www.arpalert.org/0003-MINOR-Add-ModSecurity-wrapper-as-contrib.patch

I'm so sorry for the rush. :-(

I have seen to late that you have send the patches to the list.

Thanks for the links. I will take more care in the future.

> Thierry

Aleks
Aleksandar Lazic
Re: ModSecurity: First integration patches
April 13, 2017 02:10AM
Am 12-04-2017 23:33, schrieb Aleksandar Lazic:
> Am 12-04-2017 21:28, schrieb thierry.fournier@arpalert.org:
>> On Wed, 12 Apr 2017 21:21:58 +0200
>> Aleksandar Lazic <[email protected]> wrote:

[snipp]

>>> Do you have the patches as files where I can download it?
>>> It's easier for docker to call a 'curl -vLO ...' then to go across a
>>> mail body ;-)
>>
>> Not sure to understand. I given the patches as file. Note that I'm
>> testing new email client. So I put the patches here:
>>
>> http://www.arpalert.org/0001-BUG-MINOR-change-header-declared-function-to-static-.patch
>> http://www.arpalert.org/0002-MINOR-Add-binary-encoding-request-sample-fetch.patch
>> http://www.arpalert.org/0003-MINOR-Add-ModSecurity-wrapper-as-contrib.patch
>
> I'm so sorry for the rush. :-(
>
> I have seen to late that you have send the patches to the list.
>
> Thanks for the links. I will take more care in the future.

I have now build the haproxy with modsecurity on centos 7.3 ;-)

I have used this file for modsecurity.
https://github.com/SpiderLabs/owasp-modsecurity-crs/blob/v3.0/master/crs-setup.conf.example

###
/usr/local/bin/modsecurity -f crs-setup.conf.example
1492041223.145110 [00] ModSecurity for nginx (STABLE)/2.9.1
(http://www.modsecurity.org/) configured.
1492041223.145159 [00] ModSecurity: APR compiled version="1.4.8"; loaded
version="1.4.8"
1492041223.145193 [00] ModSecurity: PCRE compiled version="8.32 ";
loaded version="8.32 2012-11-30"
1492041223.145197 [00] ModSecurity: LIBXML compiled version="2.9.1"
1492041223.145200 [00] ModSecurity: Status engine is currently disabled,
enable it by set SecStatusEngine to On.
1492041228.152877 [01] 0 clients connected
1492041228.153037 [02] 0 clients connected
1492041228.153069 [03] 0 clients connected
....
###

It was a little bit challenging.

..) the patches apply only on haproxy 1.8 because some files does not
exists on 1.7 ( e. g. include/proto/spoe.h )

git clone http://git.haproxy.org/git/haproxy.git/

patch -d haproxy -p 1 -i
/usr/src/0001-BUG-MINOR-change-header-declared-function-to-static-.patch
patch -d haproxy -p 1 -i
/usr/src/0002-MINOR-Add-binary-encoding-request-sample-fetch.patch
patch -d haproxy -p 1 -i
/usr/src/0003-MINOR-Add-ModSecurity-wrapper-as-contrib.patch

..) you will need a lot of devel packages inclusive some httpd one.

yum install -y apr-devel apr-util-devel gcc make libevent-devel
libxml2-devel libcurl-devel httpd-devel pcre-devel yajl-devel

..) I will figure out which runtime packages will be necessary.
..) I have started a Dockerfile which you can find at github.

https://github.com/git001/haproxy-waf/blob/master/Dockerfile

Open questions for me.

..) How is the transfer-encoding handled (a. k. a. streaming)?
..) How big can a content be? Where can we define some limits?
..) How can the rule-set be reloaded? stop & start || gracefully?

Again thanks Thierry for your work this looks very good.

Regards
Aleks
Thierry Fournier
Re: ModSecurity: First integration patches
April 13, 2017 12:30PM
> On 13 Apr 2017, at 02:06, Aleksandar Lazic <[email protected]> wrote:
>
>
>
> Am 12-04-2017 23:33, schrieb Aleksandar Lazic:
>> Am 12-04-2017 21:28, schrieb thierry.fournier@arpalert.org:
>>> On Wed, 12 Apr 2017 21:21:58 +0200
>>> Aleksandar Lazic <[email protected]> wrote:
>
> [snipp]
>
>>>> Do you have the patches as files where I can download it?
>>>> It's easier for docker to call a 'curl -vLO ...' then to go across a
>>>> mail body ;-)
>>> Not sure to understand. I given the patches as file. Note that I'm
>>> testing new email client. So I put the patches here:
>>> http://www.arpalert.org/0001-BUG-MINOR-change-header-declared-function-to-static-.patch
>>> http://www.arpalert.org/0002-MINOR-Add-binary-encoding-request-sample-fetch.patch
>>> http://www.arpalert.org/0003-MINOR-Add-ModSecurity-wrapper-as-contrib.patch
>> I'm so sorry for the rush. :-(
>> I have seen to late that you have send the patches to the list.
>> Thanks for the links. I will take more care in the future.
>
> I have now build the haproxy with modsecurity on centos 7.3 ;-)
>
> I have used this file for modsecurity.
> https://github.com/SpiderLabs/owasp-modsecurity-crs/blob/v3.0/master/crs-setup.conf.example
>
> ###
> /usr/local/bin/modsecurity -f crs-setup.conf.example
> 1492041223.145110 [00] ModSecurity for nginx (STABLE)/2.9.1 (http://www.modsecurity.org/) configured.
> 1492041223.145159 [00] ModSecurity: APR compiled version="1.4.8"; loaded version="1.4.8"
> 1492041223.145193 [00] ModSecurity: PCRE compiled version="8.32 "; loaded version="8.32 2012-11-30"
> 1492041223.145197 [00] ModSecurity: LIBXML compiled version="2.9.1"
> 1492041223.145200 [00] ModSecurity: Status engine is currently disabled, enable it by set SecStatusEngine to On.
> 1492041228.152877 [01] 0 clients connected
> 1492041228.153037 [02] 0 clients connected
> 1492041228.153069 [03] 0 clients connected
> ...
> ###
>
> It was a little bit challenging.
>
> .) the patches apply only on haproxy 1.8 because some files does not exists on 1.7 ( e. g. include/proto/spoe.h )


Ok. I think that SPOE was introduced in 1.7, obviously I’m wrong.


> git clone http://git.haproxy.org/git/haproxy.git/
>
> patch -d haproxy -p 1 -i /usr/src/0001-BUG-MINOR-change-header-declared-function-to-static-.patch
> patch -d haproxy -p 1 -i /usr/src/0002-MINOR-Add-binary-encoding-request-sample-fetch.patch
> patch -d haproxy -p 1 -i /usr/src/0003-MINOR-Add-ModSecurity-wrapper-as-contrib.patch
>
> .) you will need a lot of devel packages inclusive some httpd one.
>
> yum install -y apr-devel apr-util-devel gcc make libevent-devel libxml2-devel libcurl-devel httpd-devel pcre-devel yajl-devel


Yes Modsecurity is linked designed for apache and needs Apache libraries (APR), libevent is for
the SPOA. libcurl and yajl are used for the Modsecurity “mlogc” function.


> .) I will figure out which runtime packages will be necessary.
> .) I have started a Dockerfile which you can find at github.
>
> https://github.com/git001/haproxy-waf/blob/master/Dockerfile
>
> Open questions for me.


Note: I swapped the order of your questions


> .) How big can a content be? Where can we define some limits?


ModSecurity analyses an Haproxy buffer. (don’t forget the directive “option http-buffer-request”)
For my own usage, the HAProxy buffer are configured as 1MB. When the buffer is full or when
the http request is receive, all the data are offloaded towards ModSecurity.


> .) How is the transfer-encoding handled (a. k. a. streaming)?


The stream is not processed, just the first buffer containing the header request and a maximum
of the body it is.


> .) How can the rule-set be reloaded? stop & start || gracefully?


I do not process this part. Today, you must stop and start the process. The graceful doesn’t exists.
I guess than the graceful can be implemented easily. You can ensure the availability of the
SPOA Modsec using the properties of the HAProxy backend.


> Again thanks Thierry for your work this looks very good.


Thanks for testing.

Thierry


> Regards
> Aleks
Willy Tarreau
Re: ModSecurity: First integration patches
April 13, 2017 12:40PM
On Thu, Apr 13, 2017 at 12:21:20PM +0200, Thierry Fournier wrote:
> > .) the patches apply only on haproxy 1.8 because some files does not exists on 1.7 ( e. g. include/proto/spoe.h )
>
>
> Ok. I think that SPOE was introduced in 1.7, obviously I'm wrong.

No, it was introduced in 1.7 but there were some improvements later
(like pipelining etc).

(...)
> > .) How can the rule-set be reloaded? stop & start || gracefully?
>
>
> I do not process this part. Today, you must stop and start the process. The graceful doesn't exists.
> I guess than the graceful can be implemented easily. You can ensure the availability of the
> SPOA Modsec using the properties of the HAProxy backend.

Actually that's a very good point. I think it would even be possible to
ensure a graceful shutdown using disable-on-404 or using an agent so
that you can roll the restart over multiple WAF nodes.

Willy
Thierry Fournier
Re: ModSecurity: First integration patches
April 13, 2017 01:00PM
> On 13 Apr 2017, at 12:28, Willy Tarreau <[email protected]> wrote:
>
> On Thu, Apr 13, 2017 at 12:21:20PM +0200, Thierry Fournier wrote:
>>> .) the patches apply only on haproxy 1.8 because some files does not exists on 1.7 ( e. g. include/proto/spoe.h )
>>
>>
>> Ok. I think that SPOE was introduced in 1.7, obviously I'm wrong.
>
> No, it was introduced in 1.7 but there were some improvements later
> (like pipelining etc).
>
> (...)
>>> .) How can the rule-set be reloaded? stop & start || gracefully?
>>
>>
>> I do not process this part. Today, you must stop and start the process. The graceful doesn't exists.
>> I guess than the graceful can be implemented easily. You can ensure the availability of the
>> SPOA Modsec using the properties of the HAProxy backend.
>
> Actually that's a very good point. I think it would even be possible to
> ensure a graceful shutdown using disable-on-404 or using an agent so
> that you can roll the restart over multiple WAF nodes.


Interesting. I think about a system which (on SPOA side) stop listeners
and wait for the end of processing current requests. By this way, the SPOA
doesn’t accept requests, and HAProxy send requests on the other process.
Another way is using the CLI and set one spoa/modsec in graceful mode.

Adding a special check is the best way, but the daemon speaks SPOP and not
HTTP. Maybe a thread which listen on specific port dedicated to this
function ? Or improving the SPOP for asking graceful mode in the agent-hello
response message ? (it seems that haproxy send periodically haproxy-hello
messages, but maybe I’m wrong)

Thierry


> Willy
Christopher Faulet
Re: ModSecurity: First integration patches
April 13, 2017 01:30PM
Le 13/04/2017 à 12:53, Thierry Fournier a écrit :
>
>> On 13 Apr 2017, at 12:28, Willy Tarreau <[email protected]> wrote:
>>
>> On Thu, Apr 13, 2017 at 12:21:20PM +0200, Thierry Fournier wrote:
>>>> .) the patches apply only on haproxy 1.8 because some files does not exists on 1.7 ( e. g. include/proto/spoe.h )
>>>
>>>
>>> Ok. I think that SPOE was introduced in 1.7, obviously I'm wrong.
>>
>> No, it was introduced in 1.7 but there were some improvements later
>> (like pipelining etc).
>>
>> (...)
>>>> .) How can the rule-set be reloaded? stop & start || gracefully?
>>>
>>>
>>> I do not process this part. Today, you must stop and start the process. The graceful doesn't exists.
>>> I guess than the graceful can be implemented easily. You can ensure the availability of the
>>> SPOA Modsec using the properties of the HAProxy backend.
>>
>> Actually that's a very good point. I think it would even be possible to
>> ensure a graceful shutdown using disable-on-404 or using an agent so
>> that you can roll the restart over multiple WAF nodes.
>
>
> Interesting. I think about a system which (on SPOA side) stop listeners
> and wait for the end of processing current requests. By this way, the SPOA
> doesn’t accept requests, and HAProxy send requests on the other process.
> Another way is using the CLI and set one spoa/modsec in graceful mode.
>
> Adding a special check is the best way, but the daemon speaks SPOP and not
> HTTP. Maybe a thread which listen on specific port dedicated to this
> function ? Or improving the SPOP for asking graceful mode in the agent-hello
> response message ? (it seems that haproxy send periodically haproxy-hello
> messages, but maybe I’m wrong)
>

The hello-handshake is done only once, when a new connection with a SPOA
is opened. But we can improve the SPOP by adding a new frame type to
handle admin tasks (like graceful stop). This way, for a specific
connection, it would be possible to wait for last ACK frames without
sending new frames to the SPOA. Then stopping the SPOA listeners to let
the SPOP health check failed should do the trick, I guess.

--
Christopher Faulet
Aleksandar Lazic
Re: ModSecurity: First integration patches
April 14, 2017 10:30AM
Hi.

Am 13-04-2017 02:06, schrieb Aleksandar Lazic:
> Am 12-04-2017 23:33, schrieb Aleksandar Lazic:
>> Am 12-04-2017 21:28, schrieb thierry.fournier@arpalert.org:
>>> On Wed, 12 Apr 2017 21:21:58 +0200
>>> Aleksandar Lazic <[email protected]> wrote:
>
> [snipp]
>
>>>> Do you have the patches as files where I can download it?
>>>> It's easier for docker to call a 'curl -vLO ...' then to go across a
>>>> mail body ;-)
>>>
>>> Not sure to understand. I given the patches as file. Note that I'm
>>> testing new email client. So I put the patches here:
>>>
>>> http://www.arpalert.org/0001-BUG-MINOR-change-header-declared-function-to-static-.patch
>>> http://www.arpalert.org/0002-MINOR-Add-binary-encoding-request-sample-fetch.patch
>>> http://www.arpalert.org/0003-MINOR-Add-ModSecurity-wrapper-as-contrib.patch

[snipp]

Willy can you please commit this patches.

Thank you very much.

Cheers.
Aleks
Willy Tarreau
Re: ModSecurity: First integration patches
April 14, 2017 11:40AM
On Thu, Apr 13, 2017 at 01:17:07PM +0200, Christopher Faulet wrote:
> The hello-handshake is done only once, when a new connection with a SPOA is
> opened. But we can improve the SPOP by adding a new frame type to handle
> admin tasks (like graceful stop). This way, for a specific connection, it
> would be possible to wait for last ACK frames without sending new frames to
> the SPOA. Then stopping the SPOA listeners to let the SPOP health check
> failed should do the trick, I guess.

For this you need a "closing" state for your connections, where you know
they are not eligible to new requests. That's pretty much similar to what
we do by setting a server weight to zero in HTTP load balancing.

But such transient states for connections can quickly become tricky to
handle. The first thing is that you want to be sure that the replacement
process is ready or the agent has reported a zero weight before starting
to notify about the closing state, otherwise you'll end up with single-
request connections that quickly pile up and can even take CPU if they're
made over SSL. And it's very important not to conflate the agent state
with the connection state and to avoid reporting the agent state inside
existing connections. The problem when doing this precisely is during
reloads where both old and new connections coexist and report conflicting
information. That's why health checks and/or agent checks are much better
to get the active SPOA health.

For example the agent's health and willingness to accept new connections
could be reported during the handshake. This way a health check can be
developped for this and there's no risk that such information could be
misreported later and cause trouble.

Willy
Willy Tarreau
Re: ModSecurity: First integration patches
April 14, 2017 11:50AM
Hi Aleks,

On Fri, Apr 14, 2017 at 10:25:56AM +0200, Aleksandar Lazic wrote:
> Willy can you please commit this patches.

I'm fine with this, but I prefer to give some time to Christopher to
review this, as Thierry asked. It can take quite some time since the
purpse of cutting the development cycle into 3 phases precisely was
to let developers stop spending time reviewing code and focus on their
code instead :-)

But here it's going to affect only third-party code in contrib/ so the
risks for haproxy's stability don't exist and we can just take a look.
I just want not to interrupt Chris right now.

Cheers,
Willy
Aleksandar Lazic
Re: ModSecurity: First integration patches
April 14, 2017 12:00PM
Hi Willy.

Am 14-04-2017 11:41, schrieb Willy Tarreau:
> Hi Aleks,
>
> On Fri, Apr 14, 2017 at 10:25:56AM +0200, Aleksandar Lazic wrote:
>> Willy can you please commit this patches.
>
> I'm fine with this, but I prefer to give some time to Christopher to
> review this, as Thierry asked. It can take quite some time since the
> purpse of cutting the development cycle into 3 phases precisely was
> to let developers stop spending time reviewing code and focus on their
> code instead :-)

;-)

> But here it's going to affect only third-party code in contrib/ so the
> risks for haproxy's stability don't exist and we can just take a look.
> I just want not to interrupt Chris right now.

Perfect thanks.

> Cheers,
> Willy

cheers
Aleks
Christopher Faulet
Re: ModSecurity: First integration patches
April 18, 2017 12:20PM
Le 12/04/2017 à 10:49, Christopher Faulet a écrit :
> Le 11/04/2017 à 10:49, Thierry Fournier a écrit :
>> Hi list
>>
>> I join one usage of HAProxy / SPOE, it is WAF offloading.
>>
>> These patches are a first version, it have some limitations describe
>> in the README file in the directory contrib/modsecurity.
>>
>> - Christopher, please check the patch "BUG/MINOR", it is about spoe
>> functions.
>>
>> - The exemple of ModSecurity compilation can be improved. It is based
>> on my local distro.
>>
>> The feedback are welcome.
>>
>
> Hi Thierry,
>
> Really nice ! I'll take a look at it soon. Glad to see the first service
> that uses the SPOE ! Good job.
>

Hi Thierry,

I finally took the time to review your patches, mainly the second one,
about the sample fetch. I think it would be pity to introduced such
complex sample fetch. All parts, except the HTTP headers, are already
available in dedicated sample fetches. It could be better to only add a
sample fetch to get HTTP headers (req.hdrs and res.hdrs, something like
that). Because a sample fetch cannot return a list, we can probably
encode it into a binary buffer using a \0 as separator. something like:


<num-of-headers><header-name>\0<header-value>\0<header-name>\0<header-value>\0...

This way, the sample fetch does not depend on the SPOE and can be used
in another context. And concerning your SPOA, this will be quite easy to
parse it.

About the SPOA, it seems to be ok. The server part is based on the SPOA
example so it should be ok (or you can blame me for all bugs :) For the
mod_security part, I blindly trust you.

--
Christopher Faulet
Willy Tarreau
Re: ModSecurity: First integration patches
April 18, 2017 02:50PM
On Tue, Apr 18, 2017 at 12:15:20PM +0200, Christopher Faulet wrote:
> I finally took the time to review your patches, mainly the second one, about
> the sample fetch. I think it would be pity to introduced such complex sample
> fetch. All parts, except the HTTP headers, are already available in
> dedicated sample fetches. It could be better to only add a sample fetch to
> get HTTP headers (req.hdrs and res.hdrs, something like that). Because a
> sample fetch cannot return a list, we can probably encode it into a binary
> buffer using a \0 as separator. something like:
>
>
> <num-of-headers><header-name>\0<header-value>\0<header-name>\0<header-value>\0...
>
> This way, the sample fetch does not depend on the SPOE and can be used in
> another context. And concerning your SPOA, this will be quite easy to parse
> it.

Actually I'm not quite sure about this one, such an encoding could in fact
make it harder to process while most use cases will either want to log it
(thus copy the whole string as-is) or decode it as received (and might have
to rebuild the initial header block with CRLF in between).

However I agree that having something like req.hdrs / res.hdrs could be
useful instead of having the whole block containing all the buffer. We
already have "req.body" for the body as a binary block, so it totally
makes sense to have "req.hdrs" for the same purpose, and let agents deal
with either one or the other or both.

Willy
Christopher Faulet
Re: ModSecurity: First integration patches
April 18, 2017 03:10PM
Le 18/04/2017 à 14:40, Willy Tarreau a écrit :
> On Tue, Apr 18, 2017 at 12:15:20PM +0200, Christopher Faulet wrote:
>> I finally took the time to review your patches, mainly the second one, about
>> the sample fetch. I think it would be pity to introduced such complex sample
>> fetch. All parts, except the HTTP headers, are already available in
>> dedicated sample fetches. It could be better to only add a sample fetch to
>> get HTTP headers (req.hdrs and res.hdrs, something like that). Because a
>> sample fetch cannot return a list, we can probably encode it into a binary
>> buffer using a \0 as separator. something like:
>>
>>
>> <num-of-headers><header-name>\0<header-value>\0<header-name>\0<header-value>\0...
>>
>> This way, the sample fetch does not depend on the SPOE and can be used in
>> another context. And concerning your SPOA, this will be quite easy to parse
>> it.
>
> Actually I'm not quite sure about this one, such an encoding could in fact
> make it harder to process while most use cases will either want to log it
> (thus copy the whole string as-is) or decode it as received (and might have
> to rebuild the initial header block with CRLF in between).
>

My first idea was to avoid the headers parsing in the SPOA. Because it
was already done by HAProxy, it could be cool to not redo it. Maybe the
sample fetch can take an argument to choose the format (raw or encoded).
The format is open. \0 may not be the better separator. This is just a
trick to deal with a list, like req.hdr_names.

> However I agree that having something like req.hdrs / res.hdrs could be
> useful instead of having the whole block containing all the buffer. We
> already have "req.body" for the body as a binary block, so it totally
> makes sense to have "req.hdrs" for the same purpose, and let agents deal
> with either one or the other or both.
>

--
Christopher Faulet
Willy Tarreau
Re: ModSecurity: First integration patches
April 18, 2017 03:20PM
On Tue, Apr 18, 2017 at 02:59:20PM +0200, Christopher Faulet wrote:
> Le 18/04/2017 à 14:40, Willy Tarreau a écrit :
> > On Tue, Apr 18, 2017 at 12:15:20PM +0200, Christopher Faulet wrote:
> > > I finally took the time to review your patches, mainly the second one, about
> > > the sample fetch. I think it would be pity to introduced such complex sample
> > > fetch. All parts, except the HTTP headers, are already available in
> > > dedicated sample fetches. It could be better to only add a sample fetch to
> > > get HTTP headers (req.hdrs and res.hdrs, something like that). Because a
> > > sample fetch cannot return a list, we can probably encode it into a binary
> > > buffer using a \0 as separator. something like:
> > >
> > >
> > > <num-of-headers><header-name>\0<header-value>\0<header-name>\0<header-value>\0...
> > >
> > > This way, the sample fetch does not depend on the SPOE and can be used in
> > > another context. And concerning your SPOA, this will be quite easy to parse
> > > it.
> >
> > Actually I'm not quite sure about this one, such an encoding could in fact
> > make it harder to process while most use cases will either want to log it
> > (thus copy the whole string as-is) or decode it as received (and might have
> > to rebuild the initial header block with CRLF in between).
> >
>
> My first idea was to avoid the headers parsing in the SPOA. Because it was
> already done by HAProxy, it could be cool to not redo it. Maybe the sample
> fetch can take an argument to choose the format (raw or encoded). The format
> is open. \0 may not be the better separator. This is just a trick to deal
> with a list, like req.hdr_names.

I think there are different needs. I totally agree that if we can build a
list to avoid SPOAs having to parse headers it would be cool, but at the
same time we need to keep in mind that some components there will expect
to receive the request as-is and in this case having to transform it twice
needlessly adds some complexity. So in the end I'm all for having a req.hdrs
and res.hdrs which contain the exact block as received, just like we have
for the body, and on top of this we can have some variants where an easy to
use serialization and encoding is applied. Maybe we can even think about
header factorization to avoid sending multiple times the same header name
with different values. Just an idea.

Willy
Aleksandar Lazic
Re: ModSecurity: First integration patches
April 19, 2017 12:00AM
Am 18-04-2017 15:10, schrieb Willy Tarreau:
> On Tue, Apr 18, 2017 at 02:59:20PM +0200, Christopher Faulet wrote:
>> Le 18/04/2017 à 14:40, Willy Tarreau a écrit :
>> > On Tue, Apr 18, 2017 at 12:15:20PM +0200, Christopher Faulet wrote:
>> > > I finally took the time to review your patches, mainly the second one, about
>> > > the sample fetch. I think it would be pity to introduced such complex sample
>> > > fetch. All parts, except the HTTP headers, are already available in
>> > > dedicated sample fetches. It could be better to only add a sample fetch to
>> > > get HTTP headers (req.hdrs and res.hdrs, something like that). Because a
>> > > sample fetch cannot return a list, we can probably encode it into a binary
>> > > buffer using a \0 as separator. something like:
>> > >
>> > >
>> > > <num-of-headers><header-name>\0<header-value>\0<header-name>\0<header-value>\0...
>> > >
>> > > This way, the sample fetch does not depend on the SPOE and can be used in
>> > > another context. And concerning your SPOA, this will be quite easy to parse
>> > > it.
>> >
>> > Actually I'm not quite sure about this one, such an encoding could in fact
>> > make it harder to process while most use cases will either want to log it
>> > (thus copy the whole string as-is) or decode it as received (and might have
>> > to rebuild the initial header block with CRLF in between).
>> >
>>
>> My first idea was to avoid the headers parsing in the SPOA. Because it
>> was
>> already done by HAProxy, it could be cool to not redo it. Maybe the
>> sample
>> fetch can take an argument to choose the format (raw or encoded). The
>> format
>> is open. \0 may not be the better separator. This is just a trick to
>> deal
>> with a list, like req.hdr_names.
>
> I think there are different needs. I totally agree that if we can build
> a
> list to avoid SPOAs having to parse headers it would be cool, but at
> the
> same time we need to keep in mind that some components there will
> expect
> to receive the request as-is and in this case having to transform it
> twice
> needlessly adds some complexity. So in the end I'm all for having a
> req.hdrs
> and res.hdrs which contain the exact block as received, just like we
> have
> for the body, and on top of this we can have some variants where an
> easy to
> use serialization and encoding is applied. Maybe we can even think
> about
> header factorization to avoid sending multiple times the same header
> name
> with different values. Just an idea.

Why not reuse the upcoming http/2 format.
HTTP/2 is *easy* to parse and the implementations of servers are
growing?
I know this is still on the todo list but I think this is worth to go
instead to add another protocol.

Or we can take a look into grpc, which is on top of http/2 with
protobuffer.
http://www.grpc.io/

As I take more time to think in this direction why not add a grpc filter
like the spoa filter?
Opinions?
I can take a look to implement it.

> Willy

Regards
aleks
Willy Tarreau
Re: ModSecurity: First integration patches
April 19, 2017 06:00AM
On Tue, Apr 18, 2017 at 11:55:46PM +0200, Aleksandar Lazic wrote:
> Why not reuse the upcoming http/2 format.
> HTTP/2 is *easy* to parse and the implementations of servers are growing?

Are you kidding ? I mean you want everyone to have to implement HPACK etc ?

> I know this is still on the todo list but I think this is worth to go
> instead to add another protocol.
>
> Or we can take a look into grpc, which is on top of http/2 with protobuffer.
> http://www.grpc.io/
>
> As I take more time to think in this direction why not add a grpc filter
> like the spoa filter?
> Opinions?

Code generators are always a pain to deal with. It reminds me the old days
when I was using rpcgen. And grpc doesn't provide a C implementation so that
kills it :-)

Thierry told me he's going to implement a simple TLV-like list which will
be much easier to implement and easy to code on both sides. I personally
think that adding dependencies on tons of libs to implement simple stuff
is more of a problem than an help even for implementors, because when you
start to enter some dependency hell or version incompatibilities, it's a
real pain, especially when it was done only to find how to implement
just a small list. These things are useful if you want to implement heavy
inter-application communications but it's not really the case here.

Cheers,
Willy
Aleksandar Lazic
Re: ModSecurity: First integration patches
April 19, 2017 09:20AM
Am 19-04-2017 05:51, schrieb Willy Tarreau:
> On Tue, Apr 18, 2017 at 11:55:46PM +0200, Aleksandar Lazic wrote:
>> Why not reuse the upcoming http/2 format.
>> HTTP/2 is *easy* to parse and the implementations of servers are
>> growing?
>
> Are you kidding ? I mean you want everyone to have to implement HPACK
> etc ?

Well there are a currently lot of http/2 servers out there, but I
understand your concerns.

>> I know this is still on the todo list but I think this is worth to go
>> instead to add another protocol.
>>
>> Or we can take a look into grpc, which is on top of http/2 with
>> protobuffer.
>> http://www.grpc.io/
>>
>> As I take more time to think in this direction why not add a grpc
>> filter
>> like the spoa filter?
>> Opinions?
>
> Code generators are always a pain to deal with. It reminds me the old
> days
> when I was using rpcgen. And grpc doesn't provide a C implementation so
> that
> kills it :-)

Yeah I have also seen that they prefer c++, c#, go and other languages
but not offer a c lib.
This is strange just because the core is written in c, but I don't need
to understand everything ;-)

> Thierry told me he's going to implement a simple TLV-like list which
> will
> be much easier to implement and easy to code on both sides.

@Thierry:
Ah something like netstrings ?
https://cr.yp.to/proto/netstrings.txt

> I personally
> think that adding dependencies on tons of libs to implement simple
> stuff
> is more of a problem than an help even for implementors, because when
> you
> start to enter some dependency hell or version incompatibilities, it's
> a
> real pain, especially when it was done only to find how to implement
> just a small list. These things are useful if you want to implement
> heavy
> inter-application communications but it's not really the case here.

I got you.

> Cheers,
> Willy

Regards
Aleks
Thierry Fournier
Re: ModSecurity: First integration patches
April 19, 2017 11:30AM
> On 19 Apr 2017, at 09:16, Aleksandar Lazic <[email protected]> wrote:
>
>
>
> Am 19-04-2017 05:51, schrieb Willy Tarreau:
>> On Tue, Apr 18, 2017 at 11:55:46PM +0200, Aleksandar Lazic wrote:
>>> Why not reuse the upcoming http/2 format.
>>> HTTP/2 is *easy* to parse and the implementations of servers are growing?
>> Are you kidding ? I mean you want everyone to have to implement HPACK etc ?
>
> Well there are a currently lot of http/2 servers out there, but I understand your concerns.
>
>>> I know this is still on the todo list but I think this is worth to go
>>> instead to add another protocol.
>>> Or we can take a look into grpc, which is on top of http/2 with protobuffer.
>>> http://www.grpc.io/
>>> As I take more time to think in this direction why not add a grpc filter
>>> like the spoa filter?
>>> Opinions?
>> Code generators are always a pain to deal with. It reminds me the old days
>> when I was using rpcgen. And grpc doesn't provide a C implementation so that
>> kills it :-)
>
> Yeah I have also seen that they prefer c++, c#, go and other languages but not offer a c lib.
> This is strange just because the core is written in c, but I don't need to understand everything ;-)
>
>> Thierry told me he's going to implement a simple TLV-like list which will
>> be much easier to implement and easy to code on both sides.
>
> @Thierry:
> Ah something like netstrings ?
> https://cr.yp.to/proto/netstrings.txt

Hi thanks for the link. I do not like this encoding because it is not easy to
parse. I must read ascii representation of numbers and convert-it. I prefer
simple binary format, with length (encoding in the same way than the SPOE
protocol) and value.

Hi, the main goal is providing:
- a format easy to decode
- not adding lot of dependencies to haproxy

So, after brainstorm, I propose:

- remove all data already accessible via existing sample-fetches from the new
sample-fetch introduced by my previous patch. These data are:
method, path, query string, http-version and the body. All of these data are
available in some sample fetches and it will be great to reuse it. Note that
the only data which keep are the headers.

- For the headers I propose two format: The first format is for general use. It
is simply the header block in the same format that we received (with the final
\r\n for detecting truncated header bloc)

- For header I propose also a binary format like this:
<number of headers>
[
<header name length><header name content>
<header value length><header value content>
]…
This format is easy to decode because each length is encoded with the SPOE
numeric values encoding method, so it is always embedded in SPOE client.

The SPOE message description will become

spoe-message check-request
uniqueid args method path query req.ver req.hdrs_bin req.body_size req.body

uniqueid is a string which allow to do matching between the mod security logs and
the haproxy logs. I propose the uniqueid, but this string can be another
think like an header containing a uniqueid previously generated.

req.hrds_bin is the header bloc in binary format.

req.body_size is the advertised size of the body and it is used to determine if the
body is full or truncated.

Thierry

>> I personally
>> think that adding dependencies on tons of libs to implement simple stuff
>> is more of a problem than an help even for implementors, because when you
>> start to enter some dependency hell or version incompatibilities, it's a
>> real pain, especially when it was done only to find how to implement
>> just a small list. These things are useful if you want to implement heavy
>> inter-application communications but it's not really the case here.
>
> I got you.
>
>> Cheers,
>> Willy
>
> Regards
> Aleks
Anonymous User
Re: ModSecurity: First integration patches
April 19, 2017 03:40PM
Hi,

There is a new lot of patches for the spoa/modescurity contrib.

Thierry


On Wed, 19 Apr 2017 11:24:36 +0200
Thierry Fournier <[email protected]> wrote:

>
> > On 19 Apr 2017, at 09:16, Aleksandar Lazic <[email protected]> wrote:
> >
> >
> >
> > Am 19-04-2017 05:51, schrieb Willy Tarreau:
> >> On Tue, Apr 18, 2017 at 11:55:46PM +0200, Aleksandar Lazic wrote:
> >>> Why not reuse the upcoming http/2 format.
> >>> HTTP/2 is *easy* to parse and the implementations of servers are growing?
> >> Are you kidding ? I mean you want everyone to have to implement HPACK etc ?
> >
> > Well there are a currently lot of http/2 servers out there, but I understand your concerns.
> >
> >>> I know this is still on the todo list but I think this is worth to go
> >>> instead to add another protocol.
> >>> Or we can take a look into grpc, which is on top of http/2 with protobuffer.
> >>> http://www.grpc.io/
> >>> As I take more time to think in this direction why not add a grpc filter
> >>> like the spoa filter?
> >>> Opinions?
> >> Code generators are always a pain to deal with. It reminds me the old days
> >> when I was using rpcgen. And grpc doesn't provide a C implementation so that
> >> kills it :-)
> >
> > Yeah I have also seen that they prefer c++, c#, go and other languages but not offer a c lib.
> > This is strange just because the core is written in c, but I don't need to understand everything ;-)
> >
> >> Thierry told me he's going to implement a simple TLV-like list which will
> >> be much easier to implement and easy to code on both sides.
> >
> > @Thierry:
> > Ah something like netstrings ?
> > https://cr.yp.to/proto/netstrings.txt
>
> Hi thanks for the link. I do not like this encoding because it is not easy to
> parse. I must read ascii representation of numbers and convert-it. I prefer
> simple binary format, with length (encoding in the same way than the SPOE
> protocol) and value.
>
> Hi, the main goal is providing:
> - a format easy to decode
> - not adding lot of dependencies to haproxy
>
> So, after brainstorm, I propose:
>
> - remove all data already accessible via existing sample-fetches from the new
> sample-fetch introduced by my previous patch. These data are:
> method, path, query string, http-version and the body. All of these data are
> available in some sample fetches and it will be great to reuse it. Note that
> the only data which keep are the headers.
>
> - For the headers I propose two format: The first format is for general use. It
> is simply the header block in the same format that we received (with the final
> \r\n for detecting truncated header bloc)
>
> - For header I propose also a binary format like this:
> <number of headers>
> [
> <header name length><header name content>
> <header value length><header value content>
> ]…
> This format is easy to decode because each length is encoded with the SPOE
> numeric values encoding method, so it is always embedded in SPOE client.
>
> The SPOE message description will become
>
> spoe-message check-request
> uniqueid args method path query req.ver req.hdrs_bin req.body_size req.body
>
> uniqueid is a string which allow to do matching between the mod security logs and
> the haproxy logs. I propose the uniqueid, but this string can be another
> think like an header containing a uniqueid previously generated.
>
> req.hrds_bin is the header bloc in binary format.
>
> req.body_size is the advertised size of the body and it is used to determine if the
> body is full or truncated.
>
> Thierry
>
> >> I personally
> >> think that adding dependencies on tons of libs to implement simple stuff
> >> is more of a problem than an help even for implementors, because when you
> >> start to enter some dependency hell or version incompatibilities, it's a
> >> real pain, especially when it was done only to find how to implement
> >> just a small list. These things are useful if you want to implement heavy
> >> inter-application communications but it's not really the case here.
> >
> > I got you.
> >
> >> Cheers,
> >> Willy
> >
> > Regards
> > Aleks
>
>
Aleksandar Lazic
Re: ModSecurity: First integration patches
April 19, 2017 08:30PM
Am 19-04-2017 11:24, schrieb Thierry Fournier:
>> On 19 Apr 2017, at 09:16, Aleksandar Lazic <[email protected]> wrote:
>>
>>
>>
>> Am 19-04-2017 05:51, schrieb Willy Tarreau:
>>> On Tue, Apr 18, 2017 at 11:55:46PM +0200, Aleksandar Lazic wrote:
>>>> Why not reuse the upcoming http/2 format.
>>>> HTTP/2 is *easy* to parse and the implementations of servers are
>>>> growing?
>>> Are you kidding ? I mean you want everyone to have to implement HPACK
>>> etc ?
>>
>> Well there are a currently lot of http/2 servers out there, but I
>> understand your concerns.
>>
>>>> I know this is still on the todo list but I think this is worth to
>>>> go
>>>> instead to add another protocol.
>>>> Or we can take a look into grpc, which is on top of http/2 with
>>>> protobuffer.
>>>> http://www.grpc.io/
>>>> As I take more time to think in this direction why not add a grpc
>>>> filter
>>>> like the spoa filter?
>>>> Opinions?
>>> Code generators are always a pain to deal with. It reminds me the old
>>> days
>>> when I was using rpcgen. And grpc doesn't provide a C implementation
>>> so that
>>> kills it :-)
>>
>> Yeah I have also seen that they prefer c++, c#, go and other languages
>> but not offer a c lib.
>> This is strange just because the core is written in c, but I don't
>> need to understand everything ;-)
>>
>>> Thierry told me he's going to implement a simple TLV-like list which
>>> will
>>> be much easier to implement and easy to code on both sides.
>>
>> @Thierry:
>> Ah something like netstrings ?
>> https://cr.yp.to/proto/netstrings.txt
>
> Hi thanks for the link. I do not like this encoding because it is not
> easy to
> parse. I must read ascii representation of numbers and convert-it. I
> prefer
> simple binary format, with length (encoding in the same way than the
> SPOE
> protocol) and value.

Thanks for clarification.

> Hi, the main goal is providing:
> - a format easy to decode
> - not adding lot of dependencies to haproxy

That's very polite.

> So, after brainstorm, I propose:
>
> - remove all data already accessible via existing sample-fetches from
> the new
> sample-fetch introduced by my previous patch. These data are:
> method, path, query string, http-version and the body. All of these
> data are
> available in some sample fetches and it will be great to reuse it.
> Note that
> the only data which keep are the headers.
>
> - For the headers I propose two format: The first format is for
> general use. It
> is simply the header block in the same format that we received
> (with the final
> \r\n for detecting truncated header bloc)
>
> - For header I propose also a binary format like this:
> <number of headers>
> [
> <header name length><header name content>
> <header value length><header value content>
> ]…
> This format is easy to decode because each length is encoded with
> the SPOE
> numeric values encoding method, so it is always embedded in SPOE
> client.
>
> The SPOE message description will become
>
> spoe-message check-request
> uniqueid args method path query req.ver req.hdrs_bin
> req.body_size req.body
>
> uniqueid is a string which allow to do matching between the mod
> security logs and
> the haproxy logs. I propose the uniqueid, but this string can
> be another
> think like an header containing a uniqueid previously
> generated.
>
> req.hrds_bin is the header bloc in binary format.
>
> req.body_size is the advertised size of the body and it is used to
> determine if the
> body is full or truncated.

Looks good to me.

Regards
Aleks


> Thierry
>
>>> I personally
>>> think that adding dependencies on tons of libs to implement simple
>>> stuff
>>> is more of a problem than an help even for implementors, because when
>>> you
>>> start to enter some dependency hell or version incompatibilities,
>>> it's a
>>> real pain, especially when it was done only to find how to implement
>>> just a small list. These things are useful if you want to implement
>>> heavy
>>> inter-application communications but it's not really the case here.
>>
>> I got you.
>>
>>> Cheers,
>>> Willy
>>
>> Regards
>> Aleks
Thierry Fournier
Re: ModSecurity: First integration patches
April 20, 2017 03:10PM
Hi,

After a quick private brainstorm, Willy propose to me a new binary encoding
for the headers. It is useless to give the numbers of headers contained in
the block, so the end of headers is marked by an empty header (header name
and value have a length of 0).

The new patches in attachment (first patch is 0002).

Thierry



> On 19 Apr 2017, at 15:34, thierry.fournier@arpalert.org wrote:
>
> Hi,
>
> There is a new lot of patches for the spoa/modescurity contrib.
>
> Thierry
>
>
> On Wed, 19 Apr 2017 11:24:36 +0200
> Thierry Fournier <[email protected]> wrote:
>
>>
>>> On 19 Apr 2017, at 09:16, Aleksandar Lazic <[email protected]> wrote:
>>>
>>>
>>>
>>> Am 19-04-2017 05:51, schrieb Willy Tarreau:
>>>> On Tue, Apr 18, 2017 at 11:55:46PM +0200, Aleksandar Lazic wrote:
>>>>> Why not reuse the upcoming http/2 format.
>>>>> HTTP/2 is *easy* to parse and the implementations of servers are growing?
>>>> Are you kidding ? I mean you want everyone to have to implement HPACK etc ?
>>>
>>> Well there are a currently lot of http/2 servers out there, but I understand your concerns.
>>>
>>>>> I know this is still on the todo list but I think this is worth to go
>>>>> instead to add another protocol.
>>>>> Or we can take a look into grpc, which is on top of http/2 with protobuffer.
>>>>> http://www.grpc.io/
>>>>> As I take more time to think in this direction why not add a grpc filter
>>>>> like the spoa filter?
>>>>> Opinions?
>>>> Code generators are always a pain to deal with. It reminds me the old days
>>>> when I was using rpcgen. And grpc doesn't provide a C implementation so that
>>>> kills it :-)
>>>
>>> Yeah I have also seen that they prefer c++, c#, go and other languages but not offer a c lib.
>>> This is strange just because the core is written in c, but I don't need to understand everything ;-)
>>>
>>>> Thierry told me he's going to implement a simple TLV-like list which will
>>>> be much easier to implement and easy to code on both sides.
>>>
>>> @Thierry:
>>> Ah something like netstrings ?
>>> https://cr.yp.to/proto/netstrings.txt
>>
>> Hi thanks for the link. I do not like this encoding because it is not easy to
>> parse. I must read ascii representation of numbers and convert-it. I prefer
>> simple binary format, with length (encoding in the same way than the SPOE
>> protocol) and value.
>>
>> Hi, the main goal is providing:
>> - a format easy to decode
>> - not adding lot of dependencies to haproxy
>>
>> So, after brainstorm, I propose:
>>
>> - remove all data already accessible via existing sample-fetches from the new
>> sample-fetch introduced by my previous patch. These data are:
>> method, path, query string, http-version and the body. All of these data are
>> available in some sample fetches and it will be great to reuse it. Note that
>> the only data which keep are the headers.
>>
>> - For the headers I propose two format: The first format is for general use. It
>> is simply the header block in the same format that we received (with the final
>> \r\n for detecting truncated header bloc)
>>
>> - For header I propose also a binary format like this:
>> <number of headers>
>> [
>> <header name length><header name content>
>> <header value length><header value content>
>> ]…
>> This format is easy to decode because each length is encoded with the SPOE
>> numeric values encoding method, so it is always embedded in SPOE client.
>>
>> The SPOE message description will become
>>
>> spoe-message check-request
>> uniqueid args method path query req.ver req.hdrs_bin req.body_size req.body
>>
>> uniqueid is a string which allow to do matching between the mod security logs and
>> the haproxy logs. I propose the uniqueid, but this string can be another
>> think like an header containing a uniqueid previously generated.
>>
>> req.hrds_bin is the header bloc in binary format.
>>
>> req.body_size is the advertised size of the body and it is used to determine if the
>> body is full or truncated.
>>
>> Thierry
>>
>>>> I personally
>>>> think that adding dependencies on tons of libs to implement simple stuff
>>>> is more of a problem than an help even for implementors, because when you
>>>> start to enter some dependency hell or version incompatibilities, it's a
>>>> real pain, especially when it was done only to find how to implement
>>>> just a small list. These things are useful if you want to implement heavy
>>>> inter-application communications but it's not really the case here.
>>>
>>> I got you.
>>>
>>>> Cheers,
>>>> Willy
>>>
>>> Regards
>>> Aleks
>>
>>
> <0001-BUG-MINOR-change-header-declared-function-to-static-.patch><0002-REORG-spoe-move-spoe_encode_varint-spoe_decode_varin.patch><0003-MINOR-Add-binary-encoding-request-header-sample-fetc.patch><0004-MINOR-proto-http-Add-sample-fetch-wich-returns-all-H.patch><0005-MINOR-Add-ModSecurity-wrapper-as-contrib.patch>
Attachments:
open | download - 0002-BUG-MINOR-change-header-declared-function-to-static-.patch (3.7 KB)
open | download - 0003-REORG-spoe-move-spoe_encode_varint-spoe_decode_varin.patch (13.9 KB)
open | download - 0004-MINOR-Add-binary-encoding-request-header-sample-fetc.patch (5.3 KB)
open | download - 0005-MINOR-proto-http-Add-sample-fetch-wich-returns-all-H.patch (2.9 KB)
open | download - 0006-MINOR-Add-ModSecurity-wrapper-as-contrib.patch (75.7 KB)
Aleksandar Lazic
Re: ModSecurity: First integration patches
April 23, 2017 03:30PM
Hi Thierry

Am 20-04-2017 15:05, schrieb Thierry Fournier:
> Hi,
>
> After a quick private brainstorm, Willy propose to me a new binary
> encoding
> for the headers. It is useless to give the numbers of headers contained
> in
> the block, so the end of headers is marked by an empty header (header
> name
> and value have a length of 0).
>
> The new patches in attachment (first patch is 0002).

I have now created a docker image

https://hub.docker.com/r/me2digital/haproxy-waf/

The build was successfully and haproxy starts.

I have also try to check if the modsecurity starts and yeah it starts
;-)

###

1492953204.290643 [00] ModSecurity for nginx (STABLE)/2.9.1
(http://www.modsecurity.org/) configured.
1492953204.290705 [00] ModSecurity: APR compiled version="1.4.8"; loaded
version="1.4.8"
1492953204.290730 [00] ModSecurity: PCRE compiled version="8.32 ";
loaded version="8.32 2012-11-30"
1492953204.290739 [00] ModSecurity: YAJL compiled version="2.0.4"
1492953204.290743 [00] ModSecurity: LIBXML compiled version="2.9.1"
1492953204.290747 [00] ModSecurity: Status engine is currently disabled,
enable it by set SecStatusEngine to On.
1492953209.298799 [03] 0 clients connected
1492953209.298937 [04] 0 clients connected
...
###

Now we can go the next step and create a complete solution ;-)

It would be interesting to go the same way as mod_sec & nginx is going.

https://www.trustwave.com/Resources/SpiderLabs-Blog/Announcing-the-availability-of-ModSecurity-extension-for-Nginx/

> Thierry

Regards
Aleks

>
>
>
>> On 19 Apr 2017, at 15:34, thierry.fournier@arpalert.org wrote:
>>
>> Hi,
>>
>> There is a new lot of patches for the spoa/modescurity contrib.
>>
>> Thierry
>>
>>
>> On Wed, 19 Apr 2017 11:24:36 +0200
>> Thierry Fournier <[email protected]> wrote:
>>
>>>
>>>> On 19 Apr 2017, at 09:16, Aleksandar Lazic <[email protected]>
>>>> wrote:
>>>>
>>>>
>>>>
>>>> Am 19-04-2017 05:51, schrieb Willy Tarreau:
>>>>> On Tue, Apr 18, 2017 at 11:55:46PM +0200, Aleksandar Lazic wrote:
>>>>>> Why not reuse the upcoming http/2 format.
>>>>>> HTTP/2 is *easy* to parse and the implementations of servers are
>>>>>> growing?
>>>>> Are you kidding ? I mean you want everyone to have to implement
>>>>> HPACK etc ?
>>>>
>>>> Well there are a currently lot of http/2 servers out there, but I
>>>> understand your concerns.
>>>>
>>>>>> I know this is still on the todo list but I think this is worth to
>>>>>> go
>>>>>> instead to add another protocol.
>>>>>> Or we can take a look into grpc, which is on top of http/2 with
>>>>>> protobuffer.
>>>>>> http://www.grpc.io/
>>>>>> As I take more time to think in this direction why not add a grpc
>>>>>> filter
>>>>>> like the spoa filter?
>>>>>> Opinions?
>>>>> Code generators are always a pain to deal with. It reminds me the
>>>>> old days
>>>>> when I was using rpcgen. And grpc doesn't provide a C
>>>>> implementation so that
>>>>> kills it :-)
>>>>
>>>> Yeah I have also seen that they prefer c++, c#, go and other
>>>> languages but not offer a c lib.
>>>> This is strange just because the core is written in c, but I don't
>>>> need to understand everything ;-)
>>>>
>>>>> Thierry told me he's going to implement a simple TLV-like list
>>>>> which will
>>>>> be much easier to implement and easy to code on both sides.
>>>>
>>>> @Thierry:
>>>> Ah something like netstrings ?
>>>> https://cr.yp.to/proto/netstrings.txt
>>>
>>> Hi thanks for the link. I do not like this encoding because it is not
>>> easy to
>>> parse. I must read ascii representation of numbers and convert-it. I
>>> prefer
>>> simple binary format, with length (encoding in the same way than the
>>> SPOE
>>> protocol) and value.
>>>
>>> Hi, the main goal is providing:
>>> - a format easy to decode
>>> - not adding lot of dependencies to haproxy
>>>
>>> So, after brainstorm, I propose:
>>>
>>> - remove all data already accessible via existing sample-fetches from
>>> the new
>>> sample-fetch introduced by my previous patch. These data are:
>>> method, path, query string, http-version and the body. All of these
>>> data are
>>> available in some sample fetches and it will be great to reuse it.
>>> Note that
>>> the only data which keep are the headers.
>>>
>>> - For the headers I propose two format: The first format is for
>>> general use. It
>>> is simply the header block in the same format that we received
>>> (with the final
>>> \r\n for detecting truncated header bloc)
>>>
>>> - For header I propose also a binary format like this:
>>> <number of headers>
>>> [
>>> <header name length><header name content>
>>> <header value length><header value content>
>>> ]…
>>> This format is easy to decode because each length is encoded with
>>> the SPOE
>>> numeric values encoding method, so it is always embedded in SPOE
>>> client.
>>>
>>> The SPOE message description will become
>>>
>>> spoe-message check-request
>>> uniqueid args method path query req.ver req.hdrs_bin
>>> req.body_size req.body
>>>
>>> uniqueid is a string which allow to do matching between the mod
>>> security logs and
>>> the haproxy logs. I propose the uniqueid, but this string can
>>> be another
>>> think like an header containing a uniqueid previously
>>> generated.
>>>
>>> req.hrds_bin is the header bloc in binary format.
>>>
>>> req.body_size is the advertised size of the body and it is used to
>>> determine if the
>>> body is full or truncated.
>>>
>>> Thierry
>>>
>>>>> I personally
>>>>> think that adding dependencies on tons of libs to implement simple
>>>>> stuff
>>>>> is more of a problem than an help even for implementors, because
>>>>> when you
>>>>> start to enter some dependency hell or version incompatibilities,
>>>>> it's a
>>>>> real pain, especially when it was done only to find how to
>>>>> implement
>>>>> just a small list. These things are useful if you want to implement
>>>>> heavy
>>>>> inter-application communications but it's not really the case here.
>>>>
>>>> I got you.
>>>>
>>>>> Cheers,
>>>>> Willy
>>>>
>>>> Regards
>>>> Aleks
>>>
>>>
>> <0001-BUG-MINOR-change-header-declared-function-to-static-.patch><0002-REORG-spoe-move-spoe_encode_varint-spoe_decode_varin.patch><0003-MINOR-Add-binary-encoding-request-header-sample-fetc.patch><0004-MINOR-proto-http-Add-sample-fetch-wich-returns-all-H.patch><0005-MINOR-Add-ModSecurity-wrapper-as-contrib.patch>
Sorry, only registered users may post in this forum.

Click here to login