Extract PROXY Protocol v2 TLV SSL info for X-Forwarded-Proto#3945
Extract PROXY Protocol v2 TLV SSL info for X-Forwarded-Proto#3945ayruslore wants to merge 1 commit into
Conversation
ed73f87 to
f0a100e
Compare
|
|
||
| // proxySSLCache is a global, thread-safe cache of SSL information from PROXY protocol v2 TLVs | ||
| // keyed by "remoteAddr|localAddr" | ||
| var proxySSLCache sync.Map |
There was a problem hiding this comment.
Please add type information of key and value
szuecs
left a comment
szuecs
left a comment
There was a problem hiding this comment.
Tests are missing.
For example have a good test and then add failing tests also with the read/write deadlines.
33100b5 to
401a924
Compare
| if pconn, ok := conn.(*proxyproto.Conn); ok { | ||
| if header := pconn.ProxyHeader(); header != nil { | ||
| tlvs, err := header.TLVs() | ||
| if err == nil { |
There was a problem hiding this comment.
This condition makes this error hidden, if there is an error we just skip, is this the required flow?
if err != nil {
return conn, err
}| // Store using PROXY protocol's upstream addresses (what the PROXY header specifies) | ||
| if header.SourceAddr != nil { | ||
| upstreamClientKey := header.SourceAddr.String() | ||
| proxySSLCache.Store(upstreamClientKey, ssl) |
There was a problem hiding this comment.
I think this will be memory leak as we don't clean up, also it's not necessary since we already do the caching with actual connection addresses.
| func GetProxyProtoSSL(remoteAddr, localAddr string) (bool, bool) { | ||
| // First try to look up by upstream client IP from X-Forwarded-For or PROXY protocol | ||
| // Extract just the host part (without port) | ||
| clientHost := extractHost(remoteAddr) |
There was a problem hiding this comment.
Maybe I'm misunderstanding something in proxy protocol, do we need this? Why not using the fall back directly?
| localAddr := getLocalAddr(r) | ||
| ssl, ok := h.Lookup(remoteAddr, localAddr) | ||
|
|
||
| ctx := r.Context() |
There was a problem hiding this comment.
I think it's better to move this up and make the condition under Lookup function
There was a problem hiding this comment.
initialization can also be ctx := context.WithValue(r.Context(), contextKeyProxyProtoDirect{}, true)
There was a problem hiding this comment.
and remove this later in the function
ffa2850 to
c5714f1
Compare
When Proxy Protocol v2 is enabled, extract TLS information from TLV data and use it to set X-Forwarded-Proto header correctly, enabling HTTP-to-HTTPS redirect routes to work with NLB. Signed-off-by: speruri <surya.srikar.peruri@zalando.de>
c5714f1 to
1453c8d
Compare
|
trhis is not needed we can close this pr. |
3 participants
Summary
When Proxy Protocol v2 is enabled on NLB, extract TLS information from PROXY v2 TLV data to properly set X-Forwarded-Proto header. This enables HTTP-to-HTTPS redirect routes to work correctly.
Changes
Test plan