Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.
vole
Clone

Read only, no credentials:

https://rodent.software/vole.git

Write over SSH with a registered key:

git clone ssh://git@rodent.software:2222/vole.git

A free and open-source forge that lives in your Git repo.

Bound the anonymous Git HTTP read path

86991e7020a418a08a5e3a36ce8f296561392e3b · Keenan Nemetz · 13d ago · parent e05f1c48 

Bound the anonymous Git HTTP read path

The transport that needs no account had no bound on anything: the request body
was capped only at "not negative", the backend ran without a request context or
deadline, the path check accepted any suffix after `git-upload-pack` and handed
it to the CGI as `PATH_INFO`, and the server set no `ReadTimeout` and no
`MaxHeaderBytes`.

Bound what the read side can spend, which is every dimension available to a
fetch and none of the ones a push would need: a 2 MiB body cap with a required
`Content-Length`, exactly two endpoints with one method and one service each so
the CGI never sees an unvalidated path, `exec.CommandContext` plus `WaitDelay`
plus an explicit kill so a disconnect or an expired budget stops git instead of
leaving it running, and a concurrency semaphore with a per-client bound that
answers 503 with Retry-After rather than queueing.

The client address is the connection's own unless the operator names a proxy
header, because behind a proxy the connection address is the proxy. Both limits
and the header name are configuration, `git_http` turns the transport off
entirely, and `status` lists them so an operator can see what is in force.

`WriteTimeout` is deliberately not set: clone, archives, LFS objects and site
files are streamed, so those flows are bounded where they are produced.

server/githttp/limits_test.go +251

@@ -0,0 +1,251 @@
1 package githttp
2
3 // Every resource the anonymous read path can spend, and the test that it is
4 // bounded. These are all white-box: the point is the bound, not the wire format,
5 // and a proxy that accepts a body without limit fails these before it can fail a
6 // production instance.
7
8 import (
9 "context"
10 "io"
11 "log/slog"
12 "net/http"
13 "net/http/httptest"
14 "path/filepath"
15 "strings"
16 "testing"
17 "time"
18
19 "github.com/rodent-software/vole/core/git"
20 "github.com/rodent-software/vole/forge/access"
21 "github.com/rodent-software/vole/forge/repo"
22 )
23
24 // newProxyWith builds the same fixture newProxy does, with a hook to adjust the
25 // bounds, and returns the repository directory so a test can name a real commit.
26 func newProxyWith(t *testing.T, tweak func(*Config)) (*Proxy, string) {
27 t.Helper()
28 root := filepath.Join(t.TempDir(), "repos")
29 acls, err := access.Open(filepath.Join(filepath.Dir(root), "users.db"))
30 if err != nil {
31 t.Fatal(err)
32 }
33 t.Cleanup(func() { acls.Close() })
34 makeRepo(t, root, true, acls)
35 st := repo.New(root, "/bin/true", acls)
36 cfg := Config{
37 Root: root,
38 Open: st.Open,
39 Log: slog.New(slog.NewTextHandler(io.Discard, nil)),
40 }
41 if tweak != nil {
42 tweak(&cfg)
43 }
44 return New(cfg), filepath.Join(root, "demo.git")
45 }
46
47 func TestOversizedBodyRefused(t *testing.T) {
48 t.Parallel()
49 p := newProxy(t, true)
50 req := httptest.NewRequest(http.MethodPost, "/git/demo.git/git-upload-pack",
51 strings.NewReader(strings.Repeat("x", maxUploadPackBody+1)))
52 req.ContentLength = maxUploadPackBody + 1
53 rec := httptest.NewRecorder()
54 p.ServeHTTP(rec, req)
55 if rec.Code != http.StatusRequestEntityTooLarge {
56 t.Fatalf("status = %d, want 413", rec.Code)
57 }
58 }
59
60 func TestEmptyPostRefused(t *testing.T) {
61 t.Parallel()
62 p := newProxy(t, true)
63 req := httptest.NewRequest(http.MethodPost, "/git/demo.git/git-upload-pack", strings.NewReader(""))
64 req.ContentLength = 0
65 rec := httptest.NewRecorder()
66 p.ServeHTTP(rec, req)
67 if rec.Code != http.StatusRequestEntityTooLarge {
68 t.Fatalf("status = %d, want 413 for a zero-length fetch", rec.Code)
69 }
70 }
71
72 // TestOnlyTheTwoEndpointsExist covers the path validation. The old check accepted
73 // anything that started with git-upload-pack, and passed the rest of the path to
74 // the CGI as PATH_INFO.
75 func TestOnlyTheTwoEndpointsExist(t *testing.T) {
76 t.Parallel()
77 p := newProxy(t, true)
78 cases := []struct {
79 method, target string
80 code int
81 }{
82 {http.MethodGet, "/git/demo.git/info/refs?service=git-upload-pack", http.StatusOK},
83 {http.MethodGet, "/git/demo.git/info/refs", http.StatusNotFound},
84 {http.MethodGet, "/git/demo.git/info/refs?service=git-upload-pack&service=evil", http.StatusNotFound},
85 {http.MethodGet, "/git/demo.git/git-upload-pack", http.StatusNotFound},
86 {http.MethodPost, "/git/demo.git/info/refs?service=git-upload-pack", http.StatusNotFound},
87 {http.MethodGet, "/git/demo.git/git-upload-pack/../../git-receive-pack", http.StatusNotFound},
88 {http.MethodGet, "/git/demo.git/../demo.git/info/refs", http.StatusNotFound},
89 {http.MethodGet, "/git/demo.git/git-upload-pack/objects/info/packs", http.StatusNotFound},
90 {http.MethodGet, "/git/demo.git/objects/info/packs", http.StatusNotFound},
91 {http.MethodGet, "/git/demo.git/HEAD", http.StatusNotFound},
92 {http.MethodPost, "/git/demo.git/git-receive-pack", http.StatusForbidden},
93 }
94 for _, c := range cases {
95 req := httptest.NewRequest(c.method, c.target, nil)
96 if c.method == http.MethodPost {
97 req.ContentLength = 4
98 req.Body = io.NopCloser(strings.NewReader("want"))
99 }
100 rec := httptest.NewRecorder()
101 p.ServeHTTP(rec, req)
102 if rec.Code != c.code {
103 t.Errorf("%s %s = %d, want %d", c.method, c.target, rec.Code, c.code)
104 }
105 }
106 }
107
108 // TestConcurrentClonesAreBounded fills the transport and asks for one more. A
109 // refused clone is a 503 with Retry-After, not a queued goroutine.
110 func TestConcurrentClonesAreBounded(t *testing.T) {
111 t.Parallel()
112 p, _ := newProxyWith(t, func(c *Config) { c.MaxConcurrent = 1; c.MaxPerClient = 1 })
113 p.sem <- struct{}{}
114 t.Cleanup(func() { <-p.sem })
115
116 req := httptest.NewRequest(http.MethodGet, "/git/demo.git/info/refs?service=git-upload-pack", nil)
117 rec := httptest.NewRecorder()
118 p.ServeHTTP(rec, req)
119 if rec.Code != http.StatusServiceUnavailable {
120 t.Fatalf("status = %d, want 503", rec.Code)
121 }
122 if rec.Header().Get("Retry-After") == "" {
123 t.Fatal("a refused clone did not say when to come back")
124 }
125 if p.Stats()["refused_busy"] != 1 {
126 t.Fatalf("refused_busy = %d", p.Stats()["refused_busy"])
127 }
128 }
129
130 // TestPerClientClonesAreBounded: one address cannot take every transport slot.
131 func TestPerClientClonesAreBounded(t *testing.T) {
132 t.Parallel()
133 p, _ := newProxyWith(t, func(c *Config) { c.MaxConcurrent = 4; c.MaxPerClient = 1 })
134 ip := clientIP(httptest.NewRequest(http.MethodGet, "/", nil), "")
135 p.mu.Lock()
136 p.perClient[ip] = 1
137 p.mu.Unlock()
138
139 req := httptest.NewRequest(http.MethodGet, "/git/demo.git/info/refs?service=git-upload-pack", nil)
140 rec := httptest.NewRecorder()
141 p.ServeHTTP(rec, req)
142 if rec.Code != http.StatusServiceUnavailable {
143 t.Fatalf("status = %d, want 503 for the second clone from one address", rec.Code)
144 }
145 // The transport slot must be handed back, not leaked by the refusal.
146 if len(p.sem) != 0 {
147 t.Fatalf("semaphore holds %d slots after a refused clone", len(p.sem))
148 }
149 }
150
151 // TestProxyHeaderNamesTheClient covers the operator-facing knob: behind a proxy
152 // every request shares one connection address, so the limits need the address the
153 // proxy saw — and only from the header the operator named.
154 func TestProxyHeaderNamesTheClient(t *testing.T) {
155 t.Parallel()
156 req := httptest.NewRequest(http.MethodGet, "/", nil)
157 req.RemoteAddr = "127.0.0.1:5000"
158 req.Header.Set("X-Forwarded-For", "203.0.113.9, 198.51.100.7")
159 if got := clientIP(req, ""); got != "127.0.0.1" {
160 t.Fatalf("untrusted header changed the client: %q", got)
161 }
162 if got := clientIP(req, "X-Forwarded-For"); got != "198.51.100.7" {
163 t.Fatalf("client from header = %q, want the last entry (the nearest proxy's)", got)
164 }
165 }
166
167 // stalledFetch is a fetch that cannot finish: a valid want line and no flush-pkt,
168 // so upload-pack waits for input that never comes.
169 func stalledFetch(t *testing.T, dir string) (*http.Request, func()) {
170 t.Helper()
171 g := git.Open(dir)
172 sha, err := g.RevParse("refs/heads/main")
173 if err != nil || sha == "" {
174 t.Fatalf("rev-parse: %q %v", sha, err)
175 }
176 pr, pw := io.Pipe()
177 body := io.MultiReader(
178 strings.NewReader("want "+sha+" side-band-64k ofs-delta agent=git/2.0\n"),
179 pr,
180 )
181 req := httptest.NewRequest(http.MethodPost, "/git/demo.git/git-upload-pack", pipeBody{r: body, c: pr})
182 req.ContentLength = 4096
183 req.Header.Set("Content-Type", "application/x-git-upload-pack-request")
184 return req, func() { pw.Close() }
185 }
186
187 // pipeBody keeps the pipe's Close, which io.MultiReader drops: the backend closes
188 // the request body to unblock its own copy when the request is over.
189 type pipeBody struct {
190 r io.Reader
191 c io.Closer
192 }
193
194 func (b pipeBody) Read(p []byte) (int, error) { return b.r.Read(p) }
195 func (b pipeBody) Close() error { return b.c.Close() }
196
197 // TestClientDisconnectStopsTheBackend: a client that walks away must not leave a
198 // git process holding a pack open. Without the request context this blocks until
199 // the process times out on its own, which is forever.
200 func TestClientDisconnectStopsTheBackend(t *testing.T) {
201 t.Parallel()
202 p, dir := newProxyWith(t, nil)
203 req, cleanup := stalledFetch(t, dir)
204 t.Cleanup(cleanup)
205 ctx, cancel := context.WithCancel(req.Context())
206 t.Cleanup(cancel)
207 req = req.WithContext(ctx)
208
209 done := make(chan struct{})
210 go func() {
211 defer close(done)
212 p.ServeHTTP(httptest.NewRecorder(), req)
213 }()
214 time.Sleep(100 * time.Millisecond) // let the backend start and block
215 cancel()
216 select {
217 case <-done:
218 case <-time.After(10 * time.Second):
219 t.Fatal("a disconnected clone kept the backend running")
220 }
221 }
222
223 // TestRequestTimeoutEndsAStalledClone: a fetch that never finishes is stopped
224 // with an answer rather than holding a slot for hours.
225 func TestRequestTimeoutEndsAStalledClone(t *testing.T) {
226 t.Parallel()
227 p, dir := newProxyWith(t, func(c *Config) { c.RequestTimeout = 300 * time.Millisecond })
228 req, cleanup := stalledFetch(t, dir)
229 t.Cleanup(cleanup)
230
231 rec := httptest.NewRecorder()
232 start := time.Now()
233 p.ServeHTTP(rec, req)
234 elapsed := time.Since(start)
235 if elapsed > 5*time.Second {
236 t.Fatalf("a stalled fetch took %s to be stopped", elapsed)
237 }
238 // The backend had already written its headers before it stalled, so the status
239 // cannot become 504: the client sees a 200 and a truncated body, which is what
240 // a real clone sees when the server walks away mid-pack.
241 if rec.Code != http.StatusOK {
242 t.Fatalf("status = %d", rec.Code)
243 }
244 // Past the handshake there is nothing to receive, and the process is gone.
245 if body := rec.Body.Bytes(); len(body) > 1<<10 {
246 t.Fatalf("a stopped fetch still delivered %d bytes", len(body))
247 }
248 if p.Stats()["timed_out"] != 1 {
249 t.Fatalf("timed_out = %d", p.Stats()["timed_out"])
250 }
251 }