Commit ead6796abe
Verified · cmc
Layout: unified · split
.gitbay/wiki/Admin.org +6 −2
| @@ -276,8 +276,12 @@ meaningful — an unverified address never produces a =verified= badge. | |||
| 276 | The audit log is the security feed (events are the product feed): every | 276 | The audit log is the security feed (events are the product feed): every |
| 277 | successful mutating command with its argv and source credential (SSH key | 277 | successful mutating command with its argv and source credential (SSH key |
| 278 | fingerprint or API), every refused one (exit 3 or 4) as =refused | 278 | fingerprint or API), every refused one (exit 3 or 4) as =refused |
| 279 | <command>=, refused pushes as =refused git-receive-pack=, registrations, | 279 | <command>=, refused pushes as =refused git-receive-pack= (the access |
| 280 | admin actions, force-pushes, and auth failures/throttling. A refusal row | 280 | check) or =refused push= (a branch or tag rule, a release anchor or an |
| 281 | unsigned commit, with the repository and ref names), hook socket | ||
| 282 | requests failing the peer or push-token check as =refused hook= (never | ||
| 283 | the token), registrations, admin actions, force-pushes, and auth | ||
| 284 | failures/throttling. A refusal row | ||
| 281 | keeps the flag names and the first positional, not the values. | 285 | keeps the flag names and the first positional, not the values. |
| 282 | Refusals are recorded up to ten a minute per account and 600 a minute | 286 | Refusals are recorded up to ten a minute per account and 600 a minute |
| 283 | across the instance; past either, one =refused.throttled= row stands | 287 | across the instance; past either, one =refused.throttled= row stands |
CHANGELOG.org +4
| @@ -108,6 +108,10 @@ missing, =gitbayd admin backup --verify <archive>= names it, and | |||
| 108 | =refused.throttled= row stands for the rest of the minute. Under | 108 | =refused.throttled= row stands for the rest of the minute. Under |
| 109 | =ssh.mode = "system"= each =gitbayd shell= connection counts | 109 | =ssh.mode = "system"= each =gitbayd shell= connection counts |
| 110 | separately (#275). | 110 | separately (#275). |
| 111 | - Pushes refused in pre-receive (a branch or tag rule, a release | ||
| 112 | anchor, an unsigned commit) are audited as =refused push= with the | ||
| 113 | repository and ref names, and hook socket requests failing the peer | ||
| 114 | or push-token check as =refused hook=, under the same caps (#275). | ||
| 111 | - Audit retention deletes by id, up to the newest row older than the | 115 | - Audit retention deletes by id, up to the newest row older than the |
| 112 | retention, so a clock step back cannot leave a gap in the chain (#275). | 116 | retention, so a clock step back cannot leave a gap in the chain (#275). |
| 113 | - =dashboard= and =feed= print activity as sentences | 117 | - =dashboard= and =feed= print activity as sentences |
internal/hookd/hookd.go +30 −10
| @@ -122,8 +122,10 @@ func (s *Server) handle(conn net.Conn) { | |||
| 122 | defer conn.Close() | 122 | defer conn.Close() |
| 123 | dec := json.NewDecoder(conn) | 123 | dec := json.NewDecoder(conn) |
| 124 | enc := json.NewEncoder(conn) | 124 | enc := json.NewEncoder(conn) |
| 125 | if err := checkPeer(conn); err != nil { | 125 | if err := peerCheck(conn); err != nil { |
| 126 | slog.Warn("hook socket: refused connection", "err", err) | 126 | slog.Warn("hook socket: refused connection", "err", err) |
| 127 | // Nothing about the request is known yet, and no account. | ||
| 128 | control.AuditRefused(s.st, 0, "refused hook", map[string]any{"reason": err.Error()}) | ||
| 127 | enc.Encode(Response{Allow: false, Message: "hook socket: " + err.Error()}) | 129 | enc.Encode(Response{Allow: false, Message: "hook socket: " + err.Error()}) |
| 128 | return | 130 | return |
| 129 | } | 131 | } |
| @@ -132,7 +134,9 @@ func (s *Server) handle(conn net.Conn) { | |||
| 132 | enc.Encode(Response{Allow: false, Message: "bad hook request"}) | 134 | enc.Encode(Response{Allow: false, Message: "bad hook request"}) |
| 133 | return | 135 | return |
| 134 | } | 136 | } |
| 135 | if msg := s.authorize(req); msg != "" { | 137 | if actor, msg := s.authorize(req); msg != "" { |
| 138 | control.AuditRefused(s.st, actor, "refused hook", | ||
| 139 | map[string]any{"repo_id": req.RepoID, "hook": req.Hook, "reason": msg}) | ||
| 136 | enc.Encode(Response{Allow: false, Message: msg}) | 140 | enc.Encode(Response{Allow: false, Message: msg}) |
| 137 | return | 141 | return |
| 138 | } | 142 | } |
| @@ -149,18 +153,34 @@ func (s *Server) handle(conn net.Conn) { | |||
| 149 | 153 | ||
| 150 | // authorize ties a request to a receive-pack sshd started: its token | 154 | // authorize ties a request to a receive-pack sshd started: its token |
| 151 | // must be live and name the same repository, account and key scope. | 155 | // must be live and name the same repository, account and key scope. |
| 152 | func (s *Server) authorize(req Request) string { | 156 | // On a refusal actor is the token's account when the token is live, |
| 157 | // and 0 otherwise: the request's own user id is only a claim. | ||
| 158 | func (s *Server) authorize(req Request) (actor int64, msg string) { | ||
| 153 | if req.Token == "" { | 159 | if req.Token == "" { |
| 154 | return "push not started by this server" | 160 | return 0, "push not started by this server" |
| 155 | } | 161 | } |
| 156 | tok, err := s.st.PushTokenByHash(store.HashToken(req.Token)) | 162 | tok, err := s.st.PushTokenByHash(store.HashToken(req.Token)) |
| 157 | if err != nil { | 163 | if err != nil { |
| 158 | return "push not started by this server" | 164 | return 0, "push not started by this server" |
| 159 | } | 165 | } |
| 160 | if tok.RepoID != req.RepoID || tok.UserID != req.UserID || tok.Scope != req.Scope { | 166 | if tok.RepoID != req.RepoID || tok.UserID != req.UserID || tok.Scope != req.Scope { |
| 161 | return "push token does not match this request" | 167 | return tok.UserID, "push token does not match this request" |
| 162 | } | 168 | } |
| 163 | return "" | 169 | return 0, "" |
| 170 | } | ||
| 171 | |||
| 172 | // peerCheck is checkPeer; tests replace it. | ||
| 173 | var peerCheck = checkPeer | ||
| 174 | |||
| 175 | // refusePush answers a pre-receive refusal and audits it. | ||
| 176 | func (s *Server) refusePush(enc *json.Encoder, req Request, repo store.Repo, msg string) { | ||
| 177 | refs := make([]string, len(req.Updates)) | ||
| 178 | for i, u := range req.Updates { | ||
| 179 | refs[i] = u.Ref | ||
| 180 | } | ||
| 181 | control.AuditRefused(s.st, req.UserID, "refused push", | ||
| 182 | map[string]any{"repo": repo.Path(), "refs": refs, "reason": msg}) | ||
| 183 | enc.Encode(Response{Allow: false, Message: msg}) | ||
| 164 | } | 184 | } |
| 165 | 185 | ||
| 166 | func (s *Server) preReceive(req Request, dec *json.Decoder, enc *json.Encoder) { | 186 | func (s *Server) preReceive(req Request, dec *json.Decoder, enc *json.Encoder) { |
| @@ -170,11 +190,11 @@ func (s *Server) preReceive(req Request, dec *json.Decoder, enc *json.Encoder) { | |||
| 170 | return | 190 | return |
| 171 | } | 191 | } |
| 172 | if msg := policy.CheckPush(repo, req.Updates); msg != "" { | 192 | if msg := policy.CheckPush(repo, req.Updates); msg != "" { |
| 173 | enc.Encode(Response{Allow: false, Message: msg}) | 193 | s.refusePush(enc, req, repo, msg) |
| 174 | return | 194 | return |
| 175 | } | 195 | } |
| 176 | if msg := s.releaseAnchors(repo, req.Updates); msg != "" { | 196 | if msg := s.releaseAnchors(repo, req.Updates); msg != "" { |
| 177 | enc.Encode(Response{Allow: false, Message: msg}) | 197 | s.refusePush(enc, req, repo, msg) |
| 178 | return | 198 | return |
| 179 | } | 199 | } |
| 180 | if !repo.Settings.RequireSignedCommits { | 200 | if !repo.Settings.RequireSignedCommits { |
| @@ -220,7 +240,7 @@ func (s *Server) preReceive(req Request, dec *json.Decoder, enc *json.Encoder) { | |||
| 220 | } | 240 | } |
| 221 | } | 241 | } |
| 222 | if refusal != "" { | 242 | if refusal != "" { |
| 223 | enc.Encode(Response{Allow: false, Message: refusal}) | 243 | s.refusePush(enc, req, repo, refusal) |
| 224 | return | 244 | return |
| 225 | } | 245 | } |
| 226 | enc.Encode(Response{Allow: true}) | 246 | enc.Encode(Response{Allow: true}) |
internal/hookd/socket_test.go +58
| @@ -1,12 +1,15 @@ | |||
| 1 | package hookd | 1 | package hookd |
| 2 | 2 | ||
| 3 | import ( | 3 | import ( |
| 4 | "errors" | ||
| 5 | "net" | ||
| 4 | "os" | 6 | "os" |
| 5 | "path/filepath" | 7 | "path/filepath" |
| 6 | "strings" | 8 | "strings" |
| 7 | "testing" | 9 | "testing" |
| 8 | 10 | ||
| 9 | "gitbay.org/gitbay/internal/config" | 11 | "gitbay.org/gitbay/internal/config" |
| 12 | "gitbay.org/gitbay/internal/policy" | ||
| 10 | "gitbay.org/gitbay/internal/store" | 13 | "gitbay.org/gitbay/internal/store" |
| 11 | ) | 14 | ) |
| 12 | 15 | ||
| @@ -103,3 +106,58 @@ func TestHookRequestNeedsItsPushToken(t *testing.T) { | |||
| 103 | t.Fatalf("finished push: %+v, %v", resp, err) | 106 | t.Fatalf("finished push: %+v, %v", resp, err) |
| 104 | } | 107 | } |
| 105 | } | 108 | } |
| 109 | |||
| 110 | func refusedRows(t *testing.T, st *store.Store, action string) []store.AuditEntry { | ||
| 111 | t.Helper() | ||
| 112 | rows, err := st.AuditEntries(store.AuditFilter{ActionPrefix: action, Limit: 10}) | ||
| 113 | if err != nil { | ||
| 114 | t.Fatal(err) | ||
| 115 | } | ||
| 116 | return rows | ||
| 117 | } | ||
| 118 | |||
| 119 | // Refused hook requests and refused pushes are audited; the token never | ||
| 120 | // lands in a row (#275). | ||
| 121 | func TestHookRefusalsAreAudited(t *testing.T) { | ||
| 122 | sock, st, repoID, uid := serveSocket(t) | ||
| 123 | |||
| 124 | forged := Request{Hook: "pre-receive", RepoID: repoID, UserID: uid, Scope: "full", Token: "not-a-live-token"} | ||
| 125 | if resp, err := Ask(sock, forged, nil); err != nil || resp.Allow { | ||
| 126 | t.Fatalf("forged: %+v, %v", resp, err) | ||
| 127 | } | ||
| 128 | rows := refusedRows(t, st, "refused hook") | ||
| 129 | if len(rows) != 1 || rows[0].Actor != "" || strings.Contains(rows[0].Data, forged.Token) || | ||
| 130 | !strings.Contains(rows[0].Data, "not started by this server") || !strings.Contains(rows[0].Data, `"hook":"pre-receive"`) { | ||
| 131 | t.Fatalf("refused hook rows: %+v", rows) | ||
| 132 | } | ||
| 133 | |||
| 134 | token, err := st.CreatePushToken(repoID, uid, "full") | ||
| 135 | if err != nil { | ||
| 136 | t.Fatal(err) | ||
| 137 | } | ||
| 138 | req := Request{Hook: "pre-receive", RepoID: repoID, UserID: uid, Scope: "full", Token: token, | ||
| 139 | Updates: []policy.RefUpdate{{Ref: "refs/merge-requests/1/head", Old: zeroSHA40, New: strings.Repeat("a", 40)}}} | ||
| 140 | if resp, err := Ask(sock, req, nil); err != nil || resp.Allow { | ||
| 141 | t.Fatalf("push to a server-owned ref: %+v, %v", resp, err) | ||
| 142 | } | ||
| 143 | rows = refusedRows(t, st, "refused push") | ||
| 144 | if len(rows) != 1 || rows[0].Actor != "alice" || strings.Contains(rows[0].Data, token) || | ||
| 145 | !strings.Contains(rows[0].Data, "alice/app") || !strings.Contains(rows[0].Data, "refs/merge-requests/1/head") { | ||
| 146 | t.Fatalf("refused push rows: %+v", rows) | ||
| 147 | } | ||
| 148 | } | ||
| 149 | |||
| 150 | // A connection from another uid is audited with no actor. | ||
| 151 | func TestPeerRefusalIsAudited(t *testing.T) { | ||
| 152 | old := peerCheck | ||
| 153 | peerCheck = func(net.Conn) error { return errors.New("peer uid not permitted") } | ||
| 154 | t.Cleanup(func() { peerCheck = old }) | ||
| 155 | sock, st, repoID, uid := serveSocket(t) | ||
| 156 | if resp, err := Ask(sock, Request{Hook: "pre-receive", RepoID: repoID, UserID: uid}, nil); err != nil || resp.Allow { | ||
| 157 | t.Fatalf("refused peer: %+v, %v", resp, err) | ||
| 158 | } | ||
| 159 | rows := refusedRows(t, st, "refused hook") | ||
| 160 | if len(rows) != 1 || rows[0].Actor != "" || !strings.Contains(rows[0].Data, "peer uid not permitted") { | ||
| 161 | t.Fatalf("refused hook rows: %+v", rows) | ||
| 162 | } | ||
| 163 | } | ||