| Message ID | 20231206-alice-file-v2-3-af617c0d9d94@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | File abstractions needed by Rust Binder | expand |
On 12/6/23 12:59, Alice Ryhl wrote: > +impl SecurityCtx { > + /// Get the security context given its id. > + pub fn from_secid(secid: u32) -> Result<Self> { > + let mut secdata = core::ptr::null_mut(); > + let mut seclen = 0u32; > + // SAFETY: Just a C FFI call. The pointers are valid for writes. > + unsafe { > + to_result(bindings::security_secid_to_secctx( > + secid, > + &mut secdata, > + &mut seclen, > + ))?; > + } Can you move the `unsafe` block inside of the `to_result` call? That way we only have the unsafe operation in the unsafe block. Additionally, on my side it fits perfectly into 100 characters. > + // INVARIANT: If the above call did not fail, then we have a valid security context. > + Ok(Self { > + secdata, > + seclen: seclen as usize, > + }) > + } [...] > + /// Returns the bytes for this security context. > + pub fn as_bytes(&self) -> &[u8] { > + let ptr = self.secdata; > + if ptr.is_null() { > + // We can't pass a null pointer to `slice::from_raw_parts` even if the length is zero. > + debug_assert_eq!(self.seclen, 0); Would this be interesting enough to emit some kind of log message when this fails? > + return &[]; > + } > + > + // SAFETY: The call to `security_secid_to_secctx` guarantees that the pointer is valid for > + // `seclen` bytes. Furthermore, if the length is zero, then we have ensured that the > + // pointer is not null. > + unsafe { core::slice::from_raw_parts(ptr.cast(), self.seclen) } > + } > +} > + > +impl Drop for SecurityCtx { > + fn drop(&mut self) { > + // SAFETY: This frees a pointer that came from a successful call to > + // `security_secid_to_secctx` and has not yet been destroyed by `security_release_secctx`. > + unsafe { > + bindings::security_release_secctx(self.secdata, self.seclen as u32); > + } If you move the `;` to the outside of the `unsafe` block this also fits on a single line.
Benno Lossin <benno.lossin@proton.me> writes: > On 12/6/23 12:59, Alice Ryhl wrote: > > +impl SecurityCtx { > > + /// Get the security context given its id. > > + pub fn from_secid(secid: u32) -> Result<Self> { > > + let mut secdata = core::ptr::null_mut(); > > + let mut seclen = 0u32; > > + // SAFETY: Just a C FFI call. The pointers are valid for writes. > > + unsafe { > > + to_result(bindings::security_secid_to_secctx( > > + secid, > > + &mut secdata, > > + &mut seclen, > > + ))?; > > + } > > Can you move the `unsafe` block inside of the `to_result` call? That way > we only have the unsafe operation in the unsafe block. Additionally, on > my side it fits perfectly into 100 characters. Will do. > > + /// Returns the bytes for this security context. > > + pub fn as_bytes(&self) -> &[u8] { > > + let ptr = self.secdata; > > + if ptr.is_null() { > > + // We can't pass a null pointer to `slice::from_raw_parts` even if the length is zero. > > + debug_assert_eq!(self.seclen, 0); > > Would this be interesting enough to emit some kind of log message when > this fails? I'm not convinced that makes sense. I'm pretty sure that if this API returns a null pointer under any circumstances, then we're in some sort of context where security contexts don't exist at all, and then they would be hard-coded to use a length zero as well. > > + return &[]; > > + } > > + > > + // SAFETY: The call to `security_secid_to_secctx` guarantees that the pointer is valid for > > + // `seclen` bytes. Furthermore, if the length is zero, then we have ensured that the > > + // pointer is not null. > > + unsafe { core::slice::from_raw_parts(ptr.cast(), self.seclen) } > > + } > > +} > > + > > +impl Drop for SecurityCtx { > > + fn drop(&mut self) { > > + // SAFETY: This frees a pointer that came from a successful call to > > + // `security_secid_to_secctx` and has not yet been destroyed by `security_release_secctx`. > > + unsafe { > > + bindings::security_release_secctx(self.secdata, self.seclen as u32); > > + } > > If you move the `;` to the outside of the `unsafe` block this also fits > on a single line. Will do. Alice
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index 6d1bd2229aab..81b13a953eae 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -11,6 +11,7 @@ #include <linux/errname.h> #include <linux/file.h> #include <linux/fs.h> +#include <linux/security.h> #include <linux/slab.h> #include <linux/refcount.h> #include <linux/wait.h> diff --git a/rust/helpers.c b/rust/helpers.c index 10ed69f76424..fd633d9db79a 100644 --- a/rust/helpers.c +++ b/rust/helpers.c @@ -30,6 +30,7 @@ #include <linux/mutex.h> #include <linux/refcount.h> #include <linux/sched/signal.h> +#include <linux/security.h> #include <linux/spinlock.h> #include <linux/wait.h> #include <linux/workqueue.h> @@ -177,6 +178,26 @@ void rust_helper_put_cred(const struct cred *cred) } EXPORT_SYMBOL_GPL(rust_helper_put_cred); +#ifndef CONFIG_SECURITY +void rust_helper_security_cred_getsecid(const struct cred *c, u32 *secid) +{ + security_cred_getsecid(c, secid); +} +EXPORT_SYMBOL_GPL(rust_helper_security_cred_getsecid); + +int rust_helper_security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen) +{ + return security_secid_to_secctx(secid, secdata, seclen); +} +EXPORT_SYMBOL_GPL(rust_helper_security_secid_to_secctx); + +void rust_helper_security_release_secctx(char *secdata, u32 seclen) +{ + security_release_secctx(secdata, seclen); +} +EXPORT_SYMBOL_GPL(rust_helper_security_release_secctx); +#endif + /* * `bindgen` binds the C `size_t` type as the Rust `usize` type, so we can * use it in contexts where Rust expects a `usize` like slice (array) indices. diff --git a/rust/kernel/cred.rs b/rust/kernel/cred.rs index 497058ec89bb..3794937b5294 100644 --- a/rust/kernel/cred.rs +++ b/rust/kernel/cred.rs @@ -43,6 +43,14 @@ pub unsafe fn from_ptr<'a>(ptr: *const bindings::cred) -> &'a Credential { unsafe { &*ptr.cast() } } + /// Get the id for this security context. + pub fn get_secid(&self) -> u32 { + let mut secid = 0; + // SAFETY: The invariants of this type ensures that the pointer is valid. + unsafe { bindings::security_cred_getsecid(self.0.get(), &mut secid) }; + secid + } + /// Returns the effective UID of the given credential. pub fn euid(&self) -> bindings::kuid_t { // SAFETY: By the type invariant, we know that `self.0` is valid. diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 097fe9bb93ed..342cb02c495a 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -42,6 +42,7 @@ pub mod kunit; pub mod prelude; pub mod print; +pub mod security; mod static_assert; #[doc(hidden)] pub mod std_vendor; diff --git a/rust/kernel/security.rs b/rust/kernel/security.rs new file mode 100644 index 000000000000..6545bfa2fd72 --- /dev/null +++ b/rust/kernel/security.rs @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Linux Security Modules (LSM). +//! +//! C header: [`include/linux/security.h`](../../../../include/linux/security.h). + +use crate::{ + bindings, + error::{to_result, Result}, +}; + +/// A security context string. +/// +/// # Invariants +/// +/// The `secdata` and `seclen` fields correspond to a valid security context as returned by a +/// successful call to `security_secid_to_secctx`, that has not yet been destroyed by calling +/// `security_release_secctx`. +pub struct SecurityCtx { + secdata: *mut core::ffi::c_char, + seclen: usize, +} + +impl SecurityCtx { + /// Get the security context given its id. + pub fn from_secid(secid: u32) -> Result<Self> { + let mut secdata = core::ptr::null_mut(); + let mut seclen = 0u32; + // SAFETY: Just a C FFI call. The pointers are valid for writes. + unsafe { + to_result(bindings::security_secid_to_secctx( + secid, + &mut secdata, + &mut seclen, + ))?; + } + + // INVARIANT: If the above call did not fail, then we have a valid security context. + Ok(Self { + secdata, + seclen: seclen as usize, + }) + } + + /// Returns whether the security context is empty. + pub fn is_empty(&self) -> bool { + self.seclen == 0 + } + + /// Returns the length of this security context. + pub fn len(&self) -> usize { + self.seclen + } + + /// Returns the bytes for this security context. + pub fn as_bytes(&self) -> &[u8] { + let ptr = self.secdata; + if ptr.is_null() { + // We can't pass a null pointer to `slice::from_raw_parts` even if the length is zero. + debug_assert_eq!(self.seclen, 0); + return &[]; + } + + // SAFETY: The call to `security_secid_to_secctx` guarantees that the pointer is valid for + // `seclen` bytes. Furthermore, if the length is zero, then we have ensured that the + // pointer is not null. + unsafe { core::slice::from_raw_parts(ptr.cast(), self.seclen) } + } +} + +impl Drop for SecurityCtx { + fn drop(&mut self) { + // SAFETY: This frees a pointer that came from a successful call to + // `security_secid_to_secctx` and has not yet been destroyed by `security_release_secctx`. + unsafe { + bindings::security_release_secctx(self.secdata, self.seclen as u32); + } + } +}
Adds an abstraction for viewing the string representation of a security context. This is needed by Rust Binder because it has feature where a process can view the string representation of the security context for incoming transactions. The process can use that to authenticate incoming transactions, and since the feature is provided by the kernel, the process can trust that the security context is legitimate. Signed-off-by: Alice Ryhl <aliceryhl@google.com> --- rust/bindings/bindings_helper.h | 1 + rust/helpers.c | 21 +++++++++++ rust/kernel/cred.rs | 8 +++++ rust/kernel/lib.rs | 1 + rust/kernel/security.rs | 79 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 110 insertions(+)