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

Generate safe IVs #51086

Merged
merged 1 commit into from Jun 16, 2023
Merged

Conversation

aiven-sal
Copy link
Contributor

IVs should never be re-used when using CTR.
The previous implementation had a 50% probability to generate an IV that was already used after 65536 generations. This happens because std::mt19937 is seeded using a 32bit integer and returns a 32 bits integer.
Because of the birthday problem, collisions have a 50% chance after only 2^16 IV's generations.
CH uses one IV per file and, for most use cases, 65k files are not as many as they seem.
Also, the initial entropy is gathered using std::random_device which is "best-effort" and it's allowed by the standard to return a fixed sequence of numbers.

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Fixed not-so-random IV generation for CTR disk encryption.

@alexey-milovidov

IVs should never be re-used when using CTR.
The previous implementation had a 50% probability to generate
an IV that was already used after 65536 generations.
This happens because std::mt19937 is seeded using a 32bit integer
and returns a 32 bits integer.
Because of the birthday problem, collisions have a 50% chance after
only 2^16 IV's generations.
CH uses one IV per file and, for most use cases, 65k files are
not as many as they seem.
Also, the initial entropy is gathered using std::random_device
which is "best-effort" and it's allowed by the standard to
return a fixed sequence of numbers.
@alexey-milovidov alexey-milovidov merged commit 5752ad2 into ClickHouse:master Jun 16, 2023
@alexey-milovidov alexey-milovidov added the pr-must-backport Pull request should be backported intentionally. Use this label with great care! label Jun 16, 2023
@alexey-milovidov alexey-milovidov self-assigned this Jun 16, 2023
robot-clickhouse added a commit that referenced this pull request Jun 16, 2023
robot-clickhouse added a commit that referenced this pull request Jun 16, 2023
robot-clickhouse added a commit that referenced this pull request Jun 16, 2023
robot-clickhouse added a commit that referenced this pull request Jun 16, 2023
@robot-ch-test-poll3 robot-ch-test-poll3 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Jun 16, 2023
alexey-milovidov added a commit that referenced this pull request Jun 16, 2023
alexey-milovidov added a commit that referenced this pull request Jun 16, 2023
alexey-milovidov added a commit that referenced this pull request Jun 16, 2023
alexey-milovidov added a commit that referenced this pull request Jun 16, 2023
@aiven-sal aiven-sal deleted the aiven-sal/ctriv branch June 21, 2023 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-backports-created-cloud pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-must-backport-cloud
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants