Skip to content

Commit

Permalink
Fix warnings, possible linter failures and typos (#223)
Browse files Browse the repository at this point in the history 
* Fix warnings: unhandled errors

* Fix warnings: redundant type conversions

* Fix warnings: errors comparison and type conversion

* Fix warnings: redundant function arguments

* Fix warnings: built-in function name collisions

* Fix warnings: typos in comments

* Fix warnings: potential nil pointer dereference

* Fix warnings: mixed value and pointer receivers

* Fix warnings: remove deprecated weekrand.Seed() call

* Fix possible linter failures: imports order

* Fix README: caddyfile support

* Fix README: sort matchers and handlers by name

* Fix warnings: one more typo in comments

* Fix warnings: potential nil pointer dereference, optimized
  • Loading branch information
@vnxme
vnxme authored Aug 8, 2024
1 parent b79350a commit ff5d983
Show file tree
Hide file tree
Showing 39 changed files with 212 additions and 183 deletions.
14 changes: 7 additions & 7 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,29 +32,29 @@ Current matchers:

- **layer4.matchers.clock** - matches connections on the time they are wrapped/matched.
- **layer4.matchers.http** - matches connections that start with HTTP requests. In addition, any [`http.matchers` modules](https://caddyserver.com/docs/modules/) can be used for matching on HTTP-specific properties of requests, such as header or path. Note that only the first request of each connection can be used for matching.
- **layer4.matchers.tls** - matches connections that start with TLS handshakes. In addition, any [`tls.handshake_match` modules](https://caddyserver.com/docs/modules/) can be used for matching on TLS-specific properties of the ClientHello, such as ServerName (SNI).
- **layer4.matchers.ssh** - matches connections that look like SSH connections.
- **layer4.matchers.postgres** - matches connections that look like Postgres connections.
- **layer4.matchers.remote_ip** - matches connections based on remote IP (or CIDR range).
- **layer4.matchers.local_ip** - matches connections based on local IP (or CIDR range).
- **layer4.matchers.not** - matches connections that aren't matched by inner matcher sets.
- **layer4.matchers.postgres** - matches connections that look like Postgres connections.
- **layer4.matchers.proxy_protocol** - matches connections that start with [HAPROXY proxy protocol](https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt).
- **layer4.matchers.rdp** - matches connections that look like [RDP](https://winprotocoldoc.blob.core.windows.net/productionwindowsarchives/MS-RDPBCGR/%5BMS-RDPBCGR%5D.pdf).
- **layer4.matchers.regexp** - matches connections that have the first packet bytes matching a regular expression.
- **layer4.matchers.remote_ip** - matches connections based on remote IP (or CIDR range).
- **layer4.matchers.socks4** - matches connections that look like [SOCKSv4](https://www.openssh.com/txt/socks4.protocol).
- **layer4.matchers.socks5** - matches connections that look like [SOCKSv5](https://www.rfc-editor.org/rfc/rfc1928.html).
- **layer4.matchers.ssh** - matches connections that look like SSH connections.
- **layer4.matchers.tls** - matches connections that start with TLS handshakes. In addition, any [`tls.handshake_match` modules](https://caddyserver.com/docs/modules/) can be used for matching on TLS-specific properties of the ClientHello, such as ServerName (SNI).
- **layer4.matchers.xmpp** - matches connections that look like [XMPP](https://xmpp.org/about/technology-overview/).

Current handlers:

- **layer4.handlers.echo** - An echo server.
- **layer4.handlers.proxy** - Powerful layer 4 proxy, capable of multiple upstreams (with load balancing and health checks) and establishing new TLS connections to backends. Optionally supports sending the [HAProxy proxy protocol](https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt).
- **layer4.handlers.proxy_protocol** - Accepts the [HAPROXY proxy protocol](https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt) on the receiving side.
- **layer4.handlers.socks5** - Handles [SOCKSv5](https://www.rfc-editor.org/rfc/rfc1928.html) proxy protocol connections.
- **layer4.handlers.subroute** - Implements recursion logic, i.e. allows to match and handle already matched connections.
- **layer4.handlers.tee** - Branches the handling of a connection into a concurrent handler chain.
- **layer4.handlers.throttle** - Throttle connections to simulate slowness and latency.
- **layer4.handlers.tls** - TLS termination.
- **layer4.handlers.proxy_protocol** - Accepts the [HAPROXY proxy protocol](https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt) on the receiving side.
- **layer4.handlers.socks5** - Handles [SOCKSv5](https://www.rfc-editor.org/rfc/rfc1928.html) proxy protocol connections.

Like the `http` app, some handlers are "terminal" meaning that they don't call the next handler in the chain. For example: `echo` and `proxy` are terminal handlers because they consume the client's input.

Expand All @@ -75,7 +75,7 @@ Alternatively, to hack on the plugin code, you can clone it down, then build and

## Writing config

Since this app does not support Caddyfile (yet?), you will have to use Caddy's native JSON format to configure it. I highly recommend [this caddy-json-schema plugin by @abiosoft](https://github.com/abiosoft/caddy-json-schema) which can give you auto-complete and documentation right in your editor as you write your config!
This app supports Caddyfile, but you may also use Caddy's native JSON format to configure it. I highly recommend [this caddy-json-schema plugin by @abiosoft](https://github.com/abiosoft/caddy-json-schema) which can give you auto-complete and documentation right in your editor as you write your config!

See below for some examples to help you get started.

Expand Down
1 change: 1 addition & 0 deletions integration/caddyfile_adapt_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"testing"

"github.com/caddyserver/caddy/v2/caddytest"

_ "github.com/mholt/caddy-l4"
)

Expand Down
10 changes: 5 additions & 5 deletions layer4/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
)

func init() {
caddy.RegisterModule(App{})
caddy.RegisterModule(&App{})
}

// App is a Caddy app that operates closest to layer 4 of the OSI model.
Expand All @@ -40,7 +40,7 @@ type App struct {
}

// CaddyModule returns the Caddy module information.
func (App) CaddyModule() caddy.ModuleInfo {
func (*App) CaddyModule() caddy.ModuleInfo {
return caddy.ModuleInfo{
ID: "layer4",
New: func() caddy.Module { return new(App) },
Expand Down Expand Up @@ -76,11 +76,11 @@ func (a *App) Start() error {
case net.Listener:
a.listeners = append(a.listeners, ln)
lnAddr = caddy.JoinNetworkAddress(ln.Addr().Network(), ln.Addr().String(), "")
go s.serve(ln)
go func() { _ = s.serve(ln) }()
case net.PacketConn:
a.packetConns = append(a.packetConns, ln)
lnAddr = caddy.JoinNetworkAddress(ln.LocalAddr().Network(), ln.LocalAddr().String(), "")
go s.servePacket(ln)
go func() { _ = s.servePacket(ln) }()
}
s.logger.Debug("listening", zap.String("address", lnAddr))
}
Expand All @@ -90,7 +90,7 @@ func (a *App) Start() error {
}

// Stop stops the servers and closes all listeners.
func (a App) Stop() error {
func (a *App) Stop() error {
for _, pc := range a.packetConns {
err := pc.Close()
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion layer4/connection.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func (cx *Connection) GetVar(key string) interface{} {
}

// MatchingBytes returns all bytes currently available for matching. This is only intended for reading.
// Do not write into the slice. It's a view of the internal buffer and you will likely mess up the connection.
// Do not write into the slice. It's a view of the internal buffer, and you will likely mess up the connection.
func (cx *Connection) MatchingBytes() []byte {
return cx.buf[cx.offset:]
}
Expand Down
10 changes: 5 additions & 5 deletions layer4/connection_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,20 @@ import (

func TestConnection_FreezeAndUnfreeze(t *testing.T) {
in, out := net.Pipe()
defer in.Close()
defer out.Close()
defer func() { _ = in.Close() }()
defer func() { _ = out.Close() }()

cx := WrapConnection(out, []byte{}, zap.NewNop())
defer cx.Close()
defer func() { _ = cx.Close() }()

matcherData := []byte("foo")
consumeData := []byte("bar")

buf := make([]byte, len(matcherData))

go func() {
in.Write(matcherData)
in.Write(consumeData)
_, _ = in.Write(matcherData)
_, _ = in.Write(consumeData)
}()

// prefetch like server handler would
Expand Down
15 changes: 8 additions & 7 deletions layer4/listener.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
)

func init() {
caddy.RegisterModule(ListenerWrapper{})
caddy.RegisterModule(&ListenerWrapper{})
}

// ListenerWrapper is a Caddy module that wraps App as a listener wrapper, it doesn't support udp.
Expand All @@ -33,7 +33,7 @@ type ListenerWrapper struct {
}

// CaddyModule returns the Caddy module information.
func (ListenerWrapper) CaddyModule() caddy.ModuleInfo {
func (*ListenerWrapper) CaddyModule() caddy.ModuleInfo {
return caddy.ModuleInfo{
ID: "caddy.listeners.layer4",
New: func() caddy.Module { return new(ListenerWrapper) },
Expand Down Expand Up @@ -126,7 +126,8 @@ type listener struct {
func (l *listener) loop() {
for {
conn, err := l.Listener.Accept()
if nerr, ok := err.(net.Error); ok && nerr.Temporary() {
var nerr net.Error
if errors.As(err, &nerr) && nerr.Temporary() {
l.logger.Error("temporary error accepting connection", zap.Error(err))
continue
}
Expand All @@ -145,7 +146,7 @@ func (l *listener) loop() {
close(l.connChan)
}()
for conn := range l.connChan {
conn.Close()
_ = conn.Close()
}
}

Expand All @@ -156,8 +157,8 @@ func (l *listener) handle(conn net.Conn) {
var err error
defer func() {
l.wg.Done()
if err != errHijacked {
conn.Close()
if !errors.Is(err, errHijacked) {
_ = conn.Close()
}
}()

Expand All @@ -171,7 +172,7 @@ func (l *listener) handle(conn net.Conn) {
start := time.Now()
err = l.compiledRoute.Handle(cx)
duration := time.Since(start)
if err != nil && err != errHijacked {
if err != nil && !errors.Is(err, errHijacked) {
l.logger.Error("handling connection", zap.Error(err))
}

Expand Down
34 changes: 17 additions & 17 deletions layer4/matchers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ import (
)

func init() {
caddy.RegisterModule(MatchRemoteIP{})
caddy.RegisterModule(MatchLocalIP{})
caddy.RegisterModule(MatchNot{})
caddy.RegisterModule(&MatchRemoteIP{})
caddy.RegisterModule(&MatchLocalIP{})
caddy.RegisterModule(&MatchNot{})
}

// ConnMatcher is a type that can match a connection.
Expand Down Expand Up @@ -83,14 +83,14 @@ type MatcherSets []MatcherSet
// AnyMatch returns true if the connection matches any of the matcher sets
// in mss or if there are no matchers, in which case the request always
// matches. Any error terminates matching.
func (mss MatcherSets) AnyMatch(cx *Connection) (matched bool, err error) {
for _, m := range mss {
func (mss *MatcherSets) AnyMatch(cx *Connection) (matched bool, err error) {
for _, m := range *mss {
matched, err = m.Match(cx)
if matched || err != nil {
return
}
}
matched = len(mss) == 0
matched = len(*mss) == 0
return
}

Expand All @@ -117,7 +117,7 @@ type MatchRemoteIP struct {
}

// CaddyModule returns the Caddy module information.
func (MatchRemoteIP) CaddyModule() caddy.ModuleInfo {
func (*MatchRemoteIP) CaddyModule() caddy.ModuleInfo {
return caddy.ModuleInfo{
ID: "layer4.matchers.remote_ip",
New: func() caddy.Module { return new(MatchRemoteIP) },
Expand All @@ -134,7 +134,7 @@ func (m *MatchRemoteIP) Provision(_ caddy.Context) (err error) {
}

// Match returns true if the connection is from one of the designated IP ranges.
func (m MatchRemoteIP) Match(cx *Connection) (bool, error) {
func (m *MatchRemoteIP) Match(cx *Connection) (bool, error) {
clientIP, err := m.getRemoteIP(cx)
if err != nil {
return false, fmt.Errorf("getting remote IP: %v", err)
Expand All @@ -147,7 +147,7 @@ func (m MatchRemoteIP) Match(cx *Connection) (bool, error) {
return false, nil
}

func (m MatchRemoteIP) getRemoteIP(cx *Connection) (netip.Addr, error) {
func (m *MatchRemoteIP) getRemoteIP(cx *Connection) (netip.Addr, error) {
remote := cx.Conn.RemoteAddr().String()

ipStr, _, err := net.SplitHostPort(remote)
Expand All @@ -164,7 +164,7 @@ func (m MatchRemoteIP) getRemoteIP(cx *Connection) (netip.Addr, error) {

// UnmarshalCaddyfile sets up the MatchRemoteIP from Caddyfile tokens. Syntax:
//
// ip <ranges...>
// remote_ip <ranges...>
func (m *MatchRemoteIP) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
_, wrapper := d.Next(), d.Val() // consume wrapper name

Expand Down Expand Up @@ -198,15 +198,15 @@ type MatchLocalIP struct {
}

// CaddyModule returns the Caddy module information.
func (MatchLocalIP) CaddyModule() caddy.ModuleInfo {
func (*MatchLocalIP) CaddyModule() caddy.ModuleInfo {
return caddy.ModuleInfo{
ID: "layer4.matchers.local_ip",
New: func() caddy.Module { return new(MatchLocalIP) },
}
}

// Provision parses m's IP ranges, either from IP or CIDR expressions.
func (m *MatchLocalIP) Provision(ctx caddy.Context) error {
func (m *MatchLocalIP) Provision(_ caddy.Context) error {
ipnets, err := ParseNetworks(m.Ranges)
if err != nil {
return err
Expand All @@ -216,7 +216,7 @@ func (m *MatchLocalIP) Provision(ctx caddy.Context) error {
}

// Match returns true if the connection is from one of the designated IP ranges.
func (m MatchLocalIP) Match(cx *Connection) (bool, error) {
func (m *MatchLocalIP) Match(cx *Connection) (bool, error) {
localIP, err := m.getLocalIP(cx)
if err != nil {
return false, fmt.Errorf("getting local IP: %v", err)
Expand All @@ -229,7 +229,7 @@ func (m MatchLocalIP) Match(cx *Connection) (bool, error) {
return false, nil
}

func (m MatchLocalIP) getLocalIP(cx *Connection) (netip.Addr, error) {
func (m *MatchLocalIP) getLocalIP(cx *Connection) (netip.Addr, error) {
remote := cx.Conn.LocalAddr().String()

ipStr, _, err := net.SplitHostPort(remote)
Expand Down Expand Up @@ -300,7 +300,7 @@ type MatchNot struct {
}

// CaddyModule implements caddy.Module.
func (MatchNot) CaddyModule() caddy.ModuleInfo {
func (*MatchNot) CaddyModule() caddy.ModuleInfo {
return caddy.ModuleInfo{
ID: "layer4.matchers.not",
New: func() caddy.Module { return new(MatchNot) },
Expand All @@ -315,7 +315,7 @@ func (m *MatchNot) UnmarshalJSON(data []byte) error {

// MarshalJSON satisfies json.Marshaler by marshaling
// m's raw matcher sets.
func (m MatchNot) MarshalJSON() ([]byte, error) {
func (m *MatchNot) MarshalJSON() ([]byte, error) {
return json.Marshal(m.MatcherSetsRaw)
}

Expand All @@ -338,7 +338,7 @@ func (m *MatchNot) Provision(ctx caddy.Context) error {
// Match returns true if r matches m. Since this matcher negates
// the embedded matchers, false is returned if any of its matcher
// sets return true.
func (m MatchNot) Match(r *Connection) (bool, error) {
func (m *MatchNot) Match(r *Connection) (bool, error) {
for _, ms := range m.MatcherSets {
match, err := ms.Match(r)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion layer4/matchers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type provisionableMatcher interface {
}

func provision(in provisionableMatcher) ConnMatcher {
in.Provision(caddy.Context{})
_ = in.Provision(caddy.Context{})
return in
}
func TestNotMatcher(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion layer4/routes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type Route struct {
// Matchers define the conditions upon which to execute the handlers.
// All matchers within the same set must match, and at least one set
// must match; in other words, matchers are AND'ed together within a
// set, but multiple sets are OR'ed together. No matchers matches all.
// set, but multiple sets are OR'ed together. No matchers match all.
MatcherSetsRaw []caddy.ModuleMap `json:"match,omitempty" caddy:"namespace=layer4.matchers"`

// Handlers define the behavior for handling the stream. They are
Expand Down
6 changes: 3 additions & 3 deletions layer4/routes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ func TestMatchingTimeoutWorks(t *testing.T) {
}))

in, out := net.Pipe()
defer in.Close()
defer out.Close()
defer func() { _ = in.Close() }()
defer func() { _ = out.Close() }()

cx := WrapConnection(out, []byte{}, zap.NewNop())
defer cx.Close()
defer func() { _ = cx.Close() }()

err = compiledRoutes.Handle(cx)
if err != nil {
Expand Down
Loading

5 comments on commit ff5d983

@mholt
Copy link
Owner

@mholt mholt commented on ff5d983 Aug 9, 2024

Choose a reason for hiding this comment

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

@vnxme Is that something you could look into? Probably just a simple fix.

@vnxme
Copy link
Collaborator Author

@vnxme vnxme commented on ff5d983 Aug 9, 2024

Choose a reason for hiding this comment

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

@lxhao61 I haven't tested proxy_protocol option of proxy handler below, but tls sni matcher works fine for me with the latest caddy-l4 and caddy v2.8.4. My working config:

{
	layer4 {
		:443 {
			@abc tls sni abc.caddy
			route @abc {
				proxy localhost:5443
			}
			@def tls sni def.caddy
			route @def {
				proxy localhost:6443
			}
		}
	}
}

abc.caddy:5443 {
	tls {
		issuer internal
	}
	respond "5443"
}

def.caddy:6443 {
	tls {
		issuer internal
	}
	respond "6443"
}

Could you elaborate on what isn't working for you or somehow describe the bug? If you remove proxy_protocol option, does it work?

@lxhao61
Copy link

@lxhao61 lxhao61 commented on ff5d983 Aug 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@vnxme @mholt Sorry, I misjudged the problem. The correct judgment is: SNI configuration and UDP forwarding configuration coexist, and neither can be used normally. The example is as follows:

    "layer4": {
      "servers": {
        "sni": {
          "listen": [":443"],
          "routes": [{
            "match": [{
              "tls": {
                "sni": ["zv.xx.yy"]
              }
            }],
            "handle": [{
              "handler": "proxy",
              "upstreams": [{
                "dial": ["127.0.0.1:5443"]
              }],
              "proxy_protocol": "v1"
            }]
          },
          {
            "match": [{
              "tls": {}
            }],
            "handle": [{
              "handler": "proxy",
              "upstreams": [{
                "dial": ["127.0.0.1:7443"]
              }],
              "proxy_protocol": "v1"
            }]
          }]
        },
        "udppy": {
          "listen": ["udp/:443"],
          "routes": [{
            "handle": [{
              "handler": "proxy",
              "upstreams": [{
                "dial": ["udp/127.0.0.1:7443"]
              }]
            }]
          }]
        }
      }
    },

After deleting the UDP forwarding configuration, the SNI configuration can be used.

@vnxme
Copy link
Collaborator Author

@vnxme vnxme commented on ff5d983 Aug 10, 2024

Choose a reason for hiding this comment

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

@lxhao61 Are you saying that the config you posted above (with both TCP and UDP listening the same port) used to work before this commit, but won't work after it?

@lxhao61
Copy link

Choose a reason for hiding this comment

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

@vnxme Yes.

Please sign in to comment.