Skip to content

Commit b57da4a

Browse files
author
speruri
committed
Address review comments: improve docs, clean up comments, add tests
- Add links to PROXY protocol specification in net/proxyproto.go - Add type information for proxySSLCache in comments - Improve tlvExtractorListener documentation - Remove unnecessary inline comments - Add comprehensive tests for TLV SSL extraction with and without SSL TLV Signed-off-by: speruri <surya.srikar.peruri@zalando.de>
1 parent f0a100e commit b57da4a

4 files changed

Lines changed: 130 additions & 28 deletions

File tree

‎net/headers.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,15 +110,13 @@ func (h *ForwardedHeaders) Set(req *http.Request) {
110110
}
111111

112112
if h.Proto == "auto" {
113-
// Check if we have PROXY protocol v2 SSL information
114113
if ssl, ok := ProxyProtoSSLFromContext(req.Context()); ok {
115114
if ssl {
116115
req.Header.Set("X-Forwarded-Proto", "https")
117116
} else {
118117
req.Header.Set("X-Forwarded-Proto", "http")
119118
}
120119
} else if req.TLS != nil {
121-
// Fall back to checking TLS state
122120
req.Header.Set("X-Forwarded-Proto", "https")
123121
} else {
124122
req.Header.Set("X-Forwarded-Proto", "http")

‎net/proxyproto.go‎

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,58 +5,47 @@ import (
55
"encoding/binary"
66
)
77

8-
// PP2_TYPE_SSL is the PROXY protocol v2 TLV type for SSL information
8+
// PP2_TYPE_SSL is the PROXY protocol v2 TLV type for SSL information.
9+
// See https://www.haproxy.org/download/2.3/doc/proxy-protocol.txt section "TLV Format"
910
const PP2_TYPE_SSL = 0x20
1011

11-
// ParseProxyProtocolV2Header parses PROXY protocol v2 header to extract SSL information
12-
// Returns true if the header indicates an SSL/TLS connection
12+
// ParseProxyProtocolV2Header parses PROXY protocol v2 header to extract SSL information.
13+
// See https://www.haproxy.org/download/2.3/doc/proxy-protocol.txt for the protocol specification.
14+
// Returns true if the header indicates an SSL/TLS connection.
1315
func ParseProxyProtocolV2Header(data []byte) bool {
14-
// Minimum v2 header is 16 bytes
1516
if len(data) < 16 {
1617
return false
1718
}
1819

19-
// Check for PROXY protocol v2 signature: \x0D\x0A\x0D\x0A\x00\x0D\x0A\x51\x55\x49\x54\x0A
2020
sig := []byte{0x0D, 0x0A, 0x0D, 0x0A, 0x00, 0x0D, 0x0A, 0x51, 0x55, 0x49, 0x54, 0x0A}
2121
if !bytes.HasPrefix(data, sig) {
2222
return false
2323
}
2424

25-
// Get version from byte 12
2625
verCmd := data[12]
2726
version := (verCmd >> 4) & 0x0F
28-
29-
// We're looking for version 2
3027
if version != 2 {
3128
return false
3229
}
3330

34-
// Get length of the rest of the header (2 bytes at offset 14-15)
3531
len16 := binary.BigEndian.Uint16(data[14:16])
36-
37-
// TLVs start after the 16-byte base header
3832
tlvStart := 16
3933
tlvEnd := tlvStart + int(len16)
4034

41-
// Bounds check
4235
if tlvEnd > len(data) {
4336
return false
4437
}
4538

46-
// Parse TLVs to find SSL information
4739
pos := tlvStart
4840
for pos < tlvEnd {
49-
// Each TLV has: type (1 byte), length (2 bytes), value (variable)
5041
if pos+3 > tlvEnd {
5142
break
5243
}
5344

5445
tlvType := data[pos]
5546
tlvLen := binary.BigEndian.Uint16(data[pos+1 : pos+3])
5647

57-
// Check for SSL TLV (0x20)
5848
if tlvType == PP2_TYPE_SSL {
59-
// SSL TLV found - this indicates TLS connection
6049
return true
6150
}
6251

‎proxylistener/proxylistener.go‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ import (
1111
snet "github.com/zalando/skipper/net"
1212
)
1313

14-
// proxySSLCache is a global, thread-safe cache of SSL information from PROXY protocol v2 TLVs
15-
// keyed by "remoteAddr|localAddr"
14+
// proxySSLCache is a global, thread-safe cache of SSL information from PROXY protocol v2 TLVs.
15+
// Keys are strings in the format "remoteAddr|localAddr", values are bools indicating SSL/TLS presence.
1616
var proxySSLCache sync.Map
1717

1818
const (
@@ -83,11 +83,13 @@ func NewListener(opt Options) (net.Listener, error) {
8383
ConnPolicy: policyLogic,
8484
}
8585

86-
// Wrap the proxy listener to extract TLV data
8786
return &tlvExtractorListener{wrapped: pl}, nil
8887
}
8988

90-
// tlvExtractorListener wraps a proxyproto.Listener and extracts TLV SSL information
89+
// tlvExtractorListener wraps a net.Listener and extracts SSL information from
90+
// ProxyHeaders if the accepted net.Conn is a proxyproto.Conn.
91+
// The SSL information is stored in an internal cache and can be retrieved
92+
// using GetProxyProtoSSL.
9193
type tlvExtractorListener struct {
9294
wrapped net.Listener
9395
}
@@ -98,21 +100,17 @@ func (tl *tlvExtractorListener) Accept() (net.Conn, error) {
98100
return conn, err
99101
}
100102

101-
// Try to extract TLV SSL information from proxyproto.Conn
102103
if pconn, ok := conn.(*proxyproto.Conn); ok {
103104
if header := pconn.ProxyHeader(); header != nil {
104-
// Try to extract SSL TLV
105105
tlvs, err := header.TLVs()
106106
if err == nil {
107107
ssl := hasTLVSSL(tlvs)
108-
// Store SSL state keyed by connection addresses
109108
key := conn.RemoteAddr().String() + "|" + conn.LocalAddr().String()
110109
proxySSLCache.Store(key, ssl)
111110
}
112111
}
113112
}
114113

115-
// Wrap the connection to clean up the cache on close
116114
return &tlvCacheCleanupConn{conn: conn}, nil
117115
}
118116

@@ -124,7 +122,7 @@ func (tl *tlvExtractorListener) Addr() net.Addr {
124122
return tl.wrapped.Addr()
125123
}
126124

127-
// tlvCacheCleanupConn wraps a net.Conn and cleans up the TLV cache on close
125+
// tlvCacheCleanupConn wraps a net.Conn and cleans up the TLV cache on close.
128126
type tlvCacheCleanupConn struct {
129127
conn net.Conn
130128
}
@@ -138,7 +136,6 @@ func (c *tlvCacheCleanupConn) Write(b []byte) (int, error) {
138136
}
139137

140138
func (c *tlvCacheCleanupConn) Close() error {
141-
// Clean up cache entry
142139
key := c.conn.RemoteAddr().String() + "|" + c.conn.LocalAddr().String()
143140
proxySSLCache.Delete(key)
144141
return c.conn.Close()

‎proxylistener/proxylistener_test.go‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,3 +529,121 @@ func TestProxyListenerWithHttpClient(t *testing.T) {
529529
}
530530

531531
}
532+
533+
func TestProxyProtoTLVSSLExtraction(t *testing.T) {
534+
l := createTestListener()
535+
defer l.Close()
536+
537+
addr := l.Addr().String()
538+
539+
for _, tt := range []struct {
540+
name string
541+
includeSSLTLV bool
542+
expectedSSLFlag bool
543+
wantErr bool
544+
}{
545+
{
546+
name: "PROXY protocol v2 with SSL TLV",
547+
includeSSLTLV: true,
548+
expectedSSLFlag: true,
549+
wantErr: false,
550+
},
551+
{
552+
name: "PROXY protocol v2 without SSL TLV",
553+
includeSSLTLV: false,
554+
expectedSSLFlag: false,
555+
wantErr: false,
556+
},
557+
} {
558+
t.Run(tt.name, func(t *testing.T) {
559+
client := createProxyClientWithSSLTLV(addr, "127.0.0.1", 8080, tt.includeSSLTLV)
560+
561+
waitShutdownCH := make(chan struct{})
562+
srv := &http.Server{
563+
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
564+
w.WriteHeader(http.StatusOK)
565+
}),
566+
}
567+
568+
go func() {
569+
time.Sleep(100 * time.Millisecond)
570+
if err := srv.Shutdown(context.Background()); err != nil {
571+
t.Logf("Failed to graceful shutdown: %v", err)
572+
}
573+
close(waitShutdownCH)
574+
}()
575+
576+
waitServeCH := make(chan struct{})
577+
go func() {
578+
if err := srv.Serve(l); err != http.ErrServerClosed {
579+
t.Logf("Serve failed: %v", err)
580+
}
581+
close(waitServeCH)
582+
}()
583+
584+
req, err := http.NewRequest("GET", "http://"+addr+"/", nil)
585+
if err != nil {
586+
t.Fatalf("Failed to create request: %v", err)
587+
}
588+
589+
rsp, err := client.Do(req)
590+
if err != nil && !tt.wantErr {
591+
t.Fatalf("Failed to get response: %v", err)
592+
}
593+
if !tt.wantErr && rsp.StatusCode != http.StatusOK {
594+
t.Fatalf("Failed to get 200, got %d", rsp.StatusCode)
595+
}
596+
597+
<-waitShutdownCH
598+
<-waitServeCH
599+
})
600+
}
601+
}
602+
603+
func createProxyClientWithSSLTLV(proxyAddr, destAddr string, destPort int, includeSSLTLV bool) *http.Client {
604+
dialer := &net.Dialer{
605+
Timeout: 5 * time.Second,
606+
}
607+
608+
return &http.Client{
609+
Transport: &http.Transport{
610+
DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
611+
conn, err := dialer.Dial(network, proxyAddr)
612+
if err != nil {
613+
return nil, err
614+
}
615+
616+
header := &proxyproto.Header{
617+
Version: 2,
618+
Command: proxyproto.PROXY,
619+
TransportProtocol: proxyproto.TCPv4,
620+
SourceAddr: &net.TCPAddr{
621+
IP: net.ParseIP(clientIP),
622+
Port: clientPort,
623+
},
624+
DestinationAddr: &net.TCPAddr{
625+
IP: net.ParseIP(destAddr),
626+
Port: destPort,
627+
},
628+
}
629+
630+
if includeSSLTLV {
631+
tlvs := []proxyproto.TLV{
632+
{
633+
Type: 0x20,
634+
Value: []byte{0x01},
635+
},
636+
}
637+
header.SetTLVs(tlvs)
638+
}
639+
640+
if _, err := header.WriteTo(conn); err != nil {
641+
conn.Close()
642+
return nil, err
643+
}
644+
645+
return conn, nil
646+
},
647+
},
648+
}
649+
}

0 commit comments

Comments
 (0)