[PATCH 4/4] MINOR: ssl: "show ssl cert" command should print the "Chain filename:"

2020-02-18 Thread Emmanuel Hocdet 
Hi,Le 18 févr. 2020 à 11:45, Emmanuel Hocdet  a écrit :I think we will probably need more information in the "show ssl cert"output in the future so the users can debug this kind of feature easily.Yes. Show the chain-filename would be very helpful.For that i think a good way would be to keep ckch->chain and ckch->issuerwith value (or NULL) from PEM/, and resolve chain and ocsp_issuerwhen needed. « show ssl cert » will be able to find the origin of chain (and ocsp_issuer)without  store a new state. The drawback(?) is that .issuer file will be loaded, in every case, if present.Patch series to do that:example:Issuer: /C=US/O=Let's Encrypt/CN=Let's Encrypt Authority X3Chain filename: /etc/haproxy/issuers/letsencryptEC.pem++Manu

0001-MINOR-ssl-move-find-certificate-chain-code-to-its-ow.patch
Description: Binary data


0002-MINOR-ssl-resolve-issuers-chain-later.patch
Description: Binary data


0003-MINOR-ssl-resolve-ocsp_issuer-later.patch
Description: Binary data


0004-MINOR-ssl-show-ssl-cert-command-should-print-the-Cha.patch
Description: Binary data

Re: [PATCH] MINOR: ssl: add "issuers-chain-path" directive.

2020-02-18 Thread Илья Шипицин 
вт, 18 февр. 2020 г. в 21:44, Emmanuel Hocdet :

>
> > Le 18 févr. 2020 à 14:36, William Lallemand  a
> écrit :
> >
> > On Tue, Feb 18, 2020 at 01:58:39PM +0100, Emmanuel Hocdet wrote:
> >>
> >>> Le 18 févr. 2020 à 11:45, Emmanuel Hocdet  a écrit :
> >>>
>  Can you add a little bit of explanation on how the discovery of the
>  issuer is done in the documentation?
> 
> >>> ok
> >>
> >>
> >> documentation updated:
> >>
> >
> > Thanks Manu!
> >
> > Merged and pushed in master.
> >
>

src/ssl_sock.c:9860:15: error: format string is not a string literal
(potentially insecure) [-Werror,-Wformat-security]

ha_warning(warn);

   ^~~~

src/ssl_sock.c:9860:15: note: treat the string as an argument to avoid this

ha_warning(warn);

   ^

   "%s",



>
> w00t!
> Thanks
>
> Manu
>
>
>

Re: [PATCH] MINOR: ssl: add "issuers-chain-path" directive.

2020-02-18 Thread Emmanuel Hocdet 


> Le 18 févr. 2020 à 14:36, William Lallemand  a écrit :
> 
> On Tue, Feb 18, 2020 at 01:58:39PM +0100, Emmanuel Hocdet wrote:
>> 
>>> Le 18 févr. 2020 à 11:45, Emmanuel Hocdet  a écrit :
>>> 
 Can you add a little bit of explanation on how the discovery of the
 issuer is done in the documentation?
 
>>> ok
>> 
>> 
>> documentation updated:
>> 
> 
> Thanks Manu!
> 
> Merged and pushed in master.
> 

w00t!
Thanks

Manu

Re: [PATCH] compression/lua_validation.vtc depends on "which" utility

2020-02-18 Thread Willy Tarreau 
On Tue, Feb 18, 2020 at 06:44:25PM +0500,  ??? wrote:
> when specifying "shell" in vtc, can we require posix /bin/sh ? or is it by
> chance ?

I suspect it's by default /bin/sh since not specified. I'm also
seeing the string /bin/sh appearing in the vtest binary.

Willy

Re: [PATCH] compression/lua_validation.vtc depends on "which" utility

2020-02-18 Thread William Lallemand 
On Tue, Feb 18, 2020 at 06:44:25PM +0500, Илья Шипицин wrote:
> when specifying "shell" in vtc, can we require posix /bin/sh ? or is it by
> chance ?
> 
> shell {
> HOST=${h1_fe1_addr}
> if [ "${h1_fe1_addr}" = "::1" ] ; then
> HOST="\[::1\]"
> fi
> 
> md5=$(which md5 || which md5sum)
> 
> 

/bin/sh can be a symlink to bash, dash or anything else.

-- 
William Lallemand

Re: [PATCH] compression/lua_validation.vtc depends on "which" utility

2020-02-18 Thread Willy Tarreau 
On Tue, Feb 18, 2020 at 02:29:08PM +0100, Tim Düsterhus wrote:
> command -v is looking good to me:

OK now pushed, thanks!
Willy

Re: [PATCH] compression/lua_validation.vtc depends on "which" utility

2020-02-18 Thread Илья Шипицин 
when specifying "shell" in vtc, can we require posix /bin/sh ? or is it by
chance ?

shell {
HOST=${h1_fe1_addr}
if [ "${h1_fe1_addr}" = "::1" ] ; then
HOST="\[::1\]"
fi

md5=$(which md5 || which md5sum)


вт, 18 февр. 2020 г. в 18:24, Willy Tarreau :

> On Tue, Feb 18, 2020 at 02:15:18PM +0100, Tim Düsterhus wrote:
> > Willy,
> >
> > Am 18.02.20 um 14:06 schrieb Willy Tarreau:
> > > I haven't pushed yet Ilya's patch I've just merged, I'm fine with
> > > applying a change if preferred. Just let me know.
> >
> > `type` is required to be a builtin by POSIX [1]. The `-p` parameter does
> > not appear to be standardized, though. Nonetheless I expect it to work
> > in any `bash`.
>
> OK thanks for the info. But sadly it fails in other shells such as dash:
>
>   $ type -p curl
>   -p: not found
>   curl is /usr/local/bin/curl
>
> So we can only use "type" with no argument and adjust it. Or better,
> we can use "command -v" which looks OK:
>
>   bash$ command -v curl
>   /usr/local/bin/curl
>   dash$ command -v curl
>   /usr/local/bin/curl
>   ksh$ command -v curl
>   /usr/local/bin/curl
>
> And it's also in POSIX:
>https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html
>
> Willy
>

Re: [PATCH] MINOR: ssl: add "issuers-chain-path" directive.

2020-02-18 Thread William Lallemand 
On Tue, Feb 18, 2020 at 01:58:39PM +0100, Emmanuel Hocdet wrote:
> 
> > Le 18 févr. 2020 à 11:45, Emmanuel Hocdet  a écrit :
> > 
> >> Can you add a little bit of explanation on how the discovery of the
> >> issuer is done in the documentation?
> >> 
> > ok
> 
> 
> documentation updated:
> 

Thanks Manu!

Merged and pushed in master.

-- 
William Lallemand

Re: [PATCH] compression/lua_validation.vtc depends on "which" utility

2020-02-18 Thread Tim Düsterhus 
Willy,

Am 18.02.20 um 14:24 schrieb Willy Tarreau:
> So we can only use "type" with no argument and adjust it. Or better,
> we can use "command -v" which looks OK:
> 
>   bash$ command -v curl
>   /usr/local/bin/curl
>   dash$ command -v curl
>   /usr/local/bin/curl
>   ksh$ command -v curl
>   /usr/local/bin/curl
> 
> And it's also in POSIX:
>https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html
> 

command -v is looking good to me:

> fish$ command -v md5 || command -v md5sum
> /usr/bin/md5sum
> bash$ command -v md5 || command -v md5sum
> /usr/bin/md5sum

Best regards
Tim Düsterhus

Re: [PATCH] compression/lua_validation.vtc depends on "which" utility

2020-02-18 Thread Willy Tarreau 
On Tue, Feb 18, 2020 at 02:15:18PM +0100, Tim Düsterhus wrote:
> Willy,
> 
> Am 18.02.20 um 14:06 schrieb Willy Tarreau:
> > I haven't pushed yet Ilya's patch I've just merged, I'm fine with
> > applying a change if preferred. Just let me know.
> 
> `type` is required to be a builtin by POSIX [1]. The `-p` parameter does
> not appear to be standardized, though. Nonetheless I expect it to work
> in any `bash`.

OK thanks for the info. But sadly it fails in other shells such as dash:

  $ type -p curl
  -p: not found
  curl is /usr/local/bin/curl

So we can only use "type" with no argument and adjust it. Or better,
we can use "command -v" which looks OK:

  bash$ command -v curl
  /usr/local/bin/curl
  dash$ command -v curl
  /usr/local/bin/curl
  ksh$ command -v curl
  /usr/local/bin/curl

And it's also in POSIX:
   https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html

Willy

Re: [PATCH] compression/lua_validation.vtc depends on "which" utility

2020-02-18 Thread Tim Düsterhus 
Willy,

Am 18.02.20 um 14:06 schrieb Willy Tarreau:
> I haven't pushed yet Ilya's patch I've just merged, I'm fine with
> applying a change if preferred. Just let me know.

`type` is required to be a builtin by POSIX [1]. The `-p` parameter does
not appear to be standardized, though. Nonetheless I expect it to work
in any `bash`.

Thus I prefer to change the test to use `type` instead of making it
dependent on `which`.

Best regards
Tim Düsterhus

[1] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/type.html

Re: [PATCH] compression/lua_validation.vtc depends on "which" utility

2020-02-18 Thread Willy Tarreau 
On Sun, Feb 16, 2020 at 11:34:28AM +0100, Ionel GARDAIS wrote:
> It's here on osx 10.11.6 at least. 
> Looks like a sh builtin. 

I haven't pushed yet Ilya's patch I've just merged, I'm fine with
applying a change if preferred. Just let me know.

Willy

Re: [PATCH] compression/lua_validation.vtc depends on "which" utility

2020-02-18 Thread Willy Tarreau 
On Sun, Feb 16, 2020 at 12:38:46AM +0500,  ??? wrote:
> Hello,
> 
> 
> that utility is not available by default in Fedora docker image.

Not surprized, that's always the fun when relying on external tools.

Now merged, thanks Ilya!
Willy

Re: dns fails to process response / hold valid? (since commit 2.2-dev0-13a9232)

2020-02-18 Thread Baptiste 
Hi guys,

Thx Tim for investigating.
I'll check the PCAP and see why such behavior happens.

Baptiste


On Tue, Feb 18, 2020 at 12:09 AM Tim Düsterhus  wrote:

> Pieter,
>
> Am 09.02.20 um 15:35 schrieb PiBa-NL:
> > Before commit '2.2-dev0-13a9232, released 2020/01/22 (use additional
> > records from SRV responses)' i get seemingly proper working resolving of
> > server a name.
> > After this commit all responses are counted as 'invalid' in the socket
> > stats.
>
> I can confirm the issue with the provided configuration. The 'if (len ==
> 0) {' check in line 1045 of the commit causes HAProxy to consider the
> responses 'invalid':
>
>
> https://github.com/haproxy/haproxy/commit/13a9232ebc63fdf357ffcf4fa7a1a5e77a1eac2b#diff-b2ddf457bc423779995466f7d8b9d147R1045-R1048
>
> Best regards
> Tim Düsterhus
>

Re: [PATCH] MINOR: ssl: add "issuers-chain-path" directive.

2020-02-18 Thread Emmanuel Hocdet 
Le 18 févr. 2020 à 11:45, Emmanuel Hocdet  a écrit :Can you add a little bit of explanation on how the discovery of theissuer is done in the documentation?okdocumentation updated:

0001-MINOR-ssl-add-issuers-chain-path-directive.patch
Description: Binary data

Re: [PATCH] MINOR: ssl: add "issuers-chain-path" directive.

2020-02-18 Thread Emmanuel Hocdet 
Hi William

> Le 14 févr. 2020 à 15:59, William Lallemand  a écrit :
> 
> On Fri, Feb 14, 2020 at 03:25:48PM +0100, Emmanuel Hocdet wrote:
>> Hi,
>> 
>> Is there any hope that this proposal will be considered before HAproxy 2.2?
>> 
>> ++
>> Manu
> 
> Hello,
> 
> I'm ok with the feature itself. I'm still not fond of an
> "auto-discovery" based on the SKID, but I agree that's probably the most
> convenient way of doing it for the user.
> 
great news

> The way it's done we won't be able to change the issuers from the CLI
> easily, but these files don't change too often so that's not a problem
> at the moment.
> 
I agree.

> Can you add a little bit of explanation on how the discovery of the
> issuer is done in the documentation?
> 
ok

> I think we will probably need more information in the "show ssl cert"
> output in the future so the users can debug this kind of feature easily.
> 

Yes. Show the chain-filename would be very helpful.
For that i think a good way would be to keep ckch->chain and ckch->issuer
with value (or NULL) from PEM/, and resolve chain and ocsp_issuer
when needed. « show ssl cert » will be able to find the origin of chain (and 
ocsp_issuer)
without  store a new state. The drawback(?) is that .issuer file will be 
loaded, in every case, if present.
Something like i do for ‘chain’ directive POC: 
https://github.com/ehocdet/haproxy/commits/chain

++
Manu