Closed Bug 935570 Opened 11 years ago Closed 11 years ago

create an anti-spam extension

Categories

(bugzilla.mozilla.org :: Extensions, defect)

Production
x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: glob, Assigned: glob)

References

Details

Attachments

(1 file, 3 obsolete files)

goals - provide information to administrators to enable them to make better decisions on if a user is a spammer and should be blocked - gather data on accounts in the database to enable measurement of spam frequency and patterns - improvement management of blocked domains non-goals - automatic detection and blocking of accounts and/or comments details - log ip address that created the account - perform a projecthoneypot lookup and log response - http://www.projecthoneypot.org/httpbl_api.php - add a 'disabled reason' to allow for categorisation/reporting - augment user editing page - show ip, rdns, and honeypot details - add 'disabled reason' select - prefill disable text with canned responses when 'disabled reason' selected - check 'disable bugmail' by default when an account is disabled - add "this account has been disabled" to the user profile page, but don't show the reason/details schema - antispam_profiles - id :: primary key auto inc - user_id :: fk => profiles.userid - ip_address VARCHAR(15) :: remote_ip() - honeypot_threat TINYINT :: threat score (max 255) - honeypot_age TINYINT :: days since last activity - honeypot_type TINYINT :: visitor type bitmask - 0 : search engine - 1 : suspicious - 2 : harvester - 3 : comment spammer - disabled_reason TINYINT - 0 : spammer - 1 : abusive comments - 2 : invalid bugs/comments - 3 : owner requested - 4 : api/query abuse - 99 : other - antispam_domain_blocklist - id :: primary key auto inc - domain VARCHAR(255) :: domain name parameters - honeypot_api_key if i have the time, i'll make the reasons and canned responses a separate table, with an admin interface to manipulate. if not, canned responses will be constants in a data package.
Attached patch 935570_1.patch (obsolete) (deleted) — Splinter Review
in light of recent events, here's a version which just does honeypot checking, and adds the ability to block account creation from IP addresses over a specified threat level.
Attachment #828525 - Flags: review?(justdave)
Comment on attachment 828525 [details] [diff] [review] 935570_1.patch per IRC you tested this by manually setting $ip in the code with different IP addresses. I'll trust you on that assuming it's been done on bugzilla-dev so we know it's similar to production. I don't see any issues with the code on a manual review.
Attachment #828525 - Flags: review?(justdave) → review+
justdave mentioned on IRC that it probably makes sense to split this into its own ProjectHoneyPot extension, so that's what i've done. Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bmo/4.2/ modified Bugzilla/User.pm added extensions/ProjectHoneyPot added extensions/ProjectHoneyPot/Config.pm added extensions/ProjectHoneyPot/Extension.pm added extensions/ProjectHoneyPot/template added extensions/ProjectHoneyPot/template/en added extensions/ProjectHoneyPot/template/en/default added extensions/ProjectHoneyPot/template/en/default/hook added extensions/ProjectHoneyPot/template/en/default/hook/admin added extensions/ProjectHoneyPot/template/en/default/hook/admin/params added extensions/ProjectHoneyPot/template/en/default/hook/admin/params/editparams-current_panel.html.tmpl Committed revision 9134. i'll leave this bug open to work on the other things.
Attached patch 935570_3.patch (obsolete) (deleted) — Splinter Review
here's the first revision of this. i've deferred the functionality for capturing the reason for profile disabling due to time constraints; will file another bug for that once this one's done. this patch does the following: - removes the ProjectHoneyPot ext and folds it into an AntiSpam ext - adds domain name blocking at account creation time, using a table for values - adds remote ip blocking at account creation time, using a table for values - adds a comment word blocking list, again using a table - adds a generic EditTable extension (currently quick-and-dirty but functional) notes: - domain name blocking requires a full name match, regexs and partial matches are not supported - comment word blocking requires a full word match, regex and partial matches are not supported. this is implemented as \b$word\b. this is to avoid false positive hits, and should be used sparingly - while it's possible for IT to complete block an IPs ability to hit bugzilla with zeus, bringing that functionality into bmo gives us the ability to just block account creation, and provides a quicker turn-around - i'm not sure about the wording of the error you get when a comment is blocked ("Your comment contains words not accepted by Bugzilla")
Attachment #828525 - Attachment is obsolete: true
Attachment #832757 - Flags: review?(dkl)
(In reply to Byron Jones ‹:glob› from comment #4) > - i'm not sure about the wording of the error you get when a comment is > blocked ("Your comment contains words not accepted by Bugzilla") You comment contains one or more words deemed inappropriate for use by the administrators of this site.
Comment on attachment 832757 [details] [diff] [review] 935570_3.patch Review of attachment 832757 [details] [diff] [review]: ----------------------------------------------------------------- Overall an awesome start to building out a more robust tool for blocking this type of abuse. ::: extensions/AntiSpam/Extension.pm @@ +97,5 @@ > +# > + > +sub _ip_blocking { > + my ($self, $login) = @_; > + my $ip = remote_ip(); trick_taint($ip); @@ +99,5 @@ > +sub _ip_blocking { > + my ($self, $login) = @_; > + my $ip = remote_ip(); > + my $blocked = Bugzilla->dbh->selectrow_array( > + "SELECT 1 FROM antispam_ip_blocklist WHERE ip=?", s/ip=/ip_address=/ @@ +116,5 @@ > + > +sub object_end_of_create_validators { > + my ($self, $args) = @_; > + if ($args->{class} eq 'Bugzilla::Comment') { > + $self->_comment_blocking($args->{params}); Maybe skip this altogether if the comment is private or the user has editbugs or better. ::: extensions/AntiSpam/template/en/default/hook/global/user-error-errors.html.tmpl @@ +7,5 @@ > + #%] > + > +[% IF error == "antispam_comment_blocked" %] > + [% title = "Comment Blocked" %] > + Your comment contains words not accepted by [% terms.Bugzilla %]. Your comment contains one or more words deemed inappropriate for use by the administrators of this site. ::: extensions/EditTable/Config.pm @@ +9,5 @@ > +use strict; > + > +use constant NAME => 'EditTable'; > +use constant REQUIRED_MODULES => []; > +use constant OPTIONAL_MODULES => [ ]; nit: remove extra space ::: extensions/EditTable/Extension.pm @@ +98,5 @@ > + "INSERT INTO $table_name(" . join(',', @fields) . ") " . > + "VALUES ($placeholders)", > + undef, > + @$row > + ); Need to check for empty values before entering as well as duplicates which I am sure you are aware. We could just leave the dupe checking to MySQL but generates a pretty nasty error. @@ +117,5 @@ > + "SET " . join(',', map { "$_ = ?" } @fields) . " " . > + "WHERE $id_field = ?", > + undef, > + @$row, $id > + ); Same empty/duplicate checking needed. ::: extensions/EditTable/template/en/default/pages/edit_table.html.tmpl @@ +9,5 @@ > +[% PROCESS global/variables.none.tmpl %] > + > +[% PROCESS global/header.html.tmpl > + title = "Edit Table" > + yui = [ 'datatable' ] Not needed.
Attachment #832757 - Flags: review?(dkl) → review-
(In reply to David Lawrence [:dkl] from comment #6) > Need to check for empty values before entering as well as duplicates which I > am sure you are aware. We could just leave the dupe checking to MySQL but > generates a pretty nasty error. empty values can be valid; however i'll skip new rows which contain nothing but empty values. adding full data validation is something that will have to happen later (probably by the creating a javascript validator for each table which is called to validate each row prior to committing). for now i'll pretty up the mysql errors by removing unnecessary guff.
Attached patch 935570_4.patch (obsolete) (deleted) — Splinter Review
> Maybe skip this altogether if the comment is private or the user has > editbugs or better. i don't think private comments should be exempt, as they are still a source of annoyance. missing the editbugs exemption was indeed an oversight, thanks for catching that. as the goal here isn't censorship, i've also added an exemption for instances where the reporter is commenting on their own bug. can remove if you think this isn't warranted.
Attachment #832757 - Attachment is obsolete: true
Attachment #8333729 - Flags: review?(dkl)
Comment on attachment 8333729 [details] [diff] [review] 935570_4.patch Review of attachment 8333729 [details] [diff] [review]: ----------------------------------------------------------------- Everything else was fine except for new bug creation so should be quick r+ once that is solved. ::: extensions/AntiSpam/Extension.pm @@ +72,5 @@ > + return if $user->in_group('editbugs'); > + my $bug = ref($params->{bug_id}) > + ? $params->{bug_id} > + : Bugzilla::Bug->new($params->{bug_id}); > + return if $bug->reporter->id == $user->id; When creating a new bug as unprivileged user, the following errors are thrown: Use of uninitialized value $param in pattern match (m//) at Bugzilla/Bug.pm line 322. at Bugzilla/Bug.pm line 322. \tBugzilla::Bug::new(...) called at ./extensions/AntiSpam/Extension.pm line 73 \tBugzilla::Extension::AntiSpam::_comment_blocking(...) called at ./extensions/AntiSpam/Extension.pm line 133 \tBugzilla::Extension::AntiSpam::object_end_of_create_validators(...) called at Bugzilla/Hook.pm line 33 \tBugzilla::Hook::process(...) called at Bugzilla/Object.pm line 624 \tBugzilla::Object::run_create_validators(...) called at Bugzilla/Comment.pm line 192 \tBugzilla::Comment::run_create_validators(...) called at Bugzilla/Bug.pm line 1539 \tBugzilla::Bug::_check_comment(...) called at Bugzilla/Object.pm line 611 \tBugzilla::Object::run_create_validators(...) called at Bugzilla/Bug.pm line 751 \tBugzilla::Bug::run_create_validators(...) called at Bugzilla/Bug.pm line 651 \tBugzilla::Bug::create(...) called at /home/dkl/devel/htdocs/935570/post_bug.cgi line 155 Can't call method "id" on an undefined value at ./extensions/AntiSpam/Extension.pm line 79. at ./extensions/AntiSpam/Extension.pm line 79. \tBugzilla::Extension::AntiSpam::_comment_blocking(...) called at ./extensions/AntiSpam/Extension.pm line 136 \tBugzilla::Extension::AntiSpam::object_end_of_create_validators(...) called at Bugzilla/Hook.pm line 33 \tBugzilla::Hook::process(...) called at Bugzilla/Object.pm line 623 \tBugzilla::Object::run_create_validators(...) called at Bugzilla/Comment.pm line 191 \tBugzilla::Comment::run_create_validators(...) called at Bugzilla/Bug.pm line 1538 \tBugzilla::Bug::_check_comment(...) called at Bugzilla/Object.pm line 610 \tBugzilla::Object::run_create_validators(...) called at Bugzilla/Bug.pm line 751 \tBugzilla::Bug::run_create_validators(...) called at Bugzilla/Bug.pm line 651 \tBugzilla::Bug::create(...) called at /home/dkl/devel/htdocs/935570/post_bug.cgi line 155 Can't call method "id" on an undefined value at ./extensions/AntiSpam/Extension.pm line 76 Both $params->{bug_id} and $bug->{reporter_id} are not yet defined when creating new bugs. @@ +109,5 @@ > +# > + > +sub _ip_blocking { > + my ($self, $login) = @_; > + my $ip = remote_ip(); trick_taint($ip);
Attachment #8333729 - Flags: review?(dkl) → review-
Attached patch 935570_5.patch (deleted) — Splinter Review
Attachment #8333729 - Attachment is obsolete: true
Attachment #8334384 - Flags: review?(dkl)
Comment on attachment 8334384 [details] [diff] [review] 935570_5.patch r=dkl
Comment on attachment 8334384 [details] [diff] [review] 935570_5.patch this time i will update the flag too :)
Attachment #8334384 - Flags: review?(dkl) → review+
schema only: Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bmo/4.2/ added extensions/AntiSpam added extensions/AntiSpam/Config.pm added extensions/AntiSpam/Extension.pm Committed revision 9153.
rest: Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bmo/4.2/ added extensions/EditTable deleted extensions/ProjectHoneyPot modified extensions/AntiSpam/Extension.pm added extensions/AntiSpam/template added extensions/AntiSpam/template/en added extensions/AntiSpam/template/en/default added extensions/AntiSpam/template/en/default/hook added extensions/AntiSpam/template/en/default/hook/admin added extensions/AntiSpam/template/en/default/hook/global added extensions/AntiSpam/template/en/default/hook/admin/admin-end_links_right.html.tmpl added extensions/AntiSpam/template/en/default/hook/admin/params added extensions/AntiSpam/template/en/default/hook/admin/params/editparams-current_panel.html.tmpl added extensions/AntiSpam/template/en/default/hook/global/user-error-errors.html.tmpl added extensions/EditTable/Config.pm added extensions/EditTable/Extension.pm added extensions/EditTable/template added extensions/EditTable/web added extensions/EditTable/template/en added extensions/EditTable/template/en/default added extensions/EditTable/template/en/default/hook added extensions/EditTable/template/en/default/pages added extensions/EditTable/template/en/default/hook/global added extensions/EditTable/template/en/default/hook/global/user-error-auth_failure_object.html.tmpl added extensions/EditTable/template/en/default/hook/global/user-error-errors.html.tmpl added extensions/EditTable/template/en/default/pages/edit_table.html.tmpl added extensions/EditTable/web/js added extensions/EditTable/web/styles added extensions/EditTable/web/js/edit_table.js added extensions/EditTable/web/styles/edit_table.css deleted extensions/ProjectHoneyPot/Config.pm deleted extensions/ProjectHoneyPot/Extension.pm deleted extensions/ProjectHoneyPot/template deleted extensions/ProjectHoneyPot/template/en deleted extensions/ProjectHoneyPot/template/en/default deleted extensions/ProjectHoneyPot/template/en/default/hook deleted extensions/ProjectHoneyPot/template/en/default/hook/admin deleted extensions/ProjectHoneyPot/template/en/default/hook/admin/params deleted extensions/ProjectHoneyPot/template/en/default/hook/admin/params/editparams-current_panel.html.tmpl Committed revision 9155. leaving open to complete domain blocking configuration once pushed to production.
i've converted the account creation domain blocking list from the regex to the table, and reset createemailregexp back to .*
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Component: Extensions: Other → Extensions: AntiSpam
Component: Extensions: AntiSpam → Extensions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: