fix: address PR review findings for HTTP CA onboarding hardening
- docs/cli/ca.md: remove internal port suffixes from onboarding URLs - docs/config/server.md: document ACME challenge 404 exception - middleware/cidr_test: migrate HTTPSRedirect tests to httptest.NewServer - onboarding/handler: fix isRawIP for bracketed IPv6 literals - run.go: short-circuit proxyMux to deny-all on invalid proxy_allowed_ips - run_http_handlers_test: fix import grouping, use httptest.NewServer for HEAD test
- commit
- ed50492e0b80b15314062cf83f4122fa63a2830a
- author
- brice · · 2026-04-02 05:56 UTC
- parent
- d69dfa0
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +77 -41
M docs/cli/ca.md +4 -4
| @@ -83,13 +83,13 @@ sudo gordon ca install --uninstall | |||
| 83 | 83 | ||
| 84 | 84 | ## Client Trust Setup | |
| 85 | 85 | ||
| 86 | - | Clients that connect directly to Gordon's HTTPS port need the root CA certificate in their trust store. Gordon provides the certificate through a browser-accessible onboarding page at `https://<gordon-host>:<tls_port>/ca`, or over plain HTTP at `http://<gordon-host>:<port>/ca` for first-time setup. | |
| 86 | + | Clients that connect directly to Gordon's HTTPS port need the root CA certificate in their trust store. Gordon provides the certificate through a browser-accessible onboarding page at `https://<gordon-host>/ca`, or over plain HTTP at `http://<gordon-host>/ca` for first-time setup. | |
| 87 | 87 | ||
| 88 | 88 | ### Firefox / Zen (NSS certificate store) | |
| 89 | 89 | ||
| 90 | 90 | Firefox and Zen use their own certificate store, separate from the OS. | |
| 91 | 91 | ||
| 92 | - | 1. Open `https://<gordon-host>:<tls_port>/ca.crt` — Firefox will prompt to import the certificate. | |
| 92 | + | 1. Open `https://<gordon-host>/ca.crt` — Firefox will prompt to import the certificate. | |
| 93 | 93 | 2. Check **Trust this CA to identify websites** and confirm. | |
| 94 | 94 | ||
| 95 | 95 | Alternatively, import manually: **Settings > Privacy & Security > Certificates > View Certificates > Import**. | |
| @@ -98,7 +98,7 @@ Alternatively, import manually: **Settings > Privacy & Security > Certificates > | |||
| 98 | 98 | ||
| 99 | 99 | ```bash | |
| 100 | 100 | # Download the certificate | |
| 101 | - | curl -k https://<gordon-host>:<tls_port>/ca.crt -o gordon-ca.pem | |
| 101 | + | curl -k https://<gordon-host>/ca.crt -o gordon-ca.pem | |
| 102 | 102 | ||
| 103 | 103 | # Debian / Ubuntu | |
| 104 | 104 | sudo cp gordon-ca.pem /usr/local/share/ca-certificates/gordon-ca.crt | |
| @@ -111,7 +111,7 @@ sudo update-ca-trust | |||
| 111 | 111 | ||
| 112 | 112 | ### Mobile Devices (iOS / Android) | |
| 113 | 113 | ||
| 114 | - | Visit `http://<gordon-host>:<port>/ca` from the device browser. The onboarding page offers: | |
| 114 | + | Visit `http://<gordon-host>/ca` from the device browser. The onboarding page offers: | |
| 115 | 115 | ||
| 116 | 116 | - **iOS:** Download the `.mobileconfig` profile, then install it in **Settings > General > VPN & Device Management**. | |
| 117 | 117 | - **Android:** Download `ca.crt`, then install it in **Settings > Security > Encryption & credentials > Install a certificate**. | |
| @@ -83,13 +83,13 @@ sudo gordon ca install --uninstall | |||||
| 83 | 83 | ||||
| 84 | ## Client Trust Setup | 84 | ## Client Trust Setup | ||
| 85 | 85 | ||||
| 86 | - | Clients that connect directly to Gordon's HTTPS port need the root CA certificate in their trust store. Gordon provides the certificate through a browser-accessible onboarding page at `https://<gordon-host>:<tls_port>/ca`, or over plain HTTP at `http://<gordon-host>:<port>/ca` for first-time setup. | 86 | + | Clients that connect directly to Gordon's HTTPS port need the root CA certificate in their trust store. Gordon provides the certificate through a browser-accessible onboarding page at `https://<gordon-host>/ca`, or over plain HTTP at `http://<gordon-host>/ca` for first-time setup. |
| 87 | 87 | ||||
| 88 | ### Firefox / Zen (NSS certificate store) | 88 | ### Firefox / Zen (NSS certificate store) | ||
| 89 | 89 | ||||
| 90 | Firefox and Zen use their own certificate store, separate from the OS. | 90 | Firefox and Zen use their own certificate store, separate from the OS. | ||
| 91 | 91 | ||||
| 92 | - | 1. Open `https://<gordon-host>:<tls_port>/ca.crt` — Firefox will prompt to import the certificate. | 92 | + | 1. Open `https://<gordon-host>/ca.crt` — Firefox will prompt to import the certificate. |
| 93 | 2. Check **Trust this CA to identify websites** and confirm. | 93 | 2. Check **Trust this CA to identify websites** and confirm. | ||
| 94 | 94 | ||||
| 95 | Alternatively, import manually: **Settings > Privacy & Security > Certificates > View Certificates > Import**. | 95 | Alternatively, import manually: **Settings > Privacy & Security > Certificates > View Certificates > Import**. | ||
| @@ -98,7 +98,7 @@ Alternatively, import manually: **Settings > Privacy & Security > Certificates > | |||||
| 98 | 98 | ||||
| 99 | ```bash | 99 | ```bash | ||
| 100 | # Download the certificate | 100 | # Download the certificate | ||
| 101 | - | curl -k https://<gordon-host>:<tls_port>/ca.crt -o gordon-ca.pem | 101 | + | curl -k https://<gordon-host>/ca.crt -o gordon-ca.pem |
| 102 | 102 | ||||
| 103 | # Debian / Ubuntu | 103 | # Debian / Ubuntu | ||
| 104 | sudo cp gordon-ca.pem /usr/local/share/ca-certificates/gordon-ca.crt | 104 | sudo cp gordon-ca.pem /usr/local/share/ca-certificates/gordon-ca.crt | ||
| @@ -111,7 +111,7 @@ sudo update-ca-trust | |||||
| 111 | 111 | ||||
| 112 | ### Mobile Devices (iOS / Android) | 112 | ### Mobile Devices (iOS / Android) | ||
| 113 | 113 | ||||
| 114 | - | Visit `http://<gordon-host>:<port>/ca` from the device browser. The onboarding page offers: | 114 | + | Visit `http://<gordon-host>/ca` from the device browser. The onboarding page offers: |
| 115 | 115 | ||||
| 116 | - **iOS:** Download the `.mobileconfig` profile, then install it in **Settings > General > VPN & Device Management**. | 116 | - **iOS:** Download the `.mobileconfig` profile, then install it in **Settings > General > VPN & Device Management**. | ||
| 117 | - **Android:** Download `ca.crt`, then install it in **Settings > Security > Encryption & credentials > Install a certificate**. | 117 | - **Android:** Download `ca.crt`, then install it in **Settings > Security > Encryption & credentials > Install a certificate**. | ||
M docs/config/server.md +1 -0
| @@ -86,6 +86,7 @@ The static certificate is served for SNI-matching domains (including wildcards). | |||
| 86 | 86 | When `tls_port` is non-zero, direct HTTP clients (those not arriving through a trusted proxy) are restricted to CA onboarding paths only: | |
| 87 | 87 | ||
| 88 | 88 | - `/`, `/ca`, `/ca.crt`, `/ca.mobileconfig` — serve the onboarding page and CA certificate downloads | |
| 89 | + | - `/.well-known/acme-challenge/*` — returns `404 Not Found` (reserved for future ACME support) | |
| 89 | 90 | - All other paths return `403 Forbidden` | |
| 90 | 91 | ||
| 91 | 92 | This lets new clients discover and trust the internal CA over plain HTTP without exposing the full application. Trusted proxy traffic (e.g. from Cloudflare) continues through the normal HTTP proxy path unaffected. | |
| @@ -86,6 +86,7 @@ The static certificate is served for SNI-matching domains (including wildcards). | |||||
| 86 | When `tls_port` is non-zero, direct HTTP clients (those not arriving through a trusted proxy) are restricted to CA onboarding paths only: | 86 | When `tls_port` is non-zero, direct HTTP clients (those not arriving through a trusted proxy) are restricted to CA onboarding paths only: | ||
| 87 | 87 | ||||
| 88 | - `/`, `/ca`, `/ca.crt`, `/ca.mobileconfig` — serve the onboarding page and CA certificate downloads | 88 | - `/`, `/ca`, `/ca.crt`, `/ca.mobileconfig` — serve the onboarding page and CA certificate downloads | ||
| 89 | + | - `/.well-known/acme-challenge/*` — returns `404 Not Found` (reserved for future ACME support) | |||
| 89 | - All other paths return `403 Forbidden` | 90 | - All other paths return `403 Forbidden` | ||
| 90 | 91 | ||||
| 91 | This lets new clients discover and trust the internal CA over plain HTTP without exposing the full application. Trusted proxy traffic (e.g. from Cloudflare) continues through the normal HTTP proxy path unaffected. | 92 | This lets new clients discover and trust the internal CA over plain HTTP without exposing the full application. Trusted proxy traffic (e.g. from Cloudflare) continues through the normal HTTP proxy path unaffected. | ||
M internal/adapters/in/http/middleware/cidr_test.go +49 -28
| @@ -2,6 +2,7 @@ package middleware | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "encoding/json" | |
| 5 | + | "fmt" | |
| 5 | 6 | "net" | |
| 6 | 7 | "net/http" | |
| 7 | 8 | "net/http/httptest" | |
| @@ -117,16 +118,20 @@ func TestHTTPSRedirect_NoPortHost_OmitsTLSPort(t *testing.T) { | |||
| 117 | 118 | }) | |
| 118 | 119 | ||
| 119 | 120 | handler := HTTPSRedirect(nil, 8088, 8443, true, log)(ok) | |
| 121 | + | srv := httptest.NewServer(handler) | |
| 122 | + | defer srv.Close() | |
| 120 | 123 | ||
| 121 | - | req := httptest.NewRequest(http.MethodGet, "http://o2.bnema.dev/", nil) | |
| 122 | - | req.Host = "o2.bnema.dev" | |
| 123 | - | req.RemoteAddr = "203.0.113.10:12345" | |
| 124 | - | rr := httptest.NewRecorder() | |
| 124 | + | client := srv.Client() | |
| 125 | + | client.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse } | |
| 125 | 126 | ||
| 126 | - | handler.ServeHTTP(rr, req) | |
| 127 | + | req, _ := http.NewRequest(http.MethodGet, srv.URL+"/", nil) | |
| 128 | + | req.Host = "o2.bnema.dev" | |
| 129 | + | resp, err := client.Do(req) | |
| 130 | + | require.NoError(t, err) | |
| 131 | + | defer resp.Body.Close() | |
| 127 | 132 | ||
| 128 | - | assert.Equal(t, http.StatusPermanentRedirect, rr.Code) | |
| 129 | - | assert.Equal(t, "https://o2.bnema.dev/", rr.Header().Get("Location")) | |
| 133 | + | assert.Equal(t, http.StatusPermanentRedirect, resp.StatusCode) | |
| 134 | + | assert.Equal(t, "https://o2.bnema.dev/", resp.Header.Get("Location")) | |
| 130 | 135 | } | |
| 131 | 136 | ||
| 132 | 137 | func TestHTTPSRedirect_HTTPListenerPort_MapsToTLSPort(t *testing.T) { | |
| @@ -136,16 +141,20 @@ func TestHTTPSRedirect_HTTPListenerPort_MapsToTLSPort(t *testing.T) { | |||
| 136 | 141 | }) | |
| 137 | 142 | ||
| 138 | 143 | handler := HTTPSRedirect(nil, 8088, 8443, true, log)(ok) | |
| 144 | + | srv := httptest.NewServer(handler) | |
| 145 | + | defer srv.Close() | |
| 139 | 146 | ||
| 140 | - | req := httptest.NewRequest(http.MethodGet, "http://o2.bnema.dev:8088/", nil) | |
| 141 | - | req.Host = "o2.bnema.dev:8088" | |
| 142 | - | req.RemoteAddr = "203.0.113.10:12345" | |
| 143 | - | rr := httptest.NewRecorder() | |
| 147 | + | client := srv.Client() | |
| 148 | + | client.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse } | |
| 144 | 149 | ||
| 145 | - | handler.ServeHTTP(rr, req) | |
| 150 | + | req, _ := http.NewRequest(http.MethodGet, srv.URL+"/", nil) | |
| 151 | + | req.Host = "o2.bnema.dev:8088" | |
| 152 | + | resp, err := client.Do(req) | |
| 153 | + | require.NoError(t, err) | |
| 154 | + | defer resp.Body.Close() | |
| 146 | 155 | ||
| 147 | - | assert.Equal(t, http.StatusPermanentRedirect, rr.Code) | |
| 148 | - | assert.Equal(t, "https://o2.bnema.dev:8443/", rr.Header().Get("Location")) | |
| 156 | + | assert.Equal(t, http.StatusPermanentRedirect, resp.StatusCode) | |
| 157 | + | assert.Equal(t, "https://o2.bnema.dev:8443/", resp.Header.Get("Location")) | |
| 149 | 158 | } | |
| 150 | 159 | ||
| 151 | 160 | func TestHTTPSRedirect_UnknownExplicitPort_IsPreserved(t *testing.T) { | |
| @@ -155,16 +164,20 @@ func TestHTTPSRedirect_UnknownExplicitPort_IsPreserved(t *testing.T) { | |||
| 155 | 164 | }) | |
| 156 | 165 | ||
| 157 | 166 | handler := HTTPSRedirect(nil, 8088, 8443, true, log)(ok) | |
| 167 | + | srv := httptest.NewServer(handler) | |
| 168 | + | defer srv.Close() | |
| 158 | 169 | ||
| 159 | - | req := httptest.NewRequest(http.MethodGet, "http://o2.bnema.dev:9999/", nil) | |
| 160 | - | req.Host = "o2.bnema.dev:9999" | |
| 161 | - | req.RemoteAddr = "203.0.113.10:12345" | |
| 162 | - | rr := httptest.NewRecorder() | |
| 170 | + | client := srv.Client() | |
| 171 | + | client.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse } | |
| 163 | 172 | ||
| 164 | - | handler.ServeHTTP(rr, req) | |
| 173 | + | req, _ := http.NewRequest(http.MethodGet, srv.URL+"/", nil) | |
| 174 | + | req.Host = "o2.bnema.dev:9999" | |
| 175 | + | resp, err := client.Do(req) | |
| 176 | + | require.NoError(t, err) | |
| 177 | + | defer resp.Body.Close() | |
| 165 | 178 | ||
| 166 | - | assert.Equal(t, http.StatusPermanentRedirect, rr.Code) | |
| 167 | - | assert.Equal(t, "https://o2.bnema.dev:9999/", rr.Header().Get("Location")) | |
| 179 | + | assert.Equal(t, http.StatusPermanentRedirect, resp.StatusCode) | |
| 180 | + | assert.Equal(t, "https://o2.bnema.dev:9999/", resp.Header.Get("Location")) | |
| 168 | 181 | } | |
| 169 | 182 | ||
| 170 | 183 | func TestHTTPSRedirect_InvalidExplicitPort_IsPreserved(t *testing.T) { | |
| @@ -174,16 +187,24 @@ func TestHTTPSRedirect_InvalidExplicitPort_IsPreserved(t *testing.T) { | |||
| 174 | 187 | }) | |
| 175 | 188 | ||
| 176 | 189 | handler := HTTPSRedirect(nil, 8088, 8443, true, log)(ok) | |
| 190 | + | srv := httptest.NewServer(handler) | |
| 191 | + | defer srv.Close() | |
| 192 | + | ||
| 193 | + | // Use a raw TCP connection because Go's HTTP client rejects the | |
| 194 | + | // invalid port in the Location header before CheckRedirect fires. | |
| 195 | + | conn, err := net.Dial("tcp", srv.Listener.Addr().String()) | |
| 196 | + | require.NoError(t, err) | |
| 197 | + | defer conn.Close() | |
| 177 | 198 | ||
| 178 | - | req := httptest.NewRequest(http.MethodGet, "http://o2.bnema.dev/", nil) | |
| 179 | - | req.Host = "o2.bnema.dev:abcd" | |
| 180 | - | req.RemoteAddr = "203.0.113.10:12345" | |
| 181 | - | rr := httptest.NewRecorder() | |
| 199 | + | _, err = fmt.Fprintf(conn, "GET / HTTP/1.1\r\nHost: o2.bnema.dev:abcd\r\nConnection: close\r\n\r\n") | |
| 200 | + | require.NoError(t, err) | |
| 182 | 201 | ||
| 183 | - | handler.ServeHTTP(rr, req) | |
| 202 | + | var buf [4096]byte | |
| 203 | + | n, _ := conn.Read(buf[:]) | |
| 204 | + | raw := string(buf[:n]) | |
| 184 | 205 | ||
| 185 | - | assert.Equal(t, http.StatusPermanentRedirect, rr.Code) | |
| 186 | - | assert.Equal(t, "https://o2.bnema.dev:abcd/", rr.Header().Get("Location")) | |
| 206 | + | assert.Contains(t, raw, "308") | |
| 207 | + | assert.Contains(t, raw, "Location: https://o2.bnema.dev:abcd/") | |
| 187 | 208 | } | |
| 188 | 209 | ||
| 189 | 210 | func TestProxyCIDRAllowlist(t *testing.T) { | |
| @@ -2,6 +2,7 @@ package middleware | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "encoding/json" | 4 | "encoding/json" | ||
| 5 | + | "fmt" | |||
| 5 | "net" | 6 | "net" | ||
| 6 | "net/http" | 7 | "net/http" | ||
| 7 | "net/http/httptest" | 8 | "net/http/httptest" | ||
| @@ -117,16 +118,20 @@ func TestHTTPSRedirect_NoPortHost_OmitsTLSPort(t *testing.T) { | |||||
| 117 | }) | 118 | }) | ||
| 118 | 119 | ||||
| 119 | handler := HTTPSRedirect(nil, 8088, 8443, true, log)(ok) | 120 | handler := HTTPSRedirect(nil, 8088, 8443, true, log)(ok) | ||
| 121 | + | srv := httptest.NewServer(handler) | |||
| 122 | + | defer srv.Close() | |||
| 120 | 123 | ||||
| 121 | - | req := httptest.NewRequest(http.MethodGet, "http://o2.bnema.dev/", nil) | 124 | + | client := srv.Client() |
| 122 | - | req.Host = "o2.bnema.dev" | 125 | + | client.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse } |
| 123 | - | req.RemoteAddr = "203.0.113.10:12345" | |||
| 124 | - | rr := httptest.NewRecorder() | |||
| 125 | 126 | ||||
| 126 | - | handler.ServeHTTP(rr, req) | 127 | + | req, _ := http.NewRequest(http.MethodGet, srv.URL+"/", nil) |
| 128 | + | req.Host = "o2.bnema.dev" | |||
| 129 | + | resp, err := client.Do(req) | |||
| 130 | + | require.NoError(t, err) | |||
| 131 | + | defer resp.Body.Close() | |||
| 127 | 132 | ||||
| 128 | - | assert.Equal(t, http.StatusPermanentRedirect, rr.Code) | 133 | + | assert.Equal(t, http.StatusPermanentRedirect, resp.StatusCode) |
| 129 | - | assert.Equal(t, "https://o2.bnema.dev/", rr.Header().Get("Location")) | 134 | + | assert.Equal(t, "https://o2.bnema.dev/", resp.Header.Get("Location")) |
| 130 | } | 135 | } | ||
| 131 | 136 | ||||
| 132 | func TestHTTPSRedirect_HTTPListenerPort_MapsToTLSPort(t *testing.T) { | 137 | func TestHTTPSRedirect_HTTPListenerPort_MapsToTLSPort(t *testing.T) { | ||
| @@ -136,16 +141,20 @@ func TestHTTPSRedirect_HTTPListenerPort_MapsToTLSPort(t *testing.T) { | |||||
| 136 | }) | 141 | }) | ||
| 137 | 142 | ||||
| 138 | handler := HTTPSRedirect(nil, 8088, 8443, true, log)(ok) | 143 | handler := HTTPSRedirect(nil, 8088, 8443, true, log)(ok) | ||
| 144 | + | srv := httptest.NewServer(handler) | |||
| 145 | + | defer srv.Close() | |||
| 139 | 146 | ||||
| 140 | - | req := httptest.NewRequest(http.MethodGet, "http://o2.bnema.dev:8088/", nil) | 147 | + | client := srv.Client() |
| 141 | - | req.Host = "o2.bnema.dev:8088" | 148 | + | client.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse } |
| 142 | - | req.RemoteAddr = "203.0.113.10:12345" | |||
| 143 | - | rr := httptest.NewRecorder() | |||
| 144 | 149 | ||||
| 145 | - | handler.ServeHTTP(rr, req) | 150 | + | req, _ := http.NewRequest(http.MethodGet, srv.URL+"/", nil) |
| 151 | + | req.Host = "o2.bnema.dev:8088" | |||
| 152 | + | resp, err := client.Do(req) | |||
| 153 | + | require.NoError(t, err) | |||
| 154 | + | defer resp.Body.Close() | |||
| 146 | 155 | ||||
| 147 | - | assert.Equal(t, http.StatusPermanentRedirect, rr.Code) | 156 | + | assert.Equal(t, http.StatusPermanentRedirect, resp.StatusCode) |
| 148 | - | assert.Equal(t, "https://o2.bnema.dev:8443/", rr.Header().Get("Location")) | 157 | + | assert.Equal(t, "https://o2.bnema.dev:8443/", resp.Header.Get("Location")) |
| 149 | } | 158 | } | ||
| 150 | 159 | ||||
| 151 | func TestHTTPSRedirect_UnknownExplicitPort_IsPreserved(t *testing.T) { | 160 | func TestHTTPSRedirect_UnknownExplicitPort_IsPreserved(t *testing.T) { | ||
| @@ -155,16 +164,20 @@ func TestHTTPSRedirect_UnknownExplicitPort_IsPreserved(t *testing.T) { | |||||
| 155 | }) | 164 | }) | ||
| 156 | 165 | ||||
| 157 | handler := HTTPSRedirect(nil, 8088, 8443, true, log)(ok) | 166 | handler := HTTPSRedirect(nil, 8088, 8443, true, log)(ok) | ||
| 167 | + | srv := httptest.NewServer(handler) | |||
| 168 | + | defer srv.Close() | |||
| 158 | 169 | ||||
| 159 | - | req := httptest.NewRequest(http.MethodGet, "http://o2.bnema.dev:9999/", nil) | 170 | + | client := srv.Client() |
| 160 | - | req.Host = "o2.bnema.dev:9999" | 171 | + | client.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse } |
| 161 | - | req.RemoteAddr = "203.0.113.10:12345" | |||
| 162 | - | rr := httptest.NewRecorder() | |||
| 163 | 172 | ||||
| 164 | - | handler.ServeHTTP(rr, req) | 173 | + | req, _ := http.NewRequest(http.MethodGet, srv.URL+"/", nil) |
| 174 | + | req.Host = "o2.bnema.dev:9999" | |||
| 175 | + | resp, err := client.Do(req) | |||
| 176 | + | require.NoError(t, err) | |||
| 177 | + | defer resp.Body.Close() | |||
| 165 | 178 | ||||
| 166 | - | assert.Equal(t, http.StatusPermanentRedirect, rr.Code) | 179 | + | assert.Equal(t, http.StatusPermanentRedirect, resp.StatusCode) |
| 167 | - | assert.Equal(t, "https://o2.bnema.dev:9999/", rr.Header().Get("Location")) | 180 | + | assert.Equal(t, "https://o2.bnema.dev:9999/", resp.Header.Get("Location")) |
| 168 | } | 181 | } | ||
| 169 | 182 | ||||
| 170 | func TestHTTPSRedirect_InvalidExplicitPort_IsPreserved(t *testing.T) { | 183 | func TestHTTPSRedirect_InvalidExplicitPort_IsPreserved(t *testing.T) { | ||
| @@ -174,16 +187,24 @@ func TestHTTPSRedirect_InvalidExplicitPort_IsPreserved(t *testing.T) { | |||||
| 174 | }) | 187 | }) | ||
| 175 | 188 | ||||
| 176 | handler := HTTPSRedirect(nil, 8088, 8443, true, log)(ok) | 189 | handler := HTTPSRedirect(nil, 8088, 8443, true, log)(ok) | ||
| 190 | + | srv := httptest.NewServer(handler) | |||
| 191 | + | defer srv.Close() | |||
| 192 | + | ||||
| 193 | + | // Use a raw TCP connection because Go's HTTP client rejects the | |||
| 194 | + | // invalid port in the Location header before CheckRedirect fires. | |||
| 195 | + | conn, err := net.Dial("tcp", srv.Listener.Addr().String()) | |||
| 196 | + | require.NoError(t, err) | |||
| 197 | + | defer conn.Close() | |||
| 177 | 198 | ||||
| 178 | - | req := httptest.NewRequest(http.MethodGet, "http://o2.bnema.dev/", nil) | 199 | + | _, err = fmt.Fprintf(conn, "GET / HTTP/1.1\r\nHost: o2.bnema.dev:abcd\r\nConnection: close\r\n\r\n") |
| 179 | - | req.Host = "o2.bnema.dev:abcd" | 200 | + | require.NoError(t, err) |
| 180 | - | req.RemoteAddr = "203.0.113.10:12345" | |||
| 181 | - | rr := httptest.NewRecorder() | |||
| 182 | 201 | ||||
| 183 | - | handler.ServeHTTP(rr, req) | 202 | + | var buf [4096]byte |
| 203 | + | n, _ := conn.Read(buf[:]) | |||
| 204 | + | raw := string(buf[:n]) | |||
| 184 | 205 | ||||
| 185 | - | assert.Equal(t, http.StatusPermanentRedirect, rr.Code) | 206 | + | assert.Contains(t, raw, "308") |
| 186 | - | assert.Equal(t, "https://o2.bnema.dev:abcd/", rr.Header().Get("Location")) | 207 | + | assert.Contains(t, raw, "Location: https://o2.bnema.dev:abcd/") |
| 187 | } | 208 | } | ||
| 188 | 209 | ||||
| 189 | func TestProxyCIDRAllowlist(t *testing.T) { | 210 | func TestProxyCIDRAllowlist(t *testing.T) { | ||
M internal/adapters/in/http/onboarding/handler.go +3 -0
| @@ -59,6 +59,9 @@ func isRawIP(hostHeader string) bool { | |||
| 59 | 59 | if err != nil { | |
| 60 | 60 | host = hostHeader | |
| 61 | 61 | } | |
| 62 | + | if len(host) >= 2 && host[0] == '[' && host[len(host)-1] == ']' { | |
| 63 | + | host = host[1 : len(host)-1] | |
| 64 | + | } | |
| 62 | 65 | return net.ParseIP(host) != nil | |
| 63 | 66 | } | |
| 64 | 67 | ||
| @@ -59,6 +59,9 @@ func isRawIP(hostHeader string) bool { | |||||
| 59 | if err != nil { | 59 | if err != nil { | ||
| 60 | host = hostHeader | 60 | host = hostHeader | ||
| 61 | } | 61 | } | ||
| 62 | + | if len(host) >= 2 && host[0] == '[' && host[len(host)-1] == ']' { | |||
| 63 | + | host = host[1 : len(host)-1] | |||
| 64 | + | } | |||
| 62 | return net.ParseIP(host) != nil | 65 | return net.ParseIP(host) != nil | ||
| 63 | } | 66 | } | ||
| 64 | 67 | ||||
M internal/app/run.go +4 -1
| @@ -1672,7 +1672,10 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Ha | |||
| 1672 | 1672 | // Direct clients get an onboarding gate (when CA is available) placed BEFORE | |
| 1673 | 1673 | // HTTPSRedirect so force_https_redirect cannot bypass onboarding. | |
| 1674 | 1674 | proxyMux := http.NewServeMux() | |
| 1675 | - | if obHandler != nil { | |
| 1675 | + | if proxyCIDRMiddleware != nil && proxyAllowedNets == nil { | |
| 1676 | + | // Invalid proxy_allowed_ips: deny all traffic (fail-closed). | |
| 1677 | + | proxyMux.Handle("/", proxyCIDRMiddleware(httpProxyWithMiddleware)) | |
| 1678 | + | } else if obHandler != nil { | |
| 1676 | 1679 | proxyMux.Handle("/", directHTTPOnboardingGate(obHandler, proxyAllowedNets, httpProxyWithMiddleware, log)) | |
| 1677 | 1680 | } else { | |
| 1678 | 1681 | proxyMux.Handle("/", httpProxyWithMiddleware) | |
| @@ -1672,7 +1672,10 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Ha | |||||
| 1672 | // Direct clients get an onboarding gate (when CA is available) placed BEFORE | 1672 | // Direct clients get an onboarding gate (when CA is available) placed BEFORE | ||
| 1673 | // HTTPSRedirect so force_https_redirect cannot bypass onboarding. | 1673 | // HTTPSRedirect so force_https_redirect cannot bypass onboarding. | ||
| 1674 | proxyMux := http.NewServeMux() | 1674 | proxyMux := http.NewServeMux() | ||
| 1675 | - | if obHandler != nil { | 1675 | + | if proxyCIDRMiddleware != nil && proxyAllowedNets == nil { |
| 1676 | + | // Invalid proxy_allowed_ips: deny all traffic (fail-closed). | |||
| 1677 | + | proxyMux.Handle("/", proxyCIDRMiddleware(httpProxyWithMiddleware)) | |||
| 1678 | + | } else if obHandler != nil { | |||
| 1676 | proxyMux.Handle("/", directHTTPOnboardingGate(obHandler, proxyAllowedNets, httpProxyWithMiddleware, log)) | 1679 | proxyMux.Handle("/", directHTTPOnboardingGate(obHandler, proxyAllowedNets, httpProxyWithMiddleware, log)) | ||
| 1677 | } else { | 1680 | } else { | ||
| 1678 | proxyMux.Handle("/", httpProxyWithMiddleware) | 1681 | proxyMux.Handle("/", httpProxyWithMiddleware) | ||
M internal/app/run_http_handlers_test.go +16 -8
| @@ -9,12 +9,12 @@ import ( | |||
| 9 | 9 | "path/filepath" | |
| 10 | 10 | "testing" | |
| 11 | 11 | ||
| 12 | + | "github.com/bnema/zerowrap" | |
| 12 | 13 | "github.com/stretchr/testify/assert" | |
| 13 | 14 | "github.com/stretchr/testify/require" | |
| 14 | 15 | ||
| 15 | 16 | adminhttp "github.com/bnema/gordon/internal/adapters/in/http/admin" | |
| 16 | 17 | pkiadapter "github.com/bnema/gordon/internal/adapters/out/pki" | |
| 17 | - | "github.com/bnema/zerowrap" | |
| 18 | 18 | ) | |
| 19 | 19 | ||
| 20 | 20 | func TestCreateAuthService_Disabled_ReturnsNilServices(t *testing.T) { | |
| @@ -299,14 +299,22 @@ func TestCreateHTTPHandlers_DirectHTTPForbiddenHEAD_ReturnsForbidden(t *testing. | |||
| 299 | 299 | ||
| 300 | 300 | _, httpHandler, _ := createHTTPHandlers(svc, cfg, zerowrap.Default()) | |
| 301 | 301 | ||
| 302 | - | req := httptest.NewRequest(http.MethodHead, "/web", nil) | |
| 303 | - | req.RemoteAddr = directAddr | |
| 304 | - | req.Host = "o2.bnema.dev" | |
| 305 | - | rec := httptest.NewRecorder() | |
| 306 | - | httpHandler.ServeHTTP(rec, req) | |
| 302 | + | ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 303 | + | r.RemoteAddr = directAddr | |
| 304 | + | r.Host = "o2.bnema.dev" | |
| 305 | + | httpHandler.ServeHTTP(w, r) | |
| 306 | + | })) | |
| 307 | + | defer ts.Close() | |
| 307 | 308 | ||
| 308 | - | assert.Equal(t, http.StatusForbidden, rec.Code) | |
| 309 | - | assert.Empty(t, rec.Body.String(), "HEAD 403 should have empty body") | |
| 309 | + | resp, err := ts.Client().Head(ts.URL + "/web") | |
| 310 | + | require.NoError(t, err) | |
| 311 | + | ||
| 312 | + | body, err := io.ReadAll(resp.Body) | |
| 313 | + | require.NoError(t, err) | |
| 314 | + | require.NoError(t, resp.Body.Close()) | |
| 315 | + | ||
| 316 | + | assert.Equal(t, http.StatusForbidden, resp.StatusCode) | |
| 317 | + | assert.Empty(t, body, "HEAD 403 should have empty body") | |
| 310 | 318 | } | |
| 311 | 319 | ||
| 312 | 320 | func TestCreateHTTPHandlers_DirectHTTPACMEChallenge_Returns404(t *testing.T) { | |
| @@ -9,12 +9,12 @@ import ( | |||||
| 9 | "path/filepath" | 9 | "path/filepath" | ||
| 10 | "testing" | 10 | "testing" | ||
| 11 | 11 | ||||
| 12 | + | "github.com/bnema/zerowrap" | |||
| 12 | "github.com/stretchr/testify/assert" | 13 | "github.com/stretchr/testify/assert" | ||
| 13 | "github.com/stretchr/testify/require" | 14 | "github.com/stretchr/testify/require" | ||
| 14 | 15 | ||||
| 15 | adminhttp "github.com/bnema/gordon/internal/adapters/in/http/admin" | 16 | adminhttp "github.com/bnema/gordon/internal/adapters/in/http/admin" | ||
| 16 | pkiadapter "github.com/bnema/gordon/internal/adapters/out/pki" | 17 | pkiadapter "github.com/bnema/gordon/internal/adapters/out/pki" | ||
| 17 | - | "github.com/bnema/zerowrap" | |||
| 18 | ) | 18 | ) | ||
| 19 | 19 | ||||
| 20 | func TestCreateAuthService_Disabled_ReturnsNilServices(t *testing.T) { | 20 | func TestCreateAuthService_Disabled_ReturnsNilServices(t *testing.T) { | ||
| @@ -299,14 +299,22 @@ func TestCreateHTTPHandlers_DirectHTTPForbiddenHEAD_ReturnsForbidden(t *testing. | |||||
| 299 | 299 | ||||
| 300 | _, httpHandler, _ := createHTTPHandlers(svc, cfg, zerowrap.Default()) | 300 | _, httpHandler, _ := createHTTPHandlers(svc, cfg, zerowrap.Default()) | ||
| 301 | 301 | ||||
| 302 | - | req := httptest.NewRequest(http.MethodHead, "/web", nil) | 302 | + | ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| 303 | - | req.RemoteAddr = directAddr | 303 | + | r.RemoteAddr = directAddr |
| 304 | - | req.Host = "o2.bnema.dev" | 304 | + | r.Host = "o2.bnema.dev" |
| 305 | - | rec := httptest.NewRecorder() | 305 | + | httpHandler.ServeHTTP(w, r) |
| 306 | - | httpHandler.ServeHTTP(rec, req) | 306 | + | })) |
| 307 | + | defer ts.Close() | |||
| 307 | 308 | ||||
| 308 | - | assert.Equal(t, http.StatusForbidden, rec.Code) | 309 | + | resp, err := ts.Client().Head(ts.URL + "/web") |
| 309 | - | assert.Empty(t, rec.Body.String(), "HEAD 403 should have empty body") | 310 | + | require.NoError(t, err) |
| 311 | + | ||||
| 312 | + | body, err := io.ReadAll(resp.Body) | |||
| 313 | + | require.NoError(t, err) | |||
| 314 | + | require.NoError(t, resp.Body.Close()) | |||
| 315 | + | ||||
| 316 | + | assert.Equal(t, http.StatusForbidden, resp.StatusCode) | |||
| 317 | + | assert.Empty(t, body, "HEAD 403 should have empty body") | |||
| 310 | } | 318 | } | ||
| 311 | 319 | ||||
| 312 | func TestCreateHTTPHandlers_DirectHTTPACMEChallenge_Returns404(t *testing.T) { | 320 | func TestCreateHTTPHandlers_DirectHTTPACMEChallenge_Returns404(t *testing.T) { | ||