Commit 993ef5f203
993ef5f2039e63851a3b7bfe8b62c7c5a8017aa0
parent: 3e9b5f03d6
Verified · cmc ci/build: success ci/test: success ci/vuln: success
cmc <hello@cleberg.net> · 2026-09-03 18:27 UTC
http.trusted_proxies, and a cap on verification mail per account
The API rate limiter keyed anonymous callers by peer address, which is
right with nothing in front of the daemon and wrong the day a reverse
proxy is added: every caller shares one bucket. http.trusted_proxies
lists the proxies' addresses or CIDRs; a request from one of them is
attributed to the last X-Forwarded-For hop that is not itself a proxy,
and from anyone else the header is ignored. Empty, the default, keeps
the peer-only behaviour.
email add enqueued a verification mail with no limit; an account may
now ask for five codes an hour.
TestClientIPBehindProxy covers the attribution and a bad entry
refused at config load; TestEmailAddThrottled the sixth request.
Ref #136
Layout: unified · split
e2e/emailthrottle_test.go
added
+28
| @@ -0,0 +1,28 @@ |
| 1 | package e2e |
| 2 | |
| 3 | import ( |
| 4 | "fmt" |
| 5 | "strings" |
| 6 | "testing" |
| 7 | ) |
| 8 | |
| 9 | // email add mails a verification code. SSH auth and registration are |
| 10 | // rate-limited; this path was not, so an authenticated account could |
| 11 | // enqueue mail without bound (#136). |
| 12 | func TestEmailAddThrottled(t *testing.T) { |
| 13 | smtp := startFakeSMTP(t) |
| 14 | inst := startInstanceWith(t, fmt.Sprintf( |
| 15 | "[mail]\nsmtp_host = %q\nfrom = \"noreply@gitbay.test\"\n", smtp.addr)) |
| 16 | aliceKey := inst.newKey(t, "alice") |
| 17 | inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub") |
| 18 | |
| 19 | for i := 0; i < 5; i++ { |
| 20 | if _, errOut, code := inst.ssh(t, aliceKey, "", "email", "add", fmt.Sprintf("alice%d@example.test", i)); code != 0 { |
| 21 | t.Fatalf("email add %d: exit %d %s", i, code, errOut) |
| 22 | } |
| 23 | } |
| 24 | _, errOut, code := inst.ssh(t, aliceKey, "", "email", "add", "alice5@example.test") |
| 25 | if code != 4 || !strings.Contains(errOut, "last hour") { |
| 26 | t.Fatalf("sixth email add in an hour: exit %d %s", code, errOut) |
| 27 | } |
| 28 | } |
internal/config/config.go
+32
| @@ -62,6 +62,13 @@ type HTTP struct { |
| 62 | 62 | // port still works). |
| 63 | 63 | ACMEEmail string `toml:"acme_email"` |
| 64 | 64 | ACMEHTTPAddr string `toml:"acme_http_addr"` |
| 65 | // TrustedProxies are the addresses or CIDRs of reverse proxies in front |
| 66 | // of this process. A request from one of them is attributed to the |
| 67 | // last X-Forwarded-For hop that is not itself a trusted proxy; from |
| 68 | // anyone else the peer address is the client and the header is |
| 69 | // ignored. Empty means no proxy, which is how gitbayd is deployed by |
| 70 | // default: it terminates TLS itself. |
| 71 | TrustedProxies []string `toml:"trusted_proxies,omitempty"` |
| 65 | 72 | } |
| 66 | 73 | |
| 67 | 74 | type GitDaemon struct { |
| @@ -238,6 +245,9 @@ func (c Config) Validate() error { |
| 238 | 245 | if err := oneOf("http.tls", c.HTTP.TLS, "acme", "files", "off"); err != nil { |
| 239 | 246 | errs = append(errs, err) |
| 240 | 247 | } |
| 248 | if _, err := c.HTTP.TrustedProxyNets(); err != nil { |
| 249 | errs = append(errs, err) |
| 250 | } |
| 241 | 251 | if c.HTTP.TLS == "files" && (c.HTTP.CertFile == "" || c.HTTP.KeyFile == "") { |
| 242 | 252 | errs = append(errs, errors.New("http.tls = \"files\" requires cert_file and key_file")) |
| 243 | 253 | } |
| @@ -328,3 +338,25 @@ func (c Config) CheckHost() error { |
| 328 | 338 | |
| 329 | 339 | return errors.Join(errs...) |
| 330 | 340 | } |
| 341 | |
| 342 | // TrustedProxyNets parses http.trusted_proxies; a bare address is a /32 |
| 343 | // or /128. |
| 344 | func (h HTTP) TrustedProxyNets() ([]*net.IPNet, error) { |
| 345 | var nets []*net.IPNet |
| 346 | for _, p := range h.TrustedProxies { |
| 347 | if _, n, err := net.ParseCIDR(p); err == nil { |
| 348 | nets = append(nets, n) |
| 349 | continue |
| 350 | } |
| 351 | ip := net.ParseIP(p) |
| 352 | if ip == nil { |
| 353 | return nil, fmt.Errorf("http.trusted_proxies: %q is not an address or CIDR", p) |
| 354 | } |
| 355 | bits := 32 |
| 356 | if ip.To4() == nil { |
| 357 | bits = 128 |
| 358 | } |
| 359 | nets = append(nets, &net.IPNet{IP: ip, Mask: net.CIDRMask(bits, bits)}) |
| 360 | } |
| 361 | return nets, nil |
| 362 | } |
internal/control/register.go
+9
| @@ -54,6 +54,8 @@ func sendVerification(cfg config.Config, st *store.Store, userID int64, address |
| 54 | 54 | return mail.Send(cfg, address, "verify your email on "+siteHost(cfg), body) |
| 55 | 55 | } |
| 56 | 56 | |
| 57 | const maxEmailAddsPerHour = 5 |
| 58 | |
| 57 | 59 | func runEmailAdd(c *Ctx, args []string) int { |
| 58 | 60 | if len(args) != 1 || !strings.Contains(args[0], "@") { |
| 59 | 61 | return c.fail(protocol.ExitUsage, "usage: email add <address>") |
| @@ -61,6 +63,13 @@ func runEmailAdd(c *Ctx, args []string) int { |
| 61 | 63 | if c.Cfg.Mail.SMTPHost == "" { |
| 62 | 64 | return c.fail(protocol.ExitFailure, "this instance has no SMTP configured; ask an admin to verify the address (gitbayd admin email verify)") |
| 63 | 65 | } |
| 66 | // An authenticated account is not a mail cannon: a handful of codes an |
| 67 | // hour is plenty for a person and nothing for a script (#136). |
| 68 | if n, err := c.Store.CountEmailTokensSince(c.User.ID, time.Now().Add(-time.Hour)); err != nil { |
| 69 | return c.fail(protocol.ExitFailure, "%v", err) |
| 70 | } else if n >= maxEmailAddsPerHour { |
| 71 | return c.fail(protocol.ExitDenied, "%d verification mails in the last hour; try again later", n) |
| 72 | } |
| 64 | 73 | if err := c.Store.AddEmail(c.User.ID, args[0], "", false); err != nil { |
| 65 | 74 | return c.fail(protocol.ExitFailure, "%v", err) |
| 66 | 75 | } |
internal/httpd/api.go
+3 −3
| @@ -55,7 +55,7 @@ func (s *Server) apiCmd(w http.ResponseWriter, r *http.Request) { |
| 55 | 55 | if cmd, _, ok := control.Lookup(req.Argv); ok { |
| 56 | 56 | write = !cmd.ReadOnly |
| 57 | 57 | } |
| 58 | | if allowed, wait := s.apiLimit.allow(limitKey(r, user), write); !allowed { |
| 58 | if allowed, wait := s.apiLimit.allow(s.limitKey(r, user), write); !allowed { |
| 59 | 59 | tooManyRequests(w, wait) |
| 60 | 60 | return |
| 61 | 61 | } |
| @@ -114,11 +114,11 @@ func statusForExit(code int) int { |
| 114 | 114 | |
| 115 | 115 | // limitKey buckets an authenticated caller by account, so rotating tokens |
| 116 | 116 | // buys no extra budget, and everyone else by peer address. |
| 117 | | func limitKey(r *http.Request, user store.User) string { |
| 117 | func (s *Server) limitKey(r *http.Request, user store.User) string { |
| 118 | 118 | if user.ID != 0 { |
| 119 | 119 | return "u" + strconv.FormatInt(user.ID, 10) |
| 120 | 120 | } |
| 121 | | return "ip" + clientIP(r) |
| 121 | return "ip" + s.clientIP(r) |
| 122 | 122 | } |
| 123 | 123 | |
| 124 | 124 | // apiAuth resolves the bearer token; failures are uniform 401s. |
internal/httpd/apilimit.go
+36 −7
| @@ -4,6 +4,7 @@ import ( |
| 4 | 4 | "net" |
| 5 | 5 | "net/http" |
| 6 | 6 | "strconv" |
| 7 | "strings" |
| 7 | 8 | "sync" |
| 8 | 9 | "time" |
| 9 | 10 | ) |
| @@ -102,14 +103,42 @@ func minf(a, b float64) float64 { |
| 102 | 103 | return b |
| 103 | 104 | } |
| 104 | 105 | |
| 105 | | // clientIP is the peer address. No forwarded headers are trusted: nothing |
| 106 | | // in front of this process is required to set them, and honouring a |
| 107 | | // client-supplied header would let a caller pick their own bucket. |
| 108 | | func clientIP(r *http.Request) string { |
| 109 | | if host, _, err := net.SplitHostPort(r.RemoteAddr); err == nil { |
| 110 | | return host |
| 106 | // clientIP is the address a request is attributed to. With no trusted |
| 107 | // proxies configured it is the peer, and forwarded headers are ignored: |
| 108 | // honouring a client-supplied header would let a caller pick their own |
| 109 | // bucket. When the peer is a trusted proxy, it is the last |
| 110 | // X-Forwarded-For hop that is not itself a trusted proxy, so a proxied |
| 111 | // deployment does not collapse every anonymous caller into one bucket |
| 112 | // (#136). |
| 113 | func (s *Server) clientIP(r *http.Request) string { |
| 114 | peer := r.RemoteAddr |
| 115 | if host, _, err := net.SplitHostPort(peer); err == nil { |
| 116 | peer = host |
| 111 | 117 | } |
| 112 | | return r.RemoteAddr |
| 118 | if !s.trustedProxy(peer) { |
| 119 | return peer |
| 120 | } |
| 121 | hops := strings.Split(r.Header.Get("X-Forwarded-For"), ",") |
| 122 | for i := len(hops) - 1; i >= 0; i-- { |
| 123 | hop := strings.TrimSpace(hops[i]) |
| 124 | if hop != "" && !s.trustedProxy(hop) { |
| 125 | return hop |
| 126 | } |
| 127 | } |
| 128 | return peer |
| 129 | } |
| 130 | |
| 131 | func (s *Server) trustedProxy(addr string) bool { |
| 132 | ip := net.ParseIP(addr) |
| 133 | if ip == nil { |
| 134 | return false |
| 135 | } |
| 136 | for _, n := range s.proxies { |
| 137 | if n.Contains(ip) { |
| 138 | return true |
| 139 | } |
| 140 | } |
| 141 | return false |
| 113 | 142 | } |
| 114 | 143 | |
| 115 | 144 | func tooManyRequests(w http.ResponseWriter, wait time.Duration) { |
internal/httpd/apiread.go
+2 −2
| @@ -44,7 +44,7 @@ func (s *Server) apiRead(w http.ResponseWriter, r *http.Request) { |
| 44 | 44 | joinArgv(cmd.Path)+" changes state; POST it to /api/v1/cmd") |
| 45 | 45 | return |
| 46 | 46 | } |
| 47 | | if allowed, wait := s.apiLimit.allow(limitKey(r, user), false); !allowed { |
| 47 | if allowed, wait := s.apiLimit.allow(s.limitKey(r, user), false); !allowed { |
| 48 | 48 | tooManyRequests(w, wait) |
| 49 | 49 | return |
| 50 | 50 | } |
| @@ -82,7 +82,7 @@ func (s *Server) apiRead(w http.ResponseWriter, r *http.Request) { |
| 82 | 82 | // Responses are authorized per account, so the ETag is salted with the |
| 83 | 83 | // caller: two users asking the same question may get different answers, |
| 84 | 84 | // and neither should ever be served the other's. |
| 85 | | sum := sha256.Sum256(append([]byte(limitKey(r, user)+"\x00"), payload...)) |
| 85 | sum := sha256.Sum256(append([]byte(s.limitKey(r, user)+"\x00"), payload...)) |
| 86 | 86 | etag := `"` + hex.EncodeToString(sum[:16]) + `"` |
| 87 | 87 | |
| 88 | 88 | // private keeps this out of shared caches; no-cache requires a |
internal/httpd/clientip_test.go
added
+46
| @@ -0,0 +1,46 @@ |
| 1 | package httpd |
| 2 | |
| 3 | import ( |
| 4 | "net/http/httptest" |
| 5 | "testing" |
| 6 | |
| 7 | "gitbay.org/gitbay/internal/config" |
| 8 | ) |
| 9 | |
| 10 | // With no trusted proxies the peer is the client and X-Forwarded-For is |
| 11 | // ignored; behind a trusted proxy the client is the last hop that is not |
| 12 | // itself a proxy, so a spoofed leading hop still cannot pick a bucket. |
| 13 | func TestClientIPBehindProxy(t *testing.T) { |
| 14 | cases := []struct { |
| 15 | proxies []string |
| 16 | remote string |
| 17 | xff string |
| 18 | want string |
| 19 | }{ |
| 20 | {nil, "203.0.113.9:4000", "198.51.100.1", "203.0.113.9"}, |
| 21 | {[]string{"10.0.0.0/8"}, "10.1.2.3:4000", "198.51.100.1", "198.51.100.1"}, |
| 22 | {[]string{"10.0.0.0/8"}, "10.1.2.3:4000", "198.51.100.1, 10.9.9.9", "198.51.100.1"}, |
| 23 | {[]string{"10.0.0.0/8"}, "10.1.2.3:4000", "1.1.1.1, 198.51.100.1", "198.51.100.1"}, |
| 24 | {[]string{"10.0.0.0/8"}, "10.1.2.3:4000", "", "10.1.2.3"}, |
| 25 | {[]string{"10.0.0.0/8"}, "203.0.113.9:4000", "198.51.100.1", "203.0.113.9"}, |
| 26 | {[]string{"127.0.0.1"}, "127.0.0.1:4000", "198.51.100.1", "198.51.100.1"}, |
| 27 | } |
| 28 | for _, tc := range cases { |
| 29 | cfg := config.Default() |
| 30 | cfg.HTTP.TrustedProxies = tc.proxies |
| 31 | s := New(cfg, nil) |
| 32 | r := httptest.NewRequest("GET", "/api/v1/read", nil) |
| 33 | r.RemoteAddr = tc.remote |
| 34 | if tc.xff != "" { |
| 35 | r.Header.Set("X-Forwarded-For", tc.xff) |
| 36 | } |
| 37 | if got := s.clientIP(r); got != tc.want { |
| 38 | t.Errorf("proxies=%v remote=%s xff=%q: got %s, want %s", tc.proxies, tc.remote, tc.xff, got, tc.want) |
| 39 | } |
| 40 | } |
| 41 | cfg := config.Default() |
| 42 | cfg.HTTP.TrustedProxies = []string{"not-an-address"} |
| 43 | if err := cfg.Validate(); err == nil { |
| 44 | t.Error("bad trusted_proxies entry accepted") |
| 45 | } |
| 46 | } |
internal/httpd/smart.go
+4 −1
| @@ -9,6 +9,7 @@ import ( |
| 9 | 9 | "compress/gzip" |
| 10 | 10 | "fmt" |
| 11 | 11 | "io" |
| 12 | "net" |
| 12 | 13 | "net/http" |
| 13 | 14 | "os" |
| 14 | 15 | "os/exec" |
| @@ -23,10 +24,12 @@ type Server struct { |
| 23 | 24 | cfg config.Config |
| 24 | 25 | st *store.Store |
| 25 | 26 | apiLimit *apiLimiter |
| 27 | proxies []*net.IPNet // http.trusted_proxies, parsed once |
| 26 | 28 | } |
| 27 | 29 | |
| 28 | 30 | func New(cfg config.Config, st *store.Store) *Server { |
| 29 | | return &Server{cfg: cfg, st: st, apiLimit: newAPILimiter(cfg.Limits.APIRate)} |
| 31 | proxies, _ := cfg.HTTP.TrustedProxyNets() // validated at config load |
| 32 | return &Server{cfg: cfg, st: st, apiLimit: newAPILimiter(cfg.Limits.APIRate), proxies: proxies} |
| 30 | 33 | } |
| 31 | 34 | |
| 32 | 35 | // receivePackRefusal exists only to fail legibly if a client POSTs without |
internal/store/registration.go
+9
| @@ -36,6 +36,15 @@ func (s *Store) CreateEmailToken(userID int64, address, tokenHash string, ttl ti |
| 36 | 36 | return err |
| 37 | 37 | } |
| 38 | 38 | |
| 39 | // CountEmailTokensSince is how many verification codes an account has |
| 40 | // asked for since a moment, used or not. |
| 41 | func (s *Store) CountEmailTokensSince(userID int64, since time.Time) (int, error) { |
| 42 | var n int |
| 43 | err := s.DB.QueryRow("SELECT count(*) FROM email_tokens WHERE user_id = ? AND created_at > ?", |
| 44 | userID, fmtTime(since)).Scan(&n) |
| 45 | return n, err |
| 46 | } |
| 47 | |
| 39 | 48 | // ConsumeEmailToken redeems a verification code for the given user. |
| 40 | 49 | func (s *Store) ConsumeEmailToken(userID int64, tokenHash string) (string, error) { |
| 41 | 50 | res, err := s.DB.Exec(` |