Re: [PATCH 0/1] Implement new sample fetch method to get the curve name used in key agreement

2023-07-17 Thread William Lallemand
On Mon, Jul 17, 2023 at 08:22:58AM -0500, Mariam John wrote:
> This is an implementation of feature request 
> [#2165](https://github.com/haproxy/haproxy/issues/2165),
> to get the EC curve name used during the key agreement in OpenSSL. This patch 
> includes the following
> changes:
> - new sample fetch methods `ssl_fc_curve` and `ssl_bc_curve`, to get the 
> curve name
> - doc changes to add description for the new sample fetch methods
> - a new regression test 'ssl_curve_name` to test the new sample fetch 
> methods. (Tested it with the 
>   CI against my fork using github actions and it passes with all supported 
> SSL libraries and OpenSSL versions)
> 
> This uses the function `SSL_get_negotiated_group` method available from the 
> OpenSSLv3 release.
> 
> Thank you.
> 
> Mariam John (1):
>   MEDIUM: ssl: new sample fetch method to get curve name
> 
>  doc/configuration.txt| 10 +++
>  reg-tests/ssl/ssl_curve_name.vtc | 51 
>  src/ssl_sample.c | 46 
>  3 files changed, 107 insertions(+)
>  create mode 100644 reg-tests/ssl/ssl_curve_name.vtc
> 

Great, thank you!

I just pushed your patch in the master branch.

-- 
William Lallemand



RE: [PATCH 0/1] Implement new sample fetch method to get the curve name used in key agreement

2023-07-17 Thread Mariam John
Indeed I did. I send the patch from the old folder. I apologize for the mistake 
on my part. I am sending the right one now.

Thank you for your patience.

Regards,
Mariam.


From: William Lallemand 
Date: Monday, July 17, 2023 at 4:28 AM
To: Mariam John 
Cc: haproxy@formilux.org , eb...@haproxy.com 

Subject: [EXTERNAL] Re: [PATCH 0/1] Implement new sample fetch method to get 
the curve name used in key agreement
On Fri, Jul 14, 2023 at 02:59:52AM -0500, Mariam John wrote:
> This is an implementation of feature request 
> [#2165](https://github.com/haproxy/haproxy/issues/2165 ),
> to get the EC curve name used during the key agreement in OpenSSL. This patch 
> includes the following
> changes:
> - new sample fetch methods `ssl_fc_curve` and `ssl_bc_curve`, to get the 
> curve name
> - doc changes to add description for the new sample fetch methods
> - updates the existing regression tests to test the new sample fetch methods
>
> This uses the function `SSL_get_negotiated_group` method available from the 
> OpenSSLv3 release.
>
> Thank you.
>
> Mariam John (1):
>   MEDIUM: ssl: new sample fetch method to get curve name
>
>  doc/configuration.txt|  8 +
>  reg-tests/ssl/ssl_client_samples.vtc |  2 ++
>  reg-tests/ssl/ssl_curves.vtc |  4 +++
>  src/ssl_sample.c | 46 
>  4 files changed, 60 insertions(+)
>

Hello Mariam,

It seems like you sent us the previous patch by mistake?

I pushed the patch in a separate branch to run the CI on it:
https://github.com/haproxy/haproxy/actions/runs/5573879545/jobs/10181759082

Regards,

--
William Lallemand


Re: [PATCH 0/1] Implement new sample fetch method to get the curve name used in key agreement

2023-07-17 Thread William Lallemand
On Fri, Jul 14, 2023 at 02:59:52AM -0500, Mariam John wrote:
> This is an implementation of feature request 
> [#2165](https://github.com/haproxy/haproxy/issues/2165),
> to get the EC curve name used during the key agreement in OpenSSL. This patch 
> includes the following
> changes:
> - new sample fetch methods `ssl_fc_curve` and `ssl_bc_curve`, to get the 
> curve name
> - doc changes to add description for the new sample fetch methods
> - updates the existing regression tests to test the new sample fetch methods
> 
> This uses the function `SSL_get_negotiated_group` method available from the 
> OpenSSLv3 release. 
> 
> Thank you.
> 
> Mariam John (1):
>   MEDIUM: ssl: new sample fetch method to get curve name
> 
>  doc/configuration.txt|  8 +
>  reg-tests/ssl/ssl_client_samples.vtc |  2 ++
>  reg-tests/ssl/ssl_curves.vtc |  4 +++
>  src/ssl_sample.c | 46 
>  4 files changed, 60 insertions(+)
> 

Hello Mariam,

It seems like you sent us the previous patch by mistake?

I pushed the patch in a separate branch to run the CI on it:
https://github.com/haproxy/haproxy/actions/runs/5573879545/jobs/10181759082

Regards,

-- 
William Lallemand



Re: [PATCH 0/1] Implement new sample fetch method to get the curve name used in key agreement

2023-06-21 Thread William Lallemand
Hello Mariam,

On Tue, Jun 20, 2023 at 11:50:51AM -0500, Mariam John wrote:
> This is an implementation of feature request 
> [#2165](https://github.com/haproxy/haproxy/issues/2165),
> to get the EC curve name used during the key agreement in OpenSSL. This patch 
> includes the following
> changes:
> - new sample fetch methods `ssl_fc_curve` and `ssl_bc_curve`, to get the 
> curve name
> - doc changes to add description for the new sample fetch methods
> - updates the existing regression tests to test the new sample fetch methods
> 
> This uses the function `SSL_get_negotiated_group` method available from the 
> OpenSSLv3 release. 
> 

Thanks for your contribution, review below.


On Tue, Jun 20, 2023 at 11:50:52AM -0500, Mariam John wrote:
> Adds a new sample fetch method to get the curve name used in the
> key agreement to enable better observability. In OpenSSLv3, the function
> `SSL_get_negotiated_group` returns the NID of the curve and from the NID,
> we get the curve name by passing the NID to OBJ_nid2sn. This was not
> available in v1.1.1. SSL_get_curve_name(), which returns the curve name
> directly was merged into OpenSSL master branch last week but will be available
> only in its next release.
> ---
>  doc/configuration.txt|  8 +
>  reg-tests/ssl/ssl_client_samples.vtc |  2 ++
>  reg-tests/ssl/ssl_curves.vtc |  4 +++
>  src/ssl_sample.c | 46 
>  4 files changed, 60 insertions(+)
> 
> diff --git a/doc/configuration.txt b/doc/configuration.txt
> index 8bcfc3c06..d944ac132 100644
> --- a/doc/configuration.txt
> +++ b/doc/configuration.txt
> @@ -20646,6 +20646,10 @@ ssl_bc_cipher : string
>over an SSL/TLS transport layer. It can be used in a tcp-check or an
>http-check ruleset.
>  
> +ssl_bc_curve : string
> +  Returns the name of the curve used in the key agreement when the outgoing
> +  connection was made over an SSL/TLS transport layer.
> +
>  ssl_bc_client_random : binary
>Returns the client random of the back connection when the incoming 
> connection
>was made over an SSL/TLS transport layer. It is useful to to decrypt 
> traffic
> @@ -20944,6 +20948,10 @@ ssl_fc_cipher : string
>Returns the name of the used cipher when the incoming connection was made
>over an SSL/TLS transport layer.
>  
> +ssl_fc_curve : string
> +  Returns the name of the curve used in the key agreement when the incoming
> +  connection was made over an SSL/TLS transport layer.
> +

As Lukas mentioned, be careful with the alphabetical order in the
configuration and the required OpenSSL version.

>  ssl_fc_cipherlist_bin([]) : binary
>Returns the binary form of the client hello cipher list. The maximum
>returned value length is limited by the shared capture buffer size
> diff --git a/reg-tests/ssl/ssl_client_samples.vtc 
> b/reg-tests/ssl/ssl_client_samples.vtc
> index 5a84e4b25..1f078ea98 100644
> --- a/reg-tests/ssl/ssl_client_samples.vtc
> +++ b/reg-tests/ssl/ssl_client_samples.vtc
> @@ -46,6 +46,7 @@ haproxy h1 -conf {
>  http-response add-header x-ssl-s_serial %[ssl_c_serial,hex]
>  http-response add-header x-ssl-key_alg %[ssl_c_key_alg]
>  http-response add-header x-ssl-version %[ssl_c_version]
> +http-response add-header x-ssl-curve-name %[ssl_fc_curve]
>  
>  bind "${tmpdir}/ssl.sock" ssl crt ${testdir}/common.pem ca-file 
> ${testdir}/ca-auth.crt verify optional crt-ignore-err all crl-file 
> ${testdir}/crl-auth.pem
>  
> @@ -69,6 +70,7 @@ client c1 -connect ${h1_clearlst_sock} {
>  expect resp.http.x-ssl-s_serial == "02"
>  expect resp.http.x-ssl-key_alg == "rsaEncryption"
>  expect resp.http.x-ssl-version == "1"
> +expect resp.http.x-ssl-curve-name == "X25519"
>  } -run
>  

The ssl_client_samples.vtc file is dedicated to samples used in Client
Authentication, so I don't think it should be here.


> diff --git a/reg-tests/ssl/ssl_curves.vtc b/reg-tests/ssl/ssl_curves.vtc
> index 5cc70df14..3dbe47c4d 100644
> --- a/reg-tests/ssl/ssl_curves.vtc
> +++ b/reg-tests/ssl/ssl_curves.vtc
> @@ -75,6 +75,7 @@ haproxy h1 -conf {
>  listen ssl1-lst
>  bind "${tmpdir}/ssl1.sock" ssl crt ${testdir}/common.pem ca-file 
> ${testdir}/set_cafile_rootCA.crt verify optional curves P-256:P-384
>  server s1 ${s1_addr}:${s1_port}
> +http-response add-header x-ssl-fc-curve-name %[ssl_fc_curve]
>  
>  # The prime256v1 curve, which is used by default by a backend when no
>  # 'curves' or 'ecdhe' option is specified, is not allowed on this 
> listener
> @@ -98,6 +99,7 @@ haproxy h1 -conf {
>  
>  bind "${tmpdir}/ssl-ecdhe-256.sock" ssl crt ${testdir}/common.pem 
> ca-file ${testdir}/set_cafile_rootCA.crt verify optional ecdhe prime256v1
>  server s1 ${s1_addr}:${s1_port}
> +http-response add-header x-ssl-fc-curve-name %[ssl_fc_curve]
>  
>  } -start
>  
> @@ -105,6 +107,7 @@ client c1 -connect ${h1_clearlst_sock} {
>