Do passwords need to be null terminated?

[alanhe]

When password has more than 72 bytes, it is truncated and the assumption may not hold true.
I wonder if L109 is necessary.

rust-bcrypt/src/lib.rs

Lines 106 to 114 in b6af5e5

	// Passwords need to be null terminated
	let mut vec = Vec::with_capacity(password.len() + 1);
	vec.extend_from_slice(password);
	vec.push(0);
	// We only consider the first 72 chars; truncate if necessary.
	// `bcrypt` below will panic if len > 72
	let truncated = if vec.len() > 72 { &vec[..72] } else { &vec };
	let output = bcrypt::bcrypt(cost, salt, truncated);

[Keats]

I'm not sure tbh, we should see how other implementations handle it. From what I remember, it should be 71 bytes + 0 so yeah it might not be correct.

[alanhe]

By the Go implementation, I think it's 72 bytes + 0

• Reject inputs longer than 72 bytes (so the max size allowed is 72): https://cs.opensource.google/go/x/crypto/+/master:bcrypt/bcrypt.go;l=95
• Append 0: https://cs.opensource.google/go/x/crypto/+/master:bcrypt/bcrypt.go;l=226;drc=310bfa40f1e490c722a9c86d8ede143d3d506be5

// Bug compatibility with C bcrypt implementations. They use the trailing
// NULL in the key string during expansion.
// We copy the key to prevent changing the underlying array.
ckey := append(key[:len(key):len(key)], 0)

Same as this C++ implementation, https://github.com/kelektiv/node.bcrypt.js/blob/master/src/bcrypt.cc#L220-L224

---

It's not bothering me -- just out of curiosity 😄

[Keats]

I guess it's an issue in this crate then, the push should be after the truncation. I guess no one really encode things that are more than 72 bytes in practice.

[Keats]

I guess we should hash a password from the C++/Go implementation that has more than 72 bytes and ensure we can verify them