Re: [PATCH] CI: fix BoringSSL builds

2020-10-11 Thread Willy Tarreau
On Sun, Oct 11, 2020 at 11:47:05PM +0500,  ??? wrote:
> Hello,
> 
> it's shame, we got BoringSSL broken for some time for no reason.

Applied, thanks Ilya. BTW, don't feel sad when CI fails to catch
such issues, nothing ever works 100% of the time, and we certainly
don't want to add yet-another-level on top of CI. It already catches
many issues, the leftovers are tiny and rare.

Cheers,
Willy



Re: From 2.1.9 compilation fails with BoringSSL

2020-10-11 Thread Илья Шипицин
for master branch (I sent travis-ci patch):



sr/include  -DCONFIG_HAPROXY_VERSION=\"2.3-dev6\"
-DCONFIG_HAPROXY_DATE=\"2020/10/10\" -c -o src/ssl_crtlist.o
src/ssl_crtlist.c
src/ssl_crtlist.c: In function ‘ssl_sock_free_ssl_conf’:
src/ssl_crtlist.c:54:12: error: ‘struct ssl_bind_conf’ has no member named
‘ciphersuites’
   54 |   free(conf->ciphersuites);
  |^~
src/ssl_crtlist.c:55:7: error: ‘struct ssl_bind_conf’ has no member named
‘ciphersuites’
   55 |   conf->ciphersuites = NULL;
  |   ^~
src/ssl_crtlist.c: In function ‘crtlist_dup_ssl_conf’:
src/ssl_crtlist.c:113:9: error: ‘struct ssl_bind_conf’ has no member named
‘ciphersuites’
  113 |  if (src->ciphersuites) {
  | ^~
src/ssl_crtlist.c:114:6: error: ‘struct ssl_bind_conf’ has no member named
‘ciphersuites’
  114 |   dst->ciphersuites = strdup(src->ciphersuites);
  |  ^~
src/ssl_crtlist.c:114:33: error: ‘struct ssl_bind_conf’ has no member named
‘ciphersuites’
  114 |   dst->ciphersuites = strdup(src->ciphersuites);
  | ^~
src/ssl_crtlist.c:115:11: error: ‘struct ssl_bind_conf’ has no member named
‘ciphersuites’
  115 |   if (!dst->ciphersuites)
  |   ^~
src/ssl_sock.c:1288:39: error: ‘struct certificate_ocsp’ declared inside
parameter list will not be visible outside of this definition or
declaration [-Werror]
 1288 | static void ssl_sock_free_ocsp(struct certificate_ocsp *ocsp)
  |   ^~~~
src/ssl_sock.c: In function ‘ssl_sock_free_ocsp’:
src/ssl_sock.c:1293:6: error: invalid use of undefined type ‘struct
certificate_ocsp’
 1293 |  ocsp->refcount--;
  |  ^~
src/ssl_sock.c:1294:10: error: invalid use of undefined type ‘struct
certificate_ocsp’
 1294 |  if (ocsp->refcount <= 0) {
  |  ^~
src/ssl_sock.c:1295:20: error: invalid use of undefined type ‘struct
certificate_ocsp’
 1295 |   ebmb_delete(>key);
  |^~
src/ssl_sock.c:1296:22: error: invalid use of undefined type ‘struct
certificate_ocsp’
 1296 |   chunk_destroy(>response);
  |  ^~
make: *** [Makefile:889: src/ssl_crtlist.o] Error 1
make: *** Waiting for unfinished jobs
src/ssl_sock.c: In function ‘ssl_sock_prepare_ctx’:
src/ssl_sock.c:4116:43: error: ‘struct ssl_bind_conf’ has no member named
‘ciphersuites’
 4116 |  conf_ciphersuites = (ssl_conf && ssl_conf->ciphersuites) ?
ssl_conf->ciphersuites : bind_conf->ssl_conf.ciphersuites;
  |   ^~
src/ssl_sock.c:4116:69: error: ‘struct ssl_bind_conf’ has no member named
‘ciphersuites’
 4116 |  conf_ciphersuites = (ssl_conf && ssl_conf->ciphersuites) ?
ssl_conf->ciphersuites : bind_conf->ssl_conf.ciphersuites;
  |
^~
src/ssl_sock.c:4116:105: error: ‘struct ssl_bind_conf’ has no member named
‘ciphersuites’
 4116 |  conf_ciphersuites = (ssl_conf && ssl_conf->ciphersuites) ?
ssl_conf->ciphersuites : bind_conf->ssl_conf.ciphersuites;
  |
^
src/ssl_sock.c:4118:7: error: implicit declaration of function
‘SSL_CTX_set_ciphersuites’; did you mean ‘SSL_CTX_set_cipher_list’?
[-Werror=implicit-function-declaration]
 4118 |  !SSL_CTX_set_ciphersuites(ctx, conf_ciphersuites)) {
  |   ^~~~
  |   SSL_CTX_set_cipher_list
src/ssl_sock.c: In function ‘ssl_sock_prepare_srv_ctx’:
src/ssl_sock.c:4573:18: error: ‘struct ’ has no member named
‘ciphersuites’
 4573 |  if (srv->ssl_ctx.ciphersuites &&
  |  ^
src/ssl_sock.c:4574:59: error: ‘struct ’ has no member named
‘ciphersuites’
 4574 |   !SSL_CTX_set_ciphersuites(srv->ssl_ctx.ctx,
srv->ssl_ctx.ciphersuites)) {
  |   ^
src/ssl_sock.c:4577:49: error: ‘struct ’ has no member named
‘ciphersuites’
 4577 | srv->conf.file, srv->conf.line, srv->ssl_ctx.ciphersuites);
  | ^
src/ssl_sock.c: In function ‘ssl_sock_init’:
src/ssl_sock.c:5036:4: error: implicit declaration of function
‘SSL_set_max_early_data’; did you mean ‘SSL_in_early_data’?
[-Werror=implicit-function-declaration]
 5036 |SSL_set_max_early_data(ctx->ssl,
  |^~
  |SSL_in_early_data
src/ssl_sock.c: In function ‘ssl_sock_handshake’:
src/ssl_sock.c:5097:10: error: implicit declaration of function
‘SSL_read_early_data’; did you mean ‘SSL_in_early_data’?
[-Werror=implicit-function-declaration]
 5097 |ret = SSL_read_early_data(ctx->ssl,
  |  ^~~
  |  SSL_in_early_data
src/ssl_sock.c:5100:15: error: ‘SSL_READ_EARLY_DATA_ERROR’ undeclared
(first use in this function); did you mean ‘SSL_AD_DECRYPT_ERROR’?
 5100 |if (ret == SSL_READ_EARLY_DATA_ERROR)
  |   ^
  |   SSL_AD_DECRYPT_ERROR
src/ssl_sock.c:5100:15: note: each undeclared identifier is reported only
once for 

[PATCH] CI: fix BoringSSL builds

2020-10-11 Thread Илья Шипицин
Hello,

it's shame, we got BoringSSL broken for some time for no reason.

Ilya
From 8aa0885f38147572974e434518f1e4f3ab44398d Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Sun, 11 Oct 2020 23:42:51 +0500
Subject: [PATCH] CI: travis-ci: replace not defined SSL_LIB, SSL_INC for
 BotringSSL builds

after 73b520b958be4ee79b4f09a32d6718a13dc2d1fd variables SSL_LIB, SSL_INC
are not set, but still used by BoringSSL builds. That leads to error
(I wish we could stop on such errors) and using stock openssl instead
of boringssl
---
 scripts/build-ssl.sh | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/build-ssl.sh b/scripts/build-ssl.sh
index 34eee9bb9..9a6a2b241 100755
--- a/scripts/build-ssl.sh
+++ b/scripts/build-ssl.sh
@@ -103,14 +103,14 @@ if [ ! -z ${BORINGSSL+x} ]; then
 	cmake  -GNinja -DCMAKE_BUILD_TYPE=release -DBUILD_SHARED_LIBS=1 ..
 	ninja
 
-	rm -rf ${SSL_LIB} || exit 0
-	rm -rf ${SSL_INC} || exit 0
+	rm -rf ${HOME}/opt/lib || exit 0
+	rm -rf ${HOME}/opt/include || exit 0
 
-	mkdir -p ${SSL_LIB}
-	cp crypto/libcrypto.so ssl/libssl.so ${SSL_LIB}
+	mkdir -p ${HOME}/opt/lib
+	cp crypto/libcrypto.so ssl/libssl.so ${HOME}/opt/lib
 
-	mkdir -p ${SSL_INC}
-	cp -r ../include/* ${SSL_INC}
+	mkdir -p ${HOME}/opt/include
+	cp -r ../include/* ${HOME}/opt/include
 	)
 fi
 
-- 
2.26.2



Re: From 2.1.9 compilation fails with BoringSSL

2020-10-11 Thread Илья Шипицин
seems, our CI for boringssl is broken. I see "openssl" there

https://travis-ci.com/github/haproxy/haproxy/jobs/397911591#L1102-L1103

вс, 11 окт. 2020 г. в 15:06, László Soós :

> Hi Willy, All,
>
> Starting from 2.1.9 compilation fails with:
>
> src/ssl_sock.c:1231:39: warning: 'struct certificate_ocsp' declared inside
> parameter list will not be visible outside of this definition or declaration
>  1231 | static void ssl_sock_free_ocsp(struct certificate_ocsp *ocsp)
>   |   ^~~~
> src/ssl_sock.c: In function 'ssl_sock_free_ocsp':
> src/ssl_sock.c:1236:6: error: dereferencing pointer to incomplete type
> 'struct certificate_ocsp'
>  1236 |  ocsp->refcount--;
>   |  ^~
>   CC  src/mux_fcgi.o
>   CC  src/cfgparse-listen.o
>   CC  src/http_ana.o
> At top level:
> src/ssl_sock.c:1231:13: warning: 'ssl_sock_free_ocsp' defined but not used
> [-Wunused-function]
>  1231 | static void ssl_sock_free_ocsp(struct certificate_ocsp *ocsp)
>
> 
> I went and checked source code (for 2.2.4 as it has the same problem)
>
> https://git.haproxy.org/?p=haproxy-2.2.git;a=blob;f=src/ssl_sock.c;h=019597ae76f2cb926b7ad42baf6378cf3456c417;hb=HEAD
> LINE 1290
> static void ssl_sock_free_ocsp(struct certificate_ocsp *ocsp)
>
> This is defined in a section:
>
> #if ((defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined OPENSSL_NO_OCSP) 
> || defined OPENSSL_IS_BORINGSSL)
>
> If I go up to LINE 851 where struct certificate_ocsp  is defined, it's
> being in a section:
> #if (defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined OPENSSL_NO_OCSP)
>
>
> So  struct certificate_ocsp will not be defined for BORINGSSL and
> compilation fails.
>
> My quick solution was (maybe not the best but it works) to move the struct
> def above the wrong section like this:
> #if ((defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined
> OPENSSL_NO_OCSP) || defined OPENSSL_IS_BORINGSSL)
> /*
>  * struct alignment works here such that the key.key is the same as
> key_data
>  * Do not change the placement of key_data
>  */
> struct certificate_ocsp {
>struct ebmb_node key;
>unsigned char key_data[OCSP_MAX_CERTID_ASN1_LENGTH];
>struct buffer response;
>int refcount;
>long expire;
> };
> #endif
>
> Can you consider this as a bug and maybe potentially fix it in future
> releases?
>
>
> For the records after this 'patch' I get the below warnings with BoringSSL
> but I think it's safe to ignore (?):
> 
>   CC  src/hlua_fcn.o
>   CC  src/namespace.o
> src/ssl_sock.c:1292:13: warning: 'ssl_sock_free_ocsp' defined but not used
> [-Wunused-function]
>  1292 | static void ssl_sock_free_ocsp(struct certificate_ocsp *ocsp)
>   | ^~
>   CC  src/mux_fcgi.o
>   CC  src/mux_h1.o
> In file included from include/haproxy/pool.h:29,
>  from include/haproxy/chunk.h:31,
>  from include/haproxy/dynbuf.h:33,
>  from include/haproxy/channel.h:27,
>  from src/ssl_crtlist.c:23:
> src/ssl_crtlist.c: In function 'crtlist_parse_file':
> include/haproxy/list.h:51:70: warning: potential null pointer dereference
> [-Wnull-dereference]
>51 | #define LIST_ADDQ(lh, el) ({ (el)->p = (lh)->p; (el)->p->n =
> (lh)->p = (el); (el)->n = (lh); (el); })
>   |
>  ^~
> src/ssl_crtlist.c:425:3: note: in expansion of macro 'LIST_ADDQ'
>   425 |   LIST_ADDQ(>crtlist_entry, >by_ckch_store);
>   |   ^
> include/haproxy/list.h:51:44: warning: potential null pointer dereference
> [-Wnull-dereference]
>51 | #define LIST_ADDQ(lh, el) ({ (el)->p = (lh)->p; (el)->p->n =
> (lh)->p = (el); (el)->n = (lh); (el); })
>   |^~~
> src/ssl_crtlist.c:425:3: note: in expansion of macro 'LIST_ADDQ'
>   425 |   LIST_ADDQ(>crtlist_entry, >by_ckch_store);
>   |   ^
>   CC  src/mux_h2.o
>   CC  src/backend.o
>   CC  src/cfgparse.o
> 
>
> Thanks,
>   sooslaca
>
>


Re: Partial response

2020-10-11 Thread Willy Tarreau
On Sun, Oct 11, 2020 at 04:47:32PM +0330, Seena Fallah wrote:
> Hi. Does haproxy support partial response form servers?
> In nginx there is a parameter named proxy_read_timeout that defines a
> timeout for reading a response from the proxied server. The timeout is set
> only between two successive read operations, not for the transmission of
> the whole response. If the proxied server does not transmit anything within
> this time, the connection is closed.
> Does Haproxy have this option too?

I don't know why you call this a feature or "partial response", it's the
normal way to support a read timeout and I'm pretty sure that every single
proxy around supports this as it's absolutely mandatory if you don't want
to restart your process every now and then. In haproxy you even get a
warning if you forget to set such a timeout (it's called "timeout server").

Willy



Partial response

2020-10-11 Thread Seena Fallah
Hi. Does haproxy support partial response form servers?
In nginx there is a parameter named proxy_read_timeout that defines a
timeout for reading a response from the proxied server. The timeout is set
only between two successive read operations, not for the transmission of
the whole response. If the proxied server does not transmit anything within
this time, the connection is closed.
Does Haproxy have this option too?


From 2.1.9 compilation fails with BoringSSL

2020-10-11 Thread László Soós
Hi Willy, All,

Starting from 2.1.9 compilation fails with:

src/ssl_sock.c:1231:39: warning: 'struct certificate_ocsp' declared inside
parameter list will not be visible outside of this definition or declaration
 1231 | static void ssl_sock_free_ocsp(struct certificate_ocsp *ocsp)
  |   ^~~~
src/ssl_sock.c: In function 'ssl_sock_free_ocsp':
src/ssl_sock.c:1236:6: error: dereferencing pointer to incomplete type
'struct certificate_ocsp'
 1236 |  ocsp->refcount--;
  |  ^~
  CC  src/mux_fcgi.o
  CC  src/cfgparse-listen.o
  CC  src/http_ana.o
At top level:
src/ssl_sock.c:1231:13: warning: 'ssl_sock_free_ocsp' defined but not used
[-Wunused-function]
 1231 | static void ssl_sock_free_ocsp(struct certificate_ocsp *ocsp)


I went and checked source code (for 2.2.4 as it has the same problem)
https://git.haproxy.org/?p=haproxy-2.2.git;a=blob;f=src/ssl_sock.c;h=019597ae76f2cb926b7ad42baf6378cf3456c417;hb=HEAD
LINE 1290
static void ssl_sock_free_ocsp(struct certificate_ocsp *ocsp)

This is defined in a section:

#if ((defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined
OPENSSL_NO_OCSP) || defined OPENSSL_IS_BORINGSSL)

If I go up to LINE 851 where struct certificate_ocsp  is defined, it's
being in a section:
#if (defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined OPENSSL_NO_OCSP)


So  struct certificate_ocsp will not be defined for BORINGSSL and
compilation fails.

My quick solution was (maybe not the best but it works) to move the struct
def above the wrong section like this:
#if ((defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined
OPENSSL_NO_OCSP) || defined OPENSSL_IS_BORINGSSL)
/*
 * struct alignment works here such that the key.key is the same as key_data
 * Do not change the placement of key_data
 */
struct certificate_ocsp {
   struct ebmb_node key;
   unsigned char key_data[OCSP_MAX_CERTID_ASN1_LENGTH];
   struct buffer response;
   int refcount;
   long expire;
};
#endif

Can you consider this as a bug and maybe potentially fix it in future
releases?


For the records after this 'patch' I get the below warnings with BoringSSL
but I think it's safe to ignore (?):

  CC  src/hlua_fcn.o
  CC  src/namespace.o
src/ssl_sock.c:1292:13: warning: 'ssl_sock_free_ocsp' defined but not used
[-Wunused-function]
 1292 | static void ssl_sock_free_ocsp(struct certificate_ocsp *ocsp)
  | ^~
  CC  src/mux_fcgi.o
  CC  src/mux_h1.o
In file included from include/haproxy/pool.h:29,
 from include/haproxy/chunk.h:31,
 from include/haproxy/dynbuf.h:33,
 from include/haproxy/channel.h:27,
 from src/ssl_crtlist.c:23:
src/ssl_crtlist.c: In function 'crtlist_parse_file':
include/haproxy/list.h:51:70: warning: potential null pointer dereference
[-Wnull-dereference]
   51 | #define LIST_ADDQ(lh, el) ({ (el)->p = (lh)->p; (el)->p->n =
(lh)->p = (el); (el)->n = (lh); (el); })
  |
 ^~
src/ssl_crtlist.c:425:3: note: in expansion of macro 'LIST_ADDQ'
  425 |   LIST_ADDQ(>crtlist_entry, >by_ckch_store);
  |   ^
include/haproxy/list.h:51:44: warning: potential null pointer dereference
[-Wnull-dereference]
   51 | #define LIST_ADDQ(lh, el) ({ (el)->p = (lh)->p; (el)->p->n =
(lh)->p = (el); (el)->n = (lh); (el); })
  |^~~
src/ssl_crtlist.c:425:3: note: in expansion of macro 'LIST_ADDQ'
  425 |   LIST_ADDQ(>crtlist_entry, >by_ckch_store);
  |   ^
  CC  src/mux_h2.o
  CC  src/backend.o
  CC  src/cfgparse.o


Thanks,
  sooslaca