turnkey/openpgp-card
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Split the Response type into an internal RawResponse type which also contains the status bytes, and an external Response type that can only be generated from a RawResponse with status "ok".

Browse source 
This removes the need for external users of openpgp-card to check the status or operations.
That is, openpgp-card now always returns an `Err` if the status of a command is not ok.
This commit is contained in:
Heiko Schaefer 2021-08-20 13:17:08 +02:00
parent 4959307b1f
commit 8aae0a357e
9 changed files with 128 additions and 123 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

77
card-functionality/src/tests.rs
View file

@ -24,7 +24,8 @@ use crate::util;
#[derive(Debug)]
pub enum TestResult {
Status([u8; 2]),
Status(OcErrorStatus),
StatusOk,
Text(String),
}
@ -62,8 +63,7 @@ pub fn test_decrypt(
let cert = Cert::from_str(param[0])?;
let msg = param[1].to_string();
let res = ca.verify_pw1("123456")?;
res.check_ok()?;
ca.verify_pw1("123456")?;
let res = openpgp_card_sequoia::decrypt(&mut ca, &cert, msg.into_bytes())?;
let plain = String::from_utf8_lossy(&res);
@ -80,8 +80,7 @@ pub fn test_sign(
) -> Result<TestOutput, TestError> {
assert_eq!(param.len(), 1, "test_sign needs a filename for 'cert'");
let res = ca.verify_pw1_for_signing("123456")?;
res.check_ok()?;
ca.verify_pw1_for_signing("123456")?;
let cert = Cert::from_str(param[0])?;
@ -190,8 +189,7 @@ pub fn test_upload_keys(
"test_upload_keys needs a filename for 'cert'"
);
let verify = ca.verify_pw3("12345678")?;
verify.check_ok()?;
ca.verify_pw3("12345678")?;
let cert = Cert::from_file(param[0])?;
@ -211,8 +209,7 @@ pub fn test_keygen(
ca: &mut CardApp,
param: &[&str],
) -> Result<TestOutput, TestError> {
let verify = ca.verify_pw3("12345678")?;
verify.check_ok()?;
ca.verify_pw3("12345678")?;
// Generate all three subkeys on card
let algo = param[0];
@ -328,24 +325,19 @@ pub fn test_set_user_data(
ca: &mut CardApp,
_param: &[&str],
) -> Result<TestOutput, TestError> {
let res = ca.verify_pw3("12345678")?;
res.check_ok()?;
ca.verify_pw3("12345678")?;
// name
let res = ca.set_name("Bar<<Foo")?;
res.check_ok()?;
ca.set_name("Bar<<Foo")?;
// lang
let res = ca.set_lang("deen")?;
res.check_ok()?;
ca.set_lang("deen")?;
// sex
let res = ca.set_sex(Sex::Female)?;
res.check_ok()?;
ca.set_sex(Sex::Female)?;
// url
let res = ca.set_url("https://duckduckgo.com/")?;
res.check_ok()?;
ca.set_url("https://duckduckgo.com/")?;
// read all the fields back again, expect equal data
let ch = ca.get_cardholder_related_data()?;
@ -382,31 +374,46 @@ pub fn test_verify(
let mut out = vec![];
// try to set name without verify, assert result is not ok!
let res = ca.set_name("Notverified<<Hello")?;
assert_eq!(res.status(), [0x69, 0x82]); // "Security status not satisfied"
let res = ca.set_name("Notverified<<Hello");
let res = ca.verify_pw3("12345678")?;
res.check_ok()?;
if let Err(OpenpgpCardError::OcStatus(s)) = res {
assert_eq!(s, OcErrorStatus::SecurityStatusNotSatisfied);
} else {
panic!();
}
let check = ca.check_pw3()?;
// don't "check_ok()" - yubikey5 returns an error code!
out.push(TestResult::Status(check.status()));
ca.verify_pw3("12345678")?;
let res = ca.set_name("Admin<<Hello")?;
res.check_ok()?;
match ca.check_pw3() {
Err(OpenpgpCardError::OcStatus(s)) => {
// e.g. yubikey5 returns an error status!
out.push(TestResult::Status(s));
}
Err(_) => {
panic!("unexpected error");
}
Ok(_) => out.push(TestResult::StatusOk),
}
ca.set_name("Admin<<Hello")?;
let cardholder = ca.get_cardholder_related_data()?;
assert_eq!(cardholder.name, Some("Admin<<Hello".to_string()));
let res = ca.verify_pw1("123456")?;
res.check_ok()?;
ca.verify_pw1("123456")?;
let check = ca.check_pw3()?;
// don't "check_ok()" - yubikey5 returns an error code
out.push(TestResult::Status(check.status()));
match ca.check_pw3() {
Err(OpenpgpCardError::OcStatus(s)) => {
// e.g. yubikey5 returns an error status!
out.push(TestResult::Status(s));
}
Err(_) => {
panic!("unexpected error");
}
Ok(_) => out.push(TestResult::StatusOk),
}
let res = ca.set_name("There<<Hello")?;
res.check_ok()?;
ca.set_name("There<<Hello")?;
let cardholder = ca.get_cardholder_related_data()?;
assert_eq!(cardholder.name, Some("There<<Hello".to_string()));

45
openpgp-card-sequoia/src/lib.rs
View file

@ -204,8 +204,7 @@ pub fn make_cert(
)?;
// Allow signing on the card
let res = ca.verify_pw1_for_signing("123456")?;
res.check_ok()?;
ca.verify_pw1_for_signing("123456")?;
// Card-backed signer for bindings
let mut card_signer = CardSigner::with_pubkey(ca, key_sig.clone());
@ -230,8 +229,7 @@ pub fn make_cert(
.set_key_flags(KeyFlags::empty().set_authentication())?;
// Allow signing on the card
let res = ca.verify_pw1_for_signing("123456")?;
res.check_ok()?;
ca.verify_pw1_for_signing("123456")?;
// Card-backed signer for bindings
let mut card_signer = CardSigner::with_pubkey(ca, key_sig.clone());
@ -263,8 +261,7 @@ pub fn make_cert(
)?;
// Allow signing on the card
let res = ca.verify_pw1_for_signing("123456")?;
res.check_ok()?;
ca.verify_pw1_for_signing("123456")?;
// Card-backed signer for bindings
let mut card_signer = CardSigner::with_pubkey(ca, key_sig);
@ -714,15 +711,11 @@ impl CardBase {
) -> Result<CardSign, CardBase> {
assert!(pin.len() >= 6); // FIXME: Err
let res = self.card_app.verify_pw1_for_signing(pin);
if let Ok(resp) = res {
if resp.is_ok() {
return Ok(CardSign { oc: self });
}
if self.card_app.verify_pw1_for_signing(pin).is_ok() {
Ok(CardSign { oc: self })
} else {
Err(self)
}
Err(self)
}
pub fn check_pw1(&mut self) -> Result<Response, OpenpgpCardError> {
@ -732,15 +725,11 @@ impl CardBase {
pub fn verify_pw1(mut self, pin: &str) -> Result<CardUser, CardBase> {
assert!(pin.len() >= 6); // FIXME: Err
let res = self.card_app.verify_pw1(pin);
if let Ok(resp) = res {
if resp.is_ok() {
return Ok(CardUser { oc: self });
}
if self.card_app.verify_pw1(pin).is_ok() {
Ok(CardUser { oc: self })
} else {
Err(self)
}
Err(self)
}
pub fn check_pw3(&mut self) -> Result<Response, OpenpgpCardError> {
@ -750,15 +739,11 @@ impl CardBase {
pub fn verify_pw3(mut self, pin: &str) -> Result<CardAdmin, CardBase> {
assert!(pin.len() >= 8); // FIXME: Err
let res = self.card_app.verify_pw3(pin);
if let Ok(resp) = res {
if resp.is_ok() {
return Ok(CardAdmin { oc: self });
}
if self.card_app.verify_pw3(pin).is_ok() {
Ok(CardAdmin { oc: self })
} else {
Err(self)
}
Err(self)
}
}

4
openpgp-card-sequoia/src/main.rs
View file

@ -108,20 +108,16 @@ fn main() -> Result<(), Box<dyn Error>> {
let res = oc_admin.set_name("Bar<<Foo")?;
println!("set name {:x?}", res);
res.check_ok()?;
let res =
oc_admin.set_sex(openpgp_card::Sex::NotApplicable)?;
println!("set sex {:x?}", res);
res.check_ok()?;
let res = oc_admin.set_lang("en")?;
println!("set lang {:x?}", res);
res.check_ok()?;
let res = oc_admin.set_url("https://keys.openpgp.org")?;
println!("set url {:x?}", res);
res.check_ok()?;
let cert = Cert::from_file(TEST_KEY_PATH)?;

10
openpgp-card/src/apdu/mod.rs
View file

@ -11,7 +11,7 @@ use anyhow::Result;
use std::convert::TryFrom;
use crate::apdu::command::Command;
use crate::apdu::response::Response;
use crate::apdu::response::RawResponse;
use crate::errors::{OcErrorStatus, OpenpgpCardError};
use crate::CardClientBox;
@ -33,20 +33,20 @@ pub(crate) fn send_command(
card_client: &mut CardClientBox,
cmd: Command,
expect_reply: bool,
) -> Result<Response, OpenpgpCardError> {
let mut resp = Response::try_from(send_command_low_level(
) -> Result<RawResponse, OpenpgpCardError> {
let mut resp = RawResponse::try_from(send_command_low_level(
card_client,
cmd,
expect_reply,
)?)?;
while resp.status()[0] == 0x61 {
while resp.status().0 == 0x61 {
// More data is available for this command from the card
log::debug!(" response was truncated, getting more data");
// Get additional data
let next = Response::try_from(send_command_low_level(
let next = RawResponse::try_from(send_command_low_level(
card_client,
commands::get_response(),
expect_reply,

55
openpgp-card/src/apdu/response.rs
View file

@ -1,19 +1,38 @@
// SPDX-FileCopyrightText: 2021 Heiko Schaefer <heiko@schaefer.name>
// SPDX-License-Identifier: MIT OR Apache-2.0
use crate::errors::OcErrorStatus;
use crate::errors::{OcErrorStatus, OpenpgpCardError};
use std::convert::TryFrom;
/// APDU Response
#[allow(unused)]
#[derive(Clone, Debug)]
pub struct Response {
pub(crate) struct RawResponse {
data: Vec<u8>,
sw1: u8,
sw2: u8,
}
impl Response {
#[derive(Debug)]
pub struct Response {
data: Vec<u8>,
}
impl TryFrom<RawResponse> for Response {
type Error = OpenpgpCardError;
fn try_from(value: RawResponse) -> Result<Self, Self::Error> {
if value.is_ok() {
Ok(Response { data: value.data })
} else {
Err(OpenpgpCardError::OcStatus(OcErrorStatus::from(
value.status(),
)))
}
}
}
impl RawResponse {
pub fn check_ok(&self) -> Result<(), OcErrorStatus> {
if !self.is_ok() {
Err(OcErrorStatus::from((self.sw1, self.sw2)))
@ -36,22 +55,22 @@ impl Response {
&mut self.data
}
pub(crate) fn set_status(&mut self, new_status: [u8; 2]) {
self.sw1 = new_status[0];
self.sw2 = new_status[1];
pub(crate) fn set_status(&mut self, new_status: (u8, u8)) {
self.sw1 = new_status.0;
self.sw2 = new_status.1;
}
pub fn status(&self) -> [u8; 2] {
[self.sw1, self.sw2]
pub fn status(&self) -> (u8, u8) {
(self.sw1, self.sw2)
}
/// Is the response status "ok"? [0x90, 0x00]
/// Is the response status "ok"? (0x90, 0x00)
pub fn is_ok(&self) -> bool {
self.status() == [0x90, 0x00]
self.status() == (0x90, 0x00)
}
}
impl<'a> TryFrom<&[u8]> for Response {
impl<'a> TryFrom<&[u8]> for RawResponse {
type Error = OcErrorStatus;
fn try_from(buf: &[u8]) -> Result<Self, OcErrorStatus> {
@ -59,7 +78,7 @@ impl<'a> TryFrom<&[u8]> for Response {
if n < 2 {
return Err(OcErrorStatus::ResponseLength(buf.len()));
}
Ok(Response {
Ok(RawResponse {
data: buf[..n - 2].into(),
sw1: buf[n - 2],
sw2: buf[n - 1],
@ -67,7 +86,7 @@ impl<'a> TryFrom<&[u8]> for Response {
}
}
impl TryFrom<Vec<u8>> for Response {
impl TryFrom<Vec<u8>> for RawResponse {
type Error = OcErrorStatus;
fn try_from(mut data: Vec<u8>) -> Result<Self, OcErrorStatus> {
@ -78,32 +97,32 @@ impl TryFrom<Vec<u8>> for Response {
.pop()
.ok_or_else(|| OcErrorStatus::ResponseLength(data.len()))?;
Ok(Response { data, sw1, sw2 })
Ok(RawResponse { data, sw1, sw2 })
}
}
#[cfg(test)]
mod tests {
use crate::apdu::response::Response;
use crate::apdu::response::RawResponse;
use std::convert::TryFrom;
#[test]
fn test_two_bytes_data_response() {
let res = Response::try_from(vec![0x01, 0x02, 0x90, 0x00]).unwrap();
let res = RawResponse::try_from(vec![0x01, 0x02, 0x90, 0x00]).unwrap();
assert_eq!(res.is_ok(), true);
assert_eq!(res.data, vec![0x01, 0x02]);
}
#[test]
fn test_no_data_response() {
let res = Response::try_from(vec![0x90, 0x00]).unwrap();
let res = RawResponse::try_from(vec![0x90, 0x00]).unwrap();
assert_eq!(res.is_ok(), true);
assert_eq!(res.data, vec![]);
}
#[test]
fn test_more_data_response() {
let res = Response::try_from(vec![0xAB, 0x61, 0x02]).unwrap();
let res = RawResponse::try_from(vec![0xAB, 0x61, 0x02]).unwrap();
assert_eq!(res.is_ok(), false);
assert_eq!(res.data, vec![0xAB]);
}

38
openpgp-card/src/card_app.rs
View file

@ -11,6 +11,7 @@
use std::borrow::BorrowMut;
use std::convert::TryFrom;
use std::convert::TryInto;
use std::time::SystemTime;
use anyhow::{anyhow, Result};
@ -90,7 +91,8 @@ impl CardApp {
/// "Select" the OpenPGP card application
pub fn select(&mut self) -> Result<Response, OpenpgpCardError> {
let select_openpgp = commands::select_openpgp();
apdu::send_command(&mut self.card_client, select_openpgp, false)
apdu::send_command(&mut self.card_client, select_openpgp, false)?
.try_into()
}
// --- application data ---
@ -323,8 +325,8 @@ impl CardApp {
let verify = commands::verify_pw1_81([0x40; 8].to_vec());
let resp =
apdu::send_command(&mut self.card_client, verify, false)?;
if !(resp.status() == [0x69, 0x82]
|| resp.status() == [0x69, 0x83])
if !(resp.status() == (0x69, 0x82)
|| resp.status() == (0x69, 0x83))
{
return Err(anyhow!("Unexpected status for reset, at pw1."));
}
@ -337,8 +339,8 @@ impl CardApp {
let resp =
apdu::send_command(&mut self.card_client, verify, false)?;
if !(resp.status() == [0x69, 0x82]
|| resp.status() == [0x69, 0x83])
if !(resp.status() == (0x69, 0x82)
|| resp.status() == (0x69, 0x83))
{
return Err(anyhow!("Unexpected status for reset, at pw3."));
}
@ -367,12 +369,12 @@ impl CardApp {
assert!(pin.len() >= 6); // FIXME: Err
let verify = commands::verify_pw1_81(pin.as_bytes().to_vec());
apdu::send_command(&mut self.card_client, verify, false)
apdu::send_command(&mut self.card_client, verify, false)?.try_into()
}
pub fn check_pw1(&mut self) -> Result<Response, OpenpgpCardError> {
let verify = commands::verify_pw1_82(vec![]);
apdu::send_command(&mut self.card_client, verify, false)
apdu::send_command(&mut self.card_client, verify, false)?.try_into()
}
pub fn verify_pw1(
@ -382,12 +384,12 @@ impl CardApp {
assert!(pin.len() >= 6); // FIXME: Err
let verify = commands::verify_pw1_82(pin.as_bytes().to_vec());
apdu::send_command(&mut self.card_client, verify, false)
apdu::send_command(&mut self.card_client, verify, false)?.try_into()
}
pub fn check_pw3(&mut self) -> Result<Response, OpenpgpCardError> {
let verify = commands::verify_pw3(vec![]);
apdu::send_command(&mut self.card_client, verify, false)
apdu::send_command(&mut self.card_client, verify, false)?.try_into()
}
pub fn verify_pw3(
@ -397,7 +399,7 @@ impl CardApp {
assert!(pin.len() >= 8); // FIXME: Err
let verify = commands::verify_pw3(pin.as_bytes().to_vec());
apdu::send_command(&mut self.card_client, verify, false)
apdu::send_command(&mut self.card_client, verify, false)?.try_into()
}
// --- decrypt ---
@ -503,7 +505,7 @@ impl CardApp {
name: &str,
) -> Result<Response, OpenpgpCardError> {
let put_name = commands::put_name(name.as_bytes().to_vec());
apdu::send_command(&mut self.card_client, put_name, false)
apdu::send_command(&mut self.card_client, put_name, false)?.try_into()
}
pub fn set_lang(
@ -511,12 +513,14 @@ impl CardApp {
lang: &str,
) -> Result<Response, OpenpgpCardError> {
let put_lang = commands::put_lang(lang.as_bytes().to_vec());
apdu::send_command(self.card_client.borrow_mut(), put_lang, false)
apdu::send_command(self.card_client.borrow_mut(), put_lang, false)?
.try_into()
}
pub fn set_sex(&mut self, sex: Sex) -> Result<Response, OpenpgpCardError> {
let put_sex = commands::put_sex((&sex).into());
apdu::send_command(self.card_client.borrow_mut(), put_sex, false)
apdu::send_command(self.card_client.borrow_mut(), put_sex, false)?
.try_into()
}
pub fn set_url(
@ -524,7 +528,7 @@ impl CardApp {
url: &str,
) -> Result<Response, OpenpgpCardError> {
let put_url = commands::put_url(url.as_bytes().to_vec());
apdu::send_command(&mut self.card_client, put_url, false)
apdu::send_command(&mut self.card_client, put_url, false)?.try_into()
}
pub fn set_creation_time(
@ -545,7 +549,7 @@ impl CardApp {
time_value,
);
apdu::send_command(&mut self.card_client, time_cmd, false)
apdu::send_command(&mut self.card_client, time_cmd, false)?.try_into()
}
pub fn set_fingerprint(
@ -558,7 +562,7 @@ impl CardApp {
fp.to_vec(),
);
apdu::send_command(self.card(), fp_cmd, false)
apdu::send_command(self.card(), fp_cmd, false)?.try_into()
}
/// Set algorithm attributes [4.4.3.9 Algorithm Attributes]
@ -582,7 +586,7 @@ impl CardApp {
// Command to PUT the algorithm attributes
let cmd = commands::put_data(&[key_type.get_algorithm_tag()], data);
apdu::send_command(&mut self.card_client, cmd, false)
apdu::send_command(&mut self.card_client, cmd, false)?.try_into()
}
fn rsa_algo_attrs(algo_attrs: &RsaAttrs) -> Result<Vec<u8>> {

2
openpgp-card/src/errors.rs
View file

@ -41,7 +41,7 @@ impl From<anyhow::Error> for OpenpgpCardError {
}
/// OpenPGP card "Status Byte" errors
#[derive(Error, Debug)]
#[derive(Error, Debug, PartialEq)]
pub enum OcErrorStatus {
#[error("Selected file or DO in termination state")]
TerminationState,

16
openpgp-card/src/keys.rs
View file

@ -32,9 +32,7 @@ pub(crate) fn gen_key_with_metadata(
) -> Result<(PublicKeyMaterial, u32), OpenpgpCardError> {
// set algo on card if it's Some
if let Some(algo) = algo {
card_app
.set_algorithm_attributes(key_type, algo)?
.check_ok()?;
card_app.set_algorithm_attributes(key_type, algo)?;
}
// algo
@ -58,11 +56,11 @@ pub(crate) fn gen_key_with_metadata(
.map_err(|e| OpenpgpCardError::InternalError(anyhow!(e)))?
.as_secs() as u32;
card_app.set_creation_time(ts, key_type)?.check_ok()?;
card_app.set_creation_time(ts, key_type)?;
// calculate/store fingerprint
let fp = fp_from_pub(&pubkey, time, key_type)?;
card_app.set_fingerprint(fp, key_type)?.check_ok()?;
card_app.set_fingerprint(fp, key_type)?;
Ok((pubkey, ts))
}
@ -438,15 +436,13 @@ fn copy_key_to_card(
// FIXME: Only write algo attributes to the card if "extended
// capabilities" show that they are changeable!
card_app
.set_algorithm_attributes(key_type, algo)?
.check_ok()?;
card_app.set_algorithm_attributes(key_type, algo)?;
apdu::send_command(card_app.card(), key_cmd, false)?.check_ok()?;
card_app.set_fingerprint(fp, key_type)?.check_ok()?;
card_app.set_fingerprint(fp, key_type)?;
card_app.set_creation_time(ts, key_type)?.check_ok()?;
card_app.set_creation_time(ts, key_type)?;
Ok(())
}

4
pcsc/src/lib.rs
View file

@ -99,9 +99,7 @@ impl PcscClient {
let ccb = Box::new(card_client) as CardClientBox;
let mut ca = CardApp::new(ccb);
let resp = ca.select()?;
if resp.is_ok() {
if ca.select().is_ok() {
Ok(ca)
} else {
Err(OpenpgpCardError::Smartcard(