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

created separate perms for windows so that windows users will no longer experience permission denied issues #155

Merged
merged 4 commits into from
May 17, 2022

Conversation

isemaya-square
Copy link
Contributor

@isemaya-square isemaya-square commented May 16, 2022

Addressing #144

TL;DR windows does not allow 440 permissions (which were being set for key files in certstap). In reality, the key files were being given 444 permissions.

However, this was previously allowed because file permissions checking in certstrap was flawed - it was allowing files with looser permissions than expected to be used. This was fixed in #141. However, the side effect of this fix was that certstrap broke in windows due to 444 (the actual key file permissions) being looser than 440. This PR fixes this by setting special leaf perms to be 444 for windows only.

Copy link
Contributor

@jdtw jdtw left a comment

Choose a reason for hiding this comment

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

Test this on Windows if you can?

isemaya-square and others added 3 commits May 16, 2022 13:48
Adding to run tests in windows environment
Removed lint-windows because code doesn't change between platforms, so we only need one linter
@isemaya-square isemaya-square mentioned this pull request May 17, 2022
@isemaya-square isemaya-square merged commit 4b5cbd1 into master May 17, 2022
@isemaya-square isemaya-square deleted the isemaya/certstrap-special-windows-permissions branch May 17, 2022 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants