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.

Harden the web transport and bound the pages that run git

701232cf37157ff392e3b35a3279735d72e585d4 · Keenan Nemetz · 13d ago · parent 6bfc4294

Harden the web transport and bound the pages that run git

Generated clone and LFS download links were derived from the request's
Host header, with X-Forwarded-Proto believed from anyone. Those links are
copied out of a page and pasted into a terminal, and the LFS hrefs are
followed by a client, so a forged Host chose where somebody else's
download went.

Links now use public_url when it is configured, which is how a production
instance is meant to be set up; without it the request origin is all there
is, and forwarded headers are honoured only when git_http_client_ip_header
is set, because that is the instance's statement that a proxy it trusts
rewrites them. Site links follow the forge's scheme instead of always
saying http, so an HTTPS forge does not link to a mixed-content site.

The security headers now close the rest of the surface: object-src is
none, inline styles are no longer allowed (no template or generated
markup contains one), and Permissions-Policy, COOP and CORP are set. The
CSRF check gained a second lock: when the browser says where a request
came from, that origin must be this instance, on form posts and on the
JSON ceremony endpoints alike. A missing Origin and Referer is not
evidence, so only a request that names somewhere else is refused.

The pages that shell out to git are bounded. Search, compare, blame, file
history, commit history and both archive formats take a slot from a pool
of eight and run under a 30-second deadline, which is attached to the git
handle so the process is killed rather than left running after the reader
has gone. Past a five-second wait for a slot the page answers 503 with
Retry-After. Plain tar now takes the archive slot too: it is cheaper than
gzip, not free, and both stream for as long as the client reads.

Tests: clone URLs ignore a forged Host when public_url is set and honour
forwarded headers only behind a declared proxy; LFS hrefs likewise; site
links follow the scheme; the CSP has no unsafe-inline and names
object-src; a saturated git pool answers 503 with Retry-After; a
cross-origin post is refused with a valid CSRF token, end to end; the
JSON endpoints refuse a foreign origin.

Docs: public_url now explains that it decides generated links and that
forwarded headers are only trusted with the proxy header set; Sites gains
a same-site risk section recommending a separate registrable domain when
sites are not trusted.

server/web/transport_test.go +149

@@ -0,0 +1,149 @@
1 package web
2
3 // What the browser is told about the transport, where generated links point, and
4 // what happens when too many pages want git at once.
5
6 import (
7 "net/http"
8 "net/http/httptest"
9 "strings"
10 "testing"
11 "time"
12
13 "github.com/rodent-software/vole/forge/config"
14 )
15
16 // TestCloneURLPrefersTheConfiguredOrigin: a link the browser hands to a terminal
17 // must not be a link an attacker chose with a Host header.
18 func TestCloneURLPrefersTheConfiguredOrigin(t *testing.T) {
19 t.Parallel()
20 w := &Web{Cfg: &config.Config{PublicURL: "https://forge.example.com"}}
21 r := httptest.NewRequest("GET", "http://internal:8080/r/demo", nil)
22 r.Host = "evil.example"
23 r.Header.Set("X-Forwarded-Proto", "http")
24 r.Header.Set("X-Forwarded-Host", "evil.example")
25 if got, want := w.cloneURL(r, "demo"), "https://forge.example.com/git/demo.git"; got != want {
26 t.Fatalf("cloneURL = %q, want %q", got, want)
27 }
28 }
29
30 // Without a public_url the origin is all there is. The forwarding header is only
31 // believed when the instance says a proxy is in front of it.
32 func TestCloneURLFallsBackToTheRequestOrigin(t *testing.T) {
33 t.Parallel()
34 r := httptest.NewRequest("GET", "http://127.0.0.1:8080/r/demo", nil)
35 r.Host = "127.0.0.1:8080"
36 r.Header.Set("X-Forwarded-Proto", "https")
37
38 w := &Web{Cfg: &config.Config{}}
39 if got, want := w.cloneURL(r, "demo"), "http://127.0.0.1:8080/git/demo.git"; got != want {
40 t.Fatalf("cloneURL without a proxy = %q, want %q", got, want)
41 }
42 w = &Web{Cfg: &config.Config{GitHTTPClientIPHeader: "X-Forwarded-For"}}
43 if got, want := w.cloneURL(r, "demo"), "https://127.0.0.1:8080/git/demo.git"; got != want {
44 t.Fatalf("cloneURL behind a declared proxy = %q, want %q", got, want)
45 }
46 }
47
48 // A site link follows the forge's scheme: an http link from an https page is a
49 // mixed-content warning, and the site is not actually served over http.
50 func TestSiteURLFollowsTheForgeScheme(t *testing.T) {
51 t.Parallel()
52 r := httptest.NewRequest("GET", "http://forge.example.com/", nil)
53 cases := []struct {
54 public string
55 want string
56 }{
57 {"", "http://demo.forge.example.com"},
58 {"https://forge.example.com", "https://demo.forge.example.com"},
59 }
60 for _, c := range cases {
61 w := &Web{Cfg: &config.Config{PublicURL: c.public, PagesDomain: "forge.example.com"}}
62 if got := w.siteURL(r, "demo"); got != c.want {
63 t.Fatalf("public_url %q: siteURL = %q, want %q", c.public, got, c.want)
64 }
65 }
66 if got := (&Web{Cfg: &config.Config{}}).siteURL(r, "demo"); got != "" {
67 t.Fatalf("sites are disabled but siteURL = %q", got)
68 }
69 }
70
71 func TestSecurityHeadersCloseTheSurface(t *testing.T) {
72 t.Parallel()
73 w := &Web{Cfg: &config.Config{PublicURL: "https://forge.example.com"}}
74 rw := httptest.NewRecorder()
75 w.securityHeaders(rw, httptest.NewRequest("GET", "https://forge.example.com/", nil))
76
77 csp := rw.Header().Get("Content-Security-Policy")
78 for _, want := range []string{"script-src 'self'", "style-src 'self'", "object-src 'none'", "base-uri 'none'"} {
79 if !strings.Contains(csp, want) {
80 t.Fatalf("CSP is missing %q: %s", want, csp)
81 }
82 }
83 if strings.Contains(csp, "unsafe-inline") {
84 t.Fatalf("CSP still allows inline styles or scripts: %s", csp)
85 }
86 for header, want := range map[string]string{
87 "Permissions-Policy": "camera=()",
88 "Cross-Origin-Opener-Policy": "same-origin",
89 "Cross-Origin-Resource-Policy": "same-origin",
90 } {
91 if got := rw.Header().Get(header); !strings.Contains(got, want) {
92 t.Fatalf("%s = %q, want it to contain %q", header, got, want)
93 }
94 }
95 }
96
97 // TestGitSlotRefusesWhenSaturated: the pool is a bound, so a page past it is told
98 // to come back rather than queued behind an unbounded number of readers.
99 func TestGitSlotRefusesWhenSaturated(t *testing.T) {
100 t.Parallel()
101 w := &Web{Cfg: &config.Config{}, gitSem: make(chan struct{}, 1), gitWait: 20 * time.Millisecond}
102 release, ok := w.gitSlot(httptest.NewRecorder(), httptest.NewRequest("GET", "/r/demo/search", nil))
103 if !ok {
104 t.Fatal("the first page could not take a slot")
105 }
106 rw := httptest.NewRecorder()
107 if _, ok := w.gitSlot(rw, httptest.NewRequest("GET", "/r/demo/search", nil)); ok {
108 t.Fatal("a page past the pool was accepted")
109 }
110 if rw.Code != http.StatusServiceUnavailable {
111 t.Fatalf("status = %d, want 503", rw.Code)
112 }
113 if rw.Header().Get("Retry-After") == "" {
114 t.Fatal("a busy answer without Retry-After")
115 }
116 release()
117 if _, ok := w.gitSlot(httptest.NewRecorder(), httptest.NewRequest("GET", "/r/demo/search", nil)); !ok {
118 t.Fatal("a released slot was not reusable")
119 }
120 }
121
122 // sameOriginRequest is the check beside the CSRF token: a request that names
123 // another origin is refused even with the right token.
124 func TestSameOriginRequest(t *testing.T) {
125 t.Parallel()
126 w := &Web{Cfg: &config.Config{PublicURL: "https://forge.example.com"}}
127 r := httptest.NewRequest("POST", "https://forge.example.com/logout", nil)
128
129 if !w.sameOriginRequest(r) {
130 t.Fatal("a request with no Origin or Referer was refused")
131 }
132 r.Header.Set("Origin", "https://forge.example.com")
133 if !w.sameOriginRequest(r) {
134 t.Fatal("same-origin request was refused")
135 }
136 r.Header.Set("Origin", "https://evil.example")
137 if w.sameOriginRequest(r) {
138 t.Fatal("a cross-origin request was accepted")
139 }
140 r.Header.Del("Origin")
141 r.Header.Set("Referer", "https://evil.example/page")
142 if w.sameOriginRequest(r) {
143 t.Fatal("a cross-origin referrer was accepted")
144 }
145 r.Header.Set("Origin", "null")
146 if w.sameOriginRequest(r) {
147 t.Fatal("the opaque origin was accepted")
148 }
149 }