From e01a181edaabf2e88197fc4490c1c8c9770b7a4e Mon Sep 17 00:00:00 2001 From: Azareal Date: Wed, 12 May 2021 12:04:55 +1000 Subject: [PATCH] Add DisableIP config setting. Use hookgen for simple_forum_check_pre_perms. Optimise panelUserCheck()'s addPreScript() Reduce allocs slightly for PreRoute() host IP setting. Reorder error check in NoSessionMismatch() for faster failure. Reorder error check in NoUploadSessionMismatch() for faster failure. Reduce boilerplate. --- cmd/common_hook_gen/hookgen.go | 5 ++ common/routes_common.go | 90 ++++++++++++++++++++-------------- common/site.go | 7 +++ docs/configuration.md | 2 + routes/topic.go | 8 +-- 5 files changed, 70 insertions(+), 42 deletions(-) diff --git a/cmd/common_hook_gen/hookgen.go b/cmd/common_hook_gen/hookgen.go index 7cf699d7..092bc247 100644 --- a/cmd/common_hook_gen/hookgen.go +++ b/cmd/common_hook_gen/hookgen.go @@ -29,6 +29,7 @@ func AddHooks(add func(name, params, ret, htype string, multiHook, skip bool, de vhookskip := func(name, params string) { add(name, params, "(bool,RouteError)", "VhookSkippable_", false, true, "false,nil", "") } + vhookskip("simple_forum_check_pre_perms", "w http.ResponseWriter,r *http.Request,u *User,fid *int,h *HeaderLite") vhookskip("forum_check_pre_perms", "w http.ResponseWriter,r *http.Request,u *User,fid *int,h *Header") vhookskip("router_after_filters", "w http.ResponseWriter,r *http.Request,prefix string") vhookskip("router_pre_route", "w http.ResponseWriter,r *http.Request,u *User,prefix string") @@ -36,6 +37,7 @@ func AddHooks(add func(name, params, ret, htype string, multiHook, skip bool, de vhookskip("route_topic_list_start", "w http.ResponseWriter,r *http.Request,u *User,h *Header") vhookskip("route_attach_start", "w http.ResponseWriter,r *http.Request,u *User,fname string") vhookskip("route_attach_post_get", "w http.ResponseWriter,r *http.Request,u *User,a *Attachment") + vhooknoret := func(name, params string) { add(name, params, "", "Vhooks", false, false, "false,nil", "") } @@ -47,15 +49,18 @@ func AddHooks(add func(name, params, ret, htype string, multiHook, skip bool, de /*hook := func(name, params, ret, pure string) { add(name,params,ret,"Hooks",true,false,ret,pure) }*/ + hooknoret := func(name, params string) { add(name, params, "", "HooksNoRet", true, false, "", "") } hooknoret("forums_frow_assign", "f *Forum") + hookskip := func(name, params string) { add(name, params, "(skip bool)", "HooksSkip", true, true, "", "") } //hookskip("forums_frow_assign","f *Forum") hookskip("topic_create_frow_assign", "f *Forum") + hookss := func(name string) { add(name, "d string", "string", "Sshooks", true, false, "", "d") } diff --git a/common/routes_common.go b/common/routes_common.go index cc6ddaf8..a2642c0e 100644 --- a/common/routes_common.go +++ b/common/routes_common.go @@ -39,7 +39,11 @@ func simpleForumUserCheck(w http.ResponseWriter, r *http.Request, u *User, fid i } // Is there a better way of doing the skip AND the success flag on this hook like multiple returns? - skip, rerr := h.Hooks.VhookSkippable("simple_forum_check_pre_perms", w, r, u, &fid, h) + /*skip, rerr := h.Hooks.VhookSkippable("simple_forum_check_pre_perms", w, r, u, &fid, h) + if skip || rerr != nil { + return h, rerr + }*/ + skip, rerr := H_simple_forum_check_pre_perms_hook(h.Hooks, w, r, u, &fid, h) if skip || rerr != nil { return h, rerr } @@ -162,18 +166,23 @@ func panelUserCheck(w http.ResponseWriter, r *http.Request, u *User) (h *Header, stats.Themes = len(Themes) stats.Reports = 0 // TODO: Do the report count. Only show open threads? - addPreScript := func(name string) { + addPreScript := func(name string, i int) { // TODO: Optimise this by removing a superfluous string alloc - var tname string if theme.OverridenMap != nil { + //fmt.Printf("name %+v\n", name) + //fmt.Printf("theme.OverridenMap %+v\n", theme.OverridenMap) if _, ok := theme.OverridenMap[name]; ok { - tname = "_" + theme.Name + tname := "_" + theme.Name + //fmt.Printf("tname %+v\n", tname) + h.AddPreScriptAsync("tmpl_" + name + tname + ".js") + return } } - h.AddPreScriptAsync("tmpl_" + name + tname + ".js") + //fmt.Printf("tname %+v\n", tname) + h.AddPreScriptAsync(ucstrs[i]) } - addPreScript("alert") - addPreScript("notice") + addPreScript("alert", 3) + addPreScript("notice", 4) return h, stats, nil } @@ -203,7 +212,6 @@ func GetThemeByReq(r *http.Request) *Theme { if theme.Name == "" { theme = Themes[DefaultThemeBox.Load().(string)] } - return theme } @@ -341,32 +349,38 @@ func preRoute(w http.ResponseWriter, r *http.Request) (User, bool) { // TODO: WIP. Refactor this to eliminate the unnecessary query // TODO: Better take proxies into consideration - host, _, err := net.SplitHostPort(r.RemoteAddr) - if err != nil { - _ = PreError("Bad IP", w, r) - return *usercpy, false - } - - // TODO: Prefer Cf-Connecting-Ip header, fewer shenanigans - if Site.HasProxy { - // TODO: Check the right-most IP, might get tricky with multiple proxies, maybe have a setting for the number of hops we jump through - xForwardedFor := r.Header.Get("X-Forwarded-For") - if xForwardedFor != "" { - forwardedFor := strings.Split(xForwardedFor, ",") - // TODO: Check if this is a valid IP Address, reject if not - host = forwardedFor[len(forwardedFor)-1] + if !Config.DisableIP { + var host string + // TODO: Prefer Cf-Connecting-Ip header, fewer shenanigans + if Site.HasProxy { + // TODO: Check the right-most IP, might get tricky with multiple proxies, maybe have a setting for the number of hops we jump through + xForwardedFor := r.Header.Get("X-Forwarded-For") + if xForwardedFor != "" { + forwardedFor := strings.Split(xForwardedFor, ",") + // TODO: Check if this is a valid IP Address, reject if not + host = forwardedFor[len(forwardedFor)-1] + } } - } - if !Config.DisableLastIP && usercpy.Loggedin && host != usercpy.GetIP() { - mon := time.Now().Month() - err = usercpy.UpdateIP(strconv.Itoa(int(mon)) + "-" + host) - if err != nil { - _ = InternalError(err, w, r) - return *usercpy, false + if host == "" { + var e error + host, _, e = net.SplitHostPort(r.RemoteAddr) + if e != nil { + _ = PreError("Bad IP", w, r) + return *usercpy, false + } } + + if !Config.DisableLastIP && usercpy.Loggedin && host != usercpy.GetIP() { + mon := time.Now().Month() + e := usercpy.UpdateIP(strconv.Itoa(int(mon)) + "-" + host) + if e != nil { + _ = InternalError(e, w, r) + return *usercpy, false + } + } + usercpy.LastIP = host } - usercpy.LastIP = host return *usercpy, true } @@ -513,11 +527,11 @@ func NoSessionMismatch(w http.ResponseWriter, r *http.Request, u *User) RouteErr if e := r.ParseForm(); e != nil { return LocalError("Bad Form", w, r, u) } - // TODO: Try to eliminate some of these allocations - sess := []byte(u.Session) - if len(sess) == 0 { + if len(u.Session) == 0 { return SecurityError(w, r, u) } + // TODO: Try to eliminate some of these allocations + sess := []byte(u.Session) if subtle.ConstantTimeCompare([]byte(r.FormValue("session")), sess) != 1 && subtle.ConstantTimeCompare([]byte(r.FormValue("s")), sess) != 1 { return SecurityError(w, r, u) } @@ -536,19 +550,19 @@ func HandleUploadRoute(w http.ResponseWriter, r *http.Request, u *User, maxFileS } r.Body = http.MaxBytesReader(w, r.Body, r.ContentLength) - err := r.ParseMultipartForm(int64(Megabyte)) - if err != nil { + e := r.ParseMultipartForm(int64(Megabyte)) + if e != nil { return LocalError("Bad Form", w, r, u) } return nil } func NoUploadSessionMismatch(w http.ResponseWriter, r *http.Request, u *User) RouteError { - // TODO: Try to eliminate some of these allocations - sess := []byte(u.Session) - if len(sess) == 0 { + if len(u.Session) == 0 { return SecurityError(w, r, u) } + // TODO: Try to eliminate some of these allocations + sess := []byte(u.Session) if subtle.ConstantTimeCompare([]byte(r.FormValue("session")), sess) != 1 && subtle.ConstantTimeCompare([]byte(r.FormValue("s")), sess) != 1 { return SecurityError(w, r, u) } diff --git a/common/site.go b/common/site.go index ad6fc4b9..12c0578d 100644 --- a/common/site.go +++ b/common/site.go @@ -99,6 +99,7 @@ type config struct { LogPruneCutoff int //SelfDeleteTruncCutoff int // Personal data is stripped from the mod action rows only leaving the TID and the action for later investigation. + DisableIP bool DisableLastIP bool DisablePostIP bool DisablePollIP bool @@ -305,6 +306,12 @@ func ProcessConfig() (err error) { Config.Noavatar = strings.Replace(Config.Noavatar, "{site_url}", Site.URL, -1) guestAvatar = GuestAvatar{buildNoavatar(0, 200), buildNoavatar(0, 48)} + if Config.DisableIP { + Config.DisableLastIP = true + Config.DisablePostIP = true + Config.DisablePollIP = true + } + if Config.PostIPCutoff == 0 { Config.PostIPCutoff = 90 // Default cutoff } diff --git a/docs/configuration.md b/docs/configuration.md index e5d61546..b5ce5d13 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -92,6 +92,8 @@ PostIPCutoff - The number of days which need to pass before the IP data for a po PollIPCutoff - The number of days which need to pass before the IP data for a poll is automatically deleted. 0 defaults to whatever the current default is, currently 90 and -1 disables this feature. +DisableIP - Master switch to disable tracking user IPs for any purpose. May not entirely clear already stored data, or data logged by an upstream like a reverse-proxy. Currently doesn't cover net/http ErrorLog. Default: false + DisableLastIP - Disable storing last IPs for users and purge any existing user last IP data. Default: false DisablePostIP - Disable storing post IPs for users and purge any existing post IP data. Default: false diff --git a/routes/topic.go b/routes/topic.go index 4bf04ad8..33044504 100644 --- a/routes/topic.go +++ b/routes/topic.go @@ -204,12 +204,12 @@ func ViewTopic(w http.ResponseWriter, r *http.Request, user *c.User, h *c.Header } func AttachTopicActCommon(w http.ResponseWriter, r *http.Request, u *c.User, stid string) (t *c.Topic, ferr c.RouteError) { - tid, err := strconv.Atoi(stid) - if err != nil { + tid, e := strconv.Atoi(stid) + if e != nil { return t, c.LocalErrorJS(p.GetErrorPhrase("id_must_be_integer"), w, r) } - t, err = c.Topics.Get(tid) - if err != nil { + t, e = c.Topics.Get(tid) + if e != nil { return t, c.NotFoundJS(w, r) } _, ferr = c.SimpleForumUserCheck(w, r, u, t.ParentID)