Skip to content
This repository has been archived by the owner on Apr 24, 2021. It is now read-only.

Invalid signature for some bad OTP requests, but not all #8

Open
cyli opened this issue Mar 28, 2013 · 3 comments
Open

Invalid signature for some bad OTP requests, but not all #8

cyli opened this issue Mar 28, 2013 · 3 comments

Comments

@cyli
Copy link

cyli commented Mar 28, 2013

I originally opened this issue in the yubico-dotnet-client repo.

I was testing invalid OTPs, and often got an exception saying that the server signature did not match the key.

As @klali helpfully pointed out, this is because, in the server, if the parameters provided are missing or malformed then the response is sent back immediately with a bad status, without having loaded the key first. Hence the whole response is signed with an empty key, and hence the signature is wrong.

https://code.google.com/p/yubikey-val-server-php/wiki/GettingStartedWritingClients suggests that clients check the length of the OTP, but not for ModHex characters in QWERTY or Dvorak format (and the reference clients all prevent the other checks from failing, such as a malformed nonce or missing parameters). But the server does specifically check for the ModHex characters in QWERTY and Dvorak, so there can be a mismatch if I enter in random characters that are part of neither alphabet, since the reference clients (and clients conforming to the suggestions on the wiki) won't catch it.

Would it make sense for the ModHex alphabet check to be done after the key is loaded, since all the other validation failures that can happen before the key is loaded are prevented by checks/implementation of the reference clients? This way all bad otp responses that the client receives will be signed correctly, and there won't be the unexpected bad signature failure (which would suggest some type of MITM or other more serious problem).

@jas4711
Copy link
Contributor

jas4711 commented Jun 25, 2013

This sounds like a good idea. A patch would be appreciated, then we can look at how it would actually be implemented. Thanks for the report.

@cyli
Copy link
Author

cyli commented Jun 25, 2013

Will do, thanks for the response!

Edit: caveat: will TRY, as I don't actually know PHP. :)

@jas4711
Copy link
Contributor

jas4711 commented Jun 26, 2014

We believe this should be implemented by not sending the 'h=' parameter if the server doesn't have a key available.

/Simon

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants