[PATCH] fix libslz url in travis
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
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
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
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
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
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
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
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)
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
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