Pass timestamp along with the generated filter cascade

[Rob--W]

The blocklist should include a timestamp, so clients can determine whether the filter cascade can be relied upon for testing the blocklist state.

This timestamp should be synchronized with clock of the signing server, so that we do not inadvertently block add-ons that shouldn't be blocked, or allow add-ons that should have been blocked.

[eviljeff]

in https://github.com/mozilla/addons-server/pull/13699 I've added the last_sign_time property, though currently just the dumb current time on AMO. The value is the timestamp in milliseconds (i.e. milliseconds since the unix epoch), like kinto uses for it's modified record property. Will that work as a value for your implementation @Rob--W ?

I'll replace the dumb current time with the last signing time from autograph to close this issue.

[Rob--W]

Thanks, that will do for now. It will help with testing the client implementation.

I do suggest to add a # TODO with a reference to this issue.

The implementation should be corrected when we ship though. Otherwise the following can happen:

• When AMO's clock is behind Autograph's clock by X seconds, an add-on's signature may be later than last_sign_time (when the add-on is signed X seconds before the blocklist generation). Such add-ons cannot reliably be blocked.
• When AMO's clock is ahead of the Autograph's clock by X seconds, then add-ons that were signed X seconds after the generation of the blocklist have a chance of being blocked while they should not be.

Ideally X should be zero (i.e. clocks of Autograph and AMO are perfectly in sync).

[eviljeff]

so as far I can see there are two routes we can do down here: A) accurately keep track of the signing timestamps per certificate by extracting them after being returned from autograph and storing them in a field in files table (File model) B) just return current time - 1 minute* to account for possible differences in system datetime between autograph and AMO instances plus the lag between the certificate being signed by autograph and the signed xpi being received and processed by AMO.

Given Blocks need to manually created (and possibly then signed off) it's very unlikely any addon/version/file would be blocked within a minute* of it being signed, so I'd recommend B.

*even 5 minutes.

@muffinresearch @diox @jvillalobos

[jvehent]

So

• Clocks should be in near perfect sync (to the point where deltas are irrelevant).
• Certificates do not reflect the signing time. Their validity typically span over 90 days.

But why does that matter? I'm failing to make the connection between the signing dates of the blocklist and the add-ons being signed. Do we expect to put the ID+version of every single add-on ever issued in this blocklist? I thought it would only contain blocked add-ons.

[Rob--W]

We use a cascade bloom filter as the data structure to hold the blocklist. A normal bloom filter is like a set where you can query whether an item is definitely in the set. It has false-negatives though, i.e. the answer can be "yes" without the item being in the set. This is undesired (but with a low probability), and solved by creating a new set to store those known false negatives. And so on, until all known positives are "yes" and all known negatives are "no".

The cascade bloom filter can thus only be relied upon to query about items (i.e. addons) that are known at the time of that the bloom filters were comstructed. Unknown add-ons may be a false positive, and be blocked when they should not be, if we don't account for the clock to synchronize.

[jvehent]

IIRC, the probability of a false-{positive,negative} impacted an add-on signed during the clock delta would be incredibly small. Is it worth worrying about it? Those servers will be in NTP sync, so there should never be more than a few milliseconds from each other.

[jvillalobos]

I agree it sounds like we're splitting hairs here. But I like option B, to mitigate the possibility of false positives.

[bqbn]

In AMO case, we run ntpd across all the deployed instances and they sync from the same server pool (AWS ntp server pool).

I believe Autograph instances do the same, but I'm cc'ing @erkolson to confirm.

[AlexandraMoga]

@eviljeff is this new generation_time key exposed in https://kinto.dev.mozaws.net/v1/buckets/staging_addons_dev_allizom_org/collections/addons-mlbf/records or somewhere else? I'm also not sure how I could practically verify what it was required here.

[eviljeff]

my fix for https://github.com/mozilla/addons/issues/7384 had a bug in it.

[eviljeff]

@eviljeff is this new generation_time key exposed in https://kinto.dev.mozaws.net/v1/buckets/staging_addons_dev_allizom_org/collections/addons-mlbf/records or somewhere else?

yes, it should be there in the record(s) the next time the cron job runs.

[AlexandraMoga]

The generation_time key was present yesterday when I've checked the records.

I'll look again after the records are reset.