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

Consistently model pin as &[u8] in openpgp-card.

Browse source 
Fixes #22
This commit is contained in:
Heiko Schaefer 2022-02-23 14:15:00 +01:00
parent 96167f6530
commit 088bb88a02
No known key found for this signature in database
GPG key ID: 4A849A1904CCBD7D
4 changed files with 58 additions and 63 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -62,7 +62,7 @@ pub fn test_decrypt(card: &mut dyn CardBackend, param: &[&str]) -> Result<TestOu
let cert = Cert::from_str(param[0])?; let cert = Cert::from_str(param[0])?;
let msg = param[1].to_string(); let msg = param[1].to_string();
pgpt.verify_pw1("123456")?; pgpt.verify_pw1(b"123456")?;
let p = StandardPolicy::new(); let p = StandardPolicy::new();
@ -81,7 +81,7 @@ pub fn test_sign(card: &mut dyn CardBackend, param: &[&str]) -> Result<TestOutpu
assert_eq!(param.len(), 1, "test_sign needs a filename for 'cert'"); assert_eq!(param.len(), 1, "test_sign needs a filename for 'cert'");
pgpt.verify_pw1_for_signing("123456")?; pgpt.verify_pw1_for_signing(b"123456")?;
let cert = Cert::from_str(param[0])?; let cert = Cert::from_str(param[0])?;
@ -197,7 +197,7 @@ pub fn test_upload_keys(
"test_upload_keys needs a filename for 'cert'" "test_upload_keys needs a filename for 'cert'"
); );
pgpt.verify_pw3("12345678")?; pgpt.verify_pw3(b"12345678")?;
let cert = Cert::from_file(param[0])?; let cert = Cert::from_file(param[0])?;
@ -219,7 +219,7 @@ pub fn test_keygen(card: &mut dyn CardBackend, param: &[&str]) -> Result<TestOut
let mut pgp = OpenPgp::new(card); let mut pgp = OpenPgp::new(card);
let mut pgpt = pgp.transaction()?; let mut pgpt = pgp.transaction()?;
pgpt.verify_pw3("12345678")?; pgpt.verify_pw3(b"12345678")?;
// Generate all three subkeys on card // Generate all three subkeys on card
let algo = param[0]; let algo = param[0];
@ -316,7 +316,7 @@ pub fn test_set_user_data(
let mut pgp = OpenPgp::new(card); let mut pgp = OpenPgp::new(card);
let mut pgpt = pgp.transaction()?; let mut pgpt = pgp.transaction()?;
pgpt.verify_pw3("12345678")?; pgpt.verify_pw3(b"12345678")?;
// name // name
pgpt.set_name(b"Bar<<Foo")?; pgpt.set_name(b"Bar<<Foo")?;
@ -360,12 +360,12 @@ pub fn test_private_data(
let d = pgpt.private_use_do(1)?; let d = pgpt.private_use_do(1)?;
println!("data 1 {:?}", d); println!("data 1 {:?}", d);
pgpt.verify_pw1("123456")?; pgpt.verify_pw1(b"123456")?;
pgpt.set_private_use_do(1, "Foo bar1!".as_bytes().to_vec())?; pgpt.set_private_use_do(1, "Foo bar1!".as_bytes().to_vec())?;
pgpt.set_private_use_do(3, "Foo bar3!".as_bytes().to_vec())?; pgpt.set_private_use_do(3, "Foo bar3!".as_bytes().to_vec())?;
pgpt.verify_pw3("12345678")?; pgpt.verify_pw3(b"12345678")?;
pgpt.set_private_use_do(2, "Foo bar2!".as_bytes().to_vec())?; pgpt.set_private_use_do(2, "Foo bar2!".as_bytes().to_vec())?;
pgpt.set_private_use_do(4, "Foo bar4!".as_bytes().to_vec())?; pgpt.set_private_use_do(4, "Foo bar4!".as_bytes().to_vec())?;
@ -456,7 +456,7 @@ pub fn test_pw_status(
println!("pws {:?}", pws); println!("pws {:?}", pws);
pgpt.verify_pw3("12345678")?; pgpt.verify_pw3(b"12345678")?;
pws.set_pw1_cds_valid_once(false); pws.set_pw1_cds_valid_once(false);
pws.set_pw1_pin_block(true); pws.set_pw1_pin_block(true);
@ -500,7 +500,7 @@ pub fn test_verify(card: &mut dyn CardBackend, _param: &[&str]) -> Result<TestOu
panic!("Status should be 'SecurityStatusNotSatisfied'"); panic!("Status should be 'SecurityStatusNotSatisfied'");
} }
pgpt.verify_pw3("12345678")?; pgpt.verify_pw3(b"12345678")?;
match pgpt.check_pw3() { match pgpt.check_pw3() {
Err(Error::CardStatus(s)) => { Err(Error::CardStatus(s)) => {
@ -518,7 +518,7 @@ pub fn test_verify(card: &mut dyn CardBackend, _param: &[&str]) -> Result<TestOu
let cardholder = pgpt.cardholder_related_data()?; let cardholder = pgpt.cardholder_related_data()?;
assert_eq!(cardholder.name(), Some("Admin<<Hello".as_bytes())); assert_eq!(cardholder.name(), Some("Admin<<Hello".as_bytes()));
pgpt.verify_pw1("123456")?; pgpt.verify_pw1(b"123456")?;
match pgpt.check_pw3() { match pgpt.check_pw3() {
Err(Error::CardStatus(s)) => { Err(Error::CardStatus(s)) => {
@ -551,20 +551,20 @@ pub fn test_change_pw(
// first do admin-less pw1 on gnuk // first do admin-less pw1 on gnuk
// (NOTE: Gnuk requires a key to be loaded before allowing pw changes!) // (NOTE: Gnuk requires a key to be loaded before allowing pw changes!)
println!("change pw1"); println!("change pw1");
pgpt.change_pw1("123456", "abcdef00")?; pgpt.change_pw1(b"123456", b"abcdef00")?;
// also set admin pw, which means pw1 is now only user-pw again, on gnuk // also set admin pw, which means pw1 is now only user-pw again, on gnuk
println!("change pw3"); println!("change pw3");
// ca.change_pw3("abcdef00", "abcdefgh")?; // gnuk // ca.change_pw3("abcdef00", "abcdefgh")?; // gnuk
pgpt.change_pw3("12345678", "abcdefgh")?; pgpt.change_pw3(b"12345678", b"abcdefgh")?;
println!("change pw1"); println!("change pw1");
pgpt.change_pw1("abcdef00", "abcdef")?; // gnuk pgpt.change_pw1(b"abcdef00", b"abcdef")?; // gnuk
// ca.change_pw1("123456", "abcdef")?; // ca.change_pw1("123456", "abcdef")?;
println!("verify bad pw1"); println!("verify bad pw1");
match pgpt.verify_pw1("123456ab") { match pgpt.verify_pw1(b"123456ab") {
Err(Error::CardStatus(StatusBytes::SecurityStatusNotSatisfied)) => { Err(Error::CardStatus(StatusBytes::SecurityStatusNotSatisfied)) => {
// this is expected // this is expected
} }
@ -575,10 +575,10 @@ pub fn test_change_pw(
} }
println!("verify good pw1"); println!("verify good pw1");
pgpt.verify_pw1("abcdef")?; pgpt.verify_pw1(b"abcdef")?;
println!("verify bad pw3"); println!("verify bad pw3");
match pgpt.verify_pw3("00000000") { match pgpt.verify_pw3(b"00000000") {
Err(Error::CardStatus(StatusBytes::SecurityStatusNotSatisfied)) => { Err(Error::CardStatus(StatusBytes::SecurityStatusNotSatisfied)) => {
// this is expected // this is expected
} }
@ -589,13 +589,13 @@ pub fn test_change_pw(
} }
println!("verify good pw3"); println!("verify good pw3");
pgpt.verify_pw3("abcdefgh")?; pgpt.verify_pw3(b"abcdefgh")?;
println!("change pw3 back to default"); println!("change pw3 back to default");
pgpt.change_pw3("abcdefgh", "12345678")?; pgpt.change_pw3(b"abcdefgh", b"12345678")?;
println!("change pw1 back to default"); println!("change pw1 back to default");
pgpt.change_pw1("abcdef", "123456")?; pgpt.change_pw1(b"abcdef", b"123456")?;
Ok(out) Ok(out)
} }
@ -611,15 +611,15 @@ pub fn test_reset_retry_counter(
// set pw3, then pw1 (to bring gnuk into non-admin mode) // set pw3, then pw1 (to bring gnuk into non-admin mode)
println!("set pw3"); println!("set pw3");
pgpt.change_pw3("12345678", "12345678")?; pgpt.change_pw3(b"12345678", b"12345678")?;
println!("set pw1"); println!("set pw1");
pgpt.change_pw1("123456", "123456")?; pgpt.change_pw1(b"123456", b"123456")?;
println!("break pw1"); println!("break pw1");
let _ = pgpt.verify_pw1("wrong0"); let _ = pgpt.verify_pw1(b"wrong0");
let _ = pgpt.verify_pw1("wrong0"); let _ = pgpt.verify_pw1(b"wrong0");
let _ = pgpt.verify_pw1("wrong0"); let _ = pgpt.verify_pw1(b"wrong0");
let res = pgpt.verify_pw1("wrong0"); let res = pgpt.verify_pw1(b"wrong0");
match res { match res {
Err(Error::CardStatus(StatusBytes::AuthenticationMethodBlocked)) => { Err(Error::CardStatus(StatusBytes::AuthenticationMethodBlocked)) => {
@ -638,23 +638,20 @@ pub fn test_reset_retry_counter(
} }
println!("verify pw3"); println!("verify pw3");
pgpt.verify_pw3("12345678")?; pgpt.verify_pw3(b"12345678")?;
println!("set resetting code"); println!("set resetting code");
pgpt.set_resetting_code("abcdefgh".as_bytes().to_vec())?; pgpt.set_resetting_code(b"abcdefgh")?;
println!("reset retry counter"); println!("reset retry counter");
// ca.reset_retry_counter_pw1("abcdef".as_bytes().to_vec(), None)?; // ca.reset_retry_counter_pw1("abcdef".as_bytes().to_vec(), None)?;
let _res = pgpt.reset_retry_counter_pw1( let _res = pgpt.reset_retry_counter_pw1(b"abcdef", Some(b"abcdefgh"));
"abcdef".as_bytes().to_vec(),
Some("abcdefgh".as_bytes().to_vec()),
);
println!("verify good pw1"); println!("verify good pw1");
pgpt.verify_pw1("abcdef")?; pgpt.verify_pw1(b"abcdef")?;
println!("verify bad pw1"); println!("verify bad pw1");
match pgpt.verify_pw1("00000000") { match pgpt.verify_pw1(b"00000000") {
Err(Error::CardStatus(StatusBytes::SecurityStatusNotSatisfied)) => { Err(Error::CardStatus(StatusBytes::SecurityStatusNotSatisfied)) => {
// this is expected // this is expected
} }

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

@ -67,7 +67,7 @@ impl<'a> Open<'a> {
} }
pub fn verify_user(&mut self, pin: &str) -> Result<(), Error> { pub fn verify_user(&mut self, pin: &str) -> Result<(), Error> {
let _ = self.opt.verify_pw1(pin)?; let _ = self.opt.verify_pw1(pin.as_bytes())?;
self.pw1 = true; self.pw1 = true;
Ok(()) Ok(())
} }
@ -81,7 +81,7 @@ impl<'a> Open<'a> {
} }
pub fn verify_user_for_signing(&mut self, pin: &str) -> Result<(), Error> { pub fn verify_user_for_signing(&mut self, pin: &str) -> Result<(), Error> {
let _ = self.opt.verify_pw1_for_signing(pin)?; let _ = self.opt.verify_pw1_for_signing(pin.as_bytes())?;
// FIXME: depending on card mode, pw1_sign is only usable once // FIXME: depending on card mode, pw1_sign is only usable once
@ -101,7 +101,7 @@ impl<'a> Open<'a> {
} }
pub fn verify_admin(&mut self, pin: &str) -> Result<(), Error> { pub fn verify_admin(&mut self, pin: &str) -> Result<(), Error> {
let _ = self.opt.verify_pw3(pin)?; let _ = self.opt.verify_pw3(pin.as_bytes())?;
self.pw3 = true; self.pw3 = true;
Ok(()) Ok(())
} }
@ -129,7 +129,7 @@ impl<'a> Open<'a> {
} }
pub fn change_user_pin(&mut self, old: &str, new: &str) -> Result<(), Error> { pub fn change_user_pin(&mut self, old: &str, new: &str) -> Result<(), Error> {
self.opt.change_pw1(old, new) self.opt.change_pw1(old.as_bytes(), new.as_bytes())
} }
pub fn change_user_pin_pinpad(&mut self, prompt: &dyn Fn()) -> Result<(), Error> { pub fn change_user_pin_pinpad(&mut self, prompt: &dyn Fn()) -> Result<(), Error> {
@ -139,11 +139,11 @@ impl<'a> Open<'a> {
pub fn reset_user_pin(&mut self, rst: &str, new: &str) -> Result<(), Error> { pub fn reset_user_pin(&mut self, rst: &str, new: &str) -> Result<(), Error> {
self.opt self.opt
.reset_retry_counter_pw1(new.into(), Some(rst.into())) .reset_retry_counter_pw1(new.as_bytes(), Some(rst.as_bytes()))
} }
pub fn change_admin_pin(&mut self, old: &str, new: &str) -> Result<(), Error> { pub fn change_admin_pin(&mut self, old: &str, new: &str) -> Result<(), Error> {
self.opt.change_pw3(old, new) self.opt.change_pw3(old.as_bytes(), new.as_bytes())
} }
pub fn change_admin_pin_pinpad(&mut self, prompt: &dyn Fn()) -> Result<(), Error> { pub fn change_admin_pin_pinpad(&mut self, prompt: &dyn Fn()) -> Result<(), Error> {
@ -398,11 +398,11 @@ impl Admin<'_, '_> {
} }
pub fn set_resetting_code(&mut self, pin: &str) -> Result<(), Error> { pub fn set_resetting_code(&mut self, pin: &str) -> Result<(), Error> {
self.oc.opt.set_resetting_code(pin.into()) self.oc.opt.set_resetting_code(pin.as_bytes())
} }
pub fn reset_user_pin(&mut self, new: &str) -> Result<(), Error> { pub fn reset_user_pin(&mut self, new: &str) -> Result<(), Error> {
self.oc.opt.reset_retry_counter_pw1(new.into(), None) self.oc.opt.reset_retry_counter_pw1(new.as_bytes(), None)
} }
/// Upload a ValidErasedKeyAmalgamation to the card as a specific KeyType. /// Upload a ValidErasedKeyAmalgamation to the card as a specific KeyType.

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

@ -147,22 +147,20 @@ pub(crate) fn put_cardholder_certificate(data: Vec<u8>) -> Command {
/// "RESET RETRY COUNTER" (PW1, user pin) /// "RESET RETRY COUNTER" (PW1, user pin)
/// Reset the counter of PW1 and set a new pin. /// Reset the counter of PW1 and set a new pin.
pub(crate) fn reset_retry_counter_pw1( pub(crate) fn reset_retry_counter_pw1(resetting_code: Option<&[u8]>, new_pin: &[u8]) -> Command {
resetting_code: Option<Vec<u8>>,
new_pin: Vec<u8>,
) -> Command {
if let Some(resetting_code) = resetting_code { if let Some(resetting_code) = resetting_code {
// Present the Resetting Code (DO D3) in the command data (P1 = 00) // Present the Resetting Code (DO D3) in the command data (P1 = 00)
// Data field: Resetting Code + New PW // Data field: Resetting Code + New PW
let mut data = resetting_code; let mut data = vec![];
data.extend(resetting_code);
data.extend(new_pin); data.extend(new_pin);
Command::new(0x00, 0x2C, 0x00, 0x81, data) Command::new(0x00, 0x2C, 0x00, 0x81, data)
} else { } else {
// Use after correct verification of PW3 (P1 = 02) // Use after correct verification of PW3 (P1 = 02)
// (Usage of secure messaging is equivalent to PW3) // (Usage of secure messaging is equivalent to PW3)
Command::new(0x00, 0x2C, 0x02, 0x81, new_pin) Command::new(0x00, 0x2C, 0x02, 0x81, new_pin.to_vec())
} }
} }

32
openpgp-card/src/openpgp.rs
View file

@ -308,8 +308,8 @@ impl<'a> OpenPgpTransaction<'a> {
/// Depending on the PW1 status byte (see Extended Capabilities) this /// Depending on the PW1 status byte (see Extended Capabilities) this
/// access condition is only valid for one PSO:CDS command or remains /// access condition is only valid for one PSO:CDS command or remains
/// valid for several attempts. /// valid for several attempts.
pub fn verify_pw1_for_signing(&mut self, pin: &str) -> Result<(), Error> { pub fn verify_pw1_for_signing(&mut self, pin: &[u8]) -> Result<(), Error> {
let verify = commands::verify_pw1_81(pin.as_bytes().to_vec()); let verify = commands::verify_pw1_81(pin.to_vec());
apdu::send_command(self.tx(), verify, false)?.try_into() apdu::send_command(self.tx(), verify, false)?.try_into()
} }
@ -340,8 +340,8 @@ impl<'a> OpenPgpTransaction<'a> {
/// Verify PW1 (user). /// Verify PW1 (user).
/// (For operations except signing, mode 82). /// (For operations except signing, mode 82).
pub fn verify_pw1(&mut self, pin: &str) -> Result<(), Error> { pub fn verify_pw1(&mut self, pin: &[u8]) -> Result<(), Error> {
let verify = commands::verify_pw1_82(pin.as_bytes().to_vec()); let verify = commands::verify_pw1_82(pin.to_vec());
apdu::send_command(self.tx(), verify, false)?.try_into() apdu::send_command(self.tx(), verify, false)?.try_into()
} }
@ -369,8 +369,8 @@ impl<'a> OpenPgpTransaction<'a> {
} }
/// Verify PW3 (admin). /// Verify PW3 (admin).
pub fn verify_pw3(&mut self, pin: &str) -> Result<(), Error> { pub fn verify_pw3(&mut self, pin: &[u8]) -> Result<(), Error> {
let verify = commands::verify_pw3(pin.as_bytes().to_vec()); let verify = commands::verify_pw3(pin.to_vec());
apdu::send_command(self.tx(), verify, false)?.try_into() apdu::send_command(self.tx(), verify, false)?.try_into()
} }
@ -397,10 +397,10 @@ impl<'a> OpenPgpTransaction<'a> {
/// Change the value of PW1 (user password). /// Change the value of PW1 (user password).
/// ///
/// The current value of PW1 must be presented in `old` for authorization. /// The current value of PW1 must be presented in `old` for authorization.
pub fn change_pw1(&mut self, old: &str, new: &str) -> Result<(), Error> { pub fn change_pw1(&mut self, old: &[u8], new: &[u8]) -> Result<(), Error> {
let mut data = vec![]; let mut data = vec![];
data.extend(old.as_bytes()); data.extend(old);
data.extend(new.as_bytes()); data.extend(new);
let change = commands::change_pw1(data); let change = commands::change_pw1(data);
apdu::send_command(self.tx(), change, false)?.try_into() apdu::send_command(self.tx(), change, false)?.try_into()
@ -416,10 +416,10 @@ impl<'a> OpenPgpTransaction<'a> {
/// Change the value of PW3 (admin password). /// Change the value of PW3 (admin password).
/// ///
/// The current value of PW3 must be presented in `old` for authorization. /// The current value of PW3 must be presented in `old` for authorization.
pub fn change_pw3(&mut self, old: &str, new: &str) -> Result<(), Error> { pub fn change_pw3(&mut self, old: &[u8], new: &[u8]) -> Result<(), Error> {
let mut data = vec![]; let mut data = vec![];
data.extend(old.as_bytes()); data.extend(old);
data.extend(new.as_bytes()); data.extend(new);
let change = commands::change_pw3(data); let change = commands::change_pw3(data);
apdu::send_command(self.tx(), change, false)?.try_into() apdu::send_command(self.tx(), change, false)?.try_into()
@ -441,8 +441,8 @@ impl<'a> OpenPgpTransaction<'a> {
/// - the resetting_code must be presented. /// - the resetting_code must be presented.
pub fn reset_retry_counter_pw1( pub fn reset_retry_counter_pw1(
&mut self, &mut self,
new_pw1: Vec<u8>, new_pw1: &[u8],
resetting_code: Option<Vec<u8>>, resetting_code: Option<&[u8]>,
) -> Result<(), Error> { ) -> Result<(), Error> {
let reset = commands::reset_retry_counter_pw1(resetting_code, new_pw1); let reset = commands::reset_retry_counter_pw1(resetting_code, new_pw1);
apdu::send_command(self.tx(), reset, false)?.try_into() apdu::send_command(self.tx(), reset, false)?.try_into()
@ -656,8 +656,8 @@ impl<'a> OpenPgpTransaction<'a> {
/// Set resetting code /// Set resetting code
/// (4.3.4 Resetting Code) /// (4.3.4 Resetting Code)
pub fn set_resetting_code(&mut self, resetting_code: Vec<u8>) -> Result<(), Error> { pub fn set_resetting_code(&mut self, resetting_code: &[u8]) -> Result<(), Error> {
let cmd = commands::put_data(&[0xd3], resetting_code); let cmd = commands::put_data(&[0xd3], resetting_code.to_vec());
apdu::send_command(self.tx(), cmd, false)?.try_into() apdu::send_command(self.tx(), cmd, false)?.try_into()
} }