handle surplus options

Cargo.toml

@@ -1,13 +1,12 @@
 [package]
 name = "ctap_hmac"
 description = "A Rust implementation of the FIDO2 CTAP protocol, including the HMAC extension"
-version = "0.4.5"
+version = "0.3.0"
 license = "Apache-2.0/MIT"
-homepage = "https://github.com/shimunn/ctap"
+homepage = "https://github.com/ArdaXi/ctap/pull/2"
 repository = "https://github.com/shimunn/ctap"
 authors = ["Arda Xi <arda@ardaxi.com>", "shimun <shimun@shimun.net>"]
 edition = "2018"
-readme = "README.md"
 
 [dependencies]
 rand = "0.6"
@@ -20,16 +19,12 @@ cbor-codec = "0.7"
 ring = "0.13"
 untrusted = "0.6"
 rust-crypto = "0.2"
+csv-core = "0.1.6"
 derive_builder = "0.9.0"
 crossbeam = { version = "0.7.3", optional = true }
 [dev-dependencies]
 crossbeam = "0.7.3"
 hex = "0.4.0"
-[build-dependencies]
-csv = "1.1.3"
-serde = "1.0.106"
-serde_derive = "1.0.106"
-
 
 [features]
 request_multiple = ["crossbeam"]

build.rs

@@ -1,41 +0,0 @@
-use csv::{Reader, StringRecord};
-use serde_derive::Deserialize;
-use std::env;
-use std::fs::File;
-use std::io::{Result, Write};
-use std::iter::FromIterator;
-use std::string::String;
-
-fn main() {
-    parse_error_codes().expect("Failed to parse error codes")
-}
-
-fn parse_error_codes() -> Result<()> {
-    println!("cargo:rerun-if-changed=ctap_error_codes.csv");
-    let mut out_file = File::create(format!(
-        "{}/ctap_error_codes.rs",
-        env::var("OUT_DIR").unwrap()
-    ))?;
-    out_file.write_all(b"static CTAP_ERROR_CODES: &[(usize, &str, &str)] = &[")?;
-    let mut rdr = Reader::from_path("ctap_error_codes.csv")?;
-    rdr.set_headers(StringRecord::from_iter(&["code", "name", "desc"]));
-    #[derive(Debug, Deserialize)]
-    struct ErrorCode {
-        code: String,
-        name: String,
-        desc: String,
-    }
-    for result in rdr.deserialize() {
-        let record: ErrorCode = result.unwrap();
-        out_file.write_all(
-            format!(
-                "({}, \"{}\", \"{}\"),\n",
-                i64::from_str_radix(&record.code[2..], 16).unwrap(),
-                record.name,
-                record.desc
-            )
-            .as_bytes(),
-        )?;
-    }
-    out_file.write_all(b"];")
-}

examples/hmac.rs

@@ -4,6 +4,7 @@ use crypto::digest::Digest;
 use crypto::sha2::Sha256;
 use ctap::extensions::hmac::HmacExtension;
 use ctap::{FidoAssertionRequestBuilder, FidoCredential, FidoCredentialRequestBuilder};
+
 use hex;
 use std::env::args;
 use std::io::prelude::*;

examples/multiple.rs

@@ -1,4 +1,5 @@
 extern crate ctap_hmac as ctap;
+
 use ctap::{
     FidoAssertionRequestBuilder, FidoCredential, FidoCredentialRequestBuilder, FidoDevice,
     FidoResult,
@@ -6,7 +7,6 @@ use ctap::{
 
 use hex;
 use std::env::args;
-use std::time::Duration;
 
 const RP_ID: &str = "ctap_demo";
 
@@ -24,7 +24,6 @@ fn main() -> ctap::FidoResult<()> {
                 FidoDevice::new(&h).and_then(|mut dev| {
                     FidoCredentialRequestBuilder::default()
                         .rp_id(RP_ID)
-                        .user_name("test")
                         .build()
                         .unwrap()
                         .make_credential(&mut dev)
@@ -42,8 +41,7 @@ fn main() -> ctap::FidoResult<()> {
         .map(|handle| FidoDevice::new(&handle))
         .collect::<FidoResult<Vec<_>>>()?;
     // run with --features request_multiple
-    let (cred, _) =
-        ctap::get_assertion_devices(&req, devices.iter_mut(), Some(Duration::from_secs(10)))?;
+    let (cred, _) = ctap::get_assertion_devices(&req, devices.iter_mut())?;
     println!("Success, got assertion for: {}", hex::encode(&cred.id));
     Ok(())
 }

src/cbor.rs

@@ -7,7 +7,6 @@
 use cbor_codec::value;
 use cbor_codec::value::Value;
 use cbor_codec::{Config, Decoder, Encoder, GenericDecoder, GenericEncoder};
-use cbor::skip::Skip;
 
 use byteorder::{BigEndian, ByteOrder, ReadBytesExt, WriteBytesExt};
 use failure::ResultExt;
@@ -39,7 +38,7 @@ impl<'a> Request<'a> {
         }
     }
 
-    pub fn decode<R: ReadBytesExt + Skip>(&self, reader: R) -> FidoResult<Response> {
+    pub fn decode<R: ReadBytesExt>(&self, reader: R) -> FidoResult<Response> {
         Ok(match self {
             Request::MakeCredential(_) => {
                 Response::MakeCredential(MakeCredentialResponse::decode(reader)?)
@@ -278,7 +277,7 @@ pub struct GetInfoResponse {
 }
 
 impl GetInfoResponse {
-    pub fn decode<R: ReadBytesExt + Skip>(mut reader: R) -> FidoResult<Self> {
+    pub fn decode<R: ReadBytesExt>(mut reader: R) -> FidoResult<Self> {
         let status = reader.read_u8().context(FidoErrorKind::CborDecode)?;
         if status != 0 {
             Err(FidoErrorKind::CborError(CborErrorCode::from(status)))?
@@ -305,7 +304,7 @@ impl GetInfoResponse {
                         response.pin_protocols.push(decoder.u8()?);
                     }
                 }
-                _ => decoder.skip()?,
+                _ => continue,
             }
         }
         Ok(response)
@@ -598,45 +597,14 @@ impl CoseKey {
         let mut cose_key = CoseKey::default();
         cose_key.algorithm = -7;
         for _ in 0..items {
-            match generic.value()? {
-                Value::Text(value::Text::Text(text)) => match &text[..] {
-                    "type" => {
-                        cose_key.key_type = match generic.value()? {
-                            Value::Text(value::Text::Text(type_)) if &type_ == "public-key" => 0u16,
-                            Value::U16(i) => i,
-                            Value::U8(i) => i.into(),
-                            _ => {
-                                continue;
-                            }
-                        }
-                    }
-                    "alg" => cose_key.algorithm = generic.borrow_mut().i32()?,
-                    _ => continue,
-                },
-                val @ Value::I8(_)
-                | val @ Value::I16(_)
-                | val @ Value::U16(_)
-                | val @ Value::U8(_) => {
-                    let int_val = match val {
-                        Value::I8(i) => i as i32,
-                        Value::I16(i) => i as i32,
-                        Value::U8(i) => i as i32,
-                        Value::U16(i) => i as i32,
-                        _ => unreachable!(),
-                    };
-                    match int_val {
-                        0x01 => cose_key.key_type = generic.borrow_mut().u16()?,
-                        0x02 => cose_key.algorithm = generic.borrow_mut().i32()?,
-                        key if key < 0 => {
-                            cose_key.parameters.insert(key as i16, generic.value()?);
-                        }
-                        unknown => {
-                            (unknown, generic.value()?); // skip unknown parameter
-                        }
-                    }
+            match generic.borrow_mut().i16()? {
+                0x01 => cose_key.key_type = generic.borrow_mut().u16()?,
+                0x02 => cose_key.algorithm = generic.borrow_mut().i32()?,
+                key if key < 0 => {
+                    cose_key.parameters.insert(key, generic.value()?);
                 }
-                unknown => {
-                    (unknown, generic.value()?); // skip unknown parameter
+                _ => {
+                    generic.value()?; // skip unknown parameter
                 }
             }
         }

src/error.rs

@@ -5,8 +5,8 @@
 // http://opensource.org/licenses/MIT>, at your option. This file may not be
 // copied, modified, or distributed except according to those terms.
 use cbor_codec::{DecodeError, EncodeError};
+use csv_core::{ReadFieldResult, Reader};
 use failure::_core::fmt::{Error, Formatter};
-use failure::_core::option::Option;
 use failure::{Backtrace, Context, Fail};
 use std::fmt;
 use std::fmt::Display;
@@ -19,38 +19,10 @@ pub struct FidoError(Context<FidoErrorKind>);
 #[derive(Debug, Copy, Clone, Fail, Eq, PartialEq)]
 pub struct CborErrorCode(u8);
 
-// generated using build.rs
-include!(concat!(env!("OUT_DIR"), "/ctap_error_codes.rs"));
-
-impl CborErrorCode {
-    fn detail(&self) -> Option<(u8, &'static str, &'static str)> {
-        for (code, name, desc) in CTAP_ERROR_CODES {
-            if *code == self.0 as usize {
-                return Some((self.0, name, desc));
-            }
-        }
-        None
-    }
-
-    pub fn name(&self) -> Option<&'static str> {
-        self.detail().map(|(_, name, _)| name)
-    }
-
-    pub fn description(&self) -> Option<&'static str> {
-        self.detail().map(|(_, _, desc)| desc)
-    }
-
-    pub fn code(&self) -> u8 {
-        self.0
-    }
-}
-
 #[derive(Copy, Clone, Eq, PartialEq, Debug, Fail)]
 pub enum FidoErrorKind {
     #[fail(display = "Read/write error with device.")]
     Io,
-    #[fail(display = "Operation timed out")]
-    Timeout,
     #[fail(display = "Error while reading packet from device.")]
     ReadPacket,
     #[fail(display = "Error while writing packet to device.")]
@@ -75,7 +47,7 @@ pub enum FidoErrorKind {
     DecryptPin,
     #[fail(display = "Failed to verify response signature.")]
     VerifySignature,
-    #[fail(display = "Failed to verify response signature.")]
+    #[fail(display = "Supplied key has incorrect type.")]
     KeyType,
     #[fail(display = "Device returned error: {}", _0)]
     CborError(CborErrorCode),
@@ -142,24 +114,58 @@ impl From<u8> for CborErrorCode {
 
 impl Display for CborErrorCode {
     fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> {
-        if let Some((code, _name, desc)) = self.detail() {
+        let messages = include_str!("ctap_error_codes.csv");
+        let mut rdr = Reader::new();
+        let mut bytes = messages.as_bytes();
+        let mut col: usize = 0;
+        let mut row: usize = 0;
+        let mut correct_row: bool = false;
+        let mut field = [0u8; 1024];
+        let mut name: Option<String> = None;
+        let mut desc: Option<String> = None;
+        loop {
+            let (result, nin, read) = rdr.read_field(&bytes, &mut field);
+            bytes = &bytes[nin..];
+            match result {
+                ReadFieldResult::InputEmpty => {}
+                ReadFieldResult::OutputFull => panic!("field too large"),
+                ReadFieldResult::Field { record_end } => {
+                    let text = String::from_utf8(field[..read].iter().cloned().collect()).unwrap();
+                    if row > 0 {
+                        match col {
+                            0 if i64::from_str_radix(&text[2..], 16)
+                                .expect("malformed ctap_error_codes.csv")
+                                == self.0 as i64 =>
+                            {
+                                correct_row = true
+                            }
+                            1 | 2 if correct_row => {
+                                if let Some(_) = name {
+                                    desc = Some(text);
+                                    break;
+                                } else {
+                                    name = Some(text);
+                                }
+                            }
+                            _ => (),
+                        }
+                    }
+                    col += 1;
+                    if record_end {
+                        col = 0;
+                        row += 1;
+                    }
+                }
+                ReadFieldResult::End => break,
+            }
+        }
+        if let Some((code, _name, desc)) =
+            name.and_then(|name| desc.map(|desc| (self.0, name, desc)))
+        {
             write!(f, "CborError: 0x{:x?}: {}", code, desc)?;
         } else {
-            write!(f, "CborError: 0x{:x?}: unknown", self.code())?;
+            write!(f, "CborError: 0x{:x?}", self.0)?;
         }
         Ok(())
     }
 }
-
-#[cfg(test)]
-mod test {
-    use super::*;
-
-    #[test]
-    fn cbor_error_code() {
-        assert_eq!(
-            CborErrorCode(0x33).to_string(),
-            "CborError: 0x33: PIN authentication, pinAuth, verification failed."
-        )
-    }
-}

src/extensions/hmac.rs

@@ -1,4 +1,6 @@
-use crate::{FidoAssertionRequest, FidoAssertionRequestBuilder, FidoCredentialRequest};
+use crate::{
+    FidoAssertionRequest, FidoAssertionRequestBuilder, FidoCredentialRequest,
+};
 use crate::{FidoCredential, FidoDevice, FidoErrorKind, FidoResult};
 use cbor_codec::value::{Bytes, Int, Key, Text, Value};
 use cbor_codec::Encoder;
@@ -11,6 +13,7 @@ use rust_crypto::sha2::Sha256;
 use std::collections::BTreeMap;
 use std::io::Cursor;
 
+
 pub trait HmacExtension {
     fn extension_name() -> &'static str {
         "hmac-secret"

src/lib.rs

@@ -287,14 +287,14 @@ impl FidoDevice {
             cbor::Response::GetInfo(resp) => resp,
             _ => Err(FidoErrorKind::CborDecode)?,
         };
-        if !response.versions.iter().any(|ver| ["FIDO_2_0", "FIDO_2_1_PRE"].contains(&ver.as_str())) {
+        if !response.versions.iter().any(|ver| ver == "FIDO_2_0") {
             Err(FidoErrorKind::DeviceUnsupported)?
         }
         // Require pin protocol version 1, only if pin-protocol is supported at all
         if !response
             .pin_protocols
             .iter()
-            .any(|ver| *ver == 1) && response.pin_protocols.len() > 0
+            .fold(true, |supported, ver| *ver == 1 && supported)
         {
             Err(FidoErrorKind::DeviceUnsupported)?
         }
@@ -325,11 +325,6 @@ impl FidoDevice {
         }
     }
 
-    /// True if this authenticator requires a PIN
-    pub fn needs_pin(&self) -> bool {
-        self.needs_pin
-    }
-
     /// Unlock the device with the provided PIN. Internally this will generate
     /// an ECDH keypair, send the encrypted PIN to the device and store the PIN
     /// token that the device generates on every power cycle. The PIN itself is
@@ -462,6 +457,7 @@ impl FidoDevice {
     ///
     /// This method will fail if a PIN is required but the device is not
     /// unlocked or if the device returns malformed data.
+
     pub fn get_assertion<'a, 'b>(
         &mut self,
         assertion: &FidoAssertionRequest<'a, 'b>,

src/util.rs

@@ -7,8 +7,7 @@ use crate::{
 use crossbeam::thread;
 #[cfg(feature = "request_multiple")]
 use std::sync::mpsc::channel;
-#[cfg(feature = "request_multiple")]
-use std::time::Duration;
+
 #[cfg(feature = "request_multiple")]
 pub fn request_multiple_devices<
     'a,
@@ -16,7 +15,6 @@ pub fn request_multiple_devices<
     F: Fn(&mut FidoDevice) -> FidoResult<T> + 'a + Sync,
 >(
     devices: impl Iterator<Item = (&'a mut FidoDevice, &'a F)>,
-    timeout: Option<Duration>,
 ) -> FidoResult<T> {
     thread::scope(|scope| -> FidoResult<T> {
         let (tx, rx) = channel();
@@ -28,39 +26,22 @@ pub fn request_multiple_devices<
                 Ok((cancel, thread_handle))
             })
             .collect::<FidoResult<Vec<_>>>()?;
+
         let mut err = None;
-        let mut slept = Duration::from_millis(0);
-        let interval = Duration::from_millis(10);
-        let mut received = 0usize;
-        let res = loop {
-            match timeout {
-                Some(t) if t < slept => {
-                    break if let Some(cause) = err {
-                        cause
-                    } else {
-                        Err(FidoErrorKind::Timeout.into())
-                    };
+        for res in rx.iter().take(handles.len()) {
+            match res {
+                Ok(_) => {
+                    for (mut cancel, join) in handles {
+                        // Canceling out of courtesy don't care if it fails
+                        let _ = cancel.cancel();
+                        let _ = join.join();
+                    }
+                    return res;
                 }
-                _ => (),
+                e => err = Some(e),
             }
-            if timeout.map(|t| t < slept).unwrap_or(true) {}
-            if let Ok(msg) = rx.recv_timeout(interval) {
-                received += 1;
-                match msg {
-                    e @ Err(_) if received == handles.len() => break e,
-                    e @ Err(_) => err = Some(e),
-                    res @ Ok(_) => break res,
-                }
-            } else {
-                slept += interval;
-            }
-        };
-        for (mut cancel, join) in handles {
-            // Canceling out of courtesy don't care if it fails
-            let _ = cancel.cancel();
-            let _ = join.join();
         }
-        res
+        err.unwrap_or(Err(FidoErrorKind::DeviceUnsupported.into()))
     })
     .unwrap()
 }
@@ -70,10 +51,9 @@ pub fn request_multiple_devices<
 pub fn get_assertion_devices<'a>(
     assertion_request: &'a FidoAssertionRequest,
     devices: impl Iterator<Item = &'a mut FidoDevice>,
-    timeout: Option<Duration>,
 ) -> FidoResult<(&'a FidoCredential, AuthenticatorData)> {
     let get_assertion = |device: &mut FidoDevice| device.get_assertion(assertion_request);
-    request_multiple_devices(devices.map(|device| (device, &get_assertion)), timeout)
+    request_multiple_devices(devices.map(|device| (device, &get_assertion)))
 }
 
 /// Will send the `credential_request` to all supplied `devices` and return either the first credential or the last error
@@ -81,8 +61,7 @@ pub fn get_assertion_devices<'a>(
 pub fn make_credential_devices<'a>(
     credential_request: &'a FidoCredentialRequest,
     devices: impl Iterator<Item = &'a mut FidoDevice>,
-    timeout: Option<Duration>,
 ) -> FidoResult<FidoCredential> {
     let make_credential = |device: &mut FidoDevice| device.make_credential(credential_request);
-    request_multiple_devices(devices.map(|device| (device, &make_credential)), timeout)
+    request_multiple_devices(devices.map(|device| (device, &make_credential)))
 }