Welcome! Log In Create A New Profile

Advanced

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Maxim Dounin
September 04, 2022 08:54PM
Hello!

On Wed, Aug 31, 2022 at 07:52:15PM +0400, Roman Arutyunyan wrote:

> # HG changeset patch
> # User Roman Arutyunyan <arut@nginx.com>
> # Date 1661436099 -14400
> # Thu Aug 25 18:01:39 2022 +0400
> # Node ID 4b856f1dff939e4eb9c131e17af061cf2c38cfac
> # Parent 069a4813e8d6d7ec662d282a10f5f7062ebd817f
> Core: support for reading PROXY protocol v2 TLVs.

First of all, could you please provide details on the use case?
I've seen requests for writing proxy protocol TLVs to upstream
servers (see ticket #1639), but not yet seen any meaningful
reading requests.

> The TLV values are available in HTTP and Stream variables
> $proxy_protocol_tlv_0xN, where N is a hexadecimal TLV type number with no
> leading zeroes.

I can't say I like the "hexadecimal TLV type number with no
leading zeroes" approach, especially given that the specification
uses leading zeroes in TLV types. With leading zeros might be
better, to match specification.

Also, it might worth the effort to actually add names for known
types instead or in addition to numbers.

Another question is PP2_TYPE_SSL, which is itself a complex
structure and a list of multiple subtypes. Provided

Given the above, not sure if the approach with early parsing and
header-like list as in the patch is the good idea. Just
preserving TLVs as is and parsing them all during variable
evaluation might be easier and more efficient.

Also, the idea of merging TLV values with identical types looks
wrong to me, especially given that many TLSs are binary.
Specification does not seem to define the behaviour here,
unfortunately. As far as I understand, HAProxy itself still
doesn't implement PPv2 parsing, so there is not reference
implementation either. On the other hand, it should be easy
enough to check all TLVs for duplicate by using a 256-bit bitmask
and reject connections if there are any duplicates.

>
> diff --git a/src/core/ngx_proxy_protocol.c b/src/core/ngx_proxy_protocol.c
> --- a/src/core/ngx_proxy_protocol.c
> +++ b/src/core/ngx_proxy_protocol.c
> @@ -40,6 +40,12 @@ typedef struct {
> } ngx_proxy_protocol_inet6_addrs_t;
>
>
> +typedef struct {
> + u_char type;
> + u_char len[2];
> +} ngx_proxy_protocol_tlv_t;
> +
> +
> static u_char *ngx_proxy_protocol_read_addr(ngx_connection_t *c, u_char *p,
> u_char *last, ngx_str_t *addr);
> static u_char *ngx_proxy_protocol_read_port(u_char *p, u_char *last,
> @@ -273,8 +279,11 @@ ngx_proxy_protocol_v2_read(ngx_connectio
> size_t len;
> socklen_t socklen;
> ngx_uint_t version, command, family, transport;
> + ngx_list_t *pp_tlv;

tlvs?

See above though, it probably doesn't make sense to parse TLVs
here.

> ngx_sockaddr_t src_sockaddr, dst_sockaddr;
> + ngx_table_elt_t *t;

Just in case, most ngx_table_elt_t variables are named "h", as in
"header".

[...]

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org
Subject Author Views Posted

[PATCH] Core: support for reading PROXY protocol v2 TLVs

Roman Arutyunyan 734 August 31, 2022 11:54AM

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Roman Arutyunyan 54 September 01, 2022 05:48AM

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Maxim Dounin 64 September 04, 2022 08:54PM

RE: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Eran Kornblau via nginx-devel 65 September 05, 2022 01:14AM

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Roman Arutyunyan 50 September 05, 2022 09:24AM

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Maxim Dounin 109 September 05, 2022 12:00PM

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Roman Arutyunyan 59 September 09, 2022 11:48AM

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Maxim Dounin 49 September 12, 2022 05:32PM

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Roman Arutyunyan 51 September 13, 2022 11:04AM

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Maxim Dounin 55 September 19, 2022 10:48PM

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Roman Arutyunyan 52 September 27, 2022 05:42AM

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Maxim Dounin 41 October 10, 2022 09:22PM

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Roman Arutyunyan 36 October 11, 2022 09:02AM

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Maxim Dounin 38 October 11, 2022 04:56PM

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Roman Arutyunyan 31 October 31, 2022 08:08AM

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Maxim Dounin 26 November 01, 2022 10:16AM

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Roman Arutyunyan 25 November 02, 2022 09:08AM

Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs

Maxim Dounin 30 November 02, 2022 09:54PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 90
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready