Re: [PATCH] CLEANUP: Consistently `unsigned int` for bitfields
Hi Tim,
On Sat, Jan 18, 2020 at 01:32:49AM +0100, Tim Duesterhus wrote:
> Willy,
>
> I did not touch the `struct pat_time` in pattern.h which appears to be
> completely unused. Please check whether adjustments should be made there
> as well or whether it should simply be removed. Feel free to amend my
> patch if necessary.
>
> Best regards
> Tim Düsterhus
>
> Apply with `git am --scissors` to automatically cut the commit message.
>
> -- >8 --
> Subject: [PATCH] CLEANUP: Consistently `unsigned int` for bitfields
>
> - Non-ints are not allowed by the C standard.
Hmmm that's not exactly what I'm reading in C99 #6.7.2.1 here, which
explicitly permits implementation-defined types:
"A bit-field shall have a type that is a qualified or unqualified version of
_Bool, signed int, unsigned int, or some other implementation-defined type."
And the implementation we rely on (gcc) says:
https://gcc.gnu.org/onlinedocs/gcc/Structures-unions-enumerations-and-bit-fields-implementation.html
Allowable bit-field types other than _Bool, signed int, and unsigned int
(C99 and C11 6.7.2.1). Other integer types, such as long int, and
enumerated types are permitted even in strictly conforming mode.
I tend to find it extremely confusing to place "unsigned int" just after a
char, because it makes one think we're leaving a 2-byte hole before and/or
that we can use up to 32 bits without causing misalignment while in practice
we can only use the few bits left from the missing bytes. That's what we
have in fdtab:
void *owner;
unsigned char state; <-- 8-aligned
unsigned char ev; <-- 8-aligned + 1
unsigned char linger_risk:1<-- 8-aligned + 2;
unsigned char cloned:1;
unsigned char initialized:1;
With the change it remains the same except that it looks quite non-obvious
as the hole will depend on the number of bits added:
void *owner;
unsigned char state; <-- 8-aligned
unsigned char ev; <-- 8-aligned + 1
unsigned int linger_risk:1 <-- 8-aligned + 2;
unsigned int cloned:1;
unsigned int initialized:1;
Interestingly, the change above confuses pahole:
struct fdtab {
long unsigned int lock; /* 0 8 */
long unsigned int thread_mask; /* 8 8 */
long unsigned int update_mask; /*16 8 */
struct fdlist_entryupdate; /*24 8 */
void (*iocb)(int); /*32 8 */
void * owner;/*40 8 */
unsigned char state;/*48 1 */
unsigned char ev; /*49 1 */
/* Bitfield combined with previous fields */
unsigned int linger_risk:1;/*48:15 4 */
unsigned int cloned:1; /*48:14 4 */
unsigned int initialized:1;/*48:13 4 */
^^^
Here it expects to reserve 4 bytes for the field
/* size: 56, cachelines: 1, members: 11 */
/* padding: 4 */
/* bit_padding: 29 bits */
/* last cacheline: 56 bytes */
/* BRAIN FART ALERT! 56 != 56 + 0(holes), diff = 0 */
^^
And this surprizing warning (certainly a bug). Without this change, the
output looks more consistent:
unsigned char state;/*48 1 */
unsigned char ev; /*49 1 */
unsigned char linger_risk:1;/*50: 7 1 */
unsigned char cloned:1; /*50: 6 1 */
unsigned char initialized:1;/*50: 5 1 */
More annoyingly it confuses gdb:
Before:
(gdb) p [0].linger_risk
$4 = (unsigned char *) 0x32
After:
(gdb) p [0].linger_risk
$1 = (unsigned int *) 0x30
And this is not surprizing for both tools, because I think that with the
change, the unsigned int field is merged with the previous chars, which
is particularly dirty. And if we place the field before the chars, it
leaves a two-bytes hole between them.
Ideally I'd use flags there, though we'd have to place them in an unsigned
char. If you're interested in having a look at such a change I'd prefer it,
otherwise I'd prefer not to change what exists for this part because it does
exactly what we need and the change makes it wrong at least debug-wise.
> - Signed bitfields of size `1` hold the values `0` and `-1`, but are
> usually assigned `1`, possibly leading to subtle bugs when the value
> is explicitely compared against `1`.
Excellent point! The problem is even more subtle when
Re: [PATCH] BUG/MINOR: cache: Fix leak of cache name in error path
On Sat, Jan 18, 2020 at 01:46:18AM +0100, Tim Duesterhus wrote: > This issue was introduced in commit 99a17a2d91f9044ea20bba6617048488aed80555 > which first appeared in tag v1.9-dev11. This bugfix should be backported > to HAProxy 1.9+. Applied, thanks. Willy
Re: [PATCH] DOC: Fix copy and paste mistake in http-response replace-value doc
On Fri, Jan 17, 2020 at 03:53:18PM +0100, Tim Duesterhus wrote: > This fixes up commit 2252beb8557d73407b8f96eef91d6927fb855685. Applied, thanks. Willy
Re: [PATCH] BUG/MINOR: dns: Make dns_query_id_seed unsigned
On Sat, Jan 18, 2020 at 02:04:12AM +0100, Tim Duesterhus wrote: > Left shifting of large signed values and negative values is undefined. Applied, thanks. Willy
Re: customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
On Fri, Jan 17, 2020 at 03:09:53PM +0100, Tim Düsterhus wrote: > Elliot, > > Am 17.01.20 um 14:57 schrieb Elliot Otchet: > > Thanks again. Update provided: - Fixes the issue Tim points out by using > > strcmp. - Corrects the argument spacing - Updates to include my name in the > > patch submission. Yes, you can use my name. > > You missed on the commit message wrapping, but I'm sure that Willy can > do this for you. No need to resend :-) Indeed :-) Typing "gqq" 3 times isn't really extra work. Now merged. Thanks very much guys! Willy
Re: ARM(64) builds
On Fri, Jan 17, 2020 at 11:17 PM Martin Grigorov wrote: > > > On Fri, Jan 17, 2020, 23:12 William Lallemand > wrote: > >> On Fri, Jan 17, 2020 at 08:50:27PM +0200, Martin Grigorov wrote: >> > Testing with haproxy version: 2.2-dev0-70c5b0-123 >> >> This binary was built with code from 1 week ago, it's normal that the >> test does >> not work since the fix was made this week. >> > > I'm using the same steps to build HAProxy as from .travis-ci.yml > I guess I have to add "make clean" in the beginning. > I'll try it tomorrow! Thanks! > That was it! Everything is back to normal now! Thank you, William! > > >> -- >> William Lallemand >> >
[PATCH] BUG/MINOR: dns: Make dns_query_id_seed unsigned
Left shifting of large signed values and negative values is undefined. In a test script clang's ubsan rightfully complains: > runtime error: left shift of 1934242336581872173 by 13 places cannot be > represented in type 'int64_t' (aka 'long') This bug was introduced in the initial version of the DNS resolver in 325137d603aa81bd24cbd8c99d816dd42291daa7. The fix must be backported to HAProxy 1.6+. --- src/dns.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dns.c b/src/dns.c index bc68a81c0..64bb0d15f 100644 --- a/src/dns.c +++ b/src/dns.c @@ -54,7 +54,7 @@ struct list dns_resolvers = LIST_HEAD_INIT(dns_resolvers); struct list dns_srvrq_list = LIST_HEAD_INIT(dns_srvrq_list); -static THREAD_LOCAL int64_t dns_query_id_seed = 0; /* random seed */ +static THREAD_LOCAL uint64_t dns_query_id_seed = 0; /* random seed */ DECLARE_STATIC_POOL(dns_answer_item_pool, "dns_answer_item", sizeof(struct dns_answer_item)); DECLARE_STATIC_POOL(dns_resolution_pool, "dns_resolution", sizeof(struct dns_resolution)); -- 2.25.0
[PATCH] BUG/MINOR: cache: Fix leak of cache name in error path
This issue was introduced in commit 99a17a2d91f9044ea20bba6617048488aed80555 which first appeared in tag v1.9-dev11. This bugfix should be backported to HAProxy 1.9+. --- src/cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cache.c b/src/cache.c index 8e2acd1cb..dc11cf532 100644 --- a/src/cache.c +++ b/src/cache.c @@ -1452,7 +1452,7 @@ parse_cache_flt(char **args, int *cur_arg, struct proxy *px, cconf = NULL; memprintf(err, "%s: multiple explicit declarations of the cache filter '%s'", px->id, name); - return -1; + goto error; } /* Remove the implicit filter. is kept for the explicit one */ -- 2.25.0
[PATCH] CLEANUP: Consistently `unsigned int` for bitfields
Willy,
I did not touch the `struct pat_time` in pattern.h which appears to be
completely unused. Please check whether adjustments should be made there
as well or whether it should simply be removed. Feel free to amend my
patch if necessary.
Best regards
Tim Düsterhus
Apply with `git am --scissors` to automatically cut the commit message.
-- >8 --
Subject: [PATCH] CLEANUP: Consistently `unsigned int` for bitfields
- Non-ints are not allowed by the C standard.
- Signed bitfields of size `1` hold the values `0` and `-1`, but are
usually assigned `1`, possibly leading to subtle bugs when the value
is explicitely compared against `1`.
---
include/types/fd.h | 6 +++---
include/types/listener.h | 6 +++---
include/types/pattern.h | 4 ++--
include/types/ssl_sock.h | 8
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/types/fd.h b/include/types/fd.h
index dae1c90c9..61af8a523 100644
--- a/include/types/fd.h
+++ b/include/types/fd.h
@@ -129,9 +129,9 @@ struct fdtab {
void *owner; /* the connection or listener
associated with this fd, NULL if closed */
unsigned char state; /* FD state for read and write
directions (2*3 bits) */
unsigned char ev;/* event seen in return of poll()
: FD_POLL_* */
- unsigned char linger_risk:1; /* 1 if we must kill lingering
before closing */
- unsigned char cloned:1; /* 1 if a cloned socket, requires
EPOLL_CTL_DEL on close */
- unsigned char initialized:1; /* 1 if init phase was done on
this fd (e.g. set non-blocking) */
+ unsigned int linger_risk:1; /* 1 if we must kill lingering
before closing */
+ unsigned int cloned:1; /* 1 if a cloned socket, requires
EPOLL_CTL_DEL on close */
+ unsigned int initialized:1; /* 1 if init phase was done on
this fd (e.g. set non-blocking) */
}
#ifdef USE_THREAD
/* only align on cache lines when using threads; 32-bit small archs
diff --git a/include/types/listener.h b/include/types/listener.h
index 1098b42ce..bcc57ae70 100644
--- a/include/types/listener.h
+++ b/include/types/listener.h
@@ -124,9 +124,9 @@ struct ssl_bind_conf {
char *alpn_str;/* ALPN protocol string */
int alpn_len; /* ALPN protocol string length */
#endif
- int verify:3; /* verify method (set of SSL_VERIFY_* flags)
*/
- int no_ca_names:1; /* do not send ca names to clients (ca_file
related) */
- int early_data:1; /* early data allowed */
+ unsigned int verify:3; /* verify method (set of SSL_VERIFY_* flags)
*/
+ unsigned int no_ca_names:1;/* do not send ca names to clients (ca_file
related) */
+ unsigned int early_data:1; /* early data allowed */
char *ca_file; /* CAfile to use on verify */
char *crl_file;/* CRLfile to use on verify */
char *ciphers; /* cipher suite to use if non-null */
diff --git a/include/types/pattern.h b/include/types/pattern.h
index a0bb08e16..a2d609ba0 100644
--- a/include/types/pattern.h
+++ b/include/types/pattern.h
@@ -150,8 +150,8 @@ struct pattern {
int i; /* integer value */
struct {
signed long long min, max;
- int min_set :1;
- int max_set :1;
+ unsigned int min_set:1;
+ unsigned int max_set:1;
} range; /* integer range */
struct {
struct in_addr addr;
diff --git a/include/types/ssl_sock.h b/include/types/ssl_sock.h
index ccc923508..172be6772 100644
--- a/include/types/ssl_sock.h
+++ b/include/types/ssl_sock.h
@@ -36,8 +36,8 @@ struct pkey_info {
struct sni_ctx {
SSL_CTX *ctx; /* context associated to the certificate */
int order;/* load order for the certificate */
- uint8_t neg:1; /* reject if match */
- uint8_t wild:1;/* wildcard sni */
+ unsigned int neg:1; /* reject if match */
+ unsigned int wild:1;/* wildcard sni */
struct pkey_info kinfo; /* pkey info */
struct ssl_bind_conf *conf; /* ssl "bind" conf for the certificate */
struct list by_ckch_inst; /* chained in ckch_inst's list of sni_ctx */
@@ -109,8 +109,8 @@ struct cert_key_and_chain {
*/
struct ckch_store {
struct cert_key_and_chain *ckch;
- int multi:1; /* is it a multi-cert bundle ? */
- int filters:1; /* one of the instances is using filters, TODO:remove
this flag once filters are supported */
+ unsigned int multi:1; /* is it a multi-cert bundle ? */
+ unsigned int filters:1; /* one of the instances is using filters,
TODO:remove this flag once filters are supported */
Re: ARM(64) builds
On Fri, Jan 17, 2020, 23:12 William Lallemand wrote: > On Fri, Jan 17, 2020 at 08:50:27PM +0200, Martin Grigorov wrote: > > Testing with haproxy version: 2.2-dev0-70c5b0-123 > > This binary was built with code from 1 week ago, it's normal that the test > does > not work since the fix was made this week. > I'm using the same steps to build HAProxy as from .travis-ci.yml I guess I have to add "make clean" in the beginning. I'll try it tomorrow! Thanks! > -- > William Lallemand >
Re: ARM(64) builds
On Fri, Jan 17, 2020 at 08:50:27PM +0200, Martin Grigorov wrote: > Testing with haproxy version: 2.2-dev0-70c5b0-123 This binary was built with code from 1 week ago, it's normal that the test does not work since the fix was made this week. -- William Lallemand
Re: ARM(64) builds
On Fri, Jan 17, 2020 at 08:50:27PM +0200, Martin Grigorov wrote: > Hi William, > > Please find attached the output. > > this two lines look bad in it: > > *** h1 debug|[ALERT] 016/184150 (7188) : parsing [cur--1:0] : proxy > 'MASTER', another server named 'cur--1' was already defined at line 0, > please use distinct names. > *** h1 debug|[ALERT] 016/184150 (7188) : Fatal errors found in > configuration. > Are you sure your binary is up to date? because the fix for this exact problem is the commit just before the one that created the regtest: https://github.com/haproxy/haproxy/commit/a31b09e982a76cdf8761edb25d1569cb76a8ff37 -- William Lallemand
Re: ARM(64) builds
Hi Илья, On Fri, Jan 17, 2020 at 5:43 PM Илья Шипицин wrote: > > > пт, 17 янв. 2020 г. в 19:33, Martin Grigorov : > >> >> >> On Fri, Jan 17, 2020 at 4:13 PM Martin Grigorov < >> martin.grigo...@gmail.com> wrote: >> >>> Hi all, >>> >>> Today's build consistently fails on my ARM64 VM: >>> >>> ## Starting vtest ## >>> Testing with haproxy version: 2.2-dev0-70c5b0-123 >>> #top TEST reg-tests/mcli/mcli_start_progs.vtc FAILED (3.004) exit=2 >>> 1 tests failed, 0 tests skipped, 34 tests passed >>> ## Gathering results ## >>> ## Test case: reg-tests/mcli/mcli_start_progs.vtc ## >>> ## test results in: >>> "/tmp/haregtests-2020-01-17_14-01-45.SGkYcJ/vtc.12807.6adfff44" >>> h1 haproxy h1 PID file check failed: >>> h1 Bad exit status: 0x0100 exit 0x1 signal 0 core 0 >>> Makefile:964: recipe for target 'reg-tests' failed >>> make: *** [reg-tests] Error 1 >>> >> >> git bisect blaims this commit: >> >> 25b569302167e71b32e569a2366027e8e320e80a is the first bad commit >> commit 25b569302167e71b32e569a2366027e8e320e80a >> Author: William Lallemand >> Date: Tue Jan 14 15:38:43 2020 +0100 >> >> REGTEST: mcli/mcli_start_progs: start 2 programs >> >> This regtest tests the issue #446 by starting 2 programs and checking >> if >> they exist in the "show proc" of the master CLI. >> >> Should be backported as far as 2.0. >> >> >> https://travis-ci.com/haproxy/haproxy is green >> https://cirrus-ci.com/github/haproxy/haproxy is green >> and what is even more interesting is that >> https://travis-ci.org/martin-g/haproxy/builds (my fork with enabled >> ARM64 testing on TravisCI) also just passed (after few failures due to >> timing issues (I guess) >> > > > timing out might happen because of > https://github.com/haproxy/haproxy/commit/ac8147446c7a3d1aa607042bc782095b03bc8dc4 > your fork is 16 commits behind current master. try to rebase to master > Thanks! I've updated HAProxy on my ARM64 VM but didn't update my fork at GitHub so it was behind. I've just did it and the build again passed successfully: https://travis-ci.org/martin-g/haproxy/builds/638568217 > > >> >> >>> Regards, >>> Martin >>> >>> On Fri, Jan 17, 2020 at 9:22 AM Илья Шипицин >>> wrote: >>> привет! пт, 17 янв. 2020 г. в 11:42, Martin Grigorov >>> >: > Привет Илья, > > On Thu, Jan 16, 2020 at 10:37 AM Илья Шипицин > wrote: > >> >> >> чт, 16 янв. 2020 г. в 13:26, Martin Grigorov < >> martin.grigo...@gmail.com>: >> >>> Hi Илья, >>> >>> On Thu, Jan 16, 2020 at 10:19 AM Илья Шипицин >>> wrote: >>> Hello, Martin! btw, just curious, how is Apache Foundation (or you personally) interested in all that ? please do not blame me, I really like to know. >>> >> ok, so you work in some company that is interested in haproxy on >> ARM64. >> you do not want to tell it's name, at least is it legal ? is it >> related to some government ? >> if "no" and "no", I guess most people won't ask any more questions :) >> > > It is legal and I do not work for a government of any country! > > >> >> personally, I do not work at Haproxy Inc, I use haproxy, sometimes I >> contribute to it. >> Please do not consider me as an "official representative". >> >> >> I'm interested in testing haproxy on ARM64, I planned to do so. I can >> priorierize it according to your interest to it. >> and yes, I can accept hardware donation (even considering I'm not >> part of Haproxy Inc). >> >> also, from my point of view, what would be really useful in your case >> is testing not just official reg-tests, but also >> your configs. reg-tests cover only partially. If you enable clang >> asan in your own workload there's a chance to catch >> something interesting (or, to make sure your own workload is ok). I >> can help with that as well. >> > > Thanks for the offer! > I've discussed it with my managers. > Our offer is to donate a VM that could be used as an official CI agent > for the HAProxy project long term. > I'd split this into short term and long term approach. if you need to start any time soon, I'd focus on your own workload testing first. I would build stand which emulates your production workload, compile haproxy using clang address sanitizer and give it a try (functional testing, load testing, ...) I can help with that. As for long term solution, currently haproxy simply cannot attach any dedicated build agent to their CI. travis-ci does not allow attaching dedicated agents. And haproxy team is very conservative in adding new CI servers. I think, I will add arm64 (most probably openssl-1.1.1 only for now)
Re: ARM(64) builds
Hi William,
On Fri, Jan 17, 2020 at 4:46 PM William Lallemand
wrote:
> On Fri, Jan 17, 2020 at 04:33:22PM +0200, Martin Grigorov wrote:
> >
> > git bisect blaims this commit:
> >
> > 25b569302167e71b32e569a2366027e8e320e80a is the first bad commit
> > commit 25b569302167e71b32e569a2366027e8e320e80a
> > Author: William Lallemand
> > Date: Tue Jan 14 15:38:43 2020 +0100
> >
> > REGTEST: mcli/mcli_start_progs: start 2 programs
>
> Well that's the commit which introduces the vtc file so that's normal.
>
> >
> > https://travis-ci.com/haproxy/haproxy is green
> > https://cirrus-ci.com/github/haproxy/haproxy is green
> > and what is even more interesting is that
> > https://travis-ci.org/martin-g/haproxy/builds (my fork with enabled
> ARM64
> > testing on TravisCI) also just passed (after few failures due to timing
> > issues (I guess)
>
> I don't see anything regarding mcli_start_progs.vtc in this links, can you
> provide the output of
> `make reg-tests -- --debug reg-tests/mcli/mcli_start_progs.vtc` ?
>
Please find attached the output.
this two lines look bad in it:
*** h1 debug|[ALERT] 016/184150 (7188) : parsing [cur--1:0] : proxy
'MASTER', another server named 'cur--1' was already defined at line 0,
please use distinct names.
*** h1 debug|[ALERT] 016/184150 (7188) : Fatal errors found in
configuration.
$ grep -rnH 'cur'
/tmp/haregtests-2020-01-17_18-41-50.A6zVLb/vtc.7182.700b0bcd/
/tmp/haregtests-2020-01-17_18-41-50.A6zVLb/vtc.7182.700b0bcd/LOG:71:*** h1
debug|[ALERT] 016/184150 (7188) : parsing [cur--1:0] : proxy 'MASTER',
another server named 'cur--1' was already defined at line 0, please use
distinct names.
│ File:
/tmp/haregtests-2020-01-17_18-41-50.A6zVLb/vtc.7182.700b0bcd/h1/cfg
───┼─
1 │ global
2 │ stats socket
"/tmp/haregtests-2020-01-17_18-41-50.A6zVLb/vtc.7182.700b0bcd/h1/stats.sock"
level admin mode 600
3 │ stats socket "fd@${cli}" level admin
4 │
5 │ global
6 │ nbproc 1
7 │ defaults
8 │ mode http
9 │ option http-use-htx
10 │ timeout connect 1s
11 │ timeout client 1s
12 │ timeout server 1s
13 │
14 │ frontend myfrontend
15 │ bind "fd@${my_fe}"
16 │ default_backend test
17 │
18 │ backend test
19 │ server www1 127.0.0.1:39247
20 │
21 │ program foo
22 │ command sleep 10
23 │
24 │ program bar
25 │ command sleep 10
Please let me know if you need more information!
Thanks
>
> --
> William Lallemand
>
env VTEST_PROGRAM=../vtest/vtest make reg-tests -- --debug
reg-tests/mcli/mcli_start_progs.vtc
## Preparing to run tests ##
Testing with haproxy version: 2.2-dev0-70c5b0-123
Target : linux-glibc
Options : +EPOLL -KQUEUE -MY_EPOLL -MY_SPLICE +NETFILTER -PCRE -PCRE_JIT -PCRE2
-PCRE2_JIT +POLL -PRIVATE_CACHE +THREAD -PTHREAD_PSHARED -REGPARM -STATIC_PCRE
-STATIC_PCRE2 +TPROXY +LINUX_TPROXY +LINUX_SPLICE +LIBCRYPT +CRYPT_H -VSYSCALL
+GETADDRINFO -OPENSSL -LUA +FUTEX +ACCEPT4 -MY_ACCEPT4 +ZLIB -SLZ +CPU_AFFINITY
+TFO +NS +DL +RT -DEVICEATLAS -51DEGREES -WURFL -SYSTEMD -OBSOLETE_LINKER
+PRCTL +THREAD_DUMP -EVPORTS
## Gathering tests to run ##
Add test: reg-tests/mcli/mcli_start_progs.vtc
## Starting vtest ##
Testing with haproxy version: 2.2-dev0-70c5b0-123
dT 0.000
*top TEST reg-tests/mcli/mcli_start_progs.vtc starting
top extmacro def pwd=/home/ubuntu/git/haproxy/haproxy
top extmacro def no-htx=
top extmacro def localhost=127.0.0.1
top extmacro def bad_backend=127.0.0.1 45249
top extmacro def bad_ip=192.0.2.255
top macro def testdir=/home/ubuntu/git/haproxy/haproxy/reg-tests/mcli
top macro def
tmpdir=/tmp/haregtests-2020-01-17_18-41-50.A6zVLb/vtc.7182.700b0bcd
** top === varnishtest "Try to start a master CLI with 2 programs"
*top VTEST Try to start a master CLI with 2 programs
** top === feature ignore_unknown_macro
** top === server s1 {
** s1 Starting server
s1 macro def s1_addr=127.0.0.1
s1 macro def s1_port=39247
s1 macro def s1_sock=127.0.0.1 39247
*s1 Listen on 127.0.0.1 39247
** top === haproxy h1 -W -S -conf {
** s1 Started on 127.0.0.1 39247 (1 iterations)
h1 macro def h1_closed_sock=127.0.0.1 33279
h1 macro def h1_closed_addr=127.0.0.1
h1 macro def h1_closed_port=33279
dT 0.003
h1 macro def h1_cli_sock=127.0.0.1 45381
h1 macro def h1_cli_addr=127.0.0.1
h1 macro def h1_cli_port=45381
h1 setenv(cli, 6)
h1 macro def
Re: ARM(64) builds
пт, 17 янв. 2020 г. в 19:33, Martin Grigorov : > > > On Fri, Jan 17, 2020 at 4:13 PM Martin Grigorov > wrote: > >> Hi all, >> >> Today's build consistently fails on my ARM64 VM: >> >> ## Starting vtest ## >> Testing with haproxy version: 2.2-dev0-70c5b0-123 >> #top TEST reg-tests/mcli/mcli_start_progs.vtc FAILED (3.004) exit=2 >> 1 tests failed, 0 tests skipped, 34 tests passed >> ## Gathering results ## >> ## Test case: reg-tests/mcli/mcli_start_progs.vtc ## >> ## test results in: >> "/tmp/haregtests-2020-01-17_14-01-45.SGkYcJ/vtc.12807.6adfff44" >> h1 haproxy h1 PID file check failed: >> h1 Bad exit status: 0x0100 exit 0x1 signal 0 core 0 >> Makefile:964: recipe for target 'reg-tests' failed >> make: *** [reg-tests] Error 1 >> > > git bisect blaims this commit: > > 25b569302167e71b32e569a2366027e8e320e80a is the first bad commit > commit 25b569302167e71b32e569a2366027e8e320e80a > Author: William Lallemand > Date: Tue Jan 14 15:38:43 2020 +0100 > > REGTEST: mcli/mcli_start_progs: start 2 programs > > This regtest tests the issue #446 by starting 2 programs and checking > if > they exist in the "show proc" of the master CLI. > > Should be backported as far as 2.0. > > > https://travis-ci.com/haproxy/haproxy is green > https://cirrus-ci.com/github/haproxy/haproxy is green > and what is even more interesting is that > https://travis-ci.org/martin-g/haproxy/builds (my fork with enabled ARM64 > testing on TravisCI) also just passed (after few failures due to timing > issues (I guess) > timing out might happen because of https://github.com/haproxy/haproxy/commit/ac8147446c7a3d1aa607042bc782095b03bc8dc4 your fork is 16 commits behind current master. try to rebase to master > > >> Regards, >> Martin >> >> On Fri, Jan 17, 2020 at 9:22 AM Илья Шипицин >> wrote: >> >>> привет! >>> >>> пт, 17 янв. 2020 г. в 11:42, Martin Grigorov >> >: >>> Привет Илья, On Thu, Jan 16, 2020 at 10:37 AM Илья Шипицин wrote: > > > чт, 16 янв. 2020 г. в 13:26, Martin Grigorov < > martin.grigo...@gmail.com>: > >> Hi Илья, >> >> On Thu, Jan 16, 2020 at 10:19 AM Илья Шипицин >> wrote: >> >>> >>> Hello, Martin! >>> >>> btw, just curious, how is Apache Foundation (or you personally) >>> interested in all that ? >>> please do not blame me, I really like to know. >>> >> > ok, so you work in some company that is interested in haproxy on > ARM64. > you do not want to tell it's name, at least is it legal ? is it > related to some government ? > if "no" and "no", I guess most people won't ask any more questions :) > It is legal and I do not work for a government of any country! > > personally, I do not work at Haproxy Inc, I use haproxy, sometimes I > contribute to it. > Please do not consider me as an "official representative". > > > I'm interested in testing haproxy on ARM64, I planned to do so. I can > priorierize it according to your interest to it. > and yes, I can accept hardware donation (even considering I'm not part > of Haproxy Inc). > > also, from my point of view, what would be really useful in your case > is testing not just official reg-tests, but also > your configs. reg-tests cover only partially. If you enable clang asan > in your own workload there's a chance to catch > something interesting (or, to make sure your own workload is ok). I > can help with that as well. > Thanks for the offer! I've discussed it with my managers. Our offer is to donate a VM that could be used as an official CI agent for the HAProxy project long term. >>> >>> I'd split this into short term and long term approach. >>> >>> if you need to start any time soon, I'd focus on your own workload >>> testing first. I would build stand which emulates >>> your production workload, compile haproxy using clang address sanitizer >>> and give it a try (functional testing, load testing, ...) >>> >>> I can help with that. >>> >>> As for long term solution, currently haproxy simply cannot attach any >>> dedicated build agent to their CI. travis-ci does not allow >>> attaching dedicated agents. And haproxy team is very conservative in >>> adding new CI servers. >>> >>> I think, I will add arm64 (most probably openssl-1.1.1 only for now) >>> soon. Also, I'm going to investigate your libressl failures. >>> >>> so, dedicated vm definetly will help in troubleshooting issues, for >>> manual builds. It would save bunch of time. I do not mind if you will >>> add my ssh to that vm. >>> >>> >>> also, I requested access to Linaro. >>> >>> You can use Linaro for short term testing though. https://www.linaro.cloud/ Here you can request free VM for short periods:
[PATCH] DOC: Fix copy and paste mistake in http-response replace-value doc
This fixes up commit 2252beb8557d73407b8f96eef91d6927fb855685.
---
doc/configuration.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 9ac898517..6f59fac3c 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -5056,7 +5056,7 @@ http-response replace-header
http-response replace-value
[ { if | unless } ]
- This works like "http-response replace-value" except that it works on the
+ This works like "http-request replace-value" except that it works on the
server's response instead of the client's request.
Example:
--
2.25.0
Re: ARM(64) builds
On Fri, Jan 17, 2020 at 04:33:22PM +0200, Martin Grigorov wrote: > > git bisect blaims this commit: > > 25b569302167e71b32e569a2366027e8e320e80a is the first bad commit > commit 25b569302167e71b32e569a2366027e8e320e80a > Author: William Lallemand > Date: Tue Jan 14 15:38:43 2020 +0100 > > REGTEST: mcli/mcli_start_progs: start 2 programs Well that's the commit which introduces the vtc file so that's normal. > > https://travis-ci.com/haproxy/haproxy is green > https://cirrus-ci.com/github/haproxy/haproxy is green > and what is even more interesting is that > https://travis-ci.org/martin-g/haproxy/builds (my fork with enabled ARM64 > testing on TravisCI) also just passed (after few failures due to timing > issues (I guess) I don't see anything regarding mcli_start_progs.vtc in this links, can you provide the output of `make reg-tests -- --debug reg-tests/mcli/mcli_start_progs.vtc` ? Thanks -- William Lallemand
Re: ARM(64) builds
On Fri, Jan 17, 2020 at 4:13 PM Martin Grigorov wrote: > Hi all, > > Today's build consistently fails on my ARM64 VM: > > ## Starting vtest ## > Testing with haproxy version: 2.2-dev0-70c5b0-123 > #top TEST reg-tests/mcli/mcli_start_progs.vtc FAILED (3.004) exit=2 > 1 tests failed, 0 tests skipped, 34 tests passed > ## Gathering results ## > ## Test case: reg-tests/mcli/mcli_start_progs.vtc ## > ## test results in: > "/tmp/haregtests-2020-01-17_14-01-45.SGkYcJ/vtc.12807.6adfff44" > h1 haproxy h1 PID file check failed: > h1 Bad exit status: 0x0100 exit 0x1 signal 0 core 0 > Makefile:964: recipe for target 'reg-tests' failed > make: *** [reg-tests] Error 1 > git bisect blaims this commit: 25b569302167e71b32e569a2366027e8e320e80a is the first bad commit commit 25b569302167e71b32e569a2366027e8e320e80a Author: William Lallemand Date: Tue Jan 14 15:38:43 2020 +0100 REGTEST: mcli/mcli_start_progs: start 2 programs This regtest tests the issue #446 by starting 2 programs and checking if they exist in the "show proc" of the master CLI. Should be backported as far as 2.0. https://travis-ci.com/haproxy/haproxy is green https://cirrus-ci.com/github/haproxy/haproxy is green and what is even more interesting is that https://travis-ci.org/martin-g/haproxy/builds (my fork with enabled ARM64 testing on TravisCI) also just passed (after few failures due to timing issues (I guess) > Regards, > Martin > > On Fri, Jan 17, 2020 at 9:22 AM Илья Шипицин wrote: > >> привет! >> >> пт, 17 янв. 2020 г. в 11:42, Martin Grigorov : >> >>> Привет Илья, >>> >>> On Thu, Jan 16, 2020 at 10:37 AM Илья Шипицин >>> wrote: >>> чт, 16 янв. 2020 г. в 13:26, Martin Grigorov >>> >: > Hi Илья, > > On Thu, Jan 16, 2020 at 10:19 AM Илья Шипицин > wrote: > >> >> Hello, Martin! >> >> btw, just curious, how is Apache Foundation (or you personally) >> interested in all that ? >> please do not blame me, I really like to know. >> > ok, so you work in some company that is interested in haproxy on ARM64. you do not want to tell it's name, at least is it legal ? is it related to some government ? if "no" and "no", I guess most people won't ask any more questions :) >>> >>> It is legal and I do not work for a government of any country! >>> >>> personally, I do not work at Haproxy Inc, I use haproxy, sometimes I contribute to it. Please do not consider me as an "official representative". I'm interested in testing haproxy on ARM64, I planned to do so. I can priorierize it according to your interest to it. and yes, I can accept hardware donation (even considering I'm not part of Haproxy Inc). also, from my point of view, what would be really useful in your case is testing not just official reg-tests, but also your configs. reg-tests cover only partially. If you enable clang asan in your own workload there's a chance to catch something interesting (or, to make sure your own workload is ok). I can help with that as well. >>> >>> Thanks for the offer! >>> I've discussed it with my managers. >>> Our offer is to donate a VM that could be used as an official CI agent >>> for the HAProxy project long term. >>> >> >> I'd split this into short term and long term approach. >> >> if you need to start any time soon, I'd focus on your own workload >> testing first. I would build stand which emulates >> your production workload, compile haproxy using clang address sanitizer >> and give it a try (functional testing, load testing, ...) >> >> I can help with that. >> >> As for long term solution, currently haproxy simply cannot attach any >> dedicated build agent to their CI. travis-ci does not allow >> attaching dedicated agents. And haproxy team is very conservative in >> adding new CI servers. >> >> I think, I will add arm64 (most probably openssl-1.1.1 only for now) >> soon. Also, I'm going to investigate your libressl failures. >> >> so, dedicated vm definetly will help in troubleshooting issues, for >> manual builds. It would save bunch of time. I do not mind if you will >> add my ssh to that vm. >> >> >> also, I requested access to Linaro. >> >> >>> >>> You can use Linaro for short term testing though. >>> https://www.linaro.cloud/ >>> Here you can request free VM for short periods: >>> https://servicedesk.linaro.org/servicedesk/customer/portal/19/create/265 >>> P.S. Linaro is not my employer! >>> >>> >>> Regards, >>> Martin >>> >>> > >> > @apache.org is just one of my several emails. And it is the default > one in my email client. > ASF is not related anyhow to my participation here. > If I used my work email then it might look like some kind of > advertisement. I'd like to avoid that! > Next time I
Re: ARM(64) builds
Hi all, Today's build consistently fails on my ARM64 VM: ## Starting vtest ## Testing with haproxy version: 2.2-dev0-70c5b0-123 #top TEST reg-tests/mcli/mcli_start_progs.vtc FAILED (3.004) exit=2 1 tests failed, 0 tests skipped, 34 tests passed ## Gathering results ## ## Test case: reg-tests/mcli/mcli_start_progs.vtc ## ## test results in: "/tmp/haregtests-2020-01-17_14-01-45.SGkYcJ/vtc.12807.6adfff44" h1 haproxy h1 PID file check failed: h1 Bad exit status: 0x0100 exit 0x1 signal 0 core 0 Makefile:964: recipe for target 'reg-tests' failed make: *** [reg-tests] Error 1 Regards, Martin On Fri, Jan 17, 2020 at 9:22 AM Илья Шипицин wrote: > привет! > > пт, 17 янв. 2020 г. в 11:42, Martin Grigorov : > >> Привет Илья, >> >> On Thu, Jan 16, 2020 at 10:37 AM Илья Шипицин >> wrote: >> >>> >>> >>> чт, 16 янв. 2020 г. в 13:26, Martin Grigorov >> >: >>> Hi Илья, On Thu, Jan 16, 2020 at 10:19 AM Илья Шипицин wrote: > > Hello, Martin! > > btw, just curious, how is Apache Foundation (or you personally) > interested in all that ? > please do not blame me, I really like to know. > >>> ok, so you work in some company that is interested in haproxy on ARM64. >>> you do not want to tell it's name, at least is it legal ? is it related >>> to some government ? >>> if "no" and "no", I guess most people won't ask any more questions :) >>> >> >> It is legal and I do not work for a government of any country! >> >> >>> >>> personally, I do not work at Haproxy Inc, I use haproxy, sometimes I >>> contribute to it. >>> Please do not consider me as an "official representative". >>> >>> >>> I'm interested in testing haproxy on ARM64, I planned to do so. I can >>> priorierize it according to your interest to it. >>> and yes, I can accept hardware donation (even considering I'm not part >>> of Haproxy Inc). >>> >>> also, from my point of view, what would be really useful in your case is >>> testing not just official reg-tests, but also >>> your configs. reg-tests cover only partially. If you enable clang asan >>> in your own workload there's a chance to catch >>> something interesting (or, to make sure your own workload is ok). I can >>> help with that as well. >>> >> >> Thanks for the offer! >> I've discussed it with my managers. >> Our offer is to donate a VM that could be used as an official CI agent >> for the HAProxy project long term. >> > > I'd split this into short term and long term approach. > > if you need to start any time soon, I'd focus on your own workload testing > first. I would build stand which emulates > your production workload, compile haproxy using clang address sanitizer > and give it a try (functional testing, load testing, ...) > > I can help with that. > > As for long term solution, currently haproxy simply cannot attach any > dedicated build agent to their CI. travis-ci does not allow > attaching dedicated agents. And haproxy team is very conservative in > adding new CI servers. > > I think, I will add arm64 (most probably openssl-1.1.1 only for now) soon. > Also, I'm going to investigate your libressl failures. > > so, dedicated vm definetly will help in troubleshooting issues, for manual > builds. It would save bunch of time. I do not mind if you will > add my ssh to that vm. > > > also, I requested access to Linaro. > > >> >> You can use Linaro for short term testing though. >> https://www.linaro.cloud/ >> Here you can request free VM for short periods: >> https://servicedesk.linaro.org/servicedesk/customer/portal/19/create/265 >> P.S. Linaro is not my employer! >> >> >> Regards, >> Martin >> >> >>> > @apache.org is just one of my several emails. And it is the default one in my email client. ASF is not related anyhow to my participation here. If I used my work email then it might look like some kind of advertisement. I'd like to avoid that! Next time I will use my @gmail.com one, as more neutral. Actually I've used the GMail one when registering to this mailing list, so probably the post from @apache has been moderated. I'll be more careful next time! Thanks, Илья! Regards, Martin > > чт, 16 янв. 2020 г. в 12:32, Martin Grigorov : > >> Hello HAProxy developers, >> >> At work we are going to use more and more ARM64 based servers and >> HAProxy is one of the main products we rely on. >> So I went checking whether HAProxy project has a CI environment for >> testing on ARM architecture. >> > > we are looking towards > https://docs.travis-ci.com/user/multi-cpu-architectures > > >> I've found this recent discussion: >> https://www.mail-archive.com/haproxy@formilux.org/msg35302.html (I >> didn't find a way how to continue on the same mail thread, so I'm >> starting
Re: customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
Elliot, Am 17.01.20 um 14:57 schrieb Elliot Otchet: > Thanks again. Update provided: - Fixes the issue Tim points out by using > strcmp. - Corrects the argument spacing - Updates to include my name in the > patch submission. Yes, you can use my name. You missed on the commit message wrapping, but I'm sure that Willy can do this for you. No need to resend :-) It's good for me based off reading the patch and my understanding of HAProxy's code. Reviewed-by: Tim Duesterhus Best regards Tim Düsterhus
Re: customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
Tim, Willy, Thanks again. Update provided: - Fixes the issue Tim points out by using strcmp. - Corrects the argument spacing - Updates to include my name in the patch submission. Yes, you can use my name. Thanks! -Elliot On Friday, January 17, 2020, 5:58:43 AM EST, Willy Tarreau wrote: On Fri, Jan 17, 2020 at 11:45:17AM +0100, Tim Düsterhus wrote: > I did not test, but if I am not mistaken this would allow "rfc2253XXX" > as the format parameter, no? Meaning: It allows one to append arbitrary > garbage to the rfc2253 value. > Judging from `smp_check_const_bool` in sample.c a simple strcmp should > be safe to use or you can just use the chunk_strcmp you already use in > `ssl_sock_get_dn_formatted`, I guess. Does Willy have any strong opinion > about this? I noticed it as well but didn't have enough time to assign to this to look for alternatives, and considered that it was not critical. But if other places already work with an strcmp() that's definitely better indeed. Similarly, we usually prefer to avoid running strcmp() at runtime, so sometimes what's done is that the argument keyword, once parsed, is replaced by an integer that's cheaper to test. I didn't jump on the code to find one example, but I wouldn't be surprized that the json converter or anything else taking a small set of words already does something like this. But here we're dealing with TLS stuff and usually the call rate is very low so I considered that it was not important enough to require another round trip. In general, especially for first submissions, my acceptance level is lower than what some submitters are willing to do, and I prefer not to harrass them and take the patch once it's good enough. But if some want to provide top-most quality patches, they're absolutely welcome to do so! Thanks! Willy From 460a76a48a5885fd0c0c2586d8c03c4329b2b163 Mon Sep 17 00:00:00 2001 From: Elliot Otchet Date: Wed, 15 Jan 2020 08:12:14 -0500 Subject: [PATCH] MINOR: ssl: Add support for returning the dn samples from ssl_(c|f)_(i|s)_dn in LDAP v3 (RFC2253) format. Modifies the existing sample extraction methods (smp_fetch_ssl_x_i_dn, smp_fetch_ssl_x_s_dn) to accommodate a third argument that indicates the DN should be returned in LDAP v3 format. When the third argument is present, the new function (ssl_sock_get_dn_formatted) is called with three parameters including the X509_NAME, a buffer containing the format argument, and a buffer for the output. If the supplied format matches the supported format string (currently only "rfc2253" is supported), the formatted value is extracted into the supplied output buffer using OpenSSL's X509_NAME_print_ex and BIO_s_mem. 1 is returned when a dn value is retrieved. 0 is returned when a value is not retrieved. Argument validation is added to each of the related sample configurations to ensure the third argument passed is either blank or "rfc2253" using strcmp. An error is returned if the third argument is present with any other value. Documentation was updated in configuration.txt and it was noted during preliminary reviews that a CLEANUP patch should follow that adjusts the documentation. Currently, this patch and the existing documentation are copied with some minor revisions for each sample configuration. It might be better to have one entry for all of the samples or entries for each that reference back to a primary entry that explains the sample in detail. Special thanks to Chris, Willy, Tim and Aleks for the feedback. Author:Elliot Otchet --- doc/configuration.txt | 28 ++--- src/ssl_sock.c| 68 ++- 2 files changed, 86 insertions(+), 10 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index 9ac8985..1d9b3d1 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -15246,7 +15246,7 @@ ssl_c_err : integer to your SSL library's documentation to find the exhaustive list of error codes. -ssl_c_i_dn([[,]]) : string +ssl_c_i_dn([[,[,]]]) : string When the incoming connection was made over an SSL/TLS transport layer, returns the full distinguished name of the issuer of the certificate presented by the client when no is specified, or the value of the @@ -15255,6 +15255,11 @@ ssl_c_i_dn([[,]]) : string the value of the nth given entry value from the beginning/end of the DN. For instance, "ssl_c_i_dn(OU,2)" the second organization unit, and "ssl_c_i_dn(CN)" retrieves the common name. + The parameter allows you to receive the DN suitable for + consumption by different protocols. Currently supported is rfc2253 for + LDAP v3. + If you'd like to modify the format only you can specify an empty string + and zero for the first two parameters. Example: ssl_c_i_dn(,0,rfc2253) ssl_c_key_alg : string Returns the name of the algorithm used to generate the key of the certificate @@ -15271,7 +15276,7 @@ ssl_c_notbefore : string YYMMDDhhmmss[Z] when
Re: [PATCH v2 0/3] MINOR: lua: Add lua-prepend-path configuration option
Thierry, Am 14.01.20 um 19:37 schrieb Tim Düsterhus: > Willy, > > Am 14.01.20 um 14:53 schrieb Willy Tarreau: >> On Tue, Jan 14, 2020 at 02:04:04PM +0100, Thierry Fournier wrote: >>> Idea 1: >>> >>>lua-prepend-path path /etc/haproxy/lua-modules/?.lua >>>lua-prepend-path cpath /etc/haproxy/lua-cmodules/?.lua >>> >>> Idea 2: >>> >>>lua-prepend-path /etc/haproxy/lua-modules/?.lua >>>lua-prepend-path cpath /etc/haproxy/lua-cmodules/?.lua >>> >>> Idea 3: >>> >>>lua-prepend-path /etc/haproxy/lua-modules/?.lua >>>lua-prepend-cpath /etc/haproxy/lua-cmodules/?.lua >> >> I guess the third one is better, at least for a reason which is that >> it causes less confusion when asking a bug reporter "what's in your >> lua-prepend-path ?". We've seen sufficient confusion from the maxconn >> word being used for different tihngs depending on where it's set, so >> we'd rather keep this clear. > > Let me give my reasoning for my choice: > > - I wanted to avoid two completely distinct configuration options for > what is essentially the same thing. It would require adding both to the > documentation. > - I made the type optional, because I expect the majority of modules > loaded via this option to be pure Lua modules. The path is meant to be > the home of HAProxy specific modules. I consider it unlikely for an user > to develop a C based Lua module that is HAProxy specific when they can > simply use a regular C based HAProxy module without corkscrewing it > through Lua. Stuff such as cjson would be put into the system wide Lua > path and thus be available to every Lua script including HAProxy, > because it sits in the default path that is compiled into the Lua VM. > - I put the type last, because I consider optional parameters that are > in the middle to be unintuitive and it complicates parsing, because a > single index might either be the type or the path if the type is not given. > > However I don't particularly care for any of this. If you feel like we > should to lua-prepend-path and lua-prepend-cpath instead then I'm happy > to adjust the patch (or you do it). > > Best regards > Tim Düsterhus > Are you happy with my patch series as is or would you like to see changes? Should I update the config syntax? Best regards Tim Düsterhus
Re: customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
On Fri, Jan 17, 2020 at 11:45:17AM +0100, Tim Düsterhus wrote: > I did not test, but if I am not mistaken this would allow "rfc2253XXX" > as the format parameter, no? Meaning: It allows one to append arbitrary > garbage to the rfc2253 value. > Judging from `smp_check_const_bool` in sample.c a simple strcmp should > be safe to use or you can just use the chunk_strcmp you already use in > `ssl_sock_get_dn_formatted`, I guess. Does Willy have any strong opinion > about this? I noticed it as well but didn't have enough time to assign to this to look for alternatives, and considered that it was not critical. But if other places already work with an strcmp() that's definitely better indeed. Similarly, we usually prefer to avoid running strcmp() at runtime, so sometimes what's done is that the argument keyword, once parsed, is replaced by an integer that's cheaper to test. I didn't jump on the code to find one example, but I wouldn't be surprized that the json converter or anything else taking a small set of words already does something like this. But here we're dealing with TLS stuff and usually the call rate is very low so I considered that it was not important enough to require another round trip. In general, especially for first submissions, my acceptance level is lower than what some submitters are willing to do, and I prefer not to harrass them and take the patch once it's good enough. But if some want to provide top-most quality patches, they're absolutely welcome to do so! Thanks! Willy
Re: customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
Elliot,
Am 17.01.20 um 05:35 schrieb Elliot Otchet:
> An updated patch for your consideration. Let me know what you think.
>
I have some very minor suggestions:
> Subject: [PATCH] MEDIUM: ssl: Add support for returning the dn samples from
> ssl_(c|f)_(i|s)_dn in LDAP v3 (RFC2253) format.
>
> Modifies the existing sample extraction methods (smp_fetch_ssl_x_i_dn,
> smp_fetch_ssl_x_s_dn) to accommodate a third argument that indicates the DN
> should be returned in LDAP v3 format. When the third argument is present, the
> new function (ssl_sock_get_dn_formatted) is called with three parameters
> including the X509_NAME, a buffer containing the format argument, and a
> buffer for the output. If the supplied format matches the supported format
> string (currently only "rfc2253" is supported), the formatted value is
> extracted into the supplied output buffer using OpenSSL's X509_NAME_print_ex
> and BIO_s_mem. 1 is returned when a dn value is retrieved. 0 is returned
> when a value is not retrieved.
>
> Argument validation is added to each of the related sample configurations to
> ensure the third argument passed is either blank or "rfc2253". An error is
> returned if the third argument is present with any other value.
>
> Documentation was updated in configuration.txt and it was noted during
> preliminary reviews that a CLEANUP patch should follow that adjusts the
> documentation. Currently, this patch and the existing documentation are
> copied with some minor revisions for each sample configuration. It might be
> better to have one entry for all of the samples or entries for each that
> reference back to a primary entry that explains the sample in detail.
>
> Special thanks to Chris, Willy, Tim and Aleks for the feedback.
For future patches I recommend hard wrapping the commit message body at
around 74 characters for consistency with existing commit messages.
> @@ -11103,6 +11144,21 @@ err:
> }
> # endif
>
> +/* Argument validation functions */
> +
> +/* This function is used to validate the arguments passed to any "x_dn" ssl
> + * keywords. These keywords support specifying a third parameter that must be
> + * either empty or the value "rfc2253". Returns 0 on error, non-zero if OK.
> + */
> +int val_dnfmt(struct arg *arg, char **err_msg)
> +{
> + if (arg && arg[2].type == ARGT_STR && arg[2].data.str.data > 0 &&
> (strncmp(arg[2].data.str.area,"rfc2253",7) != 0)) {
I did not test, but if I am not mistaken this would allow "rfc2253XXX"
as the format parameter, no? Meaning: It allows one to append arbitrary
garbage to the rfc2253 value.
Judging from `smp_check_const_bool` in sample.c a simple strcmp should
be safe to use or you can just use the chunk_strcmp you already use in
`ssl_sock_get_dn_formatted`, I guess. Does Willy have any strong opinion
about this?
Also, minor nit: There's a space missing after the commas in the strncmp
call.
> + memprintf(err_msg, "only rfc2253 or a blank value are currently
> supported as the format argument.");
> + return 0;
> + }
> + return 1;
> +}
> +
> /* register cli keywords */
> static struct cli_kw_list cli_kws = {{ },{
> #if (defined SSL_CTRL_SET_TLSEXT_TICKET_KEY_CB && TLS_TICKETS_NO > 0)
Best regards
Tim Düsterhus
