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

Print SD card usage in Storage status #124

Closed
wants to merge 2 commits into from

Conversation

robinkrahl
Copy link
Collaborator

The Storage device keeps track of the areas of the SD card that have
been accessed during this power cycle. This data can be accessed using
the NK_get_SD_usage_data function that returns a range of the SD card
that has not been accessed. This data can be used as a guide line when
creating new hidden volumes.

This patch adds the SD card usage data to the output of the status
command for Nitrokey Storage devices.

The Storage device keeps track of the areas of the SD card that have
been accessed during this power cycle.  This data can be accessed using
the NK_get_SD_usage_data function that returns a range of the SD card
that has not been accessed.  This data can be used as a guide line when
creating new hidden volumes.

This patch adds the SD card usage data to the output of the status
command for Nitrokey Storage devices.
@d-e-s-o d-e-s-o self-assigned this Sep 11, 2020
Copy link
Owner

@d-e-s-o d-e-s-o left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I wasn't aware of this function. I think the addition makes sense (thanks!), but I have a question first.

Comment on lines +406 to +408
let sd_card_usage = device
.get_sd_card_usage()
.context("Failed to retrieve SD card usage")?;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit confused by the return value of this function for two reasons:

  • I've done nothing else than plugged in the key and the output is: 1% .. 99% not accessed. The documentation states the it should return something <= 100. Any idea why I don't see 100? I tried on two sticks.
  • I am also confused by the fact that it uses a range with an exclusive upper bound. Are you sure that's correct? I can't seem to read that interpretation from libnitrokey itself. It feels as if a user would almost certainly expect a fully inclusive range to be reported at least by nitrocli. What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Well, 99 is <= 100, isn’t it? I get the same output. Unfortunately, there is no documentation for this command. I am glad that I could at least figure out what the usage values mean. So I decided to only guarantee those invariants that I could see in the source code. In this case, there is a unit test that makes sure that 0 <= value <= 100 for both values. I am not sure why the firmware always adds/subtracts one, but I’m inclined to consider it a bug.
  • The problem is that there is no documentation, neither in libnitrokey nor in the firmware. If you refer to the NK_SD_usage_data and NK_get_SD_usage_data doc comments NitrokeyManager.h, I wrote them myself and I tried to keep them as vague as possible as I don’t know the details. I felt like returning a RangeInclusive would imply a certainty that I did not have. Maybe it would have been better to just return a tuple.

But as we don’t claim to find the largest range and as this is only a guide line for creating a hidden volume, I figured that maybe-off-by-one values are better than no information at all.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Well, 99 is <= 100, isn’t it? I get the same output. Unfortunately, there is no documentation for this command. I am glad that I could at least figure out what the usage values mean. So I decided to only guarantee those invariants that I could see in the source code. In this case, there is a unit test that makes sure that 0 <= value <= 100 for both values. I am not sure why the firmware always adds/subtracts one, but I’m inclined to consider it a bug.

Have you talked to the nitrokey guys about that or opened an issue? Seems confusing for users.

  • The problem is that there is no documentation, neither in libnitrokey nor in the firmware. If you refer to the NK_SD_usage_data and NK_get_SD_usage_data doc comments NitrokeyManager.h, I wrote them myself and I tried to keep them as vague as possible as I don’t know the details. I felt like returning a RangeInclusive would imply a certainty that I did not have. Maybe it would have been better to just return a tuple.

But as we don’t claim to find the largest range and as this is only a guide line for creating a hidden volume, I figured that maybe-off-by-one values are better than no information at all.

Sure. I just very much doubt that a half open range would be reported. It just doesn't make any sense to me. In any case, it doesn't matter here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I’m sure I brought it up somewhere, I just can’t remember where. ^^

@d-e-s-o
Copy link
Owner

d-e-s-o commented Sep 12, 2020

Okay, so I played around with it a bit and it appears that it's not so much about accesses but about writes. I read a 1.5 GiB file from my device and nothing changed in the output. It was only after writing to it that I finally saw a change:

    SD card usage:     24% .. 99% not accessed

So I'll change the string to written.

@d-e-s-o
Copy link
Owner

d-e-s-o commented Sep 12, 2020

Merged it! Thanks Robin!

@d-e-s-o d-e-s-o closed this Sep 12, 2020
@robinkrahl
Copy link
Collaborator Author

Thanks!

Indeed, while the firmware sends both the read and the write range, libnitrokey only returns the write range. I’ve opened an issue: Nitrokey/libnitrokey#187

@robinkrahl robinkrahl deleted the sd-card-usage branch September 20, 2020 21:26
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.

2 participants