Closed Bug 1482306 Opened Last year Closed Last year

Race condition when multiple tables are using the same underlying lists

Categories

(Toolkit :: Safe Browsing, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: francois, Assigned: francois)

References

Details

Attachments

(1 file)

I noticed that if I disable TP but leave annotations enabled (for example, by turning off content blocking in bug 1480450) I get the following:

SafeBrowsing: 16:37:46 GMT-0700 (PDT): phishingEnabled: true malwareEnabled: true downloadsEnabled: true passwordsEnabled: false trackingEnabled: false blockedEnabled: true trackingAnnotations true flashBlockEnabled true flashInfobarListEnabled: false
...
listmanager: 16:37:46 GMT-0700 (PDT): Enabling table updates for base-track-digest256
listmanager: 16:37:46 GMT-0700 (PDT): Enabling table updates for mozstd-trackwhite-digest256
listmanager: 16:37:46 GMT-0700 (PDT): Disabling table updates for base-track-digest256
listmanager: 16:37:46 GMT-0700 (PDT): Disabling table updates for mozstd-trackwhite-digest256
...
listmanager: 16:37:46 GMT-0700 (PDT): needsUpdate: {
...
  "https://shavar.services.mozilla.com/downloads?client=Firefox&appver=63.0a&pver=2.2": {
    "base-track-digest256": false,
    "mozstd-trackwhite-digest256": false,
...
  }
}

In other words, there's a race condition. If the disabling feature is first, the second one will re-enable the lists, but if the disabling feature goes second, then the lists will be flagged as disabled.

While tableList makes it seem like it still requests the tables:

listmanager: 16:38:05 GMT-0700 (PDT): forceUpdates with base-track-digest256,mozstd-trackwhite-digest256,content-track-digest256,mozplugin-block-digest256,mozplugin2-block-digest256,block-flash-digest256,except-flash-digest256,allow-flashallow-digest256,except-flashallow-digest256,block-flashsubdoc-digest256,except-flashsubdoc-digest256,except-flashinfobar-digest256,ads-track-digest256,social-track-digest256,analytics-track-digest256
listmanager: 16:38:05 GMT-0700 (PDT): checkForUpdates with https://shavar.services.mozilla.com/downloads?client=Firefox&appver=63.0a&pver=2.2
listmanager: 16:38:05 GMT-0700 (PDT): this.tablesData: {
...
  "base-track-digest256": {
    "updateUrl": "https://shavar.services.mozilla.com/downloads?client=Firefox&appver=63.0a&pver=2.2",
    "gethashUrl": "https://shavar.services.mozilla.com/gethash?client=Firefox&appver=63.0a&pver=2.2",
    "provider": "mozilla"
  },
  "mozstd-trackwhite-digest256": {
    "updateUrl": "https://shavar.services.mozilla.com/downloads?client=Firefox&appver=63.0a&pver=2.2",
    "gethashUrl": "https://shavar.services.mozilla.com/gethash?client=Firefox&appver=63.0a&pver=2.2",
    "provider": "mozilla"
  },
...
}
...
listmanager: 16:38:05 GMT-0700 (PDT): update request: {
  "tableList": "base-track-digest256,mozstd-trackwhite-digest256,mozplugin-block-digest256,allow-flashallow-digest256,except-flashallow-digest256,block-flash-digest256,except-flash-digest256,block-flashsubdoc-digest256,except-flashsubdoc-digest256,except-flashinfobar-digest256",
  "tableNames": {},
  "requestPayload": "except-flashallow-digest256;a:1490633678\nmozplugin-block-digest256;a:1471849627\nblock-flashsubdoc-digest256;a:1512160865\nexcept-flash-digest256;a:1494877265\nexcept-flashsubdoc-digest256;a:1517935265\nallow-flashallow-digest256;a:1490633678\nblock-flash-digest256;a:1496263270\n",
  "isPostRequest": true
}

The request payload doesn't include them:

listmanager: 16:38:05 GMT-0700 (PDT): makeUpdateRequestForEntry_: requestPayload except-flashallow-digest256;a:1490633678
mozplugin-block-digest256;a:1471849627
block-flashsubdoc-digest256;a:1512160865
except-flash-digest256;a:1494877265
except-flashsubdoc-digest256;a:1517935265
allow-flashallow-digest256;a:1490633678
block-flash-digest256;a:1496263270
 update: https://shavar.services.mozilla.com/downloads?client=Firefox&appver=63.0a&pver=2.2 tablelist: base-track-digest256,mozstd-trackwhite-digest256,mozplugin-block-digest256,allow-flashallow-digest256,except-flashallow-digest256,block-flash-digest256,except-flash-digest256,block-flashsubdoc-digest256,except-flashsubdoc-digest256,except-flashinfobar-digest256

listmanager: 16:38:05 GMT-0700 (PDT): update success for base-track-digest256,mozstd-trackwhite-digest256,mozplugin-block-digest256,allow-flashallow-digest256,except-flashallow-digest256,block-flash-digest256,except-flash-digest256,block-flashsubdoc-digest256,except-flashsubdoc-digest256,except-flashinfobar-digest256 from https://shavar.services.mozilla.com/downloads?client=Firefox&appver=63.0a&pver=2.2: 3600

and if I delete the cached files in obj-x86_64-pc-linux-gnu/tmp/profile-default/safebrowsing/ and re-trigger an update of the mozilla provider, the TP lists are not downloaded:

$ ls obj-x86_64-pc-linux-gnu/tmp/profile-default/safebrowsing/
allow-flashallow-digest256.pset      except-flashallow-digest256.pset      google4                            test-harmful-simple.sbstore  test-track-simple.sbstore
allow-flashallow-digest256.sbstore   except-flashallow-digest256.sbstore   mozplugin-block-digest256.pset     test-malware-simple.pset     test-trackwhite-simple.pset
block-flash-digest256.pset           except-flash-digest256.pset           mozplugin-block-digest256.sbstore  test-malware-simple.sbstore  test-trackwhite-simple.sbstore
block-flash-digest256.sbstore        except-flash-digest256.sbstore        test-block-simple.pset             test-phish-simple.pset       test-unwanted-simple.pset
block-flashsubdoc-digest256.pset     except-flashsubdoc-digest256.pset     test-block-simple.sbstore          test-phish-simple.sbstore    test-unwanted-simple.sbstore
block-flashsubdoc-digest256.sbstore  except-flashsubdoc-digest256.sbstore  test-harmful-simple.pset           test-track-simple.pset
See Also: → 1480450
The enable/disable logic of the list manager was wrong. If multiple features
shared a given table (e.g. tracking protection and tracking annotations)
and only one of them was enabled, then updates could either be disabled
or enabled depending on which feature was checked first.

By disabling everything at the beginning and selectively re-enabling tables,
we can ensure that no table gets both enabled and disabled by different
feature toggles.
Comment on attachment 8999741 [details]
Bug 1482306 - Ensure that tables are enabled when shared between features. r=dimi!

Dimi Lee[:dimi][:dlee] has approved the revision.
Attachment #8999741 - Flags: review+
Pushed by fmarier@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6560b92dbe64
Ensure that tables are enabled when shared between features. r=dimi!
https://hg.mozilla.org/mozilla-central/rev/6560b92dbe64
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.