Security enhancements

.github/copilot-instructions.md

@@ -0,0 +1,29 @@
+## Pull Request Review Checklist
+
+- [ ] Code is completely panic-free (no unwrap/expect/panic/indexing)
+- [ ] All fallible operations return Result or Option
+- [ ] Integer operations use checked/saturating/wrapping methods where needed
+- [ ] Array/slice access uses get() or pattern matching, not direct indexing
+- [ ] Error cases are well documented and handled appropriately
+- [ ] Tests verify error handling paths, not just happy paths
+- [ ] Unsafe code blocks are documented with safety comments
+- [ ] Hardware register access uses proper volatile operations
+- [ ] Cryptographic operations use constant-time implementations where applicable
+
+## Quick Reference: Forbidden Patterns
+
+| Forbidden Pattern | Required Alternative |
+|-------------------|----------------------|
+| `value.unwrap()` | `match value { Some(v) => v, None => return Err(...) }` |
+| `result.expect("msg")` | `match result { Ok(v) => v, Err(e) => return Err(e.into()) }` |
+| `collection[index]` | `collection.get(index).ok_or(Error::OutOfBounds)?` |
+| `a + b` (integers) | `a.checked_add(b).ok_or(Error::Overflow)?` |
+| `ptr.read()` | `ptr.read_volatile()` (for MMIO) |
+
+## Security-Specific Guidelines
+
+- **Timing attacks**: Use constant-time comparisons for secrets (subtle crate)
+- **Zeroization**: Use `zeroize` crate for sensitive data cleanup (keys, passwords, etc.)
+- **Memory safety**: Ensure sensitive data is properly zeroized after use
+- **Hardware abstraction**: All register access must go through HAL traits
+- **Error information**: Don't leak sensitive data in error messages

.github/workflows/ci.yml

@@ -89,9 +89,45 @@ jobs:
 
     - name: Install Rust toolchain
       uses: dtolnay/rust-toolchain@stable
+      with:
+        components: clippy
+
+    - name: Cache cargo registry
+      uses: actions/cache@v4
+      with:
+        path: |
+          ~/.cargo/registry
+          ~/.cargo/git
+          target
+        key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
 
     - name: Install cargo-audit
-      run: cargo install cargo-audit
+      run: cargo install cargo-audit --locked
+
+    - name: Install cargo-deny
+      run: cargo install cargo-deny --version 0.18.3 --locked
 
     - name: Run security audit
       run: cargo audit
+
+    - name: Run cargo deny checks
+      run: cargo xtask deny
+
+    - name: Run security-focused clippy lints
+      run: |
+        cargo clippy --all-targets --all-features -- \
+          -D warnings \
+          -W clippy::arithmetic_side_effects \
+          -W clippy::float_arithmetic \
+          -W clippy::indexing_slicing \
+          -W clippy::unwrap_used \
+          -W clippy::expect_used \
+          -W clippy::panic \
+          -W clippy::mem_forget \
+          -W clippy::multiple_unsafe_ops_per_block \
+          -W clippy::undocumented_unsafe_blocks
+
+    - name: Run semgrep security scan
+      uses: returntocorp/semgrep-action@v1
+      with:
+        config: p/rust

.semgrepignore

@@ -0,0 +1,10 @@
+# Licensed under the Apache-2.0 license
+
+# Ignore xtask development tools - not production code
+xtask/
+
+# Ignore build artifacts
+target/
+
+# Ignore external dependencies and examples
+external/

Cargo.toml

@@ -17,5 +17,12 @@ members = [
 ]
 resolver = "2"
 
+[workspace.package]
+license = "Apache-2.0"
+repository = "https://github.com/rusty1968/openprot"
+edition = "2021"
+
 [workspace.dependencies]
 zerocopy = { version = "0.8.25", features = ["derive"] }
+zeroize = { version = "1.7", features = ["derive"] }
+subtle = "2.5"

deny.toml

@@ -0,0 +1,32 @@
+# Licensed under the Apache-2.0 license
+
+# cargo-deny configuration file
+# Minimal configuration for OpenPRoT security checks
+
+[advisories]
+version = 2
+ignore = []
+
+[licenses]
+version = 2
+allow = [
+    "MIT",
+    "Apache-2.0", 
+    "Apache-2.0 WITH LLVM-exception",
+    "BSD-2-Clause",
+    "BSD-3-Clause",
+    "ISC",
+    "Unicode-DFS-2016",
+    "Unicode-3.0",
+    "CC0-1.0",
+]
+
+[bans]
+multiple-versions = "warn"
+wildcards = "allow"
+deny = []
+
+[sources]
+unknown-registry = "warn"
+unknown-git = "warn"
+allow-registry = ["https://github.com/rust-lang/crates.io-index"]

hal/blocking/Cargo.toml

@@ -9,4 +9,6 @@ license = "Apache-2.0"
 
 [dependencies]
 embedded-hal = "1.0"
-zerocopy = { workspace = true }
+zerocopy = { workspace = true }
+zeroize = { workspace = true }
+subtle = { workspace = true }

hal/blocking/src/mac.rs

@@ -2,9 +2,87 @@
 
 use crate::digest::Digest;
 use core::fmt::Debug;
+use subtle::ConstantTimeEq;
 use zerocopy::IntoBytes;
+use zeroize::{Zeroize, ZeroizeOnDrop};
 
-/// Common error kinds for MAC operations (reused from digest operations).
+/// Secure wrapper for MAC keys that automatically zeros on drop.
+///
+/// This wrapper ensures that cryptographic keys are securely erased from memory
+/// when no longer needed, preventing key material from remaining in memory.
+#[derive(Clone, Zeroize, ZeroizeOnDrop)]
+pub struct SecureKey<const N: usize> {
+    /// The actual key bytes, zeroized on drop
+    bytes: [u8; N],
+}
+
+impl<const N: usize> SecureKey<N> {
+    /// Create a new secure key from a byte array.
+    ///
+    /// # Security
+    /// The input array will be zeroized after copying to prevent key material
+    /// from remaining in multiple memory locations.
+    pub fn new(mut key_bytes: [u8; N]) -> Self {
+        let key = Self { bytes: key_bytes };
+        key_bytes.zeroize();
+        key
+    }
+
+    /// Create a new secure key from a byte slice.
+    ///
+    /// # Returns
+    /// - `Ok(SecureKey)` if the slice length matches the key size
+    /// - `Err(ErrorKind::InvalidInputLength)` if the slice is the wrong size
+    pub fn from_slice(key_slice: &[u8]) -> Result<Self, ErrorKind> {
+        if key_slice.len() != N {
+            return Err(ErrorKind::InvalidInputLength);
+        }
+
+        let mut key_bytes = [0u8; N];
+        key_bytes.copy_from_slice(key_slice);
+        Ok(Self::new(key_bytes))
+    }
+
+    /// Get a reference to the key bytes.
+    ///
+    /// # Security
+    /// Use this sparingly and ensure the returned reference doesn't outlive
+    /// the SecureKey instance.
+    pub fn as_bytes(&self) -> &[u8; N] {
+        &self.bytes
+    }
+
+    /// Verify a MAC tag using constant-time comparison.
+    ///
+    /// # Security
+    /// This function uses constant-time comparison to prevent timing attacks
+    /// that could reveal information about the expected MAC value.
+    pub fn verify_mac(&self, computed_mac: &[u8], expected_mac: &[u8]) -> bool {
+        if computed_mac.len() != expected_mac.len() {
+            return false;
+        }
+        computed_mac.ct_eq(expected_mac).into()
+    }
+}
+
+impl<const N: usize> Debug for SecureKey<N> {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        f.debug_struct("SecureKey")
+            .field("len", &N)
+            .field("bytes", &"[REDACTED]")
+            .finish()
+    }
+}
+
+impl<const N: usize> PartialEq for SecureKey<N> {
+    fn eq(&self, other: &Self) -> bool {
+        self.bytes.ct_eq(&other.bytes).into()
+    }
+}
+
+impl<const N: usize> Eq for SecureKey<N> {}
+
+/// Common error kinds for MAC operations.
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
 #[non_exhaustive]
 pub enum ErrorKind {
@@ -30,6 +108,8 @@ pub enum ErrorKind {
     PermissionDenied,
     /// The MAC computation context has not been initialized.
     NotInitialized,
+    /// MAC verification failed - computed MAC does not match expected value.
+    VerificationFailed,
 }
 
 /// Trait for converting implementation-specific errors into a common error kind.
@@ -116,6 +196,31 @@ pub trait MacOp: ErrorType {
     fn finalize(self) -> Result<Self::Output, Self::Error>;
 }
 
+/// Utility function for constant-time MAC verification.
+///
+/// This function provides a secure way to verify MAC values using constant-time
+/// comparison to prevent timing attacks.
+///
+/// # Parameters
+///
+/// - `computed_mac`: The computed MAC bytes
+/// - `expected_mac`: The expected MAC bytes to verify against
+///
+/// # Returns
+///
+/// `true` if the MACs match, `false` otherwise
+///
+/// # Security
+///
+/// This function uses constant-time comparison to prevent timing attacks
+/// that could reveal information about the expected MAC value.
+pub fn verify_mac_constant_time(computed_mac: &[u8], expected_mac: &[u8]) -> bool {
+    if computed_mac.len() != expected_mac.len() {
+        return false;
+    }
+    computed_mac.ct_eq(expected_mac).into()
+}
+
 // =============================================================================
 // MAC Algorithm Marker Types
 // =============================================================================
@@ -133,7 +238,7 @@ pub struct HmacSha2_256;
 impl MacAlgorithm for HmacSha2_256 {
     const OUTPUT_BITS: usize = 256;
     type MacOutput = Digest<{ Self::OUTPUT_BITS / 32 }>;
-    type Key = [u8; 32];
+    type Key = SecureKey<32>;
 }
 
 /// HMAC-SHA-384 MAC algorithm marker type.
@@ -149,7 +254,7 @@ pub struct HmacSha2_384;
 impl MacAlgorithm for HmacSha2_384 {
     const OUTPUT_BITS: usize = 384;
     type MacOutput = Digest<{ Self::OUTPUT_BITS / 32 }>;
-    type Key = [u8; 48];
+    type Key = SecureKey<48>;
 }
 
 /// HMAC-SHA-512 MAC algorithm marker type.
@@ -165,5 +270,5 @@ pub struct HmacSha2_512;
 impl MacAlgorithm for HmacSha2_512 {
     const OUTPUT_BITS: usize = 512;
     type MacOutput = Digest<{ Self::OUTPUT_BITS / 32 }>;
-    type Key = [u8; 64];
+    type Key = SecureKey<64>;
 }

openprot/Cargo.toml

@@ -3,7 +3,9 @@
 [package]
 name = "openprot"
 version = "0.1.0"
-edition = "2021"
+edition.workspace = true
+license.workspace = true
+repository.workspace = true
 
 [[bin]]
 name = "openprot"

openprot/src/lib.rs

@@ -1,7 +1,7 @@
 // Licensed under the Apache-2.0 license
 
 pub fn greet(name: &str) -> String {
-    format!("Hello, {}!", name)
+    format!("Hello, {name}!")
 }
 
 #[cfg(test)]

platform/impls/baremetal/mock/Cargo.toml

@@ -3,7 +3,9 @@
 [package]
 name = "openprot-platform-mock"
 version = "0.1.0"
-edition = "2021"
+edition.workspace = true
+license.workspace = true
+repository.workspace = true
 description = "Mock/stub implementation of OpenPRoT platform traits for testing"
 
 [features]

platform/impls/baremetal/mock/src/hash.rs

@@ -9,6 +9,11 @@
 //! - **Scoped API**: Traditional lifetime-constrained contexts for simple use cases
 //! - **Owned API**: Move-based resource management for server applications
 
+// Allow security lints for mock/test code
+#![allow(clippy::unwrap_used)]
+#![allow(clippy::expect_used)]
+#![allow(clippy::arithmetic_side_effects)]
+
 use openprot_hal_blocking::digest::{
     DigestAlgorithm, ErrorKind, ErrorType, Sha2_256, Sha2_384, Sha2_512,
 };

platform/impls/baremetal/mock/src/lib.rs

@@ -6,6 +6,10 @@
 //! for testing and development purposes when real hardware is not available.
 
 #![no_std]
+// Allow security lints for mock/test code
+#![allow(clippy::unwrap_used)]
+#![allow(clippy::expect_used)]
+#![allow(clippy::arithmetic_side_effects)]
 
 pub mod hash;
 pub mod mac;