Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proxyprotocol: use github.com/pires/go-proxyproto #5915

Merged
merged 6 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ require (
github.com/google/uuid v1.3.1
github.com/klauspost/compress v1.17.0
github.com/klauspost/cpuid/v2 v2.2.5
github.com/mastercactapus/proxyprotocol v0.0.4
github.com/mholt/acmez v1.2.0
github.com/prometheus/client_golang v1.15.1
github.com/quic-go/quic-go v0.39.1
Expand Down Expand Up @@ -117,6 +116,7 @@ require (
github.com/mitchellh/copystructure v1.2.0 // indirect
github.com/mitchellh/go-ps v1.0.0 // indirect
github.com/mitchellh/reflectwalk v1.0.2 // indirect
github.com/pires/go-proxyproto v0.7.0
github.com/pkg/errors v0.9.1 // indirect
github.com/prometheus/client_model v0.4.0 // indirect
github.com/prometheus/common v0.42.0 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,6 @@ github.com/lyft/protoc-gen-validate v0.0.13/go.mod h1:XbGvPuh87YZc5TdIa2/I4pLk0Q
github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
github.com/manifoldco/promptui v0.9.0 h1:3V4HzJk1TtXW1MTZMP7mdlwbBpIinw3HztaIlYthEiA=
github.com/manifoldco/promptui v0.9.0/go.mod h1:ka04sppxSGFAtxX0qhlYQjISsg9mR4GWtQEhdbn6Pgg=
github.com/mastercactapus/proxyprotocol v0.0.4 h1:qSY75IZF30ZqIU9iW1ip3I7gTnm8wRAnGWqPxCBVgq0=
github.com/mastercactapus/proxyprotocol v0.0.4/go.mod h1:X8FRVEDZz9FkrIoL4QYTBF4Ka4ELwTv0sah0/5NxCPw=
github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
github.com/mattn/go-colorable v0.1.1/go.mod h1:FuOcm+DKB9mbwrcAfNl7/TZVBZ6rcnceauSikq3lYCQ=
github.com/mattn/go-colorable v0.1.6/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc=
Expand Down Expand Up @@ -433,6 +431,8 @@ github.com/performancecopilot/speed v3.0.0+incompatible/go.mod h1:/CLtqpZ5gBg1M9
github.com/peterbourgon/diskv/v3 v3.0.1 h1:x06SQA46+PKIUftmEujdwSEpIx8kR+M9eLYsUxeYveU=
github.com/pierrec/lz4 v1.0.2-0.20190131084431-473cd7ce01a1/go.mod h1:3/3N9NVKO0jef7pBehbT1qWhCMrIgbYNnFAZCqQ5LRc=
github.com/pierrec/lz4 v2.0.5+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY=
github.com/pires/go-proxyproto v0.7.0 h1:IukmRewDQFWC7kfnb66CSomk2q/seBuilHBYFwyq0Hs=
github.com/pires/go-proxyproto v0.7.0/go.mod h1:Vz/1JPY/OACxWGQNIRY2BeyDmpoaWmEP40O9LbuiFR4=
github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
Expand Down
29 changes: 13 additions & 16 deletions modules/caddyhttp/proxyprotocol/listenerwrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@
package proxyprotocol

import (
"fmt"
"net"
"time"

"github.com/mastercactapus/proxyprotocol"
goproxy "github.com/pires/go-proxyproto"

"github.com/caddyserver/caddy/v2"
)
Expand All @@ -39,31 +38,29 @@ type ListenerWrapper struct {
// allow/require PROXY headers from.
Allow []string `json:"allow,omitempty"`

rules []proxyprotocol.Rule
policies []goproxy.PolicyFunc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why store this in a slice when there can be one func at once? proxyproto takes a slice of IP address and return a function. There is no need to store it in a slice.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's my attempt at future-proofing. proxyproto has a selection of policies. We can also build more as long as they conform to the PolicyFunc type. My thinking is we could expand the implementation later to unionize policies.

}

// Provision sets up the listener wrapper.
func (pp *ListenerWrapper) Provision(ctx caddy.Context) error {
rules := make([]proxyprotocol.Rule, 0, len(pp.Allow))
for _, s := range pp.Allow {
_, n, err := net.ParseCIDR(s)
if len(pp.Allow) > 0 {
allowlist, err := goproxy.LaxWhiteListPolicy(pp.Allow)
if err != nil {
return fmt.Errorf("invalid subnet '%s': %w", s, err)
return err
}
rules = append(rules, proxyprotocol.Rule{
Timeout: time.Duration(pp.Timeout),
Subnet: n,
})
pp.policies = append(pp.policies, allowlist)
}

pp.rules = rules

return nil
}

// WrapListener adds PROXY protocol support to the listener.
func (pp *ListenerWrapper) WrapListener(l net.Listener) net.Listener {
pl := proxyprotocol.NewListener(l, time.Duration(pp.Timeout))
pl.SetFilter(pp.rules)
pl := &goproxy.Listener{
Listener: l,
ReadHeaderTimeout: time.Duration(pp.Timeout),
}
if len(pp.policies) > 0 {
pl.Policy = pp.policies[0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use a field of type PolicyFunc and assign it here.

}
return pl
}
42 changes: 20 additions & 22 deletions modules/caddyhttp/reverseproxy/httptransport.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
"strings"
"time"

"github.com/mastercactapus/proxyprotocol"
"github.com/pires/go-proxyproto"
"go.uber.org/zap"
"golang.org/x/net/http2"

Expand Down Expand Up @@ -207,44 +207,42 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e
if !ok {
return nil, fmt.Errorf("failed to get proxy protocol info from context")
}

// The src and dst have to be of the some address family. As we don't know the original
header := proxyproto.Header{
SourceAddr: &net.TCPAddr{
IP: proxyProtocolInfo.AddrPort.Addr().AsSlice(),
Port: int(proxyProtocolInfo.AddrPort.Port()),
Zone: proxyProtocolInfo.AddrPort.Addr().Zone(),
},
}
// The src and dst have to be of the same address family. As we don't know the original
// dst address (it's kind of impossible to know) and this address is generelly of very
mohammed90 marked this conversation as resolved.
Show resolved Hide resolved
// little interest, we just set it to all zeros.
var destIP net.IP
switch {
case proxyProtocolInfo.AddrPort.Addr().Is4():
destIP = net.IPv4zero
header.TransportProtocol = proxyproto.TCPv4
header.DestinationAddr = &net.TCPAddr{
IP: net.IPv4zero,
}
case proxyProtocolInfo.AddrPort.Addr().Is6():
destIP = net.IPv6zero
header.TransportProtocol = proxyproto.TCPv6
header.DestinationAddr = &net.TCPAddr{
IP: net.IPv6zero,
}
default:
return nil, fmt.Errorf("unexpected remote addr type in proxy protocol info")
}

// TODO: We should probably migrate away from net.IP to use netip.Addr,
// but due to the upstream dependency, we can't do that yet.
switch h.ProxyProtocol {
case "v1":
header := proxyprotocol.HeaderV1{
SrcIP: net.IP(proxyProtocolInfo.AddrPort.Addr().AsSlice()),
SrcPort: int(proxyProtocolInfo.AddrPort.Port()),
DestIP: destIP,
DestPort: 0,
}
header.Version = 1
caddyCtx.Logger().Debug("sending proxy protocol header v1", zap.Any("header", header))
_, err = header.WriteTo(conn)
case "v2":
header := proxyprotocol.HeaderV2{
Command: proxyprotocol.CmdProxy,
Src: &net.TCPAddr{IP: net.IP(proxyProtocolInfo.AddrPort.Addr().AsSlice()), Port: int(proxyProtocolInfo.AddrPort.Port())},
Dest: &net.TCPAddr{IP: destIP, Port: 0},
}
header.Version = 2
caddyCtx.Logger().Debug("sending proxy protocol header v2", zap.Any("header", header))
_, err = header.WriteTo(conn)
default:
return nil, fmt.Errorf("unexpected proxy protocol version")
}

_, err = header.WriteTo(conn)
if err != nil {
// identify this error as one that occurred during
// dialing, which can be important when trying to
Expand Down