Skip to content

Commit 7925307

Browse files
miklosnmiklosn
committed
feat: Add configurable command whitelist
fix: Correctly log client address in proxy fix: Remove pointers from logging statements in proxy.go Refactor: Improve logging and remove unnecessary pointer dereferences fix: Correct logging of client address and connection closure fix: Correct log level for backend connection closure Improve logging
1 parent 4669473 commit 7925307

5 files changed

Lines changed: 103 additions & 54 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ clamdproxy --listen 127.0.0.1:3310 --backend 127.0.0.1:3311
7272
- `--backend`: Address of the backend clamd server (default: 127.0.0.1:3311)
7373
- `--log-level`: Logging level: debug, info, warn, error (default: warn)
7474
- `--pprof`: Address for pprof HTTP server (disabled if empty)
75+
- `--allow-command`: ClamAV command to allow (can be specified multiple times). If not specified, defaults to a subset of commands (PING, VERSION, VERSIONCOMMANDS, INSTREAM).
7576

7677
## Protocol
7778

‎main.go‎

Lines changed: 50 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,23 @@ package main
44

55
import (
66
"fmt"
7-
"github.com/alecthomas/kong"
87
"log/slog"
98
"net"
109
"net/http"
1110
_ "net/http/pprof" // Register pprof handlers
1211
"os"
1312
"strings"
14-
)
1513

14+
"github.com/alecthomas/kong"
15+
)
1616

1717
// CLI configuration structure for Kong
1818
var cli struct {
19-
Listen string `name:"listen" help:"Address to listen on" default:"127.0.0.1:3310"`
20-
Backend string `name:"backend" help:"Address of the backend clamd server" default:"127.0.0.1:3311"`
21-
LogLevel string `name:"log-level" help:"Log level (debug, info, warn, error)" default:"warn" enum:"debug,info,warn,error"`
22-
PprofAddr string `name:"pprof" help:"Address for pprof HTTP server (disabled if empty)" default:""`
19+
Listen string `name:"listen" help:"Address to listen on" default:"127.0.0.1:3310"`
20+
Backend string `name:"backend" help:"Address of the backend clamd server" default:"127.0.0.1:3311"`
21+
LogLevel string `name:"log-level" help:"Log level (debug, info, warn, error)" default:"warn" enum:"debug,info,warn,error"`
22+
PprofAddr string `name:"pprof" help:"Address for pprof HTTP server (disabled if empty)" default:""`
23+
AllowedCommands []string `name:"allow-command" help:"ClamAV commands to allow (can be specified multiple times)"`
2324
}
2425

2526
// Global logger used throughout the code
@@ -56,15 +57,18 @@ func main() {
5657
logger = getLogger(cli.LogLevel)
5758
slog.SetDefault(logger)
5859

60+
// Initialize allowed commands
61+
initAllowedCommands()
62+
5963
logger.Warn("Starting clamdproxy",
60-
"listen", &cli.Listen,
61-
"backend", &cli.Backend)
64+
"listen", cli.Listen,
65+
"backend", cli.Backend)
6266

6367
// Start pprof server if enabled
6468
if cli.PprofAddr != "" {
6569
go func() {
66-
logger.Info("Starting pprof server",
67-
"addr", &cli.PprofAddr,
70+
logger.Warn("Starting pprof server",
71+
"addr", cli.PprofAddr,
6872
"url", fmt.Sprintf("http://%s/debug/pprof/", cli.PprofAddr))
6973
if err := http.ListenAndServe(cli.PprofAddr, nil); err != nil {
7074
logger.Error("Failed to start pprof server", "error", err)
@@ -103,7 +107,7 @@ func handleConnection(clientConn net.Conn) {
103107
}()
104108
clientAddr := clientConn.RemoteAddr()
105109

106-
logger.Info("Connection established", "client", &clientAddr)
110+
logger.Info("Connection established", "client", clientAddr)
107111

108112
backendConn, err := net.Dial("tcp", cli.Backend)
109113
if err != nil {
@@ -115,14 +119,46 @@ func handleConnection(clientConn net.Conn) {
115119
}
116120
defer func() {
117121
if err := backendConn.Close(); err != nil {
118-
logger.Error("Failed to close backend connection", "error", err)
122+
logger.Debug("Failed to close backend connection", "error", err)
119123
}
120124
}()
121125

122-
logger.Info("Connected to backend", "backend", &cli.Backend, "client", &clientAddr)
126+
logger.Info("Connected to backend", "backend", cli.Backend, "client", clientAddr)
123127

124128
proxy := NewClamdProxy(clientConn, backendConn)
125129
proxy.Start()
126130

127-
logger.Info("Connection closed", "client", &clientAddr)
131+
logger.Info("Connection closed", "client", clientAddr)
132+
}
133+
134+
// initAllowedCommands initializes the allowedCommands map based on CLI options
135+
func initAllowedCommands() {
136+
// If no commands specified, use the default set
137+
if len(cli.AllowedCommands) == 0 {
138+
// Default commands are already set in the allowedCommands variable
139+
logger.Info("Using default allowed commands", "commands", allowedCommands)
140+
return
141+
}
142+
143+
// Clear the current allowed commands
144+
for cmd := range allowedCommands {
145+
delete(allowedCommands, cmd)
146+
}
147+
148+
// Add each specified command if it's valid
149+
for _, cmd := range cli.AllowedCommands {
150+
cmd = strings.ToUpper(cmd) // Normalize to uppercase
151+
if validClamdCommands[cmd] {
152+
allowedCommands[cmd] = true
153+
} else {
154+
logger.Warn("Ignoring invalid command", "command", cmd)
155+
}
156+
}
157+
158+
// Log the final set of allowed commands
159+
var cmdList []string
160+
for cmd := range allowedCommands {
161+
cmdList = append(cmdList, cmd)
162+
}
163+
logger.Info("Configured allowed commands", "commands", cmdList)
128164
}

‎proxy.go‎

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,25 @@ const (
3737
newlineDelimiter = byte('\n')
3838
)
3939

40-
// allowedCommands defines the only commands that are permitted to be forwarded
40+
// validClamdCommands defines all known valid clamd commands
41+
var validClamdCommands = map[string]bool{
42+
"PING": true,
43+
"VERSION": true,
44+
"RELOAD": true,
45+
"SHUTDOWN": true,
46+
"SCAN": true,
47+
"INSTREAM": true,
48+
"FILDES": true,
49+
"STATS": true,
50+
"IDSESSION": true,
51+
"END": true,
52+
"VERSIONCOMMANDS": true,
53+
"MULTISCAN": true,
54+
"CONTSCAN": true,
55+
"ALLMATCHSCAN": true,
56+
}
57+
58+
// allowedCommands defines the commands that are permitted to be forwarded
4159
// to the backend for security reasons
4260
var allowedCommands = map[string]bool{
4361
"PING": true,
@@ -70,7 +88,7 @@ func NewClamdProxy(client, backend net.Conn) *ClamdProxy {
7088
// directly processes backend->client traffic in the current goroutine.
7189
func (p *ClamdProxy) Start() {
7290
clientAddr := p.client.RemoteAddr()
73-
logger.Info("Starting proxy", "client", &clientAddr)
91+
logger.Info("Starting proxy", "client", clientAddr)
7492

7593
// Handle client -> backend in a separate goroutine
7694
go p.handleClientToBackend()
@@ -119,17 +137,17 @@ func (p *ClamdProxy) Start() {
119137

120138
if err != nil {
121139
if isConnectionClosed(err) {
122-
logger.Info("Backend connection closed",
123-
"client", &clientAddr,
140+
logger.Debug("Backend connection closed",
141+
"client", clientAddr,
124142
"error", err)
125143
} else {
126144
logger.Debug("Error copying from backend to client",
127-
"client", &clientAddr,
145+
"client", clientAddr,
128146
"error", err)
129147
}
130148
} else {
131149
logger.Info("Proxy completed",
132-
"client", &clientAddr,
150+
"client", clientAddr,
133151
"bytesTransferred", bytesWritten)
134152
}
135153
}
@@ -146,13 +164,13 @@ func (p *ClamdProxy) handleClientToBackend() {
146164
if err != nil {
147165
if err == io.EOF {
148166
// Normal client disconnection, log at debug level
149-
logger.Info("Client disconnected", "client", &clientAddr)
167+
logger.Info("Client disconnected", "client", clientAddr)
150168
} else {
151169
// Only log as error if it's not a connection reset or broken pipe
152170
if isConnectionClosed(err) {
153-
logger.Info("Client connection closed", "client", &clientAddr, "error", err)
171+
logger.Debug("Client connection closed", "client", clientAddr, "error", err)
154172
} else {
155-
logger.Debug("Error reading command", "client", &clientAddr, "error", err)
173+
logger.Debug("Error reading command", "client", clientAddr, "error", err)
156174
}
157175
}
158176
// Close the backend connection to signal we're done
@@ -163,7 +181,7 @@ func (p *ClamdProxy) handleClientToBackend() {
163181
}
164182

165183
// Only log commands at appropriate levels
166-
logger.Debug("Command received", "client", &clientAddr, "command", &cmd)
184+
logger.Debug("Command received", "client", clientAddr, "command", cmd)
167185

168186
// Check if command is allowed
169187
if isCommandAllowed(cmd) {
@@ -180,17 +198,17 @@ func (p *ClamdProxy) handleClientToBackend() {
180198

181199
// Handle special case for INSTREAM command (file streaming)
182200
if isInstreamCommand(cmd) {
183-
logger.Debug("Processing INSTREAM data", "client", &clientAddr)
201+
logger.Debug("Processing INSTREAM data", "client", clientAddr)
184202

185203
if err := p.handleInstream(reader); err != nil {
186204
logger.Debug("Error handling INSTREAM data",
187-
"client", &clientAddr,
205+
"client", clientAddr,
188206
"error", err)
189207
break
190208
}
191209
}
192210
} else {
193-
logger.Info("Blocked command", "client", &clientAddr, "command", &cmd)
211+
logger.Info("Blocked command", "client", clientAddr, "command", cmd)
194212
// Send error response to client using buffered writer
195213
response := "ERROR: Command not allowed\n"
196214
if _, err := p.clientBuf.WriteString(response); err != nil {
@@ -320,7 +338,7 @@ func (p *ClamdProxy) handleInstream(reader *bufio.Reader) error {
320338
// If size is 0, we're done with the stream
321339
if size == 0 {
322340
logger.Debug("INSTREAM completed",
323-
"client", &clientAddr,
341+
"client", clientAddr,
324342
"totalBytes", totalBytes,
325343
"chunks", chunks)
326344
break
@@ -359,7 +377,7 @@ func (p *ClamdProxy) handleInstream(reader *bufio.Reader) error {
359377
// Only log chunk details at the most verbose level and only occasionally
360378
if chunks%100 == 0 {
361379
logger.Debug("INSTREAM progress",
362-
"client", &clientAddr,
380+
"client", clientAddr,
363381
"chunks", chunks,
364382
"totalBytes", totalBytes)
365383
}

‎proxy_test.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,25 @@ func TestReadCommand(t *testing.T) {
8585
}
8686

8787
func TestIsCommandAllowed(t *testing.T) {
88+
// Save original allowedCommands
89+
originalAllowed := make(map[string]bool)
90+
for k, v := range allowedCommands {
91+
originalAllowed[k] = v
92+
}
93+
94+
// Restore after test
95+
defer func() {
96+
allowedCommands = originalAllowed
97+
}()
98+
99+
// Set test allowed commands
100+
allowedCommands = map[string]bool{
101+
"PING": true,
102+
"VERSION": true,
103+
"VERSIONCOMMANDS": true,
104+
"INSTREAM": true,
105+
}
106+
88107
allowedCmds := []string{
89108
"PING", "VERSION", "VERSIONCOMMANDS", "INSTREAM",
90109
"zPING", "zVERSION", "zVERSIONCOMMANDS", "zINSTREAM",

‎todo.txt‎

Lines changed: 0 additions & 25 deletions
This file was deleted.

0 commit comments

Comments
 (0)