From 3ec5ffa3406bd79c01ba3f6186e0cdd7eb1b742d Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 1 Nov 2021 01:23:07 -0400 Subject: [PATCH 1/3] Revert "fix #1676" This reverts commit 5bbee02fe6bb8ed9dea23675df4349604c5fe247. --- default.yaml | 2 +- irc/channel.go | 18 ++++++------- irc/client.go | 8 +++--- irc/handlers.go | 14 ++-------- irc/history/history.go | 24 ++++------------- irc/history/history_test.go | 4 +-- irc/history/queries.go | 21 --------------- irc/histserv.go | 2 +- irc/mysql/history.go | 54 +------------------------------------ irc/server.go | 6 ++--- irc/utils/crypto.go | 23 ++++++++++++++++ irc/utils/crypto_test.go | 29 ++++++++++++++++++++ irc/znc.go | 2 +- 13 files changed, 81 insertions(+), 126 deletions(-) diff --git a/default.yaml b/default.yaml index 14256cd4..b582e656 100644 --- a/default.yaml +++ b/default.yaml @@ -962,7 +962,7 @@ history: # if `default` is false, store TAGMSG containing any of these tags: whitelist: - "+draft/react" - - "+react" + - "react" # if `default` is true, don't store TAGMSG containing any of these tags: #blacklist: diff --git a/irc/channel.go b/irc/channel.go index 6f02aea3..e45cd8c2 100644 --- a/irc/channel.go +++ b/irc/channel.go @@ -917,7 +917,7 @@ func (channel *Channel) autoReplayHistory(client *Client, rb *ResponseBuffer, sk } if hasAutoreplayTimestamps { - _, seq, _ := channel.server.GetHistorySequence(channel, client, "", 0) + _, seq, _ := channel.server.GetHistorySequence(channel, client, "") if seq != nil { zncMax := channel.server.Config().History.ZNCMax items, _ = seq.Between(history.Selector{Time: start}, history.Selector{Time: end}, zncMax) @@ -935,7 +935,7 @@ func (channel *Channel) autoReplayHistory(client *Client, rb *ResponseBuffer, sk replayLimit = channel.server.Config().History.AutoreplayOnJoin } if 0 < replayLimit { - _, seq, _ := channel.server.GetHistorySequence(channel, client, "", 0) + _, seq, _ := channel.server.GetHistorySequence(channel, client, "") if seq != nil { items, _ = seq.Between(history.Selector{}, history.Selector{}, replayLimit) } @@ -1084,7 +1084,7 @@ func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.I } else { message = fmt.Sprintf(client.t("%[1]s [account: %[2]s] joined the channel"), nick, item.AccountName) } - rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) + rb.AddFromClient(item.Message.Time, utils.MungeSecretToken(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) } case history.Part: if eventPlayback { @@ -1094,14 +1094,14 @@ func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.I continue // #474 } message := fmt.Sprintf(client.t("%[1]s left the channel (%[2]s)"), nick, item.Message.Message) - rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) + rb.AddFromClient(item.Message.Time, utils.MungeSecretToken(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) } case history.Kick: if eventPlayback { rb.AddFromClient(item.Message.Time, item.Message.Msgid, item.Nick, item.AccountName, item.IsBot, nil, "KICK", chname, item.Params[0], item.Message.Message) } else { message := fmt.Sprintf(client.t("%[1]s kicked %[2]s (%[3]s)"), nick, item.Params[0], item.Message.Message) - rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) + rb.AddFromClient(item.Message.Time, utils.MungeSecretToken(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) } case history.Quit: if eventPlayback { @@ -1111,21 +1111,21 @@ func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.I continue // #474 } message := fmt.Sprintf(client.t("%[1]s quit (%[2]s)"), nick, item.Message.Message) - rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) + rb.AddFromClient(item.Message.Time, utils.MungeSecretToken(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) } case history.Nick: if eventPlayback { rb.AddFromClient(item.Message.Time, item.Message.Msgid, item.Nick, item.AccountName, item.IsBot, nil, "NICK", item.Params[0]) } else { message := fmt.Sprintf(client.t("%[1]s changed nick to %[2]s"), nick, item.Params[0]) - rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) + rb.AddFromClient(item.Message.Time, utils.MungeSecretToken(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) } case history.Topic: if eventPlayback { rb.AddFromClient(item.Message.Time, item.Message.Msgid, item.Nick, item.AccountName, item.IsBot, nil, "TOPIC", chname, item.Message.Message) } else { message := fmt.Sprintf(client.t("%[1]s set the channel topic to: %[2]s"), nick, item.Message.Message) - rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) + rb.AddFromClient(item.Message.Time, utils.MungeSecretToken(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) } case history.Mode: params := make([]string, len(item.Message.Split)+1) @@ -1137,7 +1137,7 @@ func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.I rb.AddFromClient(item.Message.Time, item.Message.Msgid, item.Nick, item.AccountName, item.IsBot, nil, "MODE", params...) } else { message := fmt.Sprintf(client.t("%[1]s set channel modes: %[2]s"), nick, strings.Join(params[1:], " ")) - rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) + rb.AddFromClient(item.Message.Time, utils.MungeSecretToken(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) } } } diff --git a/irc/client.go b/irc/client.go index d94255bc..dda36a4e 100644 --- a/irc/client.go +++ b/irc/client.go @@ -883,7 +883,7 @@ func (client *Client) replayPrivmsgHistory(rb *ResponseBuffer, items []history.I if hasEventPlayback { rb.AddFromClient(item.Message.Time, item.Message.Msgid, item.Nick, item.AccountName, item.IsBot, nil, "INVITE", nick, item.Message.Message) } else { - rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", fmt.Sprintf(client.t("%[1]s invited you to channel %[2]s"), NUHToNick(item.Nick), item.Message.Message)) + rb.AddFromClient(item.Message.Time, utils.MungeSecretToken(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", fmt.Sprintf(client.t("%[1]s invited you to channel %[2]s"), NUHToNick(item.Nick), item.Message.Message)) } continue case history.Privmsg: @@ -1713,7 +1713,7 @@ func (client *Client) listTargets(start, end history.Selector, limit int) (resul var base, extras []history.TargetListing var chcfnames []string for _, channel := range client.Channels() { - _, seq, err := client.server.GetHistorySequence(channel, client, "", 0) + _, seq, err := client.server.GetHistorySequence(channel, client, "") if seq == nil || err != nil { continue } @@ -1734,7 +1734,7 @@ func (client *Client) listTargets(start, end history.Selector, limit int) (resul extras = append(extras, persistentExtras...) } - _, cSeq, err := client.server.GetHistorySequence(nil, client, "", 0) + _, cSeq, err := client.server.GetHistorySequence(nil, client, "") if err == nil && cSeq != nil { correspondents, err := cSeq.ListCorrespondents(start, end, limit) if err == nil { @@ -1758,7 +1758,7 @@ func (client *Client) privmsgsBetween(startTime, endTime time.Time, targetLimit, if strings.HasPrefix(target.CfName, "#") { continue } - _, seq, err := client.server.GetHistorySequence(nil, client, target.CfName, 0) + _, seq, err := client.server.GetHistorySequence(nil, client, target.CfName) if err == nil && seq != nil { items, err := seq.Between(start, end, messageLimit) if err == nil { diff --git a/irc/handlers.go b/irc/handlers.go index c0e42574..441d0e8d 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -640,7 +640,7 @@ func chathistoryHandler(server *Server, client *Client, msg ircmsg.Message, rb * } identifier, value := strings.ToLower(pieces[0]), pieces[1] if identifier == "msgid" { - msgid, err = history.NormalizeMsgid(value), nil + msgid, err = value, nil return } else if identifier == "timestamp" { timestamp, err = time.Parse(IRCv3TimestampFormat, value) @@ -725,17 +725,7 @@ func chathistoryHandler(server *Server, client *Client, msg ircmsg.Message, rb * if listTargets { targets, err = client.listTargets(start, end, limit) } else { - // see #1676; for CHATHISTORY we need to make the paging window as exact as possible, - // hence filtering out undisplayable messages on the backend, in order to send a full - // paging window if possible - var flags history.ExcludeFlags - if !rb.session.capabilities.Has(caps.EventPlayback) { - flags |= history.ExcludeTagmsg - } - if client.AccountSettings().ReplayJoins == ReplayJoinsNever { - flags |= history.ExcludeJoins - } - channel, sequence, err = server.GetHistorySequence(nil, client, target, flags) + channel, sequence, err = server.GetHistorySequence(nil, client, target) if err != nil || sequence == nil { return } diff --git a/irc/history/history.go b/irc/history/history.go index 34f46d2d..db3c0d22 100644 --- a/irc/history/history.go +++ b/irc/history/history.go @@ -53,17 +53,6 @@ func (item *Item) HasMsgid(msgid string) bool { return item.Message.Msgid == msgid } -func (item *Item) IsExcluded(excludeFlags ExcludeFlags) bool { - switch item.Type { - case Tagmsg: - return excludeFlags&ExcludeTagmsg != 0 - case Join, Part, Quit: - return excludeFlags&ExcludeJoins != 0 - default: - return false - } -} - type Predicate func(item *Item) (matches bool) func Reverse(results []Item) { @@ -166,7 +155,7 @@ func (list *Buffer) lookup(msgid string) (result Item, found bool) { // with an indication of whether the results are complete or are missing items // because some of that period was discarded. A zero value of `before` is considered // higher than all other times. -func (list *Buffer) betweenHelper(start, end Selector, cutoff time.Time, pred Predicate, limit int, excludeFlags ExcludeFlags) (results []Item, complete bool, err error) { +func (list *Buffer) betweenHelper(start, end Selector, cutoff time.Time, pred Predicate, limit int) (results []Item, complete bool, err error) { var ascending bool defer func() { @@ -206,8 +195,7 @@ func (list *Buffer) betweenHelper(start, end Selector, cutoff time.Time, pred Pr satisfies := func(item *Item) bool { return (after.IsZero() || item.Message.Time.After(after)) && (before.IsZero() || item.Message.Time.Before(before)) && - (pred == nil || pred(item)) && - !item.IsExcluded(excludeFlags) + (pred == nil || pred(item)) } return list.matchInternal(satisfies, ascending, limit), complete, nil @@ -291,10 +279,9 @@ type bufferSequence struct { list *Buffer pred Predicate cutoff time.Time - flags ExcludeFlags } -func (list *Buffer) MakeSequence(correspondent string, cutoff time.Time, flags ExcludeFlags) Sequence { +func (list *Buffer) MakeSequence(correspondent string, cutoff time.Time) Sequence { var pred Predicate if correspondent != "" { pred = func(item *Item) bool { @@ -305,12 +292,11 @@ func (list *Buffer) MakeSequence(correspondent string, cutoff time.Time, flags E list: list, pred: pred, cutoff: cutoff, - flags: flags, } } func (seq *bufferSequence) Between(start, end Selector, limit int) (results []Item, err error) { - results, _, err = seq.list.betweenHelper(start, end, seq.cutoff, seq.pred, limit, seq.flags) + results, _, err = seq.list.betweenHelper(start, end, seq.cutoff, seq.pred, limit) return } @@ -391,7 +377,7 @@ func (list *Buffer) Delete(predicate Predicate) (count int) { // latest returns the items most recently added, up to `limit`. If `limit` is 0, // it returns all items. func (list *Buffer) latest(limit int) (results []Item) { - results, _, _ = list.betweenHelper(Selector{}, Selector{}, time.Time{}, nil, limit, 0) + results, _, _ = list.betweenHelper(Selector{}, Selector{}, time.Time{}, nil, limit) return } diff --git a/irc/history/history_test.go b/irc/history/history_test.go index 92720083..18ee965c 100644 --- a/irc/history/history_test.go +++ b/irc/history/history_test.go @@ -15,7 +15,7 @@ const ( ) func betweenTimestamps(buf *Buffer, start, end time.Time, limit int) (result []Item, complete bool) { - result, complete, _ = buf.betweenHelper(Selector{Time: start}, Selector{Time: end}, time.Time{}, nil, limit, 0) + result, complete, _ = buf.betweenHelper(Selector{Time: start}, Selector{Time: end}, time.Time{}, nil, limit) return } @@ -45,7 +45,7 @@ func TestEmptyBuffer(t *testing.T) { }) since, complete = betweenTimestamps(buf, pastTime, time.Now(), 0) if len(since) != 1 { - t.Errorf("should be able to store items in a nonempty buffer: expected %d, got %d", 1, len(since)) + t.Error("should be able to store items in a nonempty buffer") } if !complete { t.Error("results should be complete") diff --git a/irc/history/queries.go b/irc/history/queries.go index 23d89390..2c7be322 100644 --- a/irc/history/queries.go +++ b/irc/history/queries.go @@ -4,17 +4,9 @@ package history import ( - "strings" "time" ) -type ExcludeFlags uint - -const ( - ExcludeTagmsg ExcludeFlags = 1 << iota - ExcludeJoins -) - // Selector represents a parameter to a CHATHISTORY command type Selector struct { Msgid string @@ -85,16 +77,3 @@ func MinMaxAsc(after, before, cutoff time.Time) (min, max time.Time, ascending b } return after, before, ascending } - -// maps regular msgids from JOIN, etc. to a msgid suitable for attaching -// to a HistServ message describing the JOIN. See #491 for some history. -func HistservMungeMsgid(msgid string) string { - return "_" + msgid -} - -// strips munging from a msgid. future schemes may not support a well-defined -// mapping of munged msgids to true msgids, but munged msgids should always contain -// a _, with metadata in front and data (possibly the true msgid) after. -func NormalizeMsgid(msgid string) string { - return strings.TrimPrefix(msgid, "_") -} diff --git a/irc/histserv.go b/irc/histserv.go index 5341fcb1..59149fa4 100644 --- a/irc/histserv.go +++ b/irc/histserv.go @@ -199,7 +199,7 @@ func histservPlayHandler(service *ircService, server *Server, client *Client, co // handles parameter parsing and history queries for /HISTORY and /HISTSERV PLAY func easySelectHistory(server *Server, client *Client, params []string) (items []history.Item, channel *Channel, err error) { - channel, sequence, err := server.GetHistorySequence(nil, client, params[0], 0) + channel, sequence, err := server.GetHistorySequence(nil, client, params[0]) if sequence == nil || err != nil { return nil, nil, errNoSuchChannel diff --git a/irc/mysql/history.go b/irc/mysql/history.go index 1ba81e26..8d78291e 100644 --- a/irc/mysql/history.go +++ b/irc/mysql/history.go @@ -40,10 +40,6 @@ const ( keySchemaMinorVersion = "db.minorversion" cleanupRowLimit = 50 cleanupPauseTime = 10 * time.Minute - - // if we don't fill the pagination window due to exclusions, - // retry with an expanded window at most this many times - maxPaginationRetries = 3 ) type e struct{} @@ -1037,18 +1033,9 @@ type mySQLHistorySequence struct { target string correspondent string cutoff time.Time - excludeFlags history.ExcludeFlags } func (s *mySQLHistorySequence) Between(start, end history.Selector, limit int) (results []history.Item, err error) { - if s.excludeFlags == 0 { - return s.baseBetween(start, end, limit) - } else { - return s.betweenWithRetries(start, end, limit) - } -} - -func (s *mySQLHistorySequence) baseBetween(start, end history.Selector, limit int) (results []history.Item, err error) { ctx, cancel := context.WithTimeout(context.Background(), s.mysql.getTimeout()) defer cancel() @@ -1071,45 +1058,7 @@ func (s *mySQLHistorySequence) baseBetween(start, end history.Selector, limit in return results, err } -func (s *mySQLHistorySequence) betweenWithRetries(start, end history.Selector, limit int) (results []history.Item, err error) { - applyExclusions := func(currentResults []history.Item, excludeFlags history.ExcludeFlags, trueLimit int) (filteredResults []history.Item) { - filteredResults = make([]history.Item, 0, len(currentResults)) - for _, item := range currentResults { - if !item.IsExcluded(excludeFlags) { - filteredResults = append(filteredResults, item) - } - if len(filteredResults) == trueLimit { - break - } - } - return - } - - i := 1 - for { - currentLimit := limit * i - currentResults, err := s.baseBetween(start, end, currentLimit) - if err != nil { - return nil, err - } - results = applyExclusions(currentResults, s.excludeFlags, limit) - // we're done in any of these three cases: - // (1) we filled the window (2) we ran out of results on the backend (3) we can't retry anymore - if len(results) == limit || len(currentResults) < currentLimit || i == maxPaginationRetries { - return results, nil - } - i++ - } -} - func (s *mySQLHistorySequence) Around(start history.Selector, limit int) (results []history.Item, err error) { - // temporarily clear the exclude flags when running GenericAround, since we don't care about - // the exactness of the paging window at all - oldExcludeFlags := s.excludeFlags - s.excludeFlags = 0 - defer func() { - s.excludeFlags = oldExcludeFlags - }() return history.GenericAround(s, start, limit) } @@ -1134,12 +1083,11 @@ func (seq *mySQLHistorySequence) Ephemeral() bool { return false } -func (mysql *MySQL) MakeSequence(target, correspondent string, cutoff time.Time, excludeFlags history.ExcludeFlags) history.Sequence { +func (mysql *MySQL) MakeSequence(target, correspondent string, cutoff time.Time) history.Sequence { return &mySQLHistorySequence{ target: target, correspondent: correspondent, mysql: mysql, cutoff: cutoff, - excludeFlags: excludeFlags, } } diff --git a/irc/server.go b/irc/server.go index 7a43f0f4..3ac8c38f 100644 --- a/irc/server.go +++ b/irc/server.go @@ -862,7 +862,7 @@ func (server *Server) setupListeners(config *Config) (err error) { // suitable for ListCorrespondents (i.e., this function is still used to // decide whether the ringbuf or mysql is authoritative about the client's // message history). -func (server *Server) GetHistorySequence(providedChannel *Channel, client *Client, query string, excludeFlags history.ExcludeFlags) (channel *Channel, sequence history.Sequence, err error) { +func (server *Server) GetHistorySequence(providedChannel *Channel, client *Client, query string) (channel *Channel, sequence history.Sequence, err error) { config := server.Config() // 4 cases: {persistent, ephemeral} x {normal, conversation} // with ephemeral history, target is implicit in the choice of `hist`, @@ -940,9 +940,9 @@ func (server *Server) GetHistorySequence(providedChannel *Channel, client *Clien } if hist != nil { - sequence = hist.MakeSequence(correspondent, cutoff, excludeFlags) + sequence = hist.MakeSequence(correspondent, cutoff) } else if target != "" { - sequence = server.historyDB.MakeSequence(target, correspondent, cutoff, excludeFlags) + sequence = server.historyDB.MakeSequence(target, correspondent, cutoff) } return } diff --git a/irc/utils/crypto.go b/irc/utils/crypto.go index a479b96d..fe4a389c 100644 --- a/irc/utils/crypto.go +++ b/irc/utils/crypto.go @@ -42,6 +42,29 @@ func GenerateSecretToken() string { return B32Encoder.EncodeToString(buf[:]) } +// "munge" a secret token to a new value. requirements: +// 1. MUST be roughly as unlikely to collide with `GenerateSecretToken` outputs +// as those outputs are with each other +// 2. SHOULD be deterministic (motivation: if a JOIN line has msgid x, +// create a deterministic msgid y for the fake HistServ PRIVMSG that "replays" it) +// 3. SHOULD be in the same "namespace" as `GenerateSecretToken` outputs +// (same length and character set) +func MungeSecretToken(token string) (result string) { + bytes, err := B32Encoder.DecodeString(token) + if err != nil { + // this should never happen + return GenerateSecretToken() + } + // add 1 with carrying + for i := len(bytes) - 1; 0 <= i; i -= 1 { + bytes[i] += 1 + if bytes[i] != 0 { + break + } // else: overflow, carry to the next place + } + return B32Encoder.EncodeToString(bytes) +} + // securely check if a supplied token matches a stored token func SecretTokensMatch(storedToken string, suppliedToken string) bool { // XXX fix a potential gotcha: if the stored token is uninitialized, diff --git a/irc/utils/crypto_test.go b/irc/utils/crypto_test.go index bedf8ada..8140e683 100644 --- a/irc/utils/crypto_test.go +++ b/irc/utils/crypto_test.go @@ -47,12 +47,41 @@ func TestTokenCompare(t *testing.T) { } } +func TestMunging(t *testing.T) { + count := 131072 + set := make(map[string]bool) + var token string + for i := 0; i < count; i++ { + token = GenerateSecretToken() + set[token] = true + } + // all tokens generated thus far should be unique + assertEqual(len(set), count, t) + + // iteratively munge the last generated token an additional `count` times + mungedToken := token + for i := 0; i < count; i++ { + mungedToken = MungeSecretToken(mungedToken) + assertEqual(len(mungedToken), len(token), t) + set[mungedToken] = true + } + // munged tokens should not collide with generated tokens, or each other + assertEqual(len(set), count*2, t) +} + func BenchmarkGenerateSecretToken(b *testing.B) { for i := 0; i < b.N; i++ { GenerateSecretToken() } } +func BenchmarkMungeSecretToken(b *testing.B) { + t := GenerateSecretToken() + for i := 0; i < b.N; i++ { + t = MungeSecretToken(t) + } +} + func TestCertfpComparisons(t *testing.T) { opensslFP := "3D:6B:11:BF:B4:05:C3:F8:4B:38:CD:30:38:FB:EC:01:71:D5:03:54:79:04:07:88:4C:A5:5D:23:41:85:66:C9" oragonoFP := "3d6b11bfb405c3f84b38cd3038fbec0171d50354790407884ca55d23418566c9" diff --git a/irc/znc.go b/irc/znc.go index b568d3e7..5b64d285 100644 --- a/irc/znc.go +++ b/irc/znc.go @@ -189,7 +189,7 @@ func zncPlaybackPlayHandler(client *Client, command string, params []string, rb } func zncPlayPrivmsgsFrom(client *Client, rb *ResponseBuffer, target string, start, end time.Time) { - _, sequence, err := client.server.GetHistorySequence(nil, client, target, 0) + _, sequence, err := client.server.GetHistorySequence(nil, client, target) if sequence == nil || err != nil { return } From 4749d7e77675c900167765172576c78d5d487674 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 1 Nov 2021 01:24:14 -0400 Subject: [PATCH 2/3] fix #1676, take 2 Ensure the pagination window is full by making sure that every history item gets a replay line in CHATHISTORY output, even TAGMSG. --- default.yaml | 2 +- irc/channel.go | 35 ++++++++++++++++++++--------------- irc/client.go | 9 +++++++-- irc/handlers.go | 10 +++++----- irc/history/queries.go | 14 ++++++++++++++ irc/utils/crypto.go | 23 ----------------------- irc/utils/crypto_test.go | 29 ----------------------------- irc/znc.go | 4 ++-- irctest | 2 +- traditional.yaml | 2 +- 10 files changed, 51 insertions(+), 79 deletions(-) diff --git a/default.yaml b/default.yaml index b582e656..14256cd4 100644 --- a/default.yaml +++ b/default.yaml @@ -962,7 +962,7 @@ history: # if `default` is false, store TAGMSG containing any of these tags: whitelist: - "+draft/react" - - "react" + - "+react" # if `default` is true, don't store TAGMSG containing any of these tags: #blacklist: diff --git a/irc/channel.go b/irc/channel.go index e45cd8c2..91a82a5a 100644 --- a/irc/channel.go +++ b/irc/channel.go @@ -952,7 +952,7 @@ func (channel *Channel) autoReplayHistory(client *Client, rb *ResponseBuffer, sk } } if 0 < numItems { - channel.replayHistoryItems(rb, items, true) + channel.replayHistoryItems(rb, items, false) rb.Flush(true) } } @@ -1035,7 +1035,7 @@ func (channel *Channel) Part(client *Client, message string, rb *ResponseBuffer) client.server.logger.Debug("channels", fmt.Sprintf("%s left channel %s", details.nick, chname)) } -func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.Item, autoreplay bool) { +func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.Item, chathistoryCommand bool) { // send an empty batch if necessary, as per the CHATHISTORY spec chname := channel.Name() client := rb.target @@ -1043,13 +1043,15 @@ func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.I extendedJoin := rb.session.capabilities.Has(caps.ExtendedJoin) var playJoinsAsPrivmsg bool if !eventPlayback { - switch client.AccountSettings().ReplayJoins { - case ReplayJoinsCommandsOnly: - playJoinsAsPrivmsg = !autoreplay - case ReplayJoinsAlways: + if chathistoryCommand { playJoinsAsPrivmsg = true - case ReplayJoinsNever: - playJoinsAsPrivmsg = false + } else { + switch client.AccountSettings().ReplayJoins { + case ReplayJoinsCommandsOnly: + playJoinsAsPrivmsg = false + case ReplayJoinsAlways: + playJoinsAsPrivmsg = true + } } } @@ -1066,6 +1068,9 @@ func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.I case history.Tagmsg: if eventPlayback { rb.AddSplitMessageFromClient(item.Nick, item.AccountName, item.IsBot, item.Tags, "TAGMSG", chname, item.Message) + } else if chathistoryCommand { + // #1676, we have to send something here or else it breaks pagination + rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, fmt.Sprintf(client.t("%s sent a TAGMSG"), nick)) } case history.Join: if eventPlayback { @@ -1084,7 +1089,7 @@ func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.I } else { message = fmt.Sprintf(client.t("%[1]s [account: %[2]s] joined the channel"), nick, item.AccountName) } - rb.AddFromClient(item.Message.Time, utils.MungeSecretToken(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) + rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) } case history.Part: if eventPlayback { @@ -1094,14 +1099,14 @@ func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.I continue // #474 } message := fmt.Sprintf(client.t("%[1]s left the channel (%[2]s)"), nick, item.Message.Message) - rb.AddFromClient(item.Message.Time, utils.MungeSecretToken(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) + rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) } case history.Kick: if eventPlayback { rb.AddFromClient(item.Message.Time, item.Message.Msgid, item.Nick, item.AccountName, item.IsBot, nil, "KICK", chname, item.Params[0], item.Message.Message) } else { message := fmt.Sprintf(client.t("%[1]s kicked %[2]s (%[3]s)"), nick, item.Params[0], item.Message.Message) - rb.AddFromClient(item.Message.Time, utils.MungeSecretToken(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) + rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) } case history.Quit: if eventPlayback { @@ -1111,21 +1116,21 @@ func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.I continue // #474 } message := fmt.Sprintf(client.t("%[1]s quit (%[2]s)"), nick, item.Message.Message) - rb.AddFromClient(item.Message.Time, utils.MungeSecretToken(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) + rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) } case history.Nick: if eventPlayback { rb.AddFromClient(item.Message.Time, item.Message.Msgid, item.Nick, item.AccountName, item.IsBot, nil, "NICK", item.Params[0]) } else { message := fmt.Sprintf(client.t("%[1]s changed nick to %[2]s"), nick, item.Params[0]) - rb.AddFromClient(item.Message.Time, utils.MungeSecretToken(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) + rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) } case history.Topic: if eventPlayback { rb.AddFromClient(item.Message.Time, item.Message.Msgid, item.Nick, item.AccountName, item.IsBot, nil, "TOPIC", chname, item.Message.Message) } else { message := fmt.Sprintf(client.t("%[1]s set the channel topic to: %[2]s"), nick, item.Message.Message) - rb.AddFromClient(item.Message.Time, utils.MungeSecretToken(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) + rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) } case history.Mode: params := make([]string, len(item.Message.Split)+1) @@ -1137,7 +1142,7 @@ func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.I rb.AddFromClient(item.Message.Time, item.Message.Msgid, item.Nick, item.AccountName, item.IsBot, nil, "MODE", params...) } else { message := fmt.Sprintf(client.t("%[1]s set channel modes: %[2]s"), nick, strings.Join(params[1:], " ")) - rb.AddFromClient(item.Message.Time, utils.MungeSecretToken(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) + rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", chname, message) } } } diff --git a/irc/client.go b/irc/client.go index dda36a4e..6912483c 100644 --- a/irc/client.go +++ b/irc/client.go @@ -853,7 +853,7 @@ func (session *Session) Ping() { session.Send(nil, "", "PING", session.client.Nick()) } -func (client *Client) replayPrivmsgHistory(rb *ResponseBuffer, items []history.Item, target string) { +func (client *Client) replayPrivmsgHistory(rb *ResponseBuffer, items []history.Item, target string, chathistoryCommand bool) { var batchID string details := client.Details() nick := details.nick @@ -883,7 +883,7 @@ func (client *Client) replayPrivmsgHistory(rb *ResponseBuffer, items []history.I if hasEventPlayback { rb.AddFromClient(item.Message.Time, item.Message.Msgid, item.Nick, item.AccountName, item.IsBot, nil, "INVITE", nick, item.Message.Message) } else { - rb.AddFromClient(item.Message.Time, utils.MungeSecretToken(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", fmt.Sprintf(client.t("%[1]s invited you to channel %[2]s"), NUHToNick(item.Nick), item.Message.Message)) + rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", fmt.Sprintf(client.t("%[1]s invited you to channel %[2]s"), NUHToNick(item.Nick), item.Message.Message)) } continue case history.Privmsg: @@ -893,10 +893,15 @@ func (client *Client) replayPrivmsgHistory(rb *ResponseBuffer, items []history.I case history.Tagmsg: if hasEventPlayback && hasTags { command = "TAGMSG" + } else if chathistoryCommand { + // #1676: send something for TAGMSG; we can't discard it entirely + // because it'll break pagination + rb.AddFromClient(item.Message.Time, history.HistservMungeMsgid(item.Message.Msgid), histservService.prefix, "*", false, nil, "PRIVMSG", fmt.Sprintf(client.t("%[1]s sent you a TAGMSG"), NUHToNick(item.Nick))) } else { continue } default: + // see #1676, this shouldn't happen continue } var tags map[string]string diff --git a/irc/handlers.go b/irc/handlers.go index 441d0e8d..1f5982a2 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -611,9 +611,9 @@ func chathistoryHandler(server *Server, client *Client, msg ircmsg.Message, rb * target.Time.Format(IRCv3TimestampFormat)) } } else if channel != nil { - channel.replayHistoryItems(rb, items, false) + channel.replayHistoryItems(rb, items, true) } else { - client.replayPrivmsgHistory(rb, items, target) + client.replayPrivmsgHistory(rb, items, target, true) } } }() @@ -640,7 +640,7 @@ func chathistoryHandler(server *Server, client *Client, msg ircmsg.Message, rb * } identifier, value := strings.ToLower(pieces[0]), pieces[1] if identifier == "msgid" { - msgid, err = value, nil + msgid, err = history.NormalizeMsgid(value), nil return } else if identifier == "timestamp" { timestamp, err = time.Parse(IRCv3TimestampFormat, value) @@ -1122,9 +1122,9 @@ func historyHandler(server *Server, client *Client, msg ircmsg.Message, rb *Resp if len(items) != 0 { if channel != nil { - channel.replayHistoryItems(rb, items, false) + channel.replayHistoryItems(rb, items, true) } else { - client.replayPrivmsgHistory(rb, items, "") + client.replayPrivmsgHistory(rb, items, "", true) } } return false diff --git a/irc/history/queries.go b/irc/history/queries.go index 2c7be322..72b1168f 100644 --- a/irc/history/queries.go +++ b/irc/history/queries.go @@ -4,6 +4,7 @@ package history import ( + "strings" "time" ) @@ -77,3 +78,16 @@ func MinMaxAsc(after, before, cutoff time.Time) (min, max time.Time, ascending b } return after, before, ascending } + +// maps regular msgids from JOIN, etc. to a msgid suitable for attaching +// to a HistServ message describing the JOIN. See #491 for some history. +func HistservMungeMsgid(msgid string) string { + return "_" + msgid +} + +// strips munging from a msgid. future schemes may not support a well-defined +// mapping of munged msgids to true msgids, but munged msgids should always contain +// a _, with metadata in front and data (possibly the true msgid) after. +func NormalizeMsgid(msgid string) string { + return strings.TrimPrefix(msgid, "_") +} diff --git a/irc/utils/crypto.go b/irc/utils/crypto.go index fe4a389c..a479b96d 100644 --- a/irc/utils/crypto.go +++ b/irc/utils/crypto.go @@ -42,29 +42,6 @@ func GenerateSecretToken() string { return B32Encoder.EncodeToString(buf[:]) } -// "munge" a secret token to a new value. requirements: -// 1. MUST be roughly as unlikely to collide with `GenerateSecretToken` outputs -// as those outputs are with each other -// 2. SHOULD be deterministic (motivation: if a JOIN line has msgid x, -// create a deterministic msgid y for the fake HistServ PRIVMSG that "replays" it) -// 3. SHOULD be in the same "namespace" as `GenerateSecretToken` outputs -// (same length and character set) -func MungeSecretToken(token string) (result string) { - bytes, err := B32Encoder.DecodeString(token) - if err != nil { - // this should never happen - return GenerateSecretToken() - } - // add 1 with carrying - for i := len(bytes) - 1; 0 <= i; i -= 1 { - bytes[i] += 1 - if bytes[i] != 0 { - break - } // else: overflow, carry to the next place - } - return B32Encoder.EncodeToString(bytes) -} - // securely check if a supplied token matches a stored token func SecretTokensMatch(storedToken string, suppliedToken string) bool { // XXX fix a potential gotcha: if the stored token is uninitialized, diff --git a/irc/utils/crypto_test.go b/irc/utils/crypto_test.go index 8140e683..bedf8ada 100644 --- a/irc/utils/crypto_test.go +++ b/irc/utils/crypto_test.go @@ -47,41 +47,12 @@ func TestTokenCompare(t *testing.T) { } } -func TestMunging(t *testing.T) { - count := 131072 - set := make(map[string]bool) - var token string - for i := 0; i < count; i++ { - token = GenerateSecretToken() - set[token] = true - } - // all tokens generated thus far should be unique - assertEqual(len(set), count, t) - - // iteratively munge the last generated token an additional `count` times - mungedToken := token - for i := 0; i < count; i++ { - mungedToken = MungeSecretToken(mungedToken) - assertEqual(len(mungedToken), len(token), t) - set[mungedToken] = true - } - // munged tokens should not collide with generated tokens, or each other - assertEqual(len(set), count*2, t) -} - func BenchmarkGenerateSecretToken(b *testing.B) { for i := 0; i < b.N; i++ { GenerateSecretToken() } } -func BenchmarkMungeSecretToken(b *testing.B) { - t := GenerateSecretToken() - for i := 0; i < b.N; i++ { - t = MungeSecretToken(t) - } -} - func TestCertfpComparisons(t *testing.T) { opensslFP := "3D:6B:11:BF:B4:05:C3:F8:4B:38:CD:30:38:FB:EC:01:71:D5:03:54:79:04:07:88:4C:A5:5D:23:41:85:66:C9" oragonoFP := "3d6b11bfb405c3f84b38cd3038fbec0171d50354790407884ca55d23418566c9" diff --git a/irc/znc.go b/irc/znc.go index 5b64d285..11fe012c 100644 --- a/irc/znc.go +++ b/irc/znc.go @@ -196,7 +196,7 @@ func zncPlayPrivmsgsFrom(client *Client, rb *ResponseBuffer, target string, star zncMax := client.server.Config().History.ZNCMax items, err := sequence.Between(history.Selector{Time: start}, history.Selector{Time: end}, zncMax) if err == nil && len(items) != 0 { - client.replayPrivmsgHistory(rb, items, target) + client.replayPrivmsgHistory(rb, items, target, false) } } @@ -204,7 +204,7 @@ func zncPlayPrivmsgsFromAll(client *Client, rb *ResponseBuffer, start, end time. zncMax := client.server.Config().History.ZNCMax items, err := client.privmsgsBetween(start, end, maxDMTargetsForAutoplay, zncMax) if err == nil && len(items) != 0 { - client.replayPrivmsgHistory(rb, items, "") + client.replayPrivmsgHistory(rb, items, "", false) } } diff --git a/irctest b/irctest index 33f0702c..5e4ae7c9 160000 --- a/irctest +++ b/irctest @@ -1 +1 @@ -Subproject commit 33f0702c260ea716a4ad0f24821a50d44c91fca1 +Subproject commit 5e4ae7c99965801cd91d974637ad344a47b5414f diff --git a/traditional.yaml b/traditional.yaml index 948bd182..51b0a2f1 100644 --- a/traditional.yaml +++ b/traditional.yaml @@ -935,7 +935,7 @@ history: # if `default` is false, store TAGMSG containing any of these tags: whitelist: - "+draft/react" - - "react" + - "+react" # if `default` is true, don't store TAGMSG containing any of these tags: #blacklist: From 8c556fe8c541164fc0d3c2346286e4f7b87839f9 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 1 Nov 2021 04:36:06 -0400 Subject: [PATCH 3/3] schema change to remove ReplayJoinsNever See #1676 --- irc/accounts.go | 3 --- irc/database.go | 57 ++++++++++++++++++++++++++++++++++++++++++++++++- irc/nickserv.go | 6 ++---- 3 files changed, 58 insertions(+), 8 deletions(-) diff --git a/irc/accounts.go b/irc/accounts.go index 8f8e8b45..97237523 100644 --- a/irc/accounts.go +++ b/irc/accounts.go @@ -2294,7 +2294,6 @@ type ReplayJoinsSetting uint const ( ReplayJoinsCommandsOnly = iota // replay in HISTORY or CHATHISTORY output ReplayJoinsAlways // replay in HISTORY, CHATHISTORY, or autoreplay - ReplayJoinsNever // never replay ) func replayJoinsSettingFromString(str string) (result ReplayJoinsSetting, err error) { @@ -2303,8 +2302,6 @@ func replayJoinsSettingFromString(str string) (result ReplayJoinsSetting, err er result = ReplayJoinsCommandsOnly case "always": result = ReplayJoinsAlways - case "never": - result = ReplayJoinsNever default: err = errInvalidParams } diff --git a/irc/database.go b/irc/database.go index b4b417f4..bfca9d07 100644 --- a/irc/database.go +++ b/irc/database.go @@ -24,7 +24,7 @@ const ( // 'version' of the database schema keySchemaVersion = "db.version" // latest schema of the db - latestDbSchema = 21 + latestDbSchema = 22 keyCloakSecret = "crypto.cloak_secret" ) @@ -1059,6 +1059,56 @@ func schemaChangeV20To21(config *Config, tx *buntdb.Tx) error { return nil } +// #1676: we used to have ReplayJoinsNever, now it's desupported +func schemaChangeV21To22(config *Config, tx *buntdb.Tx) error { + type accountSettingsv22 struct { + AutoreplayLines *int + NickEnforcement NickEnforcementMethod + AllowBouncer MulticlientAllowedSetting + ReplayJoins ReplayJoinsSetting + AlwaysOn PersistentStatus + AutoreplayMissed bool + DMHistory HistoryStatus + AutoAway PersistentStatus + Email string + } + + var accounts []string + var serializedSettings []string + settingsPrefix := "account.settings " + tx.AscendGreaterOrEqual("", settingsPrefix, func(key, value string) bool { + if !strings.HasPrefix(key, settingsPrefix) { + return false + } + account := strings.TrimPrefix(key, settingsPrefix) + if _, err := tx.Get("account.verified " + account); err != nil { + return true + } + var settings accountSettingsv22 + err := json.Unmarshal([]byte(value), &settings) + if err != nil { + log.Printf("error (v21-22) processing settings for %s: %v\n", account, err) + return true + } + // if necessary, change ReplayJoinsNever (2) to ReplayJoinsCommandsOnly (0) + if settings.ReplayJoins == ReplayJoinsSetting(2) { + settings.ReplayJoins = ReplayJoinsSetting(0) + if b, err := json.Marshal(settings); err == nil { + accounts = append(accounts, account) + serializedSettings = append(serializedSettings, string(b)) + } else { + log.Printf("error (v21-22) processing settings for %s: %v\n", account, err) + } + } + return true + }) + + for i, account := range accounts { + tx.Set(settingsPrefix+account, serializedSettings[i], nil) + } + return nil +} + func getSchemaChange(initialVersion int) (result SchemaChange, ok bool) { for _, change := range allChanges { if initialVersion == change.InitialVersion { @@ -1169,4 +1219,9 @@ var allChanges = []SchemaChange{ TargetVersion: 21, Changer: schemaChangeV20To21, }, + { + InitialVersion: 21, + TargetVersion: 22, + Changer: schemaChangeV21To22, + }, } diff --git a/irc/nickserv.go b/irc/nickserv.go index 3658c8fb..8166fee8 100644 --- a/irc/nickserv.go +++ b/irc/nickserv.go @@ -284,8 +284,8 @@ default.`, `$bREPLAY-JOINS$b 'replay-joins' controls whether replayed channel history will include lines for join and part. This provides more information about the context of -messages, but may be spammy. Your options are 'always', 'never', and the default -of 'commands-only' (the messages will be replayed in /HISTORY output, but not +messages, but may be spammy. Your options are 'always' and the default of +'commands-only' (the messages will be replayed in CHATHISTORY output, but not during autoreplay).`, `$bALWAYS-ON$b 'always-on' controls whether your nickname/identity will remain active @@ -440,8 +440,6 @@ func displaySetting(service *ircService, settingName string, settings AccountSet service.Notice(rb, client.t("You will see JOINs and PARTs in /HISTORY output, but not in autoreplay")) case ReplayJoinsAlways: service.Notice(rb, client.t("You will see JOINs and PARTs in /HISTORY output and in autoreplay")) - case ReplayJoinsNever: - service.Notice(rb, client.t("You will not see JOINs and PARTs in /HISTORY output or in autoreplay")) } case "multiclient": if !config.Accounts.Multiclient.Enabled {