Hi, On Tue, Sep 13, 2022 at 12:30:17AM +0300, Maxim Dounin wrote: > Hello! > > On Fri, Sep 09, 2022 at 07:46:58PM +0400, Roman Arutyunyan wrote: > > > On Mon, Sep 05, 2022 at 06:58:38PM +0300, Maxim Dounin wrote: > > > Hello! > > > > > > On Mon, Sep 05, 2022 at 05:23:18PM +0400, Roman Arutyunyan wrote: > > > > > > > Hi, > > > > > > > > On Mon, Sep 05, 2022 at 03:52:49AM +0300, Maxim Dounin wrote: > > > > > Hello! > > > > > > > > > > On Wed, Aug 31, 2022 at 07:52:15PM +0400, Roman Arutyunyan wrote: > > > > > > > > > > > # HG changeset patch > > > > > > # User Roman Arutyunyan <a...@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 known cases are these: > > > > > > > > - > > > > https://docs.aws.amazon.com/elasticloadbalancing/latest/network/load-balancer-target-groups.html#proxy-protocol > > > > - > > > > https://docs.microsoft.com/en-us/azure/private-link/private-link-service-overview#getting-connection-information-using-tcp-proxy-v2 > > > > - > > > > https://cloud.google.com/vpc/docs/configure-private-service-connect-producer#proxy-protocol > > > > > > > > The data may need further parsing, but it can be done in njs or perl. > > > > > > Thanks for the details. So, basically, it's about vendor-specific > > > endpoint IDs. > > > > > > > > > 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. > > > > With on-demand approach this is no longer an issue. > > > > > > > Also, it might worth the effort to actually add names for known > > > > > types instead or in addition to numbers. > > > > > > > > This is indeed a good idea and we have such plans as a further extenion > > > > of this > > > > work. One of the problems is however that the abovementioned TLV > > > > variables > > > > are specified in internal documents of AWS/Azure/GCP which are not > > > > standards. > > > > They can be changed anytime, while we have to maintain those variables > > > > in > > > > nginx. Also, raw variables give more flexibility in supporting less > > > > known TLVs. > > > > > > Of course I'm not suggesting to ditch raw variables, at least not > > > for unknown/non-standard values. But for known/standard values it > > > should be easy enough to provide alternative names for easier use, > > > probably with type-specific parsing. > > > > > > With on-demand parsing it would be trivial to support both > > > $proxy_protocol_tlv_alpn and $proxy_protocol_tlv_0x01. Further, > > > it will be trivial to support $proxy_protocol_tlv_aws_vpc_id while > > > still providing $proxy_protocol_tlv_0xea for raw data. > > [...] > > > # HG changeset patch > > # User Roman Arutyunyan <a...@nginx.com> > > # Date 1662718130 -14400 > > # Fri Sep 09 14:08:50 2022 +0400 > > # Node ID 832f6c96b26c3009640072e3f4b1f0bf43e644d0 > > # Parent 80714f1b0f597ce5e530e7274457450db4587fc9 > > Standard PROXY protocol v2 TLV variables. > > > > diff --git a/src/core/ngx_proxy_protocol.h b/src/core/ngx_proxy_protocol.h > > --- a/src/core/ngx_proxy_protocol.h > > +++ b/src/core/ngx_proxy_protocol.h > > @@ -13,7 +13,18 @@ > > #include <ngx_core.h> > > > > > > -#define NGX_PROXY_PROTOCOL_MAX_HEADER 107 > > +#define NGX_PROXY_PROTOCOL_MAX_HEADER 107 > > + > > +#define NGX_PROXY_PROTOCOL_TLV_ALPN 0x01 > > +#define NGX_PROXY_PROTOCOL_TLV_AUTHORITY 0x02 > > +#define NGX_PROXY_PROTOCOL_TLV_UNIQUE_ID 0x05 > > +#define NGX_PROXY_PROTOCOL_TLV_SSL 0x20 > > +#define NGX_PROXY_PROTOCOL_TLV_SSL_VERSION 0x21 > > +#define NGX_PROXY_PROTOCOL_TLV_SSL_CN 0x22 > > +#define NGX_PROXY_PROTOCOL_TLV_SSL_CIPHER 0x23 > > +#define NGX_PROXY_PROTOCOL_TLV_SSL_SIG_ALG 0x24 > > +#define NGX_PROXY_PROTOCOL_TLV_SSL_KEY_ALG 0x25 > > +#define NGX_PROXY_PROTOCOL_TLV_NETNS 0x30 > > > > > > struct ngx_proxy_protocol_s { > > @@ -25,6 +36,12 @@ struct ngx_proxy_protocol_s { > > }; > > > > > > +typedef struct { > > + u_char client; > > + u_char verify[4]; > > Note that these two are completely ignored.
Yes, currently they are. I thought about adding variables for them too, but then decided to postpone this. Do you think these have enough value to be added to this patch? > > +} ngx_proxy_protocol_tlv_ssl_t; > > + > > + > > u_char *ngx_proxy_protocol_read(ngx_connection_t *c, u_char *buf, > > u_char *last); > > u_char *ngx_proxy_protocol_write(ngx_connection_t *c, u_char *buf, > > diff --git a/src/http/ngx_http_variables.c b/src/http/ngx_http_variables.c > > --- a/src/http/ngx_http_variables.c > > +++ b/src/http/ngx_http_variables.c > > @@ -63,6 +63,10 @@ static ngx_int_t ngx_http_variable_proxy > > ngx_http_variable_value_t *v, uintptr_t data); > > static ngx_int_t > > ngx_http_variable_proxy_protocol_tlv_0x(ngx_http_request_t *r, > > ngx_http_variable_value_t *v, uintptr_t data); > > +static ngx_int_t ngx_http_variable_proxy_protocol_tlv(ngx_http_request_t > > *r, > > + ngx_http_variable_value_t *v, uintptr_t data); > > +static ngx_int_t ngx_http_variable_proxy_protocol_tlv_ssl( > > + ngx_http_request_t *r, ngx_http_variable_value_t *v, uintptr_t data); > > static ngx_int_t ngx_http_variable_server_addr(ngx_http_request_t *r, > > ngx_http_variable_value_t *v, uintptr_t data); > > static ngx_int_t ngx_http_variable_server_port(ngx_http_request_t *r, > > @@ -220,6 +224,42 @@ static ngx_http_variable_t ngx_http_cor > > ngx_http_variable_proxy_protocol_tlv_0x, > > 0, NGX_HTTP_VAR_PREFIX, 0 }, > > > > + { ngx_string("proxy_protocol_tlv_alpn"), NULL, > > + ngx_http_variable_proxy_protocol_tlv, > > + NGX_PROXY_PROTOCOL_TLV_ALPN, 0, 0 }, > > + > > + { ngx_string("proxy_protocol_tlv_authority"), NULL, > > + ngx_http_variable_proxy_protocol_tlv, > > + NGX_PROXY_PROTOCOL_TLV_AUTHORITY, 0, 0 }, > > + > > + { ngx_string("proxy_protocol_tlv_unique_id"), NULL, > > + ngx_http_variable_proxy_protocol_tlv, > > + NGX_PROXY_PROTOCOL_TLV_UNIQUE_ID, 0, 0 }, > > + > > + { ngx_string("proxy_protocol_tlv_ssl_version"), NULL, > > + ngx_http_variable_proxy_protocol_tlv_ssl, > > + NGX_PROXY_PROTOCOL_TLV_SSL_VERSION, 0, 0 }, > > + > > + { ngx_string("proxy_protocol_tlv_ssl_cn"), NULL, > > + ngx_http_variable_proxy_protocol_tlv_ssl, > > + NGX_PROXY_PROTOCOL_TLV_SSL_CN, 0, 0 }, > > + > > + { ngx_string("proxy_protocol_tlv_ssl_cipher"), NULL, > > + ngx_http_variable_proxy_protocol_tlv_ssl, > > + NGX_PROXY_PROTOCOL_TLV_SSL_CIPHER, 0, 0 }, > > + > > + { ngx_string("proxy_protocol_tlv_ssl_sig_alg"), NULL, > > + ngx_http_variable_proxy_protocol_tlv_ssl, > > + NGX_PROXY_PROTOCOL_TLV_SSL_SIG_ALG, 0, 0 }, > > + > > + { ngx_string("proxy_protocol_tlv_ssl_key_alg"), NULL, > > + ngx_http_variable_proxy_protocol_tlv_ssl, > > + NGX_PROXY_PROTOCOL_TLV_SSL_KEY_ALG, 0, 0 }, > > + > > + { ngx_string("proxy_protocol_tlv_netns"), NULL, > > + ngx_http_variable_proxy_protocol_tlv, > > + NGX_PROXY_PROTOCOL_TLV_NETNS, 0, 0 }, > > + > > Further, these provide no interface to support arbitrary SSL TLVs > (that is, something like "$proxy_protocol_tlv_ssl_0x10"). > > Overall, I tend to think it would be much easier / less intrusive > to keep all the TLV handling logic in the > src/core/ngx_proxy_protocol.c, including both arbitrary TLVs with > hexadecimal type and known named TLVs, and only provide a single > prefix variable in stream / http modules. > > That is, use "$proxy_protocol_tlv_" prefix variable and call a > function to obtain TLV with the rest of the variable name. > > This might imply slightly more complex/less efficient name parsing > logic for known TLVs, but I don't think it will be noticeable. On > the other hand, it will reduce clutter in the variables hash, and > therefore will save some resources in configurations where these > variables are not used. OK. Stream/HTTP part now looks simpler. -- Roman Arutyunyan
# HG changeset patch # User Roman Arutyunyan <a...@nginx.com> # Date 1663080928 -14400 # Tue Sep 13 18:55:28 2022 +0400 # Node ID 0736b29bfafac445fb8cf57312c3b7a2c3247a60 # Parent ba5cf8f73a2d0a3615565bf9545f3d65216a0530 PROXY protocol v2 TLV variables. The variables have prefix $proxy_protocol_tlv_ and are accessible by name and by type. Examples are: $proxy_protocol_tlv_0x01, $proxy_protocol_tlv_alpn. 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,24 @@ typedef struct { } ngx_proxy_protocol_inet6_addrs_t; +typedef struct { + u_char type; + u_char len[2]; +} ngx_proxy_protocol_tlv_t; + + +typedef struct { + u_char client; + u_char verify[4]; +} ngx_proxy_protocol_tlv_ssl_t; + + +typedef struct { + ngx_str_t name; + ngx_uint_t type; +} ngx_proxy_protocol_tlv_entry_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, @@ -48,6 +66,26 @@ static u_char *ngx_proxy_protocol_v2_rea u_char *last); +static ngx_proxy_protocol_tlv_entry_t ngx_proxy_protocol_tlv_entries[] = { + { ngx_string("alpn"), 0x01 }, + { ngx_string("authority"), 0x02 }, + { ngx_string("unique_id"), 0x05 }, + { ngx_string("ssl"), 0x20 }, + { ngx_string("netns"), 0x30 }, + { ngx_null_string, 0x00 } +}; + + +static ngx_proxy_protocol_tlv_entry_t ngx_proxy_protocol_tlv_ssl_entries[] = { + { ngx_string("version"), 0x21 }, + { ngx_string("cn"), 0x22 }, + { ngx_string("cipher"), 0x23 }, + { ngx_string("sig_alg"), 0x24 }, + { ngx_string("key_alg"), 0x25 }, + { ngx_null_string, 0x00 } +}; + + u_char * ngx_proxy_protocol_read(ngx_connection_t *c, u_char *buf, u_char *last) { @@ -412,11 +450,128 @@ ngx_proxy_protocol_v2_read(ngx_connectio &pp->src_addr, pp->src_port, &pp->dst_addr, pp->dst_port); if (buf < end) { - ngx_log_debug1(NGX_LOG_DEBUG_CORE, c->log, 0, - "PROXY protocol v2 %z bytes of tlv ignored", end - buf); + pp->tlvs.data = ngx_pnalloc(c->pool, end - buf); + if (pp->tlvs.data == NULL) { + return NULL; + } + + ngx_memcpy(pp->tlvs.data, buf, end - buf); + pp->tlvs.len = end - buf; } c->proxy_protocol = pp; return end; } + + +ngx_int_t +ngx_proxy_protocol_lookup_tlv(ngx_connection_t *c, ngx_str_t *tlvs, + ngx_uint_t type, ngx_str_t *value) +{ + u_char *p; + size_t n, len; + ngx_proxy_protocol_tlv_t *tlv; + + ngx_log_debug1(NGX_LOG_DEBUG_CORE, c->log, 0, + "PROXY protocol v2 lookup tlv:%02xi", type); + + p = tlvs->data; + n = tlvs->len; + + while (n) { + if (n < sizeof(ngx_proxy_protocol_tlv_t)) { + ngx_log_error(NGX_LOG_ERR, c->log, 0, "broken PROXY protocol TLV"); + return NGX_ERROR; + } + + tlv = (ngx_proxy_protocol_tlv_t *) p; + len = ngx_proxy_protocol_parse_uint16(tlv->len); + + p += sizeof(ngx_proxy_protocol_tlv_t); + n -= sizeof(ngx_proxy_protocol_tlv_t); + + if (n < len) { + ngx_log_error(NGX_LOG_ERR, c->log, 0, "broken PROXY protocol TLV"); + return NGX_ERROR; + } + + ngx_log_debug2(NGX_LOG_DEBUG_CORE, c->log, 0, + "PROXY protocol v2 tlv:0x%02xd len:%uz", tlv->type, len); + + if (tlv->type == type) { + value->data = p; + value->len = len; + return NGX_OK; + } + + p += len; + n -= len; + } + + return NGX_DECLINED; +} + + +ngx_int_t +ngx_proxy_protocol_get_tlv(ngx_connection_t *c, ngx_str_t *name, + ngx_str_t *value) +{ + u_char *p; + size_t n; + ngx_str_t ssl, *tlvs; + ngx_int_t rc, type; + ngx_proxy_protocol_tlv_entry_t *te; + + if (c->proxy_protocol == NULL) { + return NGX_DECLINED; + } + + ngx_log_debug1(NGX_LOG_DEBUG_CORE, c->log, 0, + "PROXY protocol v2 get tlv \"%V\"", name); + + te = ngx_proxy_protocol_tlv_entries; + tlvs = &c->proxy_protocol->tlvs; + + p = name->data; + n = name->len; + + if (n >= 4 && p[0] == 's' && p[1] == 's' && p[2] == 'l' && p[3] == '_') { + + rc = ngx_proxy_protocol_lookup_tlv(c, tlvs, 0x20, &ssl); + if (rc != NGX_OK) { + return rc; + } + + if (ssl.len < sizeof(ngx_proxy_protocol_tlv_ssl_t)) { + return NGX_ERROR; + } + + ssl.data += sizeof(ngx_proxy_protocol_tlv_ssl_t); + ssl.len -= sizeof(ngx_proxy_protocol_tlv_ssl_t); + + te = ngx_proxy_protocol_tlv_ssl_entries; + tlvs = &ssl; + + p += 4; + n -= 4; + } + + if (n >= 2 && p[0] == '0' && p[1] == 'x') { + + type = ngx_hextoi(p + 2, n - 2); + if (type == NGX_ERROR) { + return NGX_ERROR; + } + + return ngx_proxy_protocol_lookup_tlv(c, tlvs, type, value); + } + + for ( /* void */ ; te->type; te++) { + if (te->name.len == n && ngx_strncmp(te->name.data, p, n) == 0) { + return ngx_proxy_protocol_lookup_tlv(c, tlvs, te->type, value); + } + } + + return NGX_DECLINED; +} diff --git a/src/core/ngx_proxy_protocol.h b/src/core/ngx_proxy_protocol.h --- a/src/core/ngx_proxy_protocol.h +++ b/src/core/ngx_proxy_protocol.h @@ -21,6 +21,7 @@ struct ngx_proxy_protocol_s { ngx_str_t dst_addr; in_port_t src_port; in_port_t dst_port; + ngx_str_t tlvs; }; @@ -28,6 +29,10 @@ u_char *ngx_proxy_protocol_read(ngx_conn u_char *last); u_char *ngx_proxy_protocol_write(ngx_connection_t *c, u_char *buf, u_char *last); +ngx_int_t ngx_proxy_protocol_lookup_tlv(ngx_connection_t *c, ngx_str_t *tlvs, + ngx_uint_t type, ngx_str_t *value); +ngx_int_t ngx_proxy_protocol_get_tlv(ngx_connection_t *c, ngx_str_t *name, + ngx_str_t *value); #endif /* _NGX_PROXY_PROTOCOL_H_INCLUDED_ */ diff --git a/src/http/ngx_http_variables.c b/src/http/ngx_http_variables.c --- a/src/http/ngx_http_variables.c +++ b/src/http/ngx_http_variables.c @@ -61,6 +61,8 @@ static ngx_int_t ngx_http_variable_proxy ngx_http_variable_value_t *v, uintptr_t data); static ngx_int_t ngx_http_variable_proxy_protocol_port(ngx_http_request_t *r, ngx_http_variable_value_t *v, uintptr_t data); +static ngx_int_t ngx_http_variable_proxy_protocol_tlv(ngx_http_request_t *r, + ngx_http_variable_value_t *v, uintptr_t data); static ngx_int_t ngx_http_variable_server_addr(ngx_http_request_t *r, ngx_http_variable_value_t *v, uintptr_t data); static ngx_int_t ngx_http_variable_server_port(ngx_http_request_t *r, @@ -214,6 +216,10 @@ static ngx_http_variable_t ngx_http_cor ngx_http_variable_proxy_protocol_port, offsetof(ngx_proxy_protocol_t, dst_port), 0, 0 }, + { ngx_string("proxy_protocol_tlv_"), NULL, + ngx_http_variable_proxy_protocol_tlv, + 0, NGX_HTTP_VAR_PREFIX, 0 }, + { ngx_string("server_addr"), NULL, ngx_http_variable_server_addr, 0, 0, 0 }, { ngx_string("server_port"), NULL, ngx_http_variable_server_port, 0, 0, 0 }, @@ -1387,6 +1393,39 @@ ngx_http_variable_proxy_protocol_port(ng static ngx_int_t +ngx_http_variable_proxy_protocol_tlv(ngx_http_request_t *r, + ngx_http_variable_value_t *v, uintptr_t data) +{ + ngx_str_t *name = (ngx_str_t *) data; + + ngx_int_t rc; + ngx_str_t tlv, value; + + tlv.len = name->len - (sizeof("proxy_protocol_tlv_") - 1); + tlv.data = name->data + sizeof("proxy_protocol_tlv_") - 1; + + rc = ngx_proxy_protocol_get_tlv(r->connection, &tlv, &value); + + if (rc == NGX_ERROR) { + return NGX_ERROR; + } + + if (rc == NGX_DECLINED) { + v->not_found = 1; + return NGX_OK; + } + + v->len = value.len; + v->valid = 1; + v->no_cacheable = 0; + v->not_found = 0; + v->data = value.data; + + return NGX_OK; +} + + +static ngx_int_t ngx_http_variable_server_addr(ngx_http_request_t *r, ngx_http_variable_value_t *v, uintptr_t data) { diff --git a/src/stream/ngx_stream_variables.c b/src/stream/ngx_stream_variables.c --- a/src/stream/ngx_stream_variables.c +++ b/src/stream/ngx_stream_variables.c @@ -23,6 +23,8 @@ static ngx_int_t ngx_stream_variable_pro ngx_stream_session_t *s, ngx_stream_variable_value_t *v, uintptr_t data); static ngx_int_t ngx_stream_variable_proxy_protocol_port( ngx_stream_session_t *s, ngx_stream_variable_value_t *v, uintptr_t data); +static ngx_int_t ngx_stream_variable_proxy_protocol_tlv( + ngx_stream_session_t *s, ngx_stream_variable_value_t *v, uintptr_t data); static ngx_int_t ngx_stream_variable_server_addr(ngx_stream_session_t *s, ngx_stream_variable_value_t *v, uintptr_t data); static ngx_int_t ngx_stream_variable_server_port(ngx_stream_session_t *s, @@ -79,6 +81,10 @@ static ngx_stream_variable_t ngx_stream ngx_stream_variable_proxy_protocol_port, offsetof(ngx_proxy_protocol_t, dst_port), 0, 0 }, + { ngx_string("proxy_protocol_tlv_"), NULL, + ngx_stream_variable_proxy_protocol_tlv, + 0, NGX_STREAM_VAR_PREFIX, 0 }, + { ngx_string("server_addr"), NULL, ngx_stream_variable_server_addr, 0, 0, 0 }, @@ -622,6 +628,39 @@ ngx_stream_variable_proxy_protocol_port( static ngx_int_t +ngx_stream_variable_proxy_protocol_tlv(ngx_stream_session_t *s, + ngx_stream_variable_value_t *v, uintptr_t data) +{ + ngx_str_t *name = (ngx_str_t *) data; + + ngx_int_t rc; + ngx_str_t tlv, value; + + tlv.len = name->len - (sizeof("proxy_protocol_tlv_") - 1); + tlv.data = name->data + sizeof("proxy_protocol_tlv_") - 1; + + rc = ngx_proxy_protocol_get_tlv(s->connection, &tlv, &value); + + if (rc == NGX_ERROR) { + return NGX_ERROR; + } + + if (rc == NGX_DECLINED) { + v->not_found = 1; + return NGX_OK; + } + + v->len = value.len; + v->valid = 1; + v->no_cacheable = 0; + v->not_found = 0; + v->data = value.data; + + return NGX_OK; +} + + +static ngx_int_t ngx_stream_variable_server_addr(ngx_stream_session_t *s, ngx_stream_variable_value_t *v, uintptr_t data) {
_______________________________________________ nginx-devel mailing list -- nginx-devel@nginx.org To unsubscribe send an email to nginx-devel-le...@nginx.org