Misleading prop250 log messages
There are various instances where we combine sr_commit_get_rsa_fpr()
with hex_str()
in log messages. This won't work because sr_commit_get_rsa_fpr()
actually calls hex_str()
underneath, and you can't have repeated calls to hex_str()
because it uses a stack buffer.
Examples:
log_debug(LD_DIR, "SR: Inspecting commit from %s (voter: %s)?",
sr_commit_get_rsa_fpr(commit),
hex_str(voter_key, DIGEST_LEN));
if (!commitments_are_the_same(commit, saved_commit)) {
log_warn(LD_DIR, "SR: Commit from authority %s is different from "
"previous commit in our state (voter: %s)",
sr_commit_get_rsa_fpr(commit),
hex_str(voter_key, DIGEST_LEN));
goto ignore;
}
{{{
if (verify_commit_and_reveal(commit) < 0) {
log_warn(LD_BUG, "SR:a Commit from authority %s has an invalid "
"reveal value. (voter: %s)",
sr_commit_get_rsa_fpr(commit),
hex_str(voter_key, DIGEST_LEN));
goto ignore;
}
}}}
I found this issue while auditing log messages that did not make sense in a Chutney network.
In general, it's naughty to hide a delicate function like hex_str()
behind other functions (like sr_commit_get_rsa_fpr()
) without making it clear that this is the case.
Fortunately, from what I see, this bug only occurs in logging cases. But we should make sure we don't have this sort of issue from now on.