alexandregz / twofactor_gauthenticator

This RoundCube plugin adds the 2-step verification(OTP) to the login proccess
MIT License
217 stars 76 forks source link

apparently CSRF detected #24

Open cjeanneret opened 9 years ago

cjeanneret commented 9 years ago

Hello,

I just installed the plugin on my roundcube 1.1.0 (latest release for now), and I get a weird error:

PHP Error: Request security check failed

Warning: Cannot modify header information - headers already sent in […]/roundcubemail-1.1.0/plugins/twofactor_gauthenticator/twofactor_gauthenticator.php on line 325

After a quick check, it seems the first error (PHP Error: Request security check failed) is raised by function "check_request", defined in program/include/rcmail.php, line 871. Its description is "CSRF attack prevention code", and it seems to check for a security token

The second error is due to the fact the twofactor_gauthenticator tries to send a header and, of course, this fails due to the presence of a string already sent to the browser.

Hence, I'm pretty sure the form allowing to enter the gauth code is missing some token in order to ensure there is no CSRF. Maybe it's new for 1.1.0 — dunno, I just migrated today and decided to enable a two-step thing for security improvement…

Info: if I go back on the page (F6 + enter), I'm authenticated and can access my mails as usual… This means the session is created as expected — not really sure this is a good thing though.

Cheers,

C.

alexandregz commented 9 years ago

Hello:

I modified recently the plugin to support security token precisely, version 0.9.5 doesn't have that, version 1.0 -and higher- have this one. With 1.0 I have no problem, all works fine. Also, I just try right now with 1.1.0 and all works fine too.

I'm going to check again, but I can't reproduce the error :confused:

cjeanneret commented 9 years ago

Hello,

hmm, maybe the installation instructions are wrong? Don't really know how composer works, but if the second field in the JSON is for a branch, it seems to point to "dev-master": http://plugins.roundcube.net/packages/alexandregz/twofactor_gauthenticator

Shall I point it to "master" directly? I indeed saw the _token in the formular… Maybe I did something wrong at roundcube configuration level, I'll double check that as well on my side.

Thank you for the fast answer!

Cheers,

C.

cjeanneret commented 9 years ago

Already back with information :)

[14-Feb-2015 09:19:38 +0100]: <…> PHP Error: Request security check failed (POST /?_task=mail&_action=) 

And this is created once I submit the gauth code.

Hope this helps.

Cheers,

C.

alexandregz commented 9 years ago

Hello:

I think that points to "dev-master" is correct. Other branchs have same "issue", dev- before branch name, eg. dev-samefield, dev-log, etc.

I'm going to log with level 1, but I can't install plugin with composer and plugins.roundcube.net utilities (manually)

alexandregz commented 9 years ago

I can confirm your issue, plugin doesn't works with 1.1.0 because make a CSRF error

I'm working on resolve this.

cjeanneret commented 9 years ago

Thank you for your time :).

alexandregz commented 9 years ago

It's a weird issue. First time plugin works fine, without CSRF error. I'm checking now, with a clean instalation and all it's ok.

I'm working on resolve this (II)

cjeanneret commented 9 years ago

Hello,

Any news? Let me know if I can help — I have "some" basis in PHP and such ;).

Cheers,

C.

alexandregz commented 9 years ago

Hello:

Nothing new about. At home, error appears; at work, all is fine (MAMP, similar php versions, same installation mode), so I can't find explication about that. Working...

skorvek commented 9 years ago

I'm still having this problem as well... disabling the plugin gets me back into my mail, but of course without any OTP protection. I'm on the master branch of the plugin, using the latest release of RoundCube (1.1 via git, fully patched)

PHP Error: Request security check failed Warning: Cannot modify header information - headers already sent in /var/www/roundcubemail/plugins/twofactor_gauthenticator/twofactor_gauthenticator.php on line 325

alexandregz commented 9 years ago

Yep, I know, sorry. I have a little idea about where is the problem and what, but nothing about how to resolve simple :-/

alexandregz commented 9 years ago

Hello:

I tried now, with clean installation downloaded from roundcube.net, last version (1.1.1), and plugin works fine. I can reproduce the issue but not now.

Are you both with same problem? And with upgrading RC soft?

Thx for feedback!

skorvek commented 9 years ago

I've still got the problem- I am using git release-1.1 branch. Did a pull today and no change.

skorvek commented 9 years ago

As an update- I disabled every plain except yours- same issue. I'm tracking the 1.1 release without any local modifications.

skorvek commented 9 years ago

plugin- sorry (autocorrect)

alexandregz commented 9 years ago

Hiya:

I created https://github.com/alexandregz/twofactor_gauthenticator/tree/skip-csrf branch, to explicit skip CSRF check, using authenticate hook from RC.

Can anyone check this, plz? Now, I can't reproduce error.

Thx all!

fsantiago07044 commented 9 years ago

doesn't work, just tested.same errors for me. and I just submitted ticket #33, apparently related to this.