Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:

steps:
- name: Set up build cache
uses: actions/cache@v2
uses: actions/cache@v4
with:
path: ${{ env.BUILDX_CACHE }}
key: ${{ runner.os }}-buildx-${{ github.sha }}
Expand Down
2 changes: 1 addition & 1 deletion .rustfmt.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
edition = "2018"
edition = "2024"

### stable features ###

Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "secrets"
version = "1.2.0"
edition = "2018"
edition = "2024"

authors = ["Stephen Touset <stephen@touset.org>"]
description = "Protected-access memory for cryptographic secrets"
Expand Down
3 changes: 2 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ FROM rust:latest
# * >= 1.38 for std::ptr::cast
# * >= 1.40 for cfg(doctest) (in a dependency)
# * >= 1.51 for const generics
# * >= 1.85 for Rust 2024 edition
ARG TOOLCHAIN
ENV TOOLCHAIN=${TOOLCHAIN:-1.51}
ENV TOOLCHAIN=${TOOLCHAIN:-1.85}

RUN --mount=type=cache,target=/var/cache/apt \
apt-get update && \
Expand Down
7 changes: 4 additions & 3 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@ impl Profile {
}
};

println!("cargo:rerun-if-env-changed=COVERAGE");
println!("cargo:rerun-if-env-changed=PROFILE");
println!("cargo:rustc-cfg=profile=\"{}\"", profile);
println!("cargo::rerun-if-env-changed=COVERAGE");
println!("cargo::rerun-if-env-changed=PROFILE");
println!("cargo::rustc-check-cfg=cfg(profile, values(\"coverage\", \"debug\", \"release\"))");
println!("cargo::rustc-cfg=profile=\"{}\"", profile);

profile
}
Expand Down
10 changes: 5 additions & 5 deletions src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl<T: Bytes> Box<T> {
{
let mut boxed = Self::new_unlocked(len);

proven!(boxed.ptr != std::ptr::NonNull::dangling());
proven!(boxed.ptr != NonNull::dangling());
proven!(boxed.len == len);

init(&mut boxed);
Expand All @@ -93,7 +93,7 @@ impl<T: Bytes> Box<T> {
{
let mut boxed = Self::new_unlocked(len);

proven!(boxed.ptr != std::ptr::NonNull::dangling());
proven!(boxed.ptr != NonNull::dangling());
proven!(boxed.len == len);

let result = init(&mut boxed);
Expand Down Expand Up @@ -249,7 +249,7 @@ impl<T: Bytes> Box<T> {
/// related panic.
fn new_unlocked(len: usize) -> Self {
tested!(len == 0);
tested!(std::mem::size_of::<T>() == 0);
tested!(size_of::<T>() == 0);

assert!(sodium::init(), "secrets: failed to initialize libsodium");

Expand Down Expand Up @@ -332,7 +332,7 @@ impl<T: Bytes> Box<T> {
Some(v) => self.refs.set(v),
None if self.is_locked() => panic!("secrets: out-of-order retain/release detected"),
None => panic!("secrets: retained too many times"),
};
}
}

/// Removes one outsdanding retain, and changes the memory
Expand Down Expand Up @@ -485,7 +485,7 @@ fn mprotect<T>(ptr: *mut T, prot: Prot) {
Prot::ReadOnly => unsafe { sodium::mprotect_readonly(ptr) },
Prot::ReadWrite => unsafe { sodium::mprotect_readwrite(ptr) },
} {
panic!("secrets: error setting memory protection to {:?}", prot);
panic!("secrets: error setting memory protection to {prot:?}");
}
}

Expand Down
25 changes: 13 additions & 12 deletions src/ffi/sodium.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

#![allow(unsafe_code)]

use std::mem;
use std::sync::Once;

use libc::{self, size_t};
Expand Down Expand Up @@ -31,7 +30,7 @@ thread_local! {
}

#[cfg(not(feature = "use-libsodium-sys"))]
extern "C" {
unsafe extern "C" {
fn sodium_init() -> c_int;

fn sodium_allocarray(count: size_t, size: size_t) -> *mut c_void;
Expand Down Expand Up @@ -111,30 +110,30 @@ pub(crate) fn init() -> bool {
/// fills that memory with garbage bytes. Callers must ensure that they
/// call [`sodium::free`] when this memory is no longer used.
pub(crate) unsafe fn allocarray<T>(count: usize) -> *mut T {
sodium_allocarray(count, mem::size_of::<T>()).cast()
unsafe { sodium_allocarray(count, size_of::<T>()).cast() }
}

/// Releases memory acquired with [`sodium::allocarray`]. This function
/// may panic if it detects that certain soundness and safety guarantees
/// have been violated (e.g., an underflowing write).
pub(crate) unsafe fn free<T>(ptr: *mut T) {
sodium_free(ptr.cast());
unsafe { sodium_free(ptr.cast()) };
}

/// Calls the platform's underlying `mlock(2)` implementation.
pub(crate) unsafe fn mlock<T>(ptr: *mut T) -> bool {
#[cfg(test)]
{ if FAIL.with(|f| f.replace(false)) { return false }; let _x = 0; };

sodium_mlock(ptr.cast(), mem::size_of::<T>()) == 0
unsafe { sodium_mlock(ptr.cast(), size_of::<T>()) == 0 }
}

/// Calls the platform's underlying `munlock(2)` implementation.
pub(crate) unsafe fn munlock<T>(ptr: *mut T) -> bool {
#[cfg(test)]
{ if FAIL.with(|f| f.replace(false)) { return false }; let _x = 0; };

sodium_munlock(ptr.cast(), mem::size_of::<T>()) == 0
unsafe { sodium_munlock(ptr.cast(), size_of::<T>()) == 0 }
}

/// Sets the page protection level of [`sodium::allocarray`]-allocated
Expand All @@ -145,7 +144,7 @@ pub(crate) unsafe fn mprotect_noaccess<T>(ptr: *mut T) -> bool {
#[cfg(test)]
{ if FAIL.with(|f| f.replace(false)) { return false }; let _x = 0; };

sodium_mprotect_noaccess(ptr.cast()) == 0
unsafe { sodium_mprotect_noaccess(ptr.cast()) == 0 }
}

/// Sets the page protection level of [`sodium::allocarray`]-allocated
Expand All @@ -156,7 +155,7 @@ pub(crate) unsafe fn mprotect_readonly<T>(ptr: *mut T) -> bool {
#[cfg(test)]
{ if FAIL.with(|f| f.replace(false)) { return false }; let _x = 0; };

sodium_mprotect_readonly(ptr.cast()) == 0
unsafe { sodium_mprotect_readonly(ptr.cast()) == 0 }
}

/// Sets the page protection level of [`sodium::allocarray`]-allocated
Expand All @@ -167,7 +166,7 @@ pub(crate) unsafe fn mprotect_readwrite<T>(ptr: *mut T) -> bool {
#[cfg(test)]
{ if FAIL.with(|f| f.replace(false)) { return false }; let _x = 0; };

sodium_mprotect_readwrite(ptr.cast()) == 0
unsafe { sodium_mprotect_readwrite(ptr.cast()) == 0 }
}

/// Compares `l` and `r` for equality in constant time, preventing
Expand Down Expand Up @@ -202,12 +201,14 @@ pub(crate) unsafe fn memtransfer(src: &mut [u8], dst: &mut [u8]) {
// this is proven since in safe rust, two mutable slices may not
// overlap with one-another
proven!(
(src.as_ptr() < dst.as_ptr() && src.as_ptr().add(src.len()) <= dst.as_ptr()) ||
(dst.as_ptr() < src.as_ptr() && dst.as_ptr().add(dst.len()) <= src.as_ptr()),
unsafe {
(src.as_ptr() < dst.as_ptr() && src.as_ptr().add(src.len()) <= dst.as_ptr()) ||
(dst.as_ptr() < src.as_ptr() && dst.as_ptr().add(dst.len()) <= src.as_ptr())
},
"secrets: may not transfer overlapping slices into one-another"
);

src.as_ptr().copy_to_nonoverlapping(dst.as_mut_ptr(), src.len());
unsafe { src.as_ptr().copy_to_nonoverlapping(dst.as_mut_ptr(), src.len()) };
memzero(src);
}

Expand Down
55 changes: 27 additions & 28 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@
#![warn(rust_2018_compatibility)]
#![warn(rust_2018_idioms)]
#![warn(rust_2021_compatibility)]
#![warn(rust_2024_compatibility)]
#![warn(unused)]

#![warn(bare_trait_objects)]
Expand All @@ -157,42 +158,40 @@
#![warn(unsafe_code)]
#![warn(variant_size_differences)]

#![cfg_attr(feature = "cargo-clippy", warn(clippy::all))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::pedantic))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::nursery))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::clone_on_ref_ptr))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::decimal_literal_representation))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::else_if_without_else))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::float_arithmetic))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::float_cmp_const))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::indexing_slicing))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::mem_forget))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::missing_docs_in_private_items))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::multiple_inherent_impl))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::multiple_inherent_impl))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::print_stdout))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::unwrap_used))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::shadow_reuse))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::shadow_same))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::unimplemented))]
#![cfg_attr(feature = "cargo-clippy", warn(clippy::use_debug))]

#![cfg_attr(feature = "cargo-clippy", allow(clippy::module_name_repetitions))]
#![cfg_attr(feature = "cargo-clippy", allow(clippy::must_use_candidate))]

// disabled due to https://github.com/rust-lang/rust-clippy/issues/5369
#![cfg_attr(feature = "cargo-clippy", allow(clippy::redundant_pub_crate))]

// disabled due to https://github.com/rust-lang/rust/issues/69952
#![cfg_attr(feature = "cargo-clippy", allow(clippy::wildcard_imports))]
#![warn(clippy::all)]
#![warn(clippy::pedantic)]
#![warn(clippy::nursery)]
#![warn(clippy::clone_on_ref_ptr)]
#![warn(clippy::decimal_literal_representation)]
#![warn(clippy::else_if_without_else)]
#![warn(clippy::float_arithmetic)]
#![warn(clippy::float_cmp_const)]
#![warn(clippy::indexing_slicing)]
#![warn(clippy::mem_forget)]
#![warn(clippy::missing_docs_in_private_items)]
#![warn(clippy::multiple_inherent_impl)]
#![warn(clippy::print_stdout)]
#![warn(clippy::unwrap_used)]
#![warn(clippy::shadow_reuse)]
#![warn(clippy::shadow_same)]
#![warn(clippy::unimplemented)]
#![warn(clippy::use_debug)]

#![allow(clippy::module_name_repetitions)]
#![allow(clippy::must_use_candidate)]
#![allow(clippy::redundant_pub_crate)]
#![allow(clippy::too_long_first_doc_paragraph)]
#![allow(clippy::wildcard_imports)]

/// Macros for ensuring code correctness inspired by [sqlite].
///
/// [sqlite]: https://www.sqlite.org/assert.html
#[cfg(profile = "debug")]
#[macro_use]
mod assert {
// Some of these macros aren't used yet, but we still want them!
#![allow(unused_macros)]
#![allow(unused_macro_rules)]

/// Results in an `assert!` in debug builds but is a no-op in
/// coverage and release builds, since we have extraordinarily high
Expand Down
16 changes: 8 additions & 8 deletions src/secret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,19 +112,19 @@ impl<T: Bytes> Secret<T> {
/// File::open("/dev/urandom")?.read_exact(&mut s[..])
/// });
/// ```
#[cfg_attr(feature = "cargo-clippy", allow(clippy::new_ret_no_self))]
#[allow(clippy::new_ret_no_self)]
pub fn new<F, U>(f: F) -> U
where
F: FnOnce(RefMut<'_, T>) -> U,
{
tested!(std::mem::size_of::<T>() == 0);
tested!(size_of::<T>() == 0);

let mut secret = Self {
data: T::uninitialized(),
};

assert!(
unsafe { sodium::mlock(&mut secret.data) },
unsafe { sodium::mlock(&raw mut secret.data) },
"secrets: unable to mlock memory for a Secret"
);

Expand Down Expand Up @@ -207,22 +207,22 @@ impl<T: Bytes> Drop for Secret<T> {
// contains the memory. If two locked items were on the same page, then the second one
// fails because it was already unlocked. On Linux, this does now throw an error. On
// Windows, it does. We'll ignore it for now, and provide a better fix later.
if unsafe { !sodium::munlock(&mut self.data) }
if unsafe { !sodium::munlock(&raw mut self.data) }
&& !(cfg!(target_family = "windows")
&& std::io::Error::last_os_error().raw_os_error().map_or(false, |c| c == 158)) {
&& (std::io::Error::last_os_error().raw_os_error() == Some(158))) {
// [`Drop::drop`] is called during stack unwinding, so we
// may be in a panic already.
assert!(
thread::panicking(),
"secrets: unable to munlock memory for a Secret"
);
};
}
}
}

impl<'a, T: Bytes> RefMut<'a, T> {
/// Instantiates a new `RefMut`.
pub(crate) fn new(data: &'a mut T) -> Self {
pub(crate) const fn new(data: &'a mut T) -> Self {
Self { data }
}
}
Expand Down Expand Up @@ -353,7 +353,7 @@ mod tests {
#[test]
#[should_panic(expected = "secrets: a Secret may not be cloned")]
fn it_panics_when_cloned() {
#[cfg_attr(feature = "cargo-clippy", allow(clippy::redundant_clone))]
#[allow(clippy::redundant_clone)]
Secret::<u16>::zero(|s| {
let _ = s.clone();
});
Expand Down
2 changes: 1 addition & 1 deletion src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@

// `clippy` currently warns when trait functions could be `const fn`s, but this
// is not actually allowed by the language
#![cfg_attr(feature = "cargo-clippy", allow(clippy::missing_const_for_fn))]
#![allow(clippy::missing_const_for_fn)]

/// Traits for types that are considered buckets of bytes.
mod bytes;
Expand Down
10 changes: 5 additions & 5 deletions src/traits/bytes.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::mem::{self, MaybeUninit};
use std::mem::MaybeUninit;
use std::slice;

/// Marker value for uninitialized data.
Expand Down Expand Up @@ -29,7 +29,7 @@ const GARBAGE_VALUE: u8 = 0xdb;
pub unsafe trait Bytes: Sized + Copy {
/// Returns an uninitialized value.
///
/// Note that this is *not* the same as [`mem::uninitialized`].
/// Note that this is *not* the same as [`std::mem::uninitialized`].
/// Values returned by this function are guaranteed to be set to a
/// well-defined bit pattern, though this function makes no
/// guarantees to what specific bit pattern will be used. The bit
Expand All @@ -46,19 +46,19 @@ pub unsafe trait Bytes: Sized + Copy {

/// Returns the size in bytes of `Self`.
fn size() -> usize {
mem::size_of::<Self>()
size_of::<Self>()
}

/// Returns a `*const u8` pointer to the beginning of the data.
#[allow(trivial_casts)] // the cast is actually required
fn as_u8_ptr(&self) -> *const u8 {
(self as *const Self).cast()
std::ptr::from_ref::<Self>(self).cast()
}

/// Returns a `*mut u8` pointer to the beginning of the data.
#[allow(trivial_casts)] // the cast is actually required
fn as_mut_u8_ptr(&mut self) -> *mut u8 {
(self as *mut Self).cast()
std::ptr::from_mut::<Self>(self).cast()
}
}

Expand Down
10 changes: 6 additions & 4 deletions src/traits/zeroable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,12 @@ pub unsafe trait Zeroable: AsContiguousBytes {
/// `other` must be at least as large as `self` and the two
/// references must not overlap.
unsafe fn transfer(&mut self, other: &mut Self) {
sodium::memtransfer(
self .as_mut_bytes(),
other.as_mut_bytes(),
);
unsafe {
sodium::memtransfer(
self .as_mut_bytes(),
other.as_mut_bytes(),
);
};
}
}

Expand Down
Loading