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

Improve error messages for overlong input data #170

Closed
wants to merge 1 commit into from

Conversation

robinkrahl
Copy link
Collaborator

Previously, we didn’t check the length of the input data in the otp set,
pws add and pws update commands. While libnitrokey does check the input
and return an error if a string is too long, it cannot inform us which
of the input strings is the problematic one.

With this patch, we manually validate the input string lengths for these
commands to improve the error messages, clearly stating which strings
are problematic, how long they are and how long they can be.

Fixes #161.

Previously, we didn’t check the length of the input data in the otp set,
pws add and pws update commands.  While libnitrokey does check the input
and return an error if a string is too long, it cannot inform us which
of the input strings is the problematic one.

With this patch, we manually validate the input string lengths for these
commands to improve the error messages, clearly stating which strings
are problematic, how long they are and how long they can be.

Fixes d-e-s-o#161.
@robinkrahl robinkrahl requested a review from d-e-s-o April 25, 2021 16:11
@d-e-s-o
Copy link
Owner

d-e-s-o commented Apr 25, 2021

Great work! Thanks Robin! Should we have an issue open and a TODO referencing it to not forget about the move away from hardcoding these maximum lengths in nitrocli?

@robinkrahl
Copy link
Collaborator Author

Thanks! We have the libnitrokey issue and PR. Once a new libnitrokey version is released, I’ll update nitrokey-sys. And for every nitrokey-sys update, I check which changes are relevant for nitrocli. So having another issue or TODO is not necessary for me, but feel free to add it if you want to.

@d-e-s-o
Copy link
Owner

d-e-s-o commented Apr 25, 2021

Sounds good. That should be good enough then. Merged!

@d-e-s-o d-e-s-o closed this Apr 25, 2021
@robinkrahl robinkrahl deleted the validate-string-length branch April 25, 2021 16:52
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 this pull request may close these issues.

Validate PWS and OTP string length
2 participants