credential-handler / authn.io

Credential Mediator Polyfill
https://github.com/w3c-ccg/credential-handler-api
Other
44 stars 8 forks source link

Prevent proof from being submitted early. #36

Closed mattcollier closed 8 years ago

mattcollier commented 8 years ago

@dlongley I spent the weekend developing at protractor test suite for bedrock-idp that allows us to hammer on the join process, including the DID registration in authio. I started noticing a failure in about 15% of cases where the DID registration would fail.

I eventually discovered that the failure was caused by the front end submitting the proof of patience token too soon. The typical discrepancy that I observed prior to the patch was ~200ms.

This is a very perplexing problem, and I was very surprised that implementing setInterval alone did not solve the problem. With setInterval alone, I saw one case where the discrepancy was 61ms.

Here is a gist that contains the root cause of the issue and a link to the code that throws the error: https://gist.github.com/mattcollier/3daf31a0ca942b20daf36c5c40455dd0

dlongley commented 8 years ago

Hmm, maybe we should make the server (the verifier side) more lenient instead. Otherwise, other clients will have similar issues.

mattcollier commented 8 years ago

@dlongley What approach would you support wrt changing the server behavior? Currently, the token validation, including the 'not before' check is handled by the jwt-simple module.

https://github.com/digitalbazaar/authorization.io/blob/master/lib/proofOfPatience.js#L133

If we want to leave all that the way it is, we could adjust the not before time on the token. The current implementation has a 1 second resolution, so if we want to make sub-second adjustments, the change would be more significant.

dlongley commented 8 years ago

Well, my understanding is that we can't address the problem by adjusting the not before time on the token, because the fact that the meaning of that field is different on the client vs. the server is the source of the trouble, i.e. the client would just send the token earlier and have the same problem.

What we need is a more relaxed check on that field against the server's clock to account for client clock skew. That means we can't use jwt-simple to do the check. Maybe there's another library that lets us pass in a time window option or something... or we'll just have to write something ourselves or find a way to ignore just that check and do it manually.

mattcollier commented 8 years ago

The jwt-simple decode API does allow for disabling the 'verify' part, and we can implement a more relaxed time check in our code.

dlongley commented 8 years ago

I think it disables the entire verification check, not just the time bit. Which means we have to reimplement everything else. :(