Wrong username sent after wrong otp entered

[tedbear]

Hi sbidy,
There is at least one more scenario that will send the wrong username. How to reproduce:

1. User A logs into ADFS and reach the OTP form page.
2. User B logs into ADFS and reach the OTP form page.
3. User A enters wrong OTP. The page naturally does a refresh and present the wrong OTP error,
4. User A enters OTP (doesnt matter if it is a correct code or not), but now the hidden username field contain the username of User B.

A "dirty hack" would be to carry the (hidden) submitted username value from the form over to the refreshed page (on error).
I am just thinking of there are even more scenarios where this might happen? Does this work if 3000 users are logging in at the same time? Here I am talking about the username being carried from the ADFS login to the OTP form page.

Best regards,
Teddy

[sbidy]

A "dirty hack" would be to carry the (hidden) submitted username value from the form over to the refreshed page (on error).

I do this already. In any cases the username and realm should be present in the error page.
So that behavior is relay like a race condition - the user A gets the authenication form from B. Because the username and realm was overwritten by the request from B.
In case of 3000 users that problem can be become relevant.
In my opinion the core problem is, that the ADFS doesn't instantiate a new object for each auth request.

I have to review the program logic to fix the problem. A possible solution would be a kind of context in which the authentication should be isolated. Maybe @cornelinux has a idea ... 😉

[tedbear]

Are you also carrying the first hidden username field from the OTP page onto the next page in case of error? Looking at the code it uses username from the claim and not the form param. Second page (after entering wrong otp) does have a username and realm attribute - but from a different user. Yes it because very relevant. It happened a few times today and we are still only 15-20 test users, and we are only logging in a few times a week (when I kill their session on Office 365 for test reasons). We are looking at using this for 18.000 users in two weeks from now, so please let me know if there is something I can help with. I will fiddle around the code tomorrow and see if I can help isolate those damn claims :) On 8 Jan 2019, at 18:42, Stephan Traub <[email protected]<mailto:[email protected]>> wrote: CAUTION: This email originated from outside the organization. Do not click on a link or open an attachment unless you recognize the sender and know the content is safe. A "dirty hack" would be to carry the (hidden) submitted username value from the form over to the refreshed page (on error). I do this already. In any cases the username and realm should be present in the error page. So that behavior is relay like a race condition - the user A gets the authenication form from B. Because the username and realm was overwritten by the request from B. In case of 3000 users that problem can be become relevant. In my opinion the core problem is, that the ADFS doesn't instantiate a new object for each auth request. I have to review the program logic to fix the problem. A possible solution would be a kind of context in which the authentication should be isolated. Maybe @cornelinux<https://github.com/cornelinux> has a idea ... 😉 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<#19 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AFxgSaL__3d9kd3w0R1rJ3f7uZhcRVMKks5vBNiZgaJpZM4Z1w_8>.

[tedbear]

I replaced the following part of the code (injected the variables from the proofData) in the "authentication not complete" section:

Afterwards I still saw a "race" issue, but currently im not able to reproduce it. I am shooting a bit blindly here.

Best regards,
Teddy

[cornelinux]

I am honestly starting to be a bit confused. Maybe due to the fact I do not exactly know how the plugin mechanism provided by ADFS works.

We can not solely rely on a hidden username for the 2nd factor, since an attacker could replace the username in a form:

Given the attacker has access to the administrators password. He would authenticate with the password in the first step. Then gets the form for 2nd factor against privacyIDEA. The attacker would replace the hidden username "administrator" with "john-little" (since he either stole the 2nd factor from john little he is john little). Then he would successfully authenticate against privacyIDEA.

Which user would be granted access by ADFS? Administrator? Thank you. 2FA successfully circumvented.

ADFS and the ADFS Plugin API needs to keep track of the session! So something seems a bit odd here.
We have very good experience with SAML via simpleSAMLphp with our plugin. A colleage of mine could have a look at it (without C# and ADFS knowledge, but simpleSAML knowledge), but currently he is still on vacation.

@sbidy Would it help to discuss some things in a telco/remote session?

[sbidy]

Give me some time to reproduce and dig into the ADFS logic.

It is fact, that the username witch is provided via HTML hidden field is generally a vulnerability (as @cornelinux explained). But the main problem is that user B can have "access" to the session to user A. If this "access" is prohibited by the ADFS(provider) you can put into the "username" filed any user but it will be not possible to hijack the other session.
But actually there is no session context implemented (I expected that from the ADFS it self!) - in my opinion all other ADFSproviders have the same/similar problem! I've locked into some other implementations and they did this in the same way.

To recap the vulnerability: The first authentication step is not securely connected to the second authentication step. It is possible to authenticate with two independent set of credentials (OTP/Token and Username/Password). But these are needed/hacked first.

So my plan is now to remove fist the "hidden field" and try to implement a encryption based approach to generate a kind of session context or encrypt the username+realm in the field. Additionally I try to get in contact with the MS guys to get a better understanding why every user goes through the same authentication context.

I'm looking forward to fix this this week.

Thank you for you help and input!! 👍

[sbidy]

Soooooooo ... It is to hart to tell 😄 - the ADFS interface does provide a IAuthenticationContext witch can be used for securely passing parameters form the BeginAuthentication to TryEndAuthentication.

privacyIDEA-ADFSProvider/privacyIDEAADFSProvider/Adapter.cs

Line 76 in b413835

authContext.Data.Add("userid", username);

Please test with the 1.3.3c at releases. This is an debug version!!

[cornelinux]

Everyone who takes up the fight with usually moderately documented Windows APIs has my greatest respect! Thanks a lot!

[sbidy]

Now fixed in the 1.3.4 (production) or 1.3.3c (debug).

[tedbear]

thank you, sbidy. Will install on our internal test environment now and then deploy on our US production instance later today.

[tedbear]

Hi sbidy,

I want to thank you a whole bunch. We have been running with the newest release for a week now and it is indeed stable. We have enrolled 900 users since Tuesday and we aim to have 18.000 users on within 2 weeks.
Big thumbs up from me!

Best regards,
Teddy