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

Print SD card usage in Storage status #124

Closed
wants to merge 2 commits into from
Closed

Print SD card usage in Storage status #124

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6965d56
Print SD card usage in Storage status
robinkrahl Sep 10, 2020
e3e0f9e
fixup! Print SD card usage in Storage status
robinkrahl Sep 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion doc/nitrocli.1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ them, set the \fB\-\-no-connect\fR option.
Print the status of the connected Nitrokey device, including the stick serial
number, the firmware version, and the PIN retry count. If the device is a
Nitrokey Storage, also print storage related information including the SD card
serial number, the encryption status, and the status of the volumes.
serial number, the SD card usage during this power cycle, the encryption
status, and the status of the volumes.
.TP
.B nitrocli lock
Lock the Nitrokey.
Expand Down Expand Up @@ -121,6 +122,10 @@ respectively, the start and end position of the hidden volume inside the
encrypted volume, as a percentage of the encrypted volume's size.
This command requires a password which is later used to look up the hidden
volume to open. Unlike a PIN, this password is not cached by \fBgpg\-agent\fR(1).

As a guide line for creating new hidden volumes, the \fBstatus\fR command
provides a range of the SD card that has not been accessed during this power
cycle.
.TP
\fBnitrocli hidden open
Open a hidden volume. The volume to open is determined based on the password
Expand Down
Binary file modified doc/nitrocli.1.pdf
View file
Open in desktop
Binary file not shown.
10 changes: 9 additions & 1 deletion src/commands.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use std::convert::TryFrom as _;
use std::fmt;
use std::mem;
use std::ops;
use std::ops::Deref as _;
use std::thread;
use std::time;
Expand Down Expand Up @@ -334,18 +335,22 @@ where
fn print_storage_status(
ctx: &mut Context<'_>,
status: &nitrokey::StorageStatus,
sd_card_usage: &ops::Range<u8>,
) -> anyhow::Result<()> {
println!(
ctx,
r#" Storage:
SD card ID: {id:#x}
SD card usage: {usagestart}% .. {usageend}% not accessed
firmware: {fw}
storage keys: {sk}
volumes:
unencrypted: {vu}
encrypted: {ve}
hidden: {vh}"#,
id = status.serial_number_sd_card,
usagestart = sd_card_usage.start,
usageend = sd_card_usage.end,
fw = if status.firmware_locked {
"locked"
} else {
Expand Down Expand Up @@ -398,8 +403,11 @@ fn print_status(
let status = device
.get_storage_status()
.context("Failed to retrieve storage status")?;
let sd_card_usage = device
.get_sd_card_usage()
.context("Failed to retrieve SD card usage")?;
Comment on lines +406 to +408
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. ^^


print_storage_status(ctx, &status)
print_storage_status(ctx, &status, &sd_card_usage)
} else {
Ok(())
}
Expand Down
1 change: 1 addition & 0 deletions src/tests/status.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ fn output_storage(model: nitrokey::Model) -> anyhow::Result<()> {
admin retry count: [0-3]
Storage:
SD card ID: 0x[[:xdigit:]]{8}
SD card usage: \d+% .. \d+% not accessed
firmware: (un)?locked
storage keys: (not )?created
volumes:
Expand Down