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

Allow setting custom initialization vector for FileAccessEncrypted. Add export setting to set static seed for PCK encryption initialization vectors. #98918

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Nov 7, 2024

  • Allow setting custom initialization vector for FileAccessEncrypted.
  • Add export setting to set static seed for PCK encryption initialization vectors, If set to non-zero value seed + file hash are used to generate IV, if zero IV is fully random (old behavior).

Fixes #98904

@bruvzg bruvzg added this to the 4.4 milestone Nov 7, 2024
@bruvzg bruvzg force-pushed the pck_enc_iv branch 2 times, most recently from 8289472 to 7a31bb3 Compare November 12, 2024 06:39
@bruvzg bruvzg marked this pull request as ready for review November 12, 2024 08:03
@bruvzg bruvzg requested review from a team as code owners November 12, 2024 08:03
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from the compat method name, seems good to me.

Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't tested but looks good

@bruvzg bruvzg force-pushed the pck_enc_iv branch 2 times, most recently from 4306c06 to 839351d Compare November 12, 2024 11:23
@Repiteo
Copy link
Contributor

Repiteo commented Nov 12, 2024

Needs to be rebased before this can be merged

Comment on lines 52 to 58
if (p_iv.is_empty()) {
iv.resize(16);
for (int i = 0; i < 16; i++) {
iv.write[i] = Math::rand() % 256;
}
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know the full context here, but does the init-vector not need to be securely random? As repeated invocations of Math::rand() probably have rather low entropy...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think it matters in this case, but changed it to CryptoCore::RandomGenerator just in case.

…dd export setting to set static seed for PCK encryption initialization vectors.
@Repiteo Repiteo merged commit 1627912 into godotengine:master Nov 13, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 13, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Encrypted PCK generate different binaries for the same input
5 participants