Skip to content

Commit

Permalink
Clippy fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
nickray committed Nov 21, 2021
1 parent cf77987 commit 4a30bef
Show file tree
Hide file tree
Showing 16 changed files with 189 additions and 201 deletions.
2 changes: 1 addition & 1 deletion build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ fn main() {
}

// place side by side with binaries
let outdir = path::PathBuf::from(path::PathBuf::from(env_outdir).ancestors().skip(3).next().unwrap());
let outdir = path::PathBuf::from(path::PathBuf::from(env_outdir).ancestors().nth(3).unwrap());
fs::create_dir_all(&outdir).unwrap();
println!("{:?}", &outdir);

Expand Down
2 changes: 1 addition & 1 deletion src/bin/lpc55/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use clap::{self, crate_authors, crate_version, App, Arg, SubCommand};

// duplicated here, because when using this `cli` module in build.rs
// to generate shell completions, there is no `lpc55` crate yet
pub const KEYSTORE_KEY_NAMES: [&'static str; 6] = [
pub const KEYSTORE_KEY_NAMES: [&str; 6] = [
"secure-boot-kek",
"user-key",
"unique-device-secret",
Expand Down
2 changes: 1 addition & 1 deletion src/bin/lpc55/logger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use log::{self, Log};
#[derive(Debug)]
pub struct Logger(());

const LOGGER: &'static Logger = &Logger(());
const LOGGER: &Logger = &Logger(());

impl Logger {
/// Create a new logger that logs to stderr and initialize it as the
Expand Down
16 changes: 7 additions & 9 deletions src/bin/lpc55/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ fn try_main(args: clap::ArgMatches<'_>) -> anyhow::Result<()> {
if let Some(command) = args.subcommand_matches("http") {
let bootloader = bootloader()?;
let addr = command.value_of("ADDR").unwrap().to_string();
let port = u16::from_str_radix(command.value_of("PORT").unwrap(), 10).unwrap();
let port = command.value_of("PORT").unwrap().parse::<u16>().unwrap();
let http_config = lpc55::http::HttpConfig { addr, port, timeout_ms: 5000 };
let server = lpc55::http::Server::new(&http_config, bootloader)?;
server.run()?;
Expand Down Expand Up @@ -169,13 +169,11 @@ fn try_main(args: clap::ArgMatches<'_>) -> anyhow::Result<()> {
info!("preserving firmware, prince-iv, and reserved fields.");

// Do not overwrite firmware versions
for i in 8 .. 16 {
settings[i] = current_pfr_raw[i];
}
let protect = 8..16;
settings[protect.clone()].clone_from_slice(&current_pfr_raw[protect]);
// Do not overwrite any of the PRINCE IV's or reserved areas.
for i in 48 .. 256 {
settings[i] = current_pfr_raw[i];
}
let protect = 48..256;
settings[protect.clone()].clone_from_slice(&current_pfr_raw[protect]);
}

trace!("writing pfr: {}", hex_str!(&settings));
Expand Down Expand Up @@ -445,8 +443,8 @@ fn try_main(args: clap::ArgMatches<'_>) -> anyhow::Result<()> {
if let Some(product_date) = command.value_of("product-date") {
use chrono::naive::NaiveDate;
let date = NaiveDate::parse_from_str(product_date, "%Y-%m-%d")
.or(NaiveDate::parse_from_str(product_date, "%Y%m%d"))
.or(NaiveDate::parse_from_str(product_date, "%y%m%d"))?;
.or_else(|_| NaiveDate::parse_from_str(product_date, "%Y%m%d"))
.or_else(|_| NaiveDate::parse_from_str(product_date, "%y%m%d"))?;
let days_since_twenties = (date - NaiveDate::from_ymd(2020, 1, 1)).num_days();
assert!(days_since_twenties > 0);
info!("overriding product.major with date {}, i.e. {}", &date, days_since_twenties);
Expand Down
5 changes: 2 additions & 3 deletions src/bootloader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,8 @@ impl Bootloader {
})
.filter_map(|(device, vid, pid)| {
let protocol = Protocol::new(device);
let bootloader = GetProperties { protocol: &protocol }.device_uuid().ok()
.map(|uuid| Self { protocol, vid, pid, uuid });
bootloader
GetProperties { protocol: &protocol }.device_uuid().ok()
.map(|uuid| Self { protocol, vid, pid, uuid })
})
.collect()
}
Expand Down
11 changes: 4 additions & 7 deletions src/bootloader/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,7 @@ pub enum DataPhase {

impl DataPhase {
pub fn has_command_data(&self) -> bool {
match self {
DataPhase::CommandData(_) => true,
_ => false,
}
matches!(self, DataPhase::CommandData(_))
}
}

Expand All @@ -93,8 +90,8 @@ impl Command {
let mut bytes = Vec::with_capacity(words.len() * 4);
let cursor = &mut bytes;

for i in 0 .. words.len() {
cursor.write_all(&words[i].to_le_bytes()).unwrap();
for word in words.iter() {
cursor.write_all(&word.to_le_bytes()).unwrap();
}

DataPhase::CommandData(bytes)
Expand Down Expand Up @@ -483,7 +480,7 @@ pub enum Key {
}

// unfortunately duplicated in cli.rs, for why see there
pub const KEYSTORE_KEY_NAMES: [&'static str; 6] = [
pub const KEYSTORE_KEY_NAMES: [&str; 6] = [
"secure-boot-kek",
"user-key",
"unique-device-secret",
Expand Down
13 changes: 6 additions & 7 deletions src/bootloader/error.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! https://github.com/NXPmicro/spsdk/blob/020a983e53769fe16cb9b49395d56f0201eccca6/spsdk/mboot/error_codes.py
//! <https://github.com/NXPmicro/spsdk/blob/020a983e53769fe16cb9b49395d56f0201eccca6/spsdk/mboot/error_codes.py>

use core::convert::TryFrom;

Expand Down Expand Up @@ -125,10 +125,10 @@ generate! { CrcCheckerError:
OutOfRange = 4,
}

impl Into<(ErrorGroup, u8)> for BootloaderError {
fn into(self) -> (ErrorGroup, u8) {
impl From<BootloaderError> for (ErrorGroup, u8) {
fn from(error: BootloaderError) -> Self {
use BootloaderError::*;
match self {
match error {
Generic(error) => (ErrorGroup::Generic, error as u8),
FlashDriver(error) => (ErrorGroup::FlashDriver, error as u8),
PropertyStore(error) => (ErrorGroup::PropertyStore, error as u8),
Expand All @@ -142,8 +142,7 @@ impl Into<(ErrorGroup, u8)> for BootloaderError {
impl From<BootloaderError> for u32 {
fn from(error: BootloaderError) -> u32 {
let (group, code) = error.into();
let status = (group as u32 * 100) + code as u32;
status
(group as u32 * 100) + code as u32
}
}

Expand All @@ -158,7 +157,7 @@ impl From<u32> for BootloaderError {
(ErrorGroup::PropertyStore, code) => PropertyStoreError::try_from(code).map_or(Unknown(status), PropertyStore),
(ErrorGroup::CrcChecker, code) => CrcCheckerError::try_from(code).map_or(Unknown(status), CrcChecker),
(ErrorGroup::SbLoader, code) => SbLoaderError::try_from(code).map_or(Unknown(status), SbLoader),
_ => return Unknown(status),
_ => Unknown(status),
}
} else {
Unknown(status)
Expand Down
2 changes: 1 addition & 1 deletion src/bootloader/property.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ impl GetProperties<'_> {
((values[3] as u128) << 96) +
((values[2] as u128) << 64) +
((values[1] as u128) << 32) +
((values[0] as u128))
(values[0] as u128)
// ((u32::from_le_bytes(values[0].to_be_bytes()) as u128) << 96) +
// ((u32::from_le_bytes(values[1].to_be_bytes()) as u128) << 64) +
// ((u32::from_le_bytes(values[2].to_be_bytes()) as u128) << 32) +
Expand Down
14 changes: 7 additions & 7 deletions src/bootloader/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ impl Protocol {
// successful status, mirroring our command header
let packet = ResponsePacket::try_from(initial_response)?;

assert_eq!(packet.has_data, false);
assert!(!packet.has_data);
if let Some(status) = packet.status {
panic!("{:?}", status);
}
Expand Down Expand Up @@ -196,7 +196,7 @@ impl Protocol {
}

let packet = ResponsePacket::try_from(self.read_packet()?)?;
assert_eq!(packet.has_data, false);
assert!(!packet.has_data);
if let Some(status) = packet.status {
panic!("unexpected status {:?}", &status);
}
Expand Down Expand Up @@ -226,7 +226,7 @@ impl Protocol {
}

let packet = ResponsePacket::try_from(self.read_packet()?)?;
assert_eq!(packet.has_data, false);
assert!(!packet.has_data);
if let Some(status) = packet.status {
panic!("unexpected status {:?}", &status);
}
Expand Down Expand Up @@ -255,7 +255,7 @@ impl Protocol {
x => x?,
})?;
// let packet = ResponsePacket::try_from(self.read_packet()?)?;
assert_eq!(packet.has_data, false);
assert!(!packet.has_data);
if let Some(status) = packet.status {
panic!("unexpected status {:?}", &status);
}
Expand Down Expand Up @@ -292,7 +292,7 @@ impl Protocol {

let packet = ResponsePacket::try_from(initial_response)?;
// assert_eq!([0x03, 0x00, 0x0C, 0x00], &initial_generic_response[..4]);
assert_eq!(packet.has_data, true);
assert!(!packet.has_data);

This comment has been minimized.

Copy link
@gcochard

gcochard Apr 27, 2022

Contributor

This is a bug. It should be assert!(packet.has_data); since you were comparing with true before. This commit breaks all read/write against the bootloader.

assert!(packet.status.is_none());
assert_eq!(packet.tag, command::ResponseTag::ReadMemory);

Expand All @@ -309,7 +309,7 @@ impl Protocol {
}

let packet = ResponsePacket::try_from(self.read_packet()?)?;
assert_eq!(packet.has_data, false);
assert!(!packet.has_data);
assert!(packet.status.is_none());

assert_eq!(packet.tag, command::ResponseTag::Generic);
Expand Down Expand Up @@ -395,7 +395,7 @@ impl Protocol {
if sent >= all {
Ok(())
} else {
Err(hidapi::HidError::IncompleteSendError { sent, all })?
Err(hidapi::HidError::IncompleteSendError { sent, all }.into())
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ pub fn nxp_aes_ctr_cipher(ciphertext: &[u8], dek: [u8; 32], nonce: [u32; 4], off
let nonce3_offset = offset_blocks + (i as u32);
let mut nonce2 = [0u8; 16];
nonce2[..4].copy_from_slice(nonce[0].to_le_bytes().as_ref());
nonce2[4..8].copy_from_slice(&nonce[1].to_le_bytes().as_ref());
nonce2[8..12].copy_from_slice(&nonce[2].to_le_bytes().as_ref());
nonce2[12..16].copy_from_slice(&(nonce[3] + nonce3_offset).to_le_bytes().as_ref());
nonce2[4..8].copy_from_slice(nonce[1].to_le_bytes().as_ref());
nonce2[8..12].copy_from_slice(nonce[2].to_le_bytes().as_ref());
nonce2[12..16].copy_from_slice((nonce[3] + nonce3_offset).to_le_bytes().as_ref());
let mut cipher = Aes256Ctr::new(dek.as_ref().into(), nonce2.as_ref().into());
cipher.apply_keystream(chunk);
}
Expand Down
2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
//!
//! There is also the somehow underadvertised <https://github.com/NXPmicro/spsdk>.

// for readability
#![allow(clippy::identity_op)]

#[macro_use]
extern crate log;
Expand Down
13 changes: 5 additions & 8 deletions src/pki.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,7 @@ impl SigningKey {
let mut hasher = sha2::Sha256::new();
hasher.update(data);
let hashed_data = hasher.finalize();
let signature = key.sign(padding_scheme, &hashed_data).expect("signatures work");
signature
key.sign(padding_scheme, &hashed_data).expect("signatures work")
}
Pkcs11Uri(uri) => {
let (context, session, object) = uri.identify_object().unwrap();
Expand All @@ -169,8 +168,7 @@ impl SigningKey {

// now do a signature, assuming this is an RSA key
context.sign_init(session, &mechanism, object).unwrap();
let signature = context.sign(session, data).unwrap();
signature
context.sign(session, data).unwrap()
}
};
assert_eq!(256, signature.len());
Expand Down Expand Up @@ -209,8 +207,7 @@ impl SigningKey {
// https://github.com/mheese/rust-pkcs11/issues/44
let n = rsa::BigUint::from_bytes_be(&n.to_bytes_le());
let e = rsa::BigUint::from_bytes_be(&e.to_bytes_le());
let public_key = rsa::RsaPublicKey::new(n, e).unwrap();
public_key
rsa::RsaPublicKey::new(n, e).unwrap()
}
})
}
Expand Down Expand Up @@ -347,7 +344,7 @@ impl Certificate {
// let OID_RSA_ENCRYPTION = oid!(1.2.840.113549.1.1.1);
assert_eq!(oid_registry::OID_PKCS1_RSAENCRYPTION, spki.algorithm.algorithm);

let public_key = PublicKey(rsa::RsaPublicKey::from_pkcs1_der(&spki.subject_public_key.data)?);
let public_key = PublicKey(rsa::RsaPublicKey::from_pkcs1_der(spki.subject_public_key.data)?);
Ok(public_key.fingerprint())
}

Expand All @@ -363,7 +360,7 @@ impl Certificate {
pub fn public_key(&self) -> PublicKey {
let spki = self.certificate().tbs_certificate.subject_pki;
assert_eq!(oid_registry::OID_PKCS1_RSAENCRYPTION, spki.algorithm.algorithm);
PublicKey(rsa::RsaPublicKey::from_pkcs1_der(&spki.subject_public_key.data).unwrap())
PublicKey(rsa::RsaPublicKey::from_pkcs1_der(spki.subject_public_key.data).unwrap())
}

pub fn fingerprint(&self) -> Sha256Hash {
Expand Down
6 changes: 3 additions & 3 deletions src/protected_flash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ impl From<u8> for BootSpeed {
0b00 => Nxp,
0b01 => Fro96,
0b10 => Fro48,
0b11 | _ => Reserved,
/*0b11 |*/ _ => Reserved,
}
}
}
Expand Down Expand Up @@ -713,8 +713,8 @@ pub struct SecureBootConfiguration {
pub trustzone_mode: TrustzoneMode,
#[serde(default)]
#[serde(skip_serializing_if = "is_default")]
/// For this DICE stuff, see also https://www.microsoft.com/en-us/research/project/dice-device-identifier-composition-engine/
/// and the actual standard https://trustedcomputinggroup.org/resource/hardware-requirements-for-a-device-identifier-composition-engine/
/// For this DICE stuff, see also <https://www.microsoft.com/en-us/research/project/dice-device-identifier-composition-engine/>
/// and the actual standard <https://trustedcomputinggroup.org/resource/hardware-requirements-for-a-device-identifier-composition-engine/>
pub dice_computation_disabled: bool,
#[serde(default)]
#[serde(skip_serializing_if = "is_default")]
Expand Down
Loading

0 comments on commit 4a30bef

Please sign in to comment.