Skip to content

Fix chmod race condition when generating key#48141

Merged
guilleiguaran merged 1 commit into
rails:mainfrom
skipkayhil:hm-fix-chmod-race
May 11, 2023
Merged

Fix chmod race condition when generating key#48141
guilleiguaran merged 1 commit into
rails:mainfrom
skipkayhil:hm-fix-chmod-race

Conversation

@skipkayhil

@skipkayhil skipkayhil commented May 5, 2023

Copy link
Copy Markdown
Member

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
Copy Markdown
Contributor

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

@guilleiguaran

Copy link
Copy Markdown
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 skipkayhil force-pushed the hm-fix-chmod-race branch from 4ce213f to 3ae8f4d Compare May 11, 2023 20:30
@skipkayhil

Copy link
Copy Markdown
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
@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