From c8dfe128ee8bec51a85bf3c29e5a409b8a79c66e Mon Sep 17 00:00:00 2001 From: mleku Date: Wed, 27 Nov 2024 12:59:49 +0000 Subject: [PATCH] fixes for delete handling closes #3 --- cmd/realy/app/implementation.go | 120 +++++++++++++++++++++++-------- ratel/main.go | 2 +- realy/add-event.go | 5 +- realy/handlers.go | 80 ++++++++++++++++++--- realy/version | 2 +- relay/interface.go | 6 +- relay/wrapper/relay_interface.go | 32 ++++++--- 7 files changed, 194 insertions(+), 53 deletions(-) diff --git a/cmd/realy/app/implementation.go b/cmd/realy/app/implementation.go index 2565cd2..4792c09 100644 --- a/cmd/realy/app/implementation.go +++ b/cmd/realy/app/implementation.go @@ -1,6 +1,7 @@ package app import ( + "bytes" "fmt" "net/http" "strconv" @@ -13,6 +14,7 @@ import ( "realy.lol/filter" "realy.lol/filters" "realy.lol/hex" + "realy.lol/ints" "realy.lol/kind" "realy.lol/kinds" "realy.lol/store" @@ -20,11 +22,18 @@ import ( ) type Relay struct { + sync.Mutex *Config - Store store.I + Store store.I + // Owners' pubkeys Owners []B Followed, Muted map[S]struct{} - sync.Mutex + // OwnersFollowLists are the event IDs of owners follow lists, which must not be deleted, only + // replaced. + OwnersFollowLists []B + // OwnersMuteLists are the event IDs of owners mute lists, which must not be deleted, only + // replaced. + OwnersMuteLists []B } func (r *Relay) Name() S { return r.Config.AppName } @@ -55,14 +64,13 @@ func (r *Relay) Init() (err E) { } func (r *Relay) AcceptEvent(c context.T, evt *event.T, hr *http.Request, origin S, - authedPubkey B) bool { + authedPubkey B) (accept bool, notice S) { // if the authenticator is enabled we require auth to accept events if !r.AuthEnabled() { - return true + return true, "" } if len(authedPubkey) != 32 { - log.E.F("client not authed with auth required %s", origin) - return false + return false, fmt.Sprintf("client not authed with auth required %s", origin) } if len(r.Owners) > 0 { r.Lock() @@ -74,48 +82,94 @@ func (r *Relay) AcceptEvent(c context.T, evt *event.T, hr *http.Request, origin // owner has updated follows or mute list, so we zero those lists so they // are regenerated for the next AcceptReq/AcceptEvent r.Followed = make(map[S]struct{}) + r.OwnersFollowLists = r.OwnersFollowLists[:0] r.Muted = make(map[S]struct{}) + r.OwnersMuteLists = r.OwnersMuteLists[:0] log.I.F("clearing owner follow/mute lists because of update from %s %0x", origin, evt.PubKey) - return true + return true, "" } } } for _, o := range r.Owners { log.T.F("%0x,%0x", o, evt.PubKey) if equals(o, evt.PubKey) { - log.W.Ln("event is from owner") - return true + // prevent owners from deleting their own mute/follow lists in case of bad + // client implementation + if evt.Kind.Equal(kind.Deletion) { + // we don't accept deletes on owners' follow or mute lists because of the + // potential for a malicious action causing this, first check for the list: + tt := tag.New(append(r.OwnersFollowLists, r.OwnersMuteLists...)...) + if evt.Tags.ContainsAny(B("e"), tt) { + return false, "cannot delete owner's follow or mute events" + } + // next, check all a tags present are not follow/mute lists of the owners + aTags := evt.Tags.GetAll(tag.New("a")) + for _, at := range aTags.F() { + split := bytes.Split(at.Value(), B{':'}) + if len(split) != 3 { + continue + } + kin := ints.New(uint16(0)) + if _, err := kin.UnmarshalJSON(split[0]); chk.E(err) { + return + } + kk := kind.New(kin.Uint16()) + if kk.Equal(kind.Deletion) { + // we don't delete delete events, period + return false, "delete event kind may not be deleted" + } + // if the kind is not parameterised replaceable, the tag is invalid and the + // delete event will not be saved. + if !kk.IsParameterizedReplaceable() { + return false, "delete tags with a tags containing " + + "non-parameterized-replaceable events cannot be processed" + } + for _, own := range r.Owners { + // don't allow owners to delete their mute or follow lists because + // they should not want to, can simply replace it, and malicious + // clients may do this specifically to attack the owner's relay (s) + if equals(own, split[1]) || + kk.Equal(kind.MuteList) || + kk.Equal(kind.FollowList) { + return false, "owners may not delete their own " + + "mute or follow lists, they can be replaced" + } + } + } + log.W.Ln("event is from owner") + return true, "" + } } - } - // check the mute list, and reject events authored by muted pubkeys, even if - // they come from a pubkey that is on the follow list. - for pk := range r.Muted { - if equals(evt.PubKey, B(pk)) { - log.I.F("rejecting event with pubkey %0x because on owner mute list", - evt.PubKey) - return false + // check the mute list, and reject events authored by muted pubkeys, even if + // they come from a pubkey that is on the follow list. + for pk := range r.Muted { + if equals(evt.PubKey, B(pk)) { + return false, "rejecting event with pubkey " + S(evt.PubKey) + + " because on owner mute list" + } } - } - // for all else, check the authed pubkey is in the follow list - for pk := range r.Followed { - // allow all events from follows of owners - if equals(authedPubkey, B(pk)) { - log.I.F("accepting event %0x because %0x on owner follow list", - evt.ID, B(pk)) - return true + // for all else, check the authed pubkey is in the follow list + for pk := range r.Followed { + // allow all events from follows of owners + if equals(authedPubkey, B(pk)) { + log.I.F("accepting event %0x because %0x on owner follow list", + evt.ID, B(pk)) + return true, "" + } } - // todo: allow accepting events with p tag of a follow that the follow has not muted - // todo: this will allow outsiders to send messages to users - // todo: users will mute the user if they don't want to receive from this sender } } // if auth is enabled and there is no moderators we just check that the pubkey // has been loaded via the auth function. - return len(authedPubkey) == schnorr.PubKeyBytesLen + accept = len(authedPubkey) == schnorr.PubKeyBytesLen + if !accept { + notice = "auth required but user not authed" + } + return } -func (r *Relay) AcceptReq(c Ctx, hr *http.Request, id B, ff *filters.T, +func (r *Relay) AcceptReq(c Ctx, hr *http.Request, idB, ff *filters.T, authedPubkey B) (allowed *filters.T, ok bool) { // if the authenticator is enabled we require auth to process requests if !r.AuthEnabled() { @@ -196,6 +250,9 @@ func (r *Relay) CheckOwnerLists(c context.T) { Kinds: kinds.New(kind.FollowList)}); chk.E(err) { } + for i := range evs { + r.OwnersFollowLists = append(r.OwnersFollowLists, evs[i].ID) + } // preallocate sufficient elements var count int for _, ev := range evs { @@ -228,6 +285,9 @@ func (r *Relay) CheckOwnerLists(c context.T) { Kinds: kinds.New(kind.MuteList)}); chk.E(err) { } + for i := range evs { + r.OwnersMuteLists = append(r.OwnersMuteLists, evs[i].ID) + } r.Muted = make(map[S]struct{}) mutes := "mutes(access blacklist),[" var first bool diff --git a/ratel/main.go b/ratel/main.go index e45e581..349725f 100644 --- a/ratel/main.go +++ b/ratel/main.go @@ -33,7 +33,7 @@ type T struct { BlockCacheSize int InitLogLevel int Logger *logger - // DB is the badger db enveloper + // DB is the badger db *badger.DB // seq is the monotonic collision free index for raw event storage. seq *badger.Sequence diff --git a/realy/add-event.go b/realy/add-event.go index 43005d2..b96a595 100644 --- a/realy/add-event.go +++ b/realy/add-event.go @@ -26,8 +26,9 @@ func AddEvent(c Ctx, rl relay.I, ev *event.T, hr *http.Request, origin S, wrapper := &wrapper.RelayWrapper{I: sto} advancedSaver, _ := sto.(relay.AdvancedSaver) - if !rl.AcceptEvent(c, ev, hr, origin, authedPubkey) { - return false, normalize.Blocked.F("event rejected by relay") + accept, notice := rl.AcceptEvent(c, ev, hr, origin, authedPubkey) + if !accept { + return false, normalize.Blocked.F(notice) } if ev.Kind.IsEphemeral() { diff --git a/realy/handlers.go b/realy/handlers.go index 75c6531..05d9f93 100644 --- a/realy/handlers.go +++ b/realy/handlers.go @@ -134,8 +134,8 @@ func (s *Server) doEvent(c Ctx, ws *web.Socket, req B, sto store.I) (msg B) { if len(rem) > 0 { log.I.F("extra '%s'", rem) } - - if !s.relay.AcceptEvent(c, env.T, ws.Req(), ws.RealRemote(), B(ws.Authed())) { + accept, notice := s.relay.AcceptEvent(c, env.T, ws.Req(), ws.RealRemote(), B(ws.Authed())) + if !accept { var auther relay.Authenticator if auther, ok = s.relay.(relay.Authenticator); ok && auther.AuthEnabled() { if !ws.AuthRequested() { @@ -161,6 +161,10 @@ func (s *Server) doEvent(c Ctx, ws *web.Socket, req B, sto store.I) (msg B) { return } } + // return an ok event containing any notice returned + if err = okenvelope.NewFrom(env.ID, false, + normalize.Invalid.F(notice)).Write(ws); chk.T(err) { + } return } // check id @@ -186,8 +190,9 @@ func (s *Server) doEvent(c Ctx, ws *web.Socket, req B, sto store.I) (msg B) { return } if env.T.Kind.K == kind.Deletion.K { + // we only handle e and a tag deletes because kind based deletes are too indiscriminate. log.I.F("delete event\n%s", env.T.Serialize()) - // event deletion -- nip09 + // event deletion -- nip-09 for _, t := range env.Tags.Value() { var res []*event.T if t.Len() >= 2 { @@ -207,6 +212,24 @@ func (s *Server) doEvent(c Ctx, ws *web.Socket, req B, sto store.I) (msg B) { } return } + for i := range res { + if res[i].Kind.Equal(kind.Deletion) { + if err = okenvelope.NewFrom(env.ID, false, + normalize.Blocked.F( + "not processing or storing delete event containing delete event references")). + Write(ws); chk.E(err) { + return + } + } + if !equals(res[i].PubKey, env.T.PubKey) { + if err = okenvelope.NewFrom(env.ID, false, + normalize.Blocked.F( + "cannot delete other users' events")). + Write(ws); chk.E(err) { + return + } + } + } case equals(t.Key(), B("a")): split := bytes.Split(t.Value(), B{':'}) if len(split) != 3 { @@ -216,8 +239,38 @@ func (s *Server) doEvent(c Ctx, ws *web.Socket, req B, sto store.I) (msg B) { if _, err = kin.UnmarshalJSON(split[0]); chk.E(err) { return } + kk := kind.New(kin.Uint16()) + if kk.Equal(kind.Deletion) { + // we don't delete delete events, period + if err = okenvelope.NewFrom(env.ID, false, + normalize.Blocked.F( + "delete event kind may not be deleted")). + Write(ws); chk.E(err) { + return + } + } + // if the kind is not parameterised replaceable, the tag is invalid and the + // delete event will not be saved. + if !kk.IsParameterizedReplaceable() { + if err = okenvelope.NewFrom(env.ID, false, + normalize.Error.F( + "delete tags with a tags containing non-parameterized-replaceable events cannot be processed")). + Write(ws); chk.E(err) { + return + } + } + // for this event kind, the second field of the tag value MUST be the pubkey + // of the author of the event + if !equals(split[1], env.T.PubKey) { + if err = okenvelope.NewFrom(env.ID, false, + normalize.Blocked.F( + "cannot delete other users' events")). + Write(ws); chk.E(err) { + return + } + } f := filter.New() - f.Kinds.K = []*kind.T{kind.New(kin.Uint16())} + f.Kinds.K = []*kind.T{kk} aut := make(B, 0, len(split[1])/2) if aut, err = hex.DecAppend(aut, split[1]); chk.E(err) { return @@ -238,6 +291,15 @@ func (s *Server) doEvent(c Ctx, ws *web.Socket, req B, sto store.I) (msg B) { // this will happen if event is not in the database continue } + // filter out any events that are newer than the delete request, deletes only work + // backwards, old delete events might match newer events for a tags. + var resTmp []*event.T + for _, v := range res { + if env.T.CreatedAt.U64() >= v.CreatedAt.U64() { + resTmp = append(resTmp, v) + } + } + res = resTmp for _, target := range res { if target.Kind.K == kind.Deletion.K { if err = okenvelope.NewFrom(env.ID, false, @@ -247,7 +309,7 @@ func (s *Server) doEvent(c Ctx, ws *web.Socket, req B, sto store.I) (msg B) { } } if target.CreatedAt.Int() > env.T.CreatedAt.Int() { - log.I.F("not replacing\n%d%\nbecause delete event is older\n%d", + log.I.F("not deleting\n%d%\nbecause delete event is older\n%d", target.CreatedAt.Int(), env.T.CreatedAt.Int()) continue } @@ -282,7 +344,7 @@ func (s *Server) doEvent(c Ctx, ws *web.Socket, req B, sto store.I) (msg B) { if err = okenvelope.NewFrom(env.ID, true).Write(ws); chk.E(err) { return } - // if the event is a delete we still want to save it. + // if the event is a deletion we still want to save it. } ok, reason := AddEvent(c, s.relay, env.T, ws.Req(), ws.RealRemote(), B(ws.Authed())) if err = okenvelope.NewFrom(env.ID, ok, reason).Write(ws); chk.E(err) { @@ -320,7 +382,8 @@ func (s *Server) doCount(c context.Context, ws *web.Socket, req B, allowed := env.Filters if accepter, ok := s.relay.(relay.ReqAcceptor); ok { var accepted bool - allowed, accepted = accepter.AcceptReq(c, ws.Req(), env.Subscription.T, env.Filters, B(ws.Authed())) + allowed, accepted = accepter.AcceptReq(c, ws.Req(), env.Subscription.T, env.Filters, + B(ws.Authed())) if !accepted || allowed == nil { var auther relay.Authenticator if auther, ok = s.relay.(relay.Authenticator); ok && @@ -432,7 +495,8 @@ func (s *Server) doReq(c Ctx, ws *web.Socket, req B, sto store.I) (r B) { allowed := env.Filters if accepter, ok := s.relay.(relay.ReqAcceptor); ok { var accepted bool - allowed, accepted = accepter.AcceptReq(c, ws.Req(), env.Subscription.T, env.Filters, B(ws.Authed())) + allowed, accepted = accepter.AcceptReq(c, ws.Req(), env.Subscription.T, env.Filters, + B(ws.Authed())) if !accepted || allowed == nil { var auther relay.Authenticator if auther, ok = s.relay.(relay.Authenticator); ok && diff --git a/realy/version b/realy/version index 166b8ce..558c7db 100644 --- a/realy/version +++ b/realy/version @@ -1 +1 @@ -v1.2.10 \ No newline at end of file +v1.2.14 \ No newline at end of file diff --git a/relay/interface.go b/relay/interface.go index df0d8bf..1ce8136 100644 --- a/relay/interface.go +++ b/relay/interface.go @@ -32,7 +32,8 @@ type I interface { // messages, that are not on the mute list, that do not yet have a reply, should accept // direct and group message events until there is three and thereafter will be restricted // until the user adds them to their follow list. - AcceptEvent(c Ctx, ev *event.T, hr *http.Request, origin S, authedPubkey B) bool + AcceptEvent(c Ctx, ev *event.T, hr *http.Request, origin S, authedPubkey B) (accept bool, + notice string) // Storage returns the realy storage implementation. Storage(Ctx) store.I } @@ -54,7 +55,8 @@ type ReqAcceptor interface { // support for in/outbox access. // // In order to support the ability to respond to - AcceptReq(ctx Ctx, hr *http.Request, id B, ff *filters.T, authedPubkey B) (allowed *filters.T, ok bool) + AcceptReq(ctx Ctx, hr *http.Request, id B, ff *filters.T, + authedPubkey B) (allowed *filters.T, ok bool) } // Authenticator is the interface for implementing NIP-42. diff --git a/relay/wrapper/relay_interface.go b/relay/wrapper/relay_interface.go index a2e702f..8bc7f74 100644 --- a/relay/wrapper/relay_interface.go +++ b/relay/wrapper/relay_interface.go @@ -51,10 +51,18 @@ func (w RelayWrapper) Publish(c Ctx, evt *event.T) (err E) { if ev.CreatedAt.Int() > evt.CreatedAt.Int() { return errorf.W(S(normalize.Invalid.F("not replacing newer event"))) } - log.T.F("%s\nreplacing\n%s", evt.Serialize(), ev.Serialize()) - if err = w.I.DeleteEvent(c, ev.EventID()); chk.E(err) { - continue - } + // defer the delete until after the save, further down, has completed. + defer func() { + if err != nil { + // something went wrong saving the replacement, so we won't delete + // the event. + return + } + log.T.F("%s\nreplacing\n%s", evt.Serialize(), ev.Serialize()) + if err = w.I.DeleteEvent(c, ev.EventID()); chk.E(err) { + return + } + }() } } } else if evt.Kind.IsParameterizedReplaceable() { @@ -83,17 +91,23 @@ func (w RelayWrapper) Publish(c Ctx, evt *event.T) (err E) { if !equals(evdt.Value(), evtdt.Value()) { continue } - log.I.F("%s\nreplacing\n%s", evt.Serialize(), ev.Serialize()) - if err = w.I.DeleteEvent(c, ev.EventID()); chk.E(err) { - continue - } + defer func() { + if err != nil { + // something went wrong saving the replacement, so we won't delete + // the event. + return + } + log.I.F("%s\nreplacing\n%s", evt.Serialize(), ev.Serialize()) + if err = w.I.DeleteEvent(c, ev.EventID()); chk.E(err) { + return + } + }() } } } if err = w.SaveEvent(c, evt); chk.E(err) && !errors.Is(err, store.ErrDupEvent) { return errorf.E("failed to save: %w", err) } - return }