feat(licensing): quota enforcement, client rejection and trial license limits by philippfalk · Pull Request #71 · ThinkParQ/beegfs-rust

philippfalk · 2026-05-19T17:28:56Z

This PR does three separate things related to licensing:

Slightly reworks the quota enforcement mechanism to detect when the feature is not licensed and to more dynamically react to system state changes (for details see commit message).
Disallows client mounts when the system doesn't have a valid license and the free client mount limit is reached
Allows only a single trial license per system

Tagging @iamjoemccormick for reviewing licensing related semantics and @rustybee42 for reviewing the code flow related changes around quotas and licensing.

Before this commit, the quota retrieval and distribution of exceeded IDs would be disabled on mgmtd startup if the feature wasn't licensed. This lead to a couple of issues with enforcement consistency: * When a license expired while mgmtd was running, enforcement would continue to work but only until mgmtd restarts * When mgmtd restarted, collection and distribution of exceeded quotas would be disabled, leading to other nodes enforcing based on an old state * When other nodes restarted, they would still be able to initially fetch outdated quota states and enforce based on them. This commit fixes all of the above by not allowing intial downloads of quota state if the feature is not licensed and not entirely disabling the quota distribution mechanism. If the quota feature is not licensed nodes quota collection from the nodes will be disabled for efficiency reasons, but nodes will still receive exceeded quota updates to allow for online changes in license state. These updates will not contain any information about exceeded IDs and will simply clear out the state on the nodes so quotas will no longer be enforced.

rustybee42 · 2026-05-20T07:18:06Z

+/// Counts the number of currently registered distinct clients.
+///
+/// # Return value
+/// Returns the number of currently registered distinct clients if successful.
+pub(crate) fn count_clients(tx: &Transaction) -> Result<u32> {
+    tx.query_row(
+        sql!("SELECT COUNT(DISTINCT node_uid) FROM nodes WHERE node_type = ?1"),
+        params![NodeType::Client.sql_variant()],
+        |row| row.get(0),
+    )
+    .map_err(|e| anyhow!(e))
+}
+


issue: Avoid adding new functions here (= in the "db layer"), just use sql directly at the callsite.

The db abstraction is mostly unnecessary and can be considered deprecated.

Ok, it's only called from one place anyway. I "inlined" it.

rustybee42 · 2026-05-20T08:06:31Z

        fail_on_pre_shutdown(app)?;

-        let node_id = update_node(self, app).await?;
+        let reject = (_req.msg_compat_feature_flags()


nitpick: since _req is no longer unsed, it should be renamed to req

Makes sense. I wonder why the compiler doesn't warn in this direction. Fixed

rustybee42 · 2026-05-20T08:13:58Z

 use common::update_node;
 use shared::bee_msg::node::*;

+const REGISTERNODEMSG_COMPATFLAG_CLIENT_SUPPORTS_REGREJ: u8 = 1;


nitpick: Unnecessary long variable name.\

Even if argued to name it same as in C (which I disagree with), at least the first part (REGISTERNODEMSG_) should be dropped, since the only reason to do that in C is it's missing scope. We don't have this problem here, it's already scoped.

This was mostly done for symmetry with the client but you are right, in Rust, this is already scoped. I dropped the prefix.

rustybee42 · 2026-05-20T08:35:53Z

    let registration_disable = app.static_info().user_config.registration_disable;

+    let licensed_clients: Option<u32> = match app.get_license_cert_data() {
+        Ok(r) => match r.result {


issue: match on r.result() which directly returns the VerifyResult

Then no match guards and as i32 below are needed

Makes a lot of sense. Sorry, I forgot about this pattern again. Fixed.

rustybee42 · 2026-05-20T08:51:48Z

+        },
+        Err(e) => {
+            log::error!(
+                "Unexpected error during license verification, limiting number of clients to {NUM_CLIENTS}: {e:#}",


issue: This will spam the log with errors if the licensing library is not loaded. Not sure if that is intended.

Which also bring me to the question if it shouldn't be made mandatory and deny startup if it isn't there. And then don't do extra error handling here but just fail using ? (because then this would only ever fail in truly unexpected circumstances - the library not being loaded is not really unexpected).

I think the important part is that we log client rejections, so users know why their clients don't mount. That is done further down in the code, so I downgraded this log to debug level. The calculation now only runs when clients register, because we don't need the information for other node registrations.

rustybee42 · 2026-05-20T08:55:16Z

+        if app
+            .get_license_cert_data()?
+            .data
+            .is_some_and(|d| d.r#type == CertType::Trial.into())


nitpick: Again, use d.r#type() to get the enum instead of converting the enum to i32.

rustybee42 · 2026-05-20T09:06:37Z

+    // Load the licensing library
+    let license = if !info.user_config.license_disable {
+        // SAFETY:
+        // There is no way to verify that the user loaded dynamic library matches the
+        // requirements of LicenseVerifier. After all, users can load anything they
+        // want. Therefore, this is just not safe to do from the Rust compilers
+        // perspective and loading anything with non-matching fp signatures or not
+        // behaving as expected will lead to undefined behavior.
+        let license = unsafe { LicenseVerifier::with_lib(&info.user_config.license_lib_file) };


issue: Loading the library should stay where it was to avoid having the whole management depend on it being present on the given path.

I intentionally loaded the library outside of here (together with config file parsing and other environment dependent stuff) so the "inners" do not depend on file system paths or external librariers being present and stay kind of "pure". This allows for easy integration testing the whole management using cargo test and just passing the test configuration to start(). It's currently not done, but is definitely planned at some point.

Loading the library from here (instead of passing it in from outside where it can omitted or mocked if needed) removes that independence.

I moved the library loading back to where it was. The certificate loading and verification is still here, though, because we need access to the database.

rustybee42 · 2026-05-20T09:35:51Z

nitpick: This is not relevant anymore (as the distribute function isn't called from inside the tested function anymore), so should be removed.

rustybee42 · 2026-05-20T09:35:59Z


-        super::update_and_distribute(&app).await.unwrap();
+        super::fetch_and_update(&app).await.unwrap();
+        super::distribute_exceeded(&app).await.unwrap();


issue: This is not part of the test, it only tests the fetching and updating.

Thus it should not be called (same below).

Yep, I overlooked that. I removed the calls from that test.

Drop the somewhat misleading "Internal" prefix for errors during license verification. These errors are usually caused by invalid or non-existing license files, which isn't an "internal" problem and requires user action.

philippfalk added 3 commits May 5, 2026 10:30

feat(license): disallow client mounts when no license loaded

dda621d

feat: only allow a single trial license

be31b42

philippfalk requested a review from iamjoemccormick May 19, 2026 17:28

philippfalk requested a review from a team as a code owner May 19, 2026 17:28

github-advanced-security AI found potential problems May 19, 2026

View reviewed changes

Comment thread mgmtd/src/bee_msg/common.rs Fixed

chore (formatting): apply cargo fmt

53ae0e1

github-advanced-security AI found potential problems May 19, 2026

View reviewed changes

Comment thread mgmtd/src/bee_msg/common.rs Fixed

rustybee42 requested changes May 20, 2026

View reviewed changes

rustybee42 reviewed May 20, 2026

View reviewed changes

philippfalk added 2 commits May 21, 2026 14:59

fix(review): address comments

567c2bb

tweak(licensing): clarify verification error message

f9f1d8a

Drop the somewhat misleading "Internal" prefix for errors during license verification. These errors are usually caused by invalid or non-existing license files, which isn't an "internal" problem and requires user action.

philippfalk requested a review from rustybee42 May 22, 2026 12:25

github-advanced-security AI found potential problems May 22, 2026

View reviewed changes

Comment thread mgmtd/src/bee_msg/common.rs Dismissed

Conversation

philippfalk commented May 19, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

philippfalk May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

philippfalk May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

philippfalk May 21, 2026 •

edited

Loading

philippfalk May 21, 2026 •

edited

Loading