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: 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 t
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
Re: customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
On Fri, Jan 17, 2020 at 04:35:38AM +, Elliot Otchet wrote: > Great feedback guys. Thank you. > I think I've incorporated all of it including > WIlly's requests for:- A better commit message- Fixing the unnecessary edits > (spaces vs tabs)- strcomp instead of strcasecmp > Tim's request for: - Correcting the documentation bracket bugs - Fixing the > typo I introduced > - Updating to his suggested explanation of the feature - An argument > validator for the format parameter. > An updated patch for your consideration. Let me know what you think. Looks good, thanks. I'm just turning the tag from medium to minor since the risk of breakage is infinitesimal. Thanks for indicating in the commit message the possible room left for improvement. I'm seeing that the patch's author is just "degroens", may I place your real name instead ? (aliases are not much welcome nor very polite when people want to discuss a patch later). So just let me know and I'll merge it. Thank you, Willy
Re: customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
Great feedback guys. Thank you.
I think I've incorporated all of it including
WIlly's requests for:- A better commit message- Fixing the unnecessary edits
(spaces vs tabs)- strcomp instead of strcasecmp
Tim's request for: - Correcting the documentation bracket bugs - Fixing the
typo I introduced
- Updating to his suggested explanation of the feature - An argument
validator for the format parameter.
An updated patch for your consideration. Let me know what you think.
Best regards,
-Elliot
On Thursday, January 16, 2020, 7:02:57 AM EST, Tim Düsterhus
wrote:
Elliot,
Am 15.01.20 um 14:24 schrieb Elliot Otchet:
> As always, feedback is greatly appreciated. If acceptable, what are the next
> steps to get this accepted?
I have a few more suggestions on top of the ones from Willy:
> @@ -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
I believe the formatting is incorrect here. It should read:
ssl_c_i_dn([[,[,]]]) or
ssl_c_i_dn([],[],[])
depending on how you consider those optional arguments to work. The
former would be more consistent with the previous formatting.
> 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,8 +15255,10 @@ 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.
> + To obtain a DN formatted according to RFC 2253 for LDAP v3, specify
> + "ssl_c_i_dn(,0,rfc2253)" in the configuration.
I'd phrase this a bit more general:
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
> +ssl_c_key_alg : string]
There's a typo here.
> @@ -15271,7 +15273,7 @@ ssl_c_notbefore : string
> YYMMDDhhmmss[Z] when the incoming connection was made over an SSL/TLS
> transport layer.
>
> -ssl_c_s_dn([[,]]) : string
> +ssl_c_s_dn([[,],]) : string
See above.
> @@ -15280,6 +15282,8 @@ ssl_c_s_dn([[,]]) : string
> the value of the nth given entry value from the beginning/end of the DN.
> For instance, "ssl_c_s_dn(OU,2)" the second organization unit, and
> "ssl_c_s_dn(CN)" retrieves the common name.
> + To obtain a DN formatted according to RFC 2253 for LDAP v3, specify
> + "ssl_c_s_dn(,0,rfc2253)" in the configuration.
I wonder if we should reference back to 'ssl_c_i_dn' to avoid explaining
the same stuff multiple times, possibly forgetting to update one of
those. It should go into a dedicated CLEANUP patch, though.
> @@ -15320,7 +15324,7 @@ ssl_f_der : binary
> incoming connection was made over an SSL/TLS transport layer. When used for
> an ACL, the value(s) to match against can be passed in hexadecimal form.
>
> -ssl_f_i_dn([[,]]) : string
> +ssl_f_i_dn([[,],]) : string
Ditto.
> @@ -15329,6 +15333,8 @@ ssl_f_i_dn([[,]]) : string
> the value of the nth given entry value from the beginning/end of the DN.
> For instance, "ssl_f_i_dn(OU,2)" the second organization unit, and
> "ssl_f_i_dn(CN)" retrieves the common name.
> + To obtain a DN formatted according to RFC 2253 for LDAP v3, specify
> + "ssl_f_i_dn(,0,rfc2253)" in the configuration.
Ditto.
> ssl_f_key_alg : string
> Returns the name of the algorithm used to generate the key of the
>certificate
> @@ -15345,7 +15351,7 @@ ssl_f_notbefore : string
> YYMMDDhhmmss[Z] when the incoming connection was made over an SSL/TLS
> transport layer.
>
> -ssl_f_s_dn([[,]]) : string
> +ssl_f_s_dn([[,],]) : string
Ditto.
> @@ -15354,6 +15360,8 @@ ssl_f_s_dn([[,]]) : string
> the value of the nth given entry value from the beginning/end of the DN.
> For instance, "ssl_f_s_dn(OU,2)" the second organization unit, and
> "ssl_f_s_dn(CN)" retrieves the common name.
> + To obtain a DN formatted according to RFC 2253 for LDAP v3, specify
> + "ssl_f_s_dn(,0,rfc2253)" in the configuration.
Ditto.
> @@ -11148,24 +11185,24 @@ static struct sample_fetch_kw_list
> sample_fetch_keywords = {ILH, {
> { "ssl_c_ca_err_depth", smp_fetch_ssl_c_ca_err_depth, 0,
> NULL, SMP_T_SINT, SMP_USE_L5CLI },
> { "ssl_c_der", smp_fetch_ssl_x_der, 0,
> NULL, SMP_T_BIN, SMP_USE_L5CLI },
> { "ssl_c_err", smp_fetch_ssl_c_err, 0,
> NULL, SMP_T_SINT, SMP_USE_L5CLI },
> - { "ssl_c_i_dn", smp_fetch_ssl_x_i_dn,
> ARG2(0,STR,SINT), NULL
Re: customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
Elliot,
Am 15.01.20 um 14:24 schrieb Elliot Otchet:
> As always, feedback is greatly appreciated. If acceptable, what are the next
> steps to get this accepted?
I have a few more suggestions on top of the ones from Willy:
> @@ -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
I believe the formatting is incorrect here. It should read:
ssl_c_i_dn([[,[,]]]) or
ssl_c_i_dn([],[],[])
depending on how you consider those optional arguments to work. The
former would be more consistent with the previous formatting.
>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,8 +15255,10 @@ 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.
> + To obtain a DN formatted according to RFC 2253 for LDAP v3, specify
> + "ssl_c_i_dn(,0,rfc2253)" in the configuration.
I'd phrase this a bit more general:
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
> +ssl_c_key_alg : string]
There's a typo here.
> @@ -15271,7 +15273,7 @@ ssl_c_notbefore : string
>YYMMDDhhmmss[Z] when the incoming connection was made over an SSL/TLS
>transport layer.
>
> -ssl_c_s_dn([[,]]) : string
> +ssl_c_s_dn([[,],]) : string
See above.
> @@ -15280,6 +15282,8 @@ ssl_c_s_dn([[,]]) : string
>the value of the nth given entry value from the beginning/end of the DN.
>For instance, "ssl_c_s_dn(OU,2)" the second organization unit, and
>"ssl_c_s_dn(CN)" retrieves the common name.
> + To obtain a DN formatted according to RFC 2253 for LDAP v3, specify
> + "ssl_c_s_dn(,0,rfc2253)" in the configuration.
I wonder if we should reference back to 'ssl_c_i_dn' to avoid explaining
the same stuff multiple times, possibly forgetting to update one of
those. It should go into a dedicated CLEANUP patch, though.
> @@ -15320,7 +15324,7 @@ ssl_f_der : binary
>incoming connection was made over an SSL/TLS transport layer. When used for
>an ACL, the value(s) to match against can be passed in hexadecimal form.
>
> -ssl_f_i_dn([[,]]) : string
> +ssl_f_i_dn([[,],]) : string
Ditto.
> @@ -15329,6 +15333,8 @@ ssl_f_i_dn([[,]]) : string
>the value of the nth given entry value from the beginning/end of the DN.
>For instance, "ssl_f_i_dn(OU,2)" the second organization unit, and
>"ssl_f_i_dn(CN)" retrieves the common name.
> + To obtain a DN formatted according to RFC 2253 for LDAP v3, specify
> + "ssl_f_i_dn(,0,rfc2253)" in the configuration.
Ditto.
> ssl_f_key_alg : string
>Returns the name of the algorithm used to generate the key of the
> certificate
> @@ -15345,7 +15351,7 @@ ssl_f_notbefore : string
>YYMMDDhhmmss[Z] when the incoming connection was made over an SSL/TLS
>transport layer.
>
> -ssl_f_s_dn([[,]]) : string
> +ssl_f_s_dn([[,],]) : string
Ditto.
> @@ -15354,6 +15360,8 @@ ssl_f_s_dn([[,]]) : string
>the value of the nth given entry value from the beginning/end of the DN.
>For instance, "ssl_f_s_dn(OU,2)" the second organization unit, and
>"ssl_f_s_dn(CN)" retrieves the common name.
> + To obtain a DN formatted according to RFC 2253 for LDAP v3, specify
> + "ssl_f_s_dn(,0,rfc2253)" in the configuration.
Ditto.
> @@ -11148,24 +11185,24 @@ static struct sample_fetch_kw_list
> sample_fetch_keywords = {ILH, {
> { "ssl_c_ca_err_depth", smp_fetch_ssl_c_ca_err_depth, 0,
>NULL,SMP_T_SINT, SMP_USE_L5CLI },
> { "ssl_c_der", smp_fetch_ssl_x_der, 0,
>NULL,SMP_T_BIN, SMP_USE_L5CLI },
> { "ssl_c_err", smp_fetch_ssl_c_err, 0,
>NULL,SMP_T_SINT, SMP_USE_L5CLI },
> - { "ssl_c_i_dn", smp_fetch_ssl_x_i_dn,
> ARG2(0,STR,SINT),NULL,SMP_T_STR, SMP_USE_L5CLI },
> + { "ssl_c_i_dn", smp_fetch_ssl_x_i_dn,
> ARG3(0,STR,SINT,STR),NULL,SMP_T_STR, SMP_USE_L5CLI },
> { "ssl_c_key_alg", smp_fetch_ssl_x_key_alg, 0,
>NULL,SMP_T_STR, SMP_USE_L5CLI },
> { "ssl_c_notafter", smp_fetch_ssl_x_notafter, 0,
>NULL,SMP_T_STR, SMP_USE_L5CLI },
> { "ssl_c_notbefore",smp_fetch_ssl_x_notbefore,0,
>NULL,SMP_T_STR, SMP_USE_L5CLI },
> { "ssl_c_sig_alg"
Re: customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
Hi Elliot, On Wed, Jan 15, 2020 at 01:24:33PM +, Elliot Otchet wrote: > Willy, > The patch attached implements as close to your intent as I could get without > making changes outside of ssl_sock.c. > If you remember, you had asked for a patch that implemented extending the > current methods with a third option to indicate the DN output should be in > RFC 2253 e.g. "ssl_c_i_dn(,,rfc2253)". > This patch comes close to that. It extends the existing methods, but the > options have to be passed as "ssl_c_i_dn(,0,rfc2253)" This is because the > second argument is an integer and the arg parser doesn't like missing > integers. > > The patch includes examples in the configuration documentation that > demonstrated how to configure it for the needed output. > As always, feedback is greatly appreciated. If acceptable, what are the next > steps to get this accepted? This looks pretty good. I'm only seeing a very minor nit which is trivial to address: the whole config is case sensitive, so better use strcmp rather than strcasecmp to match the "rfc2253" value in the 3rd argument. Otherwise there's a risk that a few users will accidently mistype it, it will work and later when passing their config through automated tools it will fail. Regarding what's missing, I'd say a commit message :-) What you described above and/or in your first presentation is OK. Please have a quick look at CONTRIBUTING and just run "git log" src/ssl_sock.c to get some inspiration on the expected format but we don't need anything complicated, just something which allows one not to have to read the patch when running "git log". Ah, be careful, you had a reindentation problem in smp_fetch_ssl_x_s_dn(), some tabs were mistakenly replaced with spaces, causing more changes than needed. Thanks! Willy
Re: customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
Willy, The patch attached implements as close to your intent as I could get without making changes outside of ssl_sock.c. If you remember, you had asked for a patch that implemented extending the current methods with a third option to indicate the DN output should be in RFC 2253 e.g. "ssl_c_i_dn(,,rfc2253)". This patch comes close to that. It extends the existing methods, but the options have to be passed as "ssl_c_i_dn(,0,rfc2253)" This is because the second argument is an integer and the arg parser doesn't like missing integers. The patch includes examples in the configuration documentation that demonstrated how to configure it for the needed output. As always, feedback is greatly appreciated. If acceptable, what are the next steps to get this accepted? Best, -Elliot On Sunday, January 12, 2020, 1:41:19 PM EST, Elliot Otchet wrote: Willy, Thanks for the note back. Your suggestion seems like a good approach. I'll submit a new patch when I have this worked up and tested for a bit. Regards, Elliot On Thursday, January 9, 2020, 12:42:36 AM EST, Willy Tarreau wrote: Hi Elliot & Chris, On Sun, Jan 05, 2020 at 09:09:18PM +, Elliot Otchet wrote: > Hi, > An updated patch is attached for the implementation that adds documentation > to the doc/configuration.txt and fixes an issue identified during testing. > I couldn't locate tests for the preexisting ssl_c_s_dn and related > parameters. I'd be happy to add tests for the new ones if someone could > point me in the right direction of the existing ones. Sorry for the late response, it seems your previous messages were sent in the unlucky period where everyone's busy doing everything but looking at the mailing list :-) I understand the problem you're facing and I agree that it's easier to have fresh new code to handle this. However instead of having a separate set of sample fetches for this, it would be much better to add an optional argument to the existing one which would change the output format. This would even allow to factor some existing code I guess. Look for example at the UUID sample fetch which is a good illustration of this: uuid([]) : string Returns a UUID following the RFC4122 standard. If the version is not specified, a UUID version 4 (fully random) is returned. I'm seeing that ssl_c_i_dn() and ssl_c_s_dn() already take up to two optional args. But that doesn't prevent one from adding a third one with the output format specification. If you just want to get it emitted with no specific field filtering you could just use "ssl_c_i_dn(,,rfc2253)" for example (or any other arg value you choose). Just my two cents, Willy From 56cb18cd01346d11980ad3e5ba8e99148e60e863 Mon Sep 17 00:00:00 2001 From: degroens Date: Wed, 15 Jan 2020 08:12:14 -0500 Subject: [PATCH] Extends _s_dn and i_dn options with the ability to output RFC2253 (LDAP v3) DNs --- doc/configuration.txt | 18 ++ src/ssl_sock.c| 69 +++ 2 files changed, 66 insertions(+), 21 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index 9ac8985..ba562f3 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,8 +15255,10 @@ 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. + To obtain a DN formatted according to RFC 2253 for LDAP v3, specify + "ssl_c_i_dn(,0,rfc2253)" in the configuration. -ssl_c_key_alg : string +ssl_c_key_alg : string] Returns the name of the algorithm used to generate the key of the certificate presented by the client when the incoming connection was made over an SSL/TLS transport layer. @@ -15271,7 +15273,7 @@ ssl_c_notbefore : string YYMMDDhhmmss[Z] when the incoming connection was made over an SSL/TLS transport layer. -ssl_c_s_dn([[,]]) : string +ssl_c_s_dn([[,],]) : string When the incoming connection was made over an SSL/TLS transport layer, returns the full distinguished name of the subject of the certificate presented by the client when no is specified, or the value of the @@ -15280,6 +15282,8 @@ ssl_c_s_dn([[,]]) : string the value of the nth given entry value from the beginning/end of the DN. For instance, "ssl_c_s_dn(OU,2)" the second organization unit, and "ssl_c_s_dn(CN)" retrieves the common name. + To obtain a DN formatted according to RFC 2253 for LDAP v3, specify + "ssl_c_s_dn(,0,rfc2253)
Re: customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
Willy, Thanks for the note back. Your suggestion seems like a good approach. I'll submit a new patch when I have this worked up and tested for a bit. Regards, Elliot On Thursday, January 9, 2020, 12:42:36 AM EST, Willy Tarreau wrote: Hi Elliot & Chris, On Sun, Jan 05, 2020 at 09:09:18PM +, Elliot Otchet wrote: > Hi, > An updated patch is attached for the implementation that adds documentation > to the doc/configuration.txt and fixes an issue identified during testing. > I couldn't locate tests for the preexisting ssl_c_s_dn and related > parameters. I'd be happy to add tests for the new ones if someone could > point me in the right direction of the existing ones. Sorry for the late response, it seems your previous messages were sent in the unlucky period where everyone's busy doing everything but looking at the mailing list :-) I understand the problem you're facing and I agree that it's easier to have fresh new code to handle this. However instead of having a separate set of sample fetches for this, it would be much better to add an optional argument to the existing one which would change the output format. This would even allow to factor some existing code I guess. Look for example at the UUID sample fetch which is a good illustration of this: uuid([]) : string Returns a UUID following the RFC4122 standard. If the version is not specified, a UUID version 4 (fully random) is returned. I'm seeing that ssl_c_i_dn() and ssl_c_s_dn() already take up to two optional args. But that doesn't prevent one from adding a third one with the output format specification. If you just want to get it emitted with no specific field filtering you could just use "ssl_c_i_dn(,,rfc2253)" for example (or any other arg value you choose). Just my two cents, Willy
Re: customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
Hi Elliot & Chris, On Sun, Jan 05, 2020 at 09:09:18PM +, Elliot Otchet wrote: > Hi, > An updated patch is attached for the implementation that adds documentation > to the doc/configuration.txt and fixes an issue identified during testing. > I couldn't locate tests for the preexisting ssl_c_s_dn and related > parameters. I'd be happy to add tests for the new ones if someone could > point me in the right direction of the existing ones. Sorry for the late response, it seems your previous messages were sent in the unlucky period where everyone's busy doing everything but looking at the mailing list :-) I understand the problem you're facing and I agree that it's easier to have fresh new code to handle this. However instead of having a separate set of sample fetches for this, it would be much better to add an optional argument to the existing one which would change the output format. This would even allow to factor some existing code I guess. Look for example at the UUID sample fetch which is a good illustration of this: uuid([]) : string Returns a UUID following the RFC4122 standard. If the version is not specified, a UUID version 4 (fully random) is returned. I'm seeing that ssl_c_i_dn() and ssl_c_s_dn() already take up to two optional args. But that doesn't prevent one from adding a third one with the output format specification. If you just want to get it emitted with no specific field filtering you could just use "ssl_c_i_dn(,,rfc2253)" for example (or any other arg value you choose). Just my two cents, Willy
Re: customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
Hi,
An updated patch is attached for the implementation that adds documentation to
the doc/configuration.txt and fixes an issue identified during testing.
I couldn't locate tests for the preexisting ssl_c_s_dn and related parameters.
I'd be happy to add tests for the new ones if someone could point me in the
right direction of the existing ones.
Best regards,
On Thursday, January 2, 2020, 1:17:23 PM EST, Aleksandar Lazic
wrote:
Hi Chris.
I can't verify the c code. I just ask to add a documentation to the patch. Can
the sample be tested with vtest?
It would be nice to mention that rfc2253 is LDAP v3. :-)
Regards
Aleks.
Jan 2, 2020 3:41:35 PM Chris Software :
> Hello haproxy,
> My team member has completed a patch which allows someone to configure the
> format of the proxied certificate's DN and Issuer DN.
> I have attached the patch here. How can we submit this for inclusion to
> haproxy?
> Chris
>
>
> On Fri, Dec 20, 2019 at 12:53 PM Chris Software < softwarechri...@gmail.com
> [mailto:softwarechri...@gmail.com] > wrote:
>
> > Hello,
> >
> > This is an update on the offchance that some diligent team member is
> > spinning their wheels on this.
> >
> > Some team members of mine are modifying the haproxy ssl.c file to make the
> > format of the ssl_c_s_dn variable configurable, and editing for simplicity
> > to use standard openssl function calls.
> >
> > There is discussion about submitting this change back as a patch.
> >
> > Chris
> >
> >
> >
> >
> >
>
From 430d5b181fda66798b2edd4b8bd69a2c1e89810d Mon Sep 17 00:00:00 2001
From: degroens
Date: Wed, 1 Jan 2020 18:42:08 -0500
Subject: [PATCH 3/3] Adds support for generating RFC2253 DNs using
configuration variables by invoking OpenSSL library features.
---
doc/configuration.txt | 22
src/ssl_sock.c| 138 ++
2 files changed, 160 insertions(+)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 9bc7d71..2876b56 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -4605,8 +4605,10 @@ http-request set-header [ { if | unless } ]
http-request set-header X-SSL-Session_ID %[ssl_fc_session_id,hex]
http-request set-header X-SSL-Client-Verify%[ssl_c_verify]
http-request set-header X-SSL-Client-DN%{+Q}[ssl_c_s_dn]
+http-request set-header X-SSL-Client-DN-2253 %{+Q}[ssl_c_s_dn_rfc2253]
http-request set-header X-SSL-Client-CN%{+Q}[ssl_c_s_dn(cn)]
http-request set-header X-SSL-Issuer %{+Q}[ssl_c_i_dn]
+http-request set-header X-SSL-Issuer-DN-2253 %{+Q}[ssl_c_i_dn_rfc2253]
http-request set-header X-SSL-Client-NotBefore %{+Q}[ssl_c_notbefore]
http-request set-header X-SSL-Client-NotAfter %{+Q}[ssl_c_notafter]
@@ -15253,6 +15255,11 @@ ssl_c_i_dn([[,]]) : string
For instance, "ssl_c_i_dn(OU,2)" the second organization unit, and
"ssl_c_i_dn(CN)" retrieves the common name.
+ssl_c_i_dn_rfc2253 : string
+ When the incoming connection was made over an SSL/TLS transport layer,
+ returns the full distinguished name of the issuer of the certifcate
+ presented by the client in RFC 2253 (LDAP v3) format.
+
ssl_c_key_alg : string
Returns the name of the algorithm used to generate the key of the certificate
presented by the client when the incoming connection was made over an SSL/TLS
@@ -15278,6 +15285,11 @@ ssl_c_s_dn([[,]]) : string
For instance, "ssl_c_s_dn(OU,2)" the second organization unit, and
"ssl_c_s_dn(CN)" retrieves the common name.
+ssl_c_s_dn_rfc2253 : string
+ When the incoming connection was made over an SSL/TLS transport layer,
+ returns the full distinguished name of the subject of the certifcate
+ presented by the client in RFC 2253 (LDAP v3) format.
+
ssl_c_serial : binary
Returns the serial of the certificate presented by the client when the
incoming connection was made over an SSL/TLS transport layer. When used for
@@ -15327,6 +15339,11 @@ ssl_f_i_dn([[,]]) : string
For instance, "ssl_f_i_dn(OU,2)" the second organization unit, and
"ssl_f_i_dn(CN)" retrieves the common name.
+ssl_f_i_dn_rfc2253 : string
+ When the incoming connection was made over an SSL/TLS transport layer,
+ returns the full distinguished name of the issuer of the certifcate
+ presented by the frontend in RFC 2253 (LDAP v3) format.
+
ssl_f_key_alg : string
Returns the name of the algorithm used to generate the key of the certificate
presented by the frontend when the incoming connection was made over an
@@ -15352,6 +15369,11 @@ ssl_f_s_dn([[,]]) : string
For instance, "ssl_f_s_dn(OU,2)" the second organization unit, and
"ssl_f_s_dn(CN)" retrieves the common name.
+ssl_f_s_dn_rfc2253 : string
+ When the incoming connection was made over an SSL/TLS transport layer,
+ returns the full distinguished name of the subject of the certifcate
+ presented by the frontend in R
Re: customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
Hi Chris. I can't verify the c code. I just ask to add a documentation to the patch. Can the sample be tested with vtest? It would be nice to mention that rfc2253 is LDAP v3. :-) Regards Aleks. Jan 2, 2020 3:41:35 PM Chris Software : > Hello haproxy, > My team member has completed a patch which allows someone to configure the > format of the proxied certificate's DN and Issuer DN. > I have attached the patch here. How can we submit this for inclusion to > haproxy? > Chris > > > On Fri, Dec 20, 2019 at 12:53 PM Chris Software < softwarechri...@gmail.com > [mailto:softwarechri...@gmail.com] > wrote: > > > Hello, > > > > This is an update on the offchance that some diligent team member is > > spinning their wheels on this. > > > > Some team members of mine are modifying the haproxy ssl.c file to make the > > format of the ssl_c_s_dn variable configurable, and editing for simplicity > > to use standard openssl function calls. > > > > There is discussion about submitting this change back as a patch. > > > > Chris > > > > > > > > > > >
Re: customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
Hello haproxy, My team member has completed a patch which allows someone to* configure* the format of the proxied certificate's DN and Issuer DN. I have attached the patch here. How can we submit this for inclusion to haproxy? Chris On Fri, Dec 20, 2019 at 12:53 PM Chris Software wrote: > Hello, > > This is an update on the offchance that some diligent team member is > spinning their wheels on this. > > Some team members of mine are modifying the haproxy ssl.c file to make the > format of the ssl_c_s_dn variable configurable, and editing for simplicity > to use standard openssl function calls. > > There is discussion about submitting this change back as a patch. > > Chris > > > > > 0001-Adds-support-for-generating-RFC2253-DNs-using-config.patch Description: Binary data
RE: customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
Hello, This is an update on the offchance that some diligent team member is spinning their wheels on this. Some team members of mine are modifying the haproxy ssl.c file to make the format of the ssl_c_s_dn variable configurable, and editing for simplicity to use standard openssl function calls. There is discussion about submitting this change back as a patch. Chris
customize format of haproxy X-ForwardedFor ssl_c_s_dn during SSL termination
I am running haproxy in an Alpine Docker container. It is doing SSL
termination for https and injecting the client DN into the X-ForwardedFor
HTTP Header. But the format it uses for the client DN is not one that my
application supports.
Can I change the format somehow, perhaps using openssl.cnf? People
apparently change encryption algorithm settings that way. Can I change my
DN format also?
This is the format that I need my certificate DN to look like in the
X-ForwardedFor header. It is rather LDAP-ey.
CN=Jane Smith,OU=org1,OU=org2,O=myorg,C=AU
But haproxy is injecting a rather ASN.1 looking format
/C=AU/O=myorg/OU=org2/OU=org1/CN=Jane Smith
These are the verisons of the software. They can be changed, as I am
compiling haproxy from source. I can also set any flags at compilation time:
1. Haproxy 2.0
2. Alpine 3.10
3. openssl 1.1.1
Here are what I think are the relevant parts of the haproxy.cfg file .
frontend fe
mode http
bind *:443 ssl no-sslv3 no-tls10 no-tlsv11 crt /certs/mycert
ca-file /certs/myca
option forwardfor
http-request set-header X-ForwardedFor %{+Q+E}[ssl_c_s_dn]
default_backend be
backend be
balance source
mode http
server server1 IP:PORT ca-file /certs/myca crt /certs/mycert ssl verify none
Is there something I can do to change the format? I have tried using the
documented structure like this: %{+Q+E}[ssl_c_s_dn(CN)] but the format of
my certificate DNs is very disorganized. There is no way to predict how
many OU, C, O, etc there might be, and sometimes they are missing. So I
don't think that is a viable solution.
I have also looked at how this question: haproxy tls hash algorithm
customizes
haproxy behavior with openssl settings. Can I do that to get the DN into a
different format? If so, how? I am not sure what steps to follow. Do I need
to modify openssl.cnf at compile time, or have it changed at runtime on the
server? What sections and values?
Thank you!
