From 1c071d4e5b02fb52f58c0ead60b8301cbb8d9fcd Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Tue, 11 Aug 2020 17:44:02 +0100 Subject: [PATCH] Add tests, tweaks to behaviour --- currentstateserver/acls/acls.go | 38 +++++--- currentstateserver/acls/acls_test.go | 105 +++++++++++++++++++++ currentstateserver/api/wrapper.go | 14 +++ currentstateserver/consumers/roomserver.go | 2 +- currentstateserver/inthttp/client.go | 2 +- currentstateserver/inthttp/server.go | 2 +- sytest-whitelist | 1 + 7 files changed, 149 insertions(+), 15 deletions(-) create mode 100644 currentstateserver/acls/acls_test.go diff --git a/currentstateserver/acls/acls.go b/currentstateserver/acls/acls.go index d36942a58..12619f5fc 100644 --- a/currentstateserver/acls/acls.go +++ b/currentstateserver/acls/acls.go @@ -1,3 +1,17 @@ +// Copyright 2020 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package acls import ( @@ -57,6 +71,13 @@ type serverACL struct { deniedRegexes []*regexp.Regexp } +func compileACLRegex(orig string) (*regexp.Regexp, error) { + escaped := regexp.QuoteMeta(orig) + escaped = strings.Replace(escaped, "\\?", ".", -1) + escaped = strings.Replace(escaped, "\\*", ".*", -1) + return regexp.Compile(escaped) +} + func (s *ServerACLs) OnServerACLUpdate(state *gomatrixserverlib.Event) { acls := &serverACL{} if err := json.Unmarshal(state.Content(), &acls.ServerACL); err != nil { @@ -68,20 +89,14 @@ func (s *ServerACLs) OnServerACLUpdate(state *gomatrixserverlib.Event) { // special characters and then replace * and ? with their regex counterparts. // https://matrix.org/docs/spec/client_server/r0.6.1#m-room-server-acl for _, orig := range acls.Allowed { - escaped := regexp.QuoteMeta(orig) - escaped = strings.Replace(escaped, "\\?", "(.)", -1) - escaped = strings.Replace(escaped, "\\*", "(.*)", -1) - if expr, err := regexp.Compile(escaped); err != nil { + if expr, err := compileACLRegex(orig); err != nil { logrus.WithError(err).Errorf("Failed to compile allowed regex") } else { acls.allowedRegexes = append(acls.allowedRegexes, expr) } } for _, orig := range acls.Denied { - escaped := regexp.QuoteMeta(orig) - escaped = strings.Replace(escaped, "\\?", "(.)", -1) - escaped = strings.Replace(escaped, "\\*", "(.*)", -1) - if expr, err := regexp.Compile(escaped); err != nil { + if expr, err := compileACLRegex(orig); err != nil { logrus.WithError(err).Errorf("Failed to compile denied regex") } else { acls.deniedRegexes = append(acls.deniedRegexes, expr) @@ -97,7 +112,7 @@ func (s *ServerACLs) OnServerACLUpdate(state *gomatrixserverlib.Event) { s.acls[state.RoomID()] = acls } -func (s *ServerACLs) IsServerBannedFromRoom(serverNameAndPort gomatrixserverlib.ServerName, roomID string) bool { +func (s *ServerACLs) IsServerBannedFromRoom(serverName gomatrixserverlib.ServerName, roomID string) bool { s.aclsMutex.RLock() // First of all check if we have an ACL for this room. If we don't then // no servers are banned from the room. @@ -109,9 +124,8 @@ func (s *ServerACLs) IsServerBannedFromRoom(serverNameAndPort gomatrixserverlib. s.aclsMutex.RUnlock() // Split the host and port apart. This is because the spec calls on us to // validate the hostname only in cases where the port is also present. - serverName, _, err := net.SplitHostPort(string(serverNameAndPort)) - if err != nil { - return true + if serverNameOnly, _, err := net.SplitHostPort(string(serverName)); err == nil { + serverName = gomatrixserverlib.ServerName(serverNameOnly) } // Check if the hostname is an IPv4 or IPv6 literal. We cheat here by adding // a /0 prefix length just to trick ParseCIDR into working. If we find that diff --git a/currentstateserver/acls/acls_test.go b/currentstateserver/acls/acls_test.go new file mode 100644 index 000000000..ed575cd58 --- /dev/null +++ b/currentstateserver/acls/acls_test.go @@ -0,0 +1,105 @@ +// Copyright 2020 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package acls + +import ( + "regexp" + "testing" +) + +func TestOpenACLsWithBlacklist(t *testing.T) { + roomID := "!test:test.com" + allowRegex, err := compileACLRegex("*") + if err != nil { + t.Fatalf(err.Error()) + } + denyRegex, err := compileACLRegex("foo.com") + if err != nil { + t.Fatalf(err.Error()) + } + + acls := ServerACLs{ + acls: make(map[string]*serverACL), + } + + acls.acls[roomID] = &serverACL{ + ServerACL: ServerACL{ + AllowIPLiterals: true, + }, + allowedRegexes: []*regexp.Regexp{allowRegex}, + deniedRegexes: []*regexp.Regexp{denyRegex}, + } + + if acls.IsServerBannedFromRoom("1.2.3.4", roomID) { + t.Fatal("Expected 1.2.3.4 to be allowed but wasn't") + } + if acls.IsServerBannedFromRoom("1.2.3.4:2345", roomID) { + t.Fatal("Expected 1.2.3.4 to be allowed but wasn't") + } + if !acls.IsServerBannedFromRoom("foo.com", roomID) { + t.Fatal("Expected foo.com to be banned but wasn't") + } + if !acls.IsServerBannedFromRoom("foo.com:3456", roomID) { + t.Fatal("Expected foo.com:3456 to be banned but wasn't") + } + if acls.IsServerBannedFromRoom("bar.com", roomID) { + t.Fatal("Expected bar.com to be allowed but wasn't") + } + if acls.IsServerBannedFromRoom("bar.com:4567", roomID) { + t.Fatal("Expected bar.com to be allowed but wasn't") + } +} + +func TestDefaultACLsWithWhitelist(t *testing.T) { + roomID := "!test:test.com" + allowRegex, err := compileACLRegex("foo.com") + if err != nil { + t.Fatalf(err.Error()) + } + + acls := ServerACLs{ + acls: make(map[string]*serverACL), + } + + acls.acls[roomID] = &serverACL{ + ServerACL: ServerACL{ + AllowIPLiterals: false, + }, + allowedRegexes: []*regexp.Regexp{allowRegex}, + deniedRegexes: []*regexp.Regexp{}, + } + + if !acls.IsServerBannedFromRoom("1.2.3.4", roomID) { + t.Fatal("Expected 1.2.3.4 to be banned but wasn't") + } + if !acls.IsServerBannedFromRoom("1.2.3.4:2345", roomID) { + t.Fatal("Expected 1.2.3.4 to be banned but wasn't") + } + if acls.IsServerBannedFromRoom("foo.com", roomID) { + t.Fatal("Expected foo.com to be allowed but wasn't") + } + if acls.IsServerBannedFromRoom("foo.com:3456", roomID) { + t.Fatal("Expected foo.com:3456 to be allowed but wasn't") + } + if !acls.IsServerBannedFromRoom("bar.com", roomID) { + t.Fatal("Expected bar.com to be allowed but wasn't") + } + if !acls.IsServerBannedFromRoom("baz.com", roomID) { + t.Fatal("Expected baz.com to be allowed but wasn't") + } + if !acls.IsServerBannedFromRoom("qux.com:4567", roomID) { + t.Fatal("Expected baz.com to be allowed but wasn't") + } +} diff --git a/currentstateserver/api/wrapper.go b/currentstateserver/api/wrapper.go index 9c4246877..317fea431 100644 --- a/currentstateserver/api/wrapper.go +++ b/currentstateserver/api/wrapper.go @@ -1,3 +1,17 @@ +// Copyright 2020 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package api import ( diff --git a/currentstateserver/consumers/roomserver.go b/currentstateserver/consumers/roomserver.go index 9535f083d..492cf6218 100644 --- a/currentstateserver/consumers/roomserver.go +++ b/currentstateserver/consumers/roomserver.go @@ -79,7 +79,7 @@ func (c *OutputRoomEventConsumer) onNewRoomEvent( ) error { ev := msg.Event - if ev.Type() == "m.room.server_acl" { + if ev.Type() == "m.room.server_acl" && ev.StateKeyEquals("") { defer c.acls.OnServerACLUpdate(&ev.Event) } diff --git a/currentstateserver/inthttp/client.go b/currentstateserver/inthttp/client.go index cb13a865f..6d54f5484 100644 --- a/currentstateserver/inthttp/client.go +++ b/currentstateserver/inthttp/client.go @@ -113,7 +113,7 @@ func (h *httpCurrentStateInternalAPI) QueryKnownUsers( func (h *httpCurrentStateInternalAPI) QueryServerBannedFromRoom( ctx context.Context, req *api.QueryServerBannedFromRoomRequest, res *api.QueryServerBannedFromRoomResponse, ) error { - span, ctx := opentracing.StartSpanFromContext(ctx, "QueryKnownUsers") + span, ctx := opentracing.StartSpanFromContext(ctx, "QueryServerBannedFromRoom") defer span.Finish() apiURL := h.apiURL + QueryServerBannedFromRoomPath diff --git a/currentstateserver/inthttp/server.go b/currentstateserver/inthttp/server.go index ce596e90f..1cf8cd2ac 100644 --- a/currentstateserver/inthttp/server.go +++ b/currentstateserver/inthttp/server.go @@ -90,7 +90,7 @@ func AddRoutes(internalAPIMux *mux.Router, intAPI api.CurrentStateInternalAPI) { return util.JSONResponse{Code: http.StatusOK, JSON: &response} }), ) - internalAPIMux.Handle(QuerySharedUsersPath, + internalAPIMux.Handle(QueryServerBannedFromRoomPath, httputil.MakeInternalAPI("queryServerBannedFromRoom", func(req *http.Request) util.JSONResponse { request := api.QueryServerBannedFromRoomRequest{} response := api.QueryServerBannedFromRoomResponse{} diff --git a/sytest-whitelist b/sytest-whitelist index 81b93c504..e92e883c6 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -447,3 +447,4 @@ Banned servers cannot /event_auth Banned servers cannot get missing events Banned servers cannot get room state ids Banned servers cannot backfill +Inbound /v1/send_leave rejects leaves from other servers