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.
This commit is contained in:
Azareal 2021-05-12 12:04:55 +10:00
parent eb67af6efd
commit e01a181eda
5 changed files with 70 additions and 42 deletions

View File

@ -29,6 +29,7 @@ func AddHooks(add func(name, params, ret, htype string, multiHook, skip bool, de
vhookskip := func(name, params string) { vhookskip := func(name, params string) {
add(name, params, "(bool,RouteError)", "VhookSkippable_", false, true, "false,nil", "") 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("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_after_filters", "w http.ResponseWriter,r *http.Request,prefix string")
vhookskip("router_pre_route", "w http.ResponseWriter,r *http.Request,u *User,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_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_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") vhookskip("route_attach_post_get", "w http.ResponseWriter,r *http.Request,u *User,a *Attachment")
vhooknoret := func(name, params string) { vhooknoret := func(name, params string) {
add(name, params, "", "Vhooks", false, false, "false,nil", "") 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) { /*hook := func(name, params, ret, pure string) {
add(name,params,ret,"Hooks",true,false,ret,pure) add(name,params,ret,"Hooks",true,false,ret,pure)
}*/ }*/
hooknoret := func(name, params string) { hooknoret := func(name, params string) {
add(name, params, "", "HooksNoRet", true, false, "", "") add(name, params, "", "HooksNoRet", true, false, "", "")
} }
hooknoret("forums_frow_assign", "f *Forum") hooknoret("forums_frow_assign", "f *Forum")
hookskip := func(name, params string) { hookskip := func(name, params string) {
add(name, params, "(skip bool)", "HooksSkip", true, true, "", "") add(name, params, "(skip bool)", "HooksSkip", true, true, "", "")
} }
//hookskip("forums_frow_assign","f *Forum") //hookskip("forums_frow_assign","f *Forum")
hookskip("topic_create_frow_assign", "f *Forum") hookskip("topic_create_frow_assign", "f *Forum")
hookss := func(name string) { hookss := func(name string) {
add(name, "d string", "string", "Sshooks", true, false, "", "d") add(name, "d string", "string", "Sshooks", true, false, "", "d")
} }

View File

@ -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? // 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 { if skip || rerr != nil {
return h, rerr return h, rerr
} }
@ -162,18 +166,23 @@ func panelUserCheck(w http.ResponseWriter, r *http.Request, u *User) (h *Header,
stats.Themes = len(Themes) stats.Themes = len(Themes)
stats.Reports = 0 // TODO: Do the report count. Only show open threads? 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 // TODO: Optimise this by removing a superfluous string alloc
var tname string
if theme.OverridenMap != nil { if theme.OverridenMap != nil {
//fmt.Printf("name %+v\n", name)
//fmt.Printf("theme.OverridenMap %+v\n", theme.OverridenMap)
if _, ok := theme.OverridenMap[name]; ok { 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("alert", 3)
addPreScript("notice") addPreScript("notice", 4)
return h, stats, nil return h, stats, nil
} }
@ -203,7 +212,6 @@ func GetThemeByReq(r *http.Request) *Theme {
if theme.Name == "" { if theme.Name == "" {
theme = Themes[DefaultThemeBox.Load().(string)] theme = Themes[DefaultThemeBox.Load().(string)]
} }
return theme 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: WIP. Refactor this to eliminate the unnecessary query
// TODO: Better take proxies into consideration // TODO: Better take proxies into consideration
host, _, err := net.SplitHostPort(r.RemoteAddr) if !Config.DisableIP {
if err != nil { var host string
_ = PreError("Bad IP", w, r) // TODO: Prefer Cf-Connecting-Ip header, fewer shenanigans
return *usercpy, false 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")
// TODO: Prefer Cf-Connecting-Ip header, fewer shenanigans if xForwardedFor != "" {
if Site.HasProxy { forwardedFor := strings.Split(xForwardedFor, ",")
// TODO: Check the right-most IP, might get tricky with multiple proxies, maybe have a setting for the number of hops we jump through // TODO: Check if this is a valid IP Address, reject if not
xForwardedFor := r.Header.Get("X-Forwarded-For") host = forwardedFor[len(forwardedFor)-1]
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() { if host == "" {
mon := time.Now().Month() var e error
err = usercpy.UpdateIP(strconv.Itoa(int(mon)) + "-" + host) host, _, e = net.SplitHostPort(r.RemoteAddr)
if err != nil { if e != nil {
_ = InternalError(err, w, r) _ = PreError("Bad IP", w, r)
return *usercpy, false 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 return *usercpy, true
} }
@ -513,11 +527,11 @@ func NoSessionMismatch(w http.ResponseWriter, r *http.Request, u *User) RouteErr
if e := r.ParseForm(); e != nil { if e := r.ParseForm(); e != nil {
return LocalError("Bad Form", w, r, u) return LocalError("Bad Form", w, r, u)
} }
// TODO: Try to eliminate some of these allocations if len(u.Session) == 0 {
sess := []byte(u.Session)
if len(sess) == 0 {
return SecurityError(w, r, u) 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 { if subtle.ConstantTimeCompare([]byte(r.FormValue("session")), sess) != 1 && subtle.ConstantTimeCompare([]byte(r.FormValue("s")), sess) != 1 {
return SecurityError(w, r, u) 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) r.Body = http.MaxBytesReader(w, r.Body, r.ContentLength)
err := r.ParseMultipartForm(int64(Megabyte)) e := r.ParseMultipartForm(int64(Megabyte))
if err != nil { if e != nil {
return LocalError("Bad Form", w, r, u) return LocalError("Bad Form", w, r, u)
} }
return nil return nil
} }
func NoUploadSessionMismatch(w http.ResponseWriter, r *http.Request, u *User) RouteError { func NoUploadSessionMismatch(w http.ResponseWriter, r *http.Request, u *User) RouteError {
// TODO: Try to eliminate some of these allocations if len(u.Session) == 0 {
sess := []byte(u.Session)
if len(sess) == 0 {
return SecurityError(w, r, u) 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 { if subtle.ConstantTimeCompare([]byte(r.FormValue("session")), sess) != 1 && subtle.ConstantTimeCompare([]byte(r.FormValue("s")), sess) != 1 {
return SecurityError(w, r, u) return SecurityError(w, r, u)
} }

View File

@ -99,6 +99,7 @@ type config struct {
LogPruneCutoff int LogPruneCutoff int
//SelfDeleteTruncCutoff int // Personal data is stripped from the mod action rows only leaving the TID and the action for later investigation. //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 DisableLastIP bool
DisablePostIP bool DisablePostIP bool
DisablePollIP bool DisablePollIP bool
@ -305,6 +306,12 @@ func ProcessConfig() (err error) {
Config.Noavatar = strings.Replace(Config.Noavatar, "{site_url}", Site.URL, -1) Config.Noavatar = strings.Replace(Config.Noavatar, "{site_url}", Site.URL, -1)
guestAvatar = GuestAvatar{buildNoavatar(0, 200), buildNoavatar(0, 48)} guestAvatar = GuestAvatar{buildNoavatar(0, 200), buildNoavatar(0, 48)}
if Config.DisableIP {
Config.DisableLastIP = true
Config.DisablePostIP = true
Config.DisablePollIP = true
}
if Config.PostIPCutoff == 0 { if Config.PostIPCutoff == 0 {
Config.PostIPCutoff = 90 // Default cutoff Config.PostIPCutoff = 90 // Default cutoff
} }

View File

@ -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. 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 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 DisablePostIP - Disable storing post IPs for users and purge any existing post IP data. Default: false

View File

@ -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) { func AttachTopicActCommon(w http.ResponseWriter, r *http.Request, u *c.User, stid string) (t *c.Topic, ferr c.RouteError) {
tid, err := strconv.Atoi(stid) tid, e := strconv.Atoi(stid)
if err != nil { if e != nil {
return t, c.LocalErrorJS(p.GetErrorPhrase("id_must_be_integer"), w, r) return t, c.LocalErrorJS(p.GetErrorPhrase("id_must_be_integer"), w, r)
} }
t, err = c.Topics.Get(tid) t, e = c.Topics.Get(tid)
if err != nil { if e != nil {
return t, c.NotFoundJS(w, r) return t, c.NotFoundJS(w, r)
} }
_, ferr = c.SimpleForumUserCheck(w, r, u, t.ParentID) _, ferr = c.SimpleForumUserCheck(w, r, u, t.ParentID)