diff --git a/litellm/proxy/auth/password_policy.py b/litellm/proxy/auth/password_policy.py index 21c93a42c76..a40fa109e82 100644 --- a/litellm/proxy/auth/password_policy.py +++ b/litellm/proxy/auth/password_policy.py @@ -145,9 +145,13 @@ async def _is_password_breached(password: str, client: AsyncHTTPHandler) -> bool def is_breach_check_enabled(general_settings: Mapping[str, object]) -> bool: - if openssl_enforces_fips(): + enabled: Final = ( + not is_offline_mode() and general_settings.get("password_policy_check_breached_passwords", True) is not False + ) + if enabled and openssl_enforces_fips(): + verbose_proxy_logger.warning("FIPS mode skips the breached password check because its lookup needs SHA-1.") return False - return not is_offline_mode() and general_settings.get("password_policy_check_breached_passwords", True) is not False + return enabled async def is_password_breached( diff --git a/litellm/proxy/utils.py b/litellm/proxy/utils.py index ba50bc1e971..03b82658ec2 100644 --- a/litellm/proxy/utils.py +++ b/litellm/proxy/utils.py @@ -8403,6 +8403,10 @@ def hash_token(token: str): return hashed_token +def _is_pbkdf2_row(stored: str) -> bool: + return re.fullmatch(r"pbkdf2:[A-Za-z0-9+/]{64}", stored) is not None + + def hash_password(password: str) -> str: """Hash a password using scrypt with a random salt, or PBKDF2 in FIPS mode.""" import base64 @@ -8425,7 +8429,7 @@ def verify_password(password: str, stored: str) -> bool: import secrets if openssl_enforces_fips(): - if re.fullmatch(r"pbkdf2:[A-Za-z0-9+/]{64}", stored) is None: + if not _is_pbkdf2_row(stored): verbose_proxy_logger.error("FIPS mode refuses a non-PBKDF2 password row. Reset it with /user/update.") return False if len(password) < MIN_ALLOWED_LENGTH: @@ -8477,7 +8481,10 @@ async def migrate_passwords_to_scrypt_async(prisma_client) -> str: plaintext_users: Final = [ (u.user_id, u.password) for u in all_with_pw - if u.password and not u.password.startswith("scrypt:") and not _is_sha256_hex(u.password) + if u.password + and not u.password.startswith("scrypt:") + and not _is_sha256_hex(u.password) + and not _is_pbkdf2_row(u.password) ] if not plaintext_users: return "No plaintext passwords found" diff --git a/tests/unit/proxy/auth/test_login_utils.py b/tests/unit/proxy/auth/test_login_utils.py index ccfffaf3999..a74a0ff0641 100644 --- a/tests/unit/proxy/auth/test_login_utils.py +++ b/tests/unit/proxy/auth/test_login_utils.py @@ -2386,14 +2386,14 @@ _SCRYPT_ROW: Final = "scrypt:AAECAwQFBgcICQoLDA0OD7hIwlM/k8ytnffjFDt6AuO+BvqtmZv async def test_login_rehash_rewrites_only_a_legacy_row(stored: str, schemes_written: list[str]) -> None: from litellm.proxy.auth.login_utils import _rehash_password_if_needed - prisma = MagicMock() + prisma: Final = MagicMock() prisma.db.litellm_usertable.update = AsyncMock() with patch( # test-quality-ok: the rehash reads proxy_server's prisma_client global; no injection seam "litellm.proxy.proxy_server.prisma_client", prisma ): await _rehash_password_if_needed("u-1", _PBKDF2_PASSWORD, stored) - updates = prisma.db.litellm_usertable.update.await_args_list + updates: Final = prisma.db.litellm_usertable.update.await_args_list assert [update.kwargs["data"]["password"].split(":")[0] for update in updates] == schemes_written @@ -2405,7 +2405,7 @@ async def test_fips_login_runs_one_pbkdf2_off_the_event_loop_and_writes_no_row( from litellm.proxy.auth.login_utils import authenticate_user monkeypatch.setenv("DATABASE_URL", "postgresql://stub") - repository = _known_user("user@corp.com") + repository: Final = _known_user("user@corp.com") repository.return_value.table.find_first.return_value.password = _PBKDF2_ROW repository.return_value.table.update = AsyncMock() with ( @@ -2419,7 +2419,7 @@ async def test_fips_login_runs_one_pbkdf2_off_the_event_loop_and_writes_no_row( "litellm.proxy.auth.login_utils.generate_key_helper_fn", new=AsyncMock(return_value={"token": "sk-ui"}) ), ): - result = await authenticate_user( + result: Final = await authenticate_user( username="user@corp.com", password=_PBKDF2_PASSWORD, master_key="sk-master", @@ -2436,9 +2436,9 @@ async def test_fips_login_runs_one_pbkdf2_off_the_event_loop_and_writes_no_row( async def _login_whose_user_gets_blocked_during_the_row_lookup(monkeypatch: pytest.MonkeyPatch, row: str): from litellm.proxy.auth.login_utils import authenticate_user - throttle = _throttle(user_limit=1) - repository = _known_user() - user = repository.return_value.table.find_first.return_value + throttle: Final = _throttle(user_limit=1) + repository: Final = _known_user() + user: Final = repository.return_value.table.find_first.return_value user.password = row async def block_the_user_then_return_the_row(**_: object) -> MagicMock: @@ -2491,13 +2491,13 @@ async def test_fips_burst_of_wrong_passwords_stops_deriving_once_its_user_is_blo from litellm.proxy.auth.login_utils import authenticate_user monkeypatch.setenv("DATABASE_URL", "postgresql://stub") - repository = _known_user() + repository: Final = _known_user() repository.return_value.table.find_first.return_value.password = _PBKDF2_ROW - throttle = _throttle(user_limit=1) + throttle: Final = _throttle(user_limit=1) with patch( # test-quality-ok: the user lookup is the database boundary "litellm.proxy.auth.login_utils.UserRepository", repository ): - refusals = await asyncio.gather( + refusals: Final = await asyncio.gather( *( authenticate_user( username="known@example.com", @@ -2520,6 +2520,6 @@ async def test_fips_burst_of_wrong_passwords_stops_deriving_once_its_user_is_blo async def test_standard_login_past_the_throttle_keeps_its_answer_when_its_user_gets_blocked( monkeypatch: pytest.MonkeyPatch, ) -> None: - result = await _login_whose_user_gets_blocked_during_the_row_lookup(monkeypatch, _SCRYPT_ROW) + result: Final = await _login_whose_user_gets_blocked_during_the_row_lookup(monkeypatch, _SCRYPT_ROW) assert result.key == "sk-ui" diff --git a/tests/unit/proxy/auth/test_password_policy.py b/tests/unit/proxy/auth/test_password_policy.py index 561422c380e..301abe3effd 100644 --- a/tests/unit/proxy/auth/test_password_policy.py +++ b/tests/unit/proxy/auth/test_password_policy.py @@ -9,6 +9,7 @@ httpx.MockTransport, so no network is touched and nothing is monkeypatched. import asyncio import hashlib +from typing import Final import httpx import pytest @@ -360,6 +361,17 @@ async def test_validate_passwords_bulk_empty_batch_makes_no_lookups(): @pytest.mark.asyncio @pytest.mark.usefixtures("fips_openssl") -async def test_fips_mode_skips_the_breach_check() -> None: - client_says_breached = _client_returning(f"{_sha1_upper(STRONG_PASSWORD)[5:]}:1387") - assert await is_password_breached(STRONG_PASSWORD, {}, client_says_breached) is False +async def test_fips_mode_skips_the_breach_check_with_a_warning(caplog: pytest.LogCaptureFixture) -> None: + client_says_breached: Final = _client_returning(f"{_sha1_upper(STRONG_PASSWORD)[5:]}:1387") + with caplog.at_level("WARNING", logger="LiteLLM Proxy"): + assert await is_password_breached(STRONG_PASSWORD, {}, client_says_breached) is False + assert [record.levelname for record in caplog.records] == ["WARNING"] + + +@pytest.mark.asyncio +@pytest.mark.usefixtures("fips_openssl") +async def test_fips_mode_stays_quiet_when_the_breach_check_is_turned_off(caplog: pytest.LogCaptureFixture) -> None: + settings: Final = {"password_policy_check_breached_passwords": False} + with caplog.at_level("WARNING", logger="LiteLLM Proxy"): + assert await is_password_breached(STRONG_PASSWORD, settings, _client_never_called()) is False + assert caplog.records == [] diff --git a/tests/unit/proxy/management_endpoints/test_password_endpoints.py b/tests/unit/proxy/management_endpoints/test_password_endpoints.py index fd66419d28e..30c299a11ee 100644 --- a/tests/unit/proxy/management_endpoints/test_password_endpoints.py +++ b/tests/unit/proxy/management_endpoints/test_password_endpoints.py @@ -497,7 +497,9 @@ async def test_change_password_requires_db(): async def test_fips_password_change_runs_both_pbkdf2_off_the_event_loop(pbkdf2_on_event_loop: list[bool]) -> None: from litellm.proxy._types import ChangePasswordRequest - prisma = _make_prisma(_make_user_row("pbkdf2:AAECAwQFBgcICQoLDA0OD+uxhNqRo/3ZFLEaKARqFBT+N3Kqek/SGM/5AvocSN0T")) + prisma: Final = _make_prisma( + _make_user_row("pbkdf2:AAECAwQFBgcICQoLDA0OD+uxhNqRo/3ZFLEaKARqFBT+N3Kqek/SGM/5AvocSN0T") + ) with ( patch( # test-quality-ok: change_password reads proxy_server module globals; no injection seam diff --git a/tests/unit/proxy/utils/prisma_and_spend/test_password_helpers.py b/tests/unit/proxy/utils/prisma_and_spend/test_password_helpers.py index f862b3413c3..cf79e30b3b3 100644 --- a/tests/unit/proxy/utils/prisma_and_spend/test_password_helpers.py +++ b/tests/unit/proxy/utils/prisma_and_spend/test_password_helpers.py @@ -239,14 +239,14 @@ def test_fips_hash_is_pbkdf2_sha256_at_600k_salted_by_openssl(monkeypatch: pytes @pytest.mark.usefixtures("fips_openssl") def test_fips_hash_salts_every_row_and_verifies() -> None: - first, second = hash_password(PASSWORD), hash_password(PASSWORD) - assert first != second + first: Final = hash_password(PASSWORD) + assert first != hash_password(PASSWORD) assert verify_password(PASSWORD, first) @pytest.mark.usefixtures("fips_openssl") def test_fips_verify_needs_the_right_password_of_at_least_eight_characters() -> None: - outcome = { + outcome: Final = { "right": verify_password(PASSWORD, PBKDF2_ROW), "wrong": verify_password("Seed-Lamp-4-Rivet", PBKDF2_ROW), "lone_surrogate": verify_password("\ud800" + PASSWORD, PBKDF2_ROW), @@ -307,7 +307,7 @@ def test_standard_mode_verifies_the_rows_it_always_did(row: str, verifies: bool) @pytest.mark.asyncio @pytest.mark.usefixtures("fips_openssl") async def test_fips_boot_migration_writes_no_row() -> None: - pc = MagicMock() + pc: Final = MagicMock() pc.db.litellm_usertable.find_many = AsyncMock( return_value=[_make_user("plaintext-user", PASSWORD), _make_user("fips-user", PBKDF2_ROW)] ) @@ -318,6 +318,20 @@ async def test_fips_boot_migration_writes_no_row() -> None: pc.db.litellm_usertable.update.assert_not_awaited() +@pytest.mark.asyncio +async def test_standard_boot_migration_hashes_a_plaintext_row_but_not_a_pbkdf2_row() -> None: + pc: Final = MagicMock() + pc.db.litellm_usertable.find_many = AsyncMock( + return_value=[_make_user("plaintext-user", "pbkdf2:plaintext"), _make_user("fips-user", PBKDF2_ROW)] + ) + pc.db.litellm_usertable.update = AsyncMock() + + await migrate_passwords_to_scrypt_async(pc) + + updated: Final = pc.db.litellm_usertable.update.await_args_list + assert [update.kwargs["where"]["user_id"] for update in updated] == ["plaintext-user"] + + def _on_event_loop() -> bool: return asyncio._get_running_loop() is not None @@ -329,13 +343,14 @@ async def test_password_kdf_runs_on_the_event_loop_in_standard_mode() -> None: @pytest.mark.asyncio @pytest.mark.usefixtures("fips_openssl") -async def test_password_kdfs_leave_the_event_loop_and_never_overlap_in_fips_mode() -> None: - second_started: Final = threading.Event() +async def test_password_kdfs_leave_the_event_loop_on_a_single_thread_in_fips_mode() -> None: + release_first: Final = threading.Event() + first_kdf: Final = asyncio.ensure_future(run_off_loop_if_fips(lambda: release_first.wait(timeout=5))) + second_kdf: Final = asyncio.ensure_future(run_off_loop_if_fips(_on_event_loop)) + await asyncio.sleep(0) + kdf_threads: Final = [thread for thread in threading.enumerate() if thread.name.startswith("fips-password-kdf")] + release_first.set() - def first() -> tuple[bool, bool]: - return _on_event_loop(), second_started.wait(timeout=0.2) - - (first_on_event_loop, overlapped), _ = await asyncio.gather( - run_off_loop_if_fips(first), run_off_loop_if_fips(second_started.set) - ) - assert (first_on_event_loop, overlapped) == (False, False) + assert len(kdf_threads) == 1 + assert await first_kdf is True + assert await second_kdf is False