diff --git a/internal/db/queries.go b/internal/db/queries.go index be27ff3..655fa04 100644 --- a/internal/db/queries.go +++ b/internal/db/queries.go @@ -1783,45 +1783,33 @@ func (database *Database) SetChannelHashcashBits( } // RecordSpentHashcash stores a spent hashcash stamp hash -// for replay prevention. +// for replay prevention. It returns false when the hash was +// already stored. The one INSERT is both the check and the +// record, so of several concurrent callers with the same +// hash exactly one gets true. func (database *Database) RecordSpentHashcash( ctx context.Context, stampHash string, -) error { - _, err := database.conn.ExecContext(ctx, +) (bool, error) { + res, err := database.conn.ExecContext(ctx, `INSERT OR IGNORE INTO spent_hashcash (stamp_hash, created_at) VALUES (?, ?)`, stampHash, time.Now()) if err != nil { - return fmt.Errorf( + return false, fmt.Errorf( "record spent hashcash: %w", err, ) } - return nil -} - -// IsHashcashSpent checks whether a hashcash stamp hash -// has already been used. -func (database *Database) IsHashcashSpent( - ctx context.Context, - stampHash string, -) (bool, error) { - var count int - - err := database.conn.QueryRowContext(ctx, - `SELECT COUNT(*) FROM spent_hashcash - WHERE stamp_hash = ?`, - stampHash, - ).Scan(&count) + inserted, err := res.RowsAffected() if err != nil { return false, fmt.Errorf( - "check spent hashcash: %w", err, + "record spent hashcash: %w", err, ) } - return count > 0, nil + return inserted > 0, nil } // PruneSpentHashcash deletes spent hashcash tokens older diff --git a/internal/handlers/api_test.go b/internal/handlers/api_test.go index 31861f3..fca94c9 100644 --- a/internal/handlers/api_test.go +++ b/internal/handlers/api_test.go @@ -3368,6 +3368,137 @@ func TestChannelHashcashRejectReplayedStamp(t *testing.T) { } } +// sendConcurrently posts cmd in the given number of requests +// at once and returns the status field of each response. +func (tserver *testServer) sendConcurrently( + token string, cmd map[string]any, requests int, +) []any { + tserver.t.Helper() + + body, err := json.Marshal(cmd) + if err != nil { + tserver.t.Fatalf("marshal command: %v", err) + } + + // Each request writes only its own entry. + statuses := make([]any, requests) + start := make(chan struct{}) + + var waitGroup sync.WaitGroup + + for idx := range requests { + waitGroup.Add(1) + + go func() { + defer waitGroup.Done() + + <-start + + resp, reqErr := doRequestAuth( + tserver.t, http.MethodPost, + tserver.url(apiMessages), token, + bytes.NewReader(body), + ) + if reqErr != nil { + tserver.t.Error(reqErr) + + return + } + + defer func() { _ = resp.Body.Close() }() + + var result map[string]any + + _ = json.NewDecoder(resp.Body).Decode(&result) + + statuses[idx] = result[statusKey] + }() + } + + close(start) + waitGroup.Wait() + + return statuses +} + +// TestChannelHashcashRejectConcurrentReplay sends one stamp +// in several requests at once and checks that exactly one is +// accepted and the other member receives exactly one copy. +func TestChannelHashcashRejectConcurrentReplay(t *testing.T) { + const ( + channel = "#hcrace" + text = "one stamp" + sender = "hcrace_sender" + requests = 16 + ) + + tserver := newTestServer(t) + senderToken := tserver.createSession(sender) + receiverToken := tserver.createSession("hcrace_receiver") + + tserver.sendCommand(senderToken, map[string]any{ + commandKey: joinCmd, toKey: channel, + }) + tserver.sendCommand(receiverToken, map[string]any{ + commandKey: joinCmd, toKey: channel, + }) + + tserver.sendCommand(senderToken, map[string]any{ + commandKey: modeCmd, + toKey: channel, + bodyKey: []string{"+H", "2"}, + }) + + _, lastID := tserver.pollMessages(receiverToken, 0) + + msgBody, marshalErr := json.Marshal([]string{text}) + if marshalErr != nil { + t.Fatal(marshalErr) + } + + stamp := mintTestChannelHashcash(t, 2, channel, msgBody) + + statuses := tserver.sendConcurrently(senderToken, map[string]any{ + commandKey: privmsgCmd, + toKey: channel, + bodyKey: []string{text}, + metaKey: map[string]any{hashcashKey: stamp}, + }, requests) + + accepted := 0 + + for _, status := range statuses { + if status == "sent" { + accepted++ + } + } + + if accepted != 1 { + t.Fatalf( + "expected 1 accepted message, got %d: %v", + accepted, statuses, + ) + } + + msgs, _ := tserver.pollMessages(receiverToken, lastID) + + delivered := 0 + + for _, msg := range msgs { + if msg[commandKey] == privmsgCmd && + msg["from"] == sender { + delivered++ + } + } + + if delivered != 1 { + t.Fatalf( + "expected 1 delivered copy, got %d: %v", + delivered, msgs, + ) + } +} + func TestChannelHashcashNoRequirementWorks(t *testing.T) { tserver := newTestServer(t) token := tserver.createSession("hcnone_user") diff --git a/internal/service/service.go b/internal/service/service.go index 4a43946..2977aa9 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -1285,8 +1285,9 @@ func (s *Service) requireOper( // checkChannelHashcash returns ERR_CANNOTSENDTOCHAN unless // the channel has no +H, or meta carries a hashcash stamp // for this channel and body with enough bits that has not -// been used before. A stamp that passes is recorded as -// spent. +// been used before. Recording the stamp as spent is the +// check that it is unused, so a stamp sent in several +// concurrent requests passes in only one of them. func (s *Service) checkChannelHashcash( ctx context.Context, chID int64, @@ -1324,12 +1325,12 @@ func (s *Service) checkChannelHashcash( stampKey := hashcash.StampHash(stamp) - spent, err := s.db.IsHashcashSpent(ctx, stampKey) + recorded, err := s.db.RecordSpentHashcash(ctx, stampKey) if err != nil { - return fmt.Errorf("check spent hashcash: %w", err) + return fmt.Errorf("record spent hashcash: %w", err) } - if spent { + if !recorded { return &IRCError{ irc.ErrCannotSendToChan, []string{channel}, @@ -1337,13 +1338,6 @@ func (s *Service) checkChannelHashcash( } } - err = s.db.RecordSpentHashcash(ctx, stampKey) - if err != nil { - s.log.Error( - "record spent hashcash", "error", err, - ) - } - return nil }