[PATCH] fix libslz url in travis

2020-05-18 Thread Илья Шипицин
Hello,

travis timed out when downloading libslz

fatal: unable to access 'http://git.1wt.eu/git/libslz.git/': Failed to
connect to git.1wt.eu port 80: Connection timed out

The command "git clone http://git.1wt.eu/git/libslz.git/; failed and
exited with 128 during .



it is something related to travis. I can access that URL.

we cannot fix travis, but we can change libslz download url :)

Ilya Shipitcin
From b1bed820dfaeadda7cef981051d647b74a50e7e8 Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Tue, 19 May 2020 01:47:07 +0500
Subject: [PATCH] CI: travis-ci: fix libslz download

let us switch to github mirror, travis cannot download original libslz

error is:
fatal: unable to access 'http://git.1wt.eu/git/libslz.git/': Failed to connect to git.1wt.eu port 80: Connection timed out
The command "git clone http://git.1wt.eu/git/libslz.git/; failed and exited with 128 during .
---
 .travis.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index e2a32a2b7..809c2292f 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -102,7 +102,7 @@ matrix:
 compiler: clang
 env: TARGET=linux-glibc FLAGS="USE_SLZ=1 USE_PCRE2=1 USE_PCRE2_JIT=1 USE_LUA=1 USE_OPENSSL=1 USE_SYSTEMD=1 USE_WURFL=1 WURFL_INC=contrib/wurfl WURFL_LIB=contrib/wurfl USE_51DEGREES=1" CC=clang-9
 before_script:
-  - git clone http://git.1wt.eu/git/libslz.git/
+  - git clone https://github.com/wtarreau/libslz
   - cd libslz && make && make PREFIX=${HOME}/opt install && cd ..
   allow_failures:
   - os: linux
-- 
2.26.2



Re: [PR] Add verfied chain

2020-05-18 Thread William Dauchy
Hi Arjen,

On Mon, May 18, 2020 at 6:02 PM Arjen Nienhuis  wrote:
> I used PKCS7 because I did not know how to parse concatenated blobs.

Mathilde, how did we planned to use it? :)

> I think you should use SSL_get_peer_cert_chain because:
> - BoringSSL has no SSL_get0_verified_chain.
> - For debugging having all the certs is better. Especially if the chain
> is not valid.
> - In theory it's not always possible to do OCSP with the verified chain.
> OCSP is part of finding a valid chain. OpenSSL could choose a cert chain
> that doesn't pass OCSP while an other chain exists that can pass OCSP.

Thank you for your feedbacks.
Do you want to handle the changes? Otherwise we can handle them and
mention you as the original proposition in the commit message. As you
wish.

-- 
William



Re: [PR] Add verfied chain

2020-05-18 Thread Arjen Nienhuis



On 18-05-2020 15:57, William Lallemand wrote:

On Mon, May 18, 2020 at 02:49:28PM +0200, William Dauchy wrote:

Hello William L.,

On Wed, Dec 4, 2019 at 4:24 PM PR Bot  wrote:

Patch title(s):
MINOR: add fetch 'ssl_c_verified_chain'
Merge branch 'master' of https://github.com/haproxy/haproxy
Link:
https://github.com/haproxy/haproxy/pull/396
Edit locally:
wget https://github.com/haproxy/haproxy/pull/396.patch && vi 396.patch
Apply locally:
curl https://github.com/haproxy/haproxy/pull/396.patch | git am -

We were wondering if you add the time to have a look at this one?
In fact we have a similar need and Mathilde started to work on a very
similar patch, see
https://github.com/ShimmerGlass/haproxy/commit/c63116fe7048320abc41709e4d1b25513da91f57
difference being, Mathilde simply concatenated the certs, and the
patch from Arjen, uses PKCS7. Is there any specific reason to use
PKCS7?

note: it also refers to https://github.com/haproxy/haproxy/issues/297

Best,

Hello,

I suppose it was put in a PKCS7 container to be able to distinguish each
DER part of the chain easily? So It can be used by an external tool. I'm not
sure of what is done with the result of this.

The two patches seem to have different approches, Arjen's one is
using a SSL_get0_verified_chain() and Mathild's one is using
SSL_get_peer_cert_chain(). I'm not sure what approach is the best, I
suppose that SSL_get_peer_cert_chain() is better if we want to have the
chain event if it wasn't verified and it could be completed with the
ssl_c_verify sample fetch if we need this information!

I will be grateful if a .vtc test file is also provided with sample
fetches patches, it's difficult to test every sample fetches nowadays.

There is already a vtc for client auth which is available here:
https://git.haproxy.org/?p=haproxy.git;a=blob;f=reg-tests/ssl/ssl_client_auth.vtc

Thanks!


Hi,

I used PKCS7 because I did not know how to parse concatenated blobs.

I think you should use SSL_get_peer_cert_chain because:

- BoringSSL has no SSL_get0_verified_chain.

- For debugging having all the certs is better. Especially if the chain 
is not valid.


- In theory it's not always possible to do OCSP with the verified chain. 
OCSP is part of finding a valid chain. OpenSSL could choose a cert chain 
that doesn't pass OCSP while an other chain exists that can pass OCSP.



Groeten, Arjen




Re: [PR] Add verfied chain

2020-05-18 Thread Emeric Brun
Hi All,

On 5/18/20 4:32 PM, William Dauchy wrote:
> On Mon, May 18, 2020 at 3:58 PM William Lallemand
>  wrote:
>> I suppose it was put in a PKCS7 container to be able to distinguish each
>> DER part of the chain easily? So It can be used by an external tool. I'm not
>> sure of what is done with the result of this.
>>
>> The two patches seem to have different approches, Arjen's one is
>> using a SSL_get0_verified_chain() and Mathild's one is using
>> SSL_get_peer_cert_chain(). I'm not sure what approach is the best, I
>> suppose that SSL_get_peer_cert_chain() is better if we want to have the
>> chain event if it wasn't verified and it could be completed with the
>> ssl_c_verify sample fetch if we need this information!
>>
>> I will be grateful if a .vtc test file is also provided with sample
>> fetches patches, it's difficult to test every sample fetches nowadays.
>>
>> There is already a vtc for client auth which is available here:
>> https://git.haproxy.org/?p=haproxy.git;a=blob;f=reg-tests/ssl/ssl_client_auth.vtc
> 
> Thanks for the feedbacks. I believe we will send our proposition soon.
> 

According to openssl's doc about SSL_get_peer_cert_chain, 
SSL_get0_verified_chain:

NOTES
If the session is resumed peers do not send certificates so a NULL pointer is 
returned by these functions.

It would be great if the ssl_c_ 's documentations precise if those information 
won't return something on resumed sessions.

R,
Emeric



Re: [PR] Add verfied chain

2020-05-18 Thread William Dauchy
On Mon, May 18, 2020 at 3:58 PM William Lallemand
 wrote:
> I suppose it was put in a PKCS7 container to be able to distinguish each
> DER part of the chain easily? So It can be used by an external tool. I'm not
> sure of what is done with the result of this.
>
> The two patches seem to have different approches, Arjen's one is
> using a SSL_get0_verified_chain() and Mathild's one is using
> SSL_get_peer_cert_chain(). I'm not sure what approach is the best, I
> suppose that SSL_get_peer_cert_chain() is better if we want to have the
> chain event if it wasn't verified and it could be completed with the
> ssl_c_verify sample fetch if we need this information!
>
> I will be grateful if a .vtc test file is also provided with sample
> fetches patches, it's difficult to test every sample fetches nowadays.
>
> There is already a vtc for client auth which is available here:
> https://git.haproxy.org/?p=haproxy.git;a=blob;f=reg-tests/ssl/ssl_client_auth.vtc

Thanks for the feedbacks. I believe we will send our proposition soon.
-- 
William



Re: [PR] Add verfied chain

2020-05-18 Thread William Lallemand
On Mon, May 18, 2020 at 02:49:28PM +0200, William Dauchy wrote:
> Hello William L.,
> 
> On Wed, Dec 4, 2019 at 4:24 PM PR Bot  wrote:
> > Patch title(s):
> >MINOR: add fetch 'ssl_c_verified_chain'
> >Merge branch 'master' of https://github.com/haproxy/haproxy
> > Link:
> >https://github.com/haproxy/haproxy/pull/396
> > Edit locally:
> >wget https://github.com/haproxy/haproxy/pull/396.patch && vi 396.patch
> > Apply locally:
> >curl https://github.com/haproxy/haproxy/pull/396.patch | git am -
> 
> We were wondering if you add the time to have a look at this one?
> In fact we have a similar need and Mathilde started to work on a very
> similar patch, see
> https://github.com/ShimmerGlass/haproxy/commit/c63116fe7048320abc41709e4d1b25513da91f57
> difference being, Mathilde simply concatenated the certs, and the
> patch from Arjen, uses PKCS7. Is there any specific reason to use
> PKCS7?
> 
> note: it also refers to https://github.com/haproxy/haproxy/issues/297
> 
> Best,

Hello,

I suppose it was put in a PKCS7 container to be able to distinguish each
DER part of the chain easily? So It can be used by an external tool. I'm not
sure of what is done with the result of this.

The two patches seem to have different approches, Arjen's one is
using a SSL_get0_verified_chain() and Mathild's one is using
SSL_get_peer_cert_chain(). I'm not sure what approach is the best, I
suppose that SSL_get_peer_cert_chain() is better if we want to have the
chain event if it wasn't verified and it could be completed with the
ssl_c_verify sample fetch if we need this information!

I will be grateful if a .vtc test file is also provided with sample
fetches patches, it's difficult to test every sample fetches nowadays.

There is already a vtc for client auth which is available here:
https://git.haproxy.org/?p=haproxy.git;a=blob;f=reg-tests/ssl/ssl_client_auth.vtc

Thanks!

-- 
William Lallemand



Re: [PR] Add verfied chain

2020-05-18 Thread William Dauchy
Hello William L.,

On Wed, Dec 4, 2019 at 4:24 PM PR Bot  wrote:
> Patch title(s):
>MINOR: add fetch 'ssl_c_verified_chain'
>Merge branch 'master' of https://github.com/haproxy/haproxy
> Link:
>https://github.com/haproxy/haproxy/pull/396
> Edit locally:
>wget https://github.com/haproxy/haproxy/pull/396.patch && vi 396.patch
> Apply locally:
>curl https://github.com/haproxy/haproxy/pull/396.patch | git am -

We were wondering if you add the time to have a look at this one?
In fact we have a similar need and Mathilde started to work on a very
similar patch, see
https://github.com/ShimmerGlass/haproxy/commit/c63116fe7048320abc41709e4d1b25513da91f57
difference being, Mathilde simply concatenated the certs, and the
patch from Arjen, uses PKCS7. Is there any specific reason to use
PKCS7?

note: it also refers to https://github.com/haproxy/haproxy/issues/297

Best,
-- 
William



Re: [PATCH] DOC/MINOR: halog: Add long help info for ic flag

2020-05-18 Thread William Lallemand
On Fri, May 15, 2020 at 11:05:17PM +0200, Aleksandar Lazic wrote:
> Hi.
> 
> attached a patch for halog.
> 
> Regards
> 
> Aleks

> From 37ba93a5f29200e34cfb31aacf93ddcd80fca2ab Mon Sep 17 00:00:00 2001
> From: Aleksandar Lazi 
> Date: Fri, 15 May 2020 22:58:30 +0200
> Subject: [PATCH] DOC/MINOR: halog: Add long help info for ic flag
> 
> Add missing long help text for the ic (ip count) flag
> ---

Thanks, applied!

-- 
William Lallemand



Re: [PATCH] cleanup: remove unused variable assignment (found by Coverity)

2020-05-18 Thread William Lallemand
On Sat, May 16, 2020 at 10:45:24PM +0500, Илья Шипицин wrote:
> Hello,
> 
> can we apply that patch ?
> it removes single assignment, fixes  #593
> 
> 
> Cheers,
> Ilya Shipitcin

> From e922cfb5e6e60f573c8861d3e10e6736be601d47 Mon Sep 17 00:00:00 2001
> From: Ilya Shipitsin 
> Date: Sat, 16 May 2020 22:42:12 +0500
> Subject: [PATCH] CLEANUP: src/acl.c: remove unused assignment
> 
> Coverity found unused variable assignment
> 
> CID 1299671 (#1 of 1): Unused value (UNUSED_VALUE)assigned_pointer: 
> Assigning value from args[arg + 1] to word here, but that stored value is 
> overwritten before it can be used.
>  958word = args[arg + 1];
>  959arg = arg_end;
> ---


I made a few changes in your patch, I replaced 'src/acl.c' by 'acl' in
the subject, and I wrapped the body to 80 columns.

Thanks, applied.

-- 
William Lallemand



Re: [PATCH] BUILD: ssl: include buffer common headers for ssl_sock_ctx

2020-05-18 Thread William Lallemand
On Sun, May 17, 2020 at 01:41:53PM +0200, William Dauchy wrote:
> since commit c0cdaffaa338 ("REORG: ssl: move ssl_sock_ctx and fix
> cross-dependencies issues"), `struct ssl_sock_ctx` was moved in
> ssl_sock.h. As it contains a `struct buffer`, including
> `common/buffer.h` is now mandatory. I encountered an issue while
> including ssl_sock.h on another patch:
> 
> include/types/ssl_sock.h:240:16: error: field ‘early_buf’ has incomplete type
>   240 |  struct buffer early_buf;  /* buffer to store the early data 
> received */
> 
> no backport needed.
> 
> Fixes: c0cdaffaa338 ("REORG: ssl: move ssl_sock_ctx and fix
> cross-dependencies issues")
> Signed-off-by: William Dauchy 

Thanks, merged!

-- 
William Lallemand