Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly generate salt in rpcauth.py #14742

Merged
merged 1 commit into from Nov 21, 2018

Conversation

Projects
None yet
7 participants
@dongcarl
Copy link
Contributor

commented Nov 17, 2018

Previously, when iterating over bytes of the generated salt to construct
a hex string, only one character would be outputted when the byte is
less than 0x10. Meaning that for a 16 byte salt, the hex string might be
less than 32 characters and collisions would occur.

@JustinTArthur

This comment has been minimized.

Copy link
Contributor

commented Nov 17, 2018

ACK e7414b4a6c10ab2c17714929286f43559965607d

@luke-jr

This comment has been minimized.

Copy link
Member

commented Nov 17, 2018

Apparently this requires Python 3.6, whereas we support Python 3.4 (held back for RHEL I guess)

@dongcarl dongcarl force-pushed the dongcarl:2018-11-fix-rpcauth-salt branch Nov 17, 2018

Properly generate salt in rpcauth.py, update tests
Previously, when iterating over bytes of the generated salt to construct
a hex string, only one character would be outputted when the byte is
less than 0x10. Meaning that for a 16 byte salt, the hex string might be
less than 32 characters and collisions would occur.

@dongcarl dongcarl force-pushed the dongcarl:2018-11-fix-rpcauth-salt branch to 6be7d14 Nov 17, 2018

@dongcarl

This comment has been minimized.

Copy link
Contributor Author

commented Nov 17, 2018

Fixed, works with Python 3.4 now. Does anyone know if we have integration tests that use this?

@bitcoin bitcoin deleted a comment from bob1975 Nov 17, 2018

@promag

This comment has been minimized.

Copy link
Member

commented Nov 19, 2018

Fixed, works with Python 3.4 now. Does anyone know if we have integration tests that use this?

I don't think so.

@promag

This comment has been minimized.

Copy link
Member

commented Nov 19, 2018

ACK 6be7d14.

@laanwj laanwj merged commit 6be7d14 into bitcoin:master Nov 21, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Nov 21, 2018

Merge #14742: Properly generate salt in rpcauth.py
6be7d14 Properly generate salt in rpcauth.py, update tests (Carl Dong)

Pull request description:

  Previously, when iterating over bytes of the generated salt to construct
  a hex string, only one character would be outputted when the byte is
  less than 0x10. Meaning that for a 16 byte salt, the hex string might be
  less than 32 characters and collisions would occur.

Tree-SHA512: 7038ecbbac846cd1851112396acd8a04475685f5b6f786e4e7316acba4a56cc711c275b7f52f0f2b6bc6cfdc0c0d9d39c3afeb2c0aff3a30fde516bf642fdf9f
@jnewbery

This comment has been minimized.

Copy link
Member

commented Nov 21, 2018

Does anyone know if we have integration tests that use this?

Do you mean that use rpcauth.py? Yes:

p = subprocess.Popen([sys.executable, gen_rpcauth, self.user], stdout=subprocess.PIPE, universal_newlines=True)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.