Using a session from a deleted user account causes an internal error

[joseph-reynolds]

Describe the bug Using a session from a deleted user account gives an incorrect error message.

Environment This happens on any use of BMCWeb.

To Reproduce Steps to reproduce the behavior:

1. Add a user "U".
2. Create a session for user U: POST /redfish/v1/SessionService/Sessions ...
3. Delete user U.
4. Attempt to use the session, for example: GET /redfish/v1/AccountService
5. The HTTP request results in an internal error.
6. I am not sure what the expected behavior should be. I think when a user is deleted, all of their BMCWeb sessions should be deleted. Then the HTTP request would result in Unauthorized.

I think this would also happen if the user was renamed instead of deleted.

Causal analysis The code which takes the internal error is apparently here: https://github.com/openbmc/bmcweb/blob/759cf1055aaf9be84ea08631578a1f3c712ecc61/http/routing.hpp#L1379 At this point in the code, an HTTP request is being validated and routed to its handler. The request is authenticated (via a session token), and we are calling D-Bus to get the user's current role. The call fails because the user doesn't exist.

Is this a regression Not a recent regression. This bug may have been introduced when BMCWeb was enhanced to retrieve the user's role for each new HTTP request.

Acknowledgement This was reported by the IBM test team.

[edtanous]

This happens on any use of BMCWeb.

This portion of the template asks: what sha1 you tested against, and what platform you tested against. This is important in this case, because I believe this issue was already solved on master here (https://gerrit.openbmc.org/c/openbmc/bmcweb/+/53796) but from your description there's no way to verify that. Please fill out the bug template as directed.

Not a recent regression. This bug may ...

I'm having trouble deciphering if this is a yes, this is a regression, or no, it is not. If it's a regression, and not already fixed on master, can you please bisect it to a commit?

[joseph-reynolds]

Sorry, I don't have details for the SHA1 and was unable to learn if this is a regression.

The gerrit review 53796 referenced above is a fix for the delete operation when invoked by a session-less user. I believe it is not related to the bug described in this issue.

I believe the problem described above is caused when sessions in the SessionStore get out of sync from the D-Bus user manager data. Specifically, when a user has an active BMCWeb session, and that user's account is either deleted or renamed, and the session is used to perform an operation, the check performed as described above will fail because the underlying account is not found in the D-Bus account data.

[joseph-reynolds]

I think there are several solutions for this:

1. Anticipate that the account lookup could fail (at the line of code identified above), and handle that as an unusable session.
2. Enhance BMCWeb to monitor for changes in user accounts. When a user account is either deleted or renamed, invalidate (remove) all sessions which have that username. (Or just remove all sessions for users which are no longer in the list of users.)
3. Enhance BMCWeb "rename user" and "delete user" operations to invalidate all sessions for the user being renamed or deleted.

[williamspatrick]

Sorry, I don't have details for the SHA1 and was unable to learn if this is a regression.

Don't you know what code was used in the test? You said "this was reported by the IBM test team". Someone must know what code you are using?

If you are using patches on top of some commit here, that's [likely] fine, but we need to know what the base was.

[edtanous]

@joseph-reynolds bump. Looking for what SHA1 you used, or whether this still occurs on a build from master. I believe the safety issue you found has been solved in code. If not, please update this with updated steps to reproduce.

[edtanous]

No response from submitter. Report appears similar to a bug already fixed on master. Closing.