Re: How to compile with packaged openssl when custom openssl installed?

[Willy Tarreau]

Hi Shawn,

On Wed, Nov 03, 2021 at 10:56:02AM -0600, Shawn Heisey wrote:
> On 11/3/21 9:25 AM,  ??? wrote:
> > you either need to specify LD_LIBRARY_PATH or add rpath during link,
> > here's example how to use rpath via ADDLIB haproxy/.travis.yml at
> > 57610c694e56a6b0d55bf42f1170bad93b7b3297 · haproxy/haproxy (github.com) 
> > 
> 
> 
> I can't tell how to actually use that for my setup from the highlighted
> lines in that github page.
> 
> Everything I have seen says that haproxy's build system is ignoring the
> SSL_INC and SSL_LIB settings I told it to use, and autodetecting the openssl
> in /usr/local.
> 
> But even if I am wrong about that, I did work out how to achieve my goals. 
> I built openssl with --prefix=/usr/local/ssl3 and made a symlink for its
> "openssl" binary to /usr/local/bin/ossl. I get to have the custom openssl
> installed and available with an altered command, but now haproxy's build
> system won't find it.

Normally you just have to specify SSL_INC and SSL_LIB at build time to
specify the one you want to build with. I'm doing exactly this when I
want to build with older versions:

  $ ls -1d /opt/openssl-* 
  /opt/openssl-0.9.8/
  /opt/openssl-1.0.0/
  /opt/openssl-1.0.2/
  /opt/openssl-1.1.0/

  $ make -j$(nproc) TARGET=linux-glibc USE_OPENSSL=1 \
SSL_INC=/opt/openssl-1.0.2/include SSL_LIB=/opt/openssl-1.0.2/lib
  ...
  LD  haproxy
  $ ./haproxy -v
  HAProxy version 2.5-dev12-726635-14 2021/11/03 - https://haproxy.org/

When used on the same machine you used to build, it's also possible to
use -rpath to store the lib's path into the executable:

  $ make -j$(nproc) TARGET=linux-glibc USE_OPENSSL=1 \
SSL_INC=/opt/openssl-1.0.2/include \
SSL_LIB="/opt/openssl-1.0.2/lib -Wl,-rpath=/opt/openssl-1.0.2/lib"

  $ ldd ./haproxy
linux-vdso.so.1 (0x7ffce1ff9000)
libcrypt.so.1 => /lib64/libcrypt.so.1 (0x7f5fa6f83000)
libdl.so.2 => /lib64/libdl.so.2 (0x7f5fa6f7e000)
librt.so.1 => /lib64/librt.so.1 (0x7f5fa6f74000)
libpthread.so.0 => /lib64/libpthread.so.0 (0x7f5fa6f52000)
libssl.so.1.0.0 => /opt/openssl-1.0.2/lib/libssl.so.1.0.0 
(0x7f5fa6ce2000)
libcrypto.so.1.0.0 => /opt/openssl-1.0.2/lib/libcrypto.so.1.0.0 
(0x7f5fa689d000)
libc.so.6 => /lib64/libc.so.6 (0x7f5fa66b6000)
/lib64/ld-linux-x86-64.so.2 (0x7f5fa700e000)

  $ ./haproxy -vv | grep -i ssl
  OPTIONS = USE_OPENSSL=1
  Feature list : +EPOLL -KQUEUE +NETFILTER -PCRE -PCRE_JIT -PCRE2 -PCRE2_JIT 
+POLL +THREAD +BACKTRACE -STATIC_PCRE -STATIC_PCRE2 +TPROXY +LINUX_TPROXY 
+LINUX_SPLICE +LIBCRYPT +CRYPT_H +GETADDRINFO +OPENSSL -LUA +ACCEPT4 -CLOSEFROM 
-ZLIB +SLZ +CPU_AFFINITY +TFO +NS +DL +RT -DEVICEATLAS -51DEGREES -WURFL 
-SYSTEMD -OBSOLETE_LINKER +PRCTL -PROCCTL +THREAD_DUMP -EVPORTS -OT -QUIC 
-PROMEX -MEMORY_PROFILING
  Built with OpenSSL version : OpenSSL 1.0.2j  26 Sep 2016
  Running on OpenSSL version : OpenSSL 1.0.2j  26 Sep 2016
  OpenSSL library supports TLS extensions : yes
  OpenSSL library supports SNI : yes
  OpenSSL library supports : SSLv3 TLSv1.0 TLSv1.1 TLSv1.2

There's no reason that wouldn't work for you, as it's commonly used.
I suspect you just have one option wrong (possibly missing /lib at
the end of the SSL_LIB for example).

Willy

[ANNOUNCE] haproxy-2.3.15

[Willy Tarreau]

Hi,

HAProxy 2.3.15 was released on 2021/11/04. It added 88 new commits
after version 2.3.14.

It's been almost two months without a 2.3 release now, and reports on it
are already fading away given that most users are on LTS versions. This
version collects the fixes that already went into 2.4.5 to 2.4.8, thus
for simplicity I'm copying some parts from the previous announces, so if
you have a deja-vu feeling, that's normal:

  - if an HTTP/1 message was blocked for analysis waiting for some more
room, sometimes it could remain stuck indefinitely, leaving a few
never-expiring entries in "show sess".

  - A very old bug was fixed in the Lua part. The wrong function was used
to start Lua tasks leading to a process freeze if the call was
performed when the time was wrapping, one millisecond every 49.7
days. On this exact millisecond, a lua task was able to be queued
with no expiration date, preventing all subsequent timers from being
seen as expired.

  - A rare possibility to divide by zero in the leastconn balance
algorithm because of a thread-unsafe use of a shared variable was
fixed.

  - Some bugs were fixed on the filters management to properly handle
client aborts and to be sure to always release allocated filters when
a stream is released.

  - The LDAP health-check was fixed to make it compatible with Active
Directory servers. The response parsing was improved to also support
servers using multi-bytes length-encoding. Active Directory servers
seems to systematically encode messages or elements length on 4 bytes
while others are using 1-byte length-encoding if possible. Now, 1, 2
and 4 bytes length-encoding are now supported. It should be good
enough to enable LDAP health-check on Active Directory servers.

  - Occasional crashes in malloc_trim() on recent glibc when running with
jemalloc were worked around.

  - The build system was improved in many ways. Several -Wundef warnings
were fixed.

  - HTTP "TE" header is now sanitized when a request is sent to a server.
Only "trailers" token is sent. It is mandatory because HAProxy only
understand chunked encoding. Other transfer encoding are not
supported.

  - A bug on health-check was fixed when a sample fetch depending on the
execution context was used in a tcpcheck rulesets defined in a
defaults section.

  - tcp-request and tcp-response content rules evaluation is now
interrupted if a read error or the end of input is detected on the
corresponding channel. This change fixes a known bug in HAProxy 2.3
and prior. However, it does not seem to affect 2.4.

  - errors detected in the expression parser used with the "set-var"
action weren't detailed enough and sometimes misleading, often
leaving doubts about what to fix. This was fixed.

  - resolvers: there were a large number of structural issues in the
code, and quite frankly we're not proud of the solutions but it's
impossible to do something more elegant in the current state without
a major rewrite. So what matters here is that all race conditions are
addressed and that the code works reliably. While the 2.5 fixes add a
lookup tree to perform significant CPU savings on SRV records, that
code was not backported because it adds further changes that do not
seem necessary in the current situation. We got the confirmation from
one of the reporters that the issue is now fixed.

  - an interesting bug in the ring API caused boundary checks for the
wrapping at the end of the buffer to be shifted by one both in the
producer and the consumer, thus they both cancel each other and are
not observable... until the byte after the buffer is not mapped or
belongs to another area. One crash was met on boot (since startup
messages are duplicated into a ring for later retrieval), and it is
possible that those sending logs over TCP might have faced it as
well, otherwise it's extremely unlikely to be observed outside of
these use cases.

  - using the tarpit could lead to head-of-line blocking of an H2
connection as the pending data were not drained. And in other
protocols, the presence of these pending data could cause a wakeup
loop between the mux and the stream, which usually ended in the
process being detected as faulty and being killed by the safety
checks.

  - a similar wakeup loop could also happen when waiting for more data
(e.g. option http-buffer-request) with lots of data already present
in the receive buffer while the lower layer could only deliver a full
block at once, that couldn't fit.

  - the h2spec tests in the CI were regularly failing on a few tests
expecting HTTP/2 GOAWAY frames that were sent (even seen in strace).
The problem was that we didn't perform a graceful shutdown and that
this copes badly with bidirectional communications as unread pending
data cause the connection to

Last-minute proposal for 2.5 about httpslog

[Willy Tarreau]

Hello,

as some of you know, 2.5 will come with a new "option httpslog" to ease
logging some useful TLS info by default.

While running some tests in production with the error-log-format, I
realized that we're not logging the SNI in "httpslog", and that it's
probably a significant miss that we ought to fix before the release.
I think it could be particularly useful for those using long crt-lists
with a default domain, as it will allow to figure which ones have been
handled by the default one possibly due to a missing certificate or a
misconfiguration.

Right now the default HTTPS format is defined this way :

log-format "%ci:%cp [%tr] %ft %b/%s %TR/%Tw/%Tc/%Tr/%Ta %ST %B %CC \
   %CS %tsc %ac/%fc/%bc/%sc/%rc %sq/%bq %hr %hs %{+Q}r \
   %[fc_conn_err]/%[ssl_fc_err,hex]/%[ssl_c_err]/\
   %[ssl_c_ca_err]/%[ssl_fc_is_resumed] %sslv/%sslc"

As it is, it closely matches the httplog one so that tools configured to
process the latter should also work unmodified with the new one.

The question is, should we add "ssl_fc_sni" somewhere in this line, and
if so, where? Logging it at the end seems sensible to me so that even if
it's absent we're not missing anything. But maybe there are better options
or opinions on the subject.

Feel free to suggest so that we put something there before tomorrow and
have it in a last dev13 before the release.

Thanks,
Willy

HAProxyConf 2021 is coming soon (16-17th)

[Willy Tarreau]

Hi all,

just as a reminder for those who don't necessarily follow the activity
around this, the HAProxyConf 2021 will be held on 16-17 of this month
(in 12 days), with live Q&A sessions after each talk. The conference is
online only, and attending it is free and open to anyone.

The list of presentations and speakers is available here:

https://www.haproxyconf.com/presentations/
https://www.haproxyconf.com/speakers/

and the detailed schedule will be published very soon. You can register
to get notified of updates.

All the details are available there:

https://www.haproxyconf.com/

See you there soon,
Willy

Re: Last-minute proposal for 2.5 about httpslog

[Aleksandar Lazic]

On 04.11.21 15:28, Willy Tarreau wrote:

Hello,

as some of you know, 2.5 will come with a new "option httpslog" to ease
logging some useful TLS info by default.

While running some tests in production with the error-log-format, I
realized that we're not logging the SNI in "httpslog", and that it's
probably a significant miss that we ought to fix before the release.
I think it could be particularly useful for those using long crt-lists
with a default domain, as it will allow to figure which ones have been
handled by the default one possibly due to a missing certificate or a
misconfiguration.

Right now the default HTTPS format is defined this way :

 log-format "%ci:%cp [%tr] %ft %b/%s %TR/%Tw/%Tc/%Tr/%Ta %ST %B %CC \
%CS %tsc %ac/%fc/%bc/%sc/%rc %sq/%bq %hr %hs %{+Q}r \
%[fc_conn_err]/%[ssl_fc_err,hex]/%[ssl_c_err]/\
%[ssl_c_ca_err]/%[ssl_fc_is_resumed] %sslv/%sslc"

As it is, it closely matches the httplog one so that tools configured to
process the latter should also work unmodified with the new one.

The question is, should we add "ssl_fc_sni" somewhere in this line, and
if so, where? Logging it at the end seems sensible to me so that even if
it's absent we're not missing anything. But maybe there are better options
or opinions on the subject.


A big bold +1 to add the sni header to the log.


Feel free to suggest so that we put something there before tomorrow and
have it in a last dev13 before the release.

Thanks,
Willy

Re: Last-minute proposal for 2.5 about httpslog

[Willy Tarreau]

On Thu, Nov 04, 2021 at 03:54:15PM +0100, Aleksandar Lazic wrote:
> On 04.11.21 15:28, Willy Tarreau wrote:
> > Hello,
> > 
> > as some of you know, 2.5 will come with a new "option httpslog" to ease
> > logging some useful TLS info by default.
> > 
> > While running some tests in production with the error-log-format, I
> > realized that we're not logging the SNI in "httpslog", and that it's
> > probably a significant miss that we ought to fix before the release.
> > I think it could be particularly useful for those using long crt-lists
> > with a default domain, as it will allow to figure which ones have been
> > handled by the default one possibly due to a missing certificate or a
> > misconfiguration.
> > 
> > Right now the default HTTPS format is defined this way :
> > 
> >  log-format "%ci:%cp [%tr] %ft %b/%s %TR/%Tw/%Tc/%Tr/%Ta %ST %B %CC \
> > %CS %tsc %ac/%fc/%bc/%sc/%rc %sq/%bq %hr %hs %{+Q}r \
> > %[fc_conn_err]/%[ssl_fc_err,hex]/%[ssl_c_err]/\
> > %[ssl_c_ca_err]/%[ssl_fc_is_resumed] %sslv/%sslc"
> > 
> > As it is, it closely matches the httplog one so that tools configured to
> > process the latter should also work unmodified with the new one.
> > 
> > The question is, should we add "ssl_fc_sni" somewhere in this line, and
> > if so, where? Logging it at the end seems sensible to me so that even if
> > it's absent we're not missing anything. But maybe there are better options
> > or opinions on the subject.
> 
> A big bold +1 to add the sni header to the log.

thanks Alex, that comforts me in the fact that I was not alone to
miss it :-)

Willy

Re: How to compile with packaged openssl when custom openssl installed?

[Илья Шипицин]

чт, 4 нояб. 2021 г. в 18:58, Willy Tarreau :

> Hi Shawn,
>
> On Wed, Nov 03, 2021 at 10:56:02AM -0600, Shawn Heisey wrote:
> > On 11/3/21 9:25 AM,  ??? wrote:
> > > you either need to specify LD_LIBRARY_PATH or add rpath during link,
> > > here's example how to use rpath via ADDLIB haproxy/.travis.yml at
> > > 57610c694e56a6b0d55bf42f1170bad93b7b3297 · haproxy/haproxy (github.com)
> <
> https://github.com/haproxy/haproxy/blob/57610c694e56a6b0d55bf42f1170bad93b7b3297/.travis.yml#L68-L85
> >
> >
> >
> > I can't tell how to actually use that for my setup from the highlighted
> > lines in that github page.
> >
> > Everything I have seen says that haproxy's build system is ignoring the
> > SSL_INC and SSL_LIB settings I told it to use, and autodetecting the
> openssl
> > in /usr/local.
> >
> > But even if I am wrong about that, I did work out how to achieve my
> goals.
> > I built openssl with --prefix=/usr/local/ssl3 and made a symlink for its
> > "openssl" binary to /usr/local/bin/ossl. I get to have the custom openssl
> > installed and available with an altered command, but now haproxy's build
> > system won't find it.
>
> Normally you just have to specify SSL_INC and SSL_LIB at build time to
> specify the one you want to build with. I'm doing exactly this when I
> want to build with older versions:
>
>   $ ls -1d /opt/openssl-*
>   /opt/openssl-0.9.8/
>   /opt/openssl-1.0.0/
>   /opt/openssl-1.0.2/
>   /opt/openssl-1.1.0/
>
>   $ make -j$(nproc) TARGET=linux-glibc USE_OPENSSL=1 \
> SSL_INC=/opt/openssl-1.0.2/include SSL_LIB=/opt/openssl-1.0.2/lib
>   ...
>   LD  haproxy
>   $ ./haproxy -v
>   HAProxy version 2.5-dev12-726635-14 2021/11/03 - https://haproxy.org/
>
> When used on the same machine you used to build, it's also possible to
> use -rpath to store the lib's path into the executable:
>
>   $ make -j$(nproc) TARGET=linux-glibc USE_OPENSSL=1 \
> SSL_INC=/opt/openssl-1.0.2/include \
> SSL_LIB="/opt/openssl-1.0.2/lib -Wl,-rpath=/opt/openssl-1.0.2/lib"
>
>   $ ldd ./haproxy
> linux-vdso.so.1 (0x7ffce1ff9000)
> libcrypt.so.1 => /lib64/libcrypt.so.1 (0x7f5fa6f83000)
> libdl.so.2 => /lib64/libdl.so.2 (0x7f5fa6f7e000)
> librt.so.1 => /lib64/librt.so.1 (0x7f5fa6f74000)
> libpthread.so.0 => /lib64/libpthread.so.0 (0x7f5fa6f52000)
> libssl.so.1.0.0 => /opt/openssl-1.0.2/lib/libssl.so.1.0.0
> (0x7f5fa6ce2000)
> libcrypto.so.1.0.0 => /opt/openssl-1.0.2/lib/libcrypto.so.1.0.0
> (0x7f5fa689d000)
> libc.so.6 => /lib64/libc.so.6 (0x7f5fa66b6000)
> /lib64/ld-linux-x86-64.so.2 (0x7f5fa700e000)
>
>   $ ./haproxy -vv | grep -i ssl
>   OPTIONS = USE_OPENSSL=1
>   Feature list : +EPOLL -KQUEUE +NETFILTER -PCRE -PCRE_JIT -PCRE2
> -PCRE2_JIT +POLL +THREAD +BACKTRACE -STATIC_PCRE -STATIC_PCRE2 +TPROXY
> +LINUX_TPROXY +LINUX_SPLICE +LIBCRYPT +CRYPT_H +GETADDRINFO +OPENSSL -LUA
> +ACCEPT4 -CLOSEFROM -ZLIB +SLZ +CPU_AFFINITY +TFO +NS +DL +RT -DEVICEATLAS
> -51DEGREES -WURFL -SYSTEMD -OBSOLETE_LINKER +PRCTL -PROCCTL +THREAD_DUMP
> -EVPORTS -OT -QUIC -PROMEX -MEMORY_PROFILING
>   Built with OpenSSL version : OpenSSL 1.0.2j  26 Sep 2016
>   Running on OpenSSL version : OpenSSL 1.0.2j  26 Sep 2016
>   OpenSSL library supports TLS extensions : yes
>   OpenSSL library supports SNI : yes
>   OpenSSL library supports : SSLv3 TLSv1.0 TLSv1.1 TLSv1.2
>
> There's no reason that wouldn't work for you, as it's commonly used.
> I suspect you just have one option wrong (possibly missing /lib at
> the end of the SSL_LIB for example).
>

wow.
we do not fail build if SSL_LIB points to wrong folder ?


>
> Willy
>
>

Re: How to compile with packaged openssl when custom openssl installed?

[Willy Tarreau]

On Thu, Nov 04, 2021 at 09:53:59PM +0500,  ??? wrote:
> we do not fail build if SSL_LIB points to wrong folder ?

For sure we do, since libs will be missing, and the linking will
fail!

Willy

Re: How to compile with packaged openssl when custom openssl installed?

[Shawn Heisey]

On 11/4/21 7:55 AM, Willy Tarreau wrote:

Normally you just have to specify SSL_INC and SSL_LIB at build time to
specify the one you want to build with. I'm doing exactly this when I
want to build with older versions:



I tried this.  My make command (building 2.4.8) had these env additions:

  SSL_INC=/usr/include/openssl \
  SSL_LIB=/usr/lib/x86_64-linux-gnu \

Which should have told it to use the openssl provided by Ubuntu 
packages.  But that didn't work, it still found the 3.x version in 
/usr/local (installed with openssl default locations for ./Configure), 
and failed to compile.


I thought I found an error in the Makefile where setting USE_OPENSSL 
clears SSL_INC and SSL_LIB, but even with that problem handled (I think 
... my Makefile experience is slim), it STILL finds the 3.x version and 
tries to use it.


Changing the prefix on the openssl compile to something nonstandard 
(/usr/local/ssl3 in my case) is the only way I have found to keep the 
haproxy build from finding it.  This is less than ideal, but sufficient 
for my needs.


Thanks,
Shawn

Re: [EXTERNAL] Re: [PATCH 1/2] MINOR: jwt: Make invalid static JWT algorithms an error in `jwt_verify` converter

[Tim Düsterhus]

Remi,

On 11/3/21 9:47 AM, Remi Tricot-Le Breton wrote:

As for the second one, it would have been easier to simply add a string
length comparison before the strcmp in my opinion. We would have had a
one line fix instead of a full conversion of strXXX calls into ist
equivalents (most of which worked fine thanks to the constant algorithm
string length).



Your patch is already merged and the bug is fixed. However I'd like to 
comment on the reasons behind why I refactored the whole function to use 
the ist API:


I *strongly* dislike code that just works because of some implicit 
assumptions that might or might not hold after future changes. This is 
C, every messed up boundary check can easily result in some major issues.


In this specific case the implicit assumption is that all supported 
algorithms are exactly 5 characters long. This is true with the current 
implementation in HAProxy which just supports the HS, RS, ES, and PS 
families. However the JOSE specification [1] also specifies other values 
for the 'alg' field (though most of them for encrypted instead of just 
signed tokens) and they have differing lengths.


If someone in the future wants to add support for a 6 character 
algorithm, then they need to remember to add the length check to not run 
into the same strncmp() issue like you did. My experience is that if a 
mistake is made once, it is going to be made again.


The ist API already contains a large number of functions for *safe* 
handling of strings that are not NUL terminated. It is very hard to 
misuse them, because they all include the appropriate checks. If you see 
isteq(dynamic, ist("static")) then it is very clear that this will match 
the string "statis" exactly. For strncmp you will need to perform the 
length check yourself. You also need to remember to manually subtract 1 
for the trailing NUL when using sizeof, whereas this is done 
automatically by the ist() function.


While I am not an expert in optimization and getting the last percent of 
performance out of a function, I don't think the ist API is going to be 
measurably slower than strncmp. It is used everywhere within HTX after 
all! For the JWT the OpenSSL crypto is going to take up the majority of 
the time anyway.


Hope this help to clarify why I'm strongly pushing the use of the ist 
API and why I've contributed a fair number of additional functions in 
the past when I encountered situations where the current functions were 
not ergonomic to use. One example is the istsplit() function which I 
added for uri_normalizer.c. It makes tokenization of strings very easy 
and safe.


Best regards
Tim Düsterhus

[1] https://www.iana.org/assignments/jose/jose.xhtml

[PATCH 1/2] DEV: coccinelle: Add ha_free.cocci

[Tim Duesterhus]

Taken from 61cfdf4fd8a93dc6fd9922d5b309a71bdc7d2853.
---
 dev/coccinelle/ha_free.cocci | 6 ++
 1 file changed, 6 insertions(+)
 create mode 100644 dev/coccinelle/ha_free.cocci

diff --git a/dev/coccinelle/ha_free.cocci b/dev/coccinelle/ha_free.cocci
new file mode 100644
index 0..00190393b
--- /dev/null
+++ b/dev/coccinelle/ha_free.cocci
@@ -0,0 +1,6 @@
+@ rule @
+expression E;
+@@
+- free(E);
+- E = NULL;
++ ha_free(&E);
-- 
2.33.1

[PATCH 2/2] CLEANUP: Apply ha_free.cocci

[Tim Duesterhus]

Use `ha_free()` where possible.
---
 src/action.c   | 3 +--
 src/server.c   | 3 +--
 src/ssl_ckch.c | 6 ++
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/action.c b/src/action.c
index ba465a253..1de97692e 100644
--- a/src/action.c
+++ b/src/action.c
@@ -39,8 +39,7 @@ int check_action_rules(struct list *rules, struct proxy *px, 
int *err_code)
err++;
}
*err_code |= warnif_tcp_http_cond(px, rule->cond);
-   free(errmsg);
-   errmsg = NULL;
+   ha_free(&errmsg);
}
 
return err;
diff --git a/src/server.c b/src/server.c
index a0206021d..a8e85a982 100644
--- a/src/server.c
+++ b/src/server.c
@@ -2380,8 +2380,7 @@ struct server *srv_drop(struct server *srv)
 
EXTRA_COUNTERS_FREE(srv->extra_counters);
 
-   free(srv);
-   srv = NULL;
+   ha_free(&srv);
 
  end:
return next;
diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c
index 2378ee349..eeb031b27 100644
--- a/src/ssl_ckch.c
+++ b/src/ssl_ckch.c
@@ -2506,8 +2506,7 @@ static int cli_parse_set_cafile(char **args, char 
*payload, struct appctx *appct
appctx->ctx.ssl.new_cafile_entry = NULL;
appctx->ctx.ssl.old_cafile_entry = NULL;
 
-   free(appctx->ctx.ssl.path);
-   appctx->ctx.ssl.path = NULL;
+   ha_free(&appctx->ctx.ssl.path);
 
HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock);
return cli_dynerr(appctx, memprintf(&err, "%sCan't update 
%s!\n", err ? err : "", args[3]));
@@ -3225,8 +3224,7 @@ static int cli_parse_set_crlfile(char **args, char 
*payload, struct appctx *appc
appctx->ctx.ssl.new_crlfile_entry = NULL;
appctx->ctx.ssl.old_crlfile_entry = NULL;
 
-   free(appctx->ctx.ssl.path);
-   appctx->ctx.ssl.path = NULL;
+   ha_free(&appctx->ctx.ssl.path);
 
HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock);
return cli_dynerr(appctx, memprintf(&err, "%sCan't update 
%s!\n", err ? err : "", args[3]));
-- 
2.33.1

[PATCH] CLEANUP: halog: Remove dead stores

[Tim Duesterhus]

Found using clang's scan-build.
---
 admin/halog/halog.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/admin/halog/halog.c b/admin/halog/halog.c
index 900cf5d46..f368c1c6f 100644
--- a/admin/halog/halog.c
+++ b/admin/halog/halog.c
@@ -551,7 +551,8 @@ int convert_date_to_timestamp(const char *field)
d = mo = y = h = m = s = 0;
e = field;
 
-   c = *(e++); // remove '['
+   e++; // remove '['
+
/* day + '/' */
while (1) {
c = *(e++) - '0';
@@ -1148,13 +1149,12 @@ int main(int argc, char **argv)
/* sort all timers */
for (f = 0; f < 5; f++) {
struct eb32_node *n;
-   int val;
 
-   val = 0;
n = eb32_first(&timers[f]);
while (n) {
int i;
double d;
+   int val;
 
t = container_of(n, struct timer, node);
last = n->key;
-- 
2.33.1

[PATCH] REGTESTS: Use `feature cmd` for 2.5+ tests (2)

[Tim Duesterhus]

This patch effectively is identical to 7ba98480cc5b2ede0fd4cca162959f66beb82c82.
---
 reg-tests/connection/cli_src_dst.vtc| 3 +--
 reg-tests/http-messaging/http_transfer_encoding.vtc | 4 ++--
 reg-tests/http-messaging/srv_ws.vtc | 5 ++---
 reg-tests/http-rules/default_rules.vtc  | 3 +--
 reg-tests/startup/default_rules.vtc | 3 +--
 reg-tests/tcp-rules/default_rules.vtc   | 3 +--
 6 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/reg-tests/connection/cli_src_dst.vtc 
b/reg-tests/connection/cli_src_dst.vtc
index cc0c94545..fa12bc805 100644
--- a/reg-tests/connection/cli_src_dst.vtc
+++ b/reg-tests/connection/cli_src_dst.vtc
@@ -1,7 +1,6 @@
 varnishtest "Test multi-level client source and destination addresses"
 
-#REQUIRE_VERSION=2.5
-
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 feature ignore_unknown_macro
 
 haproxy h1 -conf {
diff --git a/reg-tests/http-messaging/http_transfer_encoding.vtc 
b/reg-tests/http-messaging/http_transfer_encoding.vtc
index 543e965fa..258b8a9e8 100644
--- a/reg-tests/http-messaging/http_transfer_encoding.vtc
+++ b/reg-tests/http-messaging/http_transfer_encoding.vtc
@@ -1,7 +1,7 @@
 varnishtest "A test to validate Transfer-Encoding header conformance to the 
spec"
-feature ignore_unknown_macro
 
-#REQUIRE_VERSION=2.5
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
+feature ignore_unknown_macro
 
 server s1 {
 rxreq
diff --git a/reg-tests/http-messaging/srv_ws.vtc 
b/reg-tests/http-messaging/srv_ws.vtc
index bce12f6b1..32369a1a3 100644
--- a/reg-tests/http-messaging/srv_ws.vtc
+++ b/reg-tests/http-messaging/srv_ws.vtc
@@ -3,11 +3,10 @@
 
 varnishtest "h2 backend websocket management via server keyword"
 
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
+feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
 feature ignore_unknown_macro
 
-#REQUIRE_VERSION=2.5
-#REQUIRE_OPTION=OPENSSL
-
 # haproxy server
 haproxy hapsrv -conf {
defaults
diff --git a/reg-tests/http-rules/default_rules.vtc 
b/reg-tests/http-rules/default_rules.vtc
index a72776c07..3baa33a92 100644
--- a/reg-tests/http-rules/default_rules.vtc
+++ b/reg-tests/http-rules/default_rules.vtc
@@ -1,7 +1,6 @@
 varnishtest "Test declaration of HTTP rules in default sections"
 
-#REQUIRE_VERSION=2.5
-
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 feature ignore_unknown_macro
 
 server s1 {
diff --git a/reg-tests/startup/default_rules.vtc 
b/reg-tests/startup/default_rules.vtc
index 4c8051312..cd86f7414 100644
--- a/reg-tests/startup/default_rules.vtc
+++ b/reg-tests/startup/default_rules.vtc
@@ -1,7 +1,6 @@
 varnishtest "Misuses of defaults section defining TCP/HTTP rules"
 
-#REQUIRE_VERSION=2.5
-
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 feature ignore_unknown_macro
 
 #
diff --git a/reg-tests/tcp-rules/default_rules.vtc 
b/reg-tests/tcp-rules/default_rules.vtc
index 826a336cb..a2e8ce9ef 100644
--- a/reg-tests/tcp-rules/default_rules.vtc
+++ b/reg-tests/tcp-rules/default_rules.vtc
@@ -1,7 +1,6 @@
 varnishtest "Test declaration of TCP rules in default sections"
 
-#REQUIRE_VERSION=2.5
-
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 feature ignore_unknown_macro
 
 server s1 {
-- 
2.33.1

[PATCH 1/2] DEV: coccinelle: Add rule to use `istnext()` where possible

[Tim Duesterhus]

This matches both `istadv(..., 1)` as well as raw `.ptr++` uses.
---
 dev/coccinelle/ist.cocci | 16 
 1 file changed, 16 insertions(+)

diff --git a/dev/coccinelle/ist.cocci b/dev/coccinelle/ist.cocci
index c3243302f..97ce0a2ad 100644
--- a/dev/coccinelle/ist.cocci
+++ b/dev/coccinelle/ist.cocci
@@ -26,6 +26,22 @@ expression e;
 struct ist i;
 @@
 
+- i = istadv(i, 1);
++ i = istnext(i);
+
+@@
+struct ist i;
+expression e;
+@@
+
+- i.ptr++;
+- i.len--;
++ i = istnext(i);
+
+@@
+struct ist i;
+@@
+
 - i.ptr != NULL
 + isttest(i)
 
-- 
2.33.1

[PATCH 2/2] CLEANUP: Apply ist.cocci

[Tim Duesterhus]

Make use of the new rules to use `istnext()`.
---
 src/cache.c| 24 
 src/http_htx.c | 12 
 src/mqtt.c |  2 +-
 3 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index feab63f07..ba2b63c49 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -2180,49 +2180,49 @@ static int parse_encoding_value(struct ist encoding, 
unsigned int *encoding_valu
 
switch (*encoding.ptr) {
case 'a':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "aes128gcm", 
VARY_ENCODING_AES128GCM);
break;
case 'b':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "br", 
VARY_ENCODING_BR);
break;
case 'c':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "compress", 
VARY_ENCODING_COMPRESS);
break;
case 'd':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "deflate", 
VARY_ENCODING_DEFLATE);
break;
case 'e':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "exi", 
VARY_ENCODING_EXI);
break;
case 'g':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "gzip", 
VARY_ENCODING_GZIP);
break;
case 'i':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "identity", 
VARY_ENCODING_IDENTITY);
break;
case 'p':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "pack200-gzip", 
VARY_ENCODING_PACK200_GZIP);
break;
case 'x':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "x-gzip", 
VARY_ENCODING_GZIP);
if (!*encoding_value)
*encoding_value = CHECK_ENCODING(encoding, 
"x-compress", VARY_ENCODING_COMPRESS);
break;
case 'z':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "zstd", 
VARY_ENCODING_ZSTD);
break;
case '*':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = VARY_ENCODING_STAR;
break;
default:
@@ -2238,7 +2238,7 @@ static int parse_encoding_value(struct ist encoding, 
unsigned int *encoding_valu
return -1;
 
if (has_null_weight) {
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
 
encoding = http_trim_leading_spht(encoding);
 
diff --git a/src/http_htx.c b/src/http_htx.c
index bfdcaef86..8028cfc99 100644
--- a/src/http_htx.c
+++ b/src/http_htx.c
@@ -146,8 +146,7 @@ static int __http_find_header(const struct htx *htx, const 
void *pattern, struct
goto next_blk;
/* Skip comma */
if (*(v.ptr) == ',') {
-   v.ptr++;
-   v.len--;
+   v = istnext(v);
}
 
goto return_hdr;
@@ -216,8 +215,7 @@ static int __http_find_header(const struct htx *htx, const 
void *pattern, struct
ctx->lws_before = 0;
ctx->lws_after = 0;
while (v.len && HTTP_IS_LWS(*v.ptr)) {
-   v.ptr++;
-   v.len--;
+   v = istnext(v);
ctx->lws_before++;
}
if (!(flags & HTTP_FIND_FL_FULL))
@@ -457,16 +455,14 @@ int http_replace_req_query(struct htx *htx, const struct 
ist query)
uri = htx_sl_req_uri(sl);
q = uri;
while (q.len > 0 && *(q.ptr) != '?') {
-   q.ptr++;
-   q.len--;
+   q = istnext(q);
}
 
/* skip the question mark or indicate that we must insert it
 * (but only if the format string is not empty then).
 */
if (q.len) {
-   q.ptr++;
-   q.len--;
+   q = istnext(q);
}
else if (query.le

Re: [PATCH] CLEANUP: halog: Remove dead stores

[Willy Tarreau]

Hi Tim,

On Thu, Nov 04, 2021 at 09:04:24PM +0100, Tim Duesterhus wrote:
> Found using clang's scan-build.
(...)
This and your 4 other cleanup patches applied now, thank you!
Willy