Raise ValueError if password is longer than 72 bytes (#1000)

* move existing tests for long pws into a separate test to assert ValueError is raised

* add more tests for edge cases around the 72 byte length

* raise ValuError if the password passed to hashpw() is longer than 72 bytes

* improve error message

* update test_2a_wraparound_bug

* remove obsolete re-assignment of `password`

* remove obsolete tests

the "raise on passwords longer than 72 chars" behavior is already covered in `test_hashpw_raises_correctly_for_long_passwords`,
and the `test_2a_wraparound_bug` is not relevant anymore (this previously ensured truncation, which we do not do anymore)

src/_bcrypt/src/lib.rs

@@ -77,7 +77,16 @@ fn hashpw<'p>(
     // bytes on the updated prefix $2b$, but leaving $2a$ unchanged for
     // compatibility. However, pyca/bcrypt 2.0.0 *did* correctly truncate inputs
     // on $2a$, so we do it here to preserve compatibility with 2.0.0
-    let password = &password[..password.len().min(72)];
+    // Silent truncation is _probably_ not the best idea, even if the "original"
+    // OpenBSD implementation did/does this.
+    // We prefer to raise a ValueError in this case - if the user _wants_ to truncate,
+    // they can always do so manually by passing s[:72] instead of s into hashpw().
+
+    if password.len() > 72 {
+        return Err(pyo3::exceptions::PyValueError::new_err(
+            "password cannot be longer than 72 bytes, truncate manually if necessary (e.g. my_password[:72])",
+        ));
+    }
 
     // salt here is not just the salt bytes, but rather an encoded value
     // containing a version number, number of rounds, and the salt.

tests/test_bcrypt.py

@@ -121,24 +121,6 @@ _test_vectors = [
         b"$2a$05$XXXXXXXXXXXXXXXXXXXXXO",
         b"$2a$05$XXXXXXXXXXXXXXXXXXXXXOAcXxm9kjPGEMsLznoKqmqw7tc8WCx4a",
     ),
-    (
-        b"0123456789abcdefghijklmnopqrstuvwxyz"
-        b"ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
-        b"chars after 72 are ignored",
-        b"$2a$05$abcdefghijklmnopqrstuu",
-        b"$2a$05$abcdefghijklmnopqrstuu5s2v8.iXieOjg/.AySBTTZIIVFJeBui",
-    ),
-    (
-        b"\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa"
-        b"\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa"
-        b"\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa"
-        b"\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa"
-        b"\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa"
-        b"\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa"
-        b"chars after 72 are ignored as usual",
-        b"$2a$05$/OK.fbVrR/bpIqNJ5ianF.",
-        b"$2a$05$/OK.fbVrR/bpIqNJ5ianF.swQOIzjOiJ9GHEPuhEkvqrUyvWhEMx6",
-    ),
     (
         b"\xa3",
         b"$2a$05$/OK.fbVrR/bpIqNJ5ianF.",
@@ -252,6 +234,25 @@ def test_checkpw_2y_prefix(password, hashed, expected):
     assert bcrypt.checkpw(password, hashed) is True
 
 
+@pytest.mark.parametrize(
+    ("pw_length", "should_raise"),
+    [
+        (71, False),
+        (72, False),
+        (73, True),
+    ],
+)
+def test_hashpw_raises_correctly_for_long_passwords(pw_length, should_raise):
+    password = b"\xaa" * pw_length
+    salt = b"$2b$04$xnFVhJsTzsFBTeP3PpgbMe"
+
+    if should_raise:
+        with pytest.raises(ValueError):
+            bcrypt.hashpw(password, salt)
+    else:
+        bcrypt.hashpw(password, salt)
+
+
 def test_hashpw_invalid():
     with pytest.raises(ValueError):
         bcrypt.hashpw(b"password", b"$2z$04$cVWp4XaNU8a4v1uMRum2SO")
@@ -490,15 +491,6 @@ def test_invalid_params(password, salt, desired_key_bytes, rounds, error):
         bcrypt.kdf(password, salt, desired_key_bytes, rounds)
 
 
-def test_2a_wraparound_bug():
-    assert (
-        bcrypt.hashpw(
-            (b"0123456789" * 26)[:255], b"$2a$04$R1lJ2gkNaoPGdafE.H.16."
-        )
-        == b"$2a$04$R1lJ2gkNaoPGdafE.H.16.1MKHPvmKwryeulRe225LKProWYwt9Oi"
-    )
-
-
 def test_multithreading():
     def create_user(pw):
         salt = bcrypt.gensalt(4)