< dongcarl>
Any reason why `bitcoin/share/rpcauth/rpcauth.py` generates a 16 byte salt instead of a 32 byte salt? I believe RFC 2104 recommends the key length to be larger than or equal to the hash output length: https://tools.ietf.org/html/rfc2104#section-3
< dongcarl>
It also seems that the format string will only output one character if the randomly generated byte is below 0x10, which I don't think is the expected behavior...
< sipa>
dongcarl: you don't need an HMAC here
< sipa>
or ratherx you don't actually need a MAC here
< sipa>
just a hash with some protection against dictionaries
< sipa>
so making the salt large enough to make a dictionary attack infeasible is enough
< sipa>
64 bits probably suffices
< dongcarl>
sipa: right, but it wouldn't hurt to have a few more bits of entropy and conform to the spec?
< dongcarl>
Also with the format string bug we have less than 64 bits at times
< dongcarl>
That format string will only output 1 character if the r in salt_sequence is less than 0x10
< sipa>
nice catch
< sipa>
but it's not a problem; the set of encoded salts still has 128 bits of entropy
< sipa>
oh, no!
< sipa>
slightly less
< dongcarl>
Right
< sipa>
there are collisions
< dongcarl>
I'll PR to fix it to make it according to normal expectations
< sipa>
dongcarl: i don't think "comforming to the spec" matters here; the spec is to satsify the requirements of a secure MAaC; we don't need a secure MAC
< sipa>
but yes, please fix the formatting issue
< dongcarl>
Okay gotcha
< jarthur>
Wait, why slightly less than 128? If r is more than 0x10, don't we get more than 1 character?
< sipa>
the number of characters doesn't matter
< sipa>
the number of combinations matters
< sipa>
but given that there are distinct inputs that map to the same salt string (e.g. (0x1, 0x23, 0x45) and (0x12, 0x34, 0x5) both map to 12345)
< jarthur>
Makes sense now. Thanks :)
< jarthur>
The format line could just be replaced by using binascii.hexlify, that way we'll get two-digit hexes as ASCII bytes and won't need the later UTF-8 encodes. Would just need to use salt.decode('ascii') in the printed string.
< dongcarl>
jarthur: I just did os.random(16).hex()
< dongcarl>
works fine
< jarthur>
I like that even better
< jarthur>
nice
< bitcoin-git>
[bitcoin] dongcarl opened pull request #14742: Properly generate salt in rpcauth.py (master...2018-11-fix-rpcauth-salt) https://github.com/bitcoin/bitcoin/pull/14742
< sipa>
dongcarl: do we require python 3.6?
< dongcarl>
sipa: I think we require python at least
< jarthur>
dongcarl: yea, though I'm kind of lumping 3.5 and 3.6 together since the latter polished a lot of the 3.5 stuff. f-strings, async iteration and generation, optional typing, efficient dicts, path-like objects
< meshcollider>
sipa: if it's secret, how do you know about it
< bitcoin-git>
[bitcoin] vim88 opened pull request #14749: Refactor: Changes postincrement to preincrement in for loop in src/consensus files (master...postincrement_to_preincrement_src_consensus) https://github.com/bitcoin/bitcoin/pull/14749
< bitcoin-git>
[bitcoin] vim88 closed pull request #14749: Refactor: Changes postincrement to preincrement in for loop in src/consensus files (master...postincrement_to_preincrement_src_consensus) https://github.com/bitcoin/bitcoin/pull/14749