Currently if passwords are guessed incorrectly, normal flood control kicks in and OTP doesn't leak any additional information about correct/incorrect guesses. Do we need flood control for correct password guesses with incorrect TFA codes?

CommentFileSizeAuthor
#8 2922056-8.patch6.8 KBsam152
#6 2922056-6.patch5.11 KBsam152

Comments

Sam152 created an issue. See original summary.

sam152’s picture

sam152 commented

I think we should register a hit against "user.failed_login_user" when the OTP is incorrect and let the user login form protect against further login attempts.

sam152’s picture

sam152 commented

Doing #2 properly looks pretty complicated.

sam152’s picture

sam152 commented
Issue tags: +Security
sam152’s picture

sam152 commented
Title: Do we need flood control for incorrect TFA codes? » Add flood control for incorrect TFA codes
sam152’s picture

sam152 commented
Status: Active » Needs review
StatusFileSize
new 2922056-6.patch5.11 KB
sam152’s picture

sam152 commented 
+++ b/src/UserLoginEnforce.php
@@ -46,6 +66,24 @@ class UserLoginEnforce {
+    // Always register a hit against the flood tables.
+    $flood->register(static::FLOOD_NAME_UID, static::FLOOD_WINDOW, $uid);
+    $flood->register(static::FLOOD_NAME_IP, static::FLOOD_WINDOW, $ip_address);

I'm not sure this is correct. In theory you could login and logout with all correct credentials 5 times in an hour and hit the flood control.

sam152’s picture

sam152 commented
StatusFileSize
new 2922056-8.patch6.8 KB

Fixing #7.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented 
+++ b/src/UserLoginEnforce.php
@@ -46,6 +66,20 @@ class UserLoginEnforce {
+    $flood = \Drupal::service('flood');
+    $ip_address = \Drupal::requestStack()->getCurrentRequest()->getClientIp();

Can we inject these services?

sam152’s picture

sam152 commented

Fraid not, the method is a form validation callback so it needs to be static.

  • Sam152 committed e887719 on 8.x-1.x 
    Issue #2922056 by Sam152: Add flood control for incorrect TFA codes
sam152’s picture

sam152 commented
sam152’s picture

sam152 commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.