Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User command authorization works only once #7

Closed
FlorianUekermann opened this issue Jul 2, 2016 · 7 comments · Fixed by #25
Closed

User command authorization works only once #7

FlorianUekermann opened this issue Jul 2, 2016 · 7 comments · Fixed by #25
Milestone

Comments

@FlorianUekermann
Copy link
Contributor

How I think the protocol is supposed to work:

  1. The USER_AUTHENTICATE authenticate command sets a temporary password, which I assume is supposed to be used as authorization secret during a session.
  2. The USER_AUTHORIZE command informs the device about an upcoming command CRC and needs to provide the correct temporary password.
  3. Get OTP using GET_CODE
  4. Repeat from step 2.

What actually happens:

First time works fine. Second time:
The response to GET_CODE reports the status NOT_AUTHORIZED (payload 0), even though the response to USER_AUTHORIZE looks fine.

To successfully use GET_CODE repeatedly, both the USER_AUTHENTICATE and USER_AUTHORIZE command have to be used every time.

What I didn't test:
There might be a similar issue with the other temporary password (FIRST_AUTHENTICATE). Or not.

@szszszsz
Copy link
Member

Hi @MaVo159 !
Are your codes PIN protected?
I do not have to authorize getting HOTP codes otherwise AFAIR. Please check this simple example code

@FlorianUekermann
Copy link
Contributor Author

FlorianUekermann commented Jul 15, 2016

I'm testing with TOTP. I'm not sure what you mean with "Are your codes PIN protected?". The first step (USER_AUTHENTICATE) involves sending the pin if I remember correctly, so that would mean they are PIN protected, I guess. I'm fairly certain not doing the USER_AUTHENTICATE & USER_AUTHORIZE thing first gives an error even on the first GET_CODE.

Actually, I assumed you were aware of this bug since it seems familiar from nitrokey-app#82. But that is just a guess.

Your test code only does HOTP from what I can see, so I wouldn't be surprised if the bug doesn't show up there. I'll get to checking the HOTP stuff in detail at some point. So far I have only tested TOTP thoroughly.

If you can't reproduce this with TOTP, comment again. In about a week I am back from vacation and may have time to deal with this.

@szszszsz
Copy link
Member

szszszsz commented Jul 15, 2016

I have asked about PIN protection since this switch possibly could force user to authenticate each code request (it does in application and from your description it looks like it is checked on device). When I have this one checked the test code I have linked was not working. The option state is visible in Nitrokey App's Safe settings as in mentioned nitrokey-app#82.

As for HOTP & TOTP, the GET_CODE should be the same for both, since they differ only with slot number as far as I remember. If in doubt, you can always check how App handles this (the commands sequence) with running nitrokey-app -d and selecting Debug from context menu.
Let me know has this worked for you. Have a nice holidays!

@FlorianUekermann
Copy link
Contributor Author

Ah. Makes sense. I'll check.

@FlorianUekermann
Copy link
Contributor Author

Yes you were right. It is checked. Unchecking it allow multiple OTP requests. However, now it doesn't require any PIN at all.

Just so I get this straight... This does not work as intended, right? The only way I can wrap my head around all these different commands is that the idea behind the temporary password is: Do authentication via PIN once and then authorize the generation of OTPs many times via temporary password without reentering the PIN or keeping it in memory.

@szszszsz
Copy link
Member

Just to leave a solution - the culprit is here:
https://github.com/Nitrokey/nitrokey-pro-firmware/blob/master/src/keyboard/report_protocol.c#L757
https://github.com/Nitrokey/nitrokey-pro-firmware/blob/master/src/keyboard/report_protocol.c#L733

cmd_authorize should not clear temporary password, but it does.
Reseting temporary password on authorization cancels whole purpose of using it - protecting OTP codes with PIN needs providing PIN from user each time, while temporary password should be used.

This is already fixed on NK Storage in same places.

@jans23 jans23 added this to the v0.8 milestone Oct 27, 2016
@FlorianUekermann
Copy link
Contributor Author

The fix for this should be done together with #8 since the same code will be touched.

szszszsz added a commit that referenced this issue May 19, 2018
Signed-off-by: Szczepan Zalega <szczepan@nitrokey.com>
szszszsz added a commit that referenced this issue May 19, 2018
Signed-off-by: Szczepan Zalega <szczepan@nitrokey.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants