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

Fix chmod race condition when generating key #48141

Merged
merged 1 commit into from
May 11, 2023

Conversation

skipkayhil
Copy link
Member

@skipkayhil skipkayhil commented May 5, 2023

Motivation / Background

Fixes #48135

Encrypted keys were updated previously to restrict other users from reading the file by default. However, there is a brief period of time between an encrypted key being created and its permissions being set to 0600. This means that it is possible for another user to read that file during that time.

Detail

This commit fixes that issue by setting the desired permissions when the file is created. The ability to use the perm option was added in Thor 1.2.2 so the minimum version was updated in the Railties gemspec.

Additional information

I've pointed this at my branch of Thor to show that rails/thor#820 would enable fixing this, but that would have to be changed to merge this. Done!

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@rails-bot rails-bot bot added the railties label May 5, 2023
@casperisfine
Copy link
Contributor

👍 let's the the Thor feature merged and released, and then we can merge this.

@guilleiguaran
Copy link
Member

@skipkayhil please update this PR to use thor main

Encrypted keys were updated [previously][1] to restrict other users from
reading the file by default. However, there is a brief period of time
between an encrypted key being created and its permissions being set to
0600. This means that it is possible for another user to read that file
during that time.

This commit fixes that issue by setting the desired permissions when the
file is created. The ability to use the `perm` option was added in Thor
1.2.2 so the minimum version was updated in the Railties gemspec.

[1]: 4c6c357
@skipkayhil
Copy link
Member Author

Thank you all for the reviews, I've updated the PR to require the newly released Thor 1.2.2 🙇

@guilleiguaran guilleiguaran merged commit d954155 into rails:main May 11, 2023
9 checks passed
@skipkayhil skipkayhil deleted the hm-fix-chmod-race branch May 11, 2023 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Race condition with chmod when creating master.key file
3 participants