compute_x does not use leading zeroes from salt parameter

[username223]

When converting Botan::BigInts back to bytes, the leading zeroes present in the original value are not preserved. This leads the compute_x function to mistakenly use a 31 byte salt value instead of a 32 byte one.

From experimentation, the client appears to use leading zeroes for the computation of M1, meaning that users given a salt with leading zeroes will not be able to log in.
It does not appear possible to inform the client that the salt is only 31 bytes instead of 32, which means that it will always use any potential missing zeroes.

The following tests shows the problem:

TEST(srp6regression, Test) {
	//
    std::string username = "USERNAME123";
    std::string password = "PASSWORD123";

    // Should be a 32 byte value
    Botan::BigInt salt_leading_zeroes(std::string("0x00DECCE60EE2BE6BA4DC6FEDB99E66FFEBDE360F0BE2CEFA984E4CA3E5402CA5"));
    // Should be a 31 byte value
    Botan::BigInt salt_no_leading_zeroes(std::string("0xDECCE60EE2BE6BA4DC6FEDB99E66FFEBDE360F0BE2CEFA984E4CA3E5402CA5"));

    Botan::BigInt x_leading = srp::detail::compute_x(username, password, salt_leading_zeroes, srp::Compliance::GAME);
    Botan::BigInt x_no_leading = srp::detail::compute_x(username, password, salt_no_leading_zeroes, srp::Compliance::GAME);

    // Passes, both are 31 byte values
    ASSERT_EQ(x_leading, x_no_leading);
}

From experience, changing the following in compute_x seems to be consistent with the client:

if(mode == Compliance::RFC5054) {
	hasher->update(Botan::BigInt::encode(salt));
} else {
	const auto& salt_enc = detail::encode_flip(salt);
	hasher->update(salt_enc.data(), salt_enc.size());
}

to

if(mode == Compliance::RFC5054) {
	hasher->update(Botan::BigInt::encode(salt));
} else {
	// Probably make the magic 32 derived from something else
	const auto& salt_enc = detail::encode_flip_1363(salt, 32);
	hasher->update(salt_enc.data(), salt_enc.size());
}

[Chaosvex]

Good catch, thanks. I'll change the way the salt is passed around.