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

[Eran Kornblau]

> 
> 
> -----Original Message-----
> From: Maxim Dounin <mdounin at mdounin.ru> 
> Sent: Monday, 5 September 2022 3:53
> To: nginx-devel at nginx.org
> Subject: Re: [PATCH] Core: support for reading PROXY protocol v2 TLVs
> 
> Hello!
> 
> On Wed, Aug 31, 2022 at 07:52:15PM +0400, Roman Arutyunyan wrote:
> 
> > # HG changeset patch
> > # User Roman Arutyunyan <arut at 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.
> 
Hi, 

Jumping in here regarding use cases, we have a couple of use cases for this in our live video system -

1. We support video ingest over Haivision's SRT protocol (a UDP-based transport protocol designed for video streaming).
   SRT has a concept of 'stream id' (https://github.com/Haivision/srt/blob/master/docs/features/access-control.md) which we're using to
   identify the incoming video stream (a bit like the stream name/tc url in RTMP).
   In the implementation, we have separate components for handling the transport layer and the application/video layer.
   For the transport, we implemented an SRT/TCP gateway (https://github.com/kaltura/nginx-srt-module),
   while for the application layer, we are using a slightly modified version of Roman's nginx-ts-module.
   The problem is how to propagate the SRT stream-id to the TCP upstream.
   Currently, the solution we have for this is a poor man's proxy protocol - we send the stream-id on the TCP socket,
   delimited by a newline, and our modified version of nginx-ts-module extracts it.

2. Another feature we have is RTMP output, which enables our customers to send their video streams to 3rd parties.
   We wrote some custom module to translate the internal protocol we use for video transfer back to RTMP,
   but, we also want to support RTMPS output (RTMP over SSL/TLS). We can extend our module to support SSL,
   but that is probably not a small task... so, instead, we are using a local nginx 'stream' proxy that acts as a TCP->SSL gateway.
   The problem is how to pass the host/port of the external RTMP endpoint over the internal TCP connection
   (we want it to be dynamic - prefer not to use a set of predefined host names in the configuration).
   The solution we have now is the same as above - we send the URL in the beginning of the TCP stream, and we extract it 
   using some tiny custom module - https://github.com/kaltura/nginx-srt-module/blob/master/src/ngx_stream_preread_str_module.c.

In my understanding, with this patch, we should be able to solve both scenarios in a more standard/elegant way.

Hope this help,

Thanks!

Eran

> > 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
> https://eur02.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmdounin.ru%2F&data=05%7C01%7C%7C71b5ceb7d09644384d1708da8ed932e4%7C0c503748de3f4e2597e26819d53a42b6%7C1%7C0%7C637979360772824220%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2FrPlS2oVVwuMtoNTJv3iobQsr2sgBWQHb71bmLYEd6Y%3D&reserved=0
> _______________________________________________
> nginx-devel mailing list -- nginx-devel at nginx.org To unsubscribe send an email to nginx-devel-leave at nginx.org
>