Add an abstraction for viewing the string representation of a security
context.

This is needed by Rust Binder because it has a 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.

This abstraction makes the following assumptions about the C side:
* When a call to `security_secid_to_secctx` is successful, it returns a
  pointer and length. The pointer references a byte string and is valid
  for reading for that many bytes.
* The string may be referenced until `security_release_secctx` is
  called.
* If CONFIG_SECURITY is set, then the three methods mentioned in
  rust/helpers are available without a helper. (That is, they are not a
  #define or `static inline`.)

Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Reviewed-by: Trevor Gross <tmgross@umich.edu>
Reviewed-by: Gary Guo <gary@garyguo.net>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/bindings/bindings_helper.h |  1 +
 rust/helpers/helpers.c          |  1 +
 rust/helpers/security.c         | 20 +++++++++++
 rust/kernel/cred.rs             |  8 +++++
 rust/kernel/lib.rs              |  1 +
 rust/kernel/security.rs         | 74 +++++++++++++++++++++++++++++++++++++++++
 6 files changed, 105 insertions(+)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index f74247205cb5..51ec78c355c0 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -21,6 +21,7 @@
 #include <linux/phy.h>
 #include <linux/refcount.h>
 #include <linux/sched.h>
+#include <linux/security.h>
 #include <linux/slab.h>
 #include <linux/wait.h>
 #include <linux/workqueue.h>
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 16e5de352dab..62022b18caf5 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -19,6 +19,7 @@
 #include "page.c"
 #include "rbtree.c"
 #include "refcount.c"
+#include "security.c"
 #include "signal.c"
 #include "slab.c"
 #include "spinlock.c"
diff --git a/rust/helpers/security.c b/rust/helpers/security.c
new file mode 100644
index 000000000000..239e5b4745fe
--- /dev/null
+++ b/rust/helpers/security.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/security.h>
+
+#ifndef CONFIG_SECURITY
+void rust_helper_security_cred_getsecid(const struct cred *c, u32 *secid)
+{
+	security_cred_getsecid(c, secid);
+}
+
+int rust_helper_security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
+{
+	return security_secid_to_secctx(secid, secdata, seclen);
+}
+
+void rust_helper_security_release_secctx(char *secdata, u32 seclen)
+{
+	security_release_secctx(secdata, seclen);
+}
+#endif
diff --git a/rust/kernel/cred.rs b/rust/kernel/cred.rs
index acee04768927..92659649e932 100644
--- a/rust/kernel/cred.rs
+++ b/rust/kernel/cred.rs
@@ -52,6 +52,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. Furthermore, the `euid`
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index c537d17c6db9..e088c94a5a14 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -47,6 +47,7 @@
 pub mod prelude;
 pub mod print;
 pub mod rbtree;
+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..2522868862a1
--- /dev/null
+++ b/rust/kernel/security.rs
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Linux Security Modules (LSM).
+//!
+//! C header: [`include/linux/security.h`](srctree/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.
+        to_result(unsafe { 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() {
+            debug_assert_eq!(self.seclen, 0);
+            // We can't pass a null pointer to `slice::from_raw_parts` even if the length is zero.
+            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: By the invariant of `Self`, 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) };
+    }
+}

-- 
2.46.0.662.g92d0881bb0-goog

On Sun, Sep 15, 2024 at 10:31 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Add an abstraction for viewing the string representation of a security
> context.
>
> This is needed by Rust Binder because it has a 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.
>
> This abstraction makes the following assumptions about the C side:
> * When a call to `security_secid_to_secctx` is successful, it returns a
>   pointer and length. The pointer references a byte string and is valid
>   for reading for that many bytes.
> * The string may be referenced until `security_release_secctx` is
>   called.
> * If CONFIG_SECURITY is set, then the three methods mentioned in
>   rust/helpers are available without a helper. (That is, they are not a
>   #define or `static inline`.)
>
> Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
> Reviewed-by: Trevor Gross <tmgross@umich.edu>
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/bindings/bindings_helper.h |  1 +
>  rust/helpers/helpers.c          |  1 +
>  rust/helpers/security.c         | 20 +++++++++++
>  rust/kernel/cred.rs             |  8 +++++
>  rust/kernel/lib.rs              |  1 +
>  rust/kernel/security.rs         | 74 +++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 105 insertions(+)

I doubt my ACK is strictly necessary here since the Rust bindings
aren't actually modifying anything in the LSM, but just in case ...

Acked-by: Paul Moore <paul@paul-moore.com>

-- 
paul-moore.com

On Sun, Sep 15, 2024 at 02:31:31PM +0000, Alice Ryhl wrote:
> Add an abstraction for viewing the string representation of a security
> context.

Hm, this may collide with "LSM: Move away from secids" is going to happen.
https://lore.kernel.org/all/20240830003411.16818-1-casey@schaufler-ca.com/

This series is not yet landed, but in the future, the API changes should
be something like this, though the "lsmblob" name is likely to change to
"lsmprop"?
security_cred_getsecid()   -> security_cred_getlsmblob()
security_secid_to_secctx() -> security_lsmblob_to_secctx()

> This is needed by Rust Binder because it has a 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.
> 
> This abstraction makes the following assumptions about the C side:
> * When a call to `security_secid_to_secctx` is successful, it returns a
>   pointer and length. The pointer references a byte string and is valid
>   for reading for that many bytes.

Yes. (len includes trailing C-String NUL character.)

> * The string may be referenced until `security_release_secctx` is
>   called.

Yes.

> * If CONFIG_SECURITY is set, then the three methods mentioned in
>   rust/helpers are available without a helper. (That is, they are not a
>   #define or `static inline`.)

Yes.

> 
> Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
> Reviewed-by: Trevor Gross <tmgross@umich.edu>
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook

On Sun, Sep 15, 2024 at 10:58 PM Kees Cook <kees@kernel.org> wrote:
>
> On Sun, Sep 15, 2024 at 02:31:31PM +0000, Alice Ryhl wrote:
> > Add an abstraction for viewing the string representation of a security
> > context.
>
> Hm, this may collide with "LSM: Move away from secids" is going to happen.
> https://lore.kernel.org/all/20240830003411.16818-1-casey@schaufler-ca.com/
>
> This series is not yet landed, but in the future, the API changes should
> be something like this, though the "lsmblob" name is likely to change to
> "lsmprop"?
> security_cred_getsecid()   -> security_cred_getlsmblob()
> security_secid_to_secctx() -> security_lsmblob_to_secctx()

Thanks for the heads up. I'll make sure to look into how this
interacts with those changes.

> > This is needed by Rust Binder because it has a 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.
> >
> > This abstraction makes the following assumptions about the C side:
> > * When a call to `security_secid_to_secctx` is successful, it returns a
> >   pointer and length. The pointer references a byte string and is valid
> >   for reading for that many bytes.
>
> Yes. (len includes trailing C-String NUL character.)

I suppose the NUL character implies that this API always returns a
non-zero length? I could simplify the patch a little bit by not
handling empty strings.

It looks like the CONFIG_SECURITY=n case returns -EOPNOTSUPP, so we
don't get an empty string from that case, at least.

> > * The string may be referenced until `security_release_secctx` is
> >   called.
>
> Yes.
>
> > * If CONFIG_SECURITY is set, then the three methods mentioned in
> >   rust/helpers are available without a helper. (That is, they are not a
> >   #define or `static inline`.)
>
> Yes.
>
> >
> > Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> > Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
> > Reviewed-by: Trevor Gross <tmgross@umich.edu>
> > Reviewed-by: Gary Guo <gary@garyguo.net>
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
>
> Reviewed-by: Kees Cook <kees@kernel.org>

Thanks for the review!

Alice

On 9/15/2024 2:07 PM, Alice Ryhl wrote:
> On Sun, Sep 15, 2024 at 10:58 PM Kees Cook <kees@kernel.org> wrote:
>> On Sun, Sep 15, 2024 at 02:31:31PM +0000, Alice Ryhl wrote:
>>> Add an abstraction for viewing the string representation of a security
>>> context.
>> Hm, this may collide with "LSM: Move away from secids" is going to happen.
>> https://lore.kernel.org/all/20240830003411.16818-1-casey@schaufler-ca.com/
>>
>> This series is not yet landed, but in the future, the API changes should
>> be something like this, though the "lsmblob" name is likely to change to
>> "lsmprop"?
>> security_cred_getsecid()   -> security_cred_getlsmblob()
>> security_secid_to_secctx() -> security_lsmblob_to_secctx()

The referenced patch set does not change security_cred_getsecid()
nor remove security_secid_to_secctx(). There remain networking interfaces
that are unlikely to ever be allowed to move away from secids. It will
be necessary to either retain some of the secid interfaces or introduce
scaffolding around the lsm_prop structure.

Binder is currently only supported in SELinux, so this isn't a real issue
today. The BPF LSM could conceivably support binder, but only in cases where
SELinux isn't enabled. Should there be additional LSMs that support binder
the hooks would have to be changed to use lsm_prop interfaces, but I have
not included that *yet*.

> Thanks for the heads up. I'll make sure to look into how this
> interacts with those changes.

There will be a follow on patch set as well that replaces the LSMs use
of string/length pairs with a structure. This becomes necessary in cases
where more than one active LSM uses secids and security contexts. This
will affect binder.

>
>>> This is needed by Rust Binder because it has a 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.
>>>
>>> This abstraction makes the following assumptions about the C side:
>>> * When a call to `security_secid_to_secctx` is successful, it returns a
>>>   pointer and length. The pointer references a byte string and is valid
>>>   for reading for that many bytes.
>> Yes. (len includes trailing C-String NUL character.)
> I suppose the NUL character implies that this API always returns a
> non-zero length? I could simplify the patch a little bit by not
> handling empty strings.
>
> It looks like the CONFIG_SECURITY=n case returns -EOPNOTSUPP, so we
> don't get an empty string from that case, at least.
>
>>> * The string may be referenced until `security_release_secctx` is
>>>   called.
>> Yes.
>>
>>> * If CONFIG_SECURITY is set, then the three methods mentioned in
>>>   rust/helpers are available without a helper. (That is, they are not a
>>>   #define or `static inline`.)
>> Yes.
>>
>>> Reviewed-by: Benno Lossin <benno.lossin@proton.me>
>>> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
>>> Reviewed-by: Trevor Gross <tmgross@umich.edu>
>>> Reviewed-by: Gary Guo <gary@garyguo.net>
>>> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
>> Reviewed-by: Kees Cook <kees@kernel.org>
> Thanks for the review!
>
> Alice
>

On Mon, Sep 16, 2024 at 5:40 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 9/15/2024 2:07 PM, Alice Ryhl wrote:
> > On Sun, Sep 15, 2024 at 10:58 PM Kees Cook <kees@kernel.org> wrote:
> >> On Sun, Sep 15, 2024 at 02:31:31PM +0000, Alice Ryhl wrote:
> >>> Add an abstraction for viewing the string representation of a security
> >>> context.
> >> Hm, this may collide with "LSM: Move away from secids" is going to happen.
> >> https://lore.kernel.org/all/20240830003411.16818-1-casey@schaufler-ca.com/
> >>
> >> This series is not yet landed, but in the future, the API changes should
> >> be something like this, though the "lsmblob" name is likely to change to
> >> "lsmprop"?
> >> security_cred_getsecid()   -> security_cred_getlsmblob()
> >> security_secid_to_secctx() -> security_lsmblob_to_secctx()
>
> The referenced patch set does not change security_cred_getsecid()
> nor remove security_secid_to_secctx(). There remain networking interfaces
> that are unlikely to ever be allowed to move away from secids. It will
> be necessary to either retain some of the secid interfaces or introduce
> scaffolding around the lsm_prop structure.
>
> Binder is currently only supported in SELinux, so this isn't a real issue
> today. The BPF LSM could conceivably support binder, but only in cases where
> SELinux isn't enabled. Should there be additional LSMs that support binder
> the hooks would have to be changed to use lsm_prop interfaces, but I have
> not included that *yet*.
>
> > Thanks for the heads up. I'll make sure to look into how this
> > interacts with those changes.
>
> There will be a follow on patch set as well that replaces the LSMs use
> of string/length pairs with a structure. This becomes necessary in cases
> where more than one active LSM uses secids and security contexts. This
> will affect binder.

When are these things expected to land? If this patch series gets
merged in the same kernel cycle as those changes, it'll probably need
special handling.

Alice

On 9/22/2024 8:08 AM, Alice Ryhl wrote:
> On Mon, Sep 16, 2024 at 5:40 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 9/15/2024 2:07 PM, Alice Ryhl wrote:
>>> On Sun, Sep 15, 2024 at 10:58 PM Kees Cook <kees@kernel.org> wrote:
>>>> On Sun, Sep 15, 2024 at 02:31:31PM +0000, Alice Ryhl wrote:
>>>>> Add an abstraction for viewing the string representation of a security
>>>>> context.
>>>> Hm, this may collide with "LSM: Move away from secids" is going to happen.
>>>> https://lore.kernel.org/all/20240830003411.16818-1-casey@schaufler-ca.com/
>>>>
>>>> This series is not yet landed, but in the future, the API changes should
>>>> be something like this, though the "lsmblob" name is likely to change to
>>>> "lsmprop"?
>>>> security_cred_getsecid()   -> security_cred_getlsmblob()
>>>> security_secid_to_secctx() -> security_lsmblob_to_secctx()
>> The referenced patch set does not change security_cred_getsecid()
>> nor remove security_secid_to_secctx(). There remain networking interfaces
>> that are unlikely to ever be allowed to move away from secids. It will
>> be necessary to either retain some of the secid interfaces or introduce
>> scaffolding around the lsm_prop structure.
>>
>> Binder is currently only supported in SELinux, so this isn't a real issue
>> today. The BPF LSM could conceivably support binder, but only in cases where
>> SELinux isn't enabled. Should there be additional LSMs that support binder
>> the hooks would have to be changed to use lsm_prop interfaces, but I have
>> not included that *yet*.
>>
>>> Thanks for the heads up. I'll make sure to look into how this
>>> interacts with those changes.
>> There will be a follow on patch set as well that replaces the LSMs use
>> of string/length pairs with a structure. This becomes necessary in cases
>> where more than one active LSM uses secids and security contexts. This
>> will affect binder.
> When are these things expected to land?

I would like them to land in 6.14, but history would lead me to think
it will be later than that. A lot will depend on how well the large set
of LSM changes that went into 6.12 are received.

>  If this patch series gets
> merged in the same kernel cycle as those changes, it'll probably need
> special handling.

Yes, this is the fundamental downside of the tree merge development model.

> Alice

On Sun, Sep 22, 2024 at 6:50 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 9/22/2024 8:08 AM, Alice Ryhl wrote:
> > On Mon, Sep 16, 2024 at 5:40 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> On 9/15/2024 2:07 PM, Alice Ryhl wrote:
> >>> On Sun, Sep 15, 2024 at 10:58 PM Kees Cook <kees@kernel.org> wrote:
> >>>> On Sun, Sep 15, 2024 at 02:31:31PM +0000, Alice Ryhl wrote:
> >>>>> Add an abstraction for viewing the string representation of a security
> >>>>> context.
> >>>> Hm, this may collide with "LSM: Move away from secids" is going to happen.
> >>>> https://lore.kernel.org/all/20240830003411.16818-1-casey@schaufler-ca.com/
> >>>>
> >>>> This series is not yet landed, but in the future, the API changes should
> >>>> be something like this, though the "lsmblob" name is likely to change to
> >>>> "lsmprop"?
> >>>> security_cred_getsecid()   -> security_cred_getlsmblob()
> >>>> security_secid_to_secctx() -> security_lsmblob_to_secctx()
> >> The referenced patch set does not change security_cred_getsecid()
> >> nor remove security_secid_to_secctx(). There remain networking interfaces
> >> that are unlikely to ever be allowed to move away from secids. It will
> >> be necessary to either retain some of the secid interfaces or introduce
> >> scaffolding around the lsm_prop structure.
> >>
> >> Binder is currently only supported in SELinux, so this isn't a real issue
> >> today. The BPF LSM could conceivably support binder, but only in cases where
> >> SELinux isn't enabled. Should there be additional LSMs that support binder
> >> the hooks would have to be changed to use lsm_prop interfaces, but I have
> >> not included that *yet*.
> >>
> >>> Thanks for the heads up. I'll make sure to look into how this
> >>> interacts with those changes.
> >> There will be a follow on patch set as well that replaces the LSMs use
> >> of string/length pairs with a structure. This becomes necessary in cases
> >> where more than one active LSM uses secids and security contexts. This
> >> will affect binder.
> > When are these things expected to land?
>
> I would like them to land in 6.14, but history would lead me to think
> it will be later than that. A lot will depend on how well the large set
> of LSM changes that went into 6.12 are received.
>
> >  If this patch series gets
> > merged in the same kernel cycle as those changes, it'll probably need
> > special handling.
>
> Yes, this is the fundamental downside of the tree merge development model.

Okay. I'm hoping to land this series in 6.13 so hopefully we won't
need to do anything special.

Alice

On Mon, Sep 16, 2024 at 11:40 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 9/15/2024 2:07 PM, Alice Ryhl wrote:
> > On Sun, Sep 15, 2024 at 10:58 PM Kees Cook <kees@kernel.org> wrote:
> >> On Sun, Sep 15, 2024 at 02:31:31PM +0000, Alice Ryhl wrote:
> >>> Add an abstraction for viewing the string representation of a security
> >>> context.
> >> Hm, this may collide with "LSM: Move away from secids" is going to happen.
> >> https://lore.kernel.org/all/20240830003411.16818-1-casey@schaufler-ca.com/
> >>
> >> This series is not yet landed, but in the future, the API changes should
> >> be something like this, though the "lsmblob" name is likely to change to
> >> "lsmprop"?
> >> security_cred_getsecid()   -> security_cred_getlsmblob()
> >> security_secid_to_secctx() -> security_lsmblob_to_secctx()
>
> The referenced patch set does not change security_cred_getsecid()
> nor remove security_secid_to_secctx(). There remain networking interfaces
> that are unlikely to ever be allowed to move away from secids. It will
> be necessary to either retain some of the secid interfaces or introduce
> scaffolding around the lsm_prop structure ...

First, thanks for CC'ing the LSM list Alice, I appreciate it.

As Kees and Casey already pointed out, there are relevant LSM changes
that are nearing inclusion which might be relevant to the Rust
abstractions.  I don't think there is going to be anything too
painful, but I must admit that my Rust knowledge has sadly not
progressed much beyond the most basic "hello world" example.

This brings up the point I really want to discuss: what portions of
the LSM framework are currently accessible to Rust, and what do we
(the LSM devs) need to do to preserve the Rust LSM interfaces when the
LSM framework is modified?  While the LSM framework does not change
often, we do modify both the LSM hooks (the security_XXX() calls that
serve as the LSM interface/API) and the LSM callbacks (the individual
LSM hook implementations) on occasion as they are intentionally not
part of any sort of stable API.  In a perfect world we/I would have a
good enough understanding of the Rust kernel abstractions and would
submit patches to update the Rust code as appropriate, but that isn't
the current situation and I want to make sure the LSM framework and
the Rust interfaces don't fall out of sync.  Do you watch the LSM list
or linux-next for patches that could affect the Rust abstractions?  Is
there something else you would recommend?

-- 
paul-moore.com

On Tue, Sep 17, 2024 at 3:18 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Mon, Sep 16, 2024 at 11:40 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> > On 9/15/2024 2:07 PM, Alice Ryhl wrote:
> > > On Sun, Sep 15, 2024 at 10:58 PM Kees Cook <kees@kernel.org> wrote:
> > >> On Sun, Sep 15, 2024 at 02:31:31PM +0000, Alice Ryhl wrote:
> > >>> Add an abstraction for viewing the string representation of a security
> > >>> context.
> > >> Hm, this may collide with "LSM: Move away from secids" is going to happen.
> > >> https://lore.kernel.org/all/20240830003411.16818-1-casey@schaufler-ca.com/
> > >>
> > >> This series is not yet landed, but in the future, the API changes should
> > >> be something like this, though the "lsmblob" name is likely to change to
> > >> "lsmprop"?
> > >> security_cred_getsecid()   -> security_cred_getlsmblob()
> > >> security_secid_to_secctx() -> security_lsmblob_to_secctx()
> >
> > The referenced patch set does not change security_cred_getsecid()
> > nor remove security_secid_to_secctx(). There remain networking interfaces
> > that are unlikely to ever be allowed to move away from secids. It will
> > be necessary to either retain some of the secid interfaces or introduce
> > scaffolding around the lsm_prop structure ...
>
> First, thanks for CC'ing the LSM list Alice, I appreciate it.
>
> As Kees and Casey already pointed out, there are relevant LSM changes
> that are nearing inclusion which might be relevant to the Rust
> abstractions.  I don't think there is going to be anything too
> painful, but I must admit that my Rust knowledge has sadly not
> progressed much beyond the most basic "hello world" example.

We discussed this email in-person at Plumbers. I'll outline what we
discussed here.

> This brings up the point I really want to discuss: what portions of
> the LSM framework are currently accessible to Rust,

It's relatively limited. I'm adding a way to access the secctx as a
string, and a way to manipulate `struct cred`. Basically it just lets
you take and drop refcounts on the credential and pass a credential to
functions.

Other than what is in this patch series, Binder also needs a few other
methods. Here are the signatures:

fn binder_set_context_mgr(mgr: &Credential) -> Result;
fn binder_transaction(from: &Credential, to: &Credential) -> Result;
fn binder_transfer_binder(from: &Credential, to: &Credential) -> Result;
fn binder_transfer_file(from: &Credential, to: &Credential, file: &File) -> Result;

These methods just call into the equivalent C functions. The `Result`
return type can hold either an "Ok" which indicates success, or an "Err"
which indicates an error. In the latter case, it will hold whichever
errno that the C api returns.

> and what do we
> (the LSM devs) need to do to preserve the Rust LSM interfaces when the
> LSM framework is modified?  While the LSM framework does not change
> often, we do modify both the LSM hooks (the security_XXX() calls that
> serve as the LSM interface/API) and the LSM callbacks (the individual
> LSM hook implementations) on occasion as they are intentionally not
> part of any sort of stable API.

That's fine. None of the Rust APIs are stable either.

Rust uses the bindgen tool to convert C headers into Rust declarations,
so changes to the C api will result in a build failure. This makes it
easy to discover issues.

> In a perfect world we/I would have a
> good enough understanding of the Rust kernel abstractions and would
> submit patches to update the Rust code as appropriate, but that isn't
> the current situation and I want to make sure the LSM framework and
> the Rust interfaces don't fall out of sync.  Do you watch the LSM list
> or linux-next for patches that could affect the Rust abstractions?  Is
> there something else you would recommend?

Ideally, you would add a CONFIG_RUST build to your CI setup so that you
catch issues early. Of course, if something slips through, then we run
build tests on linux-next too, so anything that falls through the cracks
should get caught by that.

If anything needs Rust changes, you can CC the rust-for-linux list and
me, and we will take a look. Same applies to review of Rust code.

Alice