Skip to content
Prev Previous commit
Next Next commit
Implement Pin change
  • Loading branch information
sosthene-nitrokey committed Mar 3, 2023
commit 7fbfaef7be086569cad9d45e4e351b1c860ca0f0
15 changes: 15 additions & 0 deletions src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,21 @@ impl<P: Platform> ExtensionImpl<AuthExtension, P> for AuthBackend {
Ok(reply::GetPinKey { result: None }.into())
}
}
AuthRequest::ChangePin(request) => {
let success = PinData::load(fs, self.location, request.id)?.write(
fs,
self.location,
|data| {
data.change_pin(
&request.old_pin,
&request.new_pin,
move |rng| self.get_app_key(client_id, trussed_fs, ctx, rng),
rng,
)
},
)??;
Ok(reply::ChangePin { success }.into())
}
AuthRequest::SetPin(request) => {
let maybe_app_key = if request.derive_key {
Some(self.get_app_key(client_id, trussed_fs, ctx, rng)?)
Expand Down
83 changes: 78 additions & 5 deletions src/backend/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,13 @@ pub(crate) struct PinDataMut<'a> {

enum CheckResult {
Validated,
Derived(Key),
Derived { k: Key, app_key: Key },
Failed,
}

impl CheckResult {
fn is_success(&self) -> bool {
matches!(self, CheckResult::Validated | CheckResult::Derived(_))
matches!(self, CheckResult::Validated | CheckResult::Derived { .. })
}
}

Expand Down Expand Up @@ -178,8 +178,9 @@ impl<'a> PinDataMut<'a> {
}
}
KeyOrHash::Key { wrapped_key, tag } => {
if let Some(k) = self.unwrap_key(pin, &application_key()?, wrapped_key, &tag) {
CheckResult::Derived(k)
let app_key = application_key()?;
if let Some(k) = self.unwrap_key(pin, &app_key, wrapped_key, &tag) {
CheckResult::Derived { k, app_key }
} else {
CheckResult::Failed
}
Expand Down Expand Up @@ -235,10 +236,82 @@ impl<'a> PinDataMut<'a> {
pub fn get_pin_key(&mut self, pin: &Pin, application_key: &Key) -> Result<Option<Key>, Error> {
match self.check_or_unwrap(pin, || Ok(*application_key))? {
CheckResult::Validated => Err(Error::BadPinType),
CheckResult::Derived(k) => Ok(Some(k)),
CheckResult::Derived { k, .. } => Ok(Some(k)),
CheckResult::Failed => Ok(None),
}
}

fn new_normal_pin<R: CryptoRng + RngCore>(
&mut self,
new: &Pin,
rng: &mut R,
) -> Result<(), Error> {
*self.data = PinData::new(
self.data.id,
new,
self.data.retries.map(|r| r.max),
rng,
None,
);
Ok(())
}

fn new_wrapping_pin<R: CryptoRng + RngCore>(
&mut self,
new: &Pin,
mut old_key: Key,
application_key: &Key,
rng: &mut R,
) {
use chacha20poly1305::{AeadInPlace, KeyInit};
let mut salt = Salt::default();
rng.fill_bytes(&mut *salt);

let pin_key = derive_key(self.id, new, &salt, application_key);

let aead = ChaCha8Poly1305::new((&*pin_key).into());
// The pin key is only ever used to once to wrap a key. Nonce reuse is not a concern
// Because the salt is also used in the key derivation process, PIN reuse across PINs will still lead to different keys
let nonce = Default::default();

#[allow(clippy::expect_used)]
let tag: [u8; CHACHA_TAG_LEN] = aead
.encrypt_in_place_detached(&nonce, &[u8::from(self.id)], &mut *old_key)
.expect("Wrapping the key should always work, length are acceptable")
.into();

*self.data = PinData {
id: self.id,
retries: self.retries,
salt,
data: KeyOrHash::Key {
wrapped_key: old_key,
tag: tag.into(),
},
};
}

pub fn change_pin<R: CryptoRng + RngCore>(
&mut self,
old_pin: &Pin,
new_pin: &Pin,
application_key: impl FnOnce(&mut R) -> Result<Key, Error>,
rng: &mut R,
) -> Result<bool, Error> {
match self.check_or_unwrap(old_pin, || application_key(rng))? {
CheckResult::Validated => {
self.new_normal_pin(new_pin, rng)?;
self.modified = true;
Ok(true)
}
CheckResult::Derived { k, app_key } => {
self.new_wrapping_pin(new_pin, k, &app_key, rng);
self.modified = true;
Ok(true)
}
CheckResult::Failed => Ok(false),
}
}
}

impl Deref for PinDataMut<'_> {
Expand Down
18 changes: 18 additions & 0 deletions src/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ pub enum AuthRequest {
CheckPin(request::CheckPin),
GetPinKey(request::GetPinKey),
SetPin(request::SetPin),
ChangePin(request::ChangePin),
DeletePin(request::DeletePin),
DeleteAllPins(request::DeleteAllPins),
PinRetries(request::PinRetries),
Expand All @@ -44,6 +45,7 @@ pub enum AuthReply {
CheckPin(reply::CheckPin),
GetPinKey(reply::GetPinKey),
SetPin(reply::SetPin),
ChangePin(reply::ChangePin),
DeletePin(reply::DeletePin),
DeleteAllPins(reply::DeleteAllPins),
PinRetries(reply::PinRetries),
Expand Down Expand Up @@ -112,6 +114,22 @@ pub trait AuthClient: ExtensionClient<AuthExtension> {
})
}

/// Change the given PIN and resets its retry counter.
///
/// The key obtained by [`get_pin_key`](AuthClient::get_pin_key) will stay the same
fn change_pin<I: Into<PinId>>(
&mut self,
id: I,
old_pin: Pin,
new_pin: Pin,
) -> AuthResult<'_, reply::ChangePin, Self> {
self.extension(request::ChangePin {
id: id.into(),
old_pin,
new_pin,
})
}

/// Deletes the given PIN (if it exists).
fn delete_pin<I: Into<PinId>>(&mut self, id: I) -> AuthResult<'_, reply::DeletePin, Self> {
self.extension(request::DeletePin { id: id.into() })
Expand Down
22 changes: 22 additions & 0 deletions src/extension/reply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,28 @@ impl TryFrom<AuthReply> for SetPin {
}
}

#[derive(Debug, Deserialize, Serialize)]
pub struct ChangePin {
pub success: bool,
}

impl From<ChangePin> for AuthReply {
fn from(reply: ChangePin) -> Self {
Self::ChangePin(reply)
}
}

impl TryFrom<AuthReply> for ChangePin {
type Error = Error;

fn try_from(reply: AuthReply) -> Result<Self> {
match reply {
AuthReply::ChangePin(reply) => Ok(reply),
_ => Err(Error::InternalError),
}
}
}

#[derive(Debug, Deserialize, Serialize)]
pub struct DeletePin;

Expand Down
13 changes: 13 additions & 0 deletions src/extension/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,19 @@ impl From<SetPin> for AuthRequest {
}
}

#[derive(Debug, Deserialize, Serialize)]
pub struct ChangePin {
pub id: PinId,
pub old_pin: Pin,
pub new_pin: Pin,
}

impl From<ChangePin> for AuthRequest {
fn from(request: ChangePin) -> Self {
Self::ChangePin(request)
}
}

#[derive(Debug, Deserialize, Serialize)]
pub struct DeletePin {
pub id: PinId,
Expand Down
16 changes: 12 additions & 4 deletions tests/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,11 +346,19 @@ fn pin_key() {
let mac2 = syscall!(client.sign_hmacsha256(key2, b"Some data")).signature;
assert_eq!(mac, mac2);

assert!(!syscall!(client.check_pin(Pin::User, pin2.clone())).success);
assert!(!syscall!(client.check_pin(Pin::User, pin2.clone())).success);
assert!(!syscall!(client.check_pin(Pin::User, pin2.clone())).success);
assert!(syscall!(client.change_pin(Pin::User, pin1.clone(), pin2.clone())).success);

let key3 = syscall!(client.get_pin_key(Pin::User, pin2.clone()))
.result
.unwrap();
let mac3 = syscall!(client.sign_hmacsha256(key3, b"Some data")).signature;
assert_eq!(mac, mac3);

assert!(!syscall!(client.check_pin(Pin::User, pin1.clone())).success);
assert!(!syscall!(client.check_pin(Pin::User, pin1.clone())).success);
assert!(!syscall!(client.check_pin(Pin::User, pin1.clone())).success);
assert!(syscall!(client.get_pin_key(Pin::User, pin1.clone()))
assert!(!syscall!(client.check_pin(Pin::User, pin2.clone())).success);
assert!(syscall!(client.get_pin_key(Pin::User, pin2.clone()))
.result
.is_none());
assert_eq!(syscall!(client.pin_retries(Pin::User)).retries, Some(0));
Expand Down