653103 - Scam detection generates too many false positives to be useful and should be easier to turn off as interim fix

Status: RESOLVED FIXED

(Thunderbird :: Security, enhancement)

Description

[rsx11m]

Spin-off per bug 623198 comment #49, switching the default and providing an easier way to turn the feature off until it is completed are two different issues. For details see the discussion in bug 623198 and bug 320351.

Comment 1

[rsx11m]

Attachment: Merged link version (has l10n impact)

This is bug 623198 attachment 528503 [details] [diff] [review].

Carrying forward ui-review+ from attachment 527159 [details] [diff] [review] of that bug.
See attachment 527160 [details] for a screenshot (slightly different label).

Comment 2

[Blake Winton (:bwinton) (:☕️)]

(Just for ease of checkin-comment, it was a ui-r=bwinton that rsx11m is carrying forward.)

Comment 3

[rsx11m]

I'm preemptively nominating this as a blocker for 3.3 so that it stays on the radar towards the approaching string freeze. It's a highly visible issue and a working patch is up for review.

Comment 4

[rsx11m]

I don't know if this needs a test to be acceptable, but also to facilitate an easier review, here the steps to verify that the new link works as intended:

1. Get a message triggering the scam warning, e.g., from
> http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/message-header/evil.eml?raw=1
2. open that message, the scam notification appears
3. click on the "Disable scam detection" link, which should turn gray
4. go to a different message and come back to the scammy one
5. the scam notification should be collapsed now
6. go into the Security > E-mail Scams preferences
7. check the (currently unchecked) box again and close dialog
8. go to a different message and come back to the scammy one
9. the scam notification should be visible again, the link is blue

I won't be able to come up with a Mozmill test (and this would probably depend on bug 652942 being fixed first anyway), but that sequence should be sufficient for a litmus test until the scam feature has been improved.

Comment 5

[Mark Banner (:standard8)]

I don't think we'd block on this, but I'm reviewing the patch now.

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 528581 [details] [diff] [review]
Merged link version (has l10n impact)

I know Blake has given this ui-r+, but why not just call ReloadMessage() after setting that preference to false? Then the user would have slightly better visible feedback that something has happened, than just having the bar stay there.

Comment 7

[rsx11m]

The idea was to leave the dialog open, giving the user a chance to still use the "Ignore Warning" button. I'm fine with it either way.

Comment 8

[rsx11m]

Attachment: Link hiding the scam notification

This is an alternative patch based on comment #6 which will now let the scam notification just disappear when the link is pushed. No changes anywhere else.

I'd read Blake's bug 623198 comment #38 as this being in general covered by his ui-review+ approval, otherwise I assume that he will complain asap... ;-)

Step #3 in the steps for testing (comment #4) will have to be changed if this version is used (replace "turn gray" with "disappear).

Comment 9

[Blake Winton (:bwinton) (:☕️)]

(In reply to comment #8)
> Created attachment 529983 [details] [diff] [review] [review]
> Link hiding the scam notification
> 
> This is an alternative patch based on comment #6 which will now let the scam
> notification just disappear when the link is pushed. No changes anywhere else.

Yeah, I think that's probably the right behaviour.  If the user has said "Don't show me these ever again", I think we can assume they're not that interested in seeing the current one either.

Thanks,
Blake.

Comment 10

[rsx11m]

Comment on attachment 528581 [details] [diff] [review]
Merged link version (has l10n impact)

Ok, so let's go with the other one.

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 529983 [details] [diff] [review]
Link hiding the scam notification

> /**
>  *  Set the msg hdr flag to ignore the phishing warning and reload the message.
>+ *  - allow disabling this feature for all messages until lists are in place.
>  */
> function IgnorePhishingWarning()
> {
>   // This property should really be called skipPhishingWarning or something
>   // like that, but it's too late to change that now.
>   // This property is used to supress the phishing bar for the message.
>   setMsgHdrPropertyAndReload("notAPhishMessage", 1);
> }
> 
>+function DisablePhishingWarning()
>+{
>+  Application.prefs.setValue("mail.phishing.detection.enabled", false);
>+  ReloadMessage();
>+}
>+

The addition of the comment seems out of place. I think it'd be better to put the comment on the DisablePhishingWarning function.

r=me with that fixed.

Comment 12

[rsx11m]

Attachment: Final patch

Patch for check-in (ui-r=bwinton, r=standard8).

Comment 13

[rsx11m]

Revised litmus test for Ludo:

1. Get a message triggering the scam warning, e.g., from
http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/message-header/evil.eml?raw=1
2. open that message, the scam notification appears
3. click on the "Disable scam detection" link, which should disappear
4. go to a different message and come back to the scammy one
5. the scam notification should be collapsed now
6. go into the Security > E-mail Scams preferences
7. check the (currently unchecked) box again and close dialog
8. go to a different message and come back to the scammy one
9. the scam notification should be visible again, along with the link

Comment 14

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/b3a51479167e

Comment 15

[rsx11m]

Thanks Mark. Step #3 of the revised litmus test is unfortunately now completely ambiguous in comment #13, thus here yet another revision of the test sequence:

1. Get a message triggering the scam warning, e.g., from
http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/message-header/evil.eml?raw=1
2. open that message, the scam notification appears
3. click on the "Disable scam detection" link, notification bar disappears
4. go to a different message and come back to the scammy one
5. the scam notification should be collapsed now
6. go into the Security > E-mail Scams preferences
7. check the (currently unchecked) box again and close dialog
8. go to a different message and come back to the scammy one
9. the scam notification should be visible again, along with the link

Comment 17

[Ludovic Hirlimann [:Usul]]

https://litmus.mozilla.org/show_test.cgi?id=40442