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

Use new PWS access API #138

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Next Next commit
Use new PWS access API
In previous versions of nitrokey-rs, the getters for the name, login and
password fields of a PWS slot would return an error for empty strings as
libnitrokey uses empty strings to indicate unprogrammed slots.  But as
the user might have deliberately specified an empty value, nitrokey-rs
v0.9.0 introduces a new PWS API that makes it possible to distinguish
unprogrammed slots and empty values.

This patch updates the nitrokey-rs dependency and refactors the pws
status and pws get commands to use the new PWS API.
  • Loading branch information
robinkrahl committed Mar 26, 2021
commit 211a25421c879097ec3a8b5e030795d5f84c7261
3 changes: 1 addition & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ version = "0.2"
version = "0.1"

[dependencies.nitrokey]
version = "0.8"
git = "https://git.sr.ht/~ireas/nitrokey-rs"
branch = "pws-slots"

[dependencies.progressing]
version = "3.0.2"
Expand Down
49 changes: 16 additions & 33 deletions src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
// SPDX-License-Identifier: GPL-3.0-or-later

use std::borrow;
use std::convert::TryFrom as _;
use std::env;
use std::ffi;
use std::fmt;
Expand Down Expand Up @@ -996,20 +995,6 @@ fn print_pws_data(
Ok(())
}

fn check_slot(pws: &nitrokey::PasswordSafe<'_, '_>, slot: u8) -> anyhow::Result<()> {
if slot >= nitrokey::SLOT_COUNT {
anyhow::bail!("Slot {} is not valid", slot);
}
let status = pws
.get_slot_status()
.context("Failed to read PWS slot status")?;
if status[slot as usize] {
Ok(())
} else {
anyhow::bail!("Slot {} is not programmed", slot)
}
}

/// Read a PWS slot.
pub fn pws_get(
ctx: &mut Context<'_>,
Expand All @@ -1020,17 +1005,17 @@ pub fn pws_get(
quiet: bool,
) -> anyhow::Result<()> {
with_password_safe(ctx, |ctx, pws| {
check_slot(&pws, slot).context("Failed to access PWS slot")?;
let slot = pws.get_slot(slot).context("Failed to access PWS slot")?;
robinkrahl marked this conversation as resolved.
Show resolved Hide resolved

let show_all = !show_name && !show_login && !show_password;
if show_all || show_name {
print_pws_data(ctx, "name: ", pws.get_slot_name(slot), quiet)?;
print_pws_data(ctx, "name: ", slot.get_name(), quiet)?;
}
if show_all || show_login {
print_pws_data(ctx, "login: ", pws.get_slot_login(slot), quiet)?;
print_pws_data(ctx, "login: ", slot.get_login(), quiet)?;
}
if show_all || show_password {
print_pws_data(ctx, "password:", pws.get_slot_password(slot), quiet)?;
print_pws_data(ctx, "password:", slot.get_password(), quiet)?;
}
Ok(())
})
Expand Down Expand Up @@ -1060,31 +1045,29 @@ pub fn pws_clear(ctx: &mut Context<'_>, slot: u8) -> anyhow::Result<()> {

fn print_pws_slot(
ctx: &mut Context<'_>,
pws: &nitrokey::PasswordSafe<'_, '_>,
slot: usize,
programmed: bool,
index: usize,
slot: Option<nitrokey::PasswordSlot<'_, '_, '_>>,
) -> anyhow::Result<()> {
let slot = u8::try_from(slot).map_err(|_| anyhow::anyhow!("Invalid PWS slot number"))?;
let name = if programmed {
pws
.get_slot_name(slot)
.context("Failed to read PWS slot name")?
let name = if let Some(slot) = slot {
slot.get_name().context("Failed to read PWS slot name")?
} else {
"[not programmed]".to_string()
};
println!(ctx, "{}\t{}", slot, name)?;
println!(ctx, "{}\t{}", index, name)?;
Ok(())
}

/// Print the status of all PWS slots.
pub fn pws_status(ctx: &mut Context<'_>, all: bool) -> anyhow::Result<()> {
with_password_safe(ctx, |ctx, pws| {
let slots = pws
.get_slot_status()
.context("Failed to read PWS slot status")?;
let slots = pws.get_slots().context("Failed to read PWS slot status")?;
Copy link
Owner

Choose a reason for hiding this comment

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

Consider adjusting the error message slightly to "Failed to read PWS slots".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I’m not sure about that. Internally, we still call the GET_PW_SAFE_SLOT_STATUS command. “Failed to read PWS slots” sounds as if we tried to read the slot values.

println!(ctx, "slot\tname")?;
for (i, &value) in slots.iter().enumerate().filter(|(_, &value)| all || value) {
print_pws_slot(ctx, &pws, i, value)?;
for (i, &slot) in slots
.iter()
.enumerate()
.filter(|(_, &slot)| all || slot.is_some())
{
print_pws_slot(ctx, i, slot)?;
}
Ok(())
})
Expand Down
25 changes: 25 additions & 0 deletions src/tests/pws.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,31 @@ fn set_get(model: nitrokey::Model) -> anyhow::Result<()> {
Ok(())
}

#[test_device]
fn set_empty(model: nitrokey::Model) -> anyhow::Result<()> {
let mut ncli = Nitrocli::new().model(model);
let _ = ncli.handle(&["pws", "set", "1", "", "", ""])?;

let out = ncli.handle(&["pws", "get", "1", "--quiet", "--name"])?;
assert_eq!(out, "\n");

let out = ncli.handle(&["pws", "get", "1", "--quiet", "--login"])?;
assert_eq!(out, "\n");

let out = ncli.handle(&["pws", "get", "1", "--quiet", "--password"])?;
assert_eq!(out, "\n");

let out = ncli.handle(&["pws", "get", "1", "--quiet"])?;
assert_eq!(out, "\n\n\n");

let out = ncli.handle(&["pws", "get", "1"])?;
assert_eq!(
out,
"name: \nlogin: \npassword: \n",
);
Ok(())
}

#[test_device]
fn set_reset_get(model: nitrokey::Model) -> anyhow::Result<()> {
const NAME: &str = "some/svc";
Expand Down