Incorrect nonce expiry calculation

[nualoy]

Am I confused or the nonce expiry calculation in the authentication provider is incorrect?

Current:

if (nonce_timestamp + lifetime > current_timestamp) then throw NonceExpiredException

Note that the bigger the lifetime, the more chances to get the exception, which doesn't make sense.

Expected:

if (current_timestamp - nonce_timestamp > lifetime) then throw NonceExpiredException

[djoos]

Hi @nuqqsa,

would you be able to point me to the place in the code where you've spotted this? If I'm not mistaking the expected behaviour you've posted above is what can be found on line 63 of https://github.com/escapestudios/EscapeWSSEAuthenticationBundle/blob/master/Security/Core/Authentication/Provider/Provider.php

Thanks in advance for your feedback!

Kind regards, David

[djoos]

Ok, just realized I was looking in the wrong place :-) You were talking about line 80, right? You're absolutely right: that is incorrect - I'll try and get this sorted ASAP.

Kind regards, David

[nualoy]

Hi @djoos

Exactly, that line asserts:

file_get_contents($this->nonceDir.DIRECTORY_SEPARATOR.$nonce) + $this->lifetime > time()

While IMHO it should be:

time() - file_get_contents($this->nonceDir.DIRECTORY_SEPARATOR.$nonce) > $this->lifetime

Equivalent of:

file_get_contents($this->nonceDir.DIRECTORY_SEPARATOR.$nonce) + $this->lifetime < time()

Which is how it was before https://github.com/escapestudios/EscapeWSSEAuthenticationBundle/pull/16

[djoos]

Hi @nuqqsa,

I've had another look at it now and where line 63 actually deals with expiration, line 80 actually checks whether the nonce is unique within a specified lifetime (check on replay attacks) - which was added in #16.

Sorry, after my initial confusion I actually don't see any issue(s) with that commit. Thanks in advance for your feedback!

Kind regards, David

[djoos]

I've just pushed a commit up (ac3f700a88966e6483ff84d5de2b751d7622736d) that includes a clarification of the nonce replay attack-logic. I have to admit it got me confused as well this morning! :-)

Hope this helps!

Kind regards, David

[nualoy]

Hi @djoos,

Thanks a lot for your quick response. And sorry, I meant line 80 too. I was not having a correct understanding of the purpose of the lifetime, the new comment definitely helps clarifying that.

[djoos]

Perfect, thanks for getting in touch - have a great weekend!