935570 - create an anti-spam extension

Status: RESOLVED FIXED

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

Description

[:glob ✱]

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.

Comment 1

[:glob ✱]

Attachment: 935570_1.patch

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.

Comment 2

[Dave Miller [: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.

Comment 3

[:glob ✱]

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://[e-mail]/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.

Comment 4

[:glob ✱]

Attachment: 935570_3.patch

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")

Comment 5

[David Lawrence [: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 6

[David Lawrence [:dkl]]

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.

Comment 7

[:glob ✱]

(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.

Comment 8

[:glob ✱]

Attachment: 935570_4.patch

> 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.

Comment 9

[David Lawrence [: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);

Comment 10

[:glob ✱]

Attachment: 935570_5.patch

Comment 11

[David Lawrence [:dkl]]

Comment on attachment 8334384 [details] [diff] [review]
935570_5.patch

r=dkl

Comment 12

[David Lawrence [:dkl]]

Comment on attachment 8334384 [details] [diff] [review]
935570_5.patch

this time i will update the flag too :)

Comment 13

[:glob ✱]

schema only:

Committing to: bzr+ssh://[e-mail]/bmo/4.2/
added extensions/AntiSpam
added extensions/AntiSpam/Config.pm
added extensions/AntiSpam/Extension.pm
Committed revision 9153.

Comment 14

[:glob ✱]

rest:

Committing to: bzr+ssh://[e-mail]/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.

Comment 15

[:glob ✱]

i've converted the account creation domain blocking list from the regex to the table, and reset createemailregexp back to .*