Question about HMAC usage in active_sessions feature

[Janko Marohnić]

I'm was looking into the active_sessions feature with the goal of providing similar functionality to GitHub's sessions list in the "Account Security" section. In addition to providing the IP address and location using the PR I had sent, I realized it would also be good to keep updating the current session on every request with the current data (e.g. maybe the IP changes).

I was thinking it would make sense to just do it in the routing tree. However, I didn't see an easy way query the current session. I noticed that active_sessions stores the HMAC-ed ID in the database, and the non-HMAC-ed ID in the session, which I believe is different from most other features where the raw value is stored in the database.

Anyway, the code for updating the active session would then probably look something like this:

  DB[:account_active_session_keys]

    .where(account_id: rodauth.session_value)

    .where(session_id: rodauth.compute_hmac(session[:active_session_id]))

    .update(ip: request.ip, last_use: Time.now)

Which now that I write it out actually looks fine. But just out of curiousity, I was wondering why does active_sessions use HMACs? For features like verify_account, I could totally see how a leaked key could be used to autologin to a different account. But for active_sessions I didn't see how an attacker could use a session ID they retrieved from the database to somehow log in (given that the session data cannot be tampered with).

Kind regards,

Janko

[Jeremy Evans]

I'm not aware of any security issues that would result of not using HMACs in the active_sessions plugin.  However, I don't think it should hurt, and it may help unforeseen cases.  It's a good defensive in depth measure even if it isn't necessary.

To handle your use case for updating additional values, we could add active_sessions_update_hash  similar to the insert_hash your PR already supports.  It would only be used if the session_inactivity_deadline is set, but that is the default.  Is that something you would like to add to your PR before I merge it?

Thanks,

Jeremy

[Janko Marohnić]

> I'm not aware of any security issues that would result of not using HMACs in the active_sessions plugin.  However, I don't think it should hurt, and it may help unforeseen cases.  It's a good defensive in depth measure even if it isn't necessary.

Thanks for the reply, that makes sense.

> To handle your use case for updating additional values, we could add active_sessions_update_hash  similar to the insert_hash your PR already supports.  It would only be used if the session_inactivity_deadline is set, but that is the default.  Is that something you would like to add to your PR before I merge it?

That sounds good actually, I will add it to the PR.

Kind regards,

Janko