Add getters for OTP and PWS properties #198
Conversation
NitrokeyManager.cc
Outdated
| if (device == nullptr) { | ||
| return 0; | ||
| } else { | ||
| return 40; |
There was a problem hiding this comment.
For earlier firmware versions of Nitrokey Pro this would be 20. Please add FIXME here to remember about extending that (details are in the libnitrokey's OTP implementation); ideally you can add this implementation here already (so it would not rot until I will get there).
There was a problem hiding this comment.
Thanks, I’ll have a look at it.
There was a problem hiding this comment.
Apparently, it was changed in v0.8 for the Nitrokey Pro and v0.54 for the Nitrokey Storage, see is_320_OTP_secret_supported. But it would be nicer if we would not have to perform a GET_STATUS command to query this number. I’ll think about other solutions.
This patch extracts the HOTP and TOTP slot count as well as the maximum length for the OTP slot name and secret into defines in device_proto.h, similar to the PWS constants.
This patch adds getters for the number of PWS, TOTP and HOTP slots on the current device and the maximum length of a PWS slot name, login and password as well as a OTP slot name and secret. Fixes Nitrokey#197.
robinkrahl
force-pushed
the
otp-pws-properties
branch
from
ea7a5e5
to
db324e0
Compare
|
Hi! Please let me know once the PR is ready to go. |
|
Sure!
My current plan is to cache the firmware version for the current device.
This would avoid executing GET_STATUS multiple times for example when
first validating the string length and then writing to the slot. This
might be problematic if the user upgrades the firmware while libnitrokey
is still connected to the device, but that is a tradeoff I would be
willing to make. What do you think?
|
|
Indeed, the firmware update is not very much frequent event, but should be taken into account anyway (e.g. for the tools that check the changed firmware version). Until now I was expecting the client applications to do the caching (see Nitrokey App and its libnitrokey wrapper), and libnitrokey to do the raw communication, to not add too much hidden logic or unexpected side-effects. We need some cache invalidation mechanism with that, e.g. in a form of a logout function. Perhaps calling "device lock" could function as such, so there would not be a need for extending the API. Thoughts? |
|
Can’t we use NK_logout / NitrokeyManager::disconnect for that?
|
|
Yes, sounds like a better choice |
This patch adds getters for the number of PWS, TOTP and HOTP slots on
the current device and the maximum length of a PWS slot name, login and
password as well as a OTP slot name and secret.
Fixes #197.