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.

Sign up for GitHub

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

Jump to bottom

v0.54 updated OTP handling #85

Merged
merged 29 commits into from
Jun 13, 2019
Merged

v0.54 updated OTP handling #85

merged 29 commits into from
Jun 13, 2019

Conversation

NKelias
Copy link
Contributor

@NKelias NKelias commented Jan 27, 2019 •
edited
Loading

This reworks the OTP handling to resemble that of the Nitrokey Pro.

Functional changes:

  • OTP secret extended to 40 Bytes
  • Authorization changed from pre-authenticated CRC to temporary password in HID report
  • OTP counter transferred as 64bit unsigned Integer instead of C-String
  • OTP counter is retained when editing slots
  • Temporary passwords cleared through lock_device operation
  • HOTP verification functionality added

Further changes:

  • OTP handlng now uses struct format for message parsing and passing data around
  • Replace optimizable memset with non-optimizable memset_safe function for critical data

closes #70
closes #64
closes #26
closes #22
closes #23

Firmware:
nkstorage_v054_pr85.zip

NKelias added 20 commits December 4, 2018 19:47
@NKelias
WIP Remove old old memory addressing and functions
f5c99fa
@NKelias
WIP Refactor USB report protocol calls
1bd9f1a
@NKelias
WIP change OTP functions to struct format
f8f3c33
@NKelias
Change Write OTP to struct format
da72906
@NKelias
Add Debug file output
d778912
@NKelias
Merge branch 'debug_file' into otp_extended
40d9eaa
@NKelias
Fix multiple addressing issues
ce793cc
@NKelias
Add config parsing and refactor OTP write function
1342e60
@NKelias
Add config flags to output and refactor
726eb96
@NKelias
Fix slot editing issue and refactor write to config
7f4071a
@NKelias
Refactor auto-type function
eb70dc2
@NKelias
Remove unused code and add comments
3626144
@NKelias
Remove config bitfields from OTP slot
232ddf6
@NKelias
Extend secret length to 40 bytes and remove CRC authentication
ec5c0c1
@NKelias
Fix addressing issue in WTS, counter read issue and wrong ERASE password
c8e843b 
offset
@NKelias
Reverse Endianness of Interval/Counter
6de3db6
@NKelias
Resolve overflow issue for too long counter values
b1efb20
@NKelias
Add comments, Refactor and remove temporary passwords on lock_device
2e5cc77
@NKelias
Add memset_safe function and use it to erase critical data
0db5067
@NKelias
Re-enable erase of SD card after clearing User page
223507c
This was referenced Jan 27, 2019
Closed
Closed
Closed
@jans23
Copy link
Member

jans23 commented Jan 28, 2019

While this PR changes the stored OTP data structure, I suggest to include the OTP verification feature (Purism) too, to avoid another subsequent change.

@szszszsz
Copy link
Member

Related to #85 (comment) : #71

@NKelias
Copy link
Contributor Author

NKelias commented Jan 29, 2019 •
edited
Loading

Are we talking about the verification functionality in the NK Pro here?
https://github.com/Nitrokey/nitrokey-pro-firmware/blob/664c11b3e6429551f1b21534efd8ae364047fa87/src/keyboard/report_protocol.c#L445

@szszszsz
Copy link
Member

Exactly. In short device should be able to calculate HOTP code on a separate slot (not available to read by the usual OTP API), and compare with the received code. If matches, should wink green, or red otherwise. Device should offer 10 attempts (AFAIR).

Tests, and the client app, are provided here: nitrokey-hotp-verification.

@szszszsz szszszsz added this to the v0.54 milestone Jan 31, 2019
@NKelias
Copy link
Contributor Author

NKelias commented Feb 7, 2019

Updated firmware:
nkstorage_v054_pr85.zip

szszszsz and others added 5 commits March 18, 2019 17:22
@szszszsz
Blink much longer, so the user would notice
ffd1d10 
See Nitrokey/nitrokey-pro-firmware@c7debe2

Signed-off-by: Szczepan Zalega <szczepan@nitrokey.com>
@szszszsz
Bump version to v0.54
f7ef539 
Signed-off-by: Szczepan Zalega <szczepan@nitrokey.com>
@szszszsz
Update smart card status on Pro counters' requests
e5e1f5b 
Required for Nitrokey HOTP Verification tests pass
Not tested on libnitrokey's suite yet

Signed-off-by: Szczepan Zalega <szczepan@nitrokey.com>
@NKelias
Remove Debug output to Text file
b6b089e
@NKelias
Replace constant with dynamic value in memset_safe function
667d466
@tlaurion tlaurion mentioned this pull request Apr 27, 2019
Closed
@szszszsz szszszsz mentioned this pull request Jun 13, 2019
Closed
@szszszsz szszszsz merged commit 667d466 into Nitrokey:master Jun 13, 2019
szszszsz added a commit that referenced this pull request Jun 13, 2019
@szszszsz
Merge branch 'otp_extended'
7f41e19 
Following description from #85:

Functional changes:
    OTP secret extended to 40 Bytes
    Authorization changed from pre-authenticated CRC to temporary password in HID report
    OTP counter transferred as 64bit unsigned Integer instead of C-String
    OTP counter is retained when editing slots
    Temporary passwords cleared through lock_device operation
    HOTP verification functionality added

Further changes:
    OTP handlng now uses struct format for message parsing and passing data around
    Replace optimizable memset with non-optimizable memset_safe function for critical data

Additional:
    Smart card counters update on Pro request

Fixes #85
Fixes #70
Fixes #64
Fixes #26
Fixes #22
Fixes #23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.54
3 participants
@NKelias @jans23 @szszszsz