From 1ab030d69089b127b4af1b7a670585d6e0141b0a Mon Sep 17 00:00:00 2001 From: Jeremy Yen Date: Wed, 25 Sep 2024 04:06:13 +0800 Subject: [PATCH] Replace decisionListsMutex with RWMutex --- banjax.go | 2 +- internal/config.go | 14 +++++++------- internal/http_server.go | 30 +++++++++++++++--------------- internal/iptables.go | 2 +- internal/kafka.go | 8 ++++---- internal/regex_rate_limiter.go | 16 ++++++++-------- 6 files changed, 36 insertions(+), 36 deletions(-) diff --git a/banjax.go b/banjax.go index 2c4445e..fd66f98 100644 --- a/banjax.go +++ b/banjax.go @@ -178,7 +178,7 @@ func main() { var passwordProtectedPaths internal.PasswordProtectedPaths // XXX protects decisionLists - var decisionListsMutex sync.Mutex + var decisionListsMutex sync.RWMutex var decisionLists internal.DecisionLists standaloneTestingPtr := flag.Bool("standalone-testing", false, "makes it easy to test standalone") diff --git a/internal/config.go b/internal/config.go index c02ba8a..4baf2c2 100644 --- a/internal/config.go +++ b/internal/config.go @@ -451,7 +451,7 @@ func checkExpiringDecisionListsByDomain(domain string, decisionLists *DecisionLi // XXX mmm could hold the lock for a while? func RemoveExpiredDecisions( - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, ) { decisionListsMutex.Lock() @@ -466,7 +466,7 @@ func RemoveExpiredDecisions( } func removeExpiredDecisionsByIp( - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, ip string, ) { @@ -480,7 +480,7 @@ func removeExpiredDecisionsByIp( func updateExpiringDecisionLists( config *Config, ip string, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, expires time.Time, newDecision Decision, @@ -514,7 +514,7 @@ func updateExpiringDecisionListsSessionId( config *Config, ip string, sessionId string, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, expires time.Time, newDecision Decision, @@ -550,14 +550,14 @@ type MetricsLogLine struct { func WriteMetricsToEncoder( metricsLogEncoder *json.Encoder, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, rateLimitMutex *sync.Mutex, ipToRegexStates *IpToRegexStates, failedChallengeStates *FailedChallengeStates, ) { - decisionListsMutex.Lock() - defer decisionListsMutex.Unlock() + decisionListsMutex.RLock() + defer decisionListsMutex.RUnlock() lenExpiringChallenges := 0 lenExpiringBlocks := 0 diff --git a/internal/http_server.go b/internal/http_server.go index 74225de..e5e8adc 100644 --- a/internal/http_server.go +++ b/internal/http_server.go @@ -28,7 +28,7 @@ const ( func RunHttpServer( config *Config, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, passwordProtectedPaths *PasswordProtectedPaths, rateLimitMutex *sync.Mutex, @@ -487,7 +487,7 @@ func tooManyFailedChallenges( rateLimitMutex *sync.Mutex, failedChallengeStates *FailedChallengeStates, method string, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, ) (tooManyFailedChallengesResult TooManyFailedChallengesResult) { rateLimitMutex.Lock() @@ -591,7 +591,7 @@ func sendOrValidateShaChallenge( rateLimitMutex *sync.Mutex, failedChallengeStates *FailedChallengeStates, failAction FailAction, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, ) (sendOrValidateShaChallengeResult SendOrValidateShaChallengeResult) { clientIp := c.Request.Header.Get("X-Client-IP") @@ -693,7 +693,7 @@ func sendOrValidatePassword( banner BannerInterface, rateLimitMutex *sync.Mutex, failedChallengeStates *FailedChallengeStates, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, ) (sendOrValidatePasswordResult SendOrValidatePasswordResult) { clientIp := c.Request.Header.Get("X-Client-IP") @@ -833,7 +833,7 @@ type DecisionForNginxResult struct { func decisionForNginx( config *Config, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, passwordProtectedPaths *PasswordProtectedPaths, rateLimitMutex *sync.Mutex, @@ -868,16 +868,16 @@ func decisionForNginx( func checkPerSiteDecisionLists( config *Config, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, requestedHost string, clientIp string, ) (bool, Decision) { // XXX ugh this locking is awful // i got bit by just checking against the zero value here, which is a valid iota enum - decisionListsMutex.Lock() + decisionListsMutex.RLock() decision, ok := (*decisionLists).PerSiteDecisionLists[requestedHost][clientIp] - decisionListsMutex.Unlock() + decisionListsMutex.RUnlock() // found as plain IP form, no need to check IPFilter if ok { @@ -907,7 +907,7 @@ func checkPerSiteDecisionLists( func decisionForNginx2( c *gin.Context, config *Config, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, passwordProtectedPaths *PasswordProtectedPaths, rateLimitMutex *sync.Mutex, @@ -1020,9 +1020,9 @@ func decisionForNginx2( } } - decisionListsMutex.Lock() + decisionListsMutex.RLock() decision, ok = (*decisionLists).GlobalDecisionLists[clientIp] - decisionListsMutex.Unlock() + decisionListsMutex.RUnlock() foundInIpFilter := false if !ok { for _, iterateDecision := range []Decision{Allow, Challenge, NginxBlock, IptablesBlock} { @@ -1074,9 +1074,9 @@ func decisionForNginx2( // when we insert something into the list, really we might just be extending the expiry time and/or // changing the decision. // XXX i forget if that comment is stale^ - decisionListsMutex.Lock() + decisionListsMutex.RLock() expiringDecision, ok := checkExpiringDecisionLists(c, clientIp, decisionLists) - decisionListsMutex.Unlock() + decisionListsMutex.RUnlock() if !ok { // log.Println("no mention in expiring lists") } else { @@ -1118,9 +1118,9 @@ func decisionForNginx2( // the legacy banjax_sha_inv and user_banjax_sha_inv // difference is one blocks after many failures and the other doesn't - decisionListsMutex.Lock() + decisionListsMutex.RLock() failAction, ok := (*decisionLists).SitewideShaInvList[requestedHost] - decisionListsMutex.Unlock() + decisionListsMutex.RUnlock() if !ok { // log.Println("no mention in sitewide list") } else { diff --git a/internal/iptables.go b/internal/iptables.go index aa381a0..50886f6 100644 --- a/internal/iptables.go +++ b/internal/iptables.go @@ -126,7 +126,7 @@ type BannerInterface interface { } type Banner struct { - DecisionListsMutex *sync.Mutex + DecisionListsMutex *sync.RWMutex DecisionLists *DecisionLists Logger *log.Logger LoggerTemp *log.Logger diff --git a/internal/kafka.go b/internal/kafka.go index 5a5833a..57571e4 100644 --- a/internal/kafka.go +++ b/internal/kafka.go @@ -81,7 +81,7 @@ func getDialer(config *Config) *kafka.Dialer { func RunKafkaReader( config *Config, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, wg *sync.WaitGroup, ) { @@ -158,7 +158,7 @@ func getBlockSessionTtl(config *Config, host string) (blockSessionTtl int) { func handleCommand( config *Config, command commandMessage, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, ) { // exempt a site from baskerville according to config @@ -191,7 +191,7 @@ func handleCommand( func handleIPCommand( config *Config, command commandMessage, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, decision Decision, expireDuration int, @@ -219,7 +219,7 @@ func handleIPCommand( func handleSessionCommand( config *Config, command commandMessage, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, decision Decision, expireDuration int, diff --git a/internal/regex_rate_limiter.go b/internal/regex_rate_limiter.go index 2d4861b..274bd97 100644 --- a/internal/regex_rate_limiter.go +++ b/internal/regex_rate_limiter.go @@ -24,7 +24,7 @@ func RunLogTailer( banner BannerInterface, rateLimitMutex *sync.Mutex, ipToRegexStates *IpToRegexStates, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, wg *sync.WaitGroup, ) { @@ -120,12 +120,12 @@ func parseTimestamp(timeIpRest []string) (timestamp time.Time, err error) { func checkIpInGlobalDecisionList( ipString string, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, )(bool) { // Check if IP is in the global allow list that should be skipped - decisionListsMutex.Lock() - defer decisionListsMutex.Unlock() + decisionListsMutex.RLock() + defer decisionListsMutex.RUnlock() decision, ok := (*decisionLists).GlobalDecisionLists[ipString] if (ok && decision == Allow) { @@ -146,11 +146,11 @@ func checkIpInGlobalDecisionList( func checkIpInPerSiteDecisionList( urlString string, ipString string, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, ) (bool) { - decisionListsMutex.Lock() - defer decisionListsMutex.Unlock() + decisionListsMutex.RLock() + defer decisionListsMutex.RUnlock() decision, ok := (*decisionLists).PerSiteDecisionLists[urlString][ipString] if (ok && decision == Allow) { @@ -181,7 +181,7 @@ func consumeLine( ipToRegexStates *IpToRegexStates, banner BannerInterface, config *Config, - decisionListsMutex *sync.Mutex, + decisionListsMutex *sync.RWMutex, decisionLists *DecisionLists, ) (consumeLineResult ConsumeLineResult) {