Redmine - Defect #35417
User sessions not reset after 2FA activation
2021-06-14 11:19 - Holger Just

ステータス:

Closed

開始日:

優先度:

Normal

期日:担当者:Go MAEDA

進捗率:0%

カテゴリ:

Accounts / authentication

予定工数:

0.00時間対象バージョン:4.2.2

Resolution:

Fixed

Affected version:

説明
Felix Schäfer reports via email to security@redmine.org
Hello,Currently a user signed up on multiple browsers/machines can activate 2FA in one session but still continue using the othersessions. This presents a security risk if an attacker has or gets control of one of those other sessions.
The attached patch resets all the session, autologin and recovery keys of a user when 2FA is set up. Maybe a warning could
also be added to the 2FA set up screen about this so that users with multiple active sessions are not surprised about getting
logged out from the other sessions.
Thank you,
Felix Schäfer

関連するチケット:
関連している Redmine - Feature #1237: Add support for two-facto...

Closed2008-05-14

関係しているリビジョン
リビジョン 21069 - 2021-07-15 03:44 - Go MAEDA
User sessions not reset after 2FA activation (#35417).
Patch by Felix Schäfer.
リビジョン 21070 - 2021-07-17 09:39 - Go MAEDA
Merged r21069 from trunk to 4.2-stable (#35417).
リビジョン 21104 - 2021-07-28 17:59 - Marius B

LTEANU

Adds test for #35417.
リビジョン 21107 - 2021-07-28 19:34 - Marius B

LTEANU

Merged r21104 to 4.2-stable (#35417).

履歴#1 - 2021-06-25 11:57 - Go MAEDA- 対象バージョン を 4.2.2 にセット
Setting the target version to 4.2.2.
Maybe a warning could also be added to the 2FA set up screen about this so that users with multiple active sessions are not surprised about
getting logged out from the other sessions.

Although it would be more user-friendly to display such a message, I think the patch can be merged as-is because Redmine currently don't showsuch warning when resseting session by timeout or some reasons.

2024-11-09

1/4

#2 - 2021-06-25 17:47 - Holger Just
Go MAEDA wrote:
Although it would be more user-friendly to display such a message, I think the patch can be merged as-is because Redmine currently don't
show such warning when resseting session by timeout or some reasons.

I agree. We also destroy / logout other sessions on password change without any further warning. I think this can be merged as is.
#3 - 2021-07-15 03:44 - Go MAEDA- ステータス を New から Resolved に変更- 担当者 を Go MAEDA にセット
Committed the patch. Thank you.
#4 - 2021-07-17 09:39 - Go MAEDA- ステータス を Resolved から Closed に変更#5 - 2021-07-20 14:37 - Holger Just
- ステータス を Closed から Reopened に変更- 担当者 を Go MAEDA から Holger Just に変更
Thank you Maeda-san.
As this is now comitted, I would go ahead and reserve a CVE-ID for this issue. The CVE-ID will be marked as reserved until we have a releasecontaining this fix.
#6 - 2021-07-21 16:55 - Holger Just
CVE-2021-37156 was assigned for this.
The CVE entry is not yet public. Once we have a release with this fix, we have to update the CVE entry on https://cveform.mitre.org/ and add areference to the news and the updated /Security̲Advisories page.
#7 - 2021-07-26 12:26 - Marius B

LTEANU

We can cut a release this weekend, is it ok?
#8 - 2021-07-26 12:56 - Holger Just
Sure!
#9 - 2021-07-27 23:00 - Marius B

LTEANU- 関連している Feature #1237: Add support for two-factor authentication を追加#10 - 2021-07-27 23:16 - Marius B

LTEANU- ファイル test̲for̲35417.patch を追加
I've added a test for this case, Holger, can you take a look, please?
As Mischa mentioned in #35611, we should have this covered by tests.
#11 - 2021-07-28 11:58 - Holger Just
- 担当者 を削除 (Holger Just)
Thank you! I tested the test (heh) and it appears to work correctly (fails without the original patch, works with both in place). I think this one can bemerged.
The second part of the patch is however still untested, namely that concurrent active sessions are destroyed if 2fa is activated. I'm not sure how we
can properly test the handling of concurrent sessions though...
In any case, could you (Marius and Maeda-san) coordinate to merge this in time before the release?
#12 - 2021-07-28 12:07 - Marius B

2024-11-09

LTEANU

2/4

Holger Just wrote:
Thank you! I tested the test (heh) and it appears to work correctly (fails without the original patch, works with both in place). I think this one
can be merged.
The second part of the patch is however still untested, namely that concurrent active sessions are destroyed if 2fa is activated. I'm not sure
how we can properly test the handling of concurrent sessions though...
In any case, could you (Marius and Maeda-san) coordinate to merge this in time before the release?

Yes, I can do it later today.
#13 - 2021-07-29 08:50 - Marius B

LTEANU- ステータス を Reopened から Closed に変更- 担当者 を Go MAEDA にセット
Test committed.
#14 - 2021-07-29 18:00 - Holger Just
For the security scanner, I would then use the following definition after the release:register(id:'2021-08-01',category::privilege̲escalation,
severity::low,details:'User sessions not reset
ticket̲id: '35417',
cves:['CVE-2021-37156'],
fixed̲in:['< 4.2.0', '>= 4.2.2'])

afteractivation

oftwo-factor

authentication',with the following title:
User sessions not reset after activation of two-factor authenticationand description:
When enabling two-factor authentication for a user's account, Redmine allows existing user sessions to continue, resulting in increased risk ifan attacker has or gets control of one of those other sessions.

#15 - 2021-08-01 08:53 - Marius B

LTEANU- トラッカー を Patch から Defect に変更#16 - 2021-08-02 12:50 - Marius B

LTEANU

Holger Just wrote:
For the security scanner, I would then use the following definition after the release:[...]with the following title:
User sessions not reset after activation of two-factor authenticationand description:
When enabling two-factor authentication for a user's account, Redmine allows existing user sessions to continue, resulting in increased
risk if an attacker has or gets control of one of those other sessions.

Thanks Holger for your feedback and also for your updates made today to the Wiki page. I saw that you updated the severity to Medium for thisissue and then you reverted without any comment. Is it ok or you reverted by mistake?

2024-11-09

3/4

#17 - 2021-08-02 13:08 - Holger Just
When updating the security scanner yesterday, I thought a bit more about the possible impact of this. After comparing it with other issues we hadtagged as "low" in the past, I came to the conclusion that this probably deserved a medium. I initially tagged it in the security scanner as such.
Only after having updated the severity in the wiki page however, I saw that you mentioned the severity in the release news article as low. Unwillingto change it there too and convinced that consistency is more important than my hunch of severity, I reverted it everywhere back to low. Sorry forthis back-and-forth :(#18 - 2021-08-02 13:13 - Marius B

LTEANU

Holger Just wrote:
When updating the security scanner yesterday, I thought a bit more about the possible impact of this. After comparing it with other issues wehad tagged as "low" in the past, I came to the conclusion that this probably deserved a medium. I initially tagged it in the security scanner assuch.
Only after having updated the severity in the wiki page however, I saw that you mentioned the severity in the release news article as low.
Unwilling to change it there too and convinced that consistency is more important than my hunch of severity, I reverted it everywhere back tolow. Sorry for this back-and-forth :(Don't worry, I just wanted to double check this with you. In the future, I will avoid posting the severity in the news post.
#19 - 2021-08-03 18:00 - Holger Just
I have requested an update for CVE-2021-37156 on https://cveform.mitre.org/ with the following references:https://www.redmine.org/news/132
https://www.redmine.org/projects/redmine/wiki/Security̲Advisories
The CVE should be published in the next few hours with this new information.
#20 - 2022-06-21 08:12 - Marius B

LTEANU- プロジェクト を 2 から Redmine に変更- カテゴリ を Accounts / authentication にセット
- Resolution を Fixed にセット

ファイル
2fa-session-reset.patch

1.24 KB

2021-06-14

test̲for̲35417.patch

1.18 KB

2021-07-27

2024-11-09

Holger Just
Marius B

4/4

LTEANU