From 1e69a241b22d26efd3a9d9ca37fd7cefab1a173c Mon Sep 17 00:00:00 2001
From: Dmitry Ivanov <ivadmi5@gmail.com>
Date: Thu, 30 Nov 2023 19:21:38 +0300
Subject: [PATCH] fix(auth): check if user exists in auth attempts tracker
This patch fixes a possible DOS by checking whether the user exists
before creating a record for its unsuccessful auth attempt. This is
due to the fact that `box.session.on_auth` will be called even for
unknown users (this helps vanilla users log such events).
---
src/lib.rs | 22 ++++++++++++++++++++++
test/int/test_acl.py | 8 ++++----
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/src/lib.rs b/src/lib.rs
index 3d1ddbe441..e312d5440d 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -292,6 +292,7 @@ fn set_login_attempts_check(storage: Clusterwide) {
enum Verdict {
AuthOk,
AuthFail,
+ UnknownUser,
UserBlocked,
}
@@ -315,6 +316,19 @@ fn set_login_attempts_check(storage: Clusterwide) {
.expect("accessing storage should not fail")
};
+ let user_exists = storage
+ .users
+ .by_name(&user)
+ .expect("accessing storage should not fail")
+ .is_some();
+
+ // Prevent DOS attacks by first checking whether the user exists.
+ // If it doesn't, we shouldn't even bother tracking its attempts.
+ // Too many hashmap records will cause a global OOM event.
+ if !user_exists {
+ return Verdict::UnknownUser;
+ }
+
match attempts.entry(user) {
Entry::Occupied(e) if *e.get() >= max_login_attempts() => {
// The account is suspended until restart
@@ -365,6 +379,14 @@ fn set_login_attempts_check(storage: Clusterwide) {
user: &user,
);
}
+ Verdict::UnknownUser => {
+ crate::audit!(
+ message: "failed to authenticate unknown user `{user}`",
+ title: "auth_fail",
+ severity: High,
+ user: &user,
+ );
+ }
Verdict::UserBlocked => {
crate::audit!(
message: "failed to authenticate user `{user}`",
diff --git a/test/int/test_acl.py b/test/int/test_acl.py
index 88db16da3c..89515fe320 100644
--- a/test/int/test_acl.py
+++ b/test/int/test_acl.py
@@ -47,14 +47,14 @@ def test_max_login_attempts(cluster: Cluster):
c.eval(
"""
box.session.su(1)
- box.schema.user.create('foo', {password='bar'})
- box.schema.user.grant('foo', 'read,write,execute', 'universe')
+ pico.create_user('foo', '12345678')
+ pico.grant_privilege('foo', 'execute', 'universe')
"""
)
c.close()
# First login is successful
- c = connect(i1, user="foo", password="bar")
+ c = connect(i1, user="foo", password="12345678")
assert c
# Several failed login attempts but one less than maximum
@@ -69,7 +69,7 @@ def test_max_login_attempts(cluster: Cluster):
c = connect(
i1,
user="foo",
- password="bar",
+ password="12345678",
)
assert c
--
GitLab