Skip to content

Commit b5c7f88

Browse files
jbarwickjbarwick
committed
Address PR review: remove secrets, fix portability, harden code
PR Review Fixes (#139): Security: - Remove committed private key + certs from repo (GitGuardian finding) - Add tests/fixtures/gen_test_certs.sh for ephemeral cert generation - Add .gitignore rules for generated certs and Zone.Identifier files - Fix docker-publish.sh: use --password-stdin for Nexus login (no -p flag) - Fix basePath HTML injection: escape with html.EscapeString + json.Marshal WebSocket: - Filter hop-by-hop headers (Proxy-*, Connection, Keep-Alive, TE, Trailer, Transfer-Encoding) before forwarding to upstream (RFC 7230 §6.1) - Ensure Host header is consistent with dialed upstream target DNS Cache: - Fix cache key collision: fall back to numeric qtype for unknown types - Replace full cache clear with incremental eviction (expired first, then 10% random if still >90% capacity) to avoid latency spikes Portability: - Fix hardcoded 192.168.1.105 in proxy_concurrency_test.sh → 127.0.0.1 - Add NO_COLOR / ASCII_MODE env var support to both test scripts (see https://no-color.org/) for CI terminals without UTF-8/emoji - Fix tests/setup_test.go: honor GS_ADMIN_PORT env (default 8080) - Fix bonjour.go: use GS_ADMIN_PORT + GS_BASE_PATH instead of hardcoded 80 - Auto-generate test certs in testbed setup.sh and benchmark suite
1 parent 8f318e2 commit b5c7f88

File tree

11 files changed

+245
-45
lines changed

11 files changed

+245
-45
lines changed

‎.gitignore‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,13 @@ ui/dist/
1010
# Frontend build artifacts (generated by build.sh, embedded via //go:embed)
1111
application/webserver/frontend/files/*
1212
!application/webserver/frontend/files/.gitkeep
13+
14+
# Ephemeral test certificates — generated by tests/fixtures/gen_test_certs.sh
15+
tests/fixtures/JVJCA.crt
16+
tests/fixtures/JVJCA.key
17+
tests/fixtures/httpbin.org.crt
18+
tests/fixtures/httpbin.org.key
19+
tests/fixtures/*:Zone.Identifier
20+
21+
# Test run artifacts
22+
tests/proxy_benchmark_results.log

‎application/bonjour.go‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,32 @@ package gatesentryf
22

33
import (
44
"log"
5-
// "os"
6-
// "os/signal"
7-
// "time"
5+
"os"
6+
"strconv"
87

98
"github.com/oleksandr/bonjour"
109
)
1110

1211
func StartBonjour() {
1312
log.Println("Starting Bonjour service")
1413

14+
// Derive admin port from environment (same source of truth as main.go)
15+
adminPort := 80
16+
if envPort := os.Getenv("GS_ADMIN_PORT"); envPort != "" {
17+
if p, err := strconv.Atoi(envPort); err == nil && p > 0 {
18+
adminPort = p
19+
}
20+
}
21+
22+
// Derive base path for TXT record
23+
basePath := os.Getenv("GS_BASE_PATH")
24+
if basePath == "" {
25+
basePath = "/"
26+
}
27+
1528
// Advertise the web admin UI so browsers resolve http://gatesentry.local
1629
go func() {
17-
_, err := bonjour.Register("GateSentry", "_http._tcp", "", 80, []string{"txtv=1", "app=gatesentry", "path=/"}, nil)
30+
_, err := bonjour.Register("GateSentry", "_http._tcp", "", adminPort, []string{"txtv=1", "app=gatesentry", "path=" + basePath}, nil)
1831
if err != nil {
1932
log.Println("[Bonjour] HTTP registration error:", err.Error())
2033
}

‎application/dns/server/server.go‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,12 @@ var (
9090

9191
// dnsCacheKey builds a cache key from question name and type.
9292
func dnsCacheKey(qname string, qtype uint16) string {
93-
return strings.ToLower(qname) + "/" + dns.TypeToString[qtype]
93+
t := dns.TypeToString[qtype]
94+
if t == "" {
95+
// Fall back to numeric qtype for unknown/unsupported types to avoid key collisions.
96+
return strings.ToLower(qname) + "/" + fmt.Sprint(qtype)
97+
}
98+
return strings.ToLower(qname) + "/" + t
9499
}
95100

96101
// dnsCacheGet returns a cached response if it exists and hasn't expired.
@@ -164,10 +169,28 @@ func dnsCachePut(qname string, qtype uint16, msg *dns.Msg) {
164169
key := dnsCacheKey(qname, qtype)
165170

166171
dnsCacheMu.Lock()
167-
// Simple eviction: if cache is too large, clear it
172+
// Incremental eviction: remove expired entries first, then oldest if still over limit
168173
if len(dnsCache) >= dnsCacheMax {
169-
log.Printf("[DNS Cache] Evicting %d entries (max %d reached)", len(dnsCache), dnsCacheMax)
170-
dnsCache = make(map[string]*dnsCacheEntry)
174+
now := time.Now()
175+
expired := 0
176+
for k, e := range dnsCache {
177+
if now.After(e.expiresAt) {
178+
delete(dnsCache, k)
179+
expired++
180+
}
181+
}
182+
// If still over 90% capacity after removing expired, evict 10% oldest
183+
if len(dnsCache) >= dnsCacheMax*9/10 {
184+
evictCount := dnsCacheMax / 10
185+
for k := range dnsCache {
186+
delete(dnsCache, k)
187+
evictCount--
188+
if evictCount <= 0 {
189+
break
190+
}
191+
}
192+
}
193+
log.Printf("[DNS Cache] Evicted %d expired + trimmed to %d entries (max %d)", expired, len(dnsCache), dnsCacheMax)
171194
}
172195
dnsCache[key] = &dnsCacheEntry{
173196
msg: msg.Copy(),

‎application/webserver/frontend/frontend.go‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package gatesentryWebserverFrontend
22

33
import (
44
"embed"
5+
"encoding/json"
6+
"html"
57
"io/fs"
68
"log"
79
"net/http"
@@ -43,17 +45,18 @@ func GetIndexHtmlWithBasePath(basePath string) []byte {
4345
return raw
4446
}
4547

46-
html := string(raw)
48+
htmlStr := string(raw)
4749

48-
// Build injection tags
49-
baseHref := basePath + "/"
50+
// Build injection tags — escape basePath for safe HTML/JS injection
51+
baseHref := html.EscapeString(basePath + "/")
52+
jsPath, _ := json.Marshal(basePath) // produces a safely-quoted JSON string
5053
injection := `<base href="` + baseHref + `">` + "\n" +
51-
` <script>window.__GS_BASE_PATH__ = "` + basePath + `";</script>`
54+
` <script>window.__GS_BASE_PATH__ = ` + string(jsPath) + `;</script>`
5255

5356
// Inject after <head> or after first <meta> tag
54-
html = strings.Replace(html, "<head>", "<head>\n "+injection, 1)
57+
htmlStr = strings.Replace(htmlStr, "<head>", "<head>\n "+injection, 1)
5558

56-
return []byte(html)
59+
return []byte(htmlStr)
5760
}
5861

5962
func GetFileSystem(dir string, fsys fs.FS) http.FileSystem {

‎docker-publish.sh‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,11 @@ if [[ "$TARGET" == "nexus" ]]; then
149149
fi
150150

151151
echo "── Logging in to Nexus: ${REGISTRY} ─────────────────────────"
152-
run docker login "${REGISTRY}" -u "${NEXUS_USERNAME}" -p "${NEXUS_PASSWORD}"
152+
if [[ "$DRY_RUN" == true ]]; then
153+
echo "[DRY RUN] echo <password> | docker login \"${REGISTRY}\" -u ${NEXUS_USERNAME} --password-stdin"
154+
else
155+
echo "${NEXUS_PASSWORD}" | docker login "${REGISTRY}" -u "${NEXUS_USERNAME}" --password-stdin
156+
fi
153157

154158
else
155159
# ── Docker Hub ──

‎gatesentryproxy/websocket.go‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,27 @@ func HandleWebsocketConnection(r *http.Request, w http.ResponseWriter) {
6363
// Reconstruct the HTTP request line and headers.
6464
reqURI := r.URL.RequestURI()
6565
fmt.Fprintf(serverConn, "%s %s HTTP/1.1\r\n", r.Method, reqURI)
66-
fmt.Fprintf(serverConn, "Host: %s\r\n", r.Host)
66+
67+
// Use the determined upstream host for the Host header
68+
upstreamHost := r.Host
69+
if upstreamHost == "" {
70+
upstreamHost = host // fallback to the dialed host
71+
}
72+
fmt.Fprintf(serverConn, "Host: %s\r\n", upstreamHost)
73+
6774
for key, values := range r.Header {
75+
// Filter out hop-by-hop and proxy-only headers (RFC 7230 §6.1)
76+
keyLower := strings.ToLower(key)
77+
if keyLower == "host" ||
78+
keyLower == "connection" ||
79+
keyLower == "keep-alive" ||
80+
keyLower == "te" ||
81+
keyLower == "trailer" ||
82+
keyLower == "transfer-encoding" ||
83+
keyLower == "proxy-connection" ||
84+
strings.HasPrefix(keyLower, "proxy-") {
85+
continue
86+
}
6887
for _, v := range values {
6988
fmt.Fprintf(serverConn, "%s: %s\r\n", key, v)
7089
}

‎tests/fixtures/gen_test_certs.sh‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
#!/usr/bin/env bash
2+
###############################################################################
3+
# Generate ephemeral CA + server certificates for GateSentry test bed.
4+
#
5+
# Creates:
6+
# JVJCA.crt / JVJCA.key — self-signed CA (internal-ca)
7+
# httpbin.org.crt / httpbin.org.key — server cert signed by the CA
8+
#
9+
# All certs are written to the same directory as this script (tests/fixtures/).
10+
# They are listed in .gitignore and must NOT be committed.
11+
#
12+
# Usage:
13+
# bash tests/fixtures/gen_test_certs.sh # generate once
14+
# bash tests/fixtures/gen_test_certs.sh --force # regenerate even if exist
15+
###############################################################################
16+
17+
set -euo pipefail
18+
19+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
20+
CA_KEY="${SCRIPT_DIR}/JVJCA.key"
21+
CA_CERT="${SCRIPT_DIR}/JVJCA.crt"
22+
SERVER_KEY="${SCRIPT_DIR}/httpbin.org.key"
23+
SERVER_CERT="${SCRIPT_DIR}/httpbin.org.crt"
24+
DAYS_VALID=365
25+
26+
FORCE=false
27+
[[ "${1:-}" == "--force" ]] && FORCE=true
28+
29+
# Skip if certs already exist (unless --force)
30+
if [[ "$FORCE" == false && -f "$CA_CERT" && -f "$SERVER_CERT" && -f "$SERVER_KEY" ]]; then
31+
echo "[gen_test_certs] Certificates already exist — skipping (use --force to regenerate)"
32+
exit 0
33+
fi
34+
35+
echo "[gen_test_certs] Generating ephemeral test certificates in ${SCRIPT_DIR}/"
36+
37+
# ── 1. CA key + self-signed cert ────────────────────────────────────────────
38+
openssl genrsa -out "$CA_KEY" 2048 2>/dev/null
39+
40+
openssl req -new -x509 \
41+
-key "$CA_KEY" \
42+
-out "$CA_CERT" \
43+
-days "$DAYS_VALID" \
44+
-subj "/CN=internal-ca/C=SG/L=Singapore/O=JVJ 28 Inc." \
45+
2>/dev/null
46+
47+
echo "[gen_test_certs] CA certificate: ${CA_CERT}"
48+
49+
# ── 2. Server key + CSR + CA-signed cert ────────────────────────────────────
50+
openssl genrsa -out "$SERVER_KEY" 2048 2>/dev/null
51+
52+
openssl req -new \
53+
-key "$SERVER_KEY" \
54+
-out "${SCRIPT_DIR}/httpbin.org.csr" \
55+
-subj "/CN=httpbin.org/C=SG/L=Singapore/O=JVJ 28 Inc." \
56+
2>/dev/null
57+
58+
# SAN extension for httpbin.org + localhost
59+
cat > "${SCRIPT_DIR}/_san.cnf" <<EOF
60+
[v3_req]
61+
subjectAltName = DNS:httpbin.org, DNS:localhost, IP:127.0.0.1
62+
basicConstraints = CA:FALSE
63+
keyUsage = digitalSignature, keyEncipherment
64+
extendedKeyUsage = serverAuth
65+
EOF
66+
67+
openssl x509 -req \
68+
-in "${SCRIPT_DIR}/httpbin.org.csr" \
69+
-CA "$CA_CERT" \
70+
-CAkey "$CA_KEY" \
71+
-CAcreateserial \
72+
-out "$SERVER_CERT" \
73+
-days "$DAYS_VALID" \
74+
-extensions v3_req \
75+
-extfile "${SCRIPT_DIR}/_san.cnf" \
76+
2>/dev/null
77+
78+
echo "[gen_test_certs] Server certificate: ${SERVER_CERT}"
79+
80+
# ── Cleanup temp files ──────────────────────────────────────────────────────
81+
rm -f "${SCRIPT_DIR}/httpbin.org.csr" "${SCRIPT_DIR}/_san.cnf" "${SCRIPT_DIR}/JVJCA.srl"
82+
83+
echo "[gen_test_certs] Done — certificates valid for ${DAYS_VALID} days"

‎tests/proxy_benchmark_suite.sh‎

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,17 @@
3434
# all subsequent results. We WANT to see every failure.
3535
set -uo pipefail
3636

37-
# ── Colours ─────────────────────────────────────────────────────────────────
38-
RED='\033[0;31m'
39-
GREEN='\033[0;32m'
40-
YELLOW='\033[1;33m'
41-
CYAN='\033[0;36m'
42-
BOLD='\033[1m'
43-
NC='\033[0m' # No Colour
37+
# ── Colours (respect NO_COLOR — see https://no-color.org/) ─────────────────
38+
if [[ -n "${NO_COLOR:-}" || "${ASCII_MODE:-}" == "1" ]]; then
39+
RED=''; GREEN=''; YELLOW=''; CYAN=''; BOLD=''; NC=''
40+
else
41+
RED='\033[0;31m'
42+
GREEN='\033[0;32m'
43+
YELLOW='\033[1;33m'
44+
CYAN='\033[0;36m'
45+
BOLD='\033[1m'
46+
NC='\033[0m' # No Colour
47+
fi
4448

4549
# ── Defaults ────────────────────────────────────────────────────────────────
4650
DNS_PORT="${DNS_PORT:-10053}"
@@ -64,6 +68,13 @@ ECHO_SERVER="http://127.0.0.1:9998" # Python echo server (direct)
6468
TESTBED_FILES="${TESTBED_HTTP}/files" # Static test files (1MB, 10MB, 100MB)
6569
CA_CERT="${CA_CERT:-$(cd "$(dirname "$0")" && pwd)/fixtures/JVJCA.crt}" # Internal CA
6670

71+
# Auto-generate test certs if they don't exist
72+
FIXTURES_DIR="$(cd "$(dirname "$0")" && pwd)/fixtures"
73+
if [[ ! -f "$CA_CERT" && -f "${FIXTURES_DIR}/gen_test_certs.sh" ]]; then
74+
echo " INFO Generating ephemeral test certificates..."
75+
bash "${FIXTURES_DIR}/gen_test_certs.sh"
76+
fi
77+
6778
# ── Counters ────────────────────────────────────────────────────────────────
6879
PASS=0
6980
FAIL=0
@@ -91,30 +102,37 @@ log_section() {
91102
echo -e "${BOLD}── $1 ──${NC}"
92103
}
93104

105+
# Glyph selection (ASCII fallbacks for CI/non-UTF8 terminals)
106+
if [[ -n "${NO_COLOR:-}" || "${ASCII_MODE:-}" == "1" ]]; then
107+
_G_PASS='[PASS]'; _G_FAIL='[FAIL]'; _G_KNOWN='[KNOWN]'; _G_SKIP='[SKIP]'; _G_ARROW='->'
108+
else
109+
_G_PASS='✓ PASS'; _G_FAIL='✗ FAIL'; _G_KNOWN='⚠ KNOWN'; _G_SKIP='⊘ SKIP'; _G_ARROW='↳'
110+
fi
111+
94112
pass() {
95113
((TOTAL++)) || true
96114
((PASS++)) || true
97-
echo -e " ${GREEN}✓ PASS${NC} $1"
115+
echo -e " ${GREEN}${_G_PASS}${NC} $1"
98116
}
99117

100118
fail() {
101119
((TOTAL++)) || true
102120
((FAIL++)) || true
103-
echo -e " ${RED}✗ FAIL${NC} $1"
104-
[[ -n "${2:-}" ]] && echo -e " ${RED}↳ $2${NC}"
121+
echo -e " ${RED}${_G_FAIL}${NC} $1"
122+
[[ -n "${2:-}" ]] && echo -e " ${RED}${_G_ARROW} $2${NC}"
105123
}
106124

107125
known_issue() {
108126
((TOTAL++)) || true
109127
((KNOWN++)) || true
110-
echo -e " ${YELLOW}⚠ KNOWN${NC} $1"
111-
[[ -n "${2:-}" ]] && echo -e " ${YELLOW}↳ $2${NC}"
128+
echo -e " ${YELLOW}${_G_KNOWN}${NC} $1"
129+
[[ -n "${2:-}" ]] && echo -e " ${YELLOW}${_G_ARROW} $2${NC}"
112130
}
113131

114132
skip_test() {
115133
((TOTAL++)) || true
116134
((SKIP++)) || true
117-
echo -e " ${CYAN}⊘ SKIP${NC} $1"
135+
echo -e " ${CYAN}${_G_SKIP}${NC} $1"
118136
}
119137

120138
verbose() {

‎tests/proxy_concurrency_test.sh‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,29 @@
2020
set -euo pipefail
2121

2222
# ── Configuration ────────────────────────────────────────────────────────────
23-
PROXY_HOST="${PROXY_HOST:-192.168.1.105}"
23+
PROXY_HOST="${PROXY_HOST:-127.0.0.1}"
2424
PROXY_PORT="${PROXY_PORT:-10413}"
25-
ECHO_SERVER="${ECHO_SERVER:-http://192.168.1.105:9998}"
26-
TESTBED_HTTP="${TESTBED_HTTP:-http://192.168.1.105:9999}"
25+
ECHO_SERVER="${ECHO_SERVER:-http://127.0.0.1:9998}"
26+
TESTBED_HTTP="${TESTBED_HTTP:-http://127.0.0.1:9999}"
2727
CURL_TIMEOUT=10
2828

29-
# Colours
30-
RED='\033[0;31m'; GREEN='\033[0;32m'; YELLOW='\033[1;33m'
31-
CYAN='\033[0;36m'; NC='\033[0m'; BOLD='\033[1m'
29+
# Colours (respect NO_COLOR — see https://no-color.org/)
30+
if [[ -n "${NO_COLOR:-}" || "${ASCII_MODE:-}" == "1" ]]; then
31+
RED=''; GREEN=''; YELLOW=''; CYAN=''; NC=''; BOLD=''
32+
_PASS='PASS'; _FAIL='FAIL'; _WARN='WARN'; _ARROW='->';
33+
else
34+
RED='\033[0;31m'; GREEN='\033[0;32m'; YELLOW='\033[1;33m'
35+
CYAN='\033[0;36m'; NC='\033[0m'; BOLD='\033[1m'
36+
_PASS='✅ PASS'; _FAIL='❌ FAIL'; _WARN='⚠️ WARN'; _ARROW='↳';
37+
fi
3238

3339
PASS_COUNT=0; FAIL_COUNT=0; WARN_COUNT=0
3440

35-
pass() { ((PASS_COUNT++)) || true; echo -e " ${GREEN}✅ PASS${NC} $1"; }
36-
fail() { ((FAIL_COUNT++)) || true; echo -e " ${RED}❌ FAIL${NC} $1"; [[ -n "${2:-}" ]] && echo -e " ${YELLOW}↳ $2${NC}"; }
37-
warn() { ((WARN_COUNT++)) || true; echo -e " ${YELLOW}⚠️ WARN${NC} $1"; [[ -n "${2:-}" ]] && echo -e " ${YELLOW}↳ $2${NC}"; }
38-
log_header() { echo -e "\n${BOLD}${CYAN}═══ $1 ═══${NC}"; }
39-
log_section() { echo -e "\n${BOLD}── $1${NC}"; }
41+
pass() { ((PASS_COUNT++)) || true; echo -e " ${GREEN}${_PASS}${NC} $1"; }
42+
fail() { ((FAIL_COUNT++)) || true; echo -e " ${RED}${_FAIL}${NC} $1"; [[ -n "${2:-}" ]] && echo -e " ${YELLOW}${_ARROW} $2${NC}"; }
43+
warn() { ((WARN_COUNT++)) || true; echo -e " ${YELLOW}${_WARN}${NC} $1"; [[ -n "${2:-}" ]] && echo -e " ${YELLOW}${_ARROW} $2${NC}"; }
44+
log_header() { echo -e "\n${BOLD}${CYAN}=== $1 ===${NC}"; }
45+
log_section() { echo -e "\n${BOLD}-- $1${NC}"; }
4046

4147
gscurl() { curl --proxy "http://${PROXY_HOST}:${PROXY_PORT}" --max-time "$CURL_TIMEOUT" "$@"; }
4248

0 commit comments

Comments
 (0)