Tolerate clock skew into the future for various dates about browserid-crypto HOT 7 CLOSED

I'm going to create a branch of v0.4.2.

There is one non-trivial change in master... which I don't think we want to QA. Or should we add that to the hotpatch?

from browserid-crypto.

There is one non-trivial change in master... which I don't think we want to QA. Or should we add that to the hotpatch?

Oh, actually, that change is in bin/check-assertion which I don't think is used in production.

from browserid-crypto.

This bug is probably invalid. The APIs take a date as input. Clock skew can be adjusted for by the caller.

Our issue is can be fixed by setting new Date() to 10 seconds into the future, when we call jwcrypto.cert.verifyBundle right here.

CCing @lloyd and @warner

Closing, will open bug in core.

from browserid-crypto.

via IRC

warner: so if you pass in now+5min, it'll assert that .iat<now+5min and .exp>now+5min, which lets you tolerate a fast IDP clock for .iat, but makes it worse for .exp (the IDP clock's slowness plus the RTT from IDP to verifier must be >5min)

from browserid-crypto.

I don't see in browserid or jwcrypo where we pad for any clock skew... actually. In IRC I heard there was an allowance backed in, but I'm guessing this is using cert expiration as the skew allowance.

In test/assertion-test.js we actually test that an assertion with an expiration of 1 second ago fails to verify.

I'll add 120 seconds of skew allowance in the past, as discussed on IRC.

from browserid-crypto.

PR #59

@warner would you be available to review?

from browserid-crypto.

See also mozilla/persona#2814 where this was encountered by external IdPs (my copy being https://github.com/mook/browseridp/blob/acec78ccb6/content/interceptor.js#L148 )
(Sorry for spamming jwcrypto, nobody actually mentioned the browserid issue number)

from browserid-crypto.