From d5b483d4d1eaa1d22f4bf8564614e250f5029d9c Mon Sep 17 00:00:00 2001 From: Dessalines Date: Thu, 6 Feb 2020 16:07:59 -0500 Subject: [PATCH] Fixing rate limit checking to only ping after a success. Fixes #516 --- README.md | 14 ++-- docker/lemmy.hjson | 2 +- server/config/defaults.hjson | 2 +- server/src/websocket/server.rs | 141 ++++++++++++++++++++++----------- 4 files changed, 103 insertions(+), 56 deletions(-) diff --git a/README.md b/README.md index 1c5709745..27637b8aa 100644 --- a/README.md +++ b/README.md @@ -131,18 +131,18 @@ If you'd like to add translations, take a look at the [English translation file] lang | done | missing ---- | ---- | ------- ca | 98% | cross_posted_to,old,support_on_liberapay,time,action -de | 86% | cross_posted_to,create_private_message,send_secure_message,send_message,message,avatar,upload_avatar,show_avatars,old,docs,message_sent,messages,old_password,matrix_user_id,private_message_disclaimer,send_notifications_to_email,downvotes_disabled,enable_downvotes,open_registration,registration_closed,enable_nsfw,support_on_liberapay,donate_to_lemmy,donate,from,logged_in,email_already_exists,couldnt_create_private_message,no_private_message_edit_allowed,couldnt_update_private_message,time,action +de | 86% | cross_posted_to,create_private_message,send_secure_message,send_message,message,avatar,upload_avatar,show_avatars,old,docs,message_sent,messages,old_password,matrix_user_id,private_message_disclaimer,send_notifications_to_email,downvotes_disabled,enable_downvotes,open_registration,registration_closed,enable_nsfw,donate_to_lemmy,donate,from,logged_in,email_already_exists,couldnt_create_private_message,no_private_message_edit_allowed,couldnt_update_private_message,time,action fa | 72% | cross_post,cross_posted_to,subscribed_to_communities,trending_communities,create_private_message,send_secure_message,send_message,message,mod,mods,moderates,remove_as_mod,appoint_as_mod,modlog,stickied,ban,ban_from_site,unban,unban_from_site,banned,number_of_subscribers,subscribers,both,saved,unsubscribe,subscribe,subscribed,old,api,docs,inbox,inbox_for,message_sent,notifications_error,messages,no_email_setup,matrix_user_id,private_message_disclaimer,url,body,copy_suggested_title,community,expand_here,subscribe_to_communities,theme,sponsor_message,support_on_liberapay,general_sponsors,joined,by,to,from,landing_0,logged_in,community_moderator_already_exists,community_follower_already_exists,community_user_already_banned,no_slurs,admin_already_created,couldnt_create_private_message,no_private_message_edit_allowed,couldnt_update_private_message,time,action eo | 74% | cross_posted_to,number_of_communities,create_private_message,send_secure_message,send_message,message,preview,upload_image,avatar,upload_avatar,show_avatars,formatting_help,view_source,sticky,unsticky,archive_link,stickied,delete_account,delete_account_confirm,banned,creator,number_online,old,docs,replies,mentions,message_sent,messages,old_password,forgot_password,reset_password_mail_sent,password_change,new_password,no_email_setup,matrix_user_id,private_message_disclaimer,send_notifications_to_email,language,browser_default,downvotes_disabled,enable_downvotes,open_registration,registration_closed,enable_nsfw,theme,support_on_liberapay,donate_to_lemmy,donate,from,are_you_sure,yes,no,logged_in,email_already_exists,couldnt_create_private_message,no_private_message_edit_allowed,couldnt_update_private_message,time,action -es | 99% | cross_posted_to,support_on_liberapay +es | 100% | cross_posted_to fi | 98% | cross_posted_to,old,support_on_liberapay,time,action -fr | 81% | cross_posted_to,create_private_message,send_secure_message,send_message,message,avatar,upload_avatar,show_avatars,archive_link,old,docs,replies,mentions,message_sent,messages,old_password,forgot_password,reset_password_mail_sent,password_change,new_password,no_email_setup,matrix_user_id,private_message_disclaimer,send_notifications_to_email,language,browser_default,downvotes_disabled,enable_downvotes,open_registration,registration_closed,enable_nsfw,support_on_liberapay,donate_to_lemmy,donate,from,logged_in,email_already_exists,couldnt_create_private_message,no_private_message_edit_allowed,couldnt_update_private_message,time,action -it | 82% | cross_posted_to,create_private_message,send_secure_message,send_message,message,avatar,upload_avatar,show_avatars,archive_link,old,docs,message_sent,messages,old_password,forgot_password,reset_password_mail_sent,password_change,new_password,no_email_setup,matrix_user_id,private_message_disclaimer,send_notifications_to_email,language,browser_default,downvotes_disabled,enable_downvotes,open_registration,registration_closed,enable_nsfw,support_on_liberapay,donate_to_lemmy,donate,from,logged_in,email_already_exists,couldnt_create_private_message,no_private_message_edit_allowed,couldnt_update_private_message,time,action -nl | 98% | cross_posted_to,support_on_liberapay,time,action -pt-br | 100% | support_on_liberapay +fr | 82% | cross_posted_to,create_private_message,send_secure_message,send_message,message,avatar,upload_avatar,show_avatars,archive_link,old,docs,replies,mentions,message_sent,messages,old_password,forgot_password,reset_password_mail_sent,password_change,new_password,no_email_setup,matrix_user_id,private_message_disclaimer,send_notifications_to_email,language,browser_default,downvotes_disabled,enable_downvotes,open_registration,registration_closed,enable_nsfw,donate_to_lemmy,donate,from,logged_in,email_already_exists,couldnt_create_private_message,no_private_message_edit_allowed,couldnt_update_private_message,time,action +it | 83% | cross_posted_to,create_private_message,send_secure_message,send_message,message,avatar,upload_avatar,show_avatars,archive_link,old,docs,message_sent,messages,old_password,forgot_password,reset_password_mail_sent,password_change,new_password,no_email_setup,matrix_user_id,private_message_disclaimer,send_notifications_to_email,language,browser_default,downvotes_disabled,enable_downvotes,open_registration,registration_closed,enable_nsfw,donate_to_lemmy,donate,from,logged_in,email_already_exists,couldnt_create_private_message,no_private_message_edit_allowed,couldnt_update_private_message,time,action +nl | 99% | cross_posted_to,time,action +pt-br | 100% | ru | 70% | cross_posts,cross_post,cross_posted_to,number_of_communities,create_private_message,send_secure_message,send_message,message,preview,upload_image,avatar,upload_avatar,show_avatars,formatting_help,view_source,sticky,unsticky,archive_link,stickied,delete_account,delete_account_confirm,banned,creator,number_online,old,docs,replies,mentions,message_sent,messages,old_password,forgot_password,reset_password_mail_sent,password_change,new_password,no_email_setup,matrix_user_id,private_message_disclaimer,send_notifications_to_email,language,browser_default,downvotes_disabled,enable_downvotes,open_registration,registration_closed,enable_nsfw,recent_comments,theme,support_on_liberapay,donate_to_lemmy,donate,monero,by,to,from,transfer_community,transfer_site,are_you_sure,yes,no,logged_in,email_already_exists,couldnt_create_private_message,no_private_message_edit_allowed,couldnt_update_private_message,time,action sv | 81% | cross_posted_to,create_private_message,send_secure_message,send_message,message,avatar,upload_avatar,show_avatars,archive_link,old,docs,replies,mentions,message_sent,messages,old_password,forgot_password,reset_password_mail_sent,password_change,new_password,no_email_setup,matrix_user_id,private_message_disclaimer,send_notifications_to_email,language,browser_default,downvotes_disabled,enable_downvotes,open_registration,registration_closed,enable_nsfw,support_on_liberapay,donate_to_lemmy,donate,from,logged_in,email_already_exists,couldnt_create_private_message,no_private_message_edit_allowed,couldnt_update_private_message,time,action -zh | 69% | cross_posts,cross_post,cross_posted_to,users,number_of_communities,create_private_message,send_secure_message,send_message,message,preview,upload_image,avatar,upload_avatar,show_avatars,formatting_help,view_source,sticky,unsticky,archive_link,settings,stickied,delete_account,delete_account_confirm,banned,creator,number_online,old,docs,replies,mentions,message_sent,messages,old_password,forgot_password,reset_password_mail_sent,password_change,new_password,no_email_setup,matrix_user_id,private_message_disclaimer,send_notifications_to_email,language,browser_default,downvotes_disabled,enable_downvotes,open_registration,registration_closed,enable_nsfw,recent_comments,nsfw,show_nsfw,theme,support_on_liberapay,donate_to_lemmy,donate,monero,by,to,from,transfer_community,transfer_site,are_you_sure,yes,no,logged_in,email_already_exists,couldnt_create_private_message,no_private_message_edit_allowed,couldnt_update_private_message,time,action +zh | 69% | cross_posts,cross_post,cross_posted_to,users,number_of_communities,create_private_message,send_secure_message,send_message,message,preview,upload_image,avatar,upload_avatar,show_avatars,formatting_help,view_source,sticky,unsticky,archive_link,settings,stickied,delete_account,delete_account_confirm,banned,creator,number_online,old,docs,replies,mentions,message_sent,messages,old_password,forgot_password,reset_password_mail_sent,password_change,new_password,no_email_setup,matrix_user_id,private_message_disclaimer,send_notifications_to_email,language,browser_default,downvotes_disabled,enable_downvotes,open_registration,registration_closed,enable_nsfw,recent_comments,nsfw,show_nsfw,theme,donate_to_lemmy,donate,monero,by,to,from,transfer_community,transfer_site,are_you_sure,yes,no,logged_in,email_already_exists,couldnt_create_private_message,no_private_message_edit_allowed,couldnt_update_private_message,time,action If you'd like to update this report, run: diff --git a/docker/lemmy.hjson b/docker/lemmy.hjson index fce4470ca..55c2f2b76 100644 --- a/docker/lemmy.hjson +++ b/docker/lemmy.hjson @@ -29,7 +29,7 @@ # rate limits for various user actions, by user ip rate_limit: { # maximum number of messages created in interval - message: 30 + message: 180 # interval length for message limit message_per_second: 60 # maximum number of posts created in interval diff --git a/server/config/defaults.hjson b/server/config/defaults.hjson index 9a7ad49db..2b37f3bb3 100644 --- a/server/config/defaults.hjson +++ b/server/config/defaults.hjson @@ -30,7 +30,7 @@ # rate limits for various user actions, by user ip rate_limit: { # maximum number of messages created in interval - message: 30 + message: 180 # interval length for message limit message_per_second: 60 # maximum number of posts created in interval diff --git a/server/src/websocket/server.rs b/server/src/websocket/server.rs index a26c8144e..fc838c1fc 100644 --- a/server/src/websocket/server.rs +++ b/server/src/websocket/server.rs @@ -12,6 +12,7 @@ use serde_json::Value; use std::collections::{HashMap, HashSet}; use std::str::FromStr; use std::time::SystemTime; +use strum::IntoEnumIterator; use crate::api::comment::*; use crate::api::community::*; @@ -71,6 +72,13 @@ pub struct SessionInfo { pub ip: IPAddr, } +#[derive(Eq, PartialEq, Hash, Debug, EnumIter, Copy, Clone)] +pub enum RateLimitType { + Message, + Register, + Post, +} + /// `ChatServer` manages chat rooms and responsible for coordinating chat /// session. pub struct ChatServer { @@ -87,8 +95,8 @@ pub struct ChatServer { /// sessions (IE clients) user_rooms: HashMap>, - /// Rate limiting based on IP addr - rate_limits: HashMap, + /// Rate limiting based on rate type and IP addr + rate_limit_buckets: HashMap>, rng: ThreadRng, db: Pool>, @@ -98,7 +106,7 @@ impl ChatServer { pub fn startup(db: Pool>) -> ChatServer { ChatServer { sessions: HashMap::new(), - rate_limits: HashMap::new(), + rate_limit_buckets: HashMap::new(), post_rooms: HashMap::new(), community_rooms: HashMap::new(), user_rooms: HashMap::new(), @@ -259,60 +267,82 @@ impl ChatServer { to_json_string(&user_operation, post) } - fn check_rate_limit_register(&mut self, id: usize) -> Result<(), Error> { + fn check_rate_limit_register(&mut self, id: usize, check_only: bool) -> Result<(), Error> { self.check_rate_limit_full( + RateLimitType::Register, id, Settings::get().rate_limit.register, Settings::get().rate_limit.register_per_second, + check_only, ) } - fn check_rate_limit_post(&mut self, id: usize) -> Result<(), Error> { + fn check_rate_limit_post(&mut self, id: usize, check_only: bool) -> Result<(), Error> { self.check_rate_limit_full( + RateLimitType::Post, id, Settings::get().rate_limit.post, Settings::get().rate_limit.post_per_second, + check_only, ) } - fn check_rate_limit_message(&mut self, id: usize) -> Result<(), Error> { + fn check_rate_limit_message(&mut self, id: usize, check_only: bool) -> Result<(), Error> { self.check_rate_limit_full( + RateLimitType::Message, id, Settings::get().rate_limit.message, Settings::get().rate_limit.message_per_second, + check_only, ) } #[allow(clippy::float_cmp)] - fn check_rate_limit_full(&mut self, id: usize, rate: i32, per: i32) -> Result<(), Error> { + fn check_rate_limit_full( + &mut self, + type_: RateLimitType, + id: usize, + rate: i32, + per: i32, + check_only: bool, + ) -> Result<(), Error> { if let Some(info) = self.sessions.get(&id) { - if let Some(rate_limit) = self.rate_limits.get_mut(&info.ip) { - // The initial value - if rate_limit.allowance == -2f64 { - rate_limit.allowance = rate as f64; - }; + if let Some(bucket) = self.rate_limit_buckets.get_mut(&type_) { + if let Some(rate_limit) = bucket.get_mut(&info.ip) { + let current = SystemTime::now(); + let time_passed = current.duration_since(rate_limit.last_checked)?.as_secs() as f64; - let current = SystemTime::now(); - let time_passed = current.duration_since(rate_limit.last_checked)?.as_secs() as f64; - rate_limit.last_checked = current; - rate_limit.allowance += time_passed * (rate as f64 / per as f64); - if rate_limit.allowance > rate as f64 { - rate_limit.allowance = rate as f64; - } + // The initial value + if rate_limit.allowance == -2f64 { + rate_limit.allowance = rate as f64; + }; - if rate_limit.allowance < 1.0 { - println!( - "Rate limited IP: {}, time_passed: {}, allowance: {}", - &info.ip, time_passed, rate_limit.allowance - ); - Err( - APIError { - message: format!("Too many requests. {} per {} seconds", rate, per), + rate_limit.last_checked = current; + if !check_only { + rate_limit.allowance += time_passed * (rate as f64 / per as f64); + if rate_limit.allowance > rate as f64 { + rate_limit.allowance = rate as f64; } - .into(), - ) + } + + if rate_limit.allowance < 1.0 { + println!( + "Rate limited IP: {}, time_passed: {}, allowance: {}", + &info.ip, time_passed, rate_limit.allowance + ); + Err( + APIError { + message: format!("Too many requests. {} per {} seconds", rate, per), + } + .into(), + ) + } else { + if !check_only { + rate_limit.allowance -= 1.0; + } + Ok(()) + } } else { - rate_limit.allowance -= 1.0; Ok(()) } } else { @@ -350,14 +380,24 @@ impl Handler for ChatServer { }, ); - if self.rate_limits.get(&msg.ip).is_none() { - self.rate_limits.insert( - msg.ip, - RateLimitBucket { - last_checked: SystemTime::now(), - allowance: -2f64, - }, - ); + for rate_limit_type in RateLimitType::iter() { + if self.rate_limit_buckets.get(&rate_limit_type).is_none() { + self + .rate_limit_buckets + .insert(rate_limit_type, HashMap::new()); + } + + if let Some(bucket) = self.rate_limit_buckets.get_mut(&rate_limit_type) { + if bucket.get(&msg.ip).is_none() { + bucket.insert( + msg.ip.to_owned(), + RateLimitBucket { + last_checked: SystemTime::now(), + allowance: -2f64, + }, + ); + } + } } id @@ -446,11 +486,18 @@ fn parse_json_message(chat: &mut ChatServer, msg: StandardMessage) -> Result do_user_operation::(user_operation, data, &conn), UserOperation::Register => { - chat.check_rate_limit_register(msg.id)?; - do_user_operation::(user_operation, data, &conn) + chat.check_rate_limit_register(msg.id, true)?; + let register: Register = serde_json::from_str(data)?; + let res = Oper::new(register).perform(&conn)?; + chat.check_rate_limit_register(msg.id, false)?; + to_json_string(&user_operation, &res) } UserOperation::GetUserDetails => { do_user_operation::(user_operation, data, &conn) @@ -503,8 +550,11 @@ fn parse_json_message(chat: &mut ChatServer, msg: StandardMessage) -> Result(user_operation, data, &conn) } UserOperation::CreateCommunity => { - chat.check_rate_limit_register(msg.id)?; - do_user_operation::(user_operation, data, &conn) + chat.check_rate_limit_register(msg.id, true)?; + let create_community: CreateCommunity = serde_json::from_str(data)?; + let res = Oper::new(create_community).perform(&conn)?; + chat.check_rate_limit_register(msg.id, false)?; + to_json_string(&user_operation, &res) } UserOperation::EditCommunity => { let edit_community: EditCommunity = serde_json::from_str(data)?; @@ -566,14 +616,14 @@ fn parse_json_message(chat: &mut ChatServer, msg: StandardMessage) -> Result { - chat.check_rate_limit_post(msg.id)?; + chat.check_rate_limit_post(msg.id, true)?; let create_post: CreatePost = serde_json::from_str(data)?; let res = Oper::new(create_post).perform(&conn)?; + chat.check_rate_limit_post(msg.id, false)?; chat.post_sends(UserOperation::CreatePost, res, msg.id) } UserOperation::CreatePostLike => { - chat.check_rate_limit_message(msg.id)?; let create_post_like: CreatePostLike = serde_json::from_str(data)?; let res = Oper::new(create_post_like).perform(&conn)?; @@ -589,7 +639,6 @@ fn parse_json_message(chat: &mut ChatServer, msg: StandardMessage) -> Result(user_operation, data, &conn) } UserOperation::CreateComment => { - chat.check_rate_limit_message(msg.id)?; let create_comment: CreateComment = serde_json::from_str(data)?; let res = Oper::new(create_comment).perform(&conn)?; @@ -605,7 +654,6 @@ fn parse_json_message(chat: &mut ChatServer, msg: StandardMessage) -> Result(user_operation, data, &conn) } UserOperation::CreateCommentLike => { - chat.check_rate_limit_message(msg.id)?; let create_comment_like: CreateCommentLike = serde_json::from_str(data)?; let res = Oper::new(create_comment_like).perform(&conn)?; @@ -649,7 +697,6 @@ fn parse_json_message(chat: &mut ChatServer, msg: StandardMessage) -> Result(user_operation, data, &conn) } UserOperation::CreatePrivateMessage => { - chat.check_rate_limit_message(msg.id)?; let create_private_message: CreatePrivateMessage = serde_json::from_str(data)?; let recipient_id = create_private_message.recipient_id; let res = Oper::new(create_private_message).perform(&conn)?;