add restriction on users ability to set credentials#386
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens the authentication manager’s “set credentials” operation so a non-admin user can only change their own credentials, while admins can change any user’s credentials. It does this by threading the caller’s user ID through the UserSetCredentials backend callback and enforcing the restriction in the default/base backend.
Changes:
- Extend
UserSetCredentialscallback API to includecurrent_user_id(caller) and pass it fromwh_Auth_UserSetCredentials. - Enforce self-only credential changes for non-admins in
wh_Auth_BaseUserSetCredentials(admins exempt). - Add documentation and a new test scenario for non-admin credential-change authorization.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| wolfhsm/wh_auth.h | Updates callback signature to include current_user_id. |
| wolfhsm/wh_auth_base.h | Updates base backend API docs/signature to include caller ID and policy. |
| src/wh_auth.c | Passes context->user.user_id into backend UserSetCredentials callback. |
| src/wh_auth_base.c | Enforces “non-admin can only set own credentials” restriction. |
| test/wh_test_auth.c | Adds non-admin authorization test for set-credentials. |
| docs/src/chapter09.md | Documents the new restriction and clarifies action-bit vs backend authorization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #386
Scan targets checked: wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-src
No new issues found in the changed files. ✅
acf8f9b to
9ec0bc8
Compare
9ec0bc8 to
b7503bc
Compare
rizlik
left a comment
There was a problem hiding this comment.
Nice work.
As a small improvement in the AuthBase backend that can be postponed to a followup PR: consider to allow the admin to overwrite other user credentials without knowledge of the credentials itself
Follow up to item 3 from #270