Skip to content

Commit 97f19be

Browse files
kamirkamir
committed
Merge branch 'feature/kshark-enhancements-round-1'
2 parents 9f2f14f + 1c03b7f commit 97f19be

File tree

7 files changed

+502
-11
lines changed

7 files changed

+502
-11
lines changed

‎client-sec-test.properties‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# Properties for security testing
2+
bootstrap.servers=localhost:9092
3+
schema.registry.url=http://localhost:8081
4+
basic.auth.user.info=testuser:testpassword

‎cmd/kshark/main.go‎

Lines changed: 56 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ import (
3636
"strings"
3737
"time"
3838

39+
"regexp"
40+
3941
"github.com/segmentio/kafka-go"
4042
"github.com/segmentio/kafka-go/sasl"
4143
"github.com/segmentio/kafka-go/sasl/plain"
@@ -824,7 +826,19 @@ func runCmdIfExists(name string, args ...string) (string, error) {
824826
return buf.String(), err
825827
}
826828

829+
// isValidHostname checks if the hostname is valid and doesn't contain malicious characters.
830+
func isValidHostname(host string) bool {
831+
// Regex to match a valid hostname (alphanumeric, hyphens, periods).
832+
// It explicitly disallows characters like ';', '&', '|', '`', '$', '(', ')', etc.
833+
re := regexp.MustCompile(`^[a-zA-Z0-9\.\-]+$`)
834+
return re.MatchString(host)
835+
}
836+
827837
func bestEffortTraceroute(r *Report, host string) {
838+
if !isValidHostname(host) {
839+
addRow(r, Row{"diag", host, DIAG, FAIL, "Invalid hostname provided", "Skipping traceroute to prevent command injection."})
840+
return
841+
}
828842
// Linux: traceroute or tracepath; macOS: traceroute; Windows: tracert
829843
if out, err := runCmdIfExists("traceroute", "-n", "-w", "2", "-q", "1", host); err == nil {
830844
addRow(r, Row{"diag", host, DIAG, OK, "traceroute OK (see JSON)", ""})
@@ -845,6 +859,10 @@ func bestEffortTraceroute(r *Report, host string) {
845859
}
846860

847861
func mtuCheck(r *Report, host string) {
862+
if !isValidHostname(host) {
863+
addRow(r, Row{"diag", host, DIAG, FAIL, "Invalid hostname provided", "Skipping MTU check to prevent command injection."})
864+
return
865+
}
848866
// Linux tracepath usually reports pMTU; otherwise try ping DF
849867
if out, err := runCmdIfExists("tracepath", host); err == nil && strings.Contains(out, "pmtu") {
850868
addRow(r, Row{"diag", host, DIAG, OK, "pMTU detected via tracepath", ""})
@@ -1150,11 +1168,13 @@ endScan:
11501168
printPretty(report)
11511169

11521170
if *jsonOut != "" {
1153-
if err := writeJSON(*jsonOut, report); err != nil {
1154-
fmt.Fprintf(os.Stderr, "write json: %v\n", err)
1171+
actualPath, err := writeJSON(*jsonOut, report)
1172+
if err != nil {
1173+
fmt.Fprintf(os.Stderr, "Error writing JSON report: %v\n", err)
11551174
os.Exit(1)
11561175
}
1157-
fmt.Printf("JSON report written to %s\n", *jsonOut)
1176+
absPath, _ := filepath.Abs(actualPath)
1177+
fmt.Printf("JSON report written to %s\n", absPath)
11581178
}
11591179

11601180
if *analyze && !*noAI {
@@ -1274,28 +1294,53 @@ func writeHTMLReport(r *Report, analysis *AIAnalysisResponse) (string, error) {
12741294
return reportPath, nil
12751295
}
12761296

1277-
func writeJSON(path string, r *Report) error {
1297+
func createSafeReportPath(userInputPath string, safeSubDir string) (string, error) {
1298+
if userInputPath == "" {
1299+
return "", errors.New("output path cannot be empty")
1300+
}
1301+
1302+
// Sanitize the filename by stripping any directory components
1303+
cleanFilename := filepath.Base(userInputPath)
1304+
if cleanFilename == "." || cleanFilename == "/" || cleanFilename == ".." {
1305+
return "", fmt.Errorf("invalid filename provided: %s", userInputPath)
1306+
}
1307+
1308+
// Ensure the safe subdirectory exists
1309+
if err := os.MkdirAll(safeSubDir, 0755); err != nil {
1310+
return "", fmt.Errorf("could not create reports directory '%s': %w", safeSubDir, err)
1311+
}
1312+
1313+
// Join the safe directory and the clean filename
1314+
safePath := filepath.Join(safeSubDir, cleanFilename)
1315+
return safePath, nil
1316+
}
1317+
1318+
func writeJSON(path string, r *Report) (string, error) {
12781319
if path == "" {
1279-
return nil
1320+
return "", nil
12801321
}
1281-
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
1282-
return err
1322+
1323+
safePath, err := createSafeReportPath(path, "reports")
1324+
if err != nil {
1325+
return "", fmt.Errorf("invalid output path: %w", err)
12831326
}
1284-
f, err := os.Create(path)
1327+
1328+
f, err := os.Create(safePath)
12851329
if err != nil {
1286-
return err
1330+
return safePath, err
12871331
}
12881332
defer f.Close()
1333+
12891334
enc := json.NewEncoder(f)
12901335
enc.SetIndent("", " ")
1291-
return enc.Encode(r)
1336+
return safePath, enc.Encode(r)
12921337
}
12931338

12941339
func redactProps(p map[string]string) map[string]string {
12951340
out := map[string]string{}
12961341
for k, v := range p {
12971342
lk := strings.ToLower(k)
1298-
if strings.Contains(lk, "password") || strings.Contains(lk, "secret") || k == "sasl.oauthbearer.token" || strings.Contains(lk, "key") {
1343+
if strings.Contains(lk, "password") || strings.Contains(lk, "secret") || k == "sasl.oauthbearer.token" || strings.Contains(lk, "key") || k == "basic.auth.user.info" {
12991344
out[k] = "***"
13001345
} else {
13011346
out[k] = v

‎kshark‎

16.2 KB
Binary file not shown.

‎my-report.json‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
{
2+
"rows": [
3+
{
4+
"component": "kafka",
5+
"target": "pkc-619z3.us-east1.gcp.confluent.cloud",
6+
"layer": "L3-Network",
7+
"status": "OK",
8+
"detail": "Resolved host"
9+
},
10+
{
11+
"component": "kafka",
12+
"target": "pkc-619z3.us-east1.gcp.confluent.cloud:9092",
13+
"layer": "L4-TCP",
14+
"status": "OK",
15+
"detail": "Connected in 134ms"
16+
},
17+
{
18+
"component": "kafka",
19+
"target": "pkc-619z3.us-east1.gcp.confluent.cloud:9092",
20+
"layer": "L5-6-TLS",
21+
"status": "OK",
22+
"detail": "TLS 303; peer=*.cert-intl-gkyqm625.us-east1.gcp.confluent.cloud; expires=2025-10-12"
23+
},
24+
{
25+
"component": "kafka",
26+
"target": "pkc-619z3.us-east1.gcp.confluent.cloud:9092",
27+
"layer": "L7-Kafka",
28+
"status": "OK",
29+
"detail": "ApiVersions OK"
30+
},
31+
{
32+
"component": "kafka",
33+
"target": "",
34+
"layer": "L7-Kafka",
35+
"status": "FAIL",
36+
"detail": "Topic not found / not authorized (DescribeTopic).",
37+
"hint": "Grant Describe on topic or create it."
38+
},
39+
{
40+
"component": "diag",
41+
"target": "pkc-619z3.us-east1.gcp.confluent.cloud",
42+
"layer": "Diag",
43+
"status": "OK",
44+
"detail": "traceroute OK (see JSON)"
45+
},
46+
{
47+
"component": "diag",
48+
"target": "pkc-619z3.us-east1.gcp.confluent.cloud",
49+
"layer": "Diag",
50+
"status": "SKIP",
51+
"detail": "traceroute to pkc-619z3.us-east1.gcp.confluent.cloud (35.231.227.240), 64 hops max, 40 byte packets\n 1 192.168.0.1 2.189 ms\n 2 192.168.2.1 2.604 ms\n 3 62.155.247.158 8.347 ms\n 4 62.154.4.230 16.313 ms\n 5 80.150.170.70 15.908 ms\n 6 35.231.227.240 121.773 ms\n",
52+
"hint": "Full output in JSON if -json used."
53+
},
54+
{
55+
"component": "diag",
56+
"target": "pkc-619z3.us-east1.gcp.confluent.cloud",
57+
"layer": "Diag",
58+
"status": "OK",
59+
"detail": "MTU ok at payload 1464"
60+
},
61+
{
62+
"component": "schema-reg",
63+
"target": "psrc-lo5k9.eu-central-1.aws.confluent.cloud",
64+
"layer": "L3-Network",
65+
"status": "OK",
66+
"detail": "Resolved host"
67+
},
68+
{
69+
"component": "schema-reg",
70+
"target": "https://psrc-lo5k9.eu-central-1.aws.confluent.cloud",
71+
"layer": "L7-HTTP",
72+
"status": "OK",
73+
"detail": "GET /subjects OK"
74+
}
75+
],
76+
"summary": {
77+
"Diag": {
78+
"ok": 2,
79+
"warn": 0,
80+
"fail": 0,
81+
"skip": 1
82+
},
83+
"L3-Network": {
84+
"ok": 2,
85+
"warn": 0,
86+
"fail": 0,
87+
"skip": 0
88+
},
89+
"L4-TCP": {
90+
"ok": 1,
91+
"warn": 0,
92+
"fail": 0,
93+
"skip": 0
94+
},
95+
"L5-6-TLS": {
96+
"ok": 1,
97+
"warn": 0,
98+
"fail": 0,
99+
"skip": 0
100+
},
101+
"L7-HTTP": {
102+
"ok": 1,
103+
"warn": 0,
104+
"fail": 0,
105+
"skip": 0
106+
},
107+
"L7-Kafka": {
108+
"ok": 1,
109+
"warn": 0,
110+
"fail": 1,
111+
"skip": 0
112+
}
113+
},
114+
"started_at": "2025-09-11T09:45:55.296994+02:00",
115+
"finished_at": "2025-09-11T09:46:08.414219+02:00",
116+
"config_echo": {
117+
"basic.auth.credentials.source": "USER_INFO",
118+
"basic.auth.user.info": "***",
119+
"bootstrap.servers": "pkc-619z3.us-east1.gcp.confluent.cloud:9092",
120+
"sasl.jaas.config": "org.apache.kafka.common.security.plain.PlainLoginModule required username='5DIC7ADQTDV43IZN' password='cflt/DOxaMUtNPon0Ko/OteSVYKi7LSpVjjhSJcgmW+6MEJyW8bc/PofsiXX+k3Q';",
121+
"sasl.mechanism": "PLAIN",
122+
"sasl.password": "***",
123+
"sasl.username": "5DIC7ADQTDV43IZN",
124+
"schema.registry.url": "https://psrc-lo5k9.eu-central-1.aws.confluent.cloud",
125+
"security.protocol": "SASL_SSL"
126+
}
127+
}

‎reports/my-report.json‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
{
2+
"rows": [
3+
{
4+
"component": "kafka",
5+
"target": "pkc-619z3.us-east1.gcp.confluent.cloud",
6+
"layer": "L3-Network",
7+
"status": "OK",
8+
"detail": "Resolved host"
9+
},
10+
{
11+
"component": "kafka",
12+
"target": "pkc-619z3.us-east1.gcp.confluent.cloud:9092",
13+
"layer": "L4-TCP",
14+
"status": "OK",
15+
"detail": "Connected in 133ms"
16+
},
17+
{
18+
"component": "kafka",
19+
"target": "pkc-619z3.us-east1.gcp.confluent.cloud:9092",
20+
"layer": "L5-6-TLS",
21+
"status": "OK",
22+
"detail": "TLS 303; peer=*.cert-intl-gkyqm625.us-east1.gcp.confluent.cloud; expires=2025-10-12"
23+
},
24+
{
25+
"component": "kafka",
26+
"target": "pkc-619z3.us-east1.gcp.confluent.cloud:9092",
27+
"layer": "L7-Kafka",
28+
"status": "OK",
29+
"detail": "ApiVersions OK"
30+
},
31+
{
32+
"component": "kafka",
33+
"target": "",
34+
"layer": "L7-Kafka",
35+
"status": "FAIL",
36+
"detail": "Topic not found / not authorized (DescribeTopic).",
37+
"hint": "Grant Describe on topic or create it."
38+
},
39+
{
40+
"component": "diag",
41+
"target": "pkc-619z3.us-east1.gcp.confluent.cloud",
42+
"layer": "Diag",
43+
"status": "OK",
44+
"detail": "traceroute OK (see JSON)"
45+
},
46+
{
47+
"component": "diag",
48+
"target": "pkc-619z3.us-east1.gcp.confluent.cloud",
49+
"layer": "Diag",
50+
"status": "SKIP",
51+
"detail": "traceroute to pkc-619z3.us-east1.gcp.confluent.cloud (35.231.227.240), 64 hops max, 40 byte packets\n 1 192.168.0.1 2.352 ms\n 2 192.168.2.1 3.027 ms\n 3 62.155.247.158 8.710 ms\n 4 62.154.4.226 18.852 ms\n 5 80.150.170.70 15.815 ms\n 6 35.231.227.240 118.944 ms\n",
52+
"hint": "Full output in JSON if -json used."
53+
},
54+
{
55+
"component": "diag",
56+
"target": "pkc-619z3.us-east1.gcp.confluent.cloud",
57+
"layer": "Diag",
58+
"status": "OK",
59+
"detail": "MTU ok at payload 1464"
60+
},
61+
{
62+
"component": "schema-reg",
63+
"target": "psrc-lo5k9.eu-central-1.aws.confluent.cloud",
64+
"layer": "L3-Network",
65+
"status": "OK",
66+
"detail": "Resolved host"
67+
},
68+
{
69+
"component": "schema-reg",
70+
"target": "https://psrc-lo5k9.eu-central-1.aws.confluent.cloud",
71+
"layer": "L7-HTTP",
72+
"status": "OK",
73+
"detail": "GET /subjects OK"
74+
}
75+
],
76+
"summary": {
77+
"Diag": {
78+
"ok": 2,
79+
"warn": 0,
80+
"fail": 0,
81+
"skip": 1
82+
},
83+
"L3-Network": {
84+
"ok": 2,
85+
"warn": 0,
86+
"fail": 0,
87+
"skip": 0
88+
},
89+
"L4-TCP": {
90+
"ok": 1,
91+
"warn": 0,
92+
"fail": 0,
93+
"skip": 0
94+
},
95+
"L5-6-TLS": {
96+
"ok": 1,
97+
"warn": 0,
98+
"fail": 0,
99+
"skip": 0
100+
},
101+
"L7-HTTP": {
102+
"ok": 1,
103+
"warn": 0,
104+
"fail": 0,
105+
"skip": 0
106+
},
107+
"L7-Kafka": {
108+
"ok": 1,
109+
"warn": 0,
110+
"fail": 1,
111+
"skip": 0
112+
}
113+
},
114+
"started_at": "2025-09-11T09:51:08.335438+02:00",
115+
"finished_at": "2025-09-11T09:51:21.413915+02:00",
116+
"config_echo": {
117+
"basic.auth.credentials.source": "USER_INFO",
118+
"basic.auth.user.info": "***",
119+
"bootstrap.servers": "pkc-619z3.us-east1.gcp.confluent.cloud:9092",
120+
"sasl.jaas.config": "org.apache.kafka.common.security.plain.PlainLoginModule required username='5DIC7ADQTDV43IZN' password='cflt/DOxaMUtNPon0Ko/OteSVYKi7LSpVjjhSJcgmW+6MEJyW8bc/PofsiXX+k3Q';",
121+
"sasl.mechanism": "PLAIN",
122+
"sasl.password": "***",
123+
"sasl.username": "5DIC7ADQTDV43IZN",
124+
"schema.registry.url": "https://psrc-lo5k9.eu-central-1.aws.confluent.cloud",
125+
"security.protocol": "SASL_SSL"
126+
}
127+
}

0 commit comments

Comments
 (0)